From 30315b6eecb306ba3dee139da198bb684684092e Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Sun, 20 Aug 2023 23:53:15 +0200 Subject: [PATCH] fix(lint/useEnumInitailizers): ignore enum in ambient namespaces (#20) --- .../analyzers/style/use_enum_initializers.rs | 9 +-- .../style/useEnumInitializers/invalid.ts | 11 +++- .../style/useEnumInitializers/invalid.ts.snap | 57 +++++++++++++++++++ .../specs/style/useEnumInitializers/valid.ts | 18 ++++++ .../style/useEnumInitializers/valid.ts.snap | 18 ++++++ crates/rome_js_syntax/src/declaration_ext.rs | 20 +++++++ crates/rome_js_syntax/src/lib.rs | 1 + 7 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 crates/rome_js_syntax/src/declaration_ext.rs diff --git a/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs b/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs index 36c0ff6b791c..83c2d8bcf65e 100644 --- a/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs +++ b/crates/rome_js_analyze/src/analyzers/style/use_enum_initializers.rs @@ -4,10 +4,7 @@ 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, TsDeclareStatement, TsEnumDeclaration, - TsExportDeclareClause, -}; +use rome_js_syntax::{AnyJsExpression, AnyJsLiteralExpression, JsSyntaxKind, TsEnumDeclaration}; use rome_rowan::{AstNode, BatchMutationExt}; declare_rule! { @@ -86,9 +83,7 @@ impl Rule for UseEnumInitializers { fn run(ctx: &RuleContext) -> Self::Signals { let enum_declaration = ctx.query(); - if enum_declaration.parent::().is_some() - || enum_declaration.parent::().is_some() - { + if enum_declaration.is_ambient() { // In ambient declarations, enum members without initializers are opaque types. // They generally represent an enum with complex initializers. return None; diff --git a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts index 9f87222a8f94..9708b512ce3d 100644 --- a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts +++ b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts @@ -35,4 +35,13 @@ export enum IndexedColor { Red = "0", Green = "1", Blue, -} \ No newline at end of file +} + +export namespace A { + export namespace B { + export enum Enum { + A, + B, + } + } +} diff --git a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts.snap index 76e3f37c6708..8ebd11653631 100644 --- a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts.snap +++ b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/invalid.ts.snap @@ -42,6 +42,16 @@ export enum IndexedColor { Green = "1", Blue, } + +export namespace A { + export namespace B { + export enum Enum { + A, + B, + } + } +} + ``` # Diagnostics @@ -266,9 +276,56 @@ invalid.ts:34:13 lint/style/useEnumInitializers ━━━━━━━━━━ > 37 │ Blue, │ ^^^^ 38 │ } + 39 │ + + i Allowing implicit initializations for enum members can cause bugs if enum declarations are modified over time. + + +``` + +``` +invalid.ts:42:21 lint/style/useEnumInitializers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This enum declaration contains members that are implicitly initialized. + + 40 │ export namespace A { + 41 │ export namespace B { + > 42 │ export enum Enum { + │ ^^^^ + 43 │ A, + 44 │ B, + + i This enum member should be explicitly initialized. + + 41 │ export namespace B { + 42 │ export enum Enum { + > 43 │ A, + │ ^ + 44 │ B, + 45 │ } + + i This enum member should be explicitly initialized. + + 42 │ export enum Enum { + 43 │ A, + > 44 │ B, + │ ^ + 45 │ } + 46 │ } i Allowing implicit initializations for enum members can cause bugs if enum declarations are modified over time. + i Suggested fix: Initialize all enum members. + + 41 41 │ export namespace B { + 42 42 │ export enum Enum { + 43 │ - ············A, + 44 │ - ············B, + 43 │ + ············A·=·0, + 44 │ + ············B·=·1, + 45 45 │ } + 46 46 │ } + ``` diff --git a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts index 6887ff6cc272..f3205a76cf71 100644 --- a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts +++ b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts @@ -23,3 +23,21 @@ declare enum Weather2 { Rainy, Sunny, } + +export declare namespace A { + export namespace B { + export enum Enum { + A, + B, + } + } +} + +declare namespace A2 { + export namespace B2 { + export enum Enum { + A, + B, + } + } +} diff --git a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts.snap b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts.snap index a9ecb08ed23b..a2b89fc2262d 100644 --- a/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts.snap +++ b/crates/rome_js_analyze/tests/specs/style/useEnumInitializers/valid.ts.snap @@ -30,6 +30,24 @@ declare enum Weather2 { Sunny, } +export declare namespace A { + export namespace B { + export enum Enum { + A, + B, + } + } +} + +declare namespace A2 { + export namespace B2 { + export enum Enum { + A, + B, + } + } +} + ``` diff --git a/crates/rome_js_syntax/src/declaration_ext.rs b/crates/rome_js_syntax/src/declaration_ext.rs new file mode 100644 index 000000000000..08c39303ef90 --- /dev/null +++ b/crates/rome_js_syntax/src/declaration_ext.rs @@ -0,0 +1,20 @@ +use rome_rowan::AstNode; + +use crate::{JsSyntaxKind, JsSyntaxNode, TsEnumDeclaration}; + +impl TsEnumDeclaration { + /// Returns `true` if this enum is an ambient enum or in an ambient context. + pub fn is_ambient(&self) -> bool { + is_in_ambient_context(self.syntax()) + } +} + +/// Returns `true` if `syntax` is in an ambient context. +fn is_in_ambient_context(syntax: &JsSyntaxNode) -> bool { + syntax.ancestors().any(|x| { + matches!( + x.kind(), + JsSyntaxKind::TS_DECLARE_STATEMENT | JsSyntaxKind::TS_EXPORT_DECLARE_CLAUSE + ) + }) +} diff --git a/crates/rome_js_syntax/src/lib.rs b/crates/rome_js_syntax/src/lib.rs index 63c703d7968f..b2d25b012af5 100644 --- a/crates/rome_js_syntax/src/lib.rs +++ b/crates/rome_js_syntax/src/lib.rs @@ -5,6 +5,7 @@ #[macro_use] mod generated; pub mod binding_ext; +pub mod declaration_ext; pub mod directive_ext; pub mod expr_ext; pub mod file_source;