diff --git a/src/wasm-lib/kcl/src/ast/types/execute.rs b/src/wasm-lib/kcl/src/ast/types/execute.rs index a9eb0eef8..f1dceadc3 100644 --- a/src/wasm-lib/kcl/src/ast/types/execute.rs +++ b/src/wasm-lib/kcl/src/ast/types/execute.rs @@ -11,7 +11,7 @@ use crate::{ BodyType, ExecState, ExecutorContext, KclValue, Metadata, SourceRange, StatementKind, TagEngineInfo, TagIdentifier, }, - std::FunctionKind, + std::{args::Arg, FunctionKind}, }; use async_recursion::async_recursion; @@ -361,16 +361,17 @@ impl Node { pub async fn execute(&self, exec_state: &mut ExecState, ctx: &ExecutorContext) -> Result { let fn_name = &self.callee.name; - let mut fn_args: Vec = Vec::with_capacity(self.arguments.len()); + let mut fn_args: Vec = Vec::with_capacity(self.arguments.len()); - for arg in &self.arguments { + for arg_expr in &self.arguments { let metadata = Metadata { - source_range: SourceRange::from(arg), + source_range: SourceRange::from(arg_expr), }; - let result = ctx - .execute_expr(arg, exec_state, &metadata, StatementKind::Expression) + let value = ctx + .execute_expr(arg_expr, exec_state, &metadata, StatementKind::Expression) .await?; - fn_args.push(result); + let arg = Arg::new(value, SourceRange::from(arg_expr)); + fn_args.push(arg); } match ctx.stdlib.get_either(&self.callee.name) { @@ -470,14 +471,18 @@ impl Node { for (index, param) in required_params.iter().enumerate() { fn_memory.add( ¶m.identifier.name, - fn_args.get(index).unwrap().clone(), + fn_args.get(index).unwrap().value.clone(), param.identifier.clone().into(), )?; } // Add the optional arguments to the memory. for (index, param) in optional_params.iter().enumerate() { if let Some(arg) = fn_args.get(index + required_params.len()) { - fn_memory.add(¶m.identifier.name, arg.clone(), param.identifier.clone().into())?; + fn_memory.add( + ¶m.identifier.name, + arg.value.clone(), + param.identifier.clone().into(), + )?; } else { fn_memory.add( ¶m.identifier.name, diff --git a/src/wasm-lib/kcl/src/executor.rs b/src/wasm-lib/kcl/src/executor.rs index 389f0b2ca..1f88da71a 100644 --- a/src/wasm-lib/kcl/src/executor.rs +++ b/src/wasm-lib/kcl/src/executor.rs @@ -33,7 +33,7 @@ use crate::{ errors::{KclError, KclErrorDetails}, fs::{FileManager, FileSystem}, settings::types::UnitLength, - std::StdLib, + std::{args::Arg, StdLib}, Program, }; @@ -740,7 +740,7 @@ impl std::hash::Hash for TagIdentifier { pub type MemoryFunction = fn( - s: Vec, + s: Vec, memory: ProgramMemory, expression: crate::ast::types::BoxNode, metadata: Vec, @@ -1032,6 +1032,11 @@ impl SourceRange { Self([start, end, module_id.as_usize()]) } + /// A source range that doesn't correspond to any source code. + pub fn synthetic() -> Self { + Self::default() + } + /// Get the start of the range. pub fn start(&self) -> usize { self.0[0] @@ -2203,7 +2208,7 @@ impl ExecutorContext { /// Returns Err if too few/too many arguments were given for the function. fn assign_args_to_params( function_expression: NodeRef<'_, FunctionExpression>, - args: Vec, + args: Vec, mut fn_memory: ProgramMemory, ) -> Result { let num_args = function_expression.number_of_args(); @@ -2229,7 +2234,7 @@ fn assign_args_to_params( for (index, param) in function_expression.params.iter().enumerate() { if let Some(arg) = args.get(index) { // Argument was provided. - fn_memory.add(¶m.identifier.name, arg.clone(), (¶m.identifier).into())?; + fn_memory.add(¶m.identifier.name, arg.value.clone(), (¶m.identifier).into())?; } else { // Argument was not provided. if param.optional { @@ -2257,7 +2262,7 @@ fn assign_args_to_params( } pub(crate) async fn call_user_defined_function( - args: Vec, + args: Vec, memory: &ProgramMemory, function_expression: NodeRef<'_, FunctionExpression>, exec_state: &mut ExecState, @@ -3156,6 +3161,7 @@ let w = f() + f() return_type: None, digest: None, }); + let args = args.into_iter().map(Arg::synthetic).collect(); let actual = assign_args_to_params(func_expr, args, ProgramMemory::new()); assert_eq!( actual, expected, diff --git a/src/wasm-lib/kcl/src/function_param.rs b/src/wasm-lib/kcl/src/function_param.rs index 190cbd266..1a27c4db4 100644 --- a/src/wasm-lib/kcl/src/function_param.rs +++ b/src/wasm-lib/kcl/src/function_param.rs @@ -6,6 +6,7 @@ use crate::{ executor::{ call_user_defined_function, ExecState, ExecutorContext, KclValue, MemoryFunction, Metadata, ProgramMemory, }, + std::args::Arg, }; /// A function being used as a parameter into a stdlib function. This is a @@ -19,7 +20,7 @@ pub struct FunctionParam<'a> { } impl<'a> FunctionParam<'a> { - pub async fn call(&self, exec_state: &mut ExecState, args: Vec) -> Result, KclError> { + pub async fn call(&self, exec_state: &mut ExecState, args: Vec) -> Result, KclError> { if let Some(inner) = self.inner { inner( args, diff --git a/src/wasm-lib/kcl/src/kcl_value.rs b/src/wasm-lib/kcl/src/kcl_value.rs index ecee24eee..7507f1a83 100644 --- a/src/wasm-lib/kcl/src/kcl_value.rs +++ b/src/wasm-lib/kcl/src/kcl_value.rs @@ -9,7 +9,7 @@ use crate::{ errors::KclErrorDetails, exec::{ProgramMemory, Sketch}, executor::{Face, ImportedGeometry, MemoryFunction, Metadata, Plane, SketchSet, Solid, SolidSet, TagIdentifier}, - std::FnAsArg, + std::{args::Arg, FnAsArg}, ExecState, ExecutorContext, KclError, SourceRange, }; @@ -452,7 +452,7 @@ impl KclValue { /// If it's not a function, return Err. pub async fn call_fn( &self, - args: Vec, + args: Vec, exec_state: &mut ExecState, ctx: ExecutorContext, ) -> Result, KclError> { diff --git a/src/wasm-lib/kcl/src/simulation_tests.rs b/src/wasm-lib/kcl/src/simulation_tests.rs index cfc4d0d17..d28bf41bb 100644 --- a/src/wasm-lib/kcl/src/simulation_tests.rs +++ b/src/wasm-lib/kcl/src/simulation_tests.rs @@ -413,6 +413,36 @@ mod add_lots { super::execute(TEST_NAME, false).await } } +mod argument_error { + //! The argument error points to the problematic argument in the call site, + //! not the function definition that the variable points to. + + const TEST_NAME: &str = "argument_error"; + + /// Test tokenizing KCL. + #[test] + fn tokenize() { + super::tokenize(TEST_NAME) + } + + /// Test parsing KCL. + #[test] + fn parse() { + super::parse(TEST_NAME) + } + + /// Test that parsing and unparsing KCL produces the original KCL input. + #[test] + fn unparse() { + super::unparse(TEST_NAME) + } + + /// Test that KCL is executed correctly. + #[tokio::test(flavor = "multi_thread")] + async fn kcl_test_execute() { + super::execute(TEST_NAME, false).await + } +} mod array_elem_push { const TEST_NAME: &str = "array_elem_push"; diff --git a/src/wasm-lib/kcl/src/std/args.rs b/src/wasm-lib/kcl/src/std/args.rs index e2b926882..ac8f27242 100644 --- a/src/wasm-lib/kcl/src/std/args.rs +++ b/src/wasm-lib/kcl/src/std/args.rs @@ -16,15 +16,40 @@ use crate::{ use super::shapes::PolygonType; +#[derive(Debug, Clone)] +pub struct Arg { + /// The evaluated argument. + pub value: KclValue, + /// The source range of the unevaluated argument. + pub source_range: SourceRange, +} + +impl Arg { + pub fn new(value: KclValue, source_range: SourceRange) -> Self { + Self { value, source_range } + } + + pub fn synthetic(value: KclValue) -> Self { + Self { + value, + source_range: SourceRange::synthetic(), + } + } + + pub fn source_ranges(&self) -> Vec { + vec![self.source_range] + } +} + #[derive(Debug, Clone)] pub struct Args { - pub args: Vec, + pub args: Vec, pub source_range: SourceRange, pub ctx: ExecutorContext, } impl Args { - pub fn new(args: Vec, source_range: SourceRange, ctx: ExecutorContext) -> Self { + pub fn new(args: Vec, source_range: SourceRange, ctx: ExecutorContext) -> Self { Self { args, source_range, @@ -244,10 +269,10 @@ impl Args { .args .iter() .map(|arg| { - let Some(num) = f64::from_kcl_val(arg) else { + let Some(num) = f64::from_kcl_val(&arg.value) else { return Err(KclError::Semantic(KclErrorDetails { - source_ranges: arg.metadata().iter().map(|x| x.source_range).collect(), - message: format!("Expected a number but found {}", arg.human_friendly_type()), + source_ranges: arg.source_ranges(), + message: format!("Expected a number but found {}", arg.value.human_friendly_type()), })); }; Ok(num) @@ -509,10 +534,10 @@ impl<'a> FromArgs<'a> for Vec { source_ranges: vec![args.source_range], })); }; - let KclValue::Array { value: array, meta: _ } = arg else { - let message = format!("Expected an array but found {}", arg.human_friendly_type()); + let KclValue::Array { value: array, meta: _ } = &arg.value else { + let message = format!("Expected an array but found {}", arg.value.human_friendly_type()); return Err(KclError::Type(KclErrorDetails { - source_ranges: arg.metadata().into_iter().map(|m| m.source_range).collect(), + source_ranges: arg.source_ranges(), message, })); }; @@ -531,14 +556,14 @@ where source_ranges: vec![args.source_range], })); }; - let Some(val) = T::from_kcl_val(arg) else { + let Some(val) = T::from_kcl_val(&arg.value) else { return Err(KclError::Semantic(KclErrorDetails { message: format!( "Argument at index {i} was supposed to be type {} but found {}", type_name::(), - arg.human_friendly_type() + arg.value.human_friendly_type() ), - source_ranges: vec![args.source_range], + source_ranges: arg.source_ranges(), })); }; Ok(val) @@ -551,17 +576,17 @@ where { fn from_args(args: &'a Args, i: usize) -> Result { let Some(arg) = args.args.get(i) else { return Ok(None) }; - if crate::ast::types::KclNone::from_kcl_val(arg).is_some() { + if crate::ast::types::KclNone::from_kcl_val(&arg.value).is_some() { return Ok(None); } - let Some(val) = T::from_kcl_val(arg) else { + let Some(val) = T::from_kcl_val(&arg.value) else { return Err(KclError::Semantic(KclErrorDetails { message: format!( "Argument at index {i} was supposed to be type Option<{}> but found {}", type_name::(), - arg.human_friendly_type() + arg.value.human_friendly_type() ), - source_ranges: vec![args.source_range], + source_ranges: arg.source_ranges(), })); }; Ok(Some(val)) diff --git a/src/wasm-lib/kcl/src/std/array.rs b/src/wasm-lib/kcl/src/std/array.rs index 8a15d7467..3e5666414 100644 --- a/src/wasm-lib/kcl/src/std/array.rs +++ b/src/wasm-lib/kcl/src/std/array.rs @@ -1,6 +1,9 @@ use derive_docs::stdlib; -use super::{args::FromArgs, Args, FnAsArg}; +use super::{ + args::{Arg, FromArgs}, + Args, FnAsArg, +}; use crate::{ errors::{KclError, KclErrorDetails}, executor::{ExecState, KclValue, SourceRange}, @@ -75,7 +78,7 @@ async fn call_map_closure<'a>( source_range: SourceRange, exec_state: &mut ExecState, ) -> Result { - let output = map_fn.call(exec_state, vec![input]).await?; + let output = map_fn.call(exec_state, vec![Arg::synthetic(input)]).await?; let source_ranges = vec![source_range]; let output = output.ok_or_else(|| { KclError::Semantic(KclErrorDetails { @@ -202,7 +205,7 @@ async fn call_reduce_closure<'a>( exec_state: &mut ExecState, ) -> Result { // Call the reduce fn for this repetition. - let reduce_fn_args = vec![elem, start]; + let reduce_fn_args = vec![Arg::synthetic(elem), Arg::synthetic(start)]; let transform_fn_return = reduce_fn.call(exec_state, reduce_fn_args).await?; // Unpack the returned transform object. diff --git a/src/wasm-lib/kcl/src/std/patterns.rs b/src/wasm-lib/kcl/src/std/patterns.rs index 8e4720499..680c7029b 100644 --- a/src/wasm-lib/kcl/src/std/patterns.rs +++ b/src/wasm-lib/kcl/src/std/patterns.rs @@ -22,6 +22,8 @@ use crate::{ std::{types::Uint, Args}, }; +use super::args::Arg; + const MUST_HAVE_ONE_INSTANCE: &str = "There must be at least 1 instance of your geometry"; /// Data for a linear pattern on a 2D sketch. @@ -381,7 +383,7 @@ async fn make_transform<'a>( value: i.into(), meta: vec![source_range.into()], }; - let transform_fn_args = vec![repetition_num]; + let transform_fn_args = vec![Arg::synthetic(repetition_num)]; let transform_fn_return = transform_function.call(exec_state, transform_fn_args).await?; // Unpack the returned transform object. diff --git a/src/wasm-lib/kcl/tests/argument_error/ast.snap b/src/wasm-lib/kcl/tests/argument_error/ast.snap new file mode 100644 index 000000000..9c8e04c14 --- /dev/null +++ b/src/wasm-lib/kcl/tests/argument_error/ast.snap @@ -0,0 +1,137 @@ +--- +source: kcl/src/simulation_tests.rs +description: Result of parsing argument_error.kcl +--- +{ + "Ok": { + "body": [ + { + "declarations": [ + { + "end": 28, + "id": { + "end": 4, + "name": "f", + "start": 3, + "type": "Identifier" + }, + "init": { + "body": { + "body": [ + { + "argument": { + "end": 26, + "raw": "5", + "start": 25, + "type": "Literal", + "type": "Literal", + "value": 5 + }, + "end": 26, + "start": 18, + "type": "ReturnStatement", + "type": "ReturnStatement" + } + ], + "end": 28, + "start": 14 + }, + "end": 28, + "params": [ + { + "type": "Parameter", + "identifier": { + "end": 9, + "name": "i", + "start": 8, + "type": "Identifier" + }, + "optional": false + } + ], + "start": 7, + "type": "FunctionExpression", + "type": "FunctionExpression" + }, + "start": 3, + "type": "VariableDeclarator" + } + ], + "end": 28, + "kind": "fn", + "start": 0, + "type": "VariableDeclaration", + "type": "VariableDeclaration" + }, + { + "end": 44, + "expression": { + "arguments": [ + { + "end": 35, + "name": "f", + "start": 34, + "type": "Identifier", + "type": "Identifier" + }, + { + "elements": [ + { + "end": 39, + "raw": "0", + "start": 38, + "type": "Literal", + "type": "Literal", + "value": 0 + }, + { + "end": 42, + "raw": "1", + "start": 41, + "type": "Literal", + "type": "Literal", + "value": 1 + } + ], + "end": 43, + "start": 37, + "type": "ArrayExpression", + "type": "ArrayExpression" + } + ], + "callee": { + "end": 33, + "name": "map", + "start": 30, + "type": "Identifier" + }, + "end": 44, + "optional": false, + "start": 30, + "type": "CallExpression", + "type": "CallExpression" + }, + "start": 30, + "type": "ExpressionStatement", + "type": "ExpressionStatement" + } + ], + "end": 45, + "nonCodeMeta": { + "nonCodeNodes": { + "0": [ + { + "end": 30, + "start": 28, + "type": "NonCodeNode", + "value": { + "type": "newLine" + } + } + ] + }, + "startNodes": [] + }, + "start": 0 + } +} diff --git a/src/wasm-lib/kcl/tests/argument_error/execution_error.snap b/src/wasm-lib/kcl/tests/argument_error/execution_error.snap new file mode 100644 index 000000000..521767ab1 --- /dev/null +++ b/src/wasm-lib/kcl/tests/argument_error/execution_error.snap @@ -0,0 +1,5 @@ +--- +source: kcl/src/simulation_tests.rs +description: Error from executing argument_error.kcl +--- +type: KclErrorDetails { source_ranges: [SourceRange([34, 35, 0])], message: "Expected an array but found Function" } diff --git a/src/wasm-lib/kcl/tests/argument_error/input.kcl b/src/wasm-lib/kcl/tests/argument_error/input.kcl new file mode 100644 index 000000000..83c8aba1c --- /dev/null +++ b/src/wasm-lib/kcl/tests/argument_error/input.kcl @@ -0,0 +1,5 @@ +fn f = (i) => { + return 5 +} + +map(f, [0, 1]) diff --git a/src/wasm-lib/kcl/tests/argument_error/tokens.snap b/src/wasm-lib/kcl/tests/argument_error/tokens.snap new file mode 100644 index 000000000..9313eaeaa --- /dev/null +++ b/src/wasm-lib/kcl/tests/argument_error/tokens.snap @@ -0,0 +1,206 @@ +--- +source: kcl/src/simulation_tests.rs +description: Result of tokenizing argument_error.kcl +--- +{ + "Ok": [ + { + "type": "keyword", + "start": 0, + "end": 2, + "value": "fn" + }, + { + "type": "whitespace", + "start": 2, + "end": 3, + "value": " " + }, + { + "type": "word", + "start": 3, + "end": 4, + "value": "f" + }, + { + "type": "whitespace", + "start": 4, + "end": 5, + "value": " " + }, + { + "type": "operator", + "start": 5, + "end": 6, + "value": "=" + }, + { + "type": "whitespace", + "start": 6, + "end": 7, + "value": " " + }, + { + "type": "brace", + "start": 7, + "end": 8, + "value": "(" + }, + { + "type": "word", + "start": 8, + "end": 9, + "value": "i" + }, + { + "type": "brace", + "start": 9, + "end": 10, + "value": ")" + }, + { + "type": "whitespace", + "start": 10, + "end": 11, + "value": " " + }, + { + "type": "operator", + "start": 11, + "end": 13, + "value": "=>" + }, + { + "type": "whitespace", + "start": 13, + "end": 14, + "value": " " + }, + { + "type": "brace", + "start": 14, + "end": 15, + "value": "{" + }, + { + "type": "whitespace", + "start": 15, + "end": 18, + "value": "\n " + }, + { + "type": "keyword", + "start": 18, + "end": 24, + "value": "return" + }, + { + "type": "whitespace", + "start": 24, + "end": 25, + "value": " " + }, + { + "type": "number", + "start": 25, + "end": 26, + "value": "5" + }, + { + "type": "whitespace", + "start": 26, + "end": 27, + "value": "\n" + }, + { + "type": "brace", + "start": 27, + "end": 28, + "value": "}" + }, + { + "type": "whitespace", + "start": 28, + "end": 30, + "value": "\n\n" + }, + { + "type": "word", + "start": 30, + "end": 33, + "value": "map" + }, + { + "type": "brace", + "start": 33, + "end": 34, + "value": "(" + }, + { + "type": "word", + "start": 34, + "end": 35, + "value": "f" + }, + { + "type": "comma", + "start": 35, + "end": 36, + "value": "," + }, + { + "type": "whitespace", + "start": 36, + "end": 37, + "value": " " + }, + { + "type": "brace", + "start": 37, + "end": 38, + "value": "[" + }, + { + "type": "number", + "start": 38, + "end": 39, + "value": "0" + }, + { + "type": "comma", + "start": 39, + "end": 40, + "value": "," + }, + { + "type": "whitespace", + "start": 40, + "end": 41, + "value": " " + }, + { + "type": "number", + "start": 41, + "end": 42, + "value": "1" + }, + { + "type": "brace", + "start": 42, + "end": 43, + "value": "]" + }, + { + "type": "brace", + "start": 43, + "end": 44, + "value": ")" + }, + { + "type": "whitespace", + "start": 44, + "end": 45, + "value": "\n" + } + ] +} diff --git a/src/wasm-lib/tests/executor/main.rs b/src/wasm-lib/tests/executor/main.rs index b6526e838..430e80e75 100644 --- a/src/wasm-lib/tests/executor/main.rs +++ b/src/wasm-lib/tests/executor/main.rs @@ -866,7 +866,7 @@ part = rectShape([0, 0], 20, 20) assert!(result.is_err()); assert_eq!( result.err().unwrap().to_string(), - r#"semantic: KclErrorDetails { source_ranges: [SourceRange([863, 912, 0])], message: "Argument at index 0 was supposed to be type kcl_lib::std::shapes::CircleData but found string (text)" }"#, + r#"semantic: KclErrorDetails { source_ranges: [SourceRange([870, 874, 0])], message: "Argument at index 0 was supposed to be type kcl_lib::std::shapes::CircleData but found string (text)" }"#, ); } @@ -1351,7 +1351,7 @@ secondSketch = startSketchOn(part001, '') assert!(result.is_err()); assert_eq!( result.err().unwrap().to_string(), - r#"semantic: KclErrorDetails { source_ranges: [SourceRange([260, 286, 0])], message: "Argument at index 1 was supposed to be type Option but found string (text)" }"# + r#"semantic: KclErrorDetails { source_ranges: [SourceRange([283, 285, 0])], message: "Argument at index 1 was supposed to be type Option but found string (text)" }"# ); } @@ -1983,7 +1983,7 @@ someFunction('INVALID') assert!(result.is_err()); assert_eq!( result.err().unwrap().to_string(), - r#"semantic: KclErrorDetails { source_ranges: [SourceRange([37, 61, 0]), SourceRange([65, 88, 0])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but found string (text)" }"# + r#"semantic: KclErrorDetails { source_ranges: [SourceRange([51, 60, 0]), SourceRange([65, 88, 0])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but found string (text)" }"# ); } @@ -2004,7 +2004,7 @@ someFunction('INVALID') assert!(result.is_err()); assert_eq!( result.err().unwrap().to_string(), - r#"semantic: KclErrorDetails { source_ranges: [SourceRange([89, 114, 0]), SourceRange([126, 155, 0]), SourceRange([159, 182, 0])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but found string (text)" }"# + r#"semantic: KclErrorDetails { source_ranges: [SourceRange([103, 113, 0]), SourceRange([126, 155, 0]), SourceRange([159, 182, 0])], message: "Argument at index 0 was supposed to be type kcl_lib::std::sketch::SketchData but found string (text)" }"# ); }