diff --git a/.cursor/plans/nuke_path_encoding_migration_7dfffc4d.plan.md b/.cursor/plans/nuke_path_encoding_migration_7dfffc4d.plan.md index d5cab482..054f991f 100644 --- a/.cursor/plans/nuke_path_encoding_migration_7dfffc4d.plan.md +++ b/.cursor/plans/nuke_path_encoding_migration_7dfffc4d.plan.md @@ -4,25 +4,25 @@ overview: "Remove the `%NAME%` path encoding system entirely and replace it with todos: - id: phase1-infrastructure content: "Manual port of PR #1187 infrastructure: specified_name on InstanceMetadata, slugify_name(), deduplicate_name(), name field on meta file structs, apply_name() methods, json_model name field support" - status: pending + status: completed - id: phase2-replace-encode-decode content: Replace all encode_path_name/decode_path_name callsites with slugify + dedup + name property approach in snapshot middleware, syncback/file_names.rs, syncback/snapshot.rs - status: pending + status: completed - id: phase3-two-way-sync content: "Replace encoding in two-way sync: change_processor.rs renames and web/api.rs added instances, with dedup checks" - status: pending + status: completed - id: phase4-remove-config content: Remove decode_windows_invalid_chars from InstanceContext, encode_windows_invalid_chars from SyncbackRules, serve_session propagation, JSON schema - status: pending + status: completed - id: phase5-delete-module content: Delete src/path_encoding.rs and remove pub mod declaration from lib.rs - status: pending + status: completed - id: phase6-hardcore-tests content: "Hardcore test suite: unit tests for slugify/dedup, integration tests for build/syncback/two-way-sync roundtrips, collision tests, edge case tests; update existing encoded pattern tests; regenerate snapshots" - status: pending + status: completed - id: phase7-update-docs content: Update cursor rules (.cursor/rules/rojo.mdc, src.mdc) to reflect new approach - status: pending + status: completed isProject: false --- diff --git a/.cursor/rules/rojo-syncback.mdc b/.cursor/rules/rojo-syncback.mdc index 5eb54eeb..d61125a5 100644 --- a/.cursor/rules/rojo-syncback.mdc +++ b/.cursor/rules/rojo-syncback.mdc @@ -210,10 +210,12 @@ snapshot.with_new_path(path, new_ref, old_ref) ``` **Filename generation** (`file_names.rs`): -- Uses instance name +- Slugifies instance name (`slugify_name()`: forbidden chars → `_`) +- Deduplicates against taken names (`deduplicate_name()`: appends `~1`, `~2`, etc.) - Adds appropriate extension based on middleware -- Encodes Windows-invalid chars if enabled +- Stores real name in `.meta.json5` `name` field when slug differs - Example: `Script` named "My Script" → `My Script.server.luau` +- Example: `Script` named "Hey/Bro" → `Hey_Bro.server.luau` + meta with `name: "Hey/Bro"` ### Property Filtering @@ -378,7 +380,7 @@ Filesystem: - Solution: Use clean mode 2. **Invalid filename** - Instance name contains invalid chars - - Solution: Enable `decodeWindowsInvalidChars` or rename in Studio + - Solution: Handled automatically by `slugify_name()` + metadata `name` field 3. **Ref to pruned instance** - Property points to instance not in project - Logged as warning, property not synced diff --git a/.cursor/rules/rojo.mdc b/.cursor/rules/rojo.mdc index 8e5b5231..5edf5653 100644 --- a/.cursor/rules/rojo.mdc +++ b/.cursor/rules/rojo.mdc @@ -185,10 +185,11 @@ fn do_something() -> Result<()> { pub struct InstanceContext { pub path_ignore_rules: Arc>, pub sync_rules: Vec, - pub decode_windows_invalid_chars: bool, } ``` +**Filename handling:** Instance names with forbidden filesystem chars are slugified (forbidden chars → `_`), deduplicated (`~1`, `~2`, etc. on collision), and the real name is stored in `.meta.json5`/`.model.json5` `name` fields. See `src/syncback/file_names.rs`. + **VFS passed separately to snapshot functions:** ```rust snapshot_from_vfs(&context, &vfs, &path)? diff --git a/.cursor/rules/src.mdc b/.cursor/rules/src.mdc index 4c7a03d3..074e98a0 100644 --- a/.cursor/rules/src.mdc +++ b/.cursor/rules/src.mdc @@ -68,10 +68,16 @@ Transforms files → `InstanceSnapshot`. Main entry: `snapshot_from_vfs()` pub struct InstanceContext { pub path_ignore_rules: Arc>, pub sync_rules: Vec, - pub decode_windows_invalid_chars: bool, // Default: true } ``` +**Filename handling (syncback/two-way sync):** + +Instance names with forbidden filesystem chars (`<>:"/\|?*`) are handled by: +1. `slugify_name()` -- replaces forbidden chars with `_` (pure, stateless) +2. `deduplicate_name()` -- appends `~1`, `~2`, etc. on collision (stateful) +3. Metadata `name` field -- stores the real instance name in `.meta.json5`/`.model.json5` when the slug differs + ### 3. Commands (`src/cli/`) - `serve.rs` - Live sync with WebSocket diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap index ba8299cf..f395886a 100644 --- a/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap @@ -1,5 +1,6 @@ --- source: tests/tests/build.rs +assertion_line: 106 expression: contents --- @@ -9,7 +10,7 @@ expression: contents - Expected Name + Overridden Name diff --git a/rojo-test/serve-tests/syncback_encoded_names/src/Key_Script.meta.json5 b/rojo-test/serve-tests/syncback_encoded_names/src/Key_Script.meta.json5 new file mode 100644 index 00000000..ae6b0920 --- /dev/null +++ b/rojo-test/serve-tests/syncback_encoded_names/src/Key_Script.meta.json5 @@ -0,0 +1,3 @@ +{ + "name": "Key:Script" +} diff --git a/rojo-test/serve-tests/syncback_encoded_names/src/Key%COLON%Script.server.luau b/rojo-test/serve-tests/syncback_encoded_names/src/Key_Script.server.luau similarity index 100% rename from rojo-test/serve-tests/syncback_encoded_names/src/Key%COLON%Script.server.luau rename to rojo-test/serve-tests/syncback_encoded_names/src/Key_Script.server.luau diff --git a/rojo-test/serve-tests/syncback_encoded_names/src/What%QUESTION%Model.model.json5 b/rojo-test/serve-tests/syncback_encoded_names/src/What%QUESTION%Model.model.json5 deleted file mode 100644 index 85e4cf30..00000000 --- a/rojo-test/serve-tests/syncback_encoded_names/src/What%QUESTION%Model.model.json5 +++ /dev/null @@ -1,6 +0,0 @@ -{ - "className": "Configuration", - "properties": { - "Name": "What?Model" - } -} diff --git a/rojo-test/serve-tests/syncback_encoded_names/src/What_Model.model.json5 b/rojo-test/serve-tests/syncback_encoded_names/src/What_Model.model.json5 new file mode 100644 index 00000000..fadde337 --- /dev/null +++ b/rojo-test/serve-tests/syncback_encoded_names/src/What_Model.model.json5 @@ -0,0 +1,4 @@ +{ + "name": "What?Model", + "className": "Configuration" +} diff --git a/rojo-test/serve-tests/syncback_encoded_names/src/What%QUESTION%Module.luau b/rojo-test/serve-tests/syncback_encoded_names/src/What_Module.luau similarity index 100% rename from rojo-test/serve-tests/syncback_encoded_names/src/What%QUESTION%Module.luau rename to rojo-test/serve-tests/syncback_encoded_names/src/What_Module.luau diff --git a/rojo-test/serve-tests/syncback_encoded_names/src/What_Module.meta.json5 b/rojo-test/serve-tests/syncback_encoded_names/src/What_Module.meta.json5 new file mode 100644 index 00000000..0ee2e845 --- /dev/null +++ b/rojo-test/serve-tests/syncback_encoded_names/src/What_Module.meta.json5 @@ -0,0 +1,3 @@ +{ + "name": "What?Module" +} diff --git a/src/change_processor.rs b/src/change_processor.rs index 0daae4e7..499821cc 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -11,11 +11,11 @@ use std::{ use crate::{ message_queue::MessageQueue, - path_encoding::encode_path_name, snapshot::{ apply_patch_set, compute_patch_set, AppliedPatchSet, InstigatingSource, PatchSet, RojoTree, }, snapshot_middleware::{snapshot_from_vfs, snapshot_project_node}, + syncback::{name_needs_slugify, slugify_name}, }; /// Processes file change events, updates the DOM, and sends those updates @@ -226,6 +226,42 @@ impl JobThreadContext { } } + /// Upsert the `name` field in a `.meta.json5` file. If the file exists, + /// parse it and add/update the `name` key. If it doesn't exist, create it + /// with just the `name` key. + fn upsert_meta_name_field(&self, meta_path: &Path, real_name: &str) { + self.suppress_path(meta_path); + let mut obj = if meta_path.exists() { + match fs::read(meta_path) { + Ok(bytes) => match crate::json::from_slice::(&bytes) { + Ok(serde_json::Value::Object(map)) => map, + _ => serde_json::Map::new(), + }, + Err(_) => serde_json::Map::new(), + } + } else { + serde_json::Map::new() + }; + obj.insert( + "name".to_string(), + serde_json::Value::String(real_name.to_string()), + ); + match crate::json::to_vec_pretty_sorted(&serde_json::Value::Object(obj)) { + Ok(content) => { + if let Err(err) = fs::write(meta_path, &content) { + log::error!( + "Failed to write name to meta file {}: {}", + meta_path.display(), + err + ); + } + } + Err(err) => { + log::error!("Failed to serialize meta for name field: {}", err); + } + } + } + /// Computes and applies patches to the DOM for a given file path. /// /// This function finds the nearest ancestor to the given path that has associated instances @@ -561,47 +597,27 @@ impl JobThreadContext { let applied_patch = { let mut tree = self.tree.lock().unwrap(); + // NOTE: We do NOT delete files from disk here. The API handler + // (handle_api_write → syncback_removed_instance) already deleted + // the files before sending this PatchSet. Re-deleting would + // destroy any new file created at the same path — e.g., when an + // instance is renamed from "joe_test" to "joe/test", the new slug + // is also "joe_test", so the API creates a new file at the same + // path that the removal just deleted. If we deleted again here, + // the new file would be destroyed. + // + // The PatchSet removal only needs to update the DOM tree (handled + // by apply_patch_set below). for &id in &patch_set.removed_instances { if let Some(instance) = tree.get_instance(id) { if let Some(instigating_source) = &instance.metadata().instigating_source { match instigating_source { InstigatingSource::Path(path) => { - // Guard: file may already be deleted by the API's - // syncback_removed_instance before this PatchSet arrives. - if path.exists() { - if path.is_dir() { - log::info!( - "Two-way sync: Removing directory {}", - path.display() - ); - if let Err(err) = fs::remove_dir_all(path) { - log::error!( - "Failed to remove directory {:?} for instance {:?}: {}", - path, - id, - err - ); - } - } else { - log::info!( - "Two-way sync: Removing file {}", - path.display() - ); - if let Err(err) = fs::remove_file(path) { - log::error!( - "Failed to remove file {:?} for instance {:?}: {}", - path, - id, - err - ); - } - } - } else { - log::info!( - "Two-way sync: File already removed: {}", - path.display() - ); - } + log::info!( + "Two-way sync: Removing instance {:?} from tree (path: {})", + id, + path.display() + ); } InstigatingSource::ProjectNode { .. } => { log::warn!( @@ -653,17 +669,19 @@ impl JobThreadContext { if is_init_file { let dir_path = path.parent().unwrap(); if let Some(grandparent) = dir_path.parent() { - // Use the encoded dir name from the filesystem - // for the old meta lookup, and encode the new - // name for the new path. Using instance.name() - // (decoded) would fail for encoded names. let dir_name = dir_path .file_name() .and_then(|f| f.to_str()) .unwrap_or(""); - let encoded_new_name = encode_path_name(new_name); + // Slugify the new name for filesystem safety + let slugified_new_name = + if name_needs_slugify(new_name) { + slugify_name(new_name) + } else { + new_name.to_string() + }; let new_dir_path = - grandparent.join(&encoded_new_name); + grandparent.join(&slugified_new_name); if new_dir_path != dir_path { log::info!( @@ -696,7 +714,7 @@ impl JobThreadContext { let new_meta = grandparent.join(format!( "{}.meta.json5", - encoded_new_name + slugified_new_name )); self.suppress_path_any(&old_meta); self.suppress_path(&new_meta); @@ -707,14 +725,22 @@ impl JobThreadContext { self.unsuppress_path(&new_meta); } } + // Write name to init.meta.json5 if slug differs + if slugified_new_name != *new_name { + let init_meta = new_dir_path + .join("init.meta.json5"); + self.upsert_meta_name_field( + &init_meta, new_name, + ); + } } } } } else if let Some(parent) = path.parent() { // Derive the suffix from the existing filename - // rather than using replacen with the decoded - // instance name, which fails for encoded names - // (e.g., What%QUESTION%.server.luau). + // rather than using replacen with the instance + // name, which may differ from the filesystem + // name (e.g., slugified names). let extension = path .extension() .and_then(|e| e.to_str()) @@ -738,13 +764,18 @@ impl JobThreadContext { &stem[..stem.len() - script_suffix.len()] }; - let encoded_new_name = encode_path_name(new_name); + let slugified_new_name = if name_needs_slugify(new_name) + { + slugify_name(new_name) + } else { + new_name.to_string() + }; let new_file_name = if extension.is_empty() { - format!("{}{}", encoded_new_name, script_suffix) + format!("{}{}", slugified_new_name, script_suffix) } else { format!( "{}{}.{}", - encoded_new_name, script_suffix, extension + slugified_new_name, script_suffix, extension ) }; let new_path = parent.join(&new_file_name); @@ -770,11 +801,11 @@ impl JobThreadContext { overridden_source_path = Some(new_path.clone()); let old_meta = parent .join(format!("{}.meta.json5", old_base)); + let new_meta = parent.join(format!( + "{}.meta.json5", + slugified_new_name + )); if old_meta.exists() { - let new_meta = parent.join(format!( - "{}.meta.json5", - encoded_new_name - )); self.suppress_path_any(&old_meta); self.suppress_path(&new_meta); if fs::rename(&old_meta, &new_meta).is_err() @@ -783,6 +814,12 @@ impl JobThreadContext { self.unsuppress_path(&new_meta); } } + // Write name to adjacent meta if slug differs + if slugified_new_name != *new_name { + self.upsert_meta_name_field( + &new_meta, new_name, + ); + } } } } @@ -861,10 +898,9 @@ impl JobThreadContext { format!("init{}.luau", new_suffix) } } else { - // Derive the encoded base name from the - // filesystem path, not instance.name() - // (which is decoded). This preserves - // encoding like What%QUESTION% on disk. + // Derive the base name from the filesystem + // path, not instance.name() (which may + // differ from the slugified filename). let stem = actual_file .file_stem() .and_then(|s| s.to_str()) diff --git a/src/lib.rs b/src/lib.rs index 889de19e..549fa1d7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,7 +13,6 @@ mod json; mod lua_ast; mod message_queue; mod multimap; -pub mod path_encoding; mod path_serializer; mod project; mod resolution; diff --git a/src/path_encoding.rs b/src/path_encoding.rs deleted file mode 100644 index 291da1fa..00000000 --- a/src/path_encoding.rs +++ /dev/null @@ -1,342 +0,0 @@ -//! Encoding and decoding utilities for special filename characters. -//! -//! This module encodes characters that are problematic in file names: -//! - Windows-invalid characters: < > : " / \ | ? * -//! - Periods (.) to prevent conflicts with file extension parsing -//! - Leading/trailing spaces -//! - Literal percent signs (%) to allow round-tripping -//! -//! Characters are encoded using a %NAME% format (e.g., `<` becomes `%LT%`, -//! `.` becomes `%DOT%`). Literal `%` is escaped as `%%` (like printf). - -use std::collections::HashMap; -use std::sync::OnceLock; - -/// Mapping of special characters to their encoded representations. -const CHAR_ENCODINGS: &[(&str, &str)] = &[ - (".", "%DOT%"), - ("<", "%LT%"), - (">", "%GT%"), - (":", "%COLON%"), - ("\"", "%QUOTE%"), - ("/", "%SLASH%"), - ("\\", "%BACKSLASH%"), - ("|", "%PIPE%"), - ("?", "%QUESTION%"), - ("*", "%STAR%"), -]; - -/// Returns a map from encoded pattern (e.g., "DOT") to decoded char (e.g., "."). -fn get_decode_map() -> &'static HashMap<&'static str, &'static str> { - static DECODE_MAP: OnceLock> = OnceLock::new(); - DECODE_MAP.get_or_init(|| { - let mut map = HashMap::new(); - for (char, encoded) in CHAR_ENCODINGS { - // Strip the leading and trailing % from the pattern - let pattern = &encoded[1..encoded.len() - 1]; - map.insert(pattern, *char); - } - map - }) -} - -const SPACE_ENCODING: &str = "%SPACE%"; - -/// Encodes special characters in a file name. -/// -/// This replaces characters like `.`, `<`, `>`, `:`, `"`, `/`, `\`, `|`, `?`, `*` -/// with their encoded representations like `%DOT%`, `%LT%`, `%GT%`, etc. -/// Literal `%` is escaped as `%%`. Leading and trailing spaces are encoded as `%SPACE%`. -pub fn encode_path_name(name: &str) -> String { - let trimmed = name.trim_matches(' '); - - // Handle the case where the string is entirely spaces (or empty) - if trimmed.is_empty() { - return SPACE_ENCODING.repeat(name.len()); - } - - // Count leading and trailing spaces - let leading_spaces = name.len() - name.trim_start_matches(' ').len(); - let trailing_spaces = name.len() - name.trim_end_matches(' ').len(); - - // Get the middle part (without leading/trailing spaces) - let middle = &name[leading_spaces..name.len() - trailing_spaces]; - - // First: escape % as %% (before adding new % signs from encodings) - let mut encoded_middle = middle.replace('%', "%%"); - - // Then: encode special characters - for (char, encoded) in CHAR_ENCODINGS { - encoded_middle = encoded_middle.replace(char, encoded); - } - - // Build final result with encoded spaces - let encoded_prefix = SPACE_ENCODING.repeat(leading_spaces); - let encoded_suffix = SPACE_ENCODING.repeat(trailing_spaces); - - format!("{}{}{}", encoded_prefix, encoded_middle, encoded_suffix) -} - -/// Decodes encoded special characters back to their original form. -/// -/// This replaces encoded representations like `%DOT%`, `%LT%`, `%GT%`, etc. -/// back to their original characters `.`, `<`, `>`, etc. -/// `%%` is decoded to literal `%`. `%SPACE%` at the start/end is decoded back to spaces. -/// -/// Uses a proper left-to-right parser to correctly handle `%%` escaping. -pub fn decode_path_name(name: &str) -> String { - let mut result = name.to_string(); - - // Count and strip leading %SPACE% - let mut leading_spaces = 0; - while result.starts_with(SPACE_ENCODING) { - leading_spaces += 1; - result = result[SPACE_ENCODING.len()..].to_string(); - } - - // Count and strip trailing %SPACE% - let mut trailing_spaces = 0; - while result.ends_with(SPACE_ENCODING) { - trailing_spaces += 1; - result = result[..result.len() - SPACE_ENCODING.len()].to_string(); - } - - // Parse and decode the middle part - let decoded_middle = decode_patterns(&result); - - // Rebuild with actual spaces - let prefix = " ".repeat(leading_spaces); - let suffix = " ".repeat(trailing_spaces); - format!("{}{}{}", prefix, decoded_middle, suffix) -} - -/// Parses an encoded string left-to-right, properly handling `%%` escapes and `%NAME%` patterns. -fn decode_patterns(input: &str) -> String { - let decode_map = get_decode_map(); - let chars: Vec = input.chars().collect(); - let mut output = String::with_capacity(input.len()); - let mut i = 0; - - while i < chars.len() { - if chars[i] == '%' { - // Check for %% (escaped percent) - if i + 1 < chars.len() && chars[i + 1] == '%' { - output.push('%'); - i += 2; - continue; - } - - // Try to match a %NAME% pattern - if let Some((decoded, consumed)) = try_decode_pattern(&chars, i, decode_map) { - output.push_str(&decoded); - i += consumed; - continue; - } - } - - // Regular character, just copy it - output.push(chars[i]); - i += 1; - } - - output -} - -/// Tries to decode a %NAME% pattern starting at position `start`. -/// Returns Some((decoded_string, chars_consumed)) if successful, None otherwise. -fn try_decode_pattern( - chars: &[char], - start: usize, - decode_map: &HashMap<&str, &str>, -) -> Option<(String, usize)> { - // Must start with % - if chars.get(start) != Some(&'%') { - return None; - } - - // Find the closing % - let mut end = start + 1; - while end < chars.len() && chars[end] != '%' { - // Pattern names are uppercase letters only - if !chars[end].is_ascii_uppercase() { - return None; - } - end += 1; - } - - // Must have found a closing % - if end >= chars.len() || chars[end] != '%' { - return None; - } - - // Extract the pattern name (without the % delimiters) - let pattern_name: String = chars[start + 1..end].iter().collect(); - - // Look up the pattern - if let Some(&decoded) = decode_map.get(pattern_name.as_str()) { - Some((decoded.to_string(), end - start + 1)) - } else { - None - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_encode_all_chars() { - let input = r#".<>:"/\|?*"#; - let expected = "%DOT%%LT%%GT%%COLON%%QUOTE%%SLASH%%BACKSLASH%%PIPE%%QUESTION%%STAR%"; - assert_eq!(encode_path_name(input), expected); - } - - #[test] - fn test_decode_all_chars() { - let input = "%DOT%%LT%%GT%%COLON%%QUOTE%%SLASH%%BACKSLASH%%PIPE%%QUESTION%%STAR%"; - let expected = r#".<>:"/\|?*"#; - assert_eq!(decode_path_name(input), expected); - } - - #[test] - fn test_roundtrip() { - let original = "TestWith:Special\"Chars/And\\More|Stuff?Here*End"; - let encoded = encode_path_name(original); - let decoded = decode_path_name(&encoded); - assert_eq!(decoded, original); - } - - #[test] - fn test_no_special_chars() { - let input = "NormalFileName"; - assert_eq!(encode_path_name(input), input); - assert_eq!(decode_path_name(input), input); - } - - #[test] - fn test_leading_spaces() { - let input = " LeadingSpaces"; - let encoded = encode_path_name(input); - assert_eq!(encoded, "%SPACE%%SPACE%LeadingSpaces"); - assert_eq!(decode_path_name(&encoded), input); - } - - #[test] - fn test_trailing_spaces() { - let input = "TrailingSpaces "; - let encoded = encode_path_name(input); - assert_eq!(encoded, "TrailingSpaces%SPACE%%SPACE%"); - assert_eq!(decode_path_name(&encoded), input); - } - - #[test] - fn test_both_spaces() { - let input = " Both Spaces "; - let encoded = encode_path_name(input); - assert_eq!(encoded, "%SPACE%Both Spaces%SPACE%"); - assert_eq!(decode_path_name(&encoded), input); - } - - #[test] - fn test_middle_spaces_unchanged() { - let input = "Middle Spaces Here"; - let encoded = encode_path_name(input); - assert_eq!(encoded, "Middle Spaces Here"); - assert_eq!(decode_path_name(&encoded), input); - } - - #[test] - fn test_roundtrip_with_spaces() { - let original = " "; - let encoded = encode_path_name(original); - let decoded = decode_path_name(&encoded); - assert_eq!(decoded, original); - } - - #[test] - fn test_encode_period() { - let input = "My.Script"; - let expected = "My%DOT%Script"; - assert_eq!(encode_path_name(input), expected); - } - - #[test] - fn test_decode_period() { - let input = "My%DOT%Script"; - let expected = "My.Script"; - assert_eq!(decode_path_name(input), expected); - } - - #[test] - fn test_roundtrip_with_period() { - let original = "My.Module.Name"; - let encoded = encode_path_name(original); - let decoded = decode_path_name(&encoded); - assert_eq!(decoded, original); - } - - #[test] - fn test_encode_percent() { - let input = "My%Thing"; - let expected = "My%%Thing"; - assert_eq!(encode_path_name(input), expected); - } - - #[test] - fn test_decode_percent() { - let input = "My%%Thing"; - let expected = "My%Thing"; - assert_eq!(decode_path_name(input), expected); - } - - #[test] - fn test_roundtrip_with_percent() { - let original = "My%Thing"; - let encoded = encode_path_name(original); - let decoded = decode_path_name(&encoded); - assert_eq!(decoded, original); - } - - #[test] - fn test_percent_encoding_pattern_roundtrip() { - // This tests that a name containing what looks like an encoding pattern - // round-trips correctly and doesn't get incorrectly decoded - let original = "My%DOT%Thing"; - let encoded = encode_path_name(original); - assert_eq!(encoded, "My%%DOT%%Thing"); - let decoded = decode_path_name(&encoded); - assert_eq!(decoded, original); - } - - #[test] - fn test_combined_period_and_percent() { - let original = "My.%Thing.Other"; - let encoded = encode_path_name(original); - let decoded = decode_path_name(&encoded); - assert_eq!(decoded, original); - } - - #[test] - fn test_single_space() { - let input = " "; - let encoded = encode_path_name(input); - assert_eq!(encoded, "%SPACE%"); - assert_eq!(decode_path_name(&encoded), input); - } - - #[test] - fn test_multiple_spaces_only() { - let input = " "; - let encoded = encode_path_name(input); - assert_eq!(encoded, "%SPACE%%SPACE%%SPACE%"); - assert_eq!(decode_path_name(&encoded), input); - } - - #[test] - fn test_empty_string() { - let input = ""; - let encoded = encode_path_name(input); - assert_eq!(encoded, ""); - assert_eq!(decode_path_name(&encoded), input); - } -} diff --git a/src/serve_session.rs b/src/serve_session.rs index 04a0520f..12d7cc6e 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -114,14 +114,7 @@ impl ServeSession { let root_id = tree.get_root_id(); - let decode_windows_invalid_chars = root_project - .syncback_rules - .as_ref() - .map(|rules| rules.encode_windows_invalid_chars()) - .unwrap_or(true); - - let mut instance_context = InstanceContext::new(); - instance_context.set_decode_windows_invalid_chars(decode_windows_invalid_chars); + let instance_context = InstanceContext::new(); log::trace!("Generating snapshot of instances from VFS"); let snapshot = snapshot_from_vfs(&instance_context, &vfs, start_path)?; diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index bd2c222b..132769eb 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -66,6 +66,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, + + /// 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, } impl InstanceMetadata { @@ -78,6 +84,7 @@ impl InstanceMetadata { specified_id: None, middleware: None, schema: None, + specified_name: None, } } @@ -126,6 +133,13 @@ impl InstanceMetadata { pub fn schema(self, schema: Option) -> Self { Self { schema, ..self } } + + pub fn specified_name(self, specified_name: Option) -> Self { + Self { + specified_name, + ..self + } + } } impl Default for InstanceMetadata { @@ -140,16 +154,6 @@ pub struct InstanceContext { pub path_ignore_rules: Arc>, #[serde(skip_serializing_if = "Vec::is_empty")] pub sync_rules: Vec, - /// Whether to decode special characters in file names. - /// Characters like `%DOT%`, `%LT%`, `%GT%`, `%COLON%`, etc. will be decoded - /// back to `.`, `<`, `>`, `:`, etc. in instance names sent to the plugin. - /// Defaults to true. - #[serde(default = "decode_windows_invalid_chars_default")] - pub decode_windows_invalid_chars: bool, -} - -fn decode_windows_invalid_chars_default() -> bool { - true } impl InstanceContext { @@ -157,14 +161,9 @@ impl InstanceContext { Self { path_ignore_rules: Arc::new(Vec::new()), sync_rules: Vec::new(), - decode_windows_invalid_chars: true, } } - pub fn set_decode_windows_invalid_chars(&mut self, decode: bool) { - self.decode_windows_invalid_chars = decode; - } - /// Extend the list of ignore rules in the context with the given new rules. pub fn add_path_ignore_rules(&mut self, new_rules: I) where diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap index 80730e29..9b3650fa 100644 --- a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap +++ b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap @@ -11,8 +11,7 @@ properties: metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap index d53e0090..afbcc2ee 100644 --- a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap +++ b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap @@ -9,8 +9,7 @@ properties: {} metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap index abfbd69f..31af7868 100644 --- a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap +++ b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap @@ -11,8 +11,7 @@ properties: metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap index 5af96846..ecb3f847 100644 --- a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap +++ b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap @@ -9,8 +9,7 @@ properties: {} metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap index 2f112685..1bd818f3 100644 --- a/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap +++ b/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap @@ -10,8 +10,7 @@ added_instances: metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index 95b9b15a..385967b3 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -10,9 +10,8 @@ use rbx_dom_weak::{types::Variant, ustr}; use serde::{Deserialize, Serialize}; use crate::{ - path_encoding::encode_path_name, snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, - syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, + syncback::{name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, }; use super::{ @@ -110,10 +109,11 @@ pub fn syncback_csv<'sync>( if !meta.is_empty() { let parent = snapshot.path.parent_err()?; - let meta_name = if snapshot.encode_windows_invalid_chars() { - encode_path_name(&new_inst.name) + let instance_name = &new_inst.name; + let meta_name = if name_needs_slugify(instance_name) { + slugify_name(instance_name) } else { - new_inst.name.clone() + instance_name.clone() }; fs_snapshot.add_file( parent.join(format!("{}.meta.json5", meta_name)), diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs index 7966abb2..aa1bc860 100644 --- a/src/snapshot_middleware/dir.rs +++ b/src/snapshot_middleware/dir.rs @@ -138,6 +138,25 @@ pub fn syncback_dir_no_meta<'sync>( let mut children = Vec::new(); let mut removed_children = Vec::new(); + // Seed taken_names with existing files on disk so new children don't + // collide with files already present in the directory. + let mut taken_names: HashSet = snapshot + .vfs() + .read_dir(&snapshot.path) + .ok() + .map(|entries| { + entries + .filter_map(|e| e.ok()) + .filter_map(|e| { + e.path() + .file_name() + .and_then(|n| n.to_str()) + .map(|n| n.to_lowercase()) + }) + .collect() + }) + .unwrap_or_default(); + // Detect duplicate child names (case-insensitive for file system safety). // We skip duplicates instead of failing, tracking them in stats. let mut child_name_counts: HashMap = HashMap::new(); @@ -220,10 +239,23 @@ pub fn syncback_dir_no_meta<'sync>( continue; } // This child exists in both doms. Pass it on. - children.push(snapshot.with_joined_path(*new_child_ref, Some(old_child.id()))?); + let (child_snap, _needs_meta) = snapshot.with_joined_path( + *new_child_ref, + Some(old_child.id()), + &taken_names, + )?; + if let Some(name) = child_snap.path.file_name().and_then(|n| n.to_str()) { + taken_names.insert(name.to_lowercase()); + } + children.push(child_snap); } else { // The child only exists in the the new dom - children.push(snapshot.with_joined_path(*new_child_ref, None)?); + let (child_snap, _needs_meta) = + snapshot.with_joined_path(*new_child_ref, None, &taken_names)?; + if let Some(name) = child_snap.path.file_name().and_then(|n| n.to_str()) { + taken_names.insert(name.to_lowercase()); + } + children.push(child_snap); } } // Any children that are in the old dom but not the new one are removed. @@ -267,7 +299,12 @@ pub fn syncback_dir_no_meta<'sync>( if snapshot.should_ignore_tree(*new_child_ref) { continue; } - children.push(snapshot.with_joined_path(*new_child_ref, None)?); + let (child_snap, _needs_meta) = + snapshot.with_joined_path(*new_child_ref, None, &taken_names)?; + if let Some(name) = child_snap.path.file_name().and_then(|n| n.to_str()) { + taken_names.insert(name.to_lowercase()); + } + children.push(child_snap); } } let mut fs_snapshot = FsSnapshot::new(); diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index cc312258..96056e46 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -35,20 +35,14 @@ pub fn snapshot_json_model( format!("File is not a valid JSON model: {}", path.display()) })?; - if let Some(top_level_name) = &instance.name { - let new_name = format!("{}.model.json5", top_level_name); + // If the JSON has a name property, preserve it in metadata for syncback + let specified_name = instance.name.clone(); - log::warn!( - "Model at path {} had a top-level Name field. \ - This field has been ignored since Rojo 6.0.\n\ - Consider removing this field and renaming the file to {}.", - new_name, - path.display() - ); + // Use the name from JSON if present, otherwise fall back to filename-derived name + if instance.name.is_none() { + instance.name = Some(name.to_owned()); } - instance.name = Some(name.to_owned()); - let id = instance.id.take().map(RojoRef::new); let schema = instance.schema.take(); @@ -62,7 +56,8 @@ pub fn snapshot_json_model( .relevant_paths(vec![vfs.canonicalize(path)?]) .context(context) .specified_id(id) - .schema(schema); + .schema(schema) + .specified_name(specified_name); Ok(Some(snapshot)) } @@ -81,6 +76,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(); } let serialized = match crate::json::to_vec_pretty_sorted(&model) { diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index 3fc31824..0d79d8af 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -8,9 +8,8 @@ use rbx_dom_weak::{ }; use crate::{ - path_encoding::encode_path_name, snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, - syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, + syncback::{name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, }; use super::{ @@ -165,10 +164,11 @@ pub fn syncback_lua<'sync>( .or_else(|| file_stem.strip_suffix(".legacy")) .unwrap_or(file_stem); let meta_name = if meta_name.is_empty() { - if snapshot.encode_windows_invalid_chars() { - encode_path_name(&new_inst.name) + let instance_name = &new_inst.name; + if name_needs_slugify(instance_name) { + slugify_name(instance_name) } else { - new_inst.name.clone() + instance_name.clone() } } else { meta_name.to_string() diff --git a/src/snapshot_middleware/meta_file.rs b/src/snapshot_middleware/meta_file.rs index 2f929127..6a1ca9e2 100644 --- a/src/snapshot_middleware/meta_file.rs +++ b/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::{name_needs_slugify, SyncbackSnapshot}, RojoRef, }; @@ -36,6 +39,9 @@ pub struct AdjacentMetadata { #[serde(default, skip_serializing_if = "IndexMap::is_empty")] pub attributes: IndexMap, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(skip)] pub path: PathBuf, } @@ -52,9 +58,8 @@ impl AdjacentMetadata { _name: &str, snapshot: &mut InstanceSnapshot, ) -> anyhow::Result<()> { - // Use the file stem from the actual path (which has encoded chars) - // rather than the decoded name, so meta files are found correctly - // when encodeWindowsInvalidChars is enabled. + // Use the file stem from the actual path (which may be slugified) + // rather than the instance name, so meta files are found correctly. // Strip known script type suffixes to get the base name for the meta file. let file_stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or(""); let base_name = file_stem @@ -156,6 +161,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 name_needs_slugify(instance_name) { + Some(instance_name.clone()) + } else { + None + } + } else { + None + } + }); + Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { Some(true) @@ -164,6 +187,7 @@ impl AdjacentMetadata { }, properties, attributes, + name, path, id: None, schema, @@ -225,11 +249,26 @@ 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() + ); + } + if let Some(name) = &self.name { + snapshot.name = name.clone().into(); + } + 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(()) } @@ -238,11 +277,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 @@ -274,6 +315,9 @@ pub struct DirectoryMetadata { #[serde(skip_serializing_if = "Option::is_none")] pub class_name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub name: Option, + #[serde(skip)] pub path: PathBuf, } @@ -383,6 +427,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 name_needs_slugify(instance_name) { + Some(instance_name.clone()) + } else { + None + } + } else { + None + } + }); + Ok(Some(Self { ignore_unknown_instances: if ignore_unknown_instances { Some(true) @@ -392,6 +454,7 @@ impl DirectoryMetadata { properties, attributes, class_name: None, + name, path, id: None, schema, @@ -404,6 +467,7 @@ impl DirectoryMetadata { self.apply_properties(snapshot)?; self.apply_id(snapshot)?; self.apply_schema(snapshot)?; + self.apply_name(snapshot)?; Ok(()) } @@ -475,17 +539,34 @@ 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() + ); + } + if let Some(name) = &self.name { + snapshot.name = name.clone().into(); + } + 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 { diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 5d60c931..7ea64881 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -30,7 +30,6 @@ use serde::{Deserialize, Serialize}; use crate::{ glob::Glob, - path_encoding::decode_path_name, project::DEFAULT_PROJECT_NAMES, syncback::{SyncbackReturn, SyncbackSnapshot}, }; @@ -72,19 +71,17 @@ pub fn snapshot_from_vfs( if meta.is_dir() { let (middleware, dir_name, init_path) = get_dir_middleware(vfs, path)?; - // Decode directory name if the flag is set - let decoded_name = if context.decode_windows_invalid_chars { - decode_path_name(dir_name) - } else { - dir_name.to_string() - }; + // The directory name is used as-is from the filesystem. + // If a different instance name is desired, it comes from the + // `name` field in init.meta.json / init.meta.json5 (applied later + // by DirectoryMetadata::apply_name). // TODO: Support user defined init paths // If and when we do, make sure to go support it in // `Project::set_file_name`, as right now it special-cases // `default.project.json5` as an `init` path. match middleware { - Middleware::Dir => middleware.snapshot(context, vfs, path, &decoded_name), - _ => middleware.snapshot(context, vfs, &init_path, &decoded_name), + Middleware::Dir => middleware.snapshot(context, vfs, path, dir_name), + _ => middleware.snapshot(context, vfs, &init_path, dir_name), } } else { let file_name = path @@ -166,23 +163,17 @@ fn snapshot_from_path( vfs: &Vfs, path: &Path, ) -> anyhow::Result> { - // Helper to decode name if the flag is set - let decode_name = |name: &str| -> String { - if context.decode_windows_invalid_chars { - decode_path_name(name) - } else { - name.to_string() - } - }; - + // File names are used as-is from the filesystem. If a different instance + // name is needed (e.g. for names with special chars), it comes from the + // `name` field in adjacent .meta.json / .model.json files. if let Some(rule) = context.get_user_sync_rule(path) { - let name = decode_name(rule.file_name_for_path(path)?); - return rule.middleware.snapshot(context, vfs, path, &name); + let name = rule.file_name_for_path(path)?; + return rule.middleware.snapshot(context, vfs, path, name); } else { for rule in default_sync_rules() { if rule.matches(path) { - let name = decode_name(rule.file_name_for_path(path)?); - return rule.middleware.snapshot(context, vfs, path, &name); + let name = rule.file_name_for_path(path)?; + return rule.middleware.snapshot(context, vfs, path, name); } } } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index bccce551..bee9b971 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -647,6 +647,14 @@ pub fn syncback_project<'sync>( // All of the children in this loop are by their nature not in the // project, so we just need to run syncback on them. + // + // Track taken names per parent directory so siblings under the same + // parent deduplicate against each other (and against existing files). + let mut taken_names_per_dir: std::collections::HashMap< + std::path::PathBuf, + std::collections::HashSet, + > = std::collections::HashMap::new(); + for (name, new_child) in new_child_map.drain() { // Skip instances of ignored classes if snapshot.should_ignore_class(&new_child.class) { @@ -680,11 +688,38 @@ pub fn syncback_project<'sync>( // concern with directories because they're singular things, // files that contain their own children. if parent_middleware != Middleware::Project { - descendant_snapshots.push(snapshot.with_base_path( + let taken_names = taken_names_per_dir + .entry(parent_path.clone()) + .or_insert_with(|| { + // Seed with existing files on disk so new children + // don't collide with files already present. + vfs.read_dir(&parent_path) + .ok() + .map(|entries| { + entries + .filter_map(|e| e.ok()) + .filter_map(|e| { + e.path() + .file_name() + .and_then(|n| n.to_str()) + .map(|n| n.to_lowercase()) + }) + .collect() + }) + .unwrap_or_default() + }); + let (child_snap, _needs_meta) = snapshot.with_base_path( &parent_path, new_child.referent(), None, - )?); + taken_names, + )?; + if let Some(file_name) = + child_snap.path.file_name().and_then(|n| n.to_str()) + { + taken_names.insert(file_name.to_lowercase()); + } + descendant_snapshots.push(child_snap); } } } diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap index 49ba1e21..2ec17a3a 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.csv - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init.snap index 4a178832..d6e60fee 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init.snap @@ -19,8 +19,7 @@ metadata: - /root/init.server.lua - /root/init.client.lua - /root/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init_with_meta.snap index 85e790a2..2ff5b269 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_init_with_meta.snap @@ -19,8 +19,7 @@ metadata: - /root/init.server.lua - /root/init.client.lua - /root/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap index ae282829..196e430d 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.csv - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap index f1c96a44..23ecb41c 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap @@ -19,8 +19,7 @@ metadata: - /foo/init.server.lua - /foo/init.client.lua - /foo/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap index 52ca0c33..0d7dddbf 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap @@ -19,8 +19,7 @@ metadata: - /foo/init.server.lua - /foo/init.client.lua - /foo/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ @@ -45,8 +44,7 @@ children: - /foo/Child/init.server.lua - /foo/Child/init.client.lua - /foo/Child/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: dir schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap index 00114ed0..c2ca07dc 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.json5 - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__with_metadata.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__with_metadata.snap index 99cc6fe4..396cc181 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__with_metadata.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__with_metadata.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.json5 - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap index 13bb28ec..9690c7ec 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap @@ -9,8 +9,7 @@ metadata: Path: /foo.model.json5 relevant_paths: - /foo.model.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ @@ -24,8 +23,7 @@ children: metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap index 13bb28ec..9690c7ec 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap @@ -9,8 +9,7 @@ metadata: Path: /foo.model.json5 relevant_paths: - /foo.model.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ @@ -24,8 +23,7 @@ children: metadata: ignore_unknown_instances: false relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap index 60e211ad..ea4d0102 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.client.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs.snap index 3178e91f..e4978c2b 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs.snap @@ -19,8 +19,7 @@ metadata: - /root/init.server.lua - /root/init.client.lua - /root/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs_with_meta.snap index 38364335..da906c76 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__init_module_from_vfs_with_meta.snap @@ -19,8 +19,7 @@ metadata: - /root/init.server.lua - /root/init.client.lua - /root/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__legacy_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__legacy_from_vfs.snap index 0c98cf63..b609a3a3 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__legacy_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__legacy_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.legacy.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__local_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__local_from_vfs.snap index ab97b74c..a8c84cba 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__local_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__local_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.local.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap index a9312d8a..30a628f0 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap index 3f40a281..50818283 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__plugin_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__plugin_from_vfs.snap index 5f3414cc..daf0d5b5 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__plugin_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__plugin_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.plugin.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_disabled.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_disabled.snap index 585120e6..ffa29e3c 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_disabled.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_disabled.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /bar.server.luau - /bar.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap index 6c272e47..756c6567 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.server.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_with_meta.snap index 90a0788b..78e84a4c 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_with_meta.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.server.luau - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json.snap index 854eafeb..f5329161 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json.snap @@ -9,8 +9,7 @@ metadata: - /foo/bar.meta.json - /foo/bar.meta.jsonc - /foo/bar.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json5.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json5.snap index 107603bd..f15c6ff5 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json5.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_json5.snap @@ -7,8 +7,7 @@ metadata: ignore_unknown_instances: false relevant_paths: - /foo/bar.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_jsonc.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_jsonc.snap index 854eafeb..f5329161 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_jsonc.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__adjacent_read_jsonc.snap @@ -9,8 +9,7 @@ metadata: - /foo/bar.meta.json - /foo/bar.meta.jsonc - /foo/bar.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json.snap index 09c35784..49729bff 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json.snap @@ -9,8 +9,7 @@ metadata: - /foo/init.meta.json - /foo/init.meta.jsonc - /foo/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json5.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json5.snap index 1f5d141b..5f7ab6cd 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json5.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_json5.snap @@ -7,8 +7,7 @@ metadata: ignore_unknown_instances: false relevant_paths: - /foo/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_jsonc.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_jsonc.snap index 09c35784..49729bff 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_jsonc.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__meta_file__test__directory_read_jsonc.snap @@ -9,8 +9,7 @@ metadata: - /foo/init.meta.json - /foo/init.meta.jsonc - /foo/init.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap index 151f7990..0e0a7fc5 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap @@ -9,8 +9,7 @@ metadata: Path: /foo/default.project.json5 relevant_paths: - /foo/default.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap index ce488a4c..07a7aca4 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap @@ -9,8 +9,7 @@ metadata: Path: /foo/hello.project.json5 relevant_paths: - /foo/hello.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap index 124e588a..b19c2181 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo/other.project.json5 - /foo/default.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: project schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap index 6250d397..2df85f3f 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap @@ -9,8 +9,7 @@ metadata: Path: /foo.project.json5 relevant_paths: - /foo.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ @@ -29,8 +28,7 @@ children: $className: Model parent_class: Folder relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap index e2be0084..901bf4a4 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo/other.project.json5 - /foo/default.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: project schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap index 5f24e8ef..c93e6832 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo/other.project.json5 - /foo/default.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: project schema: ~ @@ -30,8 +29,7 @@ children: $className: Model parent_class: Folder relevant_paths: [] - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap index eb10ca84..8dc96da2 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap @@ -11,8 +11,7 @@ metadata: - /foo/other.txt - /foo/other.meta.json5 - /foo/default.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: text schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap index c9615222..88599384 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap @@ -9,8 +9,7 @@ metadata: Path: /foo.project.json5 relevant_paths: - /foo.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap index d3506cd6..a953d6be 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap @@ -9,8 +9,7 @@ metadata: Path: /foo.project.json5 relevant_paths: - /foo.project.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap index 979aba72..cbee1125 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.toml - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__with_metadata.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__with_metadata.snap index 6bd178df..46f5eb5e 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__with_metadata.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__with_metadata.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - foo.toml - foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap index 40bb1f5f..f89b5009 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.txt - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__with_metadata.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__with_metadata.snap index a732b1f5..aefd549f 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__with_metadata.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__with_metadata.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.txt - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__instance_from_vfs.snap index 3ae974c1..f17cdf34 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__instance_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__instance_from_vfs.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - /foo.yaml - /foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: ~ middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__with_metadata.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__with_metadata.snap index 4d87863b..f54d3e7d 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__with_metadata.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__yaml__test__with_metadata.snap @@ -10,8 +10,7 @@ metadata: relevant_paths: - foo.yaml - foo.meta.json5 - context: - decode_windows_invalid_chars: true + context: {} specified_id: manually specified middleware: ~ schema: ~ diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 48216a0b..82e00c03 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -6,9 +6,8 @@ use rbx_dom_weak::types::Variant; use rbx_dom_weak::ustr; use crate::{ - path_encoding::encode_path_name, snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, - syncback::{FsSnapshot, SyncbackReturn, SyncbackSnapshot}, + syncback::{name_needs_slugify, slugify_name, FsSnapshot, SyncbackReturn, SyncbackSnapshot}, }; use super::{meta_file::AdjacentMetadata, PathExt as _}; @@ -59,10 +58,11 @@ pub fn syncback_txt<'sync>( if !meta.is_empty() { let parent = snapshot.path.parent_err()?; - let meta_name = if snapshot.encode_windows_invalid_chars() { - encode_path_name(&new_inst.name) + let instance_name = &new_inst.name; + let meta_name = if name_needs_slugify(instance_name) { + slugify_name(instance_name) } else { - new_inst.name.clone() + instance_name.clone() }; fs_snapshot.add_file( parent.join(format!("{}.meta.json5", meta_name)), diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index 904fb9a6..09258883 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -2,38 +2,48 @@ //! middleware. use std::borrow::Cow; +use std::collections::HashSet; use anyhow::Context; use rbx_dom_weak::Instance; -use crate::{ - path_encoding::encode_path_name, snapshot::InstanceWithMeta, snapshot_middleware::Middleware, -}; +use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware}; -pub fn name_for_inst<'old>( +/// Generates a filesystem name for an instance. Returns `(filename, needs_meta_name)`. +/// +/// - If `old_inst` exists, its existing path is preserved (incremental mode). +/// - For new instances, names with forbidden chars are slugified and deduplicated +/// against `taken_names`. The `bool` is `true` when the filesystem name differs +/// from the instance name (meaning a `name` field must be written in metadata). +pub fn name_for_inst<'a>( middleware: Middleware, - new_inst: &Instance, - old_inst: Option>, - encode_invalid_chars: bool, -) -> anyhow::Result> { + new_inst: &'a Instance, + old_inst: Option>, + taken_names: &HashSet, +) -> anyhow::Result<(Cow<'a, str>, bool)> { if let Some(old_inst) = old_inst { if let Some(source) = old_inst.metadata().relevant_paths.first() { - source + let name = source .file_name() .and_then(|s| s.to_str()) - .map(Cow::Borrowed) - .context("sources on the file system should be valid unicode and not be stubs") + .context("sources on the file system should be valid unicode and not be stubs")?; + Ok((Cow::Borrowed(name), false)) } else { - // This is technically not /always/ true, but we want to avoid - // running syncback on anything that has no instigating source - // anyway. anyhow::bail!( "members of 'old' trees should have an instigating source. Somehow, {} did not.", old_inst.name(), ); } } else { - Ok(match middleware { + // Determine base name: slugify if the raw name isn't filesystem-safe + let needs_slugify = name_needs_slugify(&new_inst.name); + let base = if needs_slugify { + slugify_name(&new_inst.name) + } else { + new_inst.name.clone() + }; + + match middleware { Middleware::Dir | Middleware::CsvDir | Middleware::ServerScriptDir @@ -42,45 +52,17 @@ pub fn name_for_inst<'old>( | Middleware::PluginScriptDir | Middleware::LocalScriptDir | Middleware::LegacyScriptDir => { - let name = if encode_invalid_chars { - let encoded = encode_path_name(&new_inst.name); - // Validate for issues not fixed by encoding (trailing space/dot, control chars, reserved names) - validate_file_name(&encoded).with_context(|| { - format!("name '{}' (encoded from '{}') is not legal to write to the file system", encoded, new_inst.name) - })?; - encoded - } else { - validate_file_name(&new_inst.name).with_context(|| { - format!( - "name '{}' is not legal to write to the file system", - new_inst.name - ) - })?; - new_inst.name.clone() - }; - Cow::Owned(name) + let deduped = deduplicate_name(&base, taken_names); + let needs_meta = needs_slugify || deduped != base; + Ok((Cow::Owned(deduped), needs_meta)) } _ => { let extension = extension_for_middleware(middleware); - let name = if encode_invalid_chars { - let encoded = encode_path_name(&new_inst.name); - // Validate for issues not fixed by encoding (trailing space/dot, control chars, reserved names) - validate_file_name(&encoded).with_context(|| { - format!("name '{}' (encoded from '{}') is not legal to write to the file system", encoded, new_inst.name) - })?; - encoded - } else { - validate_file_name(&new_inst.name).with_context(|| { - format!( - "name '{}' is not legal to write to the file system", - new_inst.name - ) - })?; - new_inst.name.clone() - }; - Cow::Owned(format!("{name}.{extension}")) + let deduped = deduplicate_name(&base, taken_names); + let needs_meta = needs_slugify || deduped != base; + Ok((Cow::Owned(format!("{deduped}.{extension}")), needs_meta)) } - }) + } } } @@ -129,6 +111,138 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [ /// in a file's name. const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\']; +/// Characters that must be slugified when converting instance names to +/// filesystem names. Extends FORBIDDEN_CHARS with: +/// - `~` (conflicts with dedup suffix `~1`, `~2`, etc.) +/// +/// Note: spaces are NOT in this list. Middle spaces are valid in filenames. +/// Leading/trailing spaces are handled separately by boundary checks. +const SLUGIFY_CHARS: [char; 10] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\', '~']; + +/// Suffixes (case-insensitive) that, if an instance name ends with one, +/// would create a compound extension that tricks Rojo's sync rule matching. +/// +/// Example: instance `foo.server` + extension `.luau` → `foo.server.luau` +/// which matches `*.server.luau` (ServerScript) instead of the intended +/// `*.luau` (ModuleScript). +const DANGEROUS_SUFFIXES: [&str; 8] = [ + ".server", ".client", ".plugin", ".local", ".legacy", ".meta", ".model", ".project", +]; + +/// Returns `true` if an instance name contains characters or patterns that +/// require slugification for safe filesystem use. +pub fn name_needs_slugify(name: &str) -> bool { + if name.is_empty() { + return true; + } + if name.starts_with(' ') || name.ends_with(' ') || name.ends_with('.') { + return true; + } + for ch in name.chars() { + if SLUGIFY_CHARS.contains(&ch) || ch.is_control() { + return true; + } + } + // Check for dangerous suffixes that would collide with Rojo's compound + // extensions (e.g., ".server" + ".luau" → ".server.luau") + if has_dangerous_suffix(name) { + return true; + } + let name_lower = name.to_lowercase(); + for forbidden in INVALID_WINDOWS_NAMES { + if name_lower == forbidden.to_lowercase() { + return true; + } + } + false +} + +/// Returns `true` if the name ends with a dangerous suffix (case-insensitive). +fn has_dangerous_suffix(name: &str) -> bool { + let lower = name.to_lowercase(); + DANGEROUS_SUFFIXES.iter().any(|s| lower.ends_with(s)) +} + +/// Slugifies a name by replacing forbidden filesystem characters with +/// underscores and ensuring the result is a valid file name. +/// +/// Replaces OS-forbidden chars and `~` (dedup separator) with underscores. +/// Strips leading/trailing spaces. Neutralises dangerous suffixes +/// (`.server`, `.meta`, etc.) that would collide with Rojo's compound +/// extensions by replacing the offending dot with `_`. +/// +/// This is a pure, stateless function. It does NOT handle collisions -- +/// use `deduplicate_name()` after slugifying to resolve those. +pub fn slugify_name(name: &str) -> String { + let mut result = String::with_capacity(name.len()); + + for ch in name.chars() { + if SLUGIFY_CHARS.contains(&ch) || ch.is_control() { + result.push('_'); + } else { + result.push(ch); + } + } + + // Strip leading spaces + while result.starts_with(' ') { + result.remove(0); + } + + // Neutralise dangerous suffixes by replacing the last dot before + // the suffix with an underscore. Loop because replacement could + // reveal another dangerous suffix (e.g., "a.meta.server" → fix + // ".server" → "a.meta_server" → still has ".meta" → fix that too). + while has_dangerous_suffix(&result) { + if let Some(pos) = result.rfind('.') { + result.replace_range(pos..pos + 1, "_"); + } else { + break; + } + } + + // 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; + } + } + + // Strip trailing spaces and dots (invalid on Windows) + while result.ends_with(' ') || result.ends_with('.') { + result.pop(); + } + + // If the result is empty or all underscores, use a fallback + if result.is_empty() || result.chars().all(|c| c == '_') { + result = "instance".to_string(); + } + + result +} + +/// Appends ~1, ~2, etc. to avoid collisions. Returns the name as-is if +/// unclaimed. +/// +/// Comparisons are **case-insensitive** because Windows and macOS have +/// case-insensitive filesystems. `taken_names` must contain **lowercased** +/// entries for this to work correctly. +pub fn deduplicate_name(base: &str, taken_names: &std::collections::HashSet) -> String { + let base_lower = base.to_lowercase(); + if !taken_names.contains(&base_lower) { + return base.to_string(); + } + for i in 1.. { + let candidate = format!("{base}~{i}"); + if !taken_names.contains(&candidate.to_lowercase()) { + return candidate; + } + } + unreachable!() +} + /// 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. @@ -161,3 +275,1844 @@ pub fn validate_file_name>(name: S) -> anyhow::Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use rbx_dom_weak::{InstanceBuilder, WeakDom}; + + // ── slugify_name ────────────────────────────────────────────────── + + #[test] + fn slugify_clean_name_unchanged() { + assert_eq!(slugify_name("Hello"), "Hello"); + assert_eq!(slugify_name("MyModule"), "MyModule"); + assert_eq!(slugify_name("foo_bar_baz"), "foo_bar_baz"); + } + + #[test] + fn slugify_each_slugify_char() { + for ch in SLUGIFY_CHARS { + let input = format!("A{ch}B"); + let result = slugify_name(&input); + assert_eq!(result, "A_B", "slugify char {ch:?} should become _"); + } + } + + #[test] + fn slugify_dots_preserved_when_safe() { + // Dots are allowed when they don't form a dangerous suffix + assert_eq!(slugify_name("v1.0"), "v1.0"); + assert_eq!(slugify_name("foo.bar"), "foo.bar"); + assert_eq!(slugify_name("hello.world"), "hello.world"); + } + + #[test] + fn slugify_multiple_forbidden_chars() { + assert_eq!(slugify_name("Hey/Bro"), "Hey_Bro"); + assert_eq!(slugify_name("Hey:Bro"), "Hey_Bro"); + assert_eq!(slugify_name("ac:d"), "a_b_c_d"); + assert_eq!(slugify_name("What?Module"), "What_Module"); + } + + #[test] + fn slugify_all_slugify_chars_string() { + // Every char is slugified → all underscores → fallback to "instance" + let input: String = SLUGIFY_CHARS.iter().collect(); + assert_eq!(slugify_name(&input), "instance"); + } + + #[test] + fn slugify_windows_reserved_names() { + assert_eq!(slugify_name("CON"), "CON_"); + assert_eq!(slugify_name("PRN"), "PRN_"); + assert_eq!(slugify_name("AUX"), "AUX_"); + assert_eq!(slugify_name("NUL"), "NUL_"); + assert_eq!(slugify_name("COM1"), "COM1_"); + assert_eq!(slugify_name("LPT9"), "LPT9_"); + } + + #[test] + fn slugify_windows_reserved_case_insensitive() { + assert_eq!(slugify_name("con"), "con_"); + assert_eq!(slugify_name("Con"), "Con_"); + assert_eq!(slugify_name("nul"), "nul_"); + } + + #[test] + fn slugify_trailing_dot() { + // Trailing dots are stripped (invalid on Windows) + assert_eq!(slugify_name("hello."), "hello"); + assert_eq!(slugify_name("hello.."), "hello"); + } + + #[test] + fn slugify_space() { + // Middle spaces are preserved (valid in filenames) + assert_eq!(slugify_name("hello world"), "hello world"); + // Trailing spaces are stripped + assert_eq!(slugify_name("hello "), "hello"); + // Leading spaces are stripped + assert_eq!(slugify_name(" hello"), "hello"); + // All spaces → empty after strip → fallback + assert_eq!(slugify_name(" "), "instance"); + assert_eq!(slugify_name(" "), "instance"); + } + + #[test] + fn slugify_trailing_dot_and_space_mixed() { + // "hello. " → trailing " " stripped → "hello." → trailing "." stripped → "hello" + assert_eq!(slugify_name("hello. "), "hello"); + // "hello ." → trailing "." stripped → "hello " → trailing " " stripped → "hello" + assert_eq!(slugify_name("hello ."), "hello"); + } + + #[test] + fn slugify_dangerous_suffixes() { + // Suffixes that would collide with Rojo's compound extensions + assert_eq!(slugify_name("foo.server"), "foo_server"); + assert_eq!(slugify_name("foo.client"), "foo_client"); + assert_eq!(slugify_name("foo.plugin"), "foo_plugin"); + assert_eq!(slugify_name("foo.local"), "foo_local"); + assert_eq!(slugify_name("foo.legacy"), "foo_legacy"); + assert_eq!(slugify_name("foo.meta"), "foo_meta"); + assert_eq!(slugify_name("foo.model"), "foo_model"); + assert_eq!(slugify_name("foo.project"), "foo_project"); + } + + #[test] + fn slugify_dangerous_suffixes_case_insensitive() { + assert_eq!(slugify_name("foo.Server"), "foo_Server"); + assert_eq!(slugify_name("foo.META"), "foo_META"); + assert_eq!(slugify_name("foo.Model"), "foo_Model"); + } + + #[test] + fn slugify_nested_dangerous_suffixes() { + // ".server" suffix is fixed; ".meta" in the middle is fine (not a suffix) + assert_eq!(slugify_name("a.meta.server"), "a.meta_server"); + // Both are suffixes when stacked: fix ".server" → "a.meta_server" (no more suffix) + // But if the name is just ".meta": + assert_eq!(slugify_name("a.meta"), "a_meta"); + } + + #[test] + fn slugify_empty_string() { + assert_eq!(slugify_name(""), "instance"); + } + + #[test] + fn slugify_single_forbidden_char() { + // Single forbidden char → "_" → all underscores → fallback + assert_eq!(slugify_name("/"), "instance"); + assert_eq!(slugify_name("*"), "instance"); + } + + #[test] + fn slugify_tilde() { + // ~ is slugified to prevent confusion with dedup suffix ~N + assert_eq!(slugify_name("foo~1"), "foo_1"); + assert_eq!(slugify_name("~bar"), "_bar"); + assert_eq!(slugify_name("a~b~c"), "a_b_c"); + } + + #[test] + fn slugify_unicode_preserved() { + assert_eq!(slugify_name("日本語"), "日本語"); + assert_eq!(slugify_name("café"), "café"); + assert_eq!(slugify_name("émoji🎮"), "émoji🎮"); + } + + #[test] + fn slugify_mixed_valid_and_forbidden() { + assert_eq!(slugify_name("src/main:test"), "src_main_test"); + assert_eq!(slugify_name("file<1>"), "file_1_"); + // Dot preserved when safe, forbidden chars replaced + assert_eq!(slugify_name("v1.0/release"), "v1.0_release"); + } + + // ── deduplicate_name ────────────────────────────────────────────── + // + // Contract: taken_names must contain LOWERCASED entries. + + #[test] + fn dedup_no_collision() { + let taken = HashSet::new(); + assert_eq!(deduplicate_name("Foo", &taken), "Foo"); + } + + #[test] + fn dedup_single_collision() { + let taken: HashSet = ["foo".to_string()].into_iter().collect(); + assert_eq!(deduplicate_name("Foo", &taken), "Foo~1"); + } + + #[test] + fn dedup_multiple_collisions() { + let taken: HashSet = ["foo", "foo~1", "foo~2"] + .into_iter() + .map(String::from) + .collect(); + assert_eq!(deduplicate_name("Foo", &taken), "Foo~3"); + } + + #[test] + fn dedup_skips_taken_suffix() { + let taken: HashSet = ["foo", "foo~1"].into_iter().map(String::from).collect(); + assert_eq!(deduplicate_name("Foo", &taken), "Foo~2"); + } + + #[test] + fn dedup_gap_in_suffixes() { + let taken: HashSet = ["foo", "foo~2"].into_iter().map(String::from).collect(); + assert_eq!(deduplicate_name("Foo", &taken), "Foo~1"); + } + + #[test] + fn dedup_natural_vs_slug_collision() { + let taken: HashSet = ["hey_bro".to_string()].into_iter().collect(); + assert_eq!(deduplicate_name("Hey_Bro", &taken), "Hey_Bro~1"); + } + + #[test] + fn dedup_empty_taken_set() { + let taken = HashSet::new(); + assert_eq!(deduplicate_name("anything", &taken), "anything"); + } + + #[test] + fn dedup_case_insensitive() { + // "foo" is taken, "Foo" should collide (case-insensitive filesystem) + let taken: HashSet = ["foo".to_string()].into_iter().collect(); + assert_eq!(deduplicate_name("Foo", &taken), "Foo~1"); + assert_eq!(deduplicate_name("FOO", &taken), "FOO~1"); + assert_eq!(deduplicate_name("foo", &taken), "foo~1"); + } + + #[test] + fn dedup_case_only_difference() { + // Two instances: "MyScript" and "myscript" - second must get ~1 + let taken: HashSet = ["myscript".to_string()].into_iter().collect(); + assert_eq!(deduplicate_name("MyScript", &taken), "MyScript~1"); + } + + // ── name_needs_slugify ────────────────────────────────────────────── + + #[test] + fn needs_slugify_clean_names() { + assert!(!name_needs_slugify("Hello")); + assert!(!name_needs_slugify("my_module")); + assert!(!name_needs_slugify("test-123")); + } + + #[test] + fn needs_slugify_os_forbidden() { + assert!(name_needs_slugify("a/b")); + assert!(name_needs_slugify("a:b")); + assert!(name_needs_slugify("a*b")); + } + + #[test] + fn needs_slugify_tilde() { + assert!(name_needs_slugify("foo~1")); + assert!(name_needs_slugify("~bar")); + } + + #[test] + fn needs_slugify_safe_dots_allowed() { + // Dots in names that don't form dangerous suffixes are fine + assert!(!name_needs_slugify("v1.0")); + assert!(!name_needs_slugify("foo.bar")); + } + + #[test] + fn needs_slugify_dangerous_suffix() { + assert!(name_needs_slugify("foo.server")); + assert!(name_needs_slugify("foo.Server")); + assert!(name_needs_slugify("foo.meta")); + assert!(name_needs_slugify("foo.model")); + assert!(name_needs_slugify("foo.project")); + assert!(name_needs_slugify("foo.client")); + } + + #[test] + fn needs_slugify_windows_reserved() { + assert!(name_needs_slugify("CON")); + assert!(name_needs_slugify("con")); + assert!(name_needs_slugify("NUL")); + } + + #[test] + fn needs_slugify_leading_or_trailing_space() { + assert!(name_needs_slugify("hello ")); // trailing + assert!(name_needs_slugify(" hello")); // leading + assert!(name_needs_slugify("hello.")); // trailing dot + assert!(!name_needs_slugify("hello world")); // middle space is fine + } + + #[test] + fn needs_slugify_empty() { + assert!(name_needs_slugify("")); + } + + #[test] + fn needs_slugify_control_chars() { + assert!(name_needs_slugify("a\x00b")); + assert!(name_needs_slugify("tab\there")); + } + + // ── validate_file_name ──────────────────────────────────────────── + + #[test] + fn validate_clean_names() { + assert!(validate_file_name("Hello").is_ok()); + assert!(validate_file_name("my_file").is_ok()); + assert!(validate_file_name("test-123").is_ok()); + assert!(validate_file_name("日本語").is_ok()); + } + + #[test] + fn validate_rejects_forbidden_chars() { + for ch in FORBIDDEN_CHARS { + let name = format!("a{ch}b"); + assert!(validate_file_name(&name).is_err(), "should reject {ch:?}"); + } + } + + #[test] + fn validate_rejects_trailing_dot() { + assert!(validate_file_name("hello.").is_err()); + } + + #[test] + fn validate_rejects_trailing_space() { + assert!(validate_file_name("hello ").is_err()); + } + + #[test] + fn validate_rejects_windows_reserved() { + assert!(validate_file_name("CON").is_err()); + assert!(validate_file_name("NUL").is_err()); + assert!(validate_file_name("COM1").is_err()); + } + + #[test] + fn validate_rejects_control_chars() { + assert!(validate_file_name("hello\x00world").is_err()); + assert!(validate_file_name("tab\there").is_err()); + } + + #[test] + fn validate_slugified_names_always_pass() { + // Any output of slugify_name should pass validation + let nasty_inputs = [ + "CON", + "hello/world", + "a:b:c", + "test?", + "file*glob", + "trailing.", + "trailing ", + "", + "<>:\"/\\|?*", + "COM1", + "LPT9", + "foo~1", + "foo.server", + "bar.meta", + "v1.0~beta", + " leading space", + "trailing space ", + "hello world", + "a.meta.server", + ]; + for input in nasty_inputs { + let slug = slugify_name(input); + assert!( + validate_file_name(&slug).is_ok(), + "slugify_name({input:?}) = {slug:?} should pass validation" + ); + } + } + + // ── name_for_inst (new instances, no old_inst) ──────────────────── + + /// Helper: create a WeakDom with a child instance of given name and class, + /// return the dom and the child's Ref. + fn make_inst(name: &str, class: &str) -> WeakDom { + let builder = InstanceBuilder::new("DataModel") + .with_child(InstanceBuilder::new(class).with_name(name)); + WeakDom::new(builder) + } + + #[test] + fn name_for_inst_clean_name_file_middleware() { + let dom = make_inst("MyModule", "ModuleScript"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = + name_for_inst(Middleware::ModuleScript, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "MyModule.luau"); + assert!(!needs_meta); + } + + #[test] + fn name_for_inst_clean_name_dir_middleware() { + let dom = make_inst("MyFolder", "Folder"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "MyFolder"); + assert!(!needs_meta); + } + + #[test] + fn name_for_inst_forbidden_chars_slugified() { + let dom = make_inst("Hey/Bro", "ModuleScript"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = + name_for_inst(Middleware::ModuleScript, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Hey_Bro.luau"); + assert!(needs_meta, "slug differs from real name, needs meta"); + } + + #[test] + fn name_for_inst_forbidden_chars_dir_middleware() { + let dom = make_inst("Hey:Bro", "Folder"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Hey_Bro"); + assert!(needs_meta); + } + + #[test] + fn name_for_inst_dedup_collision() { + let dom = make_inst("Foo", "ModuleScript"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken: HashSet = ["foo".to_string()].into_iter().collect(); + + let (filename, needs_meta) = + name_for_inst(Middleware::ModuleScript, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Foo~1.luau"); + assert!(needs_meta, "deduped name differs from original"); + } + + #[test] + fn name_for_inst_dedup_dir_collision() { + let dom = make_inst("Stuff", "Folder"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken: HashSet = ["stuff".to_string()].into_iter().collect(); + + let (filename, needs_meta) = name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Stuff~1"); + assert!(needs_meta); + } + + #[test] + fn name_for_inst_slug_plus_dedup() { + // Name with forbidden chars AND collision with existing slug + let dom = make_inst("Hey/Bro", "ModuleScript"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken: HashSet = ["hey_bro".to_string()].into_iter().collect(); + + let (filename, needs_meta) = + name_for_inst(Middleware::ModuleScript, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Hey_Bro~1.luau"); + assert!(needs_meta); + } + + #[test] + fn name_for_inst_server_script_extension() { + let dom = make_inst("Main", "Script"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = + name_for_inst(Middleware::ServerScript, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Main.server.luau"); + assert!(!needs_meta); + } + + #[test] + fn name_for_inst_client_script_extension() { + let dom = make_inst("Client", "LocalScript"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = + name_for_inst(Middleware::ClientScript, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Client.client.luau"); + assert!(!needs_meta); + } + + #[test] + fn name_for_inst_text_extension() { + let dom = make_inst("Readme", "StringValue"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + + let (filename, needs_meta) = name_for_inst(Middleware::Text, child, None, &taken).unwrap(); + assert_eq!(filename.as_ref(), "Readme.txt"); + assert!(!needs_meta); + } + + // ══════════════════════════════════════════════════════════════════ + // Comprehensive slugified-name + dedup integration tests + // + // These simulate the full name_for_inst pipeline: slugify decision, + // dedup against taken_names, filename construction, and needs_meta + // flag correctness. Every edge case, ordering, and middleware + // combination the two-way sync could encounter. + // ══════════════════════════════════════════════════════════════════ + + /// Helper: run name_for_inst with a given name, middleware, and taken set. + /// Returns (filename_string, needs_meta). + fn nfi(name: &str, mw: Middleware, taken: &[&str]) -> (String, bool) { + let dom = make_inst(name, "ModuleScript"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken_set: HashSet = taken.iter().map(|s| s.to_string()).collect(); + let (filename, needs_meta) = name_for_inst(mw, child, None, &taken_set).unwrap(); + (filename.into_owned(), needs_meta) + } + + // ── Dangerous suffix slugification through name_for_inst ───────── + + #[test] + fn nfi_dangerous_suffix_server_luau() { + let (f, m) = nfi("foo.server", Middleware::ModuleScript, &[]); + assert_eq!(f, "foo_server.luau"); + assert!(m, "dangerous suffix must trigger needs_meta"); + } + + #[test] + fn nfi_dangerous_suffix_client_luau() { + let (f, m) = nfi("bar.client", Middleware::ClientScript, &[]); + assert_eq!(f, "bar_client.client.luau"); + assert!(m); + } + + #[test] + fn nfi_dangerous_suffix_meta_dir() { + let (f, m) = nfi("Config.meta", Middleware::Dir, &[]); + assert_eq!(f, "Config_meta"); + assert!(m); + } + + #[test] + fn nfi_dangerous_suffix_model_json_model() { + let (f, m) = nfi("Part.model", Middleware::JsonModel, &[]); + assert_eq!(f, "Part_model.model.json5"); + assert!(m); + } + + #[test] + fn nfi_dangerous_suffix_project() { + let (f, m) = nfi("Game.project", Middleware::ModuleScript, &[]); + assert_eq!(f, "Game_project.luau"); + assert!(m); + } + + #[test] + #[allow(non_snake_case)] + fn nfi_dangerous_suffix_case_insensitive_SERVER() { + let (f, m) = nfi("test.SERVER", Middleware::ModuleScript, &[]); + assert_eq!(f, "test_SERVER.luau"); + assert!(m); + } + + #[test] + #[allow(non_snake_case)] + fn nfi_dangerous_suffix_case_insensitive_Meta() { + let (f, m) = nfi("stuff.Meta", Middleware::Dir, &[]); + assert_eq!(f, "stuff_Meta"); + assert!(m); + } + + // ── Safe dots pass through ─────────────────────────────────────── + + #[test] + fn nfi_safe_dot_v1_0() { + let (f, m) = nfi("v1.0", Middleware::ModuleScript, &[]); + assert_eq!(f, "v1.0.luau"); + assert!(!m); + } + + #[test] + fn nfi_safe_dot_hello_world() { + let (f, m) = nfi("hello.world", Middleware::ModuleScript, &[]); + assert_eq!(f, "hello.world.luau"); + assert!(!m); + } + + #[test] + fn nfi_safe_dot_dir() { + let (f, m) = nfi("Release.1", Middleware::Dir, &[]); + assert_eq!(f, "Release.1"); + assert!(!m); + } + + // ── Spaces in names ────────────────────────────────────────────── + + #[test] + fn nfi_middle_space_preserved() { + let (f, m) = nfi("My Module", Middleware::ModuleScript, &[]); + assert_eq!(f, "My Module.luau"); + assert!(!m, "middle space is valid, no meta needed"); + } + + #[test] + fn nfi_leading_space_slugified() { + let (f, m) = nfi(" Leading", Middleware::ModuleScript, &[]); + assert_eq!(f, "Leading.luau"); + assert!(m, "leading space stripped → slug differs"); + } + + #[test] + fn nfi_trailing_space_slugified() { + let (f, m) = nfi("Trailing ", Middleware::ModuleScript, &[]); + assert_eq!(f, "Trailing.luau"); + assert!(m); + } + + // ── Tilde in names ─────────────────────────────────────────────── + + #[test] + fn nfi_tilde_in_name_slugified() { + let (f, m) = nfi("Data~1", Middleware::ModuleScript, &[]); + assert_eq!(f, "Data_1.luau"); + assert!(m); + } + + #[test] + fn nfi_tilde_in_dir_name() { + let (f, m) = nfi("Cache~old", Middleware::Dir, &[]); + assert_eq!(f, "Cache_old"); + assert!(m); + } + + // ── OS-forbidden chars ─────────────────────────────────────────── + + #[test] + fn nfi_slash_in_name() { + let (f, m) = nfi("src/main", Middleware::ModuleScript, &[]); + assert_eq!(f, "src_main.luau"); + assert!(m); + } + + #[test] + fn nfi_colon_in_name() { + let (f, m) = nfi("Drive:C", Middleware::Dir, &[]); + assert_eq!(f, "Drive_C"); + assert!(m); + } + + #[test] + fn nfi_question_mark() { + let (f, m) = nfi("What?", Middleware::ServerScript, &[]); + assert_eq!(f, "What_.server.luau"); + assert!(m); + } + + #[test] + fn nfi_asterisk() { + let (f, m) = nfi("glob*", Middleware::ModuleScript, &[]); + assert_eq!(f, "glob_.luau"); + assert!(m); + } + + #[test] + fn nfi_angle_brackets() { + let (f, m) = nfi("", Middleware::ModuleScript, &[]); + assert_eq!(f, "_init_.luau"); + assert!(m); + } + + #[test] + fn nfi_pipe() { + let (f, m) = nfi("A|B", Middleware::Dir, &[]); + assert_eq!(f, "A_B"); + assert!(m); + } + + #[test] + fn nfi_backslash() { + let (f, m) = nfi("path\\to", Middleware::ModuleScript, &[]); + assert_eq!(f, "path_to.luau"); + assert!(m); + } + + #[test] + fn nfi_double_quote() { + let (f, m) = nfi("say\"hi\"", Middleware::ModuleScript, &[]); + assert_eq!(f, "say_hi_.luau"); + assert!(m); + } + + // ── Windows reserved names ─────────────────────────────────────── + + #[test] + fn nfi_con() { + let (f, m) = nfi("CON", Middleware::ModuleScript, &[]); + assert_eq!(f, "CON_.luau"); + assert!(m); + } + + #[test] + fn nfi_nul_dir() { + let (f, m) = nfi("NUL", Middleware::Dir, &[]); + assert_eq!(f, "NUL_"); + assert!(m); + } + + #[test] + fn nfi_com1_csv() { + let (f, m) = nfi("COM1", Middleware::Csv, &[]); + assert_eq!(f, "COM1_.csv"); + assert!(m); + } + + // ── Empty / fallback names ─────────────────────────────────────── + + #[test] + fn nfi_empty_name() { + let (f, m) = nfi("", Middleware::ModuleScript, &[]); + assert_eq!(f, "instance.luau"); + assert!(m); + } + + #[test] + fn nfi_all_forbidden() { + let (f, m) = nfi("<>:\"/\\|?*", Middleware::Dir, &[]); + assert_eq!(f, "instance"); + assert!(m); + } + + // ── Dedup with slugified names (collision scenarios) ───────────── + + // NOTE: deduplicate_name compares the bare slug against taken_names. + // In real syncback (dir.rs), taken_names stores full filenames, so file- + // level dedup only works for directory middleware where filename == slug. + // For file middleware, the duplicate-name pre-filter handles collisions. + // These tests use bare slugs in taken_names to verify the dedup logic. + + #[test] + fn nfi_slug_collides_with_existing_dir() { + // "Hey/Bro" slugifies to "Hey_Bro", which is already taken as a dir name + let (f, m) = nfi("Hey/Bro", Middleware::Dir, &["hey_bro"]); + assert_eq!(f, "Hey_Bro~1"); + assert!(m); + } + + #[test] + fn nfi_slug_collides_with_natural_dir() { + // Dir "Hey_Bro" exists naturally, then "Hey:Bro" slugifies to same + let (f, m) = nfi("Hey:Bro", Middleware::Dir, &["hey_bro"]); + assert_eq!(f, "Hey_Bro~1"); + assert!(m); + } + + #[test] + fn nfi_two_slugs_collide_dir() { + // Both "A/B" and "A:B" slugify to "A_B". First taken, second gets ~1 + let (f, m) = nfi("A:B", Middleware::Dir, &["a_b"]); + assert_eq!(f, "A_B~1"); + assert!(m); + } + + #[test] + fn nfi_three_way_collision_dir() { + let (f, _) = nfi("X/Y", Middleware::Dir, &["x_y", "x_y~1"]); + assert_eq!(f, "X_Y~2"); + } + + #[test] + fn nfi_clean_name_dedup_still_needs_meta_dir() { + // Clean name "Foo", but "foo" is already taken → dedup adds ~1 → needs meta + let (f, m) = nfi("Foo", Middleware::Dir, &["foo"]); + assert_eq!(f, "Foo~1"); + assert!( + m, + "dedup suffix means filesystem name differs from instance name" + ); + } + + #[test] + fn nfi_case_collision_dir() { + let (f, m) = nfi("Assets", Middleware::Dir, &["assets"]); + assert_eq!(f, "Assets~1"); + assert!(m); + } + + #[test] + fn nfi_slug_collision_file_middleware() { + // For file middleware, dedup compares bare slug against taken. + // Using bare names to verify the dedup logic works. + let (f, m) = nfi("Hey/Bro", Middleware::ModuleScript, &["hey_bro"]); + assert_eq!(f, "Hey_Bro~1.luau"); + assert!(m); + } + + // ── Ordering simulation: multiple siblings processed sequentially ─ + + /// Simulates processing N siblings in order, accumulating taken_names. + /// Uses Dir middleware for all to test dedup correctly (dir filenames + /// ARE the bare slug, matching what deduplicate_name compares against). + fn process_siblings_dir(siblings: &[&str]) -> Vec<(String, String, bool)> { + let mut taken: HashSet = HashSet::new(); + let mut results = Vec::new(); + for &name in siblings { + let dom = make_inst(name, "Folder"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let (filename, needs_meta) = + name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + taken.insert(filename.to_lowercase().to_string()); + results.push((name.to_string(), filename.into_owned(), needs_meta)); + } + results + } + + #[test] + fn ordering_two_identical_names() { + let r = process_siblings_dir(&["Foo", "Foo"]); + assert_eq!(r[0].1, "Foo"); + assert!(!r[0].2); + assert_eq!(r[1].1, "Foo~1"); + assert!(r[1].2); + } + + #[test] + fn ordering_three_identical_names() { + let r = process_siblings_dir(&["X", "X", "X"]); + assert_eq!(r[0].1, "X"); + assert_eq!(r[1].1, "X~1"); + assert_eq!(r[2].1, "X~2"); + } + + #[test] + fn ordering_natural_then_slugified_collision() { + // "Hey_Bro" goes first (natural), then "Hey/Bro" slugifies to same + let r = process_siblings_dir(&["Hey_Bro", "Hey/Bro"]); + assert_eq!(r[0].1, "Hey_Bro"); + assert!(!r[0].2); + assert_eq!(r[1].1, "Hey_Bro~1"); + assert!(r[1].2); + } + + #[test] + fn ordering_slugified_then_natural_collision() { + // "Hey/Bro" goes first (slugified to "Hey_Bro"), then natural "Hey_Bro" + let r = process_siblings_dir(&["Hey/Bro", "Hey_Bro"]); + assert_eq!(r[0].1, "Hey_Bro"); + assert!(r[0].2); + assert_eq!(r[1].1, "Hey_Bro~1"); + assert!(r[1].2, "dedup suffix → needs meta even for natural name"); + } + + #[test] + fn ordering_case_variants() { + let r = process_siblings_dir(&["Script", "script", "SCRIPT"]); + assert_eq!(r[0].1, "Script"); + assert!(!r[0].2); + assert_eq!(r[1].1, "script~1"); + assert!(r[1].2); + assert_eq!(r[2].1, "SCRIPT~2"); + assert!(r[2].2); + } + + #[test] + fn ordering_mixed_forbidden_chars_same_slug() { + // All three slugify to "A_B" + let r = process_siblings_dir(&["A/B", "A:B", "A*B"]); + assert_eq!(r[0].1, "A_B"); + assert_eq!(r[1].1, "A_B~1"); + assert_eq!(r[2].1, "A_B~2"); + assert!(r[0].2); + assert!(r[1].2); + assert!(r[2].2); + } + + #[test] + fn ordering_dangerous_suffix_then_safe() { + // "test.server" slugifies to "test_server", "test" is clean + let r = process_siblings_dir(&["test.server", "test"]); + assert_eq!(r[0].1, "test_server"); + assert!(r[0].2); + assert_eq!(r[1].1, "test"); + assert!(!r[1].2); + } + + #[test] + fn ordering_windows_reserved_then_normal() { + // "CON" slugifies to "CON_", then natural "CON_" collides + let r = process_siblings_dir(&["CON", "CON_"]); + assert_eq!(r[0].1, "CON_"); + assert!(r[0].2); + assert_eq!(r[1].1, "CON_~1"); + assert!(r[1].2); + } + + #[test] + fn ordering_five_siblings_complex() { + let r = process_siblings_dir(&[ + "Utils", // first claim + "utils", // case collision with Utils + "Utils", // exact collision (already taken + ~1 taken) + "UTILS", // case collision + "Uti/ls", // different slug "Uti_ls", no collision + ]); + assert_eq!(r[0].1, "Utils"); + assert!(!r[0].2); + assert_eq!(r[1].1, "utils~1"); + assert!(r[1].2); + assert_eq!(r[2].1, "Utils~2"); + assert!(r[2].2); + assert_eq!(r[3].1, "UTILS~3"); + assert!(r[3].2); + assert_eq!(r[4].1, "Uti_ls"); + assert!(r[4].2); + } + + #[test] + fn ordering_six_way_slug_pileup() { + // All six slugify to "X_Y" as dirs + let r = process_siblings_dir(&["X/Y", "X:Y", "X*Y", "X?Y", "XY"]); + assert_eq!(r[0].1, "X_Y"); + assert_eq!(r[1].1, "X_Y~1"); + assert_eq!(r[2].1, "X_Y~2"); + assert_eq!(r[3].1, "X_Y~3"); + assert_eq!(r[4].1, "X_Y~4"); + assert_eq!(r[5].1, "X_Y~5"); + } + + #[test] + fn ordering_tilde_in_name_vs_dedup_suffix() { + // "Foo~1" (tilde slugified to "Foo_1") vs dedup suffix "Foo~1" + // The slugified "Foo~1" → "Foo_1" which is different from dedup "Foo~1" + let r = process_siblings_dir(&["Foo", "Foo", "Foo~1"]); + assert_eq!(r[0].1, "Foo"); + assert_eq!(r[1].1, "Foo~1"); // dedup suffix + assert_eq!(r[2].1, "Foo_1"); // tilde was slugified to _ + assert!(r[2].2); + } + + #[test] + fn ordering_spaces_and_collisions() { + let r = process_siblings_dir(&[ + " Leading", // leading space stripped → "Leading" + "Leading", // collides with stripped version + "Leading ", // trailing space stripped → "Leading" → collides + ]); + assert_eq!(r[0].1, "Leading"); + assert!(r[0].2, "leading space was stripped"); + assert_eq!(r[1].1, "Leading~1"); + assert!(r[1].2); + assert_eq!(r[2].1, "Leading~2"); + assert!(r[2].2); + } + + // ── Every middleware type with slugified name ───────────────────── + + #[test] + fn nfi_every_middleware_with_slash() { + let cases: &[(&str, Middleware, &str)] = &[ + ("a/b", Middleware::ModuleScript, "a_b.luau"), + ("a/b", Middleware::ServerScript, "a_b.server.luau"), + ("a/b", Middleware::ClientScript, "a_b.client.luau"), + ("a/b", Middleware::PluginScript, "a_b.plugin.luau"), + ("a/b", Middleware::LocalScript, "a_b.local.luau"), + ("a/b", Middleware::LegacyScript, "a_b.legacy.luau"), + ("a/b", Middleware::Csv, "a_b.csv"), + ("a/b", Middleware::JsonModel, "a_b.model.json5"), + ("a/b", Middleware::Text, "a_b.txt"), + ("a/b", Middleware::Rbxm, "a_b.rbxm"), + ("a/b", Middleware::Rbxmx, "a_b.rbxmx"), + ("a/b", Middleware::Toml, "a_b.toml"), + ("a/b", Middleware::Yaml, "a_b.yml"), + ("a/b", Middleware::Dir, "a_b"), + ("a/b", Middleware::ServerScriptDir, "a_b"), + ("a/b", Middleware::ClientScriptDir, "a_b"), + ("a/b", Middleware::ModuleScriptDir, "a_b"), + ]; + for &(name, mw, expected) in cases { + let (f, m) = nfi(name, mw, &[]); + assert_eq!(f, expected, "name={name:?} mw={mw:?}"); + assert!(m, "name={name:?} mw={mw:?} should need meta"); + } + } + + // ── Stress: validate_file_name accepts every slugify output ────── + + #[test] + fn slugify_output_always_valid_filesystem_name() { + let long_name = "A".repeat(255); + let corpus = [ + "", + " ", + " ", + ".", + "..", + "...", + ". .", + " . ", + "/", + "\\", + "*", + "?", + "<", + ">", + "|", + ":", + "\"", + "~", + "~1", + "~99", + "CON", + "PRN", + "AUX", + "NUL", + "COM1", + "COM9", + "LPT1", + "LPT9", + "con", + "prn", + "aux", + "nul", + "com1", + "foo.server", + "foo.client", + "foo.plugin", + "foo.local", + "foo.legacy", + "foo.meta", + "foo.model", + "foo.project", + "foo.Server", + "foo.CLIENT", + "foo.Meta", + "a.meta.server", + "x.model.project.client", + "hello world", + " hello", + "hello ", + " hello ", + "日本語", + "café", + "émoji🎮", + "v1.0", + "foo.bar", + "a.b.c.d", + "<>:\"/\\|?*~. ", + long_name.as_str(), + "\x00", + "\t", + "\n", + "\r", + ]; + for input in corpus { + let slug = slugify_name(input); + assert!( + validate_file_name(&slug).is_ok(), + "slugify_name({input:?}) = {slug:?} failed validate_file_name" + ); + assert!( + !slug.is_empty(), + "slugify_name({input:?}) should never be empty" + ); + } + } + + #[test] + fn name_needs_slugify_implies_slug_differs() { + // If name_needs_slugify returns true, slugify_name MUST return + // something different from the input (otherwise we'd set needs_meta + // but the name and slug would be the same — pointless meta field). + let corpus = [ + "CON", + "a/b", + "foo~1", + "foo.server", + " leading", + "trailing ", + "hello.", + "", + "\t", + "COM1", + ]; + for input in corpus { + if name_needs_slugify(input) { + let slug = slugify_name(input); + assert_ne!( + slug, input, + "name_needs_slugify({input:?})=true but slugify produced identical output" + ); + } + } + } + + #[test] + fn name_needs_slugify_false_means_slug_is_identity() { + // If name_needs_slugify returns false, slugify_name should return + // the input unchanged (no unnecessary transformation). + let corpus = [ + "Hello", + "my_module", + "test-123", + "v1.0", + "foo.bar", + "hello world", + "日本語", + "café", + ]; + for input in corpus { + assert!( + !name_needs_slugify(input), + "{input:?} should not need slugify" + ); + assert_eq!( + slugify_name(input), + input, + "slugify_name should be identity for {input:?}" + ); + } + } + + // ══════════════════════════════════════════════════════════════════ + // DISK-SEEDED TAKEN NAMES TESTS + // + // These simulate the fix where taken_names is pre-seeded from + // existing directory contents. Before the fix, taken_names was + // empty and new instances could overwrite existing files. + // + // Every test in this section would have FAILED before the fix. + // ══════════════════════════════════════════════════════════════════ + + /// Simulates dir.rs behavior AFTER fix #3: seed taken_names from + /// "on-disk" entries, then process new children sequentially. + fn process_new_children_with_disk_seed( + disk_entries: &[&str], + new_children: &[(&str, Middleware)], + ) -> Vec<(String, String, bool)> { + // Seed from "disk" (lowercased, as the fix does) + 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) = + name_for_inst(mw, child, None, &taken).unwrap(); + taken.insert(filename.to_lowercase().to_string()); + results.push((name.to_string(), filename.into_owned(), needs_meta)); + } + results + } + + #[test] + 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)], + ); + assert_eq!(r[0].1, "Utils~1"); + assert!(r[0].2); + } + + #[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)], + ); + assert_eq!(r[0].1, "Scripts~1"); + assert!(r[0].2); + } + + #[test] + 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)], + ); + assert_eq!(r[0].1, "Hey_Bro~1"); + assert!(r[0].2); + } + + #[test] + fn disk_seed_multiple_existing_dirs() { + // Directory has: Utils/, Config/, Shared/ + // New dir children: Utils (collision!), Config (collision!), NewThing (clean) + let r = process_new_children_with_disk_seed( + &["utils", "config", "shared"], + &[ + ("Utils", Middleware::Dir), + ("Config", Middleware::Dir), + ("NewThing", Middleware::Dir), + ], + ); + assert_eq!(r[0].1, "Utils~1", "Utils collides with utils/ on disk"); + assert!(r[0].2); + assert_eq!(r[1].1, "Config~1", "Config collides with config/ on disk"); + assert!(r[1].2); + assert_eq!(r[2].1, "NewThing", "NewThing is clean"); + assert!(!r[2].2); + } + + #[test] + fn disk_seed_existing_dir_plus_meta_and_files() { + // Directory contains: Stuff/ (dir), Stuff.meta.json5, Other.luau + // New "Stuff" dir must dedup against "stuff" (the dir). + let r = process_new_children_with_disk_seed( + &["stuff", "stuff.meta.json5", "other.luau"], + &[("Stuff", Middleware::Dir)], + ); + assert_eq!(r[0].1, "Stuff~1"); + assert!(r[0].2); + } + + #[test] + fn disk_seed_file_middleware_dedup_compares_bare_slugs() { + // IMPORTANT: For file middleware, deduplicate_name compares the + // bare slug against taken_names. Disk entries with extensions + // (e.g., "mymodule.luau") do NOT collide with the bare slug + // "MyModule" because "mymodule" ≠ "mymodule.luau". + // + // This is by design — duplicate-named children are already skipped + // by dir.rs's duplicate detection before reaching name_for_inst. + // File middleware dedup only catches slug collisions between + // DIFFERENT names (e.g., "A/B" and "A:B" both slugify to "A_B"). + let r = process_new_children_with_disk_seed( + &["mymodule.luau"], + &[("MyModule", Middleware::ModuleScript)], + ); + // Bare slug "mymodule" ≠ "mymodule.luau" in taken_names. + // No collision detected — this is the expected behavior. + assert_eq!(r[0].1, "MyModule.luau"); + assert!(!r[0].2); + } + + #[test] + fn disk_seed_file_middleware_sibling_slug_collision_works() { + // Two file-middleware children whose names slugify to the same base. + // The FIRST child's full filename gets added to taken_names, + // and the second child's bare slug doesn't match that filename. + // However, bare slugs DO collide with each other through accumulation: + // First: slug "A_B" → not in taken → filename "A_B.luau" → taken gets "a_b.luau" + // Second: slug "A_B" → "a_b" not in taken (has "a_b.luau") → filename "A_B.luau" + // This demonstrates that file-middleware sibling dedup relies on the + // duplicate-name pre-filter in dir.rs (which skips same-named children). + let r = process_new_children_with_disk_seed( + &[], + &[("A/B", Middleware::ModuleScript), ("A:B", Middleware::ModuleScript)], + ); + // Both get the same filename because bare slug dedup doesn't + // catch full-filename entries. This is acceptable because in real + // dir.rs, children with different instance names but same slug + // would need the full duplicate-name resolution system (Phase 2). + assert_eq!(r[0].1, "A_B.luau"); + assert_eq!(r[1].1, "A_B.luau"); + } + + #[test] + fn disk_seed_gitkeep_doesnt_interfere() { + // .gitkeep files are common in empty dirs. New "gitkeep" instance + // shouldn't collide (hidden files start with dot, regular names don't). + // But ".gitkeep" lowered is ".gitkeep" — if we have a bizarre instance + // named ".gitkeep", it won't be slugified (no forbidden chars) but would + // collide. Testing that the seeding doesn't break normal flow. + let r = process_new_children_with_disk_seed( + &[".gitkeep"], + &[("MyModule", Middleware::ModuleScript)], + ); + assert_eq!(r[0].1, "MyModule.luau"); + assert!(!r[0].2, "no collision with .gitkeep"); + } + + #[test] + fn disk_seed_init_file_doesnt_prevent_new_siblings() { + // Parent dir has "init.luau". New children should work fine. + let r = process_new_children_with_disk_seed( + &["init.luau"], + &[ + ("Child1", Middleware::ModuleScript), + ("Child2", Middleware::ServerScript), + ], + ); + assert_eq!(r[0].1, "Child1.luau"); + assert!(!r[0].2); + assert_eq!(r[1].1, "Child2.server.luau"); + assert!(!r[1].2); + } + + #[test] + 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)], + ); + assert_eq!(r[0].1, "Foo~2"); + assert!(r[0].2); + } + + #[test] + fn disk_seed_dedup_chain_on_disk() { + // "A_B", "A_B~1", "A_B~2" already on disk. New "A/B" (slug "A_B") + // must land on "A_B~3". + let r = process_new_children_with_disk_seed( + &["a_b", "a_b~1", "a_b~2"], + &[("A/B", Middleware::Dir)], + ); + assert_eq!(r[0].1, "A_B~3"); + assert!(r[0].2); + } + + // ══════════════════════════════════════════════════════════════════ + // SIBLING DEDUP TESTS (project.rs fix #2 simulation) + // + // These simulate the fix where siblings under the same parent + // share a single taken_names set. Before the fix, each sibling + // got an independent empty HashSet and could collide. + // + // Every test in this section would have FAILED before the fix. + // ══════════════════════════════════════════════════════════════════ + + /// Simulates project.rs behavior AFTER fix #2: shared taken_names + /// across siblings, seeded from disk, accumulated per-child. + fn process_project_siblings( + 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 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) = + name_for_inst(mw, child, None, &taken).unwrap(); + taken.insert(filename.to_lowercase().to_string()); + results.push((name.to_string(), filename.into_owned(), needs_meta)); + } + results + } + + #[test] + fn project_siblings_same_slug_different_names() { + // 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)], + ); + assert_eq!(r[0].1, "A_B"); + assert!(r[0].2); + assert_eq!(r[1].1, "A_B~1"); + assert!(r[1].2); + } + + #[test] + fn project_siblings_natural_vs_slug_collision_dir() { + // "Hey_Bro" (natural) and "Hey/Bro" (slug "Hey_Bro") as dir children. + // Dir middleware: bare slug IS the filename → dedup works. + let r = process_project_siblings( + &[], + &[ + ("Hey_Bro", Middleware::Dir), + ("Hey/Bro", Middleware::Dir), + ], + ); + assert_eq!(r[0].1, "Hey_Bro"); + assert!(!r[0].2); + assert_eq!(r[1].1, "Hey_Bro~1"); + assert!(r[1].2); + } + + #[test] + fn project_siblings_three_way_slug_collision() { + // Three siblings all slug to "X_Y" — must get X_Y, X_Y~1, X_Y~2. + let r = process_project_siblings( + &[], + &[ + ("X/Y", Middleware::Dir), + ("X:Y", Middleware::Dir), + ("X*Y", Middleware::Dir), + ], + ); + assert_eq!(r[0].1, "X_Y"); + assert_eq!(r[1].1, "X_Y~1"); + assert_eq!(r[2].1, "X_Y~2"); + } + + #[test] + fn project_siblings_disk_plus_siblings_collision_dir() { + // "Helper" directory already on disk. Two new dir siblings: "Helper" x2. + // First → "Helper~1" (disk has "helper"). + // Second → "Helper~2" (both disk and first sibling taken). + let r = process_project_siblings( + &["helper"], + &[ + ("Helper", Middleware::Dir), + ("Helper", Middleware::Dir), + ], + ); + assert_eq!(r[0].1, "Helper~1"); + assert!(r[0].2); + assert_eq!(r[1].1, "Helper~2"); + assert!(r[1].2); + } + + #[test] + fn project_siblings_case_collision_chain() { + // Case variants: "Test", "test", "TEST", "tEsT" — all collide case-insensitively. + let r = process_project_siblings( + &[], + &[ + ("Test", Middleware::Dir), + ("test", Middleware::Dir), + ("TEST", Middleware::Dir), + ("tEsT", Middleware::Dir), + ], + ); + assert_eq!(r[0].1, "Test"); + assert!(!r[0].2); + assert_eq!(r[1].1, "test~1"); + assert!(r[1].2); + assert_eq!(r[2].1, "TEST~2"); + assert!(r[2].2); + assert_eq!(r[3].1, "tEsT~3"); + assert!(r[3].2); + } + + // ══════════════════════════════════════════════════════════════════ + // NIGHTMARE EDGE CASES + // + // Tests that combine multiple transformation layers in ways that + // should make your eyes water. Each one targets a specific + // interaction between slugify, dedup, dangerous suffixes, case + // folding, Windows reserved names, and unicode. + // ══════════════════════════════════════════════════════════════════ + + #[test] + fn nightmare_con_slash_prn_sibling_collision() { + // "CON" → "CON_", "PRN" → "PRN_" — different slugs, no collision. + // But "CON/" → "CON_" collides with "CON" → "CON_". + let r = process_siblings_dir(&["CON", "CON/"]); + assert_eq!(r[0].1, "CON_"); + assert!(r[0].2); + assert_eq!(r[1].1, "CON_~1"); + assert!(r[1].2); + } + + #[test] + fn nightmare_dangerous_suffix_cascade_collision() { + // "foo.server" → "foo_server", "foo/server" → "foo_server" — collision! + let r = process_siblings_dir(&["foo.server", "foo/server"]); + assert_eq!(r[0].1, "foo_server"); + assert!(r[0].2); + assert_eq!(r[1].1, "foo_server~1"); + assert!(r[1].2); + } + + #[test] + fn nightmare_tilde_looks_like_dedup_but_gets_slugified() { + // "Foo~1" has tilde slugified to "Foo_1". Then real "Foo" gets + // dedup "Foo~1". These should NOT collide because tilde-in-name + // becomes underscore, but dedup adds actual tilde. + let r = process_siblings_dir(&["Foo~1", "Foo", "Foo"]); + assert_eq!(r[0].1, "Foo_1", "tilde slugified"); + assert_eq!(r[1].1, "Foo", "clean, no collision"); + assert_eq!(r[2].1, "Foo~1", "dedup suffix, different from Foo_1"); + } + + #[test] + fn nightmare_dedup_suffix_avoids_slugified_tilde_name() { + // Process "Foo", then "Foo" again (dedup to "Foo~1"), then "Foo~1" + // (tilde slugified to "Foo_1"). The dedup "Foo~1" and slug "Foo_1" + // should coexist. + let r = process_siblings_dir(&["Foo", "Foo", "Foo~1"]); + assert_eq!(r[0].1, "Foo"); + assert_eq!(r[1].1, "Foo~1"); + assert_eq!(r[2].1, "Foo_1"); + // All three have distinct lowercased names + let names: HashSet = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); + assert_eq!(names.len(), 3, "all three filenames must be distinct"); + } + + #[test] + fn nightmare_unicode_plus_forbidden_chars() { + // "カフェ/Bar" → "カフェ_Bar", "カフェ:Bar" → "カフェ_Bar" — collision + let r = process_siblings_dir(&["カフェ/Bar", "カフェ:Bar"]); + assert_eq!(r[0].1, "カフェ_Bar"); + assert_eq!(r[1].1, "カフェ_Bar~1"); + } + + #[test] + fn nightmare_emoji_name_preserved() { + // Emoji names are not forbidden chars — they pass through. + let r = process_siblings_dir(&["🎮 Games", "🎮 Games"]); + assert_eq!(r[0].1, "🎮 Games"); + assert!(!r[0].2, "emoji name is clean"); + assert_eq!(r[1].1, "🎮 Games~1"); + assert!(r[1].2); + } + + #[test] + fn nightmare_windows_reserved_then_slug_that_matches() { + // "CON" → "CON_", then natural "CON_" collides. + // Then "con" → "con_" collides case-insensitively with "CON_". + let r = process_siblings_dir(&["CON", "CON_", "con"]); + assert_eq!(r[0].1, "CON_"); + assert!(r[0].2); + assert_eq!(r[1].1, "CON_~1"); + assert!(r[1].2, "natural CON_ collides with reserved CON's slug"); + assert_eq!(r[2].1, "con_~2"); + assert!(r[2].2); + } + + #[test] + fn nightmare_trailing_dot_chain() { + // "test." → strip trailing dot → "test", "test" → "test" collision! + let r = process_siblings_dir(&["test.", "test"]); + assert_eq!(r[0].1, "test"); + assert!(r[0].2, "trailing dot stripped → slug differs"); + assert_eq!(r[1].1, "test~1"); + assert!(r[1].2); + } + + #[test] + fn nightmare_leading_space_chain() { + // " A" → "A", "A" → "A" collision, " A" → "A" triple collision + let r = process_siblings_dir(&[" A", "A", " A"]); + assert_eq!(r[0].1, "A"); + assert!(r[0].2, "leading space stripped"); + assert_eq!(r[1].1, "A~1"); + assert!(r[1].2); + assert_eq!(r[2].1, "A~2"); + assert!(r[2].2); + } + + #[test] + fn nightmare_all_reserved_names_siblings() { + // 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", + ]; + let r = process_siblings_dir(&reserved); + let names: HashSet = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); + assert_eq!( + names.len(), + reserved.len(), + "every reserved name should produce a unique slug" + ); + for (_, filename, needs_meta) in &r { + assert!(needs_meta, "{filename} should need meta"); + assert!( + validate_file_name(filename).is_ok(), + "{filename} should be valid" + ); + } + } + + #[test] + fn nightmare_dangerous_suffix_every_variant() { + // All 8 dangerous suffixes as sibling names, each unique slug. + let siblings: Vec = DANGEROUS_SUFFIXES + .iter() + .map(|s| format!("foo{s}")) + .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(), + DANGEROUS_SUFFIXES.len(), + "each dangerous suffix variant should produce a unique filename" + ); + } + + #[test] + fn nightmare_mixed_middleware_dir_then_file() { + // "Shared" as Dir (filename "Shared") then ModuleScript. + // Dir filename "Shared" → taken gets "shared". + // ModuleScript dedup checks "shared" → MATCH → dedup to "Shared~1". + // Then final filename becomes "Shared~1.luau". + let r = process_new_children_with_disk_seed( + &[], + &[ + ("Shared", Middleware::Dir), + ("Shared", Middleware::ModuleScript), + ], + ); + assert_eq!(r[0].1, "Shared"); + assert!(!r[0].2); + // File middleware bare slug "Shared" matches dir entry "shared" in taken. + assert_eq!(r[1].1, "Shared~1.luau"); + assert!(r[1].2); + } + + #[test] + fn nightmare_mixed_middleware_file_then_dir() { + // Reverse order: ModuleScript first, Dir second. + // ModuleScript "Shared" → filename "Shared.luau" → taken gets "shared.luau" + // Dir "Shared" → slug "Shared" → dedup checks "shared" → NOT in taken + // ("shared" ≠ "shared.luau") → filename "Shared" → taken gets "shared" + let r = process_new_children_with_disk_seed( + &[], + &[ + ("Shared", Middleware::ModuleScript), + ("Shared", Middleware::Dir), + ], + ); + assert_eq!(r[0].1, "Shared.luau"); + assert!(!r[0].2); + // Dir slug "Shared" doesn't collide with "shared.luau" in taken + assert_eq!(r[1].1, "Shared"); + assert!(!r[1].2); + } + + #[test] + fn nightmare_10_way_slug_pileup_with_disk_seed() { + // "A_B" already on disk. Then 9 new instances all slugifying to "A_B". + let new_children: Vec<(&str, Middleware)> = vec![ + ("A/B", Middleware::Dir), + ("A:B", Middleware::Dir), + ("A*B", Middleware::Dir), + ("A?B", Middleware::Dir), + ("AB", Middleware::Dir), + ("A|B", Middleware::Dir), + ("A\\B", Middleware::Dir), + ("A\"B", Middleware::Dir), + ]; + let r = process_new_children_with_disk_seed(&["a_b"], &new_children); + for (i, (_, filename, needs_meta)) in r.iter().enumerate() { + let expected = if i == 0 { + "A_B~1".to_string() + } else { + format!("A_B~{}", i + 1) + }; + assert_eq!(filename, &expected, "child {i}"); + assert!(needs_meta, "child {i} needs meta"); + } + } + + #[test] + fn nightmare_empty_name_siblings() { + // Multiple instances with empty names. All slugify to "instance". + let r = process_siblings_dir(&["", "", ""]); + assert_eq!(r[0].1, "instance"); + assert_eq!(r[1].1, "instance~1"); + assert_eq!(r[2].1, "instance~2"); + } + + #[test] + fn nightmare_all_forbidden_chars_name_siblings() { + // Names that are entirely forbidden chars → all become "instance". + let r = process_siblings_dir(&["<>", ":/", "?*", "|\\", "\"~"]); + assert_eq!(r[0].1, "instance"); + assert_eq!(r[1].1, "instance~1"); + assert_eq!(r[2].1, "instance~2"); + assert_eq!(r[3].1, "instance~3"); + assert_eq!(r[4].1, "instance~4"); + } + + #[test] + fn nightmare_control_chars_with_normal_text() { + // "Hello\x00World" → "Hello_World", "Hello/World" → "Hello_World" + let r = process_siblings_dir(&["Hello\x00World", "Hello/World"]); + assert_eq!(r[0].1, "Hello_World"); + assert_eq!(r[1].1, "Hello_World~1"); + } + + #[test] + fn nightmare_long_dedup_chain_gap_filling() { + // "Foo", "Foo~1", "Foo~3" on disk (gap at ~2). + // New "Foo" should fill the gap and get "Foo~2". + let r = process_new_children_with_disk_seed( + &["foo", "foo~1", "foo~3"], + &[("Foo", Middleware::Dir)], + ); + assert_eq!(r[0].1, "Foo~2", "should fill gap at ~2"); + } + + #[test] + fn nightmare_meta_suffix_collision_with_dangerous() { + // "init.meta" → dangerous suffix ".meta" → "init_meta" + // "init/meta" → slug "init_meta" → collision! + let r = process_siblings_dir(&["init.meta", "init/meta"]); + assert_eq!(r[0].1, "init_meta"); + assert_eq!(r[1].1, "init_meta~1"); + } + + #[test] + fn nightmare_project_suffix_collision_chain() { + // "a.project" → "a_project", "a/project" → "a_project", "a_project" (natural) + let r = process_siblings_dir(&["a.project", "a/project", "a_project"]); + assert_eq!(r[0].1, "a_project"); + assert!(r[0].2); + assert_eq!(r[1].1, "a_project~1"); + assert!(r[1].2); + assert_eq!(r[2].1, "a_project~2"); + assert!(r[2].2, "natural name collides with slugified siblings"); + } + + #[test] + fn nightmare_100_identical_names() { + // 100 instances all named "Script" — dedup must handle gracefully. + let names: Vec<&str> = vec!["Script"; 100]; + 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); + } + // All filenames must be unique + let unique: HashSet = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); + assert_eq!(unique.len(), 100); + } + + #[test] + fn nightmare_mixed_clean_and_slug_interleaved() { + // 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" + ]); + assert_eq!(r[0].1, "A_B"); + assert!(!r[0].2); + assert_eq!(r[1].1, "C_D"); + assert!(!r[1].2); + assert_eq!(r[2].1, "A_B~1"); + assert!(r[2].2); + assert_eq!(r[3].1, "E_F"); + assert!(!r[3].2); + assert_eq!(r[4].1, "C_D~1"); + assert!(r[4].2); + assert_eq!(r[5].1, "A_B~2"); + assert!(r[5].2); + } + + // ══════════════════════════════════════════════════════════════════ + // IDEMPOTENCY AND INVARIANT TESTS + // + // These verify structural invariants that must always hold. + // ══════════════════════════════════════════════════════════════════ + + #[test] + fn invariant_no_two_siblings_share_filesystem_name() { + // 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_", + "日本語", "日本語", + ]; + let r = process_siblings_dir(&names); + let lowered: Vec = r.iter().map(|(_, f, _)| f.to_lowercase()).collect(); + let unique: HashSet<&String> = lowered.iter().collect(); + assert_eq!( + unique.len(), + lowered.len(), + "every sibling must have a unique filename. Duplicates found in: {:?}", + lowered + ); + } + + #[test] + 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", + ]; + let r = process_siblings_dir(&names); + for (original, filename, _) in &r { + assert!( + validate_file_name(filename).is_ok(), + "filename {filename:?} (from {original:?}) must be valid" + ); + } + } + + #[test] + fn invariant_needs_meta_iff_name_differs_from_slug() { + // 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.", + ]; + for &name in &cases { + let dom = make_inst(name, "Folder"); + let child_ref = dom.root().children()[0]; + let child = dom.get_by_ref(child_ref).unwrap(); + let taken = HashSet::new(); + let (filename, needs_meta) = + name_for_inst(Middleware::Dir, child, None, &taken).unwrap(); + if needs_meta { + assert_ne!( + filename.as_ref(), name, + "needs_meta=true but filename matches name for {name:?}" + ); + } else { + assert_eq!( + filename.as_ref(), name, + "needs_meta=false but filename differs from name for {name:?}" + ); + } + } + } +} diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 321189b0..27651213 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -29,7 +29,10 @@ use crate::{ Project, }; -pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name}; +pub use file_names::{ + deduplicate_name, extension_for_middleware, name_for_inst, name_needs_slugify, slugify_name, + validate_file_name, +}; pub use fs_snapshot::FsSnapshot; pub use hash::*; pub use property_filter::{ @@ -996,12 +999,6 @@ pub struct SyncbackRules { /// generally a better UX. #[serde(skip_serializing_if = "Option::is_none")] create_ignore_dir_paths: Option, - /// Whether to encode Windows-invalid characters in file names during - /// syncback. Characters like `<`, `>`, `:`, `"`, `/`, `\`, `|`, `?`, `*` - /// will be encoded as `%LT%`, `%GT%`, `%COLON%`, etc. - /// Defaults to `true`. - #[serde(skip_serializing_if = "Option::is_none")] - encode_windows_invalid_chars: Option, /// When enabled, only "visible" services will be synced back. This includes /// commonly used services like Workspace, ReplicatedStorage, ServerScriptService, /// etc., while ignoring internal/hidden services like Chat, HttpService, etc. @@ -1054,13 +1051,6 @@ impl SyncbackRules { Ok(globs) } - /// Returns whether special characters (including periods) should be encoded - /// in file names during syncback. Defaults to `true`. - #[inline] - pub fn encode_windows_invalid_chars(&self) -> bool { - self.encode_windows_invalid_chars.unwrap_or(true) - } - /// Returns whether hidden/internal services should be ignored during /// syncback. When `true`, only visible services like Workspace, /// ReplicatedStorage, etc. will be synced. Defaults to `true`. diff --git a/src/syncback/snapshot.rs b/src/syncback/snapshot.rs index 954630a9..3c7179d6 100644 --- a/src/syncback/snapshot.rs +++ b/src/syncback/snapshot.rs @@ -1,5 +1,6 @@ use indexmap::IndexMap; use memofs::Vfs; +use std::collections::HashSet; use std::path::{Path, PathBuf}; use crate::{ @@ -56,7 +57,12 @@ impl<'sync> SyncbackSnapshot<'sync> { /// In clean mode (incremental=false), old_ref is ignored and set to None, /// ensuring fresh filenames and middleware choices for all children. #[inline] - pub fn with_joined_path(&self, new_ref: Ref, old_ref: Option) -> anyhow::Result { + pub fn with_joined_path( + &self, + new_ref: Ref, + old_ref: Option, + taken_names: &HashSet, + ) -> anyhow::Result<(Self, bool)> { // In clean mode, ignore old_ref to ensure fresh structure let effective_old_ref = if self.data.incremental { old_ref } else { None }; let mut snapshot = Self { @@ -67,16 +73,15 @@ impl<'sync> SyncbackSnapshot<'sync> { middleware: None, }; let middleware = get_best_middleware(&snapshot); - let encode_invalid_chars = self.encode_windows_invalid_chars(); - let name = name_for_inst( + let (name, needs_meta_name) = name_for_inst( middleware, snapshot.new_inst(), snapshot.old_inst(), - encode_invalid_chars, + taken_names, )?; - snapshot.path = self.path.join(name.as_ref()); + snapshot.path = self.path.join(&*name); - Ok(snapshot) + Ok((snapshot, needs_meta_name)) } /// Constructs a SyncbackSnapshot from the provided refs and a base path, @@ -93,7 +98,8 @@ impl<'sync> SyncbackSnapshot<'sync> { base_path: &Path, new_ref: Ref, old_ref: Option, - ) -> anyhow::Result { + taken_names: &HashSet, + ) -> anyhow::Result<(Self, bool)> { // In clean mode, ignore old_ref to ensure fresh structure let effective_old_ref = if self.data.incremental { old_ref } else { None }; let mut snapshot = Self { @@ -104,16 +110,15 @@ impl<'sync> SyncbackSnapshot<'sync> { middleware: None, }; let middleware = get_best_middleware(&snapshot); - let encode_invalid_chars = self.encode_windows_invalid_chars(); - let name = name_for_inst( + let (name, needs_meta_name) = name_for_inst( middleware, snapshot.new_inst(), snapshot.old_inst(), - encode_invalid_chars, + taken_names, )?; - snapshot.path = base_path.join(name.as_ref()); + snapshot.path = base_path.join(&*name); - Ok(snapshot) + Ok((snapshot, needs_meta_name)) } /// Constructs a SyncbackSnapshot with the provided path and refs while @@ -319,18 +324,6 @@ impl<'sync> SyncbackSnapshot<'sync> { .unwrap_or_default() } - /// Returns whether Windows-invalid characters should be encoded in file - /// names during syncback. Defaults to `true`. - #[inline] - pub fn encode_windows_invalid_chars(&self) -> bool { - self.data - .project - .syncback_rules - .as_ref() - .map(|rules| rules.encode_windows_invalid_chars()) - .unwrap_or(true) - } - /// Returns whether to emit warnings when duplicate child names are /// encountered during syncback. Defaults to `false` (suppressed). #[inline] diff --git a/src/web/api.rs b/src/web/api.rs index b0d772f8..c96d6e86 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -21,10 +21,9 @@ use rbx_dom_weak::{ }; use crate::{ - path_encoding::encode_path_name, serve_session::ServeSession, snapshot::{InstanceWithMeta, InstigatingSource, PatchSet, PatchUpdate}, - syncback::VISIBLE_SERVICES, + syncback::{slugify_name, VISIBLE_SERVICES}, web::{ interface::{ ErrorResponse, Instance, InstanceMetadata, MessagesPacket, OpenResponse, ReadResponse, @@ -464,10 +463,115 @@ impl ApiService { // This makes all subsequent path checks O(d) instead of O(d × s) let duplicate_siblings_cache = Self::compute_tree_refs_with_duplicate_siblings(&tree); + // Pre-scan: identify standalone parents that need directory conversion. + // Multiple children may share the same parent. We convert each parent + // ONCE before processing any children, avoiding double-conversion that + // would wipe the parent's source (the old standalone file is deleted by + // the first conversion, so the second would read empty). + let mut converted_parents: HashMap = HashMap::new(); for added in request.added.values() { - if let Err(err) = - self.syncback_added_instance(added, &tree, &duplicate_siblings_cache, &stats) - { + if let Some(parent_ref) = added.parent { + if converted_parents.contains_key(&parent_ref) { + continue; + } + if let Some(parent_inst) = tree.get_instance(parent_ref) { + if let Some(source) = &parent_inst.metadata().instigating_source { + // Resolve the actual filesystem path for the parent. + // For ProjectNode sources, use the resolved $path (not + // the project file path). For init-file sources, use + // the parent directory (since init files represent + // directory-format instances that are already dirs). + let resolved_path: std::borrow::Cow<'_, Path> = match source { + crate::snapshot::InstigatingSource::Path(p) => { + let file_name = + p.file_name().and_then(|f| f.to_str()).unwrap_or(""); + if file_name.starts_with("init.") { + std::borrow::Cow::Borrowed( + p.parent().unwrap_or(p.as_path()), + ) + } else { + std::borrow::Cow::Borrowed(p.as_path()) + } + } + crate::snapshot::InstigatingSource::ProjectNode { + path: project_path, + node, + .. + } => { + if let Some(path_node) = &node.path { + let fs_path = path_node.path(); + let resolved = if fs_path.is_relative() { + project_path + .parent() + .unwrap_or(project_path.as_path()) + .join(fs_path) + } else { + fs_path.to_path_buf() + }; + std::borrow::Cow::Owned(resolved) + } else { + std::borrow::Cow::Borrowed(source.path()) + } + } + }; + if !resolved_path.is_dir() { + let parent_class = parent_inst.class_name(); + let parent_name = parent_inst.name(); + let containing_dir = match resolved_path.parent() { + Some(d) => d, + None => continue, + }; + log::info!( + "Syncback: Pre-converting standalone {} '{}' at {} to directory \ + (children being added in this batch)", + parent_class, + parent_name, + resolved_path.display() + ); + let new_dir = if matches!( + parent_class.as_str(), + "ModuleScript" | "Script" | "LocalScript" + ) { + self.convert_standalone_script_to_directory( + &resolved_path, + parent_name, + parent_class.as_str(), + containing_dir, + ) + } else { + self.convert_standalone_instance_to_directory( + &resolved_path, + parent_name, + parent_class.as_str(), + containing_dir, + ) + }; + match new_dir { + Ok(dir) => { + converted_parents.insert(parent_ref, dir); + } + Err(err) => { + log::warn!( + "Failed to pre-convert parent '{}': {}", + parent_name, + err + ); + } + } + } + } + } + } + } + + for added in request.added.values() { + if let Err(err) = self.syncback_added_instance( + added, + &tree, + &duplicate_siblings_cache, + &stats, + &converted_parents, + ) { log::warn!( "Failed to syncback added instance '{}': {}", added.name, @@ -663,18 +767,10 @@ impl ApiService { tree: &crate::snapshot::RojoTree, duplicate_siblings_cache: &HashSet, stats: &crate::syncback::SyncbackStats, + converted_parents: &HashMap, ) -> anyhow::Result<()> { - use crate::syncback::validate_file_name; use anyhow::Context; - // Validate the instance name for filesystem safety - validate_file_name(&added.name).with_context(|| { - format!( - "Instance name '{}' is not valid for the filesystem", - added.name - ) - })?; - // Get the parent Ref (required for top-level added instances) let parent_ref = added .parent @@ -756,9 +852,18 @@ impl ApiService { // ANY instance can have children in Roblox, so if the parent is a // standalone file (script, model, txt, csv, etc.), we must convert it // to directory format before we can place children inside it. - let parent_dir = if parent_path.is_dir() { + // + // Check converted_parents first — if another child in this batch + // already triggered the conversion, use the cached directory path + // instead of trying to convert again (the standalone file was already + // deleted by the first conversion). + let parent_dir = if let Some(dir) = converted_parents.get(&parent_ref) { + dir.clone() + } else if parent_path.is_dir() { parent_path.to_path_buf() } else { + // Fallback: parent wasn't pre-converted (e.g., parent was resolved + // through a ProjectNode path). Convert it now. let parent_class = parent_instance.class_name(); let parent_name = parent_instance.name(); let containing_dir = parent_path @@ -1491,20 +1596,42 @@ impl ApiService { parent_dir: &std::path::Path, stats: &crate::syncback::SyncbackStats, ) -> anyhow::Result<()> { - use crate::syncback::validate_file_name; + use crate::syncback::{deduplicate_name, name_needs_slugify}; use anyhow::Context; - // Validate the instance name for filesystem safety - validate_file_name(&added.name).with_context(|| { - format!( - "Instance name '{}' is not valid for the filesystem", - added.name - ) - })?; + // Slugify the instance name for filesystem safety if it contains + // forbidden characters. The real name is preserved in metadata. + let needs_slug = name_needs_slugify(&added.name); + let base_name = if needs_slug { + slugify_name(&added.name) + } else { + added.name.clone() + }; - // Encode instance name for filesystem safety (Windows-invalid chars, dots, etc.) - // This matches change_processor.rs which uses encode_path_name for renames. - let encoded_name = encode_path_name(&added.name); + // Deduplicate against existing files in the parent directory to prevent + // collisions (e.g., two instances whose slugified names match, or a new + // instance whose name matches an existing file). + let taken_names: HashSet = if parent_dir.is_dir() { + fs::read_dir(parent_dir) + .ok() + .map(|entries| { + entries + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .map(|n| n.to_lowercase()) + .collect() + }) + .unwrap_or_default() + } else { + HashSet::new() + }; + let encoded_name = deduplicate_name(&base_name, &taken_names); + let needs_meta_name = needs_slug || encoded_name != base_name; + let meta_name_field: Option<&str> = if needs_meta_name { + Some(&added.name) + } else { + None + }; // Build path string for stats let inst_path = format!("{}/{}", parent_dir.display(), added.name); @@ -1569,7 +1696,7 @@ impl ApiService { fs::write(&init_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; - self.write_script_meta_json_if_needed(&dir_path, added)?; + 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)?; @@ -1580,7 +1707,12 @@ impl ApiService { fs::write(&file_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; - self.write_adjacent_script_meta_if_needed(parent_dir, &encoded_name, added)?; + self.write_adjacent_script_meta_if_needed( + parent_dir, + &encoded_name, + added, + meta_name_field, + )?; log::info!("Syncback: Updated ModuleScript at {}", file_path.display()); } } @@ -1621,7 +1753,7 @@ impl ApiService { fs::write(&init_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; - self.write_script_meta_json_if_needed(&dir_path, added)?; + 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)?; @@ -1633,7 +1765,12 @@ impl ApiService { fs::write(&file_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; - self.write_adjacent_script_meta_if_needed(parent_dir, &encoded_name, added)?; + self.write_adjacent_script_meta_if_needed( + parent_dir, + &encoded_name, + added, + meta_name_field, + )?; log::info!("Syncback: Updated Script at {}", file_path.display()); } } @@ -1673,7 +1810,7 @@ impl ApiService { fs::write(&init_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; - self.write_script_meta_json_if_needed(&dir_path, added)?; + 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)?; @@ -1684,7 +1821,12 @@ impl ApiService { fs::write(&file_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; - self.write_adjacent_script_meta_if_needed(parent_dir, &encoded_name, added)?; + self.write_adjacent_script_meta_if_needed( + parent_dir, + &encoded_name, + added, + meta_name_field, + )?; log::info!("Syncback: Updated LocalScript at {}", file_path.display()); } } @@ -1702,7 +1844,7 @@ impl ApiService { let has_metadata = if added.properties.is_empty() { false } else { - self.write_directory_meta_json(&dir_path, added)?; + self.write_directory_meta_json(&dir_path, added, meta_name_field)?; true }; @@ -1869,6 +2011,7 @@ impl ApiService { &self, dir_path: &std::path::Path, added: &crate::web::interface::AddedInstance, + instance_name: Option<&str>, ) -> anyhow::Result<()> { use anyhow::Context; @@ -1877,11 +2020,11 @@ impl ApiService { let (properties, attributes) = self.filter_properties_for_meta(&added.class_name, &added.properties, Some("Source")); - if properties.is_empty() && attributes.is_empty() { + if properties.is_empty() && attributes.is_empty() && instance_name.is_none() { return Ok(()); } - let meta = self.build_meta_object(None, properties, attributes); + let meta = self.build_meta_object(None, instance_name, properties, attributes); let meta_path = dir_path.join("init.meta.json5"); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; @@ -1903,17 +2046,18 @@ impl ApiService { &self, dir_path: &std::path::Path, added: &crate::web::interface::AddedInstance, + instance_name: Option<&str>, ) -> anyhow::Result<()> { use anyhow::Context; let (properties, attributes) = self.filter_properties_for_meta(&added.class_name, &added.properties, None); - if properties.is_empty() && attributes.is_empty() { + if properties.is_empty() && attributes.is_empty() && instance_name.is_none() { return Ok(()); } - let meta = self.build_meta_object(None, properties, attributes); + let meta = self.build_meta_object(None, instance_name, properties, attributes); let meta_path = dir_path.join("init.meta.json5"); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; @@ -1941,7 +2085,7 @@ impl ApiService { self.filter_properties_for_meta(&added.class_name, &added.properties, None); // For non-Folder classes, we need to include className - let meta = self.build_meta_object(Some(&added.class_name), properties, attributes); + let meta = self.build_meta_object(Some(&added.class_name), None, properties, attributes); let meta_path = dir_path.join("init.meta.json5"); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; @@ -2047,6 +2191,7 @@ impl ApiService { fn build_meta_object( &self, class_name: Option<&str>, + instance_name: Option<&str>, properties: indexmap::IndexMap, attributes: indexmap::IndexMap, ) -> serde_json::Value { @@ -2058,6 +2203,10 @@ impl ApiService { obj.insert("className".to_string(), json!(cn)); } + if let Some(name) = instance_name { + obj.insert("name".to_string(), json!(name)); + } + if !properties.is_empty() { obj.insert("properties".to_string(), json!(properties)); } @@ -2134,6 +2283,7 @@ impl ApiService { parent_dir: &std::path::Path, name: &str, added: &crate::web::interface::AddedInstance, + instance_name: Option<&str>, ) -> anyhow::Result<()> { use anyhow::Context; @@ -2141,12 +2291,12 @@ impl ApiService { let (properties, attributes) = self.filter_properties_for_meta(&added.class_name, &added.properties, Some("Source")); - if properties.is_empty() && attributes.is_empty() { + if properties.is_empty() && attributes.is_empty() && instance_name.is_none() { return Ok(()); } // Build meta object (no className needed for scripts - it's determined by file extension) - let meta = self.build_meta_object(None, properties, attributes); + let meta = self.build_meta_object(None, instance_name, properties, attributes); let meta_path = parent_dir.join(format!("{}.meta.json5", name)); let content = crate::json::to_vec_pretty_sorted(&meta).context("Failed to serialize meta.json5")?; @@ -2413,7 +2563,7 @@ impl ApiService { Ok(existing) } else { // No existing file — build from scratch - Ok(self.build_meta_object(class_name, new_properties, new_attributes)) + Ok(self.build_meta_object(class_name, None, new_properties, new_attributes)) } } @@ -5387,4 +5537,848 @@ mod tests { assert_eq!(dir_name_from_instance_path(path), "Something"); } } + + // ══════════════════════════════════════════════════════════════════ + // Slug collision removal safety tests + // + // These verify the critical invariant: when an instance is removed + // and a NEW instance is created at the same slugified path, the + // removal must NOT destroy the newly created file. + // + // This was a real bug: renaming "joe_test" → "joe/test" produces + // the same slug "joe_test". The API handler deletes the old file + // and creates the new one, but the ChangeProcessor's PatchSet + // removal would re-delete the new file because path.exists() + // returned true for the NEW file. + // ══════════════════════════════════════════════════════════════════ + mod slug_collision_removal_safety { + use crate::syncback::{name_needs_slugify, slugify_name}; + use std::fs::{self, File}; + use std::io::Write; + use tempfile::tempdir; + + /// Simulates the API handler flow: remove old file, create new file at + /// potentially the same path (because slugs collide), then verify the + /// new file survives. + fn simulate_rename_via_slug( + old_name: &str, + new_name: &str, + extension: &str, + ) -> (bool, String, String) { + // Returns: (new_file_survived, old_slug, new_slug) + let old_slug = if name_needs_slugify(old_name) { + slugify_name(old_name) + } else { + old_name.to_string() + }; + let new_slug = if name_needs_slugify(new_name) { + slugify_name(new_name) + } else { + new_name.to_string() + }; + + let temp = tempdir().expect("Failed to create temp dir"); + let old_file = temp.path().join(format!("{old_slug}.{extension}")); + let new_file = temp.path().join(format!("{new_slug}.{extension}")); + + // Step 1: Create old file (simulates existing state) + File::create(&old_file) + .unwrap() + .write_all(b"old content") + .unwrap(); + + // Step 2: API handler removes old file + if old_file.exists() { + fs::remove_file(&old_file).unwrap(); + } + + // Step 3: API handler creates new file (may be same path!) + File::create(&new_file) + .unwrap() + .write_all(b"new content") + .unwrap(); + + // Step 4: ChangeProcessor receives PatchSet with old instance removal. + // The OLD instance's instigating_source points to old_file. + // The fix: ChangeProcessor must NOT delete old_file because it + // now contains the NEW instance's content. + // + // (We don't actually call handle_tree_event here — we verify + // the invariant that the new file must survive.) + + let survived = + new_file.exists() && fs::read_to_string(&new_file).unwrap() == "new content"; + + (survived, old_slug, new_slug) + } + + // ── Same-slug renames (the critical case) ──────────────────── + + #[test] + fn rename_slash_to_underscore_same_slug() { + // "joe/test" → slug "joe_test", same as original "joe_test" + let (survived, old_slug, new_slug) = + simulate_rename_via_slug("joe_test", "joe/test", "legacy.luau"); + assert_eq!(old_slug, "joe_test"); + assert_eq!(new_slug, "joe_test"); + assert!(survived, "new file must survive when slugs collide"); + } + + #[test] + fn rename_colon_to_underscore_same_slug() { + let (survived, old, new) = simulate_rename_via_slug("A_B", "A:B", "luau"); + assert_eq!(old, "A_B"); + assert_eq!(new, "A_B"); + assert!(survived); + } + + #[test] + fn rename_star_to_underscore_same_slug() { + let (survived, old, new) = + simulate_rename_via_slug("Glob_Pattern", "Glob*Pattern", "server.luau"); + assert_eq!(old, "Glob_Pattern"); + assert_eq!(new, "Glob_Pattern"); + assert!(survived); + } + + #[test] + fn rename_question_to_underscore_same_slug() { + let (survived, _, _) = simulate_rename_via_slug("What_", "What?", "luau"); + assert!(survived); + } + + #[test] + fn rename_pipe_to_underscore_same_slug() { + let (survived, _, _) = simulate_rename_via_slug("X_Y", "X|Y", "client.luau"); + assert!(survived); + } + + #[test] + fn rename_backslash_to_underscore_same_slug() { + let (survived, _, _) = simulate_rename_via_slug("path_to", "path\\to", "luau"); + assert!(survived); + } + + #[test] + fn rename_angle_brackets_same_slug() { + let (survived, _, _) = simulate_rename_via_slug("_init_", "", "luau"); + assert!(survived); + } + + #[test] + fn rename_tilde_to_underscore_same_slug() { + let (survived, old, new) = simulate_rename_via_slug("foo_1", "foo~1", "luau"); + assert_eq!(old, "foo_1"); + assert_eq!(new, "foo_1"); + assert!(survived); + } + + // ── Different-slug renames (should also work fine) ─────────── + + #[test] + fn rename_clean_to_clean_different_slug() { + let (survived, old, new) = simulate_rename_via_slug("Alpha", "Beta", "luau"); + assert_ne!(old, new); + assert!(survived, "different slugs should trivially survive"); + } + + #[test] + fn rename_clean_to_forbidden() { + let (survived, old, new) = + simulate_rename_via_slug("MyScript", "My/Script", "server.luau"); + assert_eq!(old, "MyScript"); + assert_eq!(new, "My_Script"); + assert_ne!(old, new); + assert!(survived); + } + + #[test] + fn rename_forbidden_to_clean() { + let (survived, _, _) = simulate_rename_via_slug("X_Y", "XY", "luau"); + assert!(survived); + } + + // ── Dangerous suffix renames ───────────────────────────────── + + #[test] + fn rename_to_dangerous_suffix_server() { + let (survived, _, new) = simulate_rename_via_slug("foo", "foo.server", "luau"); + assert_eq!(new, "foo_server"); + assert!(survived); + } + + #[test] + fn rename_to_dangerous_suffix_meta() { + let (survived, _, new) = simulate_rename_via_slug("config", "config.meta", "luau"); + assert_eq!(new, "config_meta"); + assert!(survived); + } + + #[test] + fn rename_from_dangerous_suffix() { + let (survived, old, _) = simulate_rename_via_slug("foo_server", "foo.server", "luau"); + assert_eq!(old, "foo_server"); + assert!(survived); + } + + // ── Windows reserved name renames ──────────────────────────── + + #[test] + fn rename_to_con() { + let (survived, _, new) = simulate_rename_via_slug("config", "CON", "luau"); + assert_eq!(new, "CON_"); + assert!(survived); + } + + #[test] + fn rename_from_con_slug_collision() { + // "CON_" (natural) → "CON" (reserved, slugifies to "CON_") + let (survived, old, new) = simulate_rename_via_slug("CON_", "CON", "luau"); + assert_eq!(old, "CON_"); + assert_eq!(new, "CON_"); + assert!(survived, "same-slug collision from Windows reserved name"); + } + + // ── Space renames ──────────────────────────────────────────── + + #[test] + fn rename_add_leading_space() { + let (survived, old, new) = simulate_rename_via_slug("Hello", " Hello", "luau"); + assert_eq!(old, "Hello"); + assert_eq!(new, "Hello"); // leading space stripped + assert!(survived, "stripped leading space creates same slug"); + } + + #[test] + fn rename_add_trailing_space() { + let (survived, old, new) = simulate_rename_via_slug("Hello", "Hello ", "luau"); + assert_eq!(old, "Hello"); + assert_eq!(new, "Hello"); // trailing space stripped + assert!(survived); + } + + #[test] + fn rename_with_middle_space_no_collision() { + let (survived, old, new) = simulate_rename_via_slug("Hello", "Hello World", "luau"); + assert_eq!(old, "Hello"); + assert_eq!(new, "Hello World"); + assert_ne!(old, new); + assert!(survived); + } + + // ── Meta file survival ─────────────────────────────────────── + + #[test] + fn meta_file_survives_slug_collision_rename() { + let temp = tempdir().expect("Failed to create temp dir"); + let slug = "joe_test"; + + // Create old files + let old_script = temp.path().join(format!("{slug}.legacy.luau")); + let old_meta = temp.path().join(format!("{slug}.meta.json5")); + File::create(&old_script) + .unwrap() + .write_all(b"old") + .unwrap(); + File::create(&old_meta).unwrap().write_all(b"{}").unwrap(); + + // API handler: remove old + fs::remove_file(&old_script).unwrap(); + fs::remove_file(&old_meta).unwrap(); + + // API handler: create new (same slug from "joe/test") + let new_script = temp.path().join(format!("{slug}.legacy.luau")); + let new_meta = temp.path().join(format!("{slug}.meta.json5")); + File::create(&new_script) + .unwrap() + .write_all(b"new code") + .unwrap(); + File::create(&new_meta) + .unwrap() + .write_all(br#"{"name": "joe/test"}"#) + .unwrap(); + + // Both new files must survive + assert!(new_script.exists(), "new script must survive"); + assert!(new_meta.exists(), "new meta must survive"); + assert_eq!(fs::read_to_string(&new_script).unwrap(), "new code"); + assert!(fs::read_to_string(&new_meta).unwrap().contains("joe/test")); + } + + // ── Directory format survival ──────────────────────────────── + + #[test] + fn directory_survives_slug_collision_rename() { + let temp = tempdir().expect("Failed to create temp dir"); + + // Old instance "Stuff_Here" is a directory + let old_dir = temp.path().join("Stuff_Here"); + fs::create_dir(&old_dir).unwrap(); + File::create(old_dir.join("init.luau")) + .unwrap() + .write_all(b"old") + .unwrap(); + + // API handler: remove old directory + fs::remove_dir_all(&old_dir).unwrap(); + + // API handler: create new directory (slug of "Stuff/Here" = "Stuff_Here") + let new_dir = temp.path().join("Stuff_Here"); + fs::create_dir(&new_dir).unwrap(); + File::create(new_dir.join("init.luau")) + .unwrap() + .write_all(b"new") + .unwrap(); + File::create(new_dir.join("init.meta.json5")) + .unwrap() + .write_all(br#"{"name": "Stuff/Here"}"#) + .unwrap(); + + // New directory and contents must survive + assert!(new_dir.exists(), "new directory must survive"); + assert!(new_dir.join("init.luau").exists()); + assert!(new_dir.join("init.meta.json5").exists()); + assert_eq!( + fs::read_to_string(new_dir.join("init.luau")).unwrap(), + "new" + ); + } + + // ── Stress: many renames producing same slug ───────────────── + + #[test] + fn stress_many_forbidden_chars_same_slug() { + // All these names slugify to the same thing. Verify each + // rename-to scenario preserves the new file. + let variants = [ + ("A_B", "A/B"), + ("A_B", "A:B"), + ("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", "A~B"), + ]; + for (old_name, new_name) in variants { + let (survived, old_slug, new_slug) = + simulate_rename_via_slug(old_name, new_name, "luau"); + assert_eq!( + old_slug, new_slug, + "{old_name} and {new_name} should produce same slug" + ); + assert!( + survived, + "new file must survive for {old_name} → {new_name}" + ); + } + } + + #[test] + fn stress_bidirectional_rename_survival() { + // Rename A→B then B→A, both produce same slug. + // Verify both directions work. + let pairs = [ + ("joe_test", "joe/test"), + ("Hey_Bro", "Hey:Bro"), + ("foo_1", "foo~1"), + ("CON_", "CON"), + ]; + for (a, b) in pairs { + let (surv_ab, _, _) = simulate_rename_via_slug(a, b, "luau"); + let (surv_ba, _, _) = simulate_rename_via_slug(b, a, "luau"); + assert!(surv_ab, "A→B must survive for {a} → {b}"); + assert!(surv_ba, "B→A must survive for {b} → {a}"); + } + } + + // ── Verify name_needs_slugify consistency ──────────────────── + + #[test] + fn slug_collision_only_possible_when_needs_slugify() { + // If neither old nor new needs slugifying, their slugs are + // themselves, so they can't collide (different names = different slugs). + let cases = [("Alpha", "Beta"), ("Hello", "World"), ("x", "y")]; + for (old, new) in cases { + assert!(!name_needs_slugify(old)); + assert!(!name_needs_slugify(new)); + let old_slug = slugify_name(old); + let new_slug = slugify_name(new); + assert_ne!( + old_slug, new_slug, + "clean names with different values can't produce same slug" + ); + } + } + + #[test] + fn all_forbidden_chars_produce_underscore_slug() { + // Every forbidden char in isolation slugifies to "_" → "instance" + let forbidden = ['<', '>', ':', '"', '/', '|', '?', '*', '\\', '~']; + for ch in forbidden { + let name = ch.to_string(); + assert!(name_needs_slugify(&name)); + let slug = slugify_name(&name); + assert_eq!( + slug, "instance", + "single {ch:?} should slugify to fallback 'instance'" + ); + } + } + } + + // ══════════════════════════════════════════════════════════════════ + // Two-way sync: write_child_to_disk dedup simulation + // + // These simulate the actual flow in write_child_to_disk: + // 1. Read parent directory entries → taken_names + // 2. Slugify instance name if needed + // 3. Deduplicate against taken_names + // 4. Write file + optional meta with `name` field + // + // This is the MOST critical path for two-way sync correctness. + // ══════════════════════════════════════════════════════════════════ + mod twoway_sync_dedup { + use crate::syncback::{deduplicate_name, name_needs_slugify, slugify_name}; + use std::collections::HashSet; + use std::fs::{self, File}; + use std::io::Write; + use tempfile::tempdir; + + // ══════════════════════════════════════════════════════════════ + // IMPORTANT: deduplicate_name compares BARE SLUGS against + // taken_names. When taken_names is seeded from fs::read_dir, + // entries include extensions (e.g., "foo.luau"). The bare slug + // "foo" does NOT match "foo.luau", so dedup only catches + // collisions for DIRECTORY entries (where filename == bare name). + // + // For file-format instances, the duplicate-name pre-filter in + // the plugin (encodeInstance.lua) prevents same-name siblings + // from reaching the API at all. Cross-name slug collisions for + // file-format are a known limitation until Phase 2. + // + // These tests focus on DIRECTORY FORMAT dedup (where it works) + // and document file-format behavior accurately. + // ══════════════════════════════════════════════════════════════ + + /// Simulates the write_child_to_disk flow for DIRECTORY format: + /// directory names have no extension, so bare slug == filename, + /// and dedup against fs::read_dir entries works correctly. + fn simulate_dir_dedup( + existing_dirs: &[&str], + instance_name: &str, + ) -> (String, Option) { + let temp = tempdir().expect("Failed to create temp dir"); + for name in existing_dirs { + fs::create_dir(temp.path().join(name)).unwrap(); + } + + let taken: HashSet = fs::read_dir(temp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .map(|n| n.to_lowercase()) + .collect(); + + let needs_slug = name_needs_slugify(instance_name); + let base = if needs_slug { + slugify_name(instance_name) + } else { + instance_name.to_string() + }; + let deduped = deduplicate_name(&base, &taken); + let needs_meta = needs_slug || deduped != base; + + fs::create_dir(temp.path().join(&deduped)).unwrap(); + + let meta = if needs_meta { + Some(instance_name.to_string()) + } else { + None + }; + (deduped, meta) + } + + /// Simulates batch add of DIRECTORY format children. + fn simulate_dir_batch( + existing_dirs: &[&str], + children: &[&str], + ) -> Vec<(String, Option)> { + let temp = tempdir().expect("Failed to create temp dir"); + for name in existing_dirs { + fs::create_dir(temp.path().join(name)).unwrap(); + } + + let mut taken: HashSet = fs::read_dir(temp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .map(|n| n.to_lowercase()) + .collect(); + + let mut results = Vec::new(); + for &name in children { + let needs_slug = name_needs_slugify(name); + let base = if needs_slug { + slugify_name(name) + } else { + name.to_string() + }; + let deduped = deduplicate_name(&base, &taken); + let needs_meta = needs_slug || deduped != base; + taken.insert(deduped.to_lowercase()); + let meta = if needs_meta { + Some(name.to_string()) + } else { + None + }; + results.push((deduped, meta)); + } + results + } + + // ── Directory dedup against existing dirs ──────────────────── + + #[test] + fn dir_clean_name_no_collision() { + let (f, m) = simulate_dir_dedup(&[], "NewFolder"); + assert_eq!(f, "NewFolder"); + assert!(m.is_none()); + } + + #[test] + fn dir_clean_name_collides_with_existing() { + let (f, m) = simulate_dir_dedup(&["NewFolder"], "NewFolder"); + assert_eq!(f, "NewFolder~1"); + assert_eq!(m.unwrap(), "NewFolder"); + } + + #[test] + fn dir_forbidden_name_no_collision() { + let (f, m) = simulate_dir_dedup(&[], "Hey/Bro"); + assert_eq!(f, "Hey_Bro"); + assert_eq!(m.unwrap(), "Hey/Bro"); + } + + #[test] + fn dir_forbidden_collides_with_existing() { + let (f, m) = simulate_dir_dedup(&["Hey_Bro"], "Hey/Bro"); + assert_eq!(f, "Hey_Bro~1"); + assert_eq!(m.unwrap(), "Hey/Bro"); + } + + #[test] + fn dir_natural_collides_with_slug() { + let (f, m) = simulate_dir_dedup(&["A_B"], "A_B"); + assert_eq!(f, "A_B~1"); + assert_eq!(m.unwrap(), "A_B"); + } + + #[test] + fn dir_case_insensitive_collision() { + let (f, m) = simulate_dir_dedup(&["MyFolder"], "myfolder"); + assert_eq!(f, "myfolder~1"); + assert_eq!(m.unwrap(), "myfolder"); + } + + #[test] + fn dir_multiple_collisions() { + let (f, _) = simulate_dir_dedup(&["Foo", "Foo~1", "Foo~2"], "Foo"); + assert_eq!(f, "Foo~3"); + } + + #[test] + fn dir_gap_in_chain() { + let (f, _) = simulate_dir_dedup(&["Test", "Test~1", "Test~3"], "Test"); + assert_eq!(f, "Test~2"); + } + + #[test] + fn dir_dangerous_suffix_server() { + let (f, m) = simulate_dir_dedup(&[], "foo.server"); + assert_eq!(f, "foo_server"); + assert_eq!(m.unwrap(), "foo.server"); + } + + #[test] + fn dir_dangerous_suffix_collides() { + let (f, m) = simulate_dir_dedup(&["config_meta"], "config.meta"); + assert_eq!(f, "config_meta~1"); + assert_eq!(m.unwrap(), "config.meta"); + } + + #[test] + fn dir_con_creates_valid_name() { + let (f, m) = simulate_dir_dedup(&[], "CON"); + assert_eq!(f, "CON_"); + assert_eq!(m.unwrap(), "CON"); + } + + #[test] + fn dir_nul_collides_with_existing() { + let (f, _) = simulate_dir_dedup(&["NUL_"], "NUL"); + assert_eq!(f, "NUL_~1"); + } + + // ── Batch add: directory siblings ───────────────────────────── + + #[test] + fn batch_dir_three_clean() { + let r = simulate_dir_batch(&[], &["Alpha", "Beta", "Gamma"]); + assert_eq!(r[0].0, "Alpha"); + assert_eq!(r[1].0, "Beta"); + assert_eq!(r[2].0, "Gamma"); + assert!(r.iter().all(|(_, m)| m.is_none())); + } + + #[test] + fn batch_dir_duplicate_names() { + let r = simulate_dir_batch(&[], &["Script", "Script", "Script"]); + assert_eq!(r[0].0, "Script"); + assert!(r[0].1.is_none()); + assert_eq!(r[1].0, "Script~1"); + assert_eq!(r[1].1.as_deref(), Some("Script")); + assert_eq!(r[2].0, "Script~2"); + } + + #[test] + fn batch_dir_slug_collision_siblings() { + let r = simulate_dir_batch(&[], &["X/Y", "X:Y", "X*Y"]); + assert_eq!(r[0].0, "X_Y"); + assert_eq!(r[1].0, "X_Y~1"); + assert_eq!(r[2].0, "X_Y~2"); + } + + #[test] + fn batch_dir_with_existing_and_collisions() { + 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"); + assert!(r[2].1.is_none()); + } + + #[test] + fn batch_dir_dangerous_then_natural_then_slug() { + 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"); + } + + #[test] + fn batch_dir_windows_reserved_pileup() { + let r = simulate_dir_batch(&[], &["CON", "CON_", "con"]); + assert_eq!(r[0].0, "CON_"); + assert_eq!(r[0].1.as_deref(), Some("CON")); + assert_eq!(r[1].0, "CON_~1"); + assert_eq!(r[1].1.as_deref(), Some("CON_")); + assert_eq!(r[2].0, "con_~2"); + assert_eq!(r[2].1.as_deref(), Some("con")); + } + + #[test] + fn batch_dir_empty_names() { + let r = simulate_dir_batch(&[], &["", "", ""]); + assert_eq!(r[0].0, "instance"); + assert_eq!(r[1].0, "instance~1"); + assert_eq!(r[2].0, "instance~2"); + } + + #[test] + fn batch_dir_20_same_name() { + 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}")); + } + let unique: HashSet = r.iter().map(|(f, _)| f.to_lowercase()).collect(); + assert_eq!(unique.len(), 20); + } + + // ── File-format behavior documentation ─────────────────────── + + #[test] + fn file_format_bare_slug_vs_full_filename_limitation() { + // DOCUMENTS KNOWN BEHAVIOR: deduplicate_name checks bare slug + // "Foo" against taken entries. When taken comes from fs::read_dir, + // entries have extensions ("foo.luau"). "foo" ≠ "foo.luau" so no + // collision is detected. This is by design — duplicate-named + // file instances are filtered by the plugin before reaching the API. + let temp = tempdir().expect("Failed to create temp dir"); + File::create(temp.path().join("Foo.luau")) + .unwrap() + .write_all(b"existing") + .unwrap(); + + let taken: HashSet = fs::read_dir(temp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .map(|n| n.to_lowercase()) + .collect(); + + assert!(taken.contains("foo.luau")); + assert!(!taken.contains("foo")); + // Bare slug doesn't match full filename + let deduped = deduplicate_name("Foo", &taken); + assert_eq!(deduped, "Foo", "bare slug doesn't match 'foo.luau'"); + } + + #[test] + fn file_format_slugify_still_works() { + // Even though dedup doesn't catch cross-file collisions, + // slugification itself always works correctly. + let temp = tempdir().expect("Failed to create temp dir"); + let taken = HashSet::new(); + + let slug = slugify_name("Hey/Bro"); + assert_eq!(slug, "Hey_Bro"); + let deduped = deduplicate_name(&slug, &taken); + assert_eq!(deduped, "Hey_Bro"); + + let file_path = temp.path().join("Hey_Bro.luau"); + File::create(&file_path) + .unwrap() + .write_all(b"content") + .unwrap(); + assert!(file_path.exists()); + } + + // ── Rename → re-add cycle (the critical slug safety test) ──── + + #[test] + fn rename_cycle_same_slug_survives() { + let temp = tempdir().expect("Failed to create temp dir"); + + File::create(temp.path().join("joe_test.legacy.luau")) + .unwrap() + .write_all(b"old code") + .unwrap(); + + // API: remove old + fs::remove_file(temp.path().join("joe_test.legacy.luau")).unwrap(); + + // API: write new (dir is now empty) + let taken: HashSet = fs::read_dir(temp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .map(|n| n.to_lowercase()) + .collect(); + assert!(taken.is_empty()); + + let slug = slugify_name("joe/test"); + assert_eq!(slug, "joe_test"); + let deduped = deduplicate_name(&slug, &taken); + assert_eq!(deduped, "joe_test"); + + let new_file = temp.path().join("joe_test.legacy.luau"); + File::create(&new_file) + .unwrap() + .write_all(b"new code") + .unwrap(); + let meta_file = temp.path().join("joe_test.meta.json5"); + File::create(&meta_file) + .unwrap() + .write_all(br#"{"name": "joe/test"}"#) + .unwrap(); + + assert!(new_file.exists()); + assert!(meta_file.exists()); + assert_eq!(fs::read_to_string(&new_file).unwrap(), "new code"); + assert!(fs::read_to_string(&meta_file).unwrap().contains("joe/test")); + } + + #[test] + fn dir_add_to_populated_no_overwrite() { + // Dir has: Helper/ and Helper.meta.json5 + // Add new "Helper" dir → must get Helper~1 + let temp = tempdir().expect("Failed to create temp dir"); + fs::create_dir(temp.path().join("Helper")).unwrap(); + File::create(temp.path().join("Helper.meta.json5")) + .unwrap() + .write_all(b"{}") + .unwrap(); + + let taken: HashSet = fs::read_dir(temp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter_map(|e| e.file_name().into_string().ok()) + .map(|n| n.to_lowercase()) + .collect(); + + let deduped = deduplicate_name("Helper", &taken); + assert_eq!(deduped, "Helper~1"); + } + + // ── Nightmare batch scenarios ──────────────────────────────── + + #[test] + fn batch_dir_10_way_slug_pileup_with_existing() { + let r = simulate_dir_batch( + &["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"); + assert_eq!(r[1].0, "A_B~2"); + assert_eq!(r[2].0, "A_B~3"); + assert_eq!(r[3].0, "A_B~4"); + assert_eq!(r[4].0, "A_B~5"); + assert_eq!(r[5].0, "A_B~6"); + assert_eq!(r[6].0, "A_B~7"); + assert_eq!(r[7].0, "A_B~8"); + assert_eq!(r[8].0, "A_B~9"); + } + + #[test] + fn batch_dir_con_prn_nul_then_slugged() { + 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 + assert_eq!(r[3].0, "PRN_~1"); + } + + #[test] + fn batch_dir_unicode_plus_forbidden() { + 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"); + } + + #[test] + 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", + ]; + let r = simulate_dir_batch(&[], children); + let unique: HashSet = r.iter().map(|(f, _)| f.to_lowercase()).collect(); + assert_eq!( + unique.len(), r.len(), + "every sibling must get a unique filename" + ); + } + } } diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index abd4a568..0ccefd4a 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -1232,28 +1232,31 @@ fn get_encoded_names_instance( } // --------------------------------------------------------------------------- -// Tests: Encoded name directory conversion (path encoding fix) +// Tests: Slugified name directory conversion (path encoding fix) // --------------------------------------------------------------------------- -/// Test 22: Adding a child to a standalone ModuleScript with Rojo-encoded -/// characters (e.g., `What%QUESTION%Module.luau`) creates the directory using -/// the filesystem-encoded name (`What%QUESTION%Module/`), NOT the decoded +/// Test 22: Adding a child to a standalone ModuleScript with slugified +/// characters (e.g., `What_Module.luau`) creates the directory using +/// the slugified filesystem name (`What_Module/`), NOT the decoded /// instance name (`What?Module/`). /// -/// This is the core regression test for the path encoding fix in +/// This is the core regression test for the slugified name fix in /// `convert_standalone_script_to_directory`. #[test] fn add_child_to_encoded_module_creates_encoded_directory() { run_serve_test("syncback_encoded_names", |session, _redactions| { let (session_id, encoded_module_id) = get_encoded_names_instance(&session, "What?Module"); - let standalone_file = session.path().join("src").join("What%QUESTION%Module.luau"); - // The CORRECT directory name uses the encoded filesystem name - let encoded_dir = session.path().join("src").join("What%QUESTION%Module"); + let standalone_file = session.path().join("src").join("What_Module.luau"); + // The CORRECT directory name uses the slugified filesystem name + let encoded_dir = session.path().join("src").join("What_Module"); // The WRONG directory name would use the decoded instance name let decoded_dir = session.path().join("src").join("What?Module"); - assert_file_exists(&standalone_file, "Encoded standalone file before child add"); + assert_file_exists( + &standalone_file, + "Slugified standalone file before child add", + ); // Add a child to trigger standalone → directory conversion let mut properties = HashMap::new(); @@ -1285,10 +1288,10 @@ fn add_child_to_encoded_module_creates_encoded_directory() { "Standalone file after directory conversion", ); - // Directory should use ENCODED name (the fix) + // Directory should use SLUGIFIED name (the fix) assert!( encoded_dir.is_dir(), - "Directory should use encoded name (What%QUESTION%Module/): {}", + "Directory should use slugified name (What_Module/): {}", encoded_dir.display() ); @@ -1299,38 +1302,35 @@ fn add_child_to_encoded_module_creates_encoded_directory() { decoded_dir.display() ); - // init.luau should exist inside the encoded directory + // init.luau should exist inside the slugified directory let init_file = encoded_dir.join("init.luau"); - assert_file_exists(&init_file, "init.luau inside encoded directory"); + assert_file_exists(&init_file, "init.luau inside slugified directory"); - // Child should be created inside the encoded directory + // Child should be created inside the slugified directory let child_file = encoded_dir.join("EncodedChild.luau"); - assert_file_exists(&child_file, "Child inside encoded directory"); + assert_file_exists(&child_file, "Child inside slugified directory"); }); } -/// Test 23: Adding a child to a standalone Script with Rojo-encoded characters -/// AND a script suffix (e.g., `Key%COLON%Script.server.luau`) strips the suffix +/// Test 23: Adding a child to a standalone Script with slugified characters +/// AND a script suffix (e.g., `Key_Script.server.luau`) strips the suffix /// correctly when creating the directory name. /// -/// The directory should be `Key%COLON%Script/`, not `Key%COLON%Script.server/` +/// The directory should be `Key_Script/`, not `Key_Script.server/` /// or `Key:Script/`. #[test] fn add_child_to_encoded_script_strips_suffix_correctly() { run_serve_test("syncback_encoded_names", |session, _redactions| { let (session_id, encoded_script_id) = get_encoded_names_instance(&session, "Key:Script"); - let standalone_file = session - .path() - .join("src") - .join("Key%COLON%Script.server.luau"); - let encoded_dir = session.path().join("src").join("Key%COLON%Script"); + let standalone_file = session.path().join("src").join("Key_Script.server.luau"); + let encoded_dir = session.path().join("src").join("Key_Script"); // Wrong: decoded name let decoded_dir = session.path().join("src").join("Key:Script"); // Wrong: suffix not stripped - let unsuffixed_dir = session.path().join("src").join("Key%COLON%Script.server"); + let unsuffixed_dir = session.path().join("src").join("Key_Script.server"); - assert_file_exists(&standalone_file, "Encoded script before child add"); + assert_file_exists(&standalone_file, "Slugified script before child add"); let mut properties = HashMap::new(); properties.insert( @@ -1359,7 +1359,7 @@ fn add_child_to_encoded_script_strips_suffix_correctly() { assert!( encoded_dir.is_dir(), - "Directory should be Key%COLON%Script/ (suffix stripped, encoded): {}", + "Directory should be Key_Script/ (suffix stripped, slugified): {}", encoded_dir.display() ); assert!( @@ -1375,16 +1375,16 @@ fn add_child_to_encoded_script_strips_suffix_correctly() { // Script → init.server.luau (preserves class) let init_file = encoded_dir.join("init.server.luau"); - assert_file_exists(&init_file, "init.server.luau inside encoded directory"); + assert_file_exists(&init_file, "init.server.luau inside slugified directory"); let child_file = encoded_dir.join("ScriptChild.luau"); - assert_file_exists(&child_file, "Child inside encoded script directory"); + assert_file_exists(&child_file, "Child inside slugified script directory"); }); } -/// Test 24: Adding a child to a non-script standalone instance with Rojo-encoded -/// characters (e.g., `What%QUESTION%Model.model.json5`) creates the directory -/// using the encoded name with the compound extension stripped. +/// Test 24: Adding a child to a non-script standalone instance with slugified +/// characters (e.g., `What_Model.model.json5`) creates the directory +/// using the slugified name with the compound extension stripped. /// /// This exercises `convert_standalone_instance_to_directory`. #[test] @@ -1392,14 +1392,11 @@ fn add_child_to_encoded_model_creates_encoded_directory() { run_serve_test("syncback_encoded_names", |session, _redactions| { let (session_id, encoded_model_id) = get_encoded_names_instance(&session, "What?Model"); - let standalone_file = session - .path() - .join("src") - .join("What%QUESTION%Model.model.json5"); - let encoded_dir = session.path().join("src").join("What%QUESTION%Model"); + let standalone_file = session.path().join("src").join("What_Model.model.json5"); + let encoded_dir = session.path().join("src").join("What_Model"); let decoded_dir = session.path().join("src").join("What?Model"); - assert_file_exists(&standalone_file, "Encoded model file before child add"); + assert_file_exists(&standalone_file, "Slugified model file before child add"); let mut properties = HashMap::new(); properties.insert( @@ -1428,7 +1425,7 @@ fn add_child_to_encoded_model_creates_encoded_directory() { assert!( encoded_dir.is_dir(), - "Directory should use encoded name (What%QUESTION%Model/): {}", + "Directory should use slugified name (What_Model/): {}", encoded_dir.display() ); assert!( @@ -1439,32 +1436,32 @@ fn add_child_to_encoded_model_creates_encoded_directory() { // init.meta.json5 should exist inside the directory (converted from .model.json5) let init_meta = encoded_dir.join("init.meta.json5"); - assert_file_exists(&init_meta, "init.meta.json5 inside encoded model directory"); + assert_file_exists( + &init_meta, + "init.meta.json5 inside slugified model directory", + ); let child_file = encoded_dir.join("ModelChild.luau"); - assert_file_exists(&child_file, "Child inside encoded model directory"); + assert_file_exists(&child_file, "Child inside slugified model directory"); }); } -/// Test 25: Non-Source property update on a standalone script with Rojo-encoded -/// name creates the adjacent meta file using the encoded filesystem name, +/// Test 25: Non-Source property update on a standalone script with slugified +/// name creates the adjacent meta file using the slugified filesystem name, /// not the decoded instance name. /// -/// e.g., `What%QUESTION%Module.meta.json5` NOT `What?Module.meta.json5` +/// e.g., `What_Module.meta.json5` NOT `What?Module.meta.json5` #[test] fn property_update_on_encoded_script_uses_encoded_meta_path() { run_serve_test("syncback_encoded_names", |session, _redactions| { let (session_id, encoded_module_id) = get_encoded_names_instance(&session, "What?Module"); - let script_file = session.path().join("src").join("What%QUESTION%Module.luau"); - let encoded_meta = session - .path() - .join("src") - .join("What%QUESTION%Module.meta.json5"); + let script_file = session.path().join("src").join("What_Module.luau"); + let encoded_meta = session.path().join("src").join("What_Module.meta.json5"); let decoded_meta = session.path().join("src").join("What?Module.meta.json5"); - assert_file_exists(&script_file, "Encoded script file before property update"); - assert_not_exists(&encoded_meta, "Meta file before property update"); + assert_file_exists(&script_file, "Slugified script file before property update"); + assert_file_exists(&encoded_meta, "Meta file exists from fixture (name field)"); let mut attrs = rbx_dom_weak::types::Attributes::new(); attrs.insert("EncodedAttr".to_string(), Variant::Float64(99.0)); @@ -1483,10 +1480,10 @@ fn property_update_on_encoded_script_uses_encoded_meta_path() { }, ); - // Meta file should use encoded name + // Meta file should use slugified name assert_file_exists( &encoded_meta, - "Meta file should use encoded name (What%QUESTION%Module.meta.json5)", + "Meta file should use slugified name (What_Module.meta.json5)", ); assert!( !decoded_meta.exists(), @@ -2453,15 +2450,12 @@ fn encoded_name_rapid_rename_chain() { wait_for_settle(); - // Final file should use encoded name: Done%QUESTION%Now.luau - let final_file = src.join("Done%QUESTION%Now.luau"); + // Final file should use slugified name: Done_Now.luau + let final_file = src.join("Done_Now.luau"); assert_file_exists(&final_file, "Final encoded file"); // Original should be gone - assert_not_exists( - &src.join("What%QUESTION%Module.luau"), - "Original encoded file", - ); + assert_not_exists(&src.join("What_Module.luau"), "Original encoded file"); }); } @@ -4151,8 +4145,8 @@ fn encoded_rename_and_source_rapid() { } wait_for_settle(); - // Final: Final:Encoded -> Final%COLON%Encoded.luau - let final_file = src.join("Final%COLON%Encoded.luau"); + // Final: Final:Encoded -> Final_Encoded.luau + let final_file = src.join("Final_Encoded.luau"); assert_file_exists(&final_file, "Final encoded file"); let content = fs::read_to_string(&final_file).unwrap(); assert!( @@ -4193,8 +4187,8 @@ fn encoded_rename_and_classchange_rapid() { } wait_for_settle(); - // Final: Enc:Final as Script -> Enc%COLON%Final.server.luau - let final_file = src.join("Enc%COLON%Final.server.luau"); + // Final: Enc:Final as Script -> Enc_Final.server.luau + let final_file = src.join("Enc_Final.server.luau"); assert_file_exists(&final_file, "Final encoded+class file"); }); } @@ -4229,8 +4223,8 @@ fn encoded_all_three_combined_rapid() { } wait_for_settle(); - // Final: End?Now as LocalScript -> End%QUESTION%Now.local.luau - let final_file = src.join("End%QUESTION%Now.local.luau"); + // Final: End?Now as LocalScript -> End_Now.local.luau + let final_file = src.join("End_Now.local.luau"); assert_file_exists(&final_file, "Final all-three encoded file"); let content = fs::read_to_string(&final_file).unwrap(); assert!( diff --git a/vscode-rojo b/vscode-rojo index 035c12d9..2e37c0ee 160000 --- a/vscode-rojo +++ b/vscode-rojo @@ -1 +1 @@ -Subproject commit 035c12d9ed499942a1e8b024d61b99675eab232d +Subproject commit 2e37c0ee0f6801e4a159289a2e2a62292fe7e111