refactor(rome_formatter): Use custom trait for unique LabelIds (#4599)

MichaReiser · web-flow · commit def1bac3ac5c · 2023-06-22T21:42:02.000+01:00
diff --git a/crates/rome_formatter/src/builders.rs b/crates/rome_formatter/src/builders.rs
@@ -493,10 +493,25 @@ impl<Context> Format<Context> for LineSuffixBoundary {
 /// ## Examples
 ///
 /// ```rust
-/// use rome_formatter::prelude::*;
-/// use rome_formatter::{format, write, LineWidth};
+/// # use rome_formatter::prelude::*;
+/// # use rome_formatter::{format, write, LineWidth};
 ///
-/// enum SomeLabelId {}
+/// #[derive(Debug, Copy, Clone)]
+/// enum MyLabels {
+///     Main
+/// }
+///
+/// impl tag::Label for MyLabels {
+///     fn id(&self) -> u64 {
+///         *self as u64
+///     }
+///
+///     fn debug_name(&self) -> &'static str {
+///         match self {
+///             Self::Main => "Main"
+///         }
+///     }
+/// }
 ///
 /// # fn main() -> FormatResult<()> {
 /// let formatted = format!(
@@ -505,31 +520,31 @@ impl<Context> Format<Context> for LineSuffixBoundary {
 ///         let mut recording = f.start_recording();
 ///         write!(recording, [
 ///             labelled(
-///                 LabelId::of::<SomeLabelId>(),
+///                 LabelId::of(MyLabels::Main),
 ///                 &text("'I have a label'")
 ///             )
 ///         ])?;
 ///
 ///         let recorded = recording.stop();
 ///
-///         let is_labelled = recorded.first().map_or(false, |element| element.has_label(LabelId::of::<SomeLabelId>()));
+///         let is_labelled = recorded.first().map_or(false, |element| element.has_label(LabelId::of(MyLabels::Main)));
 ///
 ///         if is_labelled {
-///             write!(f, [text(" has label SomeLabelId")])
+///             write!(f, [text(" has label `Main`")])
 ///         } else {
-///             write!(f, [text(" doesn't have label SomeLabelId")])
+///             write!(f, [text(" doesn't have label `Main`")])
 ///         }
 ///     })]
 /// )?;
 ///
-/// assert_eq!("'I have a label' has label SomeLabelId", formatted.print()?.as_code());
+/// assert_eq!("'I have a label' has label `Main`", formatted.print()?.as_code());
 /// # Ok(())
 /// # }
 /// ```
 ///
 /// ## Alternatives
 ///
-/// Use `Memoized.inspect(f)?.has_label(LabelId::of::<SomeLabelId>()` if you need to know if some content breaks that should
+/// Use `Memoized.inspect(f)?.has_label(LabelId::of(MyLabels::Main)` if you need to know if some content breaks that should
 /// only be written later.
 #[inline]
 pub fn labelled<Content, Context>(label_id: LabelId, content: &Content) -> FormatLabelled<Context>
diff --git a/crates/rome_formatter/src/format_element/tag.rs b/crates/rome_formatter/src/format_element/tag.rs
@@ -1,8 +1,5 @@
 use crate::format_element::PrintMode;
 use crate::{GroupId, TextSize};
-#[cfg(debug_assertions)]
-use std::any::type_name;
-use std::any::TypeId;
 use std::cell::Cell;
 use std::num::NonZeroU8;
 
@@ -239,37 +236,48 @@ impl Align {
     }
 }
 
-#[derive(Eq, PartialEq, Copy, Clone)]
+#[derive(Debug, Eq, Copy, Clone)]
 pub struct LabelId {
-    id: TypeId,
+    value: u64,
     #[cfg(debug_assertions)]
-    label: &'static str,
+    name: &'static str,
 }
 
-#[cfg(debug_assertions)]
-impl std::fmt::Debug for LabelId {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(self.label)
-    }
-}
+impl PartialEq for LabelId {
+    fn eq(&self, other: &Self) -> bool {
+        let is_equal = self.value == other.value;
+
+        #[cfg(debug_assertions)]
+        {
+            if is_equal {
+                assert_eq!(self.name, other.name, "Two `LabelId`s with different names have the same `value`. Are you mixing labels of two different `LabelDefinition` or are the values returned by the `LabelDefinition` not unique?");
+            }
+        }
 
-#[cfg(not(debug_assertions))]
-impl std::fmt::Debug for LabelId {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        std::write!(f, "#{:?}", self.id)
+        is_equal
     }
 }
 
 impl LabelId {
-    pub fn of<T: ?Sized + 'static>() -> Self {
+    pub fn of<T: Label>(label: T) -> Self {
         Self {
-            id: TypeId::of::<T>(),
+            value: label.id(),
             #[cfg(debug_assertions)]
-            label: type_name::<T>(),
+            name: label.debug_name(),
         }
     }
 }
 
+/// Defines the valid labels of a language. You want to have at most one implementation per formatter
+/// project.
+pub trait Label {
+    /// Returns the `u64` uniquely identifying this specific label.
+    fn id(&self) -> u64;
+
+    /// Returns the name of the label that is shown in debug builds.
+    fn debug_name(&self) -> &'static str;
+}
+
 #[derive(Clone, Copy, Eq, PartialEq, Debug)]
 pub enum VerbatimKind {
     Bogus,
diff --git a/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs b/crates/rome_js_formatter/src/js/expressions/static_member_expression.rs
@@ -2,7 +2,7 @@ use crate::prelude::*;
 
 use crate::js::expressions::computed_member_expression::AnyJsComputedMemberLike;
 use crate::parentheses::NeedsParentheses;
-use crate::utils::member_chain::MemberChainLabel;
+use crate::JsLabels;
 use rome_formatter::{format_args, write};
 use rome_js_syntax::{
     AnyJsAssignment, AnyJsAssignmentPattern, AnyJsExpression, AnyJsName, JsAssignmentExpression,
@@ -45,7 +45,7 @@ impl Format<JsFormatContext> for AnyJsStaticMemberLike {
 
             recording
                 .stop()
-                .has_label(LabelId::of::<MemberChainLabel>())
+                .has_label(LabelId::of(JsLabels::MemberChain))
         };
 
         let layout = self.layout(is_member_chain)?;
@@ -60,7 +60,7 @@ impl Format<JsFormatContext> for AnyJsStaticMemberLike {
                     write!(
                         f,
                         [labelled(
-                            LabelId::of::<MemberChainLabel>(),
+                            LabelId::of(JsLabels::MemberChain),
                             &format_no_break
                         )]
                     )
diff --git a/crates/rome_js_formatter/src/lib.rs b/crates/rome_js_formatter/src/lib.rs
@@ -179,6 +179,7 @@ mod parentheses;
 pub(crate) mod separated;
 mod syntax_rewriter;
 
+use rome_formatter::format_element::tag::Label;
 use rome_formatter::prelude::*;
 use rome_formatter::{
     comments::Comments, write, CstFormatContext, Format, FormatLanguage, FormatToken,
@@ -537,6 +538,23 @@ pub fn format_sub_tree(options: JsFormatOptions, root: &JsSyntaxNode) -> FormatR
     rome_formatter::format_sub_tree(root, JsFormatLanguage::new(options))
 }
 
+#[derive(Copy, Clone, Debug)]
+pub(crate) enum JsLabels {
+    MemberChain,
+}
+
+impl Label for JsLabels {
+    fn id(&self) -> u64 {
+        *self as u64
+    }
+
+    fn debug_name(&self) -> &'static str {
+        match self {
+            JsLabels::MemberChain => "MemberChain",
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
 
diff --git a/crates/rome_js_formatter/src/utils/member_chain/mod.rs b/crates/rome_js_formatter/src/utils/member_chain/mod.rs
@@ -114,6 +114,7 @@ use crate::utils::member_chain::groups::{
     MemberChainGroup, MemberChainGroupsBuilder, TailChainGroups,
 };
 use crate::utils::member_chain::simple_argument::SimpleArgument;
+use crate::JsLabels;
 use rome_formatter::{write, Buffer};
 use rome_js_syntax::{
     AnyJsCallArgument, AnyJsExpression, AnyJsLiteralExpression, JsCallExpression,
@@ -122,8 +123,6 @@ use rome_js_syntax::{
 use rome_rowan::{AstNode, SyntaxResult};
 use std::iter::FusedIterator;
 
-pub(crate) enum MemberChainLabel {}
-
 #[derive(Debug, Clone)]
 pub(crate) struct MemberChain {
     root: JsCallExpression,
@@ -399,7 +398,10 @@ impl Format<JsFormatContext> for MemberChain {
 
         write!(
             f,
-            [labelled(LabelId::of::<MemberChainLabel>(), &format_content)]
+            [labelled(
+                LabelId::of(JsLabels::MemberChain),
+                &format_content
+            )]
         )
     }
 }
