Merge pull request #1573 from gtk-rs/bilelmoussaoui/props-fixes

Various properties fixes

src/analysis/object.rs

@@ -268,6 +268,7 @@ pub fn class(env: &Env, obj: &GObject, deps: &[library::TypeId]) -> Option<Info>
         &mut imports,
         &signatures,
         deps,
+        &functions,
     );
 
     let builder_properties =
@@ -429,6 +430,7 @@ pub fn interface(env: &Env, obj: &GObject, deps: &[library::TypeId]) -> Option<I
         &mut imports,
         &signatures,
         deps,
+        &functions,
     );
 
     let base = InfoBase {

src/analysis/properties.rs

@@ -10,7 +10,7 @@ use crate::{
         signatures::{Signature, Signatures},
         trampolines,
     },
-    config::{self, GObject, PropertyGenerateFlags},
+    config::{self, gobjects::GStatus, GObject, PropertyGenerateFlags},
     env::Env,
     library, nameutil,
     traits::*,
@@ -45,6 +45,7 @@ pub fn analyze(
     imports: &mut Imports,
     signatures: &Signatures,
     deps: &[library::TypeId],
+    functions: &[crate::analysis::functions::Info],
 ) -> (Vec<Property>, Vec<signals::Info>) {
     let mut properties = Vec::new();
     let mut notify_signals = Vec::new();
@@ -80,6 +81,7 @@ pub fn analyze(
             imports,
             signatures,
             deps,
+            functions,
         );
 
         if let Some(notify_signal) = notify_signal {
@@ -108,6 +110,7 @@ fn analyze_property(
     imports: &mut Imports,
     signatures: &Signatures,
     deps: &[library::TypeId],
+    functions: &[crate::analysis::functions::Info],
 ) -> (Option<Property>, Option<Property>, Option<signals::Info>) {
     let type_name = type_tid.full_name(&env.library);
     let name = prop.name.clone();
@@ -143,19 +146,6 @@ fn analyze_property(
     let mut set_func_name = format!("set_{name_for_func}");
     let mut set_prop_name = Some(format!("set_property_{name_for_func}"));
 
-    let has_getter = prop.getter.as_ref().is_some_and(|getter| {
-        obj.functions
-            .matched(getter)
-            .iter()
-            .all(|f| f.status.need_generate())
-    });
-    let has_setter = prop.setter.as_ref().is_some_and(|setter| {
-        obj.functions
-            .matched(setter)
-            .iter()
-            .all(|f| f.status.need_generate())
-    });
-
     let mut readable = prop.readable;
     let mut writable = if prop.construct_only {
         false
@@ -232,7 +222,20 @@ fn analyze_property(
 
     let (get_out_ref_mode, set_in_ref_mode, nullable) = get_property_ref_modes(env, prop);
 
-    let getter = if readable && !has_getter {
+    let getter_func = functions.iter().find(|f| {
+        f.get_property.as_ref() == Some(&prop.name) && Some(&f.name) == prop.getter.as_ref()
+    });
+    let setter_func = functions.iter().find(|f| {
+        f.set_property.as_ref() == Some(&prop.name) && Some(&f.name) == prop.setter.as_ref()
+    });
+
+    let has_getter =
+        getter_func.is_some_and(|g| matches!(g.status, GStatus::Generate | GStatus::Manual));
+    let has_setter =
+        setter_func.is_some_and(|s| matches!(s.status, GStatus::Generate | GStatus::Manual));
+
+    let getter = if readable && (!has_getter || prop.version < getter_func.and_then(|g| g.version))
+    {
         if let Ok(rust_type) = RustType::builder(env, prop.typ)
             .direction(library::ParameterDirection::Out)
             .try_build()
@@ -243,6 +246,17 @@ fn analyze_property(
             imports.add("glib::prelude::*");
         }
 
+        let mut getter_version = prop_version;
+        if has_getter {
+            let getter = getter_func.unwrap();
+            get_func_name = getter.new_name.as_ref().unwrap_or(&getter.name).to_string();
+            get_prop_name = Some(getter.name.clone());
+            getter_version = getter.version.map(|mut g| {
+                g.as_opposite();
+                g
+            });
+        }
+
         Some(Property {
             name: name.clone(),
             var_name: nameutil::mangle_keywords(&*name_for_func).into_owned(),
@@ -255,14 +269,15 @@ fn analyze_property(
             set_in_ref_mode,
             set_bound: None,
             bounds: Bounds::default(),
-            version: prop_version,
+            version: getter_version,
             deprecated_version: prop.deprecated_version,
         })
     } else {
         None
     };
 
-    let setter = if writable && !has_setter {
+    let setter = if writable && (!has_setter || prop.version < setter_func.and_then(|s| s.version))
+    {
         if let Ok(rust_type) = RustType::builder(env, prop.typ)
             .direction(library::ParameterDirection::In)
             .try_build()
@@ -284,6 +299,17 @@ fn analyze_property(
             }
         }
 
+        let mut setter_version = prop_version;
+        if has_setter {
+            let setter = setter_func.unwrap();
+            set_func_name = setter.new_name.as_ref().unwrap_or(&setter.name).to_string();
+            set_prop_name = Some(setter.name.clone());
+            setter_version = setter.version.map(|mut s| {
+                s.as_opposite();
+                s
+            });
+        }
+
         Some(Property {
             name: name.clone(),
             var_name: nameutil::mangle_keywords(&*name_for_func).into_owned(),
@@ -296,7 +322,7 @@ fn analyze_property(
             set_in_ref_mode,
             set_bound,
             bounds: Bounds::default(),
-            version: prop_version,
+            version: setter_version,
             deprecated_version: prop.deprecated_version,
         })
     } else {

src/config/functions.rs

@@ -527,7 +527,7 @@ version = "3.20"
 "#,
         );
         let f = Function::parse(&toml, "a").unwrap();
-        assert_eq!(f.version, Some(Version(3, 20, 0)));
+        assert_eq!(f.version, Some(Version::new(3, 20, 0)));
     }
 
     #[test]

src/config/members.rs

@@ -135,7 +135,7 @@ version = "3.20"
 "#,
         );
         let f = Member::parse(&toml, "a").unwrap();
-        assert_eq!(f.version, Some(Version(3, 20, 0)));
+        assert_eq!(f.version, Some(Version::new(3, 20, 0)));
     }
 
     #[test]

src/config/properties.rs

@@ -185,7 +185,7 @@ version = "3.20"
 "#,
         );
         let p = Property::parse(&toml, "a").unwrap();
-        assert_eq!(p.version, Some(Version(3, 20, 0)));
+        assert_eq!(p.version, Some(Version::new(3, 20, 0)));
     }
 
     #[test]

src/parser.rs

@@ -195,11 +195,13 @@ impl Library {
             "signal" => self
                 .read_signal(parser, ns_id, elem)
                 .map(|s| signals.push(s)),
-            "property" => self.read_property(parser, ns_id, elem).map(|p| {
-                if let Some(p) = p {
-                    properties.push(p);
-                }
-            }),
+            "property" => self
+                .read_property(parser, ns_id, elem, &symbol_prefix)
+                .map(|p| {
+                    if let Some(p) = p {
+                        properties.push(p);
+                    }
+                }),
             "field" => self.read_field(parser, ns_id, elem).map(|f| {
                 fields.push(f);
             }),
@@ -704,11 +706,13 @@ impl Library {
             "signal" => self
                 .read_signal(parser, ns_id, elem)
                 .map(|s| signals.push(s)),
-            "property" => self.read_property(parser, ns_id, elem).map(|p| {
-                if let Some(p) = p {
-                    properties.push(p);
-                }
-            }),
+            "property" => self
+                .read_property(parser, ns_id, elem, &symbol_prefix)
+                .map(|p| {
+                    if let Some(p) = p {
+                        properties.push(p);
+                    }
+                }),
             "doc" => parser.text().map(|t| doc = Some(t)),
             "doc-deprecated" => parser.text().map(|t| doc_deprecated = Some(t)),
             "virtual-method" => self
@@ -1079,14 +1083,8 @@ impl Library {
             _ => Err(parser.unexpected_element(elem)),
         })?;
 
-        let get_property = elem
-            .attr("get-property")
-            .map(ToString::to_string)
-            .or(gtk_get_property);
-        let set_property = elem
-            .attr("set-property")
-            .map(ToString::to_string)
-            .or(gtk_set_property);
+        let get_property = gtk_get_property.or(elem.attr("get-property").map(ToString::to_string));
+        let set_property = gtk_set_property.or(elem.attr("set-property").map(ToString::to_string));
         // The last argument of a callback is ALWAYS user data, so it has to be marked as such
         // in case it's missing.
         if is_callback && params.last().map(|x| x.closure.is_none()).unwrap_or(false) {
@@ -1357,6 +1355,7 @@ impl Library {
         parser: &mut XmlParser<'_>,
         ns_id: u16,
         elem: &Element,
+        symbol_prefix: &str,
     ) -> Result<Option<Property>, String> {
         let prop_name = elem.attr_required("name")?;
         let readable = elem.attr_bool("readable", true);
@@ -1400,11 +1399,19 @@ impl Library {
                 if let (Some(name), Some(value)) = (elem.attr("name"), elem.attr("value")) {
                     match name {
                         "org.gtk.Property.get" => {
-                            gtk_getter = Some(value.to_string());
+                            gtk_getter = value
+                                .split(symbol_prefix)
+                                .last()
+                                .and_then(|p| p.strip_prefix('_'))
+                                .map(|p| p.to_string());
                             Ok(())
                         }
                         "org.gtk.Property.set" => {
-                            gtk_setter = Some(value.to_string());
+                            gtk_setter = value
+                                .split(symbol_prefix)
+                                .last()
+                                .and_then(|p| p.strip_prefix('_'))
+                                .map(|p| p.to_string());
                             Ok(())
                         }
                         _ => parser.ignore_element(),
@@ -1416,8 +1423,8 @@ impl Library {
             _ => Err(parser.unexpected_element(elem)),
         })?;
 
-        let getter = elem.attr("getter").map(ToString::to_string).or(gtk_getter);
-        let setter = elem.attr("setter").map(ToString::to_string).or(gtk_setter);
+        let getter = gtk_getter.or(elem.attr("getter").map(ToString::to_string));
+        let setter = gtk_setter.or(elem.attr("setter").map(ToString::to_string));
         if has_empty_type_tag {
             return Ok(None);
         }

src/version.rs

@@ -5,9 +5,13 @@ use std::{
 
 /// Major, minor and patch version
 #[derive(Debug, Default, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub struct Version(pub u16, pub u16, pub u16);
+pub struct Version(u16, u16, u16, bool);
 
 impl Version {
+    pub fn new(major: u16, minor: u16, patch: u16) -> Self {
+        Self(major, minor, patch, true)
+    }
+
     /// Convert a version number to a config guard
     ///
     /// When generating a builder pattern, properties could be from a super-type
@@ -16,18 +20,28 @@ impl Version {
     /// is different from the main crate. For those cases you can pass
     /// the crate name as the `prefix` parameter
     pub fn to_cfg(self, prefix: Option<&str>) -> String {
-        if let Some(p) = prefix {
-            format!("feature = \"{}_{}\"", p, self.to_feature())
+        if self.3 {
+            if let Some(p) = prefix {
+                format!("feature = \"{}_{}\"", p, self.to_feature())
+            } else {
+                format!("feature = \"{}\"", self.to_feature())
+            }
+        } else if let Some(p) = prefix {
+            format!("not(feature = \"{}_{}\")", p, self.to_feature())
         } else {
-            format!("feature = \"{}\"", self.to_feature())
+            format!("not(feature = \"{}\")", self.to_feature())
         }
     }
 
+    pub fn as_opposite(&mut self) {
+        self.3 = !self.3;
+    }
+
     pub fn to_feature(self) -> String {
         match self {
-            Self(major, 0, 0) => format!("v{major}"),
-            Self(major, minor, 0) => format!("v{major}_{minor}"),
-            Self(major, minor, patch) => format!("v{major}_{minor}_{patch}"),
+            Self(major, 0, 0, _) => format!("v{major}"),
+            Self(major, minor, 0, _) => format!("v{major}_{minor}"),
+            Self(major, minor, patch, _) => format!("v{major}_{minor}_{patch}"),
         }
     }
 
@@ -56,24 +70,24 @@ impl FromStr for Version {
                 .map(str::parse)
                 .take_while(Result::is_ok)
                 .map(Result::unwrap);
-            Ok(Self(
+            Ok(Self::new(
                 parts.next().unwrap_or(0),
                 parts.next().unwrap_or(0),
                 parts.next().unwrap_or(0),
             ))
         } else {
             let val = s.parse::<u16>();
-            Ok(Self(val.unwrap_or(0), 0, 0))
+            Ok(Self::new(val.unwrap_or(0), 0, 0))
         }
     }
 }
 
 impl Display for Version {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), fmt::Error> {
         match *self {
-            Self(major, 0, 0) => write!(f, "{major}"),
-            Self(major, minor, 0) => write!(f, "{major}.{minor}"),
-            Self(major, minor, patch) => write!(f, "{major}.{minor}.{patch}"),
+            Self(major, 0, 0, _) => write!(f, "{major}"),
+            Self(major, minor, 0, _) => write!(f, "{major}.{minor}"),
+            Self(major, minor, patch, _) => write!(f, "{major}.{minor}.{patch}"),
         }
     }
 }
@@ -86,24 +100,24 @@ mod tests {
 
     #[test]
     fn from_str_works() {
-        assert_eq!(FromStr::from_str("1"), Ok(Version(1, 0, 0)));
-        assert_eq!(FromStr::from_str("2.1"), Ok(Version(2, 1, 0)));
-        assert_eq!(FromStr::from_str("3.2.1"), Ok(Version(3, 2, 1)));
-        assert_eq!(FromStr::from_str("3.ff.1"), Ok(Version(3, 0, 0)));
+        assert_eq!(FromStr::from_str("1"), Ok(Version::new(1, 0, 0)));
+        assert_eq!(FromStr::from_str("2.1"), Ok(Version::new(2, 1, 0)));
+        assert_eq!(FromStr::from_str("3.2.1"), Ok(Version::new(3, 2, 1)));
+        assert_eq!(FromStr::from_str("3.ff.1"), Ok(Version::new(3, 0, 0)));
     }
 
     #[test]
     fn parse_works() {
-        assert_eq!("1".parse(), Ok(Version(1, 0, 0)));
+        assert_eq!("1".parse(), Ok(Version::new(1, 0, 0)));
     }
 
     #[test]
     fn ord() {
-        assert!(Version(0, 0, 0) < Version(1, 2, 3));
-        assert!(Version(1, 0, 0) < Version(1, 2, 3));
-        assert!(Version(1, 2, 0) < Version(1, 2, 3));
-        assert!(Version(1, 2, 3) == Version(1, 2, 3));
-        assert!(Version(1, 0, 0) < Version(2, 0, 0));
-        assert!(Version(3, 0, 0) == Version(3, 0, 0));
+        assert!(Version::new(0, 0, 0) < Version::new(1, 2, 3));
+        assert!(Version::new(1, 0, 0) < Version::new(1, 2, 3));
+        assert!(Version::new(1, 2, 0) < Version::new(1, 2, 3));
+        assert!(Version::new(1, 2, 3) == Version::new(1, 2, 3));
+        assert!(Version::new(1, 0, 0) < Version::new(2, 0, 0));
+        assert!(Version::new(3, 0, 0) == Version::new(3, 0, 0));
     }
 }