Make the name field in projects optional (#870)

Closes #858.

If a project is named `default.project.json`, it acts as an `init` file
and gains the name of the folder it's inside of. If it is named
something other than `default.project.json`, it gains the name of the
file with `.project.json` trimmed off. So e.g. `foo.project.json`
becomes `foo`.

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<String>,
 
     /// The tree of instances described by this project. Projects always
     /// describe at least one instance.

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<u16> {
@@ -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]

src/snapshot_middleware/mod.rs

@@ -66,7 +66,13 @@ pub fn snapshot_from_vfs(
             for rule in default_sync_rules() {
                 if rule.matches(&init_path) {
                     return match rule.middleware {
-                        Middleware::Project => snapshot_project(context, vfs, &init_path),
+                        Middleware::Project => {
+                            let name = init_path
+                                .parent()
+                                .and_then(Path::file_name)
+                                .and_then(|s| s.to_str()).expect("default.project.json should be inside a folder with a unicode name");
+                            snapshot_project(context, vfs, &init_path, name)
+                        }
 
                         Middleware::ModuleScript => {
                             snapshot_lua_init(context, vfs, &init_path, ScriptType::Module)
@@ -218,9 +224,7 @@ impl Middleware {
             Self::ServerScript => snapshot_lua(context, vfs, path, name, ScriptType::Server),
             Self::ClientScript => snapshot_lua(context, vfs, path, name, ScriptType::Client),
             Self::ModuleScript => snapshot_lua(context, vfs, path, name, ScriptType::Module),
-            // At the moment, snapshot_project does not use `name` so we
-            // don't provide it.
-            Self::Project => snapshot_project(context, vfs, path),
+            Self::Project => snapshot_project(context, vfs, path, name),
             Self::Rbxm => snapshot_rbxm(context, vfs, path, name),
             Self::Rbxmx => snapshot_rbxmx(context, vfs, path, name),
             Self::Toml => snapshot_toml(context, vfs, path, name),
@@ -280,8 +284,7 @@ fn default_sync_rules() -> &'static [SyncRule] {
             sync_rule!("*.client.lua", ClientScript, ".client.lua"),
             sync_rule!("*.client.luau", ClientScript, ".client.luau"),
             sync_rule!("*.{lua,luau}", ModuleScript),
-            // Project middleware doesn't use the file name.
-            sync_rule!("*.project.json", Project),
+            sync_rule!("*.project.json", Project, ".project.json"),
             sync_rule!("*.model.json", JsonModel, ".model.json"),
             sync_rule!("*.json", Json, ".json", "*.meta.json"),
             sync_rule!("*.toml", Toml),

src/snapshot_middleware/project.rs

@@ -19,9 +19,11 @@ pub fn snapshot_project(
     context: &InstanceContext,
     vfs: &Vfs,
     path: &Path,
+    name: &str,
 ) -> anyhow::Result<Option<InstanceSnapshot>> {
     let project = Project::load_from_slice(&vfs.read(path)?, path)
         .with_context(|| format!("File was not a valid Rojo project: {}", path.display()))?;
+    let project_name = project.name.as_deref().unwrap_or(name);
 
     let mut context = context.clone();
     context.clear_sync_rules();
@@ -45,7 +47,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
@@ -354,12 +356,16 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
-            snapshot_project(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
-                .expect("snapshot error")
-                .expect("snapshot returned no instances");
+        let instance_snapshot = snapshot_project(
+            &InstanceContext::default(),
+            &vfs,
+            Path::new("/foo"),
+            "NOT_IN_SNAPSHOT",
+        )
+        .expect("snapshot error")
+        .expect("snapshot returned no instances");
 
         insta::assert_yaml_snapshot!(instance_snapshot);
     }
@@ -384,12 +390,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo/hello.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -422,12 +429,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -458,12 +466,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -495,12 +504,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -529,12 +539,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo/default.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -570,12 +581,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo/default.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -615,12 +627,13 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo/default.project.json"),
+            "NOT_IN_SNAPSHOT",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
@@ -665,12 +678,46 @@ mod test {
         )
         .unwrap();
 
-        let mut vfs = Vfs::new(imfs);
+        let vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
-            &mut vfs,
+            &vfs,
             Path::new("/foo/default.project.json"),
+            "NOT_IN_SNAPSHOT",
+        )
+        .expect("snapshot error")
+        .expect("snapshot returned no instances");
+
+        insta::assert_yaml_snapshot!(instance_snapshot);
+    }
+
+    #[test]
+    fn no_name_project() {
+        let _ = env_logger::try_init();
+
+        let mut imfs = InMemoryFs::new();
+        imfs.load_snapshot(
+            "/foo",
+            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("/foo/default.project.json"),
+            "no_name_project",
         )
         .expect("snapshot error")
         .expect("snapshot returned no instances");

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap

@@ -0,0 +1,19 @@
+---
+source: src/snapshot_middleware/project.rs
+assertion_line: 725
+expression: instance_snapshot
+---
+snapshot_id: "00000000000000000000000000000000"
+metadata:
+  ignore_unknown_instances: true
+  instigating_source:
+    Path: /foo/default.project.json
+  relevant_paths:
+    - /foo/default.project.json
+  context:
+    emit_legacy_scripts: true
+name: no_name_project
+class_name: Model
+properties: {}
+children: []
+