Simplify usage of attributes. (#574)

* Support implicit values for primitive attributes

This commit adds support for strings, numbers, and booleans to be implicitly typed in attribute maps, reducing the redundancy of needing to specify their types.

I also quietly adjusted one of the tests to use a more stable class/property pair. Since SourceAssetId is locked to Roblox, it could potentially disappear at any time.

* Apply formatting.

* Address feedback

* Backwards compatible format usage.

* Axe UnresolvedValueMap in favor of $attributes

Attributes can be defined directly on instances, with support for unambiguous types.

* Adjust test.

* to_string() -> into()

* Made attribute test more concise.

* small cleanup

* Update src/resolution.rs

* Update src/resolution.rs

* Update src/resolution.rs

* Update src/resolution.rs

Co-authored-by: Lucien Greathouse <me@lpghatguy.com>

rojo-test/build-test-snapshots/end_to_end__tests__build__attributes.snap

@@ -1,6 +1,5 @@
 ---
 source: tests/tests/build.rs
-assertion_line: 99
 expression: contents
 ---
 <roblox version="4">
@@ -11,12 +10,24 @@ expression: contents
     <Item class="Folder" referent="1">
       <Properties>
         <string name="Name">Explicit</string>
-        <BinaryString name="AttributesSerialize">AgAAAAUAAABIZWxsbwIFAAAAV29ybGQGAAAAVmVjdG9yEQAAgD8AAABAAABAQA==</BinaryString>
+        <BinaryString name="AttributesSerialize">DQAAAAQAAABCb29sAwEKAAAAQnJpY2tDb2xvcg4BAAAABgAAAENvbG9yMw8AAAAAAAAAAAAAAAANAAAAQ29sb3JTZXF1ZW5jZRkCAAAAAAAAAAAAAAAAAIA/AACAPwAAgD8AAAAAAACAPwAAgD8AAIA/AACAPwcAAABGbG9hdDMyBQAAAAAHAAAARmxvYXQ2NAYAAAAAAAAAAAsAAABOdW1iZXJSYW5nZRsAAAAAAAAAAA4AAABOdW1iZXJTZXF1ZW5jZRcCAAAAAAAAAAAAAAAAAAAAAAAAAAAAgD8AAAAABAAAAFJlY3QcAAAAAAAAAAAAAAAAAAAAAAQAAABVRGltCQAAAAAAAAAABQAAAFVEaW0yCgAAAAAAAAAAAAAAAAAAAAAHAAAAVmVjdG9yMhAAAAAAAAAAAAcAAABWZWN0b3IzEQAAAAAAAAAAAAAAAA==</BinaryString>
       </Properties>
     </Item>
     <Item class="Folder" referent="2">
       <Properties>
-        <string name="Name">ImplicitAttributes</string>
+        <string name="Name">Implicit</string>
+        <BinaryString name="AttributesSerialize">AwAAAAQAAABCb29sAwEGAAAATnVtYmVyBgAAAAAAAOA/BgAAAFN0cmluZwIEAAAAVGVzdA==</BinaryString>
+      </Properties>
+    </Item>
+    <Item class="Folder" referent="3">
+      <Properties>
+        <string name="Name">LegacyExplicit</string>
+        <BinaryString name="AttributesSerialize">AgAAAAUAAABIZWxsbwIFAAAAV29ybGQGAAAAVmVjdG9yEQAAgD8AAABAAABAQA==</BinaryString>
+      </Properties>
+    </Item>
+    <Item class="Folder" referent="4">
+      <Properties>
+        <string name="Name">LegacyImplicit</string>
         <BinaryString name="AttributesSerialize">AgAAAAMAAABIZXkCBwAAAEdyYW5kbWEGAAAAVmVjdG9yEQAAgEAAAKBAAADAQA==</BinaryString>
       </Properties>
     </Item>

rojo-test/build-tests/attributes/default.project.json

@@ -3,7 +3,85 @@
   "tree": {
     "$className": "Folder",
 
+    "Implicit": {
+      "$className": "Folder",
+      "$attributes": {
+        "Bool": true,
+        "Number": 0.5,
+        "String": "Test"
+      }
+    },
+
     "Explicit": {
+      "$className": "Folder",
+      "$attributes": {
+        "Bool": {
+          "Bool": true 
+        },
+        "Float32": {
+          "Float32": 0
+        },
+        "Float64": {
+          "Float64": 0
+        },
+        "UDim": {
+          "UDim": [0, 0]
+        },
+        "UDim2": {
+          "UDim2": [[0, 0], [0, 0]]
+        },
+        "BrickColor": {
+          "BrickColor": 1
+        },
+        "Color3": {
+          "Color3": [0, 0, 0]
+        },
+        "Vector2": {
+          "Vector2": [0, 0] 
+        },
+        "Vector3": {
+          "Vector3": [0, 0, 0]
+        },
+        "NumberSequence": {
+          "NumberSequence": {
+            "keypoints": [
+              {
+                "time": 0,
+                "value": 0,
+                "envelope": 0
+              },
+              {
+                "time": 1,
+                "value": 0,
+                "envelope": 0
+              }
+            ]
+          }
+        },
+        "ColorSequence": {
+          "ColorSequence": {
+            "keypoints": [
+              {
+                "time": 0,
+                "color": [1, 1, 1]
+              },
+              {
+                "time": 1,
+                "color": [1, 1, 1]
+              }
+            ]
+          }
+        },
+        "NumberRange": {
+          "NumberRange": [0, 0]
+        },
+        "Rect": {
+          "Rect": [[0, 0], [0, 0]]
+        }
+      }
+    },
+
+    "LegacyExplicit": {
       "$className": "Folder",
       "$properties": {
         "Attributes": {
@@ -19,7 +97,7 @@
       }
     },
 
-    "ImplicitAttributes": {
+    "LegacyImplicit": {
       "$className": "Folder",
       "$properties": {
         "Attributes": {

src/project.rs

@@ -229,6 +229,13 @@ pub struct ProjectNode {
     )]
     pub properties: HashMap<String, UnresolvedValue>,
 
+    #[serde(
+        rename = "$attributes",
+        default,
+        skip_serializing_if = "HashMap::is_empty"
+    )]
+    pub attributes: HashMap<String, UnresolvedValue>,
+
     /// Defines the behavior when Rojo encounters unknown instances in Roblox
     /// Studio during live sync. `$ignoreUnknownInstances` should be considered
     /// a large hammer and used with care.

src/resolution.rs

@@ -1,6 +1,6 @@
 use std::borrow::Borrow;
 
-use anyhow::format_err;
+use anyhow::{bail, format_err};
 use rbx_dom_weak::types::{
     Attributes, CFrame, Color3, Content, Enum, Matrix3, Tags, Variant, VariantType, Vector2,
     Vector3,
@@ -28,6 +28,13 @@ impl UnresolvedValue {
             UnresolvedValue::Ambiguous(partial) => partial.resolve(class_name, prop_name),
         }
     }
+
+    pub fn resolve_unambiguous(self) -> anyhow::Result<Variant> {
+        match self {
+            UnresolvedValue::FullyQualified(full) => Ok(full),
+            UnresolvedValue::Ambiguous(partial) => partial.resolve_unambiguous(),
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -148,6 +155,16 @@ impl AmbiguousValue {
         }
     }
 
+    pub fn resolve_unambiguous(self) -> anyhow::Result<Variant> {
+        match self {
+            AmbiguousValue::Bool(value) => Ok(value.into()),
+            AmbiguousValue::Number(value) => Ok(value.into()),
+            AmbiguousValue::String(value) => Ok(value.into()),
+
+            other => bail!("Cannot unambiguously resolve the value {other:?}"),
+        }
+    }
+
     fn describe(&self) -> &'static str {
         match self {
             AmbiguousValue::Bool(_) => "a bool",
@@ -218,12 +235,20 @@ mod test {
         unresolved.resolve(class, prop).unwrap()
     }
 
+    fn resolve_unambiguous(json_value: &str) -> Variant {
+        let unresolved: UnresolvedValue = serde_json::from_str(json_value).unwrap();
+        unresolved.resolve_unambiguous().unwrap()
+    }
+
     #[test]
     fn bools() {
         assert_eq!(resolve("BoolValue", "Value", "false"), Variant::Bool(false));
 
         // Script.Disabled is inherited from BaseScript
         assert_eq!(resolve("Script", "Disabled", "true"), Variant::Bool(true));
+
+        assert_eq!(resolve_unambiguous("false"), Variant::Bool(false));
+        assert_eq!(resolve_unambiguous("true"), Variant::Bool(true));
     }
 
     #[test]
@@ -247,6 +272,11 @@ mod test {
         //     resolve("Folder", "Tags", "\"a\\u0000b\\u0000c\""),
         //     Variant::BinaryString(b"a\0b\0c".to_vec().into()),
         // );
+
+        assert_eq!(
+            resolve_unambiguous("\"Hello world!\""),
+            Variant::String("Hello world!".into()),
+        );
     }
 
     #[test]
@@ -257,12 +287,14 @@ mod test {
         );
 
         assert_eq!(
-            resolve("Folder", "SourceAssetId", "532413"),
+            resolve("IntValue", "Value", "532413"),
             Variant::Int64(532413),
         );
 
         assert_eq!(resolve("Part", "Transparency", "1"), Variant::Float32(1.0));
         assert_eq!(resolve("NumberValue", "Value", "1"), Variant::Float64(1.0));
+
+        assert_eq!(resolve_unambiguous("12.5"), Variant::Float64(12.5));
     }
 
     #[test]

src/snapshot_middleware/json_model.rs

@@ -2,6 +2,7 @@ use std::{borrow::Cow, collections::HashMap, path::Path, str};
 
 use anyhow::Context;
 use memofs::Vfs;
+use rbx_dom_weak::types::Attributes;
 use serde::Deserialize;
 
 use crate::{
@@ -78,6 +79,9 @@ struct JsonModel {
         skip_serializing_if = "HashMap::is_empty"
     )]
     properties: HashMap<String, UnresolvedValue>,
+
+    #[serde(default = "HashMap::new", skip_serializing_if = "HashMap::is_empty")]
+    attributes: HashMap<String, UnresolvedValue>,
 }
 
 impl JsonModel {
@@ -96,6 +100,17 @@ impl JsonModel {
             properties.insert(key, value);
         }
 
+        if !self.attributes.is_empty() {
+            let mut attributes = Attributes::new();
+
+            for (key, unresolved) in self.attributes {
+                let value = unresolved.resolve_unambiguous()?;
+                attributes.insert(key, value);
+            }
+
+            properties.insert("Attributes".into(), attributes.into());
+        }
+
         Ok(InstanceSnapshot {
             snapshot_id: None,
             metadata: Default::default(),

src/snapshot_middleware/meta_file.rs

@@ -1,6 +1,7 @@
 use std::{borrow::Cow, collections::HashMap, path::PathBuf};
 
 use anyhow::{format_err, Context};
+use rbx_dom_weak::types::Attributes;
 use serde::{Deserialize, Serialize};
 
 use crate::{resolution::UnresolvedValue, snapshot::InstanceSnapshot};
@@ -78,6 +79,9 @@ pub struct DirectoryMetadata {
     #[serde(default, skip_serializing_if = "HashMap::is_empty")]
     pub properties: HashMap<String, UnresolvedValue>,
 
+    #[serde(default, skip_serializing_if = "HashMap::is_empty")]
+    pub attributes: HashMap<String, UnresolvedValue>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub class_name: Option<String>,
 
@@ -139,6 +143,19 @@ impl DirectoryMetadata {
             snapshot.properties.insert(key, value);
         }
 
+        if !self.attributes.is_empty() {
+            let mut attributes = Attributes::new();
+
+            for (key, unresolved) in self.attributes.drain() {
+                let value = unresolved.resolve_unambiguous()?;
+                attributes.insert(key, value);
+            }
+
+            snapshot
+                .properties
+                .insert("Attributes".into(), attributes.into());
+        }
+
         Ok(())
     }
 }

src/snapshot_middleware/project.rs

@@ -2,6 +2,7 @@ use std::{borrow::Cow, collections::HashMap, path::Path};
 
 use anyhow::{bail, Context};
 use memofs::Vfs;
+use rbx_dom_weak::types::Attributes;
 use rbx_reflection::ClassTag;
 
 use crate::{
@@ -231,6 +232,23 @@ pub fn snapshot_project_node(
         properties.insert(key.clone(), value);
     }
 
+    if !node.attributes.is_empty() {
+        let mut attributes = Attributes::new();
+
+        for (key, unresolved) in &node.attributes {
+            let value = unresolved.clone().resolve_unambiguous().with_context(|| {
+                format!(
+                    "Unresolvable attribute in project at path {}",
+                    project_path.display()
+                )
+            })?;
+
+            attributes.insert(key.clone(), value);
+        }
+
+        properties.insert("Attributes".into(), attributes.into());
+    }
+
     // If the user specified $ignoreUnknownInstances, overwrite the existing
     // value.
     //