diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f0f7a787..4dd8c0c40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Rojo Changelog ## Unreleased Changes +* Made the `name` field optional on project files ([#870]) + Files named `default.project.json` inherit the name of the folder they're in and all other projects + are named as expect (e.g. `foo.project.json` becomes an Instance named `foo`) + + There is no change in behavior if `name` is set. * Fixed incorrect results when building model pivots ([#865]) * Fixed incorrect results when serving model pivots ([#868]) * Rojo now converts any line endings to LF, preventing spurious diffs when syncing Lua files on Windows ([#854]) @@ -15,6 +20,7 @@ [#854]: https://github.com/rojo-rbx/rojo/pull/854 [#865]: https://github.com/rojo-rbx/rojo/pull/865 [#868]: https://github.com/rojo-rbx/rojo/pull/868 +[#870]: https://github.com/rojo-rbx/rojo/pull/870 ## [7.4.0] - January 16, 2024 * Improved the visualization for array properties like Tags ([#829]) diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_default_project.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_default_project.snap new file mode 100644 index 000000000..fbbd49116 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_default_project.snap @@ -0,0 +1,22 @@ +--- +source: tests/tests/build.rs +expression: contents +--- + + + + top-level + + + + second-level + + + + third-level + 1337 + + + + + diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_project.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_project.snap new file mode 100644 index 000000000..588979334 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_project.snap @@ -0,0 +1,22 @@ +--- +source: tests/tests/build.rs +expression: contents +--- + + + + no_name_project + + + + second-level + + + + bool_value + true + + + + + diff --git a/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_top_level_project.snap b/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_top_level_project.snap new file mode 100644 index 000000000..5f82ec821 --- /dev/null +++ b/rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_top_level_project.snap @@ -0,0 +1,13 @@ +--- +source: tests/tests/build.rs +assertion_line: 104 +expression: contents +--- + + + + no_name_top_level_project + If this isn't named `no_name_top_level_project`, something went wrong! + + + diff --git a/rojo-test/build-tests/no_name_default_project/default.project.json b/rojo-test/build-tests/no_name_default_project/default.project.json new file mode 100644 index 000000000..1c2a1400d --- /dev/null +++ b/rojo-test/build-tests/no_name_default_project/default.project.json @@ -0,0 +1,9 @@ +{ + "name": "top-level", + "tree": { + "$className": "Folder", + "second-level": { + "$path": "src" + } + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/no_name_default_project/src/third-level/default.project.json b/rojo-test/build-tests/no_name_default_project/src/third-level/default.project.json new file mode 100644 index 000000000..e7b87be1c --- /dev/null +++ b/rojo-test/build-tests/no_name_default_project/src/third-level/default.project.json @@ -0,0 +1,8 @@ +{ + "tree": { + "$className": "IntValue", + "$properties": { + "Value": 1337 + } + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/no_name_project/default.project.json b/rojo-test/build-tests/no_name_project/default.project.json new file mode 100644 index 000000000..93ef2e8ce --- /dev/null +++ b/rojo-test/build-tests/no_name_project/default.project.json @@ -0,0 +1,9 @@ +{ + "name": "no_name_project", + "tree": { + "$className": "Folder", + "second-level": { + "$path": "src" + } + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/no_name_project/src/bool_value.project.json b/rojo-test/build-tests/no_name_project/src/bool_value.project.json new file mode 100644 index 000000000..6be58a3ce --- /dev/null +++ b/rojo-test/build-tests/no_name_project/src/bool_value.project.json @@ -0,0 +1,8 @@ +{ + "tree": { + "$className": "BoolValue", + "$properties": { + "Value": true + } + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/no_name_top_level_project/default.project.json b/rojo-test/build-tests/no_name_top_level_project/default.project.json new file mode 100644 index 000000000..845ae3725 --- /dev/null +++ b/rojo-test/build-tests/no_name_top_level_project/default.project.json @@ -0,0 +1,8 @@ +{ + "tree": { + "$className": "StringValue", + "$properties": { + "Value": "If this isn't named `no_name_top_level_project`, something went wrong!" + } + } +} \ No newline at end of file diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_all.snap new file mode 100644 index 000000000..a8179a919 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_all.snap @@ -0,0 +1,39 @@ +--- +source: tests/tests/serve.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: true + Name: top-level + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: + - id-4 + ClassName: Folder + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: second-level + Parent: id-2 + Properties: {} + id-4: + Children: [] + ClassName: IntValue + Id: id-4 + Metadata: + ignoreUnknownInstances: true + Name: third-level + Parent: id-3 + Properties: + Value: + Int64: 1337 +messageCursor: 0 +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap new file mode 100644 index 000000000..954ba6229 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap @@ -0,0 +1,14 @@ +--- +source: tests/tests/serve.rs +assertion_line: 316 +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: top-level +protocolVersion: 4 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_all.snap new file mode 100644 index 000000000..1b398a928 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_all.snap @@ -0,0 +1,40 @@ +--- +source: tests/tests/serve.rs +assertion_line: 338 +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: true + Name: no_name_project + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: + - id-4 + ClassName: Folder + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: second-level + Parent: id-2 + Properties: {} + id-4: + Children: [] + ClassName: BoolValue + Id: id-4 + Metadata: + ignoreUnknownInstances: true + Name: bool_value + Parent: id-3 + Properties: + Value: + Bool: true +messageCursor: 0 +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap new file mode 100644 index 000000000..d9772de1c --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap @@ -0,0 +1,14 @@ +--- +source: tests/tests/serve.rs +assertion_line: 335 +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: no_name_project +protocolVersion: 4 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_all.snap new file mode 100644 index 000000000..f26e1c6c0 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_all.snap @@ -0,0 +1,20 @@ +--- +source: tests/tests/serve.rs +assertion_line: 306 +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: [] + ClassName: StringValue + Id: id-2 + Metadata: + ignoreUnknownInstances: true + Name: no_name_top_level_project + Parent: "00000000000000000000000000000000" + Properties: + Value: + String: "If this isn't named `no_name_top_level_project`, something went wrong!" +messageCursor: 0 +sessionId: id-1 + diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap new file mode 100644 index 000000000..fd7f94ebb --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap @@ -0,0 +1,14 @@ +--- +source: tests/tests/serve.rs +assertion_line: 300 +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: no_name_top_level_project +protocolVersion: 4 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 + diff --git a/rojo-test/serve-tests/no_name_default_project/default.project.json b/rojo-test/serve-tests/no_name_default_project/default.project.json new file mode 100644 index 000000000..1c2a1400d --- /dev/null +++ b/rojo-test/serve-tests/no_name_default_project/default.project.json @@ -0,0 +1,9 @@ +{ + "name": "top-level", + "tree": { + "$className": "Folder", + "second-level": { + "$path": "src" + } + } +} \ No newline at end of file diff --git a/rojo-test/serve-tests/no_name_default_project/src/third-level/default.project.json b/rojo-test/serve-tests/no_name_default_project/src/third-level/default.project.json new file mode 100644 index 000000000..e7b87be1c --- /dev/null +++ b/rojo-test/serve-tests/no_name_default_project/src/third-level/default.project.json @@ -0,0 +1,8 @@ +{ + "tree": { + "$className": "IntValue", + "$properties": { + "Value": 1337 + } + } +} \ No newline at end of file diff --git a/rojo-test/serve-tests/no_name_project/default.project.json b/rojo-test/serve-tests/no_name_project/default.project.json new file mode 100644 index 000000000..93ef2e8ce --- /dev/null +++ b/rojo-test/serve-tests/no_name_project/default.project.json @@ -0,0 +1,9 @@ +{ + "name": "no_name_project", + "tree": { + "$className": "Folder", + "second-level": { + "$path": "src" + } + } +} \ No newline at end of file diff --git a/rojo-test/serve-tests/no_name_project/src/bool_value.project.json b/rojo-test/serve-tests/no_name_project/src/bool_value.project.json new file mode 100644 index 000000000..6be58a3ce --- /dev/null +++ b/rojo-test/serve-tests/no_name_project/src/bool_value.project.json @@ -0,0 +1,8 @@ +{ + "tree": { + "$className": "BoolValue", + "$properties": { + "Value": true + } + } +} \ No newline at end of file diff --git a/rojo-test/serve-tests/no_name_top_level_project/default.project.json b/rojo-test/serve-tests/no_name_top_level_project/default.project.json new file mode 100644 index 000000000..845ae3725 --- /dev/null +++ b/rojo-test/serve-tests/no_name_top_level_project/default.project.json @@ -0,0 +1,8 @@ +{ + "tree": { + "$className": "StringValue", + "$properties": { + "Value": "If this isn't named `no_name_top_level_project`, something went wrong!" + } + } +} \ No newline at end of file diff --git a/src/project.rs b/src/project.rs index 93a3cb218..125b0e560 100644 --- a/src/project.rs +++ b/src/project.rs @@ -39,7 +39,7 @@ enum Error { #[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct Project { /// The name of the top-level instance described by the project. - pub name: String, + pub name: Option, /// The tree of instances described by this project. Projects always /// describe at least one instance. diff --git a/src/serve_session.rs b/src/serve_session.rs index 85ae77934..dbc841d74 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -110,7 +110,7 @@ impl ServeSession { log::debug!("Loading project file from {}", project_path.display()); - let root_project = match vfs.read(&project_path).with_not_found()? { + let mut root_project = match vfs.read(&project_path).with_not_found()? { Some(contents) => Project::load_from_slice(&contents, &project_path)?, None => { return Err(ServeSessionError::NoProjectFound { @@ -118,6 +118,35 @@ impl ServeSession { }); } }; + if root_project.name.is_none() { + if let Some(file_name) = project_path.file_name().and_then(|s| s.to_str()) { + if file_name == "default.project.json" { + let folder_name = project_path + .parent() + .and_then(Path::file_name) + .and_then(|s| s.to_str()); + if let Some(folder_name) = folder_name { + root_project.name = Some(folder_name.to_string()); + } else { + return Err(ServeSessionError::FolderNameInvalid { + path: project_path.to_path_buf(), + }); + } + } else if let Some(trimmed) = file_name.strip_suffix(".project.json") { + root_project.name = Some(trimmed.to_string()); + } else { + return Err(ServeSessionError::ProjectNameInvalid { + path: project_path.to_path_buf(), + }); + } + } else { + return Err(ServeSessionError::ProjectNameInvalid { + path: project_path.to_path_buf(), + }); + } + } + // Rebind it to make it no longer mutable + let root_project = root_project; let mut tree = RojoTree::new(InstanceSnapshot::new()); @@ -190,7 +219,10 @@ impl ServeSession { } pub fn project_name(&self) -> &str { - &self.root_project.name + self.root_project + .name + .as_ref() + .expect("all top-level projects must have their name set") } pub fn project_port(&self) -> Option { @@ -231,6 +263,14 @@ pub enum ServeSessionError { )] NoProjectFound { path: PathBuf }, + #[error("The folder for the provided project cannot be used as a project name: {}\n\ + Consider setting the `name` field on this project.", .path.display())] + FolderNameInvalid { path: PathBuf }, + + #[error("The file name of the provided project cannot be used as a project name: {}.\n\ + Consider setting the `name` field on this project.", .path.display())] + ProjectNameInvalid { path: PathBuf }, + #[error(transparent)] Io { #[from] diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index d1b4a2e48..f186830fa 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -22,6 +22,25 @@ pub fn snapshot_project( let project = Project::load_from_slice(&vfs.read(path)?, path) .with_context(|| format!("File was not a valid Rojo project: {}", path.display()))?; + // This is not how I would normally do this, but this is a temporary + // implementation. The one in 7.5+ is better. + let project_name = project.name.as_deref().unwrap_or_else(|| { + let file_name = path + .file_name() + .and_then(|s| s.to_str()) + .expect("project file names should be valid UTF-8"); + if file_name == "default.project.json" { + path.parent() + .and_then(Path::file_name) + .and_then(|s| s.to_str()) + .expect("default.project.json should be inside a folder with a valid UTF-8 name") + } else { + file_name + .strip_suffix(".project.json") + .expect("project file names should end with .project.json") + } + }); + let mut context = context.clone(); let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule { @@ -37,7 +56,7 @@ pub fn snapshot_project( .unwrap(), ); - match snapshot_project_node(&context, path, &project.name, &project.tree, vfs, None)? { + match snapshot_project_node(&context, path, project_name, &project.tree, vfs, None)? { Some(found_snapshot) => { let mut snapshot = found_snapshot; // Setting the instigating source to the project file path is a little @@ -669,4 +688,36 @@ mod test { insta::assert_yaml_snapshot!(instance_snapshot); } + + #[test] + fn no_name_project() { + let _ = env_logger::try_init(); + + let mut imfs = InMemoryFs::new(); + imfs.load_snapshot( + "/no_name_project", + VfsSnapshot::dir(hashmap! { + "default.project.json" => VfsSnapshot::file(r#" + { + "tree": { + "$className": "Model" + } + } + "#), + }), + ) + .unwrap(); + + let vfs = Vfs::new(imfs); + + let instance_snapshot = snapshot_project( + &InstanceContext::default(), + &vfs, + Path::new("/no_name_project/default.project.json"), + ) + .expect("snapshot error") + .expect("snapshot returned no instances"); + + insta::assert_yaml_snapshot!(instance_snapshot); + } } 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 new file mode 100644 index 000000000..f9c238cc7 --- /dev/null +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap @@ -0,0 +1,18 @@ +--- +source: src/snapshot_middleware/project.rs +expression: instance_snapshot +--- +snapshot_id: "00000000000000000000000000000000" +metadata: + ignore_unknown_instances: true + instigating_source: + Path: /no_name_project/default.project.json + relevant_paths: + - /no_name_project/default.project.json + context: + emit_legacy_scripts: true +name: no_name_project +class_name: Model +properties: {} +children: [] + diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 01e87da73..f2b1d5dfc 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -59,6 +59,9 @@ gen_build_tests! { txt_in_folder, unresolved_values, weldconstraint, + no_name_default_project, + no_name_project, + no_name_top_level_project, } fn run_build_test(test_name: &str) { @@ -70,7 +73,7 @@ fn run_build_test(test_name: &str) { let output_path = output_dir.path().join(format!("{}.rbxmx", test_name)); let output = Command::new(ROJO_PATH) - .args(&[ + .args([ "build", input_path.to_str().unwrap(), "-o", diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index d55d339bb..4f358b27c 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -255,3 +255,57 @@ fn add_optional_folder() { ); }); } + +#[test] +fn no_name_default_project() { + run_serve_test("no_name_default_project", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!( + "no_name_default_project_info", + redactions.redacted_yaml(info) + ); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "no_name_default_project_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} + +#[test] +fn no_name_project() { + run_serve_test("no_name_project", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!("no_name_project_info", redactions.redacted_yaml(info)); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "no_name_project_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} + +#[test] +fn no_name_top_level_project() { + run_serve_test("no_name_top_level_project", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!( + "no_name_top_level_project_info", + redactions.redacted_yaml(info) + ); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "no_name_top_level_project_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +}