Add actual argument type to error message

KittyCAD · Aug 13, 2024 · d68c11e · d68c11e
1 parent 6a09bbc
commit d68c11e
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 7 deletions.
diff --git a/src/wasm-lib/kcl/src/executor.rs b/src/wasm-lib/kcl/src/executor.rs
@@ -311,6 +311,31 @@ impl KclValue {
             _ => anyhow::bail!("Not a extrude group or extrude groups: {:?}", self),
         }
     }
+
+    /// Human readable type name used in error messages.  Should not be relied
+    /// on for program logic.
+    pub(crate) fn display_type_name(&self) -> &'static str {
+        match self {
+            MemoryItem::UserVal(u) => match u.value {
+                JValue::Null => "Null",
+                JValue::Bool(_) => "Bool",
+                JValue::Number(_) => "Number",
+                JValue::String(_) => "String",
+                JValue::Array(_) => "Array",
+                JValue::Object(_) => "Object",
+            },
+            MemoryItem::TagDeclarator(_) => "TagDeclarator",
+            MemoryItem::TagIdentifier(_) => "TagIdentifier",
+            MemoryItem::SketchGroup(_) => "SketchGroup",
+            MemoryItem::SketchGroups { .. } => "SketchGroups",
+            MemoryItem::ExtrudeGroup(_) => "ExtrudeGroup",
+            MemoryItem::ExtrudeGroups { .. } => "ExtrudeGroups",
+            MemoryItem::ImportedGeometry(_) => "ImportedGeometry",
+            MemoryItem::Function { .. } => "Function",
+            MemoryItem::Plane(_) => "Plane",
+            MemoryItem::Face(_) => "Face",
+        }
+    }
 }
 
 impl From<SketchGroupSet> for KclValue {
@@ -2355,6 +2380,29 @@ const thisBox = box({start: [0,0], l: 6, w: 10, h: 3})
         parse_execute(ast).await.unwrap();
     }
 
+    #[tokio::test(flavor = "multi_thread")]
+    #[ignore] // https://github.com/KittyCAD/modeling-app/issues/3338
+    async fn test_object_member_starting_pipeline() {
+        let ast = r#"
+fn test2 = () => {
+  return {
+    thing: startSketchOn('XY')
+      |> startProfileAt([0, 0], %)
+      |> line([0, 1], %)
+      |> line([1, 0], %)
+      |> line([0, -1], %)
+      |> close(%)
+  }
+}
+
+const x2 = test2()
+
+x2.thing
+  |> extrude(10, %)
+"#;
+        parse_execute(ast).await.unwrap();
+    }
+
     #[tokio::test(flavor = "multi_thread")]
     #[ignore] // ignore til we get loops
     async fn test_execute_with_function_sketch_loop_objects() {

diff --git a/src/wasm-lib/kcl/src/std/args.rs b/src/wasm-lib/kcl/src/std/args.rs
@@ -460,8 +460,9 @@ where
         let Some(val) = T::from_mem_item(arg) else {
             return Err(KclError::Semantic(KclErrorDetails {
                 message: format!(
-                    "Argument at index {i} was supposed to be type {} but wasn't",
+                    "Argument at index {i} was supposed to be type {} but found {}",
                     type_name::<T>(),
+                    arg.display_type_name()
                 ),
                 source_ranges: vec![args.source_range],
             }));
@@ -479,8 +480,9 @@ where
         let Some(val) = T::from_mem_item(arg) else {
             return Err(KclError::Semantic(KclErrorDetails {
                 message: format!(
-                    "Argument at index {i} was supposed to be type {} but wasn't",
-                    type_name::<T>()
+                    "Argument at index {i} was supposed to be type {} but found {}",
+                    type_name::<T>(),
+                    arg.display_type_name()
                 ),
                 source_ranges: vec![args.source_range],
             }));

diff --git a/src/wasm-lib/tests/executor/main.rs b/src/wasm-lib/tests/executor/main.rs
@@ -903,7 +903,7 @@ const part = rectShape([0, 0], 20, 20)
     assert!(result.is_err());
     assert_eq!(
         result.err().unwrap().to_string(),
-        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([887, 936])], message: "Argument at index 0 was supposed to be type [f64; 2] but wasn't" }"#,
+        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([887, 936])], message: "Argument at index 0 was supposed to be type [f64; 2] but found String" }"#,
     );
 }
 
@@ -1425,7 +1425,7 @@ const secondSketch = startSketchOn(part001, '')
     assert!(result.is_err());
     assert_eq!(
         result.err().unwrap().to_string(),
-        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([272, 298])], message: "Argument at index 1 was supposed to be type kcl_lib::std::sketch::FaceTag but wasn't" }"#
+        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([272, 298])], message: "Argument at index 1 was supposed to be type kcl_lib::std::sketch::FaceTag but found String" }"#
     );
 }
 
@@ -2236,7 +2236,7 @@ someFunction('INVALID')
     assert!(result.is_err());
     assert_eq!(
         result.err().unwrap().to_string(),
-        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([37, 61]), SourceRange([65, 88])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but wasn't" }"#
+        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([37, 61]), SourceRange([65, 88])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but found String" }"#
     );
 }
 
@@ -2257,7 +2257,7 @@ someFunction('INVALID')
     assert!(result.is_err());
     assert_eq!(
         result.err().unwrap().to_string(),
-        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([89, 114]), SourceRange([126, 155]), SourceRange([159, 182])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but wasn't" }"#
+        r#"semantic: KclErrorDetails { source_ranges: [SourceRange([89, 114]), SourceRange([126, 155]), SourceRange([159, 182])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but found String" }"#
     );
 }