Add display of array element types in error messages (#7113)

* Add test showing unhelpful error message

* Add display of array element types in error messages

* Change to prose description

* Update output
This commit is contained in:
Jonathan Tran
2025-05-20 20:50:24 -04:00
committed by GitHub
parent a13b6b2b70
commit 0f0fc39d07
13 changed files with 473 additions and 11 deletions

View File

@ -759,7 +759,7 @@ fn apply_ascription(
};
KclError::Semantic(KclErrorDetails::new(
format!(
"could not coerce {} value to type {ty}{suggestion}",
"could not coerce value of type {} to type {ty}{suggestion}",
value.human_friendly_type()
),
vec![source_range],
@ -1652,7 +1652,7 @@ a = 42: string
let err = result.unwrap_err();
assert!(
err.to_string()
.contains("could not coerce number(default units) value to type string"),
.contains("could not coerce value of type number(default units) to type string"),
"Expected error but found {err:?}"
);
@ -1663,7 +1663,7 @@ a = 42: Plane
let err = result.unwrap_err();
assert!(
err.to_string()
.contains("could not coerce number(default units) value to type Plane"),
.contains("could not coerce value of type number(default units) to type Plane"),
"Expected error but found {err:?}"
);
@ -1673,8 +1673,9 @@ arr = [0]: [string]
let result = parse_execute(program).await;
let err = result.unwrap_err();
assert!(
err.to_string()
.contains("could not coerce [any; 1] value to type [string]"),
err.to_string().contains(
"could not coerce value of type array of number(default units) with 1 value to type [string]"
),
"Expected error but found {err:?}"
);
@ -1685,7 +1686,7 @@ mixedArr = [0, "a"]: [number(mm)]
let err = result.unwrap_err();
assert!(
err.to_string()
.contains("could not coerce [any; 2] value to type [number(mm)]"),
.contains("could not coerce value of type array of number(default units), string with 2 values to type [number(mm)]"),
"Expected error but found {err:?}"
);
}

View File

@ -281,8 +281,34 @@ impl KclValue {
/// Human readable type name used in error messages. Should not be relied
/// on for program logic.
pub(crate) fn human_friendly_type(&self) -> String {
if let Some(t) = self.principal_type() {
return t.to_string();
self.inner_human_friendly_type(1)
}
fn inner_human_friendly_type(&self, max_depth: usize) -> String {
if let Some(pt) = self.principal_type() {
if max_depth > 0 {
// The principal type of an array uses the array's element type,
// which is oftentimes `any`, and that's not a helpful message. So
// we show the actual elements.
if let Some(elements) = self.as_array() {
// If it's empty, we want to show the type of the array.
if !elements.is_empty() {
// A max of 3 is good because it's common to use 3D points.
let max = 3;
let len = elements.len();
let ellipsis = if len > max { ", ..." } else { "" };
let element_label = if len == 1 { "value" } else { "values" };
let element_tys = elements
.iter()
.take(max)
.map(|elem| elem.inner_human_friendly_type(max_depth - 1))
.collect::<Vec<_>>()
.join(", ");
return format!("array of {element_tys}{ellipsis} with {len} {element_label}");
}
}
}
return pt.to_string();
}
match self {
KclValue::Uuid { .. } => "Unique ID (uuid)",
@ -644,3 +670,88 @@ impl From<GeometryWithImportedGeometry> for KclValue {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_human_friendly_type() {
let len = KclValue::Number {
value: 1.0,
ty: NumericType::Known(UnitType::Length(UnitLen::Unknown)),
meta: vec![],
};
assert_eq!(len.human_friendly_type(), "number(Length)".to_string());
let unknown = KclValue::Number {
value: 1.0,
ty: NumericType::Unknown,
meta: vec![],
};
assert_eq!(unknown.human_friendly_type(), "number(unknown units)".to_string());
let mm = KclValue::Number {
value: 1.0,
ty: NumericType::Known(UnitType::Length(UnitLen::Mm)),
meta: vec![],
};
assert_eq!(mm.human_friendly_type(), "number(mm)".to_string());
let array1_mm = KclValue::HomArray {
value: vec![mm.clone()],
ty: RuntimeType::any(),
};
assert_eq!(
array1_mm.human_friendly_type(),
"array of number(mm) with 1 value".to_string()
);
let array2_mm = KclValue::HomArray {
value: vec![mm.clone(), mm.clone()],
ty: RuntimeType::any(),
};
assert_eq!(
array2_mm.human_friendly_type(),
"array of number(mm), number(mm) with 2 values".to_string()
);
let array3_mm = KclValue::HomArray {
value: vec![mm.clone(), mm.clone(), mm.clone()],
ty: RuntimeType::any(),
};
assert_eq!(
array3_mm.human_friendly_type(),
"array of number(mm), number(mm), number(mm) with 3 values".to_string()
);
let inches = KclValue::Number {
value: 1.0,
ty: NumericType::Known(UnitType::Length(UnitLen::Inches)),
meta: vec![],
};
let array4 = KclValue::HomArray {
value: vec![mm.clone(), mm.clone(), inches.clone(), mm.clone()],
ty: RuntimeType::any(),
};
assert_eq!(
array4.human_friendly_type(),
"array of number(mm), number(mm), number(in), ... with 4 values".to_string()
);
let empty_array = KclValue::HomArray {
value: vec![],
ty: RuntimeType::any(),
};
assert_eq!(empty_array.human_friendly_type(), "[any; 0]".to_string());
let array_nested = KclValue::HomArray {
value: vec![array2_mm.clone()],
ty: RuntimeType::any(),
};
assert_eq!(
array_nested.human_friendly_type(),
"array of [any; 2] with 1 value".to_string()
);
}
}

View File

@ -384,6 +384,27 @@ mod any_type {
super::execute(TEST_NAME, false).await
}
}
mod error_with_point_shows_numeric_units {
const TEST_NAME: &str = "error_with_point_shows_numeric_units";
/// 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
}
}
mod artifact_graph_example_code1 {
const TEST_NAME: &str = "artifact_graph_example_code1";

View File

@ -4,7 +4,8 @@ description: Error from executing argument_error.kcl
---
KCL Semantic error
× semantic: f requires a value with type `fn(any): any`, but found [any; 2]
× semantic: f requires a value with type `fn(any): any`, but found array of
│ number(default units), number(default units) with 2 values
╭─[5:1]
4 │
5 │ map(f, f = [0, 1])
@ -15,7 +16,7 @@ KCL Semantic error
╰─▶ KCL Semantic error
× semantic: f requires a value with type `fn(any): any`, but found
[any; 2]
array of number(default units), number(default units) with 2 values
╭─[5:12]
4 │
5 │ map(f, f = [0, 1])

View File

@ -1,5 +1,5 @@
---
source: kcl/src/simulation_tests.rs
source: kcl-lib/src/simulation_tests.rs
description: Error from executing array_elem_pop_fail.kcl
---
KCL UndefinedValue error

View File

@ -0,0 +1,57 @@
---
source: kcl-lib/src/simulation_tests.rs
description: Artifact commands error_with_point_shows_numeric_units.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
}
},
{
"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": 0.0,
"z": 1.0
},
"size": 60.0,
"clobber": false,
"hide": true
}
}
]

