Type ascription produces two incompatible fields (#7355)
# Symptoms This code produces a big ugly confusing error in the frontend, see #7340. # Root cause I added a new test case, with an unknown type. In `ast.snap` under `body[0].declaration.init.ty` there two different `type` fields in the AST node for the type's name, and they have conflicting values Primitive and Identifier. <img width="602" alt="Screenshot 2025-06-03 at 4 04 55 PM" src="https://github.com/user-attachments/assets/913a0fa0-3e8d-473f-bb64-003d44915be0" /> # Solution Change the `enum PrimitiveType` variant from `Named(Node<Identifier>)` to `Named { name: Node<Identifier> }` so that the fields nest differently. Now the error correctly points out to the user that the type `NotARealType` can't be found. Much better error message that shows the user the problem. # Alternative solutions Stop the duplicated JSON fields altogether. I tried this previously in https://github.com/KittyCAD/modeling-app/pull/4369 but it was very involved, and I didn't think it was worth it. Maybe I should reopen that PR and solve this properly. Closes #7340
This commit is contained in:
@ -187,7 +187,7 @@ impl RuntimeType {
|
|||||||
};
|
};
|
||||||
RuntimeType::Primitive(PrimitiveType::Number(ty))
|
RuntimeType::Primitive(PrimitiveType::Number(ty))
|
||||||
}
|
}
|
||||||
AstPrimitiveType::Named(name) => Self::from_alias(&name.name, exec_state, source_range)?,
|
AstPrimitiveType::Named { id } => Self::from_alias(&id.name, exec_state, source_range)?,
|
||||||
AstPrimitiveType::Tag => RuntimeType::Primitive(PrimitiveType::Tag),
|
AstPrimitiveType::Tag => RuntimeType::Primitive(PrimitiveType::Tag),
|
||||||
AstPrimitiveType::ImportedGeometry => RuntimeType::Primitive(PrimitiveType::ImportedGeometry),
|
AstPrimitiveType::ImportedGeometry => RuntimeType::Primitive(PrimitiveType::ImportedGeometry),
|
||||||
AstPrimitiveType::Function(_) => RuntimeType::Primitive(PrimitiveType::Function),
|
AstPrimitiveType::Function(_) => RuntimeType::Primitive(PrimitiveType::Function),
|
||||||
|
@ -228,7 +228,7 @@ impl PrimitiveType {
|
|||||||
let mut hasher = Sha256::new();
|
let mut hasher = Sha256::new();
|
||||||
match self {
|
match self {
|
||||||
PrimitiveType::Any => hasher.update(b"any"),
|
PrimitiveType::Any => hasher.update(b"any"),
|
||||||
PrimitiveType::Named(id) => hasher.update(id.compute_digest()),
|
PrimitiveType::Named { id } => hasher.update(id.compute_digest()),
|
||||||
PrimitiveType::String => hasher.update(b"string"),
|
PrimitiveType::String => hasher.update(b"string"),
|
||||||
PrimitiveType::Number(suffix) => hasher.update(suffix.digestable_id()),
|
PrimitiveType::Number(suffix) => hasher.update(suffix.digestable_id()),
|
||||||
PrimitiveType::Boolean => hasher.update(b"bool"),
|
PrimitiveType::Boolean => hasher.update(b"bool"),
|
||||||
|
@ -3230,7 +3230,7 @@ pub enum PrimitiveType {
|
|||||||
/// `fn`, type of functions.
|
/// `fn`, type of functions.
|
||||||
Function(FunctionType),
|
Function(FunctionType),
|
||||||
/// An identifier used as a type (not really a primitive type, but whatever).
|
/// An identifier used as a type (not really a primitive type, but whatever).
|
||||||
Named(Node<Identifier>),
|
Named { id: Node<Identifier> },
|
||||||
}
|
}
|
||||||
|
|
||||||
impl PrimitiveType {
|
impl PrimitiveType {
|
||||||
@ -3286,7 +3286,7 @@ impl fmt::Display for PrimitiveType {
|
|||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
PrimitiveType::Named(n) => write!(f, "{}", n.name),
|
PrimitiveType::Named { id: n } => write!(f, "{}", n.name),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -2938,7 +2938,7 @@ fn primitive_type(i: &mut TokenSlice) -> ModalResult<Node<PrimitiveType>> {
|
|||||||
(identifier, opt(delimited(open_paren, uom_for_type, close_paren))).map(|(ident, suffix)| {
|
(identifier, opt(delimited(open_paren, uom_for_type, close_paren))).map(|(ident, suffix)| {
|
||||||
let mut result = Node::new(PrimitiveType::Boolean, ident.start, ident.end, ident.module_id);
|
let mut result = Node::new(PrimitiveType::Boolean, ident.start, ident.end, ident.module_id);
|
||||||
result.inner =
|
result.inner =
|
||||||
PrimitiveType::primitive_from_str(&ident.name, suffix).unwrap_or(PrimitiveType::Named(ident));
|
PrimitiveType::primitive_from_str(&ident.name, suffix).unwrap_or(PrimitiveType::Named { id: ident });
|
||||||
result
|
result
|
||||||
}),
|
}),
|
||||||
))
|
))
|
||||||
|
@ -3504,3 +3504,24 @@ mod var_ref_in_own_def {
|
|||||||
super::execute(TEST_NAME, true).await
|
super::execute(TEST_NAME, true).await
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
mod ascription_unknown_type {
|
||||||
|
const TEST_NAME: &str = "ascription_unknown_type";
|
||||||
|
|
||||||
|
/// Test parsing KCL.
|
||||||
|
#[test]
|
||||||
|
fn parse() {
|
||||||
|
super::parse(TEST_NAME)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Test that parsing and unparsing KCL produces the original KCL input.
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn unparse() {
|
||||||
|
super::unparse(TEST_NAME).await
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Test that KCL is executed correctly.
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn kcl_test_execute() {
|
||||||
|
super::execute(TEST_NAME, true).await
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -0,0 +1,32 @@
|
|||||||
|
---
|
||||||
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
|
description: Artifact commands ascription_unknown_type.kcl
|
||||||
|
---
|
||||||
|
[
|
||||||
|
{
|
||||||
|
"cmdId": "[uuid]",
|
||||||
|
"range": [],
|
||||||
|
"command": {
|
||||||
|
"type": "edge_lines_visible",
|
||||||
|
"hidden": false
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"cmdId": "[uuid]",
|
||||||
|
"range": [],
|
||||||
|
"command": {
|
||||||
|
"type": "object_visible",
|
||||||
|
"object_id": "[uuid]",
|
||||||
|
"hidden": true
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"cmdId": "[uuid]",
|
||||||
|
"range": [],
|
||||||
|
"command": {
|
||||||
|
"type": "object_visible",
|
||||||
|
"object_id": "[uuid]",
|
||||||
|
"hidden": true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
]
|
@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
|
description: Artifact graph flowchart ascription_unknown_type.kcl
|
||||||
|
extension: md
|
||||||
|
snapshot_kind: binary
|
||||||
|
---
|
@ -0,0 +1,3 @@
|
|||||||
|
```mermaid
|
||||||
|
flowchart LR
|
||||||
|
```
|
67
rust/kcl-lib/tests/ascription_unknown_type/ast.snap
Normal file
67
rust/kcl-lib/tests/ascription_unknown_type/ast.snap
Normal file
@ -0,0 +1,67 @@
|
|||||||
|
---
|
||||||
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
|
description: Result of parsing ascription_unknown_type.kcl
|
||||||
|
---
|
||||||
|
{
|
||||||
|
"Ok": {
|
||||||
|
"body": [
|
||||||
|
{
|
||||||
|
"commentStart": 0,
|
||||||
|
"declaration": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"id": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"name": "z",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Identifier"
|
||||||
|
},
|
||||||
|
"init": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"expr": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"raw": "10",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Literal",
|
||||||
|
"type": "Literal",
|
||||||
|
"value": {
|
||||||
|
"value": 10.0,
|
||||||
|
"suffix": "None"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"start": 0,
|
||||||
|
"ty": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"id": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"name": "NotARealType",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Identifier"
|
||||||
|
},
|
||||||
|
"p_type": "Named",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Primitive"
|
||||||
|
},
|
||||||
|
"type": "AscribedExpression",
|
||||||
|
"type": "AscribedExpression"
|
||||||
|
},
|
||||||
|
"start": 0,
|
||||||
|
"type": "VariableDeclarator"
|
||||||
|
},
|
||||||
|
"end": 0,
|
||||||
|
"kind": "const",
|
||||||
|
"start": 0,
|
||||||
|
"type": "VariableDeclaration",
|
||||||
|
"type": "VariableDeclaration"
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"start": 0
|
||||||
|
}
|
||||||
|
}
|
@ -0,0 +1,12 @@
|
|||||||
|
---
|
||||||
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
|
description: Error from executing ascription_unknown_type.kcl
|
||||||
|
---
|
||||||
|
KCL Semantic error
|
||||||
|
|
||||||
|
× semantic: Unknown type: NotARealType
|
||||||
|
╭────
|
||||||
|
1 │ z = 10: NotARealType
|
||||||
|
· ─┬
|
||||||
|
· ╰── tests/ascription_unknown_type/input.kcl
|
||||||
|
╰────
|
1
rust/kcl-lib/tests/ascription_unknown_type/input.kcl
Normal file
1
rust/kcl-lib/tests/ascription_unknown_type/input.kcl
Normal file
@ -0,0 +1 @@
|
|||||||
|
z = 10: NotARealType
|
5
rust/kcl-lib/tests/ascription_unknown_type/ops.snap
Normal file
5
rust/kcl-lib/tests/ascription_unknown_type/ops.snap
Normal file
@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
|
description: Operations executed ascription_unknown_type.kcl
|
||||||
|
---
|
||||||
|
[]
|
5
rust/kcl-lib/tests/ascription_unknown_type/unparsed.snap
Normal file
5
rust/kcl-lib/tests/ascription_unknown_type/unparsed.snap
Normal file
@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
|
description: Result of unparsing ascription_unknown_type.kcl
|
||||||
|
---
|
||||||
|
z = 10: NotARealType
|
Reference in New Issue
Block a user