From 6a1fffd1cecb4d22ea995fed9bab380ae49f33f7 Mon Sep 17 00:00:00 2001 From: jeparlefrancais <35781636+jeparlefrancais@users.noreply.github.com> Date: Sun, 29 Mar 2020 19:03:15 -0400 Subject: [PATCH] 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 --- ..._test__build_test__infer_service_name.snap | 28 +++++++++++++ ...est__build_test__infer_starter_player.snap | 26 ++++++++++++ .../infer_service_name/default.project.json | 16 ++++++++ .../build-tests/infer_service_name/main.lua | 1 + .../infer_starter_player/default.project.json | 11 +++++ .../build-tests/infer_starter_player/main.lua | 1 + rojo-test/src/build_test.rs | 4 +- src/change_processor.rs | 7 ++-- src/snapshot/metadata.rs | 8 ++-- src/snapshot_middleware/project.rs | 41 +++++++++++++++++-- ..._project__test__project_with_children.snap | 1 + ...ct_with_path_to_project_with_children.snap | 1 + 12 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 rojo-test/build-test-snapshots/rojo_test__build_test__infer_service_name.snap create mode 100644 rojo-test/build-test-snapshots/rojo_test__build_test__infer_starter_player.snap create mode 100644 rojo-test/build-tests/infer_service_name/default.project.json create mode 100644 rojo-test/build-tests/infer_service_name/main.lua create mode 100644 rojo-test/build-tests/infer_starter_player/default.project.json create mode 100644 rojo-test/build-tests/infer_starter_player/main.lua diff --git a/rojo-test/build-test-snapshots/rojo_test__build_test__infer_service_name.snap b/rojo-test/build-test-snapshots/rojo_test__build_test__infer_service_name.snap new file mode 100644 index 00000000..bda746ce --- /dev/null +++ b/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 +--- + + + + infer-service-name + + + + HttpService + true + + + + + ReplicatedStorage + + + + Main + -- hello, from main + + + + + diff --git a/rojo-test/build-test-snapshots/rojo_test__build_test__infer_starter_player.snap b/rojo-test/build-test-snapshots/rojo_test__build_test__infer_starter_player.snap new file mode 100644 index 00000000..13574f23 --- /dev/null +++ b/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 +--- + + + + infer-service-name + + + + StarterPlayer + + + + StarterCharacterScripts + + + + + StarterPlayerScripts + + + + + diff --git a/rojo-test/build-tests/infer_service_name/default.project.json b/rojo-test/build-tests/infer_service_name/default.project.json new file mode 100644 index 00000000..36d8906c --- /dev/null +++ b/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 + } + } + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/infer_service_name/main.lua b/rojo-test/build-tests/infer_service_name/main.lua new file mode 100644 index 00000000..36c9d9bc --- /dev/null +++ b/rojo-test/build-tests/infer_service_name/main.lua @@ -0,0 +1 @@ +-- hello, from main \ No newline at end of file diff --git a/rojo-test/build-tests/infer_starter_player/default.project.json b/rojo-test/build-tests/infer_starter_player/default.project.json new file mode 100644 index 00000000..3e505e66 --- /dev/null +++ b/rojo-test/build-tests/infer_starter_player/default.project.json @@ -0,0 +1,11 @@ +{ + "name": "infer-service-name", + "tree": { + "$className": "DataModel", + + "StarterPlayer": { + "StarterPlayerScripts": {}, + "StarterCharacterScripts": {} + } + } +} \ No newline at end of file diff --git a/rojo-test/build-tests/infer_starter_player/main.lua b/rojo-test/build-tests/infer_starter_player/main.lua new file mode 100644 index 00000000..36c9d9bc --- /dev/null +++ b/rojo-test/build-tests/infer_starter_player/main.lua @@ -0,0 +1 @@ +-- hello, from main \ No newline at end of file diff --git a/rojo-test/src/build_test.rs b/rojo-test/src/build_test.rs index 7939f206..f0d6424d 100644 --- a/rojo-test/src/build_test.rs +++ b/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, } diff --git a/src/change_processor.rs b/src/change_processor.rs index 15f264c8..9e7797ee 100644 --- a/src/change_processor.rs +++ b/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 { diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs index 983b426e..192a0001 100644 --- a/src/snapshot/metadata.rs +++ b/src/snapshot/metadata.rs @@ -163,6 +163,7 @@ pub enum InstigatingSource { #[serde(serialize_with = "path_serializer::serialize_absolute")] PathBuf, String, ProjectNode, + Option, ), } @@ -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, ), } } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index e2d0c3a7..cefed521 100644 --- a/src/snapshot_middleware/project.rs +++ b/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) = - snapshot_project_node(context, project_folder, child_name, child_project_node, vfs)? - { + if let Some(child) = snapshot_project_node( + 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 { diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap index ab659524..e5603c97 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap +++ b/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 diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap index ba3fb273..389d24e2 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap +++ b/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