KCL: Better error message when using var in its own definition (#7339)
I thought I did this in https://github.com/KittyCAD/modeling-app/pull/7325, but I forgot to actually set the better message. Actually fixes, for real this time, https://github.com/KittyCAD/modeling-app/issues/6072 this time.
This commit is contained in:
@ -304,9 +304,10 @@ impl ExecutorContext {
|
|||||||
|
|
||||||
let annotations = &variable_declaration.outer_attrs;
|
let annotations = &variable_declaration.outer_attrs;
|
||||||
|
|
||||||
// During the evaluation of the variable's LHS, set context that this is all happening inside a variable
|
// During the evaluation of the variable's RHS, set context that this is all happening inside a variable
|
||||||
// declaration, for the given name. This helps improve user-facing error messages.
|
// declaration, for the given name. This helps improve user-facing error messages.
|
||||||
exec_state.mod_local.being_declared = Some(variable_declaration.inner.name().to_owned());
|
let lhs = variable_declaration.inner.name().to_owned();
|
||||||
|
exec_state.mod_local.being_declared = Some(lhs);
|
||||||
let rhs_result = self
|
let rhs_result = self
|
||||||
.execute_expr(
|
.execute_expr(
|
||||||
&variable_declaration.declaration.init,
|
&variable_declaration.declaration.init,
|
||||||
@ -645,7 +646,12 @@ impl ExecutorContext {
|
|||||||
Expr::Literal(literal) => KclValue::from_literal((**literal).clone(), exec_state),
|
Expr::Literal(literal) => KclValue::from_literal((**literal).clone(), exec_state),
|
||||||
Expr::TagDeclarator(tag) => tag.execute(exec_state).await?,
|
Expr::TagDeclarator(tag) => tag.execute(exec_state).await?,
|
||||||
Expr::Name(name) => {
|
Expr::Name(name) => {
|
||||||
let value = name.get_result(exec_state, self).await?.clone();
|
let being_declared = exec_state.mod_local.being_declared.clone();
|
||||||
|
let value = name
|
||||||
|
.get_result(exec_state, self)
|
||||||
|
.await
|
||||||
|
.map_err(|e| var_in_own_ref_err(e, &being_declared))?
|
||||||
|
.clone();
|
||||||
if let KclValue::Module { value: module_id, meta } = value {
|
if let KclValue::Module { value: module_id, meta } = value {
|
||||||
self.exec_module_for_result(
|
self.exec_module_for_result(
|
||||||
module_id,
|
module_id,
|
||||||
@ -751,6 +757,24 @@ impl ExecutorContext {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// If the error is about an undefined name, and that name matches the name being defined,
|
||||||
|
/// make the error message more specific.
|
||||||
|
fn var_in_own_ref_err(e: KclError, being_declared: &Option<String>) -> KclError {
|
||||||
|
let KclError::UndefinedValue { name, mut details } = e else {
|
||||||
|
return e;
|
||||||
|
};
|
||||||
|
// TODO after June 26th: replace this with a let-chain,
|
||||||
|
// which will be available in Rust 1.88
|
||||||
|
// https://rust-lang.github.io/rfcs/2497-if-let-chains.html
|
||||||
|
match (&being_declared, &name) {
|
||||||
|
(Some(name0), Some(name1)) if name0 == name1 => {
|
||||||
|
details.message = format!("You can't use `{name0}` because you're currently trying to define it. Use a different variable here instead.");
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
|
KclError::UndefinedValue { details, name }
|
||||||
|
}
|
||||||
|
|
||||||
impl Node<AscribedExpression> {
|
impl Node<AscribedExpression> {
|
||||||
#[async_recursion]
|
#[async_recursion]
|
||||||
pub async fn get_result(&self, exec_state: &mut ExecState, ctx: &ExecutorContext) -> Result<KclValue, KclError> {
|
pub async fn get_result(&self, exec_state: &mut ExecState, ctx: &ExecutorContext) -> Result<KclValue, KclError> {
|
||||||
|
@ -28,5 +28,30 @@ description: Artifact commands var_ref_in_own_def.kcl
|
|||||||
"object_id": "[uuid]",
|
"object_id": "[uuid]",
|
||||||
"hidden": true
|
"hidden": true
|
||||||
}
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"cmdId": "[uuid]",
|
||||||
|
"range": [],
|
||||||
|
"command": {
|
||||||
|
"type": "make_plane",
|
||||||
|
"origin": {
|
||||||
|
"x": 0.0,
|
||||||
|
"y": 0.0,
|
||||||
|
"z": 0.0
|
||||||
|
},
|
||||||
|
"x_axis": {
|
||||||
|
"x": 1.0,
|
||||||
|
"y": 0.0,
|
||||||
|
"z": 0.0
|
||||||
|
},
|
||||||
|
"y_axis": {
|
||||||
|
"x": 0.0,
|
||||||
|
"y": 1.0,
|
||||||
|
"z": 0.0
|
||||||
|
},
|
||||||
|
"size": 60.0,
|
||||||
|
"clobber": false,
|
||||||
|
"hide": true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
@ -1,3 +1,5 @@
|
|||||||
```mermaid
|
```mermaid
|
||||||
flowchart LR
|
flowchart LR
|
||||||
|
1["Plane<br>[95, 112, 0]"]
|
||||||
|
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 0 }]
|
||||||
```
|
```
|
||||||
|
@ -13,11 +13,13 @@ description: Result of parsing var_ref_in_own_def.kcl
|
|||||||
"id": {
|
"id": {
|
||||||
"commentStart": 0,
|
"commentStart": 0,
|
||||||
"end": 0,
|
"end": 0,
|
||||||
"name": "x",
|
"name": "sketch001",
|
||||||
"start": 0,
|
"start": 0,
|
||||||
"type": "Identifier"
|
"type": "Identifier"
|
||||||
},
|
},
|
||||||
"init": {
|
"init": {
|
||||||
|
"body": [
|
||||||
|
{
|
||||||
"callee": {
|
"callee": {
|
||||||
"abs_path": false,
|
"abs_path": false,
|
||||||
"commentStart": 0,
|
"commentStart": 0,
|
||||||
@ -25,7 +27,7 @@ description: Result of parsing var_ref_in_own_def.kcl
|
|||||||
"name": {
|
"name": {
|
||||||
"commentStart": 0,
|
"commentStart": 0,
|
||||||
"end": 0,
|
"end": 0,
|
||||||
"name": "cos",
|
"name": "startSketchOn",
|
||||||
"start": 0,
|
"start": 0,
|
||||||
"type": "Identifier"
|
"type": "Identifier"
|
||||||
},
|
},
|
||||||
@ -45,7 +47,7 @@ description: Result of parsing var_ref_in_own_def.kcl
|
|||||||
"name": {
|
"name": {
|
||||||
"commentStart": 0,
|
"commentStart": 0,
|
||||||
"end": 0,
|
"end": 0,
|
||||||
"name": "x",
|
"name": "XY",
|
||||||
"start": 0,
|
"start": 0,
|
||||||
"type": "Identifier"
|
"type": "Identifier"
|
||||||
},
|
},
|
||||||
@ -55,13 +57,105 @@ description: Result of parsing var_ref_in_own_def.kcl
|
|||||||
"type": "Name"
|
"type": "Name"
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"arguments": [
|
||||||
|
{
|
||||||
|
"type": "LabeledArg",
|
||||||
|
"label": null,
|
||||||
|
"arg": {
|
||||||
|
"abs_path": false,
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"name": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"name": "sketch001",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Identifier"
|
||||||
|
},
|
||||||
|
"path": [],
|
||||||
|
"start": 0,
|
||||||
|
"type": "Name",
|
||||||
|
"type": "Name"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"callee": {
|
||||||
|
"abs_path": false,
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"name": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"name": "startProfileAt",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Identifier"
|
||||||
|
},
|
||||||
|
"path": [],
|
||||||
|
"start": 0,
|
||||||
|
"type": "Name"
|
||||||
|
},
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"start": 0,
|
||||||
|
"type": "CallExpressionKw",
|
||||||
|
"type": "CallExpressionKw",
|
||||||
|
"unlabeled": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"elements": [
|
||||||
|
{
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"raw": "20",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Literal",
|
||||||
|
"type": "Literal",
|
||||||
|
"value": {
|
||||||
|
"value": 20.0,
|
||||||
|
"suffix": "None"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"argument": {
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"raw": "20",
|
||||||
|
"start": 0,
|
||||||
|
"type": "Literal",
|
||||||
|
"type": "Literal",
|
||||||
|
"value": {
|
||||||
|
"value": 20.0,
|
||||||
|
"suffix": "None"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"operator": "-",
|
||||||
|
"start": 0,
|
||||||
|
"type": "UnaryExpression",
|
||||||
|
"type": "UnaryExpression"
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"end": 0,
|
||||||
|
"start": 0,
|
||||||
|
"type": "ArrayExpression",
|
||||||
|
"type": "ArrayExpression"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"commentStart": 0,
|
||||||
|
"end": 0,
|
||||||
|
"start": 0,
|
||||||
|
"type": "PipeExpression",
|
||||||
|
"type": "PipeExpression"
|
||||||
|
},
|
||||||
"start": 0,
|
"start": 0,
|
||||||
"type": "VariableDeclarator"
|
"type": "VariableDeclarator"
|
||||||
},
|
},
|
||||||
"end": 0,
|
"end": 0,
|
||||||
"kind": "const",
|
"kind": "const",
|
||||||
"preComments": [
|
"preComments": [
|
||||||
"// This won't work, because `x` is being referenced in its own definition."
|
"// This won't work, because `sketch001` is being referenced in its own definition."
|
||||||
],
|
],
|
||||||
"start": 0,
|
"start": 0,
|
||||||
"type": "VariableDeclaration",
|
"type": "VariableDeclaration",
|
||||||
|
@ -4,10 +4,11 @@ description: Error from executing var_ref_in_own_def.kcl
|
|||||||
---
|
---
|
||||||
KCL UndefinedValue error
|
KCL UndefinedValue error
|
||||||
|
|
||||||
× undefined value: `x` is not defined
|
× undefined value: You can't use `sketch001` because you're currently trying
|
||||||
╭─[2:9]
|
│ to define it. Use a different variable here instead.
|
||||||
1 │ // This won't work, because `x` is being referenced in its own definition.
|
╭─[3:32]
|
||||||
2 │ x = cos(x)
|
2 │ sketch001 = startSketchOn(XY)
|
||||||
· ┬
|
3 │ |> startProfileAt([20, -20], sketch001)
|
||||||
|
· ────┬────
|
||||||
· ╰── tests/var_ref_in_own_def/input.kcl
|
· ╰── tests/var_ref_in_own_def/input.kcl
|
||||||
╰────
|
╰────
|
||||||
|
@ -1,2 +1,3 @@
|
|||||||
// This won't work, because `x` is being referenced in its own definition.
|
// This won't work, because `sketch001` is being referenced in its own definition.
|
||||||
x = cos(x)
|
sketch001 = startSketchOn(XY)
|
||||||
|
|> startProfileAt([20, -20], sketch001)
|
||||||
|
@ -2,4 +2,18 @@
|
|||||||
source: kcl-lib/src/simulation_tests.rs
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
description: Operations executed var_ref_in_own_def.kcl
|
description: Operations executed var_ref_in_own_def.kcl
|
||||||
---
|
---
|
||||||
[]
|
[
|
||||||
|
{
|
||||||
|
"type": "StdLibCall",
|
||||||
|
"name": "startSketchOn",
|
||||||
|
"unlabeledArg": {
|
||||||
|
"value": {
|
||||||
|
"type": "Plane",
|
||||||
|
"artifact_id": "[uuid]"
|
||||||
|
},
|
||||||
|
"sourceRange": []
|
||||||
|
},
|
||||||
|
"labeledArgs": {},
|
||||||
|
"sourceRange": []
|
||||||
|
}
|
||||||
|
]
|
||||||
|
@ -2,5 +2,6 @@
|
|||||||
source: kcl-lib/src/simulation_tests.rs
|
source: kcl-lib/src/simulation_tests.rs
|
||||||
description: Result of unparsing var_ref_in_own_def.kcl
|
description: Result of unparsing var_ref_in_own_def.kcl
|
||||||
---
|
---
|
||||||
// This won't work, because `x` is being referenced in its own definition.
|
// This won't work, because `sketch001` is being referenced in its own definition.
|
||||||
x = cos(x)
|
sketch001 = startSketchOn(XY)
|
||||||
|
|> startProfileAt([20, -20], sketch001)
|
||||||
|
Reference in New Issue
Block a user