feat: support name property in meta and model jsons

rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap

@@ -0,0 +1,14 @@
+---
+source: tests/rojo_test/syncback_util.rs
+assertion_line: 101
+expression: "String::from_utf8_lossy(&output.stdout)"
+---
+Writing default.project.json
+Writing src/Camera.rbxm
+Writing src/Terrain.rbxm
+Writing src/_Folder/init.meta.json
+Writing src/_Script/init.meta.json
+Writing src/_Script/init.server.luau
+Writing src
+Writing src/_Folder
+Writing src/_Script

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder.model.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Folder.model.json
+---
+{
+  "className": "Folder"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder__init.meta.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Folder/init.meta.json
+---
+{
+  "name": "/Folder"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.meta.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script.meta.json
+---
+{
+  "name": "/Script"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.server.luau.snap

@@ -0,0 +1,6 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script.server.luau
+---
+print("Hello world!")

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.meta.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script/init.meta.json
+---
+{
+  "name": "/Script"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.server.luau.snap

@@ -0,0 +1,6 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script/init.server.luau
+---
+print("Hello world!")

rojo-test/syncback-tests/slugified_name/input-project/default.project.json

@@ -0,0 +1,10 @@
+{
+  "name": "slugified_name",
+  "tree": {
+    "$className": "DataModel",
+    "Workspace": {
+      "$className": "Workspace",
+      "$path": "src"
+    }
+  }
+}

src/snapshot/metadata.rs

@@ -70,6 +70,12 @@ pub struct InstanceMetadata {
     /// A schema provided via a JSON file, if one exists. Will be `None` for
     /// all non-JSON middleware.
     pub schema: Option<String>,
+
+    /// A custom name specified via meta.json or model.json files. If present,
+    /// this name will be used for the instance while the filesystem name will
+    /// be slugified to remove illegal characters.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub specified_name: Option<String>,
 }
 
 impl InstanceMetadata {
@@ -82,6 +88,7 @@ impl InstanceMetadata {
             specified_id: None,
             middleware: None,
             schema: None,
+            specified_name: None,
         }
     }
 
@@ -130,6 +137,13 @@ impl InstanceMetadata {
     pub fn schema(self, schema: Option<String>) -> Self {
         Self { schema, ..self }
     }
+
+    pub fn specified_name(self, specified_name: Option<String>) -> Self {
+        Self {
+            specified_name,
+            ..self
+        }
+    }
 }
 
 impl Default for InstanceMetadata {

src/snapshot_middleware/json_model.rs

@@ -81,6 +81,7 @@ pub fn syncback_json_model<'sync>(
         // schemas will ever exist in one project for it to matter, but it
         // could have a performance cost.
         model.schema = old_inst.metadata().schema.clone();
+        model.name = old_inst.metadata().specified_name.clone();
     }
 
     Ok(SyncbackReturn {

src/snapshot_middleware/lua.rs

@@ -158,8 +158,14 @@ pub fn syncback_lua<'sync>(
 
         if !meta.is_empty() {
             let parent_location = snapshot.path.parent_err()?;
+            let instance_name = &snapshot.new_inst().name;
+            let meta_name = if crate::syncback::validate_file_name(instance_name).is_err() {
+                crate::syncback::slugify_name(instance_name)
+            } else {
+                instance_name.clone()
+            };
             fs_snapshot.add_file(
-                parent_location.join(format!("{}.meta.json", new_inst.name)),
+                parent_location.join(format!("{}.meta.json", meta_name)),
                 serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
             );
         }

src/snapshot_middleware/meta_file.rs

@@ -10,7 +10,10 @@ use rbx_dom_weak::{
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot,
+    json,
+    resolution::UnresolvedValue,
+    snapshot::InstanceSnapshot,
+    syncback::{validate_file_name, SyncbackSnapshot},
     RojoRef,
 };
 
@@ -36,6 +39,9 @@ pub struct AdjacentMetadata {
     #[serde(default, skip_serializing_if = "IndexMap::is_empty")]
     pub attributes: IndexMap<String, UnresolvedValue>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub name: Option<String>,
+
     #[serde(skip)]
     pub path: PathBuf,
 }
@@ -144,6 +150,24 @@ impl AdjacentMetadata {
             }
         }
 
+        let name = snapshot
+            .old_inst()
+            .and_then(|inst| inst.metadata().specified_name.clone())
+            .or_else(|| {
+                // If this is a new instance and its name is invalid for the filesystem,
+                // we need to specify the name in meta.json so it can be preserved
+                if snapshot.old_inst().is_none() {
+                    let instance_name = &snapshot.new_inst().name;
+                    if validate_file_name(instance_name).is_err() {
+                        Some(instance_name.clone())
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            });
+
         Ok(Some(Self {
             ignore_unknown_instances: if ignore_unknown_instances {
                 Some(true)
@@ -155,6 +179,7 @@ impl AdjacentMetadata {
             path,
             id: None,
             schema,
+            name,
         }))
     }
 
@@ -213,11 +238,23 @@ impl AdjacentMetadata {
         Ok(())
     }
 
+    fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
+        if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
+            anyhow::bail!(
+                "cannot specify a name using {} (instance has a name from somewhere else)",
+                self.path.display()
+            );
+        }
+        snapshot.metadata.specified_name = self.name.take();
+        Ok(())
+    }
+
     pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
         self.apply_ignore_unknown_instances(snapshot);
         self.apply_properties(snapshot)?;
         self.apply_id(snapshot)?;
         self.apply_schema(snapshot)?;
+        self.apply_name(snapshot)?;
         Ok(())
     }
 
@@ -226,11 +263,13 @@ impl AdjacentMetadata {
     ///
     /// - The number of properties and attributes is 0
     /// - `ignore_unknown_instances` is None
+    /// - `name` is None
     #[inline]
     pub fn is_empty(&self) -> bool {
         self.attributes.is_empty()
             && self.properties.is_empty()
             && self.ignore_unknown_instances.is_none()
+            && self.name.is_none()
     }
 
     // TODO: Add method to allow selectively applying parts of metadata and
@@ -262,6 +301,9 @@ pub struct DirectoryMetadata {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub class_name: Option<Ustr>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub name: Option<String>,
+
     #[serde(skip)]
     pub path: PathBuf,
 }
@@ -372,6 +414,24 @@ impl DirectoryMetadata {
             }
         }
 
+        let name = snapshot
+            .old_inst()
+            .and_then(|inst| inst.metadata().specified_name.clone())
+            .or_else(|| {
+                // If this is a new instance and its name is invalid for the filesystem,
+                // we need to specify the name in meta.json so it can be preserved
+                if snapshot.old_inst().is_none() {
+                    let instance_name = &snapshot.new_inst().name;
+                    if validate_file_name(instance_name).is_err() {
+                        Some(instance_name.clone())
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            });
+
         Ok(Some(Self {
             ignore_unknown_instances: if ignore_unknown_instances {
                 Some(true)
@@ -384,6 +444,7 @@ impl DirectoryMetadata {
             path,
             id: None,
             schema,
+            name,
         }))
     }
 
@@ -393,6 +454,7 @@ impl DirectoryMetadata {
         self.apply_properties(snapshot)?;
         self.apply_id(snapshot)?;
         self.apply_schema(snapshot)?;
+        self.apply_name(snapshot)?;
 
         Ok(())
     }
@@ -464,17 +526,30 @@ impl DirectoryMetadata {
         snapshot.metadata.schema = self.schema.take();
         Ok(())
     }
+
+    fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
+        if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
+            anyhow::bail!(
+                "cannot specify a name using {} (instance has a name from somewhere else)",
+                self.path.display()
+            );
+        }
+        snapshot.metadata.specified_name = self.name.take();
+        Ok(())
+    }
     /// Returns whether the metadata is 'empty', meaning it doesn't have anything
     /// worth persisting in it. Specifically:
     ///
     /// - The number of properties and attributes is 0
     /// - `ignore_unknown_instances` is None
     /// - `class_name` is either None or not Some("Folder")
+    /// - `name` is None
     #[inline]
     pub fn is_empty(&self) -> bool {
         self.attributes.is_empty()
             && self.properties.is_empty()
             && self.ignore_unknown_instances.is_none()
+            && self.name.is_none()
             && if let Some(class) = &self.class_name {
                 class == "Folder"
             } else {

src/syncback/file_names.rs

@@ -35,14 +35,23 @@ pub fn name_for_inst<'old>(
             | Middleware::CsvDir
             | Middleware::ServerScriptDir
             | Middleware::ClientScriptDir
-            | Middleware::ModuleScriptDir => Cow::Owned(new_inst.name.clone()),
+            | Middleware::ModuleScriptDir => {
+                let name = if validate_file_name(&new_inst.name).is_err() {
+                    Cow::Owned(slugify_name(&new_inst.name))
+                } else {
+                    Cow::Owned(new_inst.name.clone())
+                };
+                name
+            }
             _ => {
                 let extension = extension_for_middleware(middleware);
-                let name = &new_inst.name;
-                validate_file_name(name).with_context(|| {
-                    format!("name '{name}' is not legal to write to the file system")
-                })?;
-                Cow::Owned(format!("{name}.{extension}"))
+                let final_name = if validate_file_name(&new_inst.name).is_err() {
+                    slugify_name(&new_inst.name)
+                } else {
+                    new_inst.name.clone()
+                };
+
+                Cow::Owned(format!("{final_name}.{extension}"))
             }
         })
     }
@@ -94,6 +103,39 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [
 /// in a file's name.
 const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\'];
 
+/// Slugifies a name by replacing forbidden characters with underscores
+/// and ensuring the result is a valid file name
+pub fn slugify_name(name: &str) -> String {
+    let mut result = String::with_capacity(name.len());
+
+    for ch in name.chars() {
+        if FORBIDDEN_CHARS.contains(&ch) {
+            result.push('_');
+        } else {
+            result.push(ch);
+        }
+    }
+
+    // Handle Windows reserved names by appending an underscore
+    let result_lower = result.to_lowercase();
+    for forbidden in INVALID_WINDOWS_NAMES {
+        if result_lower == forbidden.to_lowercase() {
+            result.push('_');
+            break;
+        }
+    }
+
+    while result.ends_with(' ') || result.ends_with('.') {
+        result.pop();
+    }
+
+    if result.is_empty() || result.chars().all(|c| c == '_') {
+        result = "instance".to_string();
+    }
+
+    result
+}
+
 /// Validates a provided file name to ensure it's allowed on the file system. An
 /// error is returned if the name isn't allowed, indicating why.
 /// This takes into account rules for Windows, MacOS, and Linux.

src/syncback/mod.rs

@@ -28,7 +28,7 @@ use crate::{
     Project,
 };
 
-pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name};
+pub use file_names::{extension_for_middleware, name_for_inst, slugify_name, validate_file_name};
 pub use fs_snapshot::FsSnapshot;
 pub use hash::*;
 pub use property_filter::{filter_properties, filter_properties_preallocated};
@@ -360,6 +360,19 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware {
         }
     }
 
+    // If the instance name is invalid for the filesystem, use directory middleware
+    // to allow preserving the name in meta.json
+    if crate::syncback::file_names::validate_file_name(&inst.name).is_err() {
+        middleware = match middleware {
+            Middleware::ServerScript => Middleware::ServerScriptDir,
+            Middleware::ClientScript => Middleware::ClientScriptDir,
+            Middleware::ModuleScript => Middleware::ModuleScriptDir,
+            Middleware::Csv => Middleware::CsvDir,
+            Middleware::JsonModel | Middleware::Text => Middleware::Dir,
+            _ => middleware,
+        }
+    }
+
     if middleware == Middleware::Rbxm {
         middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
             Ok(value) if value == "1" => Middleware::Rbxmx,

tests/tests/syncback.rs

@@ -86,4 +86,7 @@ syncback_tests! {
     sync_rules => ["src/module.modulescript", "src/text.text"],
     // Ensures that the `syncUnscriptable` setting works
     unscriptable_properties => ["default.project.json"],
+    // Ensures that instances with names containing illegal characters get slugified filenames
+    // and preserve their original names in meta.json
+    slugified_name => ["src/_Script/init.meta.json", "src/_Script/init.server.luau", "src/_Folder/init.meta.json"],
 }