Interpreter: add type checking for declared properties

issue #512

sixtyfps_runtime/interpreter/api.rs

@@ -1295,6 +1295,54 @@ fn globals() {
     );
 }
 
+#[test]
+fn component_definition_struct_properties() {
+    let mut compiler = ComponentCompiler::default();
+    let comp_def = spin_on::spin_on(
+        compiler.build_from_source(
+            r#"
+    export struct Settings := {
+        string_value: string,
+    }
+    export Dummy := Rectangle {
+        property <Settings> test;
+    }"#
+            .into(),
+            "".into(),
+        ),
+    )
+    .unwrap();
+
+    let props = comp_def.properties().collect::<Vec<(_, _)>>();
+
+    assert_eq!(props.len(), 1);
+    assert_eq!(props[0].0, "test");
+    assert_eq!(props[0].1, ValueType::Struct);
+
+    let instance = comp_def.create();
+
+    let valid_struct: Struct =
+        [("string_value".to_string(), Value::String("hello".into()))].iter().cloned().collect();
+
+    assert_eq!(instance.set_property("test", Value::Struct(valid_struct.clone())), Ok(()));
+    assert_eq!(instance.get_property("test").unwrap().value_type(), ValueType::Struct);
+
+    assert_eq!(instance.set_property("test", Value::Number(42.)), Err(SetPropertyError::WrongType));
+
+    let mut invalid_struct = valid_struct.clone();
+    invalid_struct.set_field("other".into(), Value::Number(44.));
+    assert_eq!(
+        instance.set_property("test", Value::Struct(invalid_struct)),
+        Err(SetPropertyError::WrongType)
+    );
+    let mut invalid_struct = valid_struct.clone();
+    invalid_struct.set_field("string_value".into(), Value::Number(44.));
+    assert_eq!(
+        instance.set_property("test", Value::Struct(invalid_struct)),
+        Err(SetPropertyError::WrongType)
+    );
+}
+
 #[cfg(feature = "ffi")]
 #[allow(missing_docs)]
 #[path = "ffi.rs"]

sixtyfps_runtime/interpreter/eval.rs

@@ -713,6 +713,19 @@ pub fn store_property(
     let component = element.borrow().enclosing_component.upgrade().unwrap();
     if element.borrow().id == component.root_element.borrow().id {
         if let Some(x) = enclosing_component.component_type.custom_properties.get(name) {
+            if let Some(orig_decl) = enclosing_component
+                .component_type
+                .original
+                .root_element
+                .borrow()
+                .property_declarations
+                .get(name)
+            {
+                // Do an extra type checking because PropertyInfo::set won't do it for custom structures or array
+                if !check_value_type(&value, &orig_decl.property_type) {
+                    return Err(SetPropertyError::WrongType);
+                }
+            }
             unsafe {
                 let p = Pin::new_unchecked(&*enclosing_component.as_ptr().add(x.offset));
                 return x
@@ -727,10 +740,54 @@ pub fn store_property(
     let item_info = &enclosing_component.component_type.items[element.borrow().id.as_str()];
     let item = unsafe { item_info.item_from_component(enclosing_component.as_ptr()) };
     let p = &item_info.rtti.properties.get(name).ok_or(SetPropertyError::NoSuchProperty)?;
-    p.set(item, value, maybe_animation.as_animation()).unwrap();
+    p.set(item, value, maybe_animation.as_animation()).map_err(|()| SetPropertyError::WrongType)?;
     Ok(())
 }
 
+fn check_value_type(value: &Value, ty: &Type) -> bool {
+    match ty {
+        Type::Void => true,
+        Type::Invalid
+        | Type::InferredProperty
+        | Type::InferredCallback
+        | Type::Component(_)
+        | Type::Builtin(_)
+        | Type::Native(_)
+        | Type::Callback { .. }
+        | Type::Function { .. }
+        | Type::ElementReference => panic!("not valid property type"),
+        Type::Float32 => matches!(value, Value::Number(_)),
+        Type::Int32 => matches!(value, Value::Number(_)),
+        Type::String => matches!(value, Value::String(_)),
+        Type::Color => matches!(value, Value::Brush(_)),
+        Type::UnitProduct(_)
+        | Type::Duration
+        | Type::PhysicalLength
+        | Type::LogicalLength
+        | Type::Angle
+        | Type::Percent => matches!(value, Value::Number(_)),
+        Type::Image => matches!(value, Value::Image(_)),
+        Type::Bool => matches!(value, Value::Bool(_)),
+        Type::Model => {
+            matches!(value, Value::Model(_) | Value::Bool(_) | Value::Number(_) | Value::Array(_))
+        }
+        Type::PathElements => matches!(value, Value::PathElements(_)),
+        Type::Easing => matches!(value, Value::EasingCurve(_)),
+        Type::Brush => matches!(value, Value::Brush(_)),
+        Type::Array(inner) => {
+            matches!(value, Value::Model(_))
+                || matches!(value, Value::Array(vec) if vec.iter().all(|x| check_value_type(x, inner)))
+        }
+        Type::Struct { fields, .. } => {
+            matches!(value, Value::Struct(str) if str.iter().all(|(k, v)| fields.get(k).map_or(false, |ty| check_value_type(v, ty))))
+        }
+        Type::Enumeration(en) => {
+            matches!(value, Value::EnumerationValue(name, _) if name == en.name.as_str())
+        }
+        Type::LayoutCache => matches!(value, Value::LayoutCache(_)),
+    }
+}
+
 pub(crate) fn invoke_callback(
     component_instance: ComponentInstance,
     element: &ElementRc,