internal: Use compact strings for IDs/targets. (#1667)

* Update ID. * Update target. * Polish.
moonrepo · Oct 7, 2024 · 3191e48 · 3191e48
1 parent 78c6107
commit 3191e48
Show file tree

Hide file tree

Showing 16 changed files with 95 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,10 @@
 - Updated `moon query tasks` to include the task type and platform, and the task description as a
   trailing value.
 
+#### ⚙️ Internal
+
+- Updated identifiers and targets to use [compact strings](https://crates.io/crates/compact_str).
+
 ## 1.28.3
 
 #### 🐞 Fixes

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -28,6 +28,9 @@ clap = { version = "4.5.16", default-features = false, features = [
 	"error-context",
 ] }
 clap_complete = "4.5.24"
+compact_str = { version = "0.8.0", default-features = false, features = [
+	"serde",
+] }
 console = "0.15.8"
 dirs = "5.0.1"
 indexmap = "2.5.0"

diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml
@@ -9,6 +9,7 @@ repository = "https://github.com/moonrepo/moon"
 publish = true
 
 [dependencies]
+compact_str = { workspace = true }
 dirs = { workspace = true }
 miette = { workspace = true }
 relative-path = { workspace = true, features = ["serde"] }

diff --git a/crates/common/src/id.rs b/crates/common/src/id.rs
@@ -1,3 +1,4 @@
+use compact_str::CompactString;
 use miette::Diagnostic;
 use regex::Regex;
 use schematic::{Schema, SchemaBuilder, Schematic};
@@ -17,7 +18,7 @@ pub static ID_CLEAN: OnceLock<Regex> = OnceLock::new();
 pub struct IdError(String);
 
 #[derive(Clone, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
-pub struct Id(String);
+pub struct Id(CompactString);
 
 impl Id {
     pub fn new<S: AsRef<str>>(id: S) -> Result<Id, IdError> {
@@ -46,7 +47,7 @@ impl Id {
     }
 
     pub fn raw<S: AsRef<str>>(id: S) -> Id {
-        Id(id.as_ref().to_owned())
+        Id(CompactString::new(id))
     }
 
     pub fn as_str(&self) -> &str {
@@ -66,26 +67,32 @@ impl fmt::Display for Id {
     }
 }
 
-impl AsRef<str> for Id {
-    fn as_ref(&self) -> &str {
-        &self.0
+impl Stylize for Id {
+    fn style(&self, style: Style) -> String {
+        self.to_string().style(style)
     }
 }
 
-impl AsRef<String> for Id {
-    fn as_ref(&self) -> &String {
+impl AsRef<str> for Id {
+    fn as_ref(&self) -> &str {
         &self.0
     }
 }
 
+// impl AsRef<String> for Id {
+//     fn as_ref(&self) -> &String {
+//         &self.0
+//     }
+// }
+
 impl AsRef<Id> for Id {
     fn as_ref(&self) -> &Id {
         self
     }
 }
 
 impl Deref for Id {
-    type Target = String;
+    type Target = str;
 
     fn deref(&self) -> &Self::Target {
         &self.0
@@ -100,23 +107,23 @@ impl PartialEq<str> for Id {
 
 impl PartialEq<&str> for Id {
     fn eq(&self, other: &&str) -> bool {
-        &self.0 == other
+        self.0 == other
     }
 }
 
 impl PartialEq<String> for Id {
     fn eq(&self, other: &String) -> bool {
-        &self.0 == other
+        self.0 == other
     }
 }
 
 // Allows strings to be used for collection keys
 
-impl Borrow<String> for Id {
-    fn borrow(&self) -> &String {
-        &self.0
-    }
-}
+// impl Borrow<String> for Id {
+//     fn borrow(&self) -> &String {
+//         &self.0
+//     }
+// }
 
 impl Borrow<str> for Id {
     fn borrow(&self) -> &str {

diff --git a/crates/project-expander/src/tasks_expander_error.rs b/crates/project-expander/src/tasks_expander_error.rs
@@ -17,17 +17,17 @@ pub enum TasksExpanderError {
     #[diagnostic(code(task_expander::dependency::no_allowed_failures))]
     #[error(
         "Task {} cannot depend on task {}, as it is allowed to fail, which may cause unwanted side-effects.\nA task is marked to allow failure with the {} setting.",
-        .task.id.style(Style::Label),
-        .dep.id.style(Style::Label),
+        .task.style(Style::Label),
+        .dep.style(Style::Label),
         "options.allowFailure".style(Style::Symbol),
     )]
     AllowFailureDepRequirement { dep: Target, task: Target },
 
     #[diagnostic(code(task_expander::dependency::persistent_requirement))]
     #[error(
         "Non-persistent task {} cannot depend on persistent task {}.\nA task is marked persistent with the {} or {} settings.\n\nIf you're looking to avoid the cache, disable {} instead.",
-        .task.id.style(Style::Label),
-        .dep.id.style(Style::Label),
+        .task.style(Style::Label),
+        .dep.style(Style::Label),
         "local".style(Style::Symbol),
         "options.persistent".style(Style::Symbol),
         "options.cache".style(Style::Symbol),
@@ -37,25 +37,25 @@ pub enum TasksExpanderError {
     #[diagnostic(code(task_expander::dependency::run_in_ci_mismatch))]
     #[error(
         "Task {} cannot depend on task {}, as the dependency cannot run in CI because {} is disabled. Because of this, the pipeline will not run tasks correctly.",
-        .task.id.style(Style::Label),
-        .dep.id.style(Style::Label),
+        .task.style(Style::Label),
+        .dep.style(Style::Label),
         "options.runInCI".style(Style::Symbol),
     )]
     RunInCiDepRequirement { dep: Target, task: Target },
 
     #[diagnostic(code(task_expander::unknown_target))]
     #[error(
         "Invalid dependency {} for {}, target does not exist.",
-        .dep.id.style(Style::Label),
-        .task.id.style(Style::Label),
+        .dep.style(Style::Label),
+        .task.style(Style::Label),
     )]
     UnknownTarget { dep: Target, task: Target },
 
     #[diagnostic(code(task_expander::unsupported_target_scope))]
     #[error(
         "Invalid dependency {} for {}. All (:) scope is not supported.",
-        .dep.id.style(Style::Label),
-        .task.id.style(Style::Label),
+        .dep.style(Style::Label),
+        .task.style(Style::Label),
     )]
     UnsupportedTargetScopeInDeps { dep: Target, task: Target },
 }
diff --git a/crates/project-graph/src/project_graph_builder.rs b/crates/project-graph/src/project_graph_builder.rs
@@ -555,7 +555,7 @@ impl<'app> ProjectGraphBuilder<'app> {
             }
 
             // Skip aliases that would override an ID
-            if self.sources.contains_key(&alias) {
+            if self.sources.contains_key(alias.as_str()) {
                 debug!(
                     "Skipping alias {} (for project {}) as it conflicts with the project {}",
                     color::label(&alias),

diff --git a/crates/target/Cargo.toml b/crates/target/Cargo.toml
@@ -10,6 +10,7 @@ publish = true
 
 [dependencies]
 moon_common = { version = "0.0.8", path = "../common" }
+compact_str = { workspace = true }
 miette = { workspace = true }
 once_cell = { workspace = true }
 regex = { workspace = true }

diff --git a/crates/target/src/target.rs b/crates/target/src/target.rs
@@ -1,14 +1,12 @@
 use crate::target_error::TargetError;
 use crate::target_scope::TargetScope;
-use moon_common::{color, Id, ID_CHARS};
+use compact_str::CompactString;
+use moon_common::{color, Id, Style, Stylize, ID_CHARS};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use schematic::{Schema, SchemaBuilder, Schematic};
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
-use std::{
-    cmp::Ordering,
-    fmt::{self, Display},
-};
+use std::{cmp::Ordering, fmt};
 use tracing::instrument;
 
 // The @ is to support npm package scopes!
@@ -22,7 +20,7 @@ pub static TARGET_PATTERN: Lazy<Regex> = Lazy::new(|| {
 
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub struct Target {
-    pub id: String,
+    pub id: CompactString,
     pub scope: TargetScope,
     pub task_id: Id,
 }
@@ -40,7 +38,7 @@ impl Target {
         let scope = TargetScope::Project(Id::new(scope_id).map_err(handle_error)?);
 
         Ok(Target {
-            id: Target::format(&scope, task_id),
+            id: CompactString::new(Target::format(&scope, task_id)),
             scope,
             task_id: Id::new(task_id).map_err(handle_error)?,
         })
@@ -53,7 +51,7 @@ impl Target {
         let task_id = task_id.as_ref();
 
         Ok(Target {
-            id: Target::format(TargetScope::OwnSelf, task_id),
+            id: CompactString::new(Target::format(TargetScope::OwnSelf, task_id)),
             scope: TargetScope::OwnSelf,
             task_id: Id::new(task_id)
                 .map_err(|_| TargetError::InvalidFormat(format!("~:{task_id}")))?,
@@ -102,7 +100,7 @@ impl Target {
             .map_err(|_| TargetError::InvalidFormat(target_id.to_owned()))?;
 
         Ok(Target {
-            id: target_id.to_owned(),
+            id: CompactString::new(target_id),
             scope,
             task_id,
         })
@@ -165,12 +163,18 @@ impl Default for Target {
     }
 }
 
-impl Display for Target {
+impl fmt::Display for Target {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self.id)
     }
 }
 
+impl Stylize for Target {
+    fn style(&self, style: Style) -> String {
+        self.to_string().style(style)
+    }
+}
+
 impl AsRef<Target> for Target {
     fn as_ref(&self) -> &Target {
         self

diff --git a/crates/target/tests/target_test.rs b/crates/target/tests/target_test.rs
@@ -1,3 +1,4 @@
+use compact_str::CompactString;
 use moon_common::Id;
 use moon_target::{Target, TargetScope};
 
@@ -71,7 +72,7 @@ fn parse_ids() {
     assert_eq!(
         Target::parse("foo:build").unwrap(),
         Target {
-            id: String::from("foo:build"),
+            id: CompactString::from("foo:build"),
             scope: TargetScope::Project(Id::raw("foo")),
             task_id: Id::raw("build"),
         }
@@ -83,7 +84,7 @@ fn parse_deps_scope() {
     assert_eq!(
         Target::parse("^:build").unwrap(),
         Target {
-            id: String::from("^:build"),
+            id: CompactString::from("^:build"),
             scope: TargetScope::Deps,
             task_id: Id::raw("build"),
         }
@@ -107,7 +108,7 @@ fn parse_self_scope() {
     assert_eq!(
         Target::parse("~:build").unwrap(),
         Target {
-            id: String::from("~:build"),
+            id: CompactString::from("~:build"),
             scope: TargetScope::OwnSelf,
             task_id: Id::raw("build"),
         }
@@ -119,7 +120,7 @@ fn parse_self_when_no_colon() {
     assert_eq!(
         Target::parse("build").unwrap(),
         Target {
-            id: String::from("~:build"),
+            id: CompactString::from("~:build"),
             scope: TargetScope::OwnSelf,
             task_id: Id::raw("build"),
         }
@@ -143,7 +144,7 @@ fn parse_all_scopes() {
     assert_eq!(
         Target::parse(":build").unwrap(),
         Target {
-            id: String::from(":build"),
+            id: CompactString::from(":build"),
             scope: TargetScope::All,
             task_id: Id::raw("build"),
         }
@@ -167,7 +168,7 @@ fn parse_node_package() {
     assert_eq!(
         Target::parse("@scope/foo:build").unwrap(),
         Target {
-            id: String::from("@scope/foo:build"),
+            id: CompactString::from("@scope/foo:build"),
             scope: TargetScope::Project(Id::raw("@scope/foo")),
             task_id: Id::raw("build"),
         }
@@ -179,7 +180,7 @@ fn parse_slashes() {
     assert_eq!(
         Target::parse("foo/sub:build/esm").unwrap(),
         Target {
-            id: String::from("foo/sub:build/esm"),
+            id: CompactString::from("foo/sub:build/esm"),
             scope: TargetScope::Project(Id::raw("foo/sub")),
             task_id: Id::raw("build/esm"),
         }