Flatten snapshot middleware to be much simpler (#324)

* First take at flattening middleware for simpler code and better perf

* Undo debug prints

* Fix using wrong path in snapshot_from_vfs

* Disable some broken tests

* Re-enable (mistakenly?) disabled CSV test

* Fix some tests

* Update project file tests

* Fix benchmark

benches/build.rs

@@ -35,6 +35,7 @@ fn place_setup<P: AsRef<Path>>(input_path: P) -> (TempDir, BuildCommand) {
 
     let options = BuildCommand {
         project: input,
+        watch: false,
         output,
     };
 

src/snapshot_middleware/csv.rs

@@ -8,27 +8,15 @@ use serde::Serialize;
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
 use super::{
-    error::SnapshotError,
+    error::SnapshotError, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult,
-    meta_file::AdjacentMetadata,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
 };
 
-pub struct SnapshotCsv;
+pub fn snapshot_csv(
-
+    _context: &InstanceContext,
-impl SnapshotMiddleware for SnapshotCsv {
+    vfs: &Vfs,
-    fn from_vfs(_context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
+    path: &Path,
-        let meta = vfs.metadata(path)?;
+    instance_name: &str,
-
+) -> SnapshotInstanceResult {
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".csv") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
-
     let meta_path = path.with_file_name(format!("{}.meta.json", instance_name));
     let contents = vfs.read(path)?;
 
@@ -55,7 +43,6 @@ impl SnapshotMiddleware for SnapshotCsv {
     }
 
     Ok(Some(snapshot))
-    }
 }
 
 /// Struct that holds any valid row from a Roblox CSV translation table.
@@ -156,8 +143,12 @@ Ack,Ack!,,An exclamation of despair,¡Ay!"#,
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_csv(
-            SnapshotCsv::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.csv"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo.csv"),
+            "foo",
+        )
         .unwrap()
         .unwrap();
 
@@ -184,9 +175,15 @@ Ack,Ack!,,An exclamation of despair,¡Ay!"#,
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_csv(
-            SnapshotCsv::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.csv"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo.csv"),
+            "foo",
+        )
         .unwrap()
         .unwrap();
+
+        insta::assert_yaml_snapshot!(instance_snapshot);
     }
 }

src/snapshot_middleware/dir.rs

@@ -5,22 +5,11 @@ use memofs::{DirEntry, IoResultExt, Vfs};
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
 use super::{
-    error::SnapshotError,
+    error::SnapshotError, meta_file::DirectoryMetadata, middleware::SnapshotInstanceResult,
-    meta_file::DirectoryMetadata,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
     snapshot_from_vfs,
 };
 