View File

@ -0,0 +1,6 @@
---
source: kcl-lib/src/simulation_tests.rs
description: Artifact graph flowchart error_with_point_shows_numeric_units.kcl
extension: md
snapshot_kind: binary
---

View File

@ -0,0 +1,5 @@
```mermaid
flowchart LR
1["Plane<br>[0, 17, 0]"]
%% [ProgramBodyItem { index: 0 }, ExpressionStatementExpr, PipeBodyItem { index: 0 }]
```

View File

@ -0,0 +1,201 @@
---
source: kcl-lib/src/simulation_tests.rs
description: Result of parsing error_with_point_shows_numeric_units.kcl
---
{
"Ok": {
"body": [
{
"commentStart": 0,
"end": 0,
"expression": {
"body": [
{
"callee": {
"abs_path": false,
"commentStart": 0,
"end": 0,
"name": {
"commentStart": 0,
"end": 0,
"name": "startSketchOn",
"start": 0,
"type": "Identifier"
},
"path": [],
"start": 0,
"type": "Name"
},
"commentStart": 0,
"end": 0,
"start": 0,
"type": "CallExpressionKw",
"type": "CallExpressionKw",
"unlabeled": {
"abs_path": false,
"commentStart": 0,
"end": 0,
"name": {
"commentStart": 0,
"end": 0,
"name": "XZ",
"start": 0,
"type": "Identifier"
},
"path": [],
"start": 0,
"type": "Name",
"type": "Name"
}
},
{
"arguments": [
{
"type": "LabeledArg",
"label": {
"commentStart": 0,
"end": 0,
"name": "center",
"start": 0,
"type": "Identifier"
},
"arg": {
"commentStart": 0,
"elements": [
{
"callee": {
"abs_path": false,
"commentStart": 0,
"end": 0,
"name": {
"commentStart": 0,
"end": 0,
"name": "sin",
"start": 0,
"type": "Identifier"
},
"path": [],
"start": 0,
"type": "Name"
},
"commentStart": 0,
"end": 0,
"start": 0,
"type": "CallExpressionKw",
"type": "CallExpressionKw",
"unlabeled": {
"commentStart": 0,
"end": 0,
"raw": "66.6deg",
"start": 0,
"type": "Literal",
"type": "Literal",
"value": {
"value": 66.6,
"suffix": "Deg"
}
}
},
{
"callee": {
"abs_path": false,
"commentStart": 0,
"end": 0,
"name": {
"commentStart": 0,
"end": 0,
"name": "cos",
"start": 0,
"type": "Identifier"
},
"path": [],
"start": 0,
"type": "Name"
},
"commentStart": 0,
"end": 0,
"start": 0,
"type": "CallExpressionKw",
"type": "CallExpressionKw",
"unlabeled": {
"commentStart": 0,
"end": 0,
"raw": "66.6deg",
"start": 0,
"type": "Literal",
"type": "Literal",
"value": {
"value": 66.6,
"suffix": "Deg"
}
}
}
],
"end": 0,
"start": 0,
"type": "ArrayExpression",
"type": "ArrayExpression"
}
},
{
"type": "LabeledArg",
"label": {
"commentStart": 0,
"end": 0,
"name": "radius",
"start": 0,
"type": "Identifier"
},
"arg": {
"commentStart": 0,
"end": 0,
"raw": "1",
"start": 0,
"type": "Literal",
"type": "Literal",
"value": {
"value": 1.0,
"suffix": "None"
}
}
}
],
"callee": {
"abs_path": false,
"commentStart": 0,
"end": 0,
"name": {
"commentStart": 0,
"end": 0,
"name": "circle",
"start": 0,
"type": "Identifier"
},
"path": [],
"start": 0,
"type": "Name"
},
"commentStart": 0,
"end": 0,
"start": 0,
"type": "CallExpressionKw",
"type": "CallExpressionKw",
"unlabeled": null
}
],
"commentStart": 0,
"end": 0,
"start": 0,
"type": "PipeExpression",
"type": "PipeExpression"
},
"start": 0,
"type": "ExpressionStatement",
"type": "ExpressionStatement"
}
],
"commentStart": 0,
"end": 0,
"start": 0
}
}

