Change JSON model naming behavior.

- JSON model names now come from the file name
- The 'Name' field is now optional for the top-level instance
- Snapshot tests run way faster by executing Rojo directly instead of Cargo

Cargo.lock

@@ -1533,6 +1533,7 @@ version = "0.1.0"
 dependencies = [
  "insta 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "paste 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rojo 0.5.0-alpha.13",
  "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

rojo-test/Cargo.toml

@@ -8,4 +8,7 @@ publish = false
 [dependencies]
 insta = "0.10.0"
 paste = "0.1.5"
-tempfile = "3.1.0"
+tempfile = "3.1.0"
+
+# We execute Rojo via std::process::Command, so depend on it so it's built!
+rojo = { path = "../server" }

rojo-test/build-tests/json_model_in_folder/folder/simple-model.model.json

@@ -1,5 +1,4 @@
 {
-  "Name": "Top-Level Value",
   "ClassName": "IntValue",
   "Properties": {
     "Value": 5

rojo-test/build-tests/json_model_legacy_name/default.project.json

@@ -0,0 +1,6 @@
+{
+  "name": "json_model_legacy_name",
+  "tree": {
+    "$path": "folder"
+  }
+}

rojo-test/build-tests/json_model_legacy_name/folder/Expected Name.model.json

@@ -0,0 +1,4 @@
+{
+  "Name": "Overridden Name",
+  "ClassName": "Folder"
+}

rojo-test/src/build_test.rs

@@ -27,6 +27,7 @@ gen_build_tests! {
     csv_in_folder,
     gitkeep,
     json_model_in_folder,
+    json_model_legacy_name,
     module_in_folder,
     rbxm_in_folder,
     rbxmx_in_folder,
@@ -44,11 +45,16 @@ fn run_build_test(test_name: &str) {
     let input_path = build_test_path.join(test_name);
     let output_path = output_dir.path().join(format!("{}.rbxmx", test_name));
 
-    let status = Command::new("cargo")
+    let mut exe_path = working_dir.join("target/debug/rojo");
+    if cfg!(windows) {
+        exe_path.set_extension("exe");
+    }
+
+    let status = Command::new(exe_path)
         .args(&[
-            "run", "--quiet", "--",
             "build", input_path.to_str().unwrap(), "-o", output_path.to_str().unwrap(),
         ])
+        .env("RUST_LOG", "error")
         .current_dir(working_dir)
         .status()
         .expect("Couldn't start Rojo");

rojo-test/src/snapshots/build_test__json_model_in_folder.snap

@@ -1,5 +1,5 @@
 ---
-created: "2019-08-15T01:02:40.318937300Z"
+created: "2019-08-15T20:40:50.467761900Z"
 creator: insta@0.10.0
 source: rojo-test/src/build_test.rs
 expression: contents
@@ -11,7 +11,7 @@ expression: contents
     </Properties>
     <Item class="IntValue" referent="1">
       <Properties>
-        <string name="Name">Top-Level Value</string>
+        <string name="Name">simple-model</string>
         <int name="Value">5</int>
       </Properties>
       <Item class="Folder" referent="2">

rojo-test/src/snapshots/build_test__json_model_legacy_name.snap

@@ -0,0 +1,18 @@
+---
+created: "2019-08-15T20:41:28.743618200Z"
+creator: insta@0.10.0
+source: rojo-test/src/build_test.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">json_model_legacy_name</string>
+    </Properties>
+    <Item class="Folder" referent="1">
+      <Properties>
+        <string name="Name">Expected Name</string>
+      </Properties>
+    </Item>
+  </Item>
+</roblox>

server/src/rbx_snapshot.rs

@@ -724,28 +724,63 @@ fn snapshot_csv_file<'source>(
 fn snapshot_json_model_file<'source>(
     file: &'source ImfsFile,
 ) -> SnapshotResult<'source> {
+    let file_name = file.path
+        .file_name().expect("Could not extract file stem")
+        .to_str().expect("Could not convert path to UTF-8");
+
+    let instance_name = match_trailing(file_name, ".model.json")
+        .expect("JSON model file did not end in .model.json");
+
     let contents = str::from_utf8(&file.contents)
         .map_err(|inner| SnapshotError::Utf8Error {
             inner,
             path: file.path.to_owned(),
         })?;
 
-    let json_instance: JsonModelInstance = serde_json::from_str(contents)
+    let json_instance: JsonModel = serde_json::from_str(contents)
         .map_err(|inner| SnapshotError::JsonModelDecodeError {
             inner,
             path: file.path.to_owned(),
         })?;
 
-    let mut snapshot = json_instance.into_snapshot()?;
+    if let Some(json_name) = &json_instance.name {
+        if json_name != instance_name {
+            log::warn!("Name from JSON model did not match its file name: {}", file.path.display());
+            log::warn!("In Rojo <  alpha 14, this model is named \"{}\" (from its 'Name' property)", json_name);
+            log::warn!("In Rojo >= alpha 14, this model is named \"{}\" (from its file name)", instance_name);
+            log::warn!("'Name' for the top-level instance in a JSON model is now optional and will be ignored.");
+        }
+    }
+
+    let mut snapshot = json_instance.core.into_snapshot(instance_name.to_owned())?;
+
+    snapshot.name = Cow::Borrowed(instance_name);
     snapshot.metadata.source_path = Some(file.path.to_owned());
 
     Ok(Some(snapshot))
 }
 
+#[derive(Debug, Serialize, Deserialize)]
+#[serde(rename_all = "PascalCase")]
+struct JsonModel {
+    name: Option<String>,
+
+    #[serde(flatten)]
+    core: JsonModelCore,
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "PascalCase")]
 struct JsonModelInstance {
     name: String,
+
+    #[serde(flatten)]
+    core: JsonModelCore,
+}
+
+#[derive(Debug, Serialize, Deserialize)]
+#[serde(rename_all = "PascalCase")]
+struct JsonModelCore {
     class_name: String,
 
     #[serde(default = "Vec::new", skip_serializing_if = "Vec::is_empty")]
@@ -755,12 +790,12 @@ struct JsonModelInstance {
     properties: HashMap<String, UnresolvedRbxValue>,
 }
 
-impl JsonModelInstance {
-    fn into_snapshot(self) -> Result<RbxSnapshotInstance<'static>, SnapshotError> {
+impl JsonModelCore {
+    fn into_snapshot(self, name: String) -> Result<RbxSnapshotInstance<'static>, SnapshotError> {
         let mut children = Vec::with_capacity(self.children.len());
 
         for child in self.children {
-            children.push(child.into_snapshot()?);
+            children.push(child.core.into_snapshot(child.name)?);
         }
 
         let mut properties = HashMap::with_capacity(self.properties.len());
@@ -771,7 +806,7 @@ impl JsonModelInstance {
         }
 
         Ok(RbxSnapshotInstance {
-            name: Cow::Owned(self.name),
+            name: Cow::Owned(name),
             class_name: Cow::Owned(self.class_name),
             properties,
             children,