-pub struct SnapshotDir;
+pub fn snapshot_dir(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-
-impl SnapshotMiddleware for SnapshotDir {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_file() {
-            return Ok(None);
-        }
-
     let passes_filter_rules = |child: &DirEntry| {
         context
             .path_ignore_rules
@@ -79,7 +68,6 @@ impl SnapshotMiddleware for SnapshotDir {
     }
 
     Ok(Some(snapshot))
-    }
 }
 
 #[cfg(test)]
@@ -98,7 +86,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot =
-            SnapshotDir::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            snapshot_dir(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
                 .unwrap()
                 .unwrap();
 
@@ -119,7 +107,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot =
-            SnapshotDir::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            snapshot_dir(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
                 .unwrap()
                 .unwrap();
 

src/snapshot_middleware/json.rs

@@ -10,36 +10,15 @@ use crate::{
 };
 
 use super::{
-    error::SnapshotError,
+    error::SnapshotError, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult,
-    meta_file::AdjacentMetadata,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
 };
 
-/// Catch-all middleware for snapshots on JSON files that aren't used for other
+pub fn snapshot_json(
-/// features, like Rojo projects, JSON models, or meta files.
+    context: &InstanceContext,
-pub struct SnapshotJson;
+    vfs: &Vfs,
-
+    path: &Path,
-impl SnapshotMiddleware for SnapshotJson {
+    instance_name: &str,
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
+) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        // FIXME: This middleware should not need to know about the .meta.json
-        // middleware. Should there be a way to signal "I'm not returning an
-        // instance and no one should"?
-        if match_file_name(path, ".meta.json").is_some() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".json") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
-
     let contents = vfs.read(path)?;
 
     let value: serde_json::Value = serde_json::from_slice(&contents)
@@ -72,7 +51,6 @@ impl SnapshotMiddleware for SnapshotJson {
     }
 
     Ok(Some(snapshot))
-    }
 }
 
 fn json_to_lua(value: serde_json::Value) -> Statement {
@@ -129,10 +107,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs.clone());
 
-        let instance_snapshot = SnapshotJson::from_vfs(
+        let instance_snapshot = snapshot_json(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.json"),
+            "foo",
         )
         .unwrap()
         .unwrap();

src/snapshot_middleware/json_model.rs

@@ -7,27 +7,14 @@ use serde::Deserialize;
 
 use crate::snapshot::{InstanceContext, InstanceSnapshot};
 
-use super::{
+use super::{error::SnapshotError, middleware::SnapshotInstanceResult};
-    error::SnapshotError,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
-};
-
-pub struct SnapshotJsonModel;
-
-impl SnapshotMiddleware for SnapshotJsonModel {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".model.json") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
 
+pub fn snapshot_json_model(
+    context: &InstanceContext,
+    vfs: &Vfs,
+    path: &Path,
+    instance_name: &str,
+) -> SnapshotInstanceResult {
     let contents = vfs.read(path)?;
     let instance: JsonModel = serde_json::from_slice(&contents)
         .map_err(|source| SnapshotError::malformed_model_json(source, path))?;
@@ -59,16 +46,6 @@ impl SnapshotMiddleware for SnapshotJsonModel {
         .context(context);
 
     Ok(Some(snapshot))
-    }
-}
-
-fn match_trailing<'a>(input: &'a str, trailer: &str) -> Option<&'a str> {
-    if input.ends_with(trailer) {
-        let end = input.len().saturating_sub(trailer.len());
-        Some(&input[..end])
-    } else {
-        None
-    }
 }
 
 #[derive(Debug, Deserialize)]
@@ -164,10 +141,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotJsonModel::from_vfs(
+        let instance_snapshot = snapshot_json_model(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.model.json"),
+            "foo",
         )
         .unwrap()
         .unwrap();

src/snapshot_middleware/lua.rs

@@ -7,50 +7,12 @@ use rbx_dom_weak::RbxValue;
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
 use super::{
-    dir::SnapshotDir,
+    dir::snapshot_dir, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult,
-    meta_file::AdjacentMetadata,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
     util::match_trailing,
 };
 
-pub struct SnapshotLua;
-
-impl SnapshotMiddleware for SnapshotLua {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let file_name = path.file_name().unwrap().to_string_lossy();
-
-        // These paths alter their parent instance, so we don't need to turn
-        // them into a script instance here.
-        match &*file_name {
-            "init.lua" | "init.server.lua" | "init.client.lua" => return Ok(None),
-            _ => {}
-        }
-
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_file() {
-            snapshot_lua_file(context, vfs, path)
-        } else {
-            // At this point, our entry is definitely a directory!
-
-            if let Some(snapshot) = snapshot_init(context, vfs, path, "init.lua")? {
-                // An `init.lua` file turns its parent into a ModuleScript
-                Ok(Some(snapshot))
-            } else if let Some(snapshot) = snapshot_init(context, vfs, path, "init.server.lua")? {
-                // An `init.server.lua` file turns its parent into a Script
-                Ok(Some(snapshot))
-            } else if let Some(snapshot) = snapshot_init(context, vfs, path, "init.client.lua")? {
-                // An `init.client.lua` file turns its parent into a LocalScript
-                Ok(Some(snapshot))
-            } else {
-                Ok(None)
-            }
-        }
-    }
-}
-
 /// Core routine for turning Lua files into snapshots.
