refactor(rome_js_analyze): improve useEnumInitializers (#4733)

CHANGELOG.md

@@ -201,6 +201,20 @@ if no error diagnostics are emitted.
 
 -  Fix [`noDuplicateCase`](https://docs.rome.tools/lint/rules/noDuplicateCase/) rule that erroneously reported as equals the strings literals `"'"` and `'"'` [#4706](https://github.com/rome/tools/issues/4706).
 
+- Improve [`useEnumInitializers`](https://docs.rome.tools/lint/rules/useEnumInitializers/)
+
+  The rule now reports all uninitialized members of an enum in a single diagnostic.
+
+  Moreover, ambient enum declarations are now ignored.
+  This avoids reporting ambient enum declarations in _TypeScript_ declaration files.
+
+  ```ts
+  declare enum Weather {
+    Rainy,
+    Sunny,
+  }
+  ```
+
 - Relax [`noBannedTypes`](https://docs.rome.tools/lint/rules/nobannedtypes/) and improve documentation
 
   The rule no longer reports a user type that reuses a banned type name.

crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs

@@ -4,15 +4,20 @@ use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
 use rome_console::markup;
 use rome_diagnostics::Applicability;
 use rome_js_factory::make;
-use rome_js_syntax::{AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsEnumMember};
-use rome_rowan::{AstNode, BatchMutationExt, Direction};
+use rome_js_syntax::{
+    AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsDeclareStatement, TsEnumDeclaration,
+    TsExportDeclareClause,
+};
+use rome_rowan::{AstNode, BatchMutationExt};
 
 declare_rule! {
     /// Require that each enum member value be explicitly initialized.
     ///
-    /// TypeScript enums are a practical way to organize semantically related constant values. Members of enums that don't have explicit values are by default given sequentially increasing numbers.
+    /// _TypeScript_ enums are a practical way to organize semantically related constant values.
+    /// Members of enums that don't have explicit values are by default given sequentially increasing numbers.
     ///
-    /// When the value of enum members are important, allowing implicit values for enum members can cause bugs if enum declarations are modified over time.
+    /// When the value of enum members are important,
+    /// allowing implicit values for enum members can cause bugs if enum declarations are modified over time.
     ///
     /// Source: https://typescript-eslint.io/rules/prefer-enum-initializers
     ///
@@ -58,6 +63,12 @@ declare_rule! {
     /// }
     /// ```
     ///
+    /// ```ts
+    /// declare enum Weather {
+    ///     Rainy,
+    ///     Sunny,
+    /// }
+    /// ```
     pub(crate) UseEnumInitializers {
         version: "11.0.0",
         name: "useEnumInitializers",
@@ -66,92 +77,126 @@ declare_rule! {
 }
 
 impl Rule for UseEnumInitializers {
-    type Query = Ast<TsEnumMember>;
+    // We apply the rule on an entire enum declaration to avoid reporting
+    // a diagnostic for every enum members without initializers.
+    type Query = Ast<TsEnumDeclaration>;
     type State = ();
     type Signals = Option<Self::State>;
     type Options = ();
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
-        let enum_member = ctx.query();
-        enum_member.initializer().is_none().then_some(())
+        let enum_declaration = ctx.query();
+        if enum_declaration.parent::<TsDeclareStatement>().is_some()
+            || enum_declaration.parent::<TsExportDeclareClause>().is_some()
+        {
+            // In ambient declarations, enum members without initializers are opaque types.
+            // They generally represent an enum with complex initializers.
+            return None;
+        }
+        for enum_member in enum_declaration.members() {
+            let enum_member = enum_member.ok()?;
+            if enum_member.initializer().is_none() {
+                return Some(());
+            }
+        }
+        None
     }
 
     fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
-        let enum_member = ctx.query();
-        Some(RuleDiagnostic::new(
+        let enum_declaration = ctx.query();
+        let mut diagnostic = RuleDiagnostic::new(
             rule_category!(),
-            enum_member.range(),
+            enum_declaration.id().ok()?.range(),
             markup! {
-                "The "<Emphasis>"enum member"</Emphasis>" should be initialized."
+                "This "<Emphasis>"enum declaration"</Emphasis>" contains members that are implicitly initialized."
             },
-        ).note(
-            "Allowing implicit values for enum members can cause bugs if enum declarations are modified over time."
+        );
+        for enum_member in enum_declaration.members() {
+            let enum_member = enum_member.ok()?;
+            if enum_member.initializer().is_none() {
+                diagnostic = diagnostic.detail(enum_member.range(), markup! {
+                    "This "<Emphasis>"enum member"</Emphasis>" should be explicitly initialized."
+                });
+            }
+        }
+        Some(diagnostic.note(
+            "Allowing implicit initializations for enum members can cause bugs if enum declarations are modified over time."
         ))
     }
 
     fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
-        let enum_member = ctx.query();
-        let prev_enum_members = enum_member
-            .syntax()
-            .siblings(Direction::Prev)
-            .skip(1) // consume enum_member
-            .filter_map(TsEnumMember::cast);
-        let mut count = 0;
-        let mut prev_enum_member_info = None;
-        for prev_enum_member in prev_enum_members {
-            count += 1;
-            if let Some(initializer) = prev_enum_member.initializer() {
-                prev_enum_member_info = Some((initializer, prev_enum_member.name().ok()?));
-                break;
-            }
-        }
-        let expr = if let Some((prev_initializer, prev_name)) = prev_enum_member_info {
-            let expr = prev_initializer.expression().ok()?;
-            let expr = expr.as_any_js_literal_expression()?;
-            match expr {
-                AnyJsLiteralExpression::JsNumberLiteralExpression(expr) => {
-                    Some(AnyJsLiteralExpression::JsNumberLiteralExpression(
-                        make::js_number_literal_expression(make::js_number_literal(
-                            expr.as_number()? + f64::from(count),
-                        )),
-                    ))
+        let enum_declaration = ctx.query();
+        let mut mutation = ctx.root().begin();
+        let mut has_mutations = false;
+        let mut next_member_value = EnumInitializer::Integer(0);
+        for enum_member in enum_declaration.members() {
+            let enum_member = enum_member.ok()?;
+            if let Some(initializer) = enum_member.initializer() {
+                next_member_value = EnumInitializer::Other;
+                let expr = initializer.expression().ok()?.omit_parentheses();
+                if let Some(expr) = expr.as_any_js_literal_expression() {
+                    match expr {
+                        AnyJsLiteralExpression::JsNumberLiteralExpression(expr) => {
+                            let n = expr.value_token().ok()?;
+                            let n = n.text_trimmed();
+                            if let Ok(n) = n.parse::<i64>() {
+                                next_member_value = EnumInitializer::Integer(n + 1);
+                            }
+                        }
+                        AnyJsLiteralExpression::JsStringLiteralExpression(expr) => {
+                            if enum_member.name().ok()?.name() == expr.inner_string_text().ok() {
+                                next_member_value = EnumInitializer::EnumName;
+                            }
+                        }
+                        _ => {}
+                    }
                 }
-                AnyJsLiteralExpression::JsStringLiteralExpression(expr) => {
-                    let prev_enum_val = expr.inner_string_text().ok()?;
-                    if prev_name.name() == Some(prev_enum_val) {
-                        let enum_name = enum_member.name().ok()?.text();
+            } else {
+                let x = match next_member_value {
+                    EnumInitializer::Integer(n) => {
+                        next_member_value = EnumInitializer::Integer(n + 1);
+                        Some(AnyJsLiteralExpression::JsNumberLiteralExpression(
+                            make::js_number_literal_expression(make::js_number_literal(n)),
+                        ))
+                    }
+                    EnumInitializer::EnumName => {
+                        let enum_name = enum_member.name().ok()?.name()?;
+                        let enum_name = enum_name.text();
                         Some(AnyJsLiteralExpression::JsStringLiteralExpression(
-                            make::js_string_literal_expression(make::js_string_literal(&enum_name)),
+                            make::js_string_literal_expression(make::js_string_literal(enum_name)),
                         ))
-                    } else {
-                        None
                     }
+                    EnumInitializer::Other => None,
+                };
+                if let Some(x) = x {
+                    has_mutations = true;
+                    let new_enum_member =
+                        enum_member
+                            .clone()
+                            .with_initializer(Some(make::js_initializer_clause(
+                                make::token_decorated_with_space(JsSyntaxKind::EQ),
+                                AnyJsExpression::AnyJsLiteralExpression(x),
+                            )));
+                    mutation.replace_node_discard_trivia(enum_member, new_enum_member);
                 }
-                _ => None,
             }
-        } else {
-            Some(AnyJsLiteralExpression::JsNumberLiteralExpression(
-                make::js_number_literal_expression(make::js_number_literal(count)),
-            ))
-        };
-        if let Some(expr) = expr {
-            let new_enum_member =
-                enum_member
-                    .to_owned()
-                    .with_initializer(Some(make::js_initializer_clause(
-                        make::token_decorated_with_space(JsSyntaxKind::EQ),
-                        AnyJsExpression::AnyJsLiteralExpression(expr),
-                    )));
-            let mut mutation = ctx.root().begin();
-            mutation.replace_node_discard_trivia(enum_member.to_owned(), new_enum_member);
-            Some(JsRuleAction {
+        }
+        if has_mutations {
+            return Some(JsRuleAction {
                 category: ActionCategory::QuickFix,
                 applicability: Applicability::MaybeIncorrect,
-                message: markup! { "" }.to_owned(),
+                message: markup! { "Initialize all enum members." }.to_owned(),
                 mutation,
-            })
-        } else {
-            None
+            });
         }
+        None
     }
 }
+
+enum EnumInitializer {
+    // The member is initialized with an integer
+    Integer(i64),
+    /// The member name is equal to the member value
+    EnumName,
+    Other,
+}

crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts

@@ -24,6 +24,13 @@ export enum Color {
 	Blue,
 }
 
+export enum Exotic {
+	A = 0.1,
+	B,
+	C = "Special",
+	D,
+}
+
 export enum IndexedColor {
 	Red = "0",
 	Green = "1",