Infer class name (#210)

* infer service names

* Update project code and add support for StarterPlayer

* Store parent_class in InstigatingSource

* Update snapshots

Co-authored-by: Lucien Greathouse <me@lpghatguy.com>

rojo-test/build-test-snapshots/rojo_test__build_test__infer_service_name.snap

@@ -0,0 +1,28 @@
+---
+source: rojo-test/src/build_test.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="DataModel" referent="0">
+    <Properties>
+      <string name="Name">infer-service-name</string>
+    </Properties>
+    <Item class="HttpService" referent="1">
+      <Properties>
+        <string name="Name">HttpService</string>
+        <bool name="HttpEnabled">true</bool>
+      </Properties>
+    </Item>
+    <Item class="ReplicatedStorage" referent="2">
+      <Properties>
+        <string name="Name">ReplicatedStorage</string>
+      </Properties>
+      <Item class="ModuleScript" referent="3">
+        <Properties>
+          <string name="Name">Main</string>
+          <string name="Source">-- hello, from main</string>
+        </Properties>
+      </Item>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/rojo_test__build_test__infer_starter_player.snap

@@ -0,0 +1,26 @@
+---
+source: rojo-test/src/build_test.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="DataModel" referent="0">
+    <Properties>
+      <string name="Name">infer-service-name</string>
+    </Properties>
+    <Item class="StarterPlayer" referent="1">
+      <Properties>
+        <string name="Name">StarterPlayer</string>
+      </Properties>
+      <Item class="StarterCharacterScripts" referent="2">
+        <Properties>
+          <string name="Name">StarterCharacterScripts</string>
+        </Properties>
+      </Item>
+      <Item class="StarterPlayerScripts" referent="3">
+        <Properties>
+          <string name="Name">StarterPlayerScripts</string>
+        </Properties>
+      </Item>
+    </Item>
+  </Item>
+</roblox>

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

@@ -0,0 +1,16 @@
+{
+  "name": "infer-service-name",
+  "tree": {
+    "$className": "DataModel",
+    "ReplicatedStorage": {
+      "Main": {
+        "$path": "main.lua"
+      }
+    },
+    "HttpService": {
+      "$properties": {
+        "HttpEnabled": true
+      }
+    }
+  }
+}

rojo-test/build-tests/infer_service_name/main.lua

@@ -0,0 +1 @@
+-- hello, from main

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

@@ -0,0 +1,11 @@
+{
+  "name": "infer-service-name",
+  "tree": {
+    "$className": "DataModel",
+
+    "StarterPlayer": {
+      "StarterPlayerScripts": {},
+      "StarterCharacterScripts": {}
+    }
+  }
+}

rojo-test/build-tests/infer_starter_player/main.lua

@@ -0,0 +1 @@
+-- hello, from main

rojo-test/src/build_test.rs

@@ -28,6 +28,8 @@ gen_build_tests! {
     csv_in_folder,
     deep_nesting,
     gitkeep,
+    infer_service_name,
+    infer_starter_player,
     init_meta_class_name,
     init_meta_properties,
     init_with_children,
@@ -38,10 +40,10 @@ gen_build_tests! {
     module_init,
     rbxm_in_folder,
     rbxmx_in_folder,
+    rbxmx_ref,
     script_meta_disabled,
     server_in_folder,
     server_init,
-    rbxmx_ref,
     txt,
     txt_in_folder,
 }

src/change_processor.rs

@@ -178,7 +178,7 @@ impl JobThreadContext {
                     if let Some(instigating_source) = &instance.metadata().instigating_source {
                         match instigating_source {
                             InstigatingSource::Path(path) => fs::remove_file(path).unwrap(),
-                            InstigatingSource::ProjectNode(_, _, _) => {
+                            InstigatingSource::ProjectNode(_, _, _, _) => {
                                 log::warn!(
                                     "Cannot remove instance {}, it's from a project file",
                                     id
@@ -226,7 +226,7 @@ impl JobThreadContext {
                                             log::warn!("Cannot change Source to non-string value.");
                                         }
                                     }
-                                    InstigatingSource::ProjectNode(_, _, _) => {
+                                    InstigatingSource::ProjectNode(_, _, _, _) => {
                                         log::warn!(
                                             "Cannot remove instance {}, it's from a project file",
                                             id
@@ -318,7 +318,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: RbxId) -> Optio
             }
         },
 
-        InstigatingSource::ProjectNode(project_path, instance_name, project_node) => {
+        InstigatingSource::ProjectNode(project_path, instance_name, project_node, parent_class) => {
             // This instance is the direct subject of a project node. Since
             // there might be information associated with our instance from
             // the project file, we snapshot the entire project node again.
@@ -329,6 +329,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: RbxId) -> Optio
                 instance_name,
                 project_node,
                 &vfs,
+                parent_class.as_ref().map(|name| name.as_str()),
             );
 
             let snapshot = match snapshot_result {

src/snapshot/metadata.rs

@@ -163,6 +163,7 @@ pub enum InstigatingSource {
         #[serde(serialize_with = "path_serializer::serialize_absolute")] PathBuf,
         String,
         ProjectNode,
+        Option<String>,
     ),
 }
 
@@ -170,12 +171,13 @@ impl fmt::Debug for InstigatingSource {
     fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             InstigatingSource::Path(path) => write!(formatter, "Path({})", path.display()),
-            InstigatingSource::ProjectNode(path, name, node) => write!(
+            InstigatingSource::ProjectNode(path, name, node, parent_class) => write!(
                 formatter,
-                "ProjectNode({}: {:?}) from path {}",
+                "ProjectNode({}: {:?}) from path {} and parent class {:?}",
                 name,
                 node,
-                path.display()
+                path.display(),
+                parent_class,
             ),
         }
     }

src/snapshot_middleware/project.rs

@@ -1,7 +1,7 @@
 use std::{borrow::Cow, collections::HashMap, path::Path};
 
 use memofs::{IoResultExt, Vfs};
-use rbx_reflection::try_resolve_value;
+use rbx_reflection::{get_class_descriptor, try_resolve_value};
 
 use crate::{
     project::{Project, ProjectNode},
@@ -62,6 +62,7 @@ impl SnapshotMiddleware for SnapshotProject {
             &project.name,
             &project.tree,
             vfs,
+            None,
         )?
         .unwrap();
 
@@ -93,6 +94,7 @@ pub fn snapshot_project_node(
     instance_name: &str,
     node: &ProjectNode,
     vfs: &Vfs,
+    parent_class: Option<&str>,
 ) -> SnapshotInstanceResult {
     let name = Cow::Owned(instance_name.to_owned());
     let mut class_name = node
@@ -158,13 +160,43 @@ pub fn snapshot_project_node(
     }
 
     let class_name = class_name
+        .or_else(|| {
+            // If className wasn't defined from another source, we may be able
+            // to infer one.
+
+            let parent_class = parent_class?;
+
+            if parent_class == "DataModel" {
+                // Members of DataModel with names that match known services are
+                // probably supposed to be those services.
+
+                let descriptor = get_class_descriptor(&name)?;
+
+                if descriptor.is_service() {
+                    return Some(name.clone());
+                }
+            } else if parent_class == "StarterPlayer" {
+                // StarterPlayer has two special members with their own classes.
+
+                if name == "StarterPlayerScripts" || name == "StarterCharacterScripts" {
+                    return Some(name.clone());
+                }
+            }
+
+            None
+        })
         // TODO: Turn this into an error object.
         .expect("$className or $path must be specified");
 
     for (child_name, child_project_node) in &node.children {
-        if let Some(child) =
+        if let Some(child) = snapshot_project_node(
-            snapshot_project_node(context, project_folder, child_name, child_project_node, vfs)?
+            context,
-        {
+            project_folder,
+            child_name,
+            child_project_node,
+            vfs,
+            Some(&class_name),
+        )? {
             children.push(child);
         }
     }
@@ -194,6 +226,7 @@ pub fn snapshot_project_node(
         project_folder.to_path_buf(),
         instance_name.to_string(),
         node.clone(),
+        parent_class.map(|name| name.to_owned()),
     ));
 
     Ok(Some(InstanceSnapshot {

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap

@@ -22,6 +22,7 @@ children:
           - /foo
           - Child
           - $className: Model
+          - Folder
       relevant_paths: []
       context: {}
     name: Child

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap

@@ -23,6 +23,7 @@ children:
           - /foo
           - SomeChild
           - $className: Model
+          - Folder
       relevant_paths: []
       context: {}
     name: SomeChild