-fn snapshot_lua_file(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
+pub fn snapshot_lua(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
     let file_name = path.file_name().unwrap().to_string_lossy();
 
     let (class_name, instance_name) = if let Some(name) = match_trailing(&file_name, ".server.lua")
@@ -100,17 +62,14 @@ fn snapshot_lua_file(context: &InstanceContext, vfs: &Vfs, path: &Path) -> Snaps
 ///
 /// Scripts named `init.lua`, `init.server.lua`, or `init.client.lua` usurp
 /// their parents, which acts similarly to `__init__.py` from the Python world.
-fn snapshot_init(
+pub fn snapshot_lua_init(
     context: &InstanceContext,
     vfs: &Vfs,
-    folder_path: &Path,
+    init_path: &Path,
-    init_name: &str,
 ) -> SnapshotInstanceResult {
-    let init_path = folder_path.join(init_name);
+    let folder_path = init_path.parent().unwrap();
+    let dir_snapshot = snapshot_dir(context, vfs, folder_path)?.unwrap();
 
-    if vfs.metadata(&init_path).with_not_found()?.is_some() {
-        if let Some(dir_snapshot) = SnapshotDir::from_vfs(context, vfs, folder_path)? {
-            if let Some(mut init_snapshot) = snapshot_lua_file(context, vfs, &init_path)? {
     if dir_snapshot.class_name != "Folder" {
         panic!(
             "init.lua, init.server.lua, and init.client.lua can \
@@ -119,16 +78,13 @@ fn snapshot_init(
         );
     }
 
+    let mut init_snapshot = snapshot_lua(context, vfs, init_path)?.unwrap();
+
     init_snapshot.name = dir_snapshot.name;
     init_snapshot.children = dir_snapshot.children;
     init_snapshot.metadata = dir_snapshot.metadata;
 
-                return Ok(Some(init_snapshot));
+    Ok(Some(init_snapshot))
-            }
-        }
-    }
-
-    Ok(None)
 }
 
 #[cfg(test)]
@@ -146,7 +102,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot =
-            SnapshotLua::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua"))
+            snapshot_lua(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua"))
                 .unwrap()
                 .unwrap();
 
@@ -161,7 +117,7 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotLua::from_vfs(
+        let instance_snapshot = snapshot_lua(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.server.lua"),
@@ -180,7 +136,7 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotLua::from_vfs(
+        let instance_snapshot = snapshot_lua(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.client.lua"),
@@ -191,6 +147,7 @@ mod test {
         insta::assert_yaml_snapshot!(instance_snapshot);
     }
 
+    #[ignore = "init.lua functionality has moved to the root snapshot function"]
     #[test]
     fn init_module_from_vfs() {
         let mut imfs = InMemoryFs::new();
@@ -205,7 +162,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot =
-            SnapshotLua::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/root"))
+            snapshot_lua(&InstanceContext::default(), &mut vfs, Path::new("/root"))
                 .unwrap()
                 .unwrap();
 
@@ -232,7 +189,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot =
-            SnapshotLua::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua"))
+            snapshot_lua(&InstanceContext::default(), &mut vfs, Path::new("/foo.lua"))
                 .unwrap()
                 .unwrap();
 
@@ -258,7 +215,7 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotLua::from_vfs(
+        let instance_snapshot = snapshot_lua(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.server.lua"),
@@ -290,7 +247,7 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotLua::from_vfs(
+        let instance_snapshot = snapshot_lua(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/bar.server.lua"),

src/snapshot_middleware/middleware.rs

@@ -1,13 +1,5 @@
-use std::path::Path;
+use crate::snapshot::InstanceSnapshot;
-
-use memofs::Vfs;
-
-use crate::snapshot::{InstanceContext, InstanceSnapshot};
 
 use super::error::SnapshotError;
 
 pub type SnapshotInstanceResult = Result<Option<InstanceSnapshot>, SnapshotError>;
-
-pub trait SnapshotMiddleware {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult;
-}

src/snapshot_middleware/mod.rs

@@ -12,7 +12,6 @@ mod lua;
 mod meta_file;
 mod middleware;
 mod project;
-mod rbxlx;
 mod rbxm;
 mod rbxmx;
 mod txt;
@@ -20,59 +19,87 @@ mod util;
 
 use std::path::Path;
 
-use memofs::Vfs;
+use memofs::{IoResultExt, Vfs};
 
 use crate::snapshot::InstanceContext;
 
 use self::{
-    csv::SnapshotCsv,
+    csv::snapshot_csv,
-    dir::SnapshotDir,
+    dir::snapshot_dir,
-    json::SnapshotJson,
+    json::snapshot_json,
-    json_model::SnapshotJsonModel,
+    json_model::snapshot_json_model,
-    lua::SnapshotLua,
+    lua::{snapshot_lua, snapshot_lua_init},
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
+    middleware::SnapshotInstanceResult,
-    project::SnapshotProject,
+    project::snapshot_project,
-    rbxlx::SnapshotRbxlx,
+    rbxm::snapshot_rbxm,
-    rbxm::SnapshotRbxm,
+    rbxmx::snapshot_rbxmx,
-    rbxmx::SnapshotRbxmx,
+    txt::snapshot_txt,
-    txt::SnapshotTxt,
+    util::match_file_name,
 };
 
 pub use self::error::*;
 pub use self::project::snapshot_project_node;
 
-macro_rules! middlewares {
+pub fn snapshot_from_vfs(
-    ( $($middleware: ident,)* ) => {
-        /// Generates a snapshot of instances from the given path.
-        pub fn snapshot_from_vfs(
     context: &InstanceContext,
     vfs: &Vfs,
     path: &Path,
-        ) -> SnapshotInstanceResult {
+) -> SnapshotInstanceResult {
-            $(
+    let meta = match vfs.metadata(path).with_not_found()? {
-                log::trace!("trying middleware {} on {}", stringify!($middleware), path.display());
+        Some(meta) => meta,
+        None => return Ok(None),
+    };
 
-                if let Some(snapshot) = $middleware::from_vfs(context, vfs, path)? {
+    if meta.is_dir() {
-                    log::trace!("middleware {} success on {}", stringify!($middleware), path.display());
+        let project_path = path.join("default.project.json");
-                    return Ok(Some(snapshot));
+        if vfs.metadata(&project_path).with_not_found()?.is_some() {
+            return snapshot_project(context, vfs, &project_path);
+        }
+
+        let init_path = path.join("init.lua");
+        if vfs.metadata(&init_path).with_not_found()?.is_some() {
+            return snapshot_lua_init(context, vfs, &init_path);
+        }
+
+        let init_path = path.join("init.server.lua");
+        if vfs.metadata(&init_path).with_not_found()?.is_some() {
+            return snapshot_lua_init(context, vfs, &init_path);
+        }
+
+        let init_path = path.join("init.client.lua");
+        if vfs.metadata(&init_path).with_not_found()?.is_some() {
+            return snapshot_lua_init(context, vfs, &init_path);
+        }
+
+        snapshot_dir(context, vfs, path)
+    } else {
+        if let Some(name) = match_file_name(path, ".lua") {
+            match name {
+                // init scripts are handled elsewhere and should not turn into
+                // their own children.
+                "init" | "init.client" | "init.server" => return Ok(None),
+
+                _ => return snapshot_lua(context, vfs, path),
+            }
+        } else if let Some(_name) = match_file_name(path, ".project.json") {
+            return snapshot_project(context, vfs, path);
+        } else if let Some(name) = match_file_name(path, ".model.json") {
+            return snapshot_json_model(context, vfs, path, name);
+        } else if let Some(_name) = match_file_name(path, ".meta.json") {
+            // .meta.json files do not turn into their own instances.
+            return Ok(None);
+        } else if let Some(name) = match_file_name(path, ".json") {
+            return snapshot_json(context, vfs, path, name);
+        } else if let Some(name) = match_file_name(path, ".csv") {
+            return snapshot_csv(context, vfs, path, name);
+        } else if let Some(name) = match_file_name(path, ".txt") {
+            return snapshot_txt(context, vfs, path, name);
+        } else if let Some(name) = match_file_name(path, ".rbxmx") {
+            return snapshot_rbxmx(context, vfs, path, name);
+        } else if let Some(name) = match_file_name(path, ".rbxm") {
+            return snapshot_rbxm(context, vfs, path, name);
         }
-            )*
 
-            log::trace!("no middleware returned Ok(Some)");
         Ok(None)
     }
-    };
-}
-
-middlewares! {
-    SnapshotProject,
-    SnapshotJsonModel,
-    SnapshotRbxlx,
-    SnapshotRbxmx,
-    SnapshotRbxm,
-    SnapshotLua,
-    SnapshotCsv,
-    SnapshotTxt,
-    SnapshotJson,
-    SnapshotDir,
 }

src/snapshot_middleware/project.rs

@@ -1,6 +1,6 @@
 use std::{borrow::Cow, collections::HashMap, path::Path};
 
-use memofs::{IoResultExt, Vfs};
+use memofs::Vfs;
 use rbx_reflection::{get_class_descriptor, try_resolve_value};
 
 use crate::{
@@ -10,38 +10,13 @@ use crate::{
     },
 };
 
-use super::{
+use super::{error::SnapshotError, middleware::SnapshotInstanceResult, snapshot_from_vfs};
-    error::SnapshotError,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    snapshot_from_vfs,
-};
-
-/// Handles snapshots for:
-/// * Files ending in `.project.json`
-/// * Folders containing a file named `default.project.json`
-pub struct SnapshotProject;
-
-impl SnapshotMiddleware for SnapshotProject {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_dir() {
-            let project_path = path.join("default.project.json");
-
-            match vfs.metadata(&project_path).with_not_found()? {
-                // TODO: Do we need to muck with the relevant paths if we're a
-                // project file within a folder? Should the folder path be the
-                // relevant path instead of the project file path?
-                Some(_meta) => return SnapshotProject::from_vfs(context, vfs, &project_path),
-                None => return Ok(None),
-            }
-        }
-
-        if !path.to_string_lossy().ends_with(".project.json") {
-            // This isn't a project file, so it's not our job.
-            return Ok(None);
-        }
 
+pub fn snapshot_project(
+    context: &InstanceContext,
+    vfs: &Vfs,
+    path: &Path,
+) -> SnapshotInstanceResult {
     let project = Project::load_from_slice(&vfs.read(path)?, path)
         .map_err(|err| SnapshotError::malformed_project(err, path))?;
 
@@ -54,17 +29,10 @@ impl SnapshotMiddleware for SnapshotProject {
 
     context.add_path_ignore_rules(rules);
 
-        // Snapshotting a project should always return an instance, so this
+    // TODO: If this project node is a path to an instance that Rojo doesn't
-        // unwrap is safe.
+    // understand, this may panic!
-        let mut snapshot = snapshot_project_node(
+    let mut snapshot =
-            &context,
+        snapshot_project_node(&context, path, &project.name, &project.tree, vfs, None)?.unwrap();
-            project.folder_location(),
-            &project.name,
-            &project.tree,
-            vfs,
-            None,
-        )?
-        .unwrap();
 
     // Setting the instigating source to the project file path is a little
     // coarse.
@@ -85,17 +53,18 @@ impl SnapshotMiddleware for SnapshotProject {
     snapshot.metadata.relevant_paths.push(path.to_path_buf());
 
     Ok(Some(snapshot))
-    }
 }
 
 pub fn snapshot_project_node(
     context: &InstanceContext,
-    project_folder: &Path,
+    project_path: &Path,
     instance_name: &str,
     node: &ProjectNode,
     vfs: &Vfs,
     parent_class: Option<&str>,
 ) -> SnapshotInstanceResult {
+    let project_folder = project_path.parent().unwrap();
+
     let name = Cow::Owned(instance_name.to_owned());
     let mut class_name = node
         .class_name
@@ -191,7 +160,7 @@ pub fn snapshot_project_node(
     for (child_name, child_project_node) in &node.children {
         if let Some(child) = snapshot_project_node(
             context,
-            project_folder,
+            project_path,
             child_name,
             child_project_node,
             vfs,
@@ -223,7 +192,7 @@ pub fn snapshot_project_node(
     }
 
     metadata.instigating_source = Some(InstigatingSource::ProjectNode(
-        project_folder.to_path_buf(),
+        project_path.to_path_buf(),
         instance_name.to_string(),
         node.clone(),
         parent_class.map(|name| name.to_owned()),
@@ -239,6 +208,7 @@ pub fn snapshot_project_node(
     }))
 }
 
+// #[cfg(feature = "broken-tests")]
 #[cfg(test)]
 mod test {
     use super::*;
@@ -246,6 +216,7 @@ mod test {
     use maplit::hashmap;
     use memofs::{InMemoryFs, VfsSnapshot};
 
+    #[ignore = "Functionality moved to root snapshot middleware"]
     #[test]
     fn project_from_folder() {
         let _ = env_logger::try_init();
@@ -269,7 +240,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot =
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            snapshot_project(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
                 .expect("snapshot error")
                 .expect("snapshot returned no instances");
 
@@ -298,7 +269,7 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotProject::from_vfs(
+        let instance_snapshot = snapshot_project(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo/hello.project.json"),
@@ -315,9 +286,9 @@ mod test {
 
         let mut imfs = InMemoryFs::new();
         imfs.load_snapshot(
-            "/foo",
+            "/foo.project.json",
-            VfsSnapshot::dir(hashmap! {
+            VfsSnapshot::file(
-                "default.project.json" => VfsSnapshot::file(r#"
+                r#"
                     {
                         "name": "resolved-properties",
                         "tree": {
@@ -330,15 +301,18 @@ mod test {
                             }
                         }
                     }
-                "#),
+                "#,
-            }),
+            ),
         )
         .unwrap();
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 
@@ -351,9 +325,9 @@ mod test {
 
         let mut imfs = InMemoryFs::new();
         imfs.load_snapshot(
-            "/foo",
+            "/foo.project.json",
-            VfsSnapshot::dir(hashmap! {
+            VfsSnapshot::file(
-                "default.project.json" => VfsSnapshot::file(r#"
+                r#"
                     {
                         "name": "unresolved-properties",
                         "tree": {
@@ -363,15 +337,18 @@ mod test {
                             }
                         }
                     }
-                "#),
+                "#,
-            }),
+            ),
         )
         .unwrap();
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 
@@ -384,9 +361,9 @@ mod test {
 
         let mut imfs = InMemoryFs::new();
         imfs.load_snapshot(
-            "/foo",
+            "/foo.project.json",
-            VfsSnapshot::dir(hashmap! {
+            VfsSnapshot::file(
-                "default.project.json" => VfsSnapshot::file(r#"
+                r#"
                     {
                         "name": "children",
                         "tree": {
@@ -397,15 +374,18 @@ mod test {
                             }
                         }
                     }
-                "#),
+                "#,
-            }),
+            ),
         )
         .unwrap();
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 
@@ -435,8 +415,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo/default.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 
@@ -473,8 +456,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo/default.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 
@@ -515,8 +501,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo/default.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 
@@ -562,8 +551,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_project(
-            SnapshotProject::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo/default.project.json"),
+        )
         .expect("snapshot error")
         .expect("snapshot returned no instances");
 

src/snapshot_middleware/rbxlx.rs

@@ -1,46 +0,0 @@
-use std::path::Path;
-
-use memofs::Vfs;
-
-use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
-
-use super::{
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
-};
-
-pub struct SnapshotRbxlx;
-
-impl SnapshotMiddleware for SnapshotRbxlx {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".rbxlx") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
-
-        let options = rbx_xml::DecodeOptions::new()
-            .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown);
-
-        let temp_tree = rbx_xml::from_reader(vfs.read(path)?.as_slice(), options)
-            .expect("TODO: Handle rbx_xml errors");
-
-        let root_id = temp_tree.get_root_id();
-
-        let snapshot = InstanceSnapshot::from_tree(&temp_tree, root_id)
-            .name(instance_name)
-            .metadata(
-                InstanceMetadata::new()
-                    .instigating_source(path)
-                    .relevant_paths(vec![path.to_path_buf()])
-                    .context(context),
-            );
-
-        Ok(Some(snapshot))
-    }
-}

src/snapshot_middleware/rbxm.rs

@@ -5,26 +5,14 @@ use rbx_dom_weak::{RbxInstanceProperties, RbxTree};
 
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
-use super::{
+use super::middleware::SnapshotInstanceResult;
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
-};
-
-pub struct SnapshotRbxm;
-
-impl SnapshotMiddleware for SnapshotRbxm {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".rbxm") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
 
+pub fn snapshot_rbxm(
+    context: &InstanceContext,
+    vfs: &Vfs,
+    path: &Path,
+    instance_name: &str,
+) -> SnapshotInstanceResult {
     let mut temp_tree = RbxTree::new(RbxInstanceProperties {
         name: "DataModel".to_owned(),
         class_name: "DataModel".to_owned(),
@@ -52,7 +40,6 @@ impl SnapshotMiddleware for SnapshotRbxm {
     } else {
         panic!("Rojo doesn't have support for model files with zero or more than one top-level instances yet.");
     }
-    }
 }
 
 #[cfg(test)]
@@ -72,10 +59,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotRbxm::from_vfs(
+        let instance_snapshot = snapshot_rbxm(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.rbxm"),
+            "foo",
         )
         .unwrap()
         .unwrap();

src/snapshot_middleware/rbxmx.rs

@@ -4,26 +4,14 @@ use memofs::Vfs;
 
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
-use super::{
+use super::middleware::SnapshotInstanceResult;
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
-};
-
-pub struct SnapshotRbxmx;
-
-impl SnapshotMiddleware for SnapshotRbxmx {
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
-        let meta = vfs.metadata(path)?;
-
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".rbxmx") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
 
+pub fn snapshot_rbxmx(
+    context: &InstanceContext,
+    vfs: &Vfs,
+    path: &Path,
+    instance_name: &str,
+) -> SnapshotInstanceResult {
     let options = rbx_xml::DecodeOptions::new()
         .property_behavior(rbx_xml::DecodePropertyBehavior::ReadUnknown);
 
@@ -47,7 +35,6 @@ impl SnapshotMiddleware for SnapshotRbxmx {
     } else {
         panic!("Rojo doesn't have support for model files with zero or more than one top-level instances yet.");
     }
-    }
 }
 
 #[cfg(test)]
@@ -77,10 +64,11 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = SnapshotRbxmx::from_vfs(
+        let instance_snapshot = snapshot_rbxmx(
             &InstanceContext::default(),
             &mut vfs,
             Path::new("/foo.rbxmx"),
+            "foo",
         )
         .unwrap()
         .unwrap();

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap

@@ -6,9 +6,9 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: true
   instigating_source:
-    Path: /foo/default.project.json
+    Path: /foo.project.json
   relevant_paths:
-    - /foo/default.project.json
+    - /foo.project.json
   context: {}
 name: children
 class_name: Folder
@@ -19,7 +19,7 @@ children:
       ignore_unknown_instances: true
       instigating_source:
         ProjectNode:
-          - /foo
+          - /foo.project.json
           - Child
           - $className: Model
           - Folder

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap

@@ -20,7 +20,7 @@ children:
       ignore_unknown_instances: true
       instigating_source:
         ProjectNode:
-          - /foo
+          - /foo/other.project.json
           - SomeChild
           - $className: Model
           - Folder

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap

@@ -6,9 +6,9 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: true
   instigating_source:
-    Path: /foo/default.project.json
+    Path: /foo.project.json
   relevant_paths:
-    - /foo/default.project.json
+    - /foo.project.json
   context: {}
 name: resolved-properties
 class_name: StringValue

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap

@@ -6,9 +6,9 @@ snapshot_id: ~
 metadata:
   ignore_unknown_instances: true
   instigating_source:
-    Path: /foo/default.project.json
+    Path: /foo.project.json
   relevant_paths:
-    - /foo/default.project.json
+    - /foo.project.json
   context: {}
 name: unresolved-properties
 class_name: StringValue

src/snapshot_middleware/txt.rs

@@ -7,27 +7,15 @@ use rbx_dom_weak::RbxValue;
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
 use super::{
-    error::SnapshotError,
+    error::SnapshotError, meta_file::AdjacentMetadata, middleware::SnapshotInstanceResult,
-    meta_file::AdjacentMetadata,
-    middleware::{SnapshotInstanceResult, SnapshotMiddleware},
-    util::match_file_name,
 };
 
-pub struct SnapshotTxt;
+pub fn snapshot_txt(
-
+    context: &InstanceContext,
-impl SnapshotMiddleware for SnapshotTxt {
+    vfs: &Vfs,
-    fn from_vfs(context: &InstanceContext, vfs: &Vfs, path: &Path) -> SnapshotInstanceResult {
+    path: &Path,
-        let meta = vfs.metadata(path)?;
+    instance_name: &str,
-
+) -> SnapshotInstanceResult {
-        if meta.is_dir() {
-            return Ok(None);
-        }
-
-        let instance_name = match match_file_name(path, ".txt") {
-            Some(name) => name,
-            None => return Ok(None),
-        };
-
     let contents = vfs.read(path)?;
     let contents_str = str::from_utf8(&contents)
         .map_err(|err| SnapshotError::file_contents_bad_unicode(err, path))?
@@ -58,7 +46,6 @@ impl SnapshotMiddleware for SnapshotTxt {
     }
 
     Ok(Some(snapshot))
-    }
 }
 
 #[cfg(test)]
@@ -75,8 +62,12 @@ mod test {
 
         let mut vfs = Vfs::new(imfs.clone());
 
-        let instance_snapshot =
+        let instance_snapshot = snapshot_txt(
-            SnapshotTxt::from_vfs(&InstanceContext::default(), &mut vfs, Path::new("/foo.txt"))
+            &InstanceContext::default(),
+            &mut vfs,
+            Path::new("/foo.txt"),
+            "foo",
+        )
         .unwrap()
         .unwrap();
 

src/snapshot_middleware/user_plugins.rs

@@ -1,56 +0,0 @@
-use crate::{
-    snapshot::InstanceContext,
-    vfs::{Vfs, VfsEntry, VfsFetcher},
-};
-
-use super::middleware::{SnapshotInstanceResult, SnapshotMiddleware};
-
-/// Handles snapshotting of any file that a user plugin wants to handle.
-///
-/// User plugins are specified in the project file, but there are never user
-/// plugins specified unless a Cargo feature is enabled, `user-plugins`.
-/// Additionally, extra data needs to be set up inside the snapshot context
-/// which is not currently wired up.
-pub struct SnapshotUserPlugins;
-
-impl SnapshotMiddleware for SnapshotUserPlugins {
-    fn from_vfs<F: VfsFetcher>(
-        _context: &InstanceContext,
-        _vfs: &Vfs<F>,
-        _entry: &VfsEntry,
-    ) -> SnapshotInstanceResult {
-        // TODO: Invoke plugin here and get result out.
-
-        // The current plan for plugins here is to make them work
-        // like Redux/Rodux middleware. A plugin will be a function
-        // that accepts the next middleware in the chain as a
-        // function and the snapshot subject (the VFS entry).
-        //
-        // Plugins can (but don't have to) invoke the next snapshot
-        // function and may or may not mutate the result. The hope
-        // is that this model enables the most flexibility possible
-        // for plugins to modify existing Rojo output, as well as
-        // generate new outputs.
-        //
-        // Open questions:
-        // * How will middleware be ordered? Does putting user
-        //   middleware always at the beginning or always at the end
-        //   of the chain reduce the scope of what that middleware
-        //   can do?
-        //
-        // * Will plugins hurt Rojo's ability to parallelize
-        //   snapshotting in the future?
-        //
-        // * Do the mutable handles to the Vfs and the snapshot
-        //   context prevent plugins from invoking other plugins
-        //   indirectly?
-        //
-        // * Will there be problems using a single Lua state because
-        //   of re-entrancy?
-        //
-        // * Can the Lua <-> Rojo bindings used for middleware be
-        //   reused for or from another project like Remodel?
-
-        Ok(None)
-    }
-}