diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index c03a55c0..e23138e0 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/src/snapshot_middleware/meta_file.rs @@ -10,10 +10,7 @@ use rbx_dom_weak::{ use serde::{Deserialize, Serialize}; use crate::{ - json, - resolution::UnresolvedValue, - snapshot::InstanceSnapshot, - syncback::SyncbackSnapshot, + json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot, RojoRef, }; diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 9f04fc0e..3a1b1445 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -690,7 +690,7 @@ pub fn syncback_project<'sync>( if parent_middleware != Middleware::Project { let taken_names = taken_names_per_dir .entry(parent_path.clone()) - .or_insert_with(std::collections::HashSet::new); + .or_default(); let (child_snap, _needs_meta, dedup_key) = snapshot.with_base_path( &parent_path, new_child.referent(), diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index c9955126..53c1c623 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -74,7 +74,11 @@ pub fn name_for_inst<'a>( let deduped = deduplicate_name(&base, taken_names); let needs_meta = needs_slugify || deduped != base; let dedup_key = deduped.clone(); - Ok((Cow::Owned(format!("{deduped}.{extension}")), needs_meta, dedup_key)) + Ok(( + Cow::Owned(format!("{deduped}.{extension}")), + needs_meta, + dedup_key, + )) } } } @@ -703,7 +707,8 @@ mod tests { let child = dom.get_by_ref(child_ref).unwrap(); let taken = HashSet::new(); - let (filename, needs_meta, _dk) = name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + let (filename, needs_meta, _dk) = + name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); assert_eq!(filename.as_ref(), "MyFolder"); assert!(!needs_meta); } @@ -728,7 +733,8 @@ mod tests { let child = dom.get_by_ref(child_ref).unwrap(); let taken = HashSet::new(); - let (filename, needs_meta, _dk) = name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + let (filename, needs_meta, _dk) = + name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); assert_eq!(filename.as_ref(), "Hey_Bro"); assert!(needs_meta); } @@ -753,7 +759,8 @@ mod tests { let child = dom.get_by_ref(child_ref).unwrap(); let taken: HashSet = ["stuff".to_string()].into_iter().collect(); - let (filename, needs_meta, _dk) = name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + let (filename, needs_meta, _dk) = + name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); assert_eq!(filename.as_ref(), "Stuff~1"); assert!(needs_meta); } @@ -805,7 +812,8 @@ mod tests { let child = dom.get_by_ref(child_ref).unwrap(); let taken = HashSet::new(); - let (filename, needs_meta, _dk) = name_for_inst(Middleware::Text, child, None, &taken).unwrap(); + let (filename, needs_meta, _dk) = + name_for_inst(Middleware::Text, child, None, &taken).unwrap(); assert_eq!(filename.as_ref(), "Readme.txt"); assert!(!needs_meta); } @@ -1450,17 +1458,13 @@ mod tests { new_children: &[(&str, Middleware)], ) -> Vec<(String, String, bool)> { // Seed from bare slugs (simulates tree-based seeding) - let mut taken: HashSet = disk_entries - .iter() - .map(|e| e.to_lowercase()) - .collect(); + let mut taken: HashSet = disk_entries.iter().map(|e| e.to_lowercase()).collect(); let mut results = Vec::new(); for &(name, mw) in new_children { let dom = make_inst(name, "ModuleScript"); let child_ref = dom.root().children()[0]; let child = dom.get_by_ref(child_ref).unwrap(); - let (filename, needs_meta, dedup_key) = - name_for_inst(mw, child, None, &taken).unwrap(); + let (filename, needs_meta, dedup_key) = name_for_inst(mw, child, None, &taken).unwrap(); taken.insert(dedup_key.to_lowercase()); results.push((name.to_string(), filename.into_owned(), needs_meta)); } @@ -1471,10 +1475,7 @@ mod tests { fn disk_seed_prevents_overwrite_of_existing_dir() { // "Utils" directory exists on disk. New instance "Utils" (dir) // must get "Utils~1". - let r = process_new_children_with_disk_seed( - &["utils"], - &[("Utils", Middleware::Dir)], - ); + let r = process_new_children_with_disk_seed(&["utils"], &[("Utils", Middleware::Dir)]); assert_eq!(r[0].1, "Utils~1"); assert!(r[0].2); } @@ -1482,10 +1483,7 @@ mod tests { #[test] fn disk_seed_prevents_overwrite_of_existing_dir_case_insensitive() { // "SCRIPTS" directory on disk, new instance "Scripts" (dir) must dedup. - let r = process_new_children_with_disk_seed( - &["scripts"], - &[("Scripts", Middleware::Dir)], - ); + let r = process_new_children_with_disk_seed(&["scripts"], &[("Scripts", Middleware::Dir)]); assert_eq!(r[0].1, "Scripts~1"); assert!(r[0].2); } @@ -1494,10 +1492,7 @@ mod tests { fn disk_seed_slug_collision_with_existing_dir() { // "Hey_Bro" directory on disk. New instance "Hey/Bro" slugifies // to "Hey_Bro" — must dedup to "Hey_Bro~1". - let r = process_new_children_with_disk_seed( - &["hey_bro"], - &[("Hey/Bro", Middleware::Dir)], - ); + let r = process_new_children_with_disk_seed(&["hey_bro"], &[("Hey/Bro", Middleware::Dir)]); assert_eq!(r[0].1, "Hey_Bro~1"); assert!(r[0].2); } @@ -1553,7 +1548,10 @@ mod tests { // child correctly detects the collision and gets ~1. let r = process_new_children_with_disk_seed( &[], - &[("A/B", Middleware::ModuleScript), ("A:B", Middleware::ModuleScript)], + &[ + ("A/B", Middleware::ModuleScript), + ("A:B", Middleware::ModuleScript), + ], ); assert_eq!(r[0].1, "A_B.luau"); assert!(r[0].2); @@ -1596,10 +1594,7 @@ mod tests { fn disk_seed_dedup_suffix_dir_already_on_disk() { // Pathological: "Foo" AND "Foo~1" directories already on disk. // New "Foo" dir must skip both and land on "Foo~2". - let r = process_new_children_with_disk_seed( - &["foo", "foo~1"], - &[("Foo", Middleware::Dir)], - ); + let r = process_new_children_with_disk_seed(&["foo", "foo~1"], &[("Foo", Middleware::Dir)]); assert_eq!(r[0].1, "Foo~2"); assert!(r[0].2); } @@ -1632,17 +1627,13 @@ mod tests { disk_entries: &[&str], siblings: &[(&str, Middleware)], ) -> Vec<(String, String, bool)> { - let mut taken: HashSet = disk_entries - .iter() - .map(|e| e.to_lowercase()) - .collect(); + let mut taken: HashSet = disk_entries.iter().map(|e| e.to_lowercase()).collect(); let mut results = Vec::new(); for &(name, mw) in siblings { let dom = make_inst(name, "ModuleScript"); let child_ref = dom.root().children()[0]; let child = dom.get_by_ref(child_ref).unwrap(); - let (filename, needs_meta, dedup_key) = - name_for_inst(mw, child, None, &taken).unwrap(); + let (filename, needs_meta, dedup_key) = name_for_inst(mw, child, None, &taken).unwrap(); taken.insert(dedup_key.to_lowercase()); results.push((name.to_string(), filename.into_owned(), needs_meta)); } @@ -1654,10 +1645,8 @@ mod tests { // Two new project children: "A/B" and "A:B" both slug to "A_B". // Before fix: both got "A_B" path (collision!). // After fix: second gets "A_B~1". - let r = process_project_siblings( - &[], - &[("A/B", Middleware::Dir), ("A:B", Middleware::Dir)], - ); + let r = + process_project_siblings(&[], &[("A/B", Middleware::Dir), ("A:B", Middleware::Dir)]); assert_eq!(r[0].1, "A_B"); assert!(r[0].2); assert_eq!(r[1].1, "A_B~1"); @@ -1670,10 +1659,7 @@ mod tests { // Dir middleware: bare slug IS the filename → dedup works. let r = process_project_siblings( &[], - &[ - ("Hey_Bro", Middleware::Dir), - ("Hey/Bro", Middleware::Dir), - ], + &[("Hey_Bro", Middleware::Dir), ("Hey/Bro", Middleware::Dir)], ); assert_eq!(r[0].1, "Hey_Bro"); assert!(!r[0].2); @@ -1704,10 +1690,7 @@ mod tests { // Second → "Helper~2" (both disk and first sibling taken). let r = process_project_siblings( &["helper"], - &[ - ("Helper", Middleware::Dir), - ("Helper", Middleware::Dir), - ], + &[("Helper", Middleware::Dir), ("Helper", Middleware::Dir)], ); assert_eq!(r[0].1, "Helper~1"); assert!(r[0].2); @@ -1850,9 +1833,8 @@ mod tests { // Every Windows reserved name as a sibling — all get "_" suffix, // and they should NOT collide with each other since the base names differ. let reserved = [ - "CON", "PRN", "AUX", "NUL", - "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", - "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", + "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", + "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", ]; let r = process_siblings_dir(&reserved); let names: HashSet = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); @@ -1877,9 +1859,7 @@ mod tests { .iter() .map(|s| format!("foo{s}")) .collect(); - let r = process_siblings_dir( - &siblings.iter().map(|s| s.as_str()).collect::>(), - ); + let r = process_siblings_dir(&siblings.iter().map(|s| s.as_str()).collect::>()); let filenames: HashSet = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); assert_eq!( filenames.len(), @@ -2021,9 +2001,9 @@ mod tests { let r = process_siblings_dir(&names); assert_eq!(r[0].1, "Script"); assert!(!r[0].2); - for i in 1..100 { - assert_eq!(r[i].1, format!("Script~{i}")); - assert!(r[i].2); + for (i, entry) in r.iter().enumerate().skip(1) { + assert_eq!(entry.1, format!("Script~{i}")); + assert!(entry.2); } // All filenames must be unique let unique: HashSet = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); @@ -2035,12 +2015,12 @@ mod tests { // Interleaved clean and forbidden-char names that produce same slug. // Tests that accumulation order doesn't matter. let r = process_siblings_dir(&[ - "A_B", // clean, claims "A_B" - "C_D", // clean, claims "C_D" - "A/B", // slug "A_B" → collision → "A_B~1" - "E_F", // clean, claims "E_F" - "C:D", // slug "C_D" → collision → "C_D~1" - "A:B", // slug "A_B" → collision with A_B and A_B~1 → "A_B~2" + "A_B", // clean, claims "A_B" + "C_D", // clean, claims "C_D" + "A/B", // slug "A_B" → collision → "A_B~1" + "E_F", // clean, claims "E_F" + "C:D", // slug "C_D" → collision → "C_D~1" + "A:B", // slug "A_B" → collision with A_B and A_B~1 → "A_B~2" ]); assert_eq!(r[0].1, "A_B"); assert!(!r[0].2); @@ -2067,14 +2047,32 @@ mod tests { // Process a massive batch of problematic names and verify // every resulting filename is unique (case-insensitive). let names = [ - "Foo", "foo", "FOO", "Foo", "Foo", - "A/B", "A:B", "A*B", "A_B", - "CON", "CON_", "con", "Con", - "test.server", "test_server", "test/server", - "", "", "", - " X", "X", "X ", - "Hello\x00", "Hello_", - "日本語", "日本語", + "Foo", + "foo", + "FOO", + "Foo", + "Foo", + "A/B", + "A:B", + "A*B", + "A_B", + "CON", + "CON_", + "con", + "Con", + "test.server", + "test_server", + "test/server", + "", + "", + "", + " X", + "X", + "X ", + "Hello\x00", + "Hello_", + "日本語", + "日本語", ]; let r = process_siblings_dir(&names); let lowered: Vec = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); @@ -2091,13 +2089,32 @@ mod tests { fn invariant_every_filename_passes_validation() { // Every filename produced by the pipeline must pass validate_file_name. let names = [ - "CON", "PRN", "NUL", "COM1", "LPT9", - "foo.server", "bar.meta", "baz.model", - "A/B", "C:D", "E*F", "G?H", - "", "a|b", "x\\y", "q\"r", - "", " ", ".", "..", - "hello.", "hello ", " hello", - "~1", "~~", "a~b~c", + "CON", + "PRN", + "NUL", + "COM1", + "LPT9", + "foo.server", + "bar.meta", + "baz.model", + "A/B", + "C:D", + "E*F", + "G?H", + "", + "a|b", + "x\\y", + "q\"r", + "", + " ", + ".", + "..", + "hello.", + "hello ", + " hello", + "~1", + "~~", + "a~b~c", ]; let r = process_siblings_dir(&names); for (original, filename, _) in &r { @@ -2113,9 +2130,17 @@ mod tests { // For new instances: needs_meta is true IFF the filesystem name // differs from the instance name. This is the contract. let cases = [ - "Clean", "Hello World", "test-123", - "A/B", "CON", "foo.server", " X", "X ", "", - "Foo~1", "hello.", + "Clean", + "Hello World", + "test-123", + "A/B", + "CON", + "foo.server", + " X", + "X ", + "", + "Foo~1", + "hello.", ]; for &name in &cases { let dom = make_inst(name, "Folder"); @@ -2126,12 +2151,14 @@ mod tests { name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); if needs_meta { assert_ne!( - filename.as_ref(), name, + filename.as_ref(), + name, "needs_meta=true but filename matches name for {name:?}" ); } else { assert_eq!( - filename.as_ref(), name, + filename.as_ref(), + name, "needs_meta=false but filename differs from name for {name:?}" ); } diff --git a/src/web/api.rs b/src/web/api.rs index 995df0d9..b92c4a6e 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -1078,7 +1078,12 @@ impl ApiService { // This will use detect_existing_script_format to check existing files let child_slugs = Self::compute_sibling_slugs(&unique_children); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &children_dir, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &children_dir, + stats, + &child_slugs, + )?; } } } @@ -1104,7 +1109,12 @@ impl ApiService { let child_slugs = Self::compute_sibling_slugs(&unique_children); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, existing_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + existing_path, + stats, + &child_slugs, + )?; } } } else { @@ -1735,7 +1745,12 @@ impl ApiService { self.write_script_meta_json_if_needed(&dir_path, added, meta_name_field)?; log::info!("Syncback: Updated ModuleScript at {}", init_path.display()); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } else { let file_path = parent_dir.join(format!("{}.luau", encoded_name)); @@ -1792,7 +1807,12 @@ impl ApiService { self.write_script_meta_json_if_needed(&dir_path, added, meta_name_field)?; log::info!("Syncback: Updated Script at {}", init_path.display()); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } else { let file_path = @@ -1849,7 +1869,12 @@ impl ApiService { self.write_script_meta_json_if_needed(&dir_path, added, meta_name_field)?; log::info!("Syncback: Updated LocalScript at {}", init_path.display()); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } else { let file_path = parent_dir.join(format!("{}.local.luau", encoded_name)); @@ -1900,7 +1925,12 @@ impl ApiService { // Recursively process children for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } @@ -1919,7 +1949,12 @@ impl ApiService { dir_path.display() ); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } else { let value = added @@ -1961,7 +1996,12 @@ impl ApiService { dir_path.display() ); for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } else { let file_path = parent_dir.join(format!("{}.csv", encoded_name)); @@ -2015,7 +2055,12 @@ impl ApiService { // Recursively process children for child in &unique_children { - self.syncback_instance_to_path_with_stats(child, &dir_path, stats, &child_slugs)?; + self.syncback_instance_to_path_with_stats( + child, + &dir_path, + stats, + &child_slugs, + )?; } } else { let content = self.serialize_instance_to_model_json(added)?; @@ -6189,10 +6234,7 @@ mod tests { #[test] fn batch_dir_with_existing_and_collisions() { - let r = simulate_dir_batch( - &["Utils", "Config"], - &["Utils", "Config", "NewThing"], - ); + let r = simulate_dir_batch(&["Utils", "Config"], &["Utils", "Config", "NewThing"]); assert_eq!(r[0].0, "Utils~1"); assert_eq!(r[1].0, "Config~1"); assert_eq!(r[2].0, "NewThing"); @@ -6201,10 +6243,7 @@ mod tests { #[test] fn batch_dir_dangerous_then_natural_then_slug() { - let r = simulate_dir_batch( - &[], - &["foo.server", "foo_server", "foo/server"], - ); + let r = simulate_dir_batch(&[], &["foo.server", "foo_server", "foo/server"]); assert_eq!(r[0].0, "foo_server"); assert_eq!(r[1].0, "foo_server~1"); assert_eq!(r[2].0, "foo_server~2"); @@ -6234,8 +6273,8 @@ mod tests { let children: Vec<&str> = vec!["Spam"; 20]; let r = simulate_dir_batch(&[], &children); assert_eq!(r[0].0, "Spam"); - for i in 1..20 { - assert_eq!(r[i].0, format!("Spam~{i}")); + for (i, entry) in r.iter().enumerate().skip(1) { + assert_eq!(entry.0, format!("Spam~{i}")); } let unique: HashSet = r.iter().map(|(f, _)| f.to_lowercase()).collect(); assert_eq!(unique.len(), 20); @@ -6339,8 +6378,7 @@ mod tests { let r = simulate_dir_batch( &["A_B"], &[ - "A/B", "A:B", "A*B", "A?B", "AB", "A|B", - "A\\B", "A\"B", + "A/B", "A:B", "A*B", "A?B", "AB", "A|B", "A\\B", "A\"B", ], ); assert_eq!(r[0].0, "A_B~1"); @@ -6356,10 +6394,7 @@ mod tests { #[test] fn batch_dir_con_prn_nul_then_slugged() { - let r = simulate_dir_batch( - &[], - &["CON", "PRN", "CON/", "PRN/"], - ); + let r = simulate_dir_batch(&[], &["CON", "PRN", "CON/", "PRN/"]); assert_eq!(r[0].0, "CON_"); assert_eq!(r[1].0, "PRN_"); assert_eq!(r[2].0, "CON_~1"); // "CON/" slug "CON_" collides @@ -6368,10 +6403,7 @@ mod tests { #[test] fn batch_dir_unicode_plus_forbidden() { - let r = simulate_dir_batch( - &[], - &["カフェ/Bar", "カフェ:Bar", "カフェ_Bar"], - ); + let r = simulate_dir_batch(&[], &["カフェ/Bar", "カフェ:Bar", "カフェ_Bar"]); assert_eq!(r[0].0, "カフェ_Bar"); assert_eq!(r[1].0, "カフェ_Bar~1"); assert_eq!(r[2].0, "カフェ_Bar~2"); @@ -6381,13 +6413,24 @@ mod tests { fn batch_dir_all_unique_filenames_invariant() { // A massive batch — every result must be unique. let children = &[ - "Foo", "foo", "FOO", "A/B", "A:B", "A_B", - "CON", "CON_", "", "", "test.server", "test_server", + "Foo", + "foo", + "FOO", + "A/B", + "A:B", + "A_B", + "CON", + "CON_", + "", + "", + "test.server", + "test_server", ]; let r = simulate_dir_batch(&[], children); let unique: HashSet = r.iter().map(|(f, _)| f.to_lowercase()).collect(); assert_eq!( - unique.len(), r.len(), + unique.len(), + r.len(), "every sibling must get a unique filename" ); }