diff --git a/src/wasm-lib/kcl/src/ast/types.rs b/src/wasm-lib/kcl/src/ast/types.rs index aaaff9971..d4f489522 100644 --- a/src/wasm-lib/kcl/src/ast/types.rs +++ b/src/wasm-lib/kcl/src/ast/types.rs @@ -2862,7 +2862,7 @@ impl MemberExpression { // Actually evaluate memory to compute the property. let prop = memory.get(&name, property_src)?; let MemoryItem::UserVal(prop) = prop else { - return Err(KclError::Syntax(KclErrorDetails { + return Err(KclError::Semantic(KclErrorDetails { source_ranges: property_sr, message: format!( "{name} is not a valid property/index, you can only use a string or int (>= 0) here", @@ -2876,17 +2876,17 @@ impl MemberExpression { .and_then(|x| usize::try_from(x).ok()) .map(Property::Number) .ok_or_else(|| { - KclError::Syntax(KclErrorDetails { + KclError::Semantic(KclErrorDetails { source_ranges: property_sr, message: format!( - "{name} is not a valid property/index, you can only use a string or int (>= 0) here", + "{name}'s value is not a valid property/index, you can only use a string or int (>= 0) here", ), }) })? } JValue::String(ref x) => Property::String(x.to_owned()), _ => { - return Err(KclError::Syntax(KclErrorDetails { + return Err(KclError::Semantic(KclErrorDetails { source_ranges: property_sr, message: format!( "{name} is not a valid property/index, you can only use a string to get the property of an object, or an int (>= 0) to get an item in an array", @@ -2903,7 +2903,7 @@ impl MemberExpression { if let Ok(x) = u64::try_from(x) { Property::Number(x.try_into().unwrap()) } else { - return Err(KclError::Syntax(KclErrorDetails { + return Err(KclError::Semantic(KclErrorDetails { source_ranges: property_sr, message: format!("{x} is not a valid index, indices must be whole numbers >= 0"), })); @@ -2911,7 +2911,7 @@ impl MemberExpression { } LiteralValue::String(s) => Property::String(s), _ => { - return Err(KclError::Syntax(KclErrorDetails { + return Err(KclError::Semantic(KclErrorDetails { source_ranges: vec![self.into()], message: "Only strings or ints (>= 0) can be properties/indexes".to_owned(), })); @@ -2943,7 +2943,7 @@ impl MemberExpression { })) } else { Err(KclError::UndefinedValue(KclErrorDetails { - message: format!("Property {property} not found in object"), + message: format!("Property '{property}' not found in object"), source_ranges: vec![self.clone().into()], })) } @@ -2978,10 +2978,13 @@ impl MemberExpression { ), source_ranges: vec![self.clone().into()], })), - (_, _) => Err(KclError::Semantic(KclErrorDetails { - message: "Only arrays and objects can be indexed".to_owned(), - source_ranges: vec![self.clone().into()], - })), + (being_indexed, _) => { + let t = human_friendly_type(being_indexed); + Err(KclError::Semantic(KclErrorDetails { + message: format!("Only arrays and objects can be indexed, but you're trying to index a {t}"), + source_ranges: vec![self.clone().into()], + })) + } } } @@ -4070,6 +4073,17 @@ impl ConstraintLevels { } } +fn human_friendly_type(j: JValue) -> &'static str { + match j { + JValue::Null => "null", + JValue::Bool(_) => "boolean (true/false value)", + JValue::Number(_) => "number", + JValue::String(_) => "string (text)", + JValue::Array(_) => "array (list)", + JValue::Object(_) => "object", + } +} + #[cfg(test)] mod tests { use pretty_assertions::assert_eq; diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/array_index_oob.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/array_index_oob.kcl new file mode 100644 index 000000000..1cd77ca46 --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/array_index_oob.kcl @@ -0,0 +1,2 @@ +let arr = [] +let x = arr[0] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/index_of_array.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/index_of_array.kcl new file mode 100644 index 000000000..19d2d35be --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/index_of_array.kcl @@ -0,0 +1,18 @@ +// This tests indexing an array. + +const array = [90, 91, 92] + +// Test: literal index. + +const result0 = array[1] + +assertLessThanOrEq(result0, 91, "Literal property lookup") +assertGreaterThanOrEq(result0, 91, "Literal property lookup") + +// Test: computed index. + +const i = int(1 + 0) +const result1 = array[i] + +assertLessThanOrEq(result1, 91, "Computed property lookup") +assertGreaterThanOrEq(result1, 91, "Computed property lookup") diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_fractional.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_fractional.kcl new file mode 100644 index 000000000..eefab701d --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_fractional.kcl @@ -0,0 +1,2 @@ +let arr = [1, 2, 3] +let x = arr[1.2] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_negative.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_negative.kcl new file mode 100644 index 000000000..19a0fd9a1 --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_negative.kcl @@ -0,0 +1,3 @@ +let arr = [1, 2, 3] +let i = -1 +let x = arr[i] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_str.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_str.kcl new file mode 100644 index 000000000..21ab041ef --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_index_str.kcl @@ -0,0 +1,2 @@ +let arr = [1, 2, 3] +let x = arr["s"] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_member_object.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_member_object.kcl new file mode 100644 index 000000000..0348e9c51 --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_member_object.kcl @@ -0,0 +1,2 @@ +let num = 999 +let x = num[3] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_member_object_prop.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_member_object_prop.kcl new file mode 100644 index 000000000..e01d88d11 --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/invalid_member_object_prop.kcl @@ -0,0 +1,2 @@ +let b = true +let x = b["property"] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/non_string_key_of_object.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/non_string_key_of_object.kcl new file mode 100644 index 000000000..778ef6732 --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/non_string_key_of_object.kcl @@ -0,0 +1,2 @@ +let obj = {key: 123} +let num = obj[3] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/object_prop_not_found.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/object_prop_not_found.kcl new file mode 100644 index 000000000..83b814999 --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/object_prop_not_found.kcl @@ -0,0 +1,2 @@ +let obj = {} +let k = obj["age"] diff --git a/src/wasm-lib/tests/executor/inputs/no_visuals/property_of_object.kcl b/src/wasm-lib/tests/executor/inputs/no_visuals/property_of_object.kcl new file mode 100644 index 000000000..70867e84d --- /dev/null +++ b/src/wasm-lib/tests/executor/inputs/no_visuals/property_of_object.kcl @@ -0,0 +1,40 @@ +// This tests evaluating properties of objects. + +const obj = { + foo: 1, + bar: 0, +} + +// Test: the property is a literal. + +const one_a = obj["foo"] + +assertLessThanOrEq(one_a, 1, "Literal property lookup") +assertGreaterThanOrEq(one_a, 1, "Literal property lookup") + +// Test: the property is a variable, +// which must be evaluated before looking it up. + +const p = "foo" +const one_b = obj[p] + +assertLessThanOrEq(one_b, 1, "Computed property lookup") +assertGreaterThanOrEq(one_b, 1, "Computed property lookup") + +// Test: multiple literal properties. + +const obj2 = { + inner: obj, +} + +const one_c = obj2.inner["foo"] + +assertLessThanOrEq(one_c, 1, "Literal property lookup") +assertGreaterThanOrEq(one_c, 1, "Literal property lookup") + +// Test: multiple properties, mix of literal and computed. + +const one_d = obj2.inner[p] + +assertLessThanOrEq(one_d, 1, "Computed property lookup") +assertGreaterThanOrEq(one_d, 1, "Computed property lookup") diff --git a/src/wasm-lib/tests/executor/main.rs b/src/wasm-lib/tests/executor/main.rs index 0d1ed3b47..17186b26c 100644 --- a/src/wasm-lib/tests/executor/main.rs +++ b/src/wasm-lib/tests/executor/main.rs @@ -4,7 +4,7 @@ use kcl_lib::{settings::types::UnitLength, test_server::execute_and_snapshot}; /// i.e. how different the current model snapshot can be from the previous saved one. const MIN_DIFF: f64 = 0.99; -// mod server; +mod no_visuals; macro_rules! kcl_input { ($file:literal) => { diff --git a/src/wasm-lib/tests/executor/no_visuals.rs b/src/wasm-lib/tests/executor/no_visuals.rs new file mode 100644 index 000000000..f5df78f9f --- /dev/null +++ b/src/wasm-lib/tests/executor/no_visuals.rs @@ -0,0 +1,87 @@ +use kcl_lib::{ast::types::Program, errors::KclError, executor::ExecutorContext}; + +macro_rules! gen_test { + ($file:ident) => { + #[tokio::test] + async fn $file() { + let code = include_str!(concat!("inputs/no_visuals/", stringify!($file), ".kcl")); + run(&code).await; + } + }; +} + +macro_rules! gen_test_fail { + ($file:ident, $expected:literal) => { + #[tokio::test] + async fn $file() { + let code = include_str!(concat!("inputs/no_visuals/", stringify!($file), ".kcl")); + let actual = run_fail(&code).await; + assert_eq!(actual.get_message(), $expected); + } + }; +} + +async fn run(code: &str) { + let (ctx, program) = setup(code).await; + + ctx.run(&program, None).await.unwrap(); +} + +async fn setup(program: &str) -> (ExecutorContext, Program) { + let tokens = kcl_lib::token::lexer(program).unwrap(); + let parser = kcl_lib::parser::Parser::new(tokens); + let program = parser.ast().unwrap(); + let ctx = kcl_lib::executor::ExecutorContext { + engine: std::sync::Arc::new(Box::new( + kcl_lib::engine::conn_mock::EngineConnection::new().await.unwrap(), + )), + fs: std::sync::Arc::new(kcl_lib::fs::FileManager::new()), + stdlib: std::sync::Arc::new(kcl_lib::std::StdLib::new()), + settings: Default::default(), + is_mock: true, + }; + (ctx, program) +} + +async fn run_fail(code: &str) -> KclError { + let (ctx, program) = setup(code).await; + let Err(e) = ctx.run(&program, None).await else { + panic!("Expected this KCL program to fail, but it (incorrectly) never threw an error."); + }; + e +} + +gen_test!(property_of_object); +gen_test!(index_of_array); +gen_test_fail!( + invalid_index_str, + "semantic: Only integers >= 0 can be used as the index of an array, but you're using a string" +); +gen_test_fail!( + invalid_index_negative, + "semantic: i's value is not a valid property/index, you can only use a string or int (>= 0) here" +); +gen_test_fail!( + invalid_index_fractional, + "semantic: Only strings or ints (>= 0) can be properties/indexes" +); +gen_test_fail!( + invalid_member_object, + "semantic: Only arrays and objects can be indexed, but you're trying to index a number" +); +gen_test_fail!( + invalid_member_object_prop, + "semantic: Only arrays and objects can be indexed, but you're trying to index a boolean (true/false value)" +); +gen_test_fail!( + non_string_key_of_object, + "semantic: Only strings can be used as the property of an object, but you're using a number" +); +gen_test_fail!( + array_index_oob, + "undefined value: The array doesn't have any item at index 0" +); +gen_test_fail!( + object_prop_not_found, + "undefined value: Property 'age' not found in object" +);