Merge pull request gtk-rs#774 from sdroege/dont-allow-construct-property-twice-on-construction

glib: Don't allow passing a construct(-only) property twice during ob…

Author: GuillaumeGomez

glib/src/object.rs

@@ -1365,18 +1365,33 @@ impl Object {
                 .ok_or_else(|| bool_error!("Can't retrieve class for type '{}'", type_))?;
             let pspecs = klass.list_properties();
 
-            properties
-                .iter()
-                .map(|(name, value)| {
-                    let pspec = pspecs.iter().find(|p| p.name() == *name).ok_or_else(|| {
-                        bool_error!("Can't find property '{}' for type '{}'", name, type_)
-                    })?;
-
-                    let mut value = value.to_value();
-                    validate_property_type(type_, true, pspec, &mut value)?;
-                    Ok((pspec.name().as_ptr(), value))
-                })
-                .collect::<Result<smallvec::SmallVec<[_; 10]>, _>>()?
+            let mut validated_properties =
+                smallvec::SmallVec::<[_; 16]>::with_capacity(properties.len());
+            for (idx, (name, value)) in properties.iter().enumerate() {
+                let pspec = pspecs.iter().find(|p| p.name() == *name).ok_or_else(|| {
+                    bool_error!("Can't find property '{}' for type '{}'", name, type_)
+                })?;
+
+                if (pspec.flags().contains(crate::ParamFlags::CONSTRUCT)
+                    || pspec.flags().contains(crate::ParamFlags::CONSTRUCT_ONLY))
+                    && properties[0..idx]
+                        .iter()
+                        .any(|(other_name, _)| name == other_name)
+                {
+                    return Err(bool_error!(
+                        "Can't set construct property '{}' for type '{}' twice",
+                        name,
+                        type_
+                    ));
+                }
+
+                let mut value = value.to_value();
+                validate_property_type(type_, true, pspec, &mut value)?;
+
+                validated_properties.push((pspec.name().as_ptr(), value));
+            }
+
+            validated_properties
         } else {
             smallvec::SmallVec::new()
         };
@@ -1395,18 +1410,33 @@ impl Object {
                 .ok_or_else(|| bool_error!("Can't retrieve class for type '{}'", type_))?;
             let pspecs = klass.list_properties();
 
-            properties
-                .iter()
-                .map(|(name, value)| {
-                    let pspec = pspecs.iter().find(|p| p.name() == *name).ok_or_else(|| {
-                        bool_error!("Can't find property '{}' for type '{}'", name, type_)
-                    })?;
-
-                    let mut value = value.clone();
-                    validate_property_type(type_, true, pspec, &mut value)?;
-                    Ok((pspec.name().as_ptr(), value))
-                })
-                .collect::<Result<smallvec::SmallVec<[_; 10]>, _>>()?
+            let mut validated_properties =
+                smallvec::SmallVec::<[_; 16]>::with_capacity(properties.len());
+            for (idx, (name, value)) in properties.iter().enumerate() {
+                let pspec = pspecs.iter().find(|p| p.name() == *name).ok_or_else(|| {
+                    bool_error!("Can't find property '{}' for type '{}'", name, type_)
+                })?;
+
+                if (pspec.flags().contains(crate::ParamFlags::CONSTRUCT)
+                    || pspec.flags().contains(crate::ParamFlags::CONSTRUCT_ONLY))
+                    && properties[0..idx]
+                        .iter()
+                        .any(|(other_name, _)| name == other_name)
+                {
+                    return Err(bool_error!(
+                        "Can't set construct property '{}' for type '{}' twice",
+                        name,
+                        type_
+                    ));
+                }
+
+                let mut value = value.clone();
+                validate_property_type(type_, true, pspec, &mut value)?;
+
+                validated_properties.push((pspec.name().as_ptr(), value));
+            }
+
+            validated_properties
         } else {
             smallvec::SmallVec::new()
         };

glib/src/subclass/object.rs

@@ -527,6 +527,19 @@ mod test {
         assert!(obj.try_set_property("child", &child).is_ok());
     }
 
+    #[test]
+    fn test_construct_property_set_twice() {
+        assert_eq!(
+            Object::with_type(
+                SimpleObject::static_type(),
+                &[("construct-name", &"meh"), ("construct-name", &"meh2")],
+            )
+            .expect_err("Can't set construct property twice")
+            .to_string(),
+            "Can't set construct property 'construct-name' for type 'SimpleObject' twice",
+        );
+    }
+
     #[test]
     fn test_signals() {
         use std::sync::atomic::{AtomicBool, Ordering};