View File

@ -0,0 +1,29 @@
---
source: kcl-lib/src/simulation_tests.rs
description: Error from executing error_with_point_shows_numeric_units.kcl
---
KCL Semantic error
× semantic: center requires a value with type `Point2d`, but found array of
│ number(Count), number(Count) with 2 values
╭─[2:6]
1 │ startSketchOn(XZ)
2 │ ╭──▶ |> circle(center = [
3 │ ││ sin(66.6deg),
4 │ ││ cos(66.6deg)
5 │ ├──▶ ], radius = 1)
· ╰───── tests/error_with_point_shows_numeric_units/input.kcl
· ╰───── tests/error_with_point_shows_numeric_units/input.kcl
╰────
╰─▶ KCL Semantic error
× semantic: center requires a value with type `Point2d`, but found
│ array of number(Count), number(Count) with 2 values
╭─[2:22]
1 │ startSketchOn(XZ)
2 │ ╭─▶ |> circle(center = [
3 │ │ sin(66.6deg),
4 │ │ cos(66.6deg)
5 │ ├─▶ ], radius = 1)
· ╰──── tests/error_with_point_shows_numeric_units/input.kcl
╰────

View File

@ -0,0 +1,5 @@
startSketchOn(XZ)
|> circle(center = [
sin(66.6deg),
cos(66.6deg)
], radius = 1)

View File

@ -0,0 +1,19 @@
---
source: kcl-lib/src/simulation_tests.rs
description: Operations executed error_with_point_shows_numeric_units.kcl
---
[
{
"type": "StdLibCall",
"name": "startSketchOn",
"unlabeledArg": {
"value": {
"type": "Plane",
"artifact_id": "[uuid]"
},
"sourceRange": []
},
"labeledArgs": {},
"sourceRange": []
}
]

View File

@ -0,0 +1,6 @@
---
source: kcl-lib/src/simulation_tests.rs
description: Result of unparsing error_with_point_shows_numeric_units.kcl
---
startSketchOn(XZ)
|> circle(center = [sin(66.6deg), cos(66.6deg)], radius = 1)