Fix computed properties of KCL objects (#3246)

* Fix computed properties of KCL objects

Fixes https://github.com/KittyCAD/modeling-app/issues/3201

* Incorporate Jon's feedback

src/wasm-lib/kcl/src/ast/types.rs

@@ -2834,31 +2834,94 @@ impl MemberExpression {
     }
 
     pub fn get_result(&self, memory: &mut ProgramMemory) -> Result<MemoryItem, KclError> {
-        let property_name = match &self.property {
-            LiteralIdentifier::Identifier(identifier) => identifier.name.to_string(),
+        #[derive(Debug)]
+        enum Property {
+            Number(usize),
+            String(String),
+        }
+
+        impl Property {
+            fn type_name(&self) -> &'static str {
+                match self {
+                    Property::Number(_) => "number",
+                    Property::String(_) => "string",
+                }
+            }
+        }
+
+        let property_src: SourceRange = self.property.clone().into();
+        let property_sr = vec![property_src];
+
+        let property: Property = match self.property.clone() {
+            LiteralIdentifier::Identifier(identifier) => {
+                let name = identifier.name;
+                if !self.computed {
+                    // Treat the property as a literal
+                    Property::String(name.to_string())
+                } else {
+                    // 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 {
+                            source_ranges: property_sr,
+                            message: format!(
+                                "{name} is not a valid property/index, you can only use a string or int (>= 0) here",
+                            ),
+                        }));
+                    };
+                    match prop.value {
+                        JValue::Number(ref num) => {
+                            num
+                                .as_u64()
+                                .and_then(|x| usize::try_from(x).ok())
+                                .map(Property::Number)
+                                .ok_or_else(|| {
+                                    KclError::Syntax(KclErrorDetails {
+                                        source_ranges: property_sr,
+                                        message: format!(
+                                            "{name} 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 {
+                                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",
+                                ),
+                            }));
+                        }
+                    }
+                }
+            }
             LiteralIdentifier::Literal(literal) => {
                 let value = literal.value.clone();
                 match value {
-                    LiteralValue::IInteger(x) if x >= 0 => return self.get_result_array(memory, x as usize),
                     LiteralValue::IInteger(x) => {
+                        if let Ok(x) = u64::try_from(x) {
+                            Property::Number(x.try_into().unwrap())
+                        } else {
+                            return Err(KclError::Syntax(KclErrorDetails {
+                                source_ranges: property_sr,
+                                message: format!("{x} is not a valid index, indices must be whole numbers >= 0"),
+                            }));
+                        }
+                    }
+                    LiteralValue::String(s) => Property::String(s),
+                    _ => {
                         return Err(KclError::Syntax(KclErrorDetails {
                             source_ranges: vec![self.into()],
-                            message: format!("invalid index: {x}"),
-                        }))
+                            message: "Only strings or ints (>= 0) can be properties/indexes".to_owned(),
+                        }));
                     }
-                    LiteralValue::Fractional(x) => {
-                        return Err(KclError::Syntax(KclErrorDetails {
-                            source_ranges: vec![self.into()],
-                            message: format!("invalid index: {x}"),
-                        }))
-                    }
-                    LiteralValue::String(s) => s,
-                    LiteralValue::Bool(b) => b.to_string(),
                 }
             }
         };
 
         let object = match &self.object {
+            // TODO: Don't use recursion here, use a loop.
             MemberObject::MemberExpression(member_expr) => member_expr.get_result(memory)?,
             MemberObject::Identifier(identifier) => {
                 let value = memory.get(&identifier.name, identifier.into())?;
@@ -2868,25 +2931,57 @@ impl MemberExpression {
 
         let object_json = object.get_json_value()?;
 
-        if let serde_json::Value::Object(map) = object_json {
-            if let Some(value) = map.get(&property_name) {
-                Ok(MemoryItem::UserVal(UserVal {
-                    value: value.clone(),
-                    meta: vec![Metadata {
-                        source_range: self.into(),
-                    }],
-                }))
-            } else {
-                Err(KclError::UndefinedValue(KclErrorDetails {
-                    message: format!("Property {} not found in object", property_name),
-                    source_ranges: vec![self.clone().into()],
-                }))
+        // Check the property and object match -- e.g. ints for arrays, strs for objects.
+        match (object_json, property) {
+            (JValue::Object(map), Property::String(property)) => {
+                if let Some(value) = map.get(&property) {
+                    Ok(MemoryItem::UserVal(UserVal {
+                        value: value.clone(),
+                        meta: vec![Metadata {
+                            source_range: self.into(),
+                        }],
+                    }))
+                } else {
+                    Err(KclError::UndefinedValue(KclErrorDetails {
+                        message: format!("Property {property} not found in object"),
+                        source_ranges: vec![self.clone().into()],
+                    }))
+                }
             }
-        } else {
-            Err(KclError::Semantic(KclErrorDetails {
-                message: format!("MemberExpression object is not an object: {:?}", object),
+            (JValue::Object(_), p) => Err(KclError::Semantic(KclErrorDetails {
+                message: format!(
+                    "Only strings can be used as the property of an object, but you're using a {}",
+                    p.type_name()
+                ),
                 source_ranges: vec![self.clone().into()],
-            }))
+            })),
+            (JValue::Array(arr), Property::Number(index)) => {
+                let value_of_arr: Option<&JValue> = arr.get(index);
+                if let Some(value) = value_of_arr {
+                    Ok(MemoryItem::UserVal(UserVal {
+                        value: value.clone(),
+                        meta: vec![Metadata {
+                            source_range: self.into(),
+                        }],
+                    }))
+                } else {
+                    Err(KclError::UndefinedValue(KclErrorDetails {
+                        message: format!("The array doesn't have any item at index {index}"),
+                        source_ranges: vec![self.clone().into()],
+                    }))
+                }
+            }
+            (JValue::Array(_), p) => Err(KclError::Semantic(KclErrorDetails {
+                message: format!(
+                    "Only integers >= 0 can be used as the index of an array, but you're using a {}",
+                    p.type_name()
+                ),
+                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()],
+            })),
         }
     }
 

src/wasm-lib/tests/executor/inputs/computed_var.kcl

@@ -0,0 +1,18 @@
+// This tests computed properties. 
+
+const arr = [0, 0, 0, 10]
+const i = 3
+const ten = arr[i]
+
+assertLessThanOrEq(ten, 10, "oops")
+assertGreaterThanOrEq(ten, 10, "oops2")
+
+const p = "foo"
+const obj = {
+  foo: 1,
+  bar: 0,
+}
+const one = obj[p]
+
+assertLessThanOrEq(one, 1, "oops")
+assertGreaterThanOrEq(one, 1, "oops2")

src/wasm-lib/tests/executor/main.rs

@@ -38,6 +38,13 @@ async fn kcl_test_lego() {
     assert_out("lego", &result);
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn kcl_test_computed_var() {
+    let code = kcl_input!("computed_var");
+    let result = execute_and_snapshot(code, UnitLength::Mm).await.unwrap();
+    assert_out("computed_var", &result);
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn kcl_test_pipe_as_arg() {
     let code = kcl_input!("pipe_as_arg");