From 269069103cd0887ee584b97c18dbc254743b629d Mon Sep 17 00:00:00 2001 From: burlinchen Date: Thu, 8 Aug 2024 12:16:33 +0800 Subject: [PATCH 1/9] fix(linter): improve prefer_namespace_keyword rule This commit enhances the `prefer_namespace_keyword` rule in the TypeScript linter: - Introduce helper functions `is_nest_module`, `is_valid_module`, and `is_invalid_module` to improve code readability and maintainability - Refactor the main `run` function to use these new helper functions - Add support for detecting and fixing nested module declarations (e.g., `module A.B {}`) - Update test cases to cover nested module scenarios - Improve error reporting for nested modules --- .../typescript/prefer_namespace_keyword.rs | 43 ++++++++++++++++--- .../snapshots/prefer_namespace_keyword.snap | 13 ++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index aaa3abff1cc9c..667d66c20802e 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -1,5 +1,5 @@ use oxc_ast::{ - ast::{TSModuleDeclarationKind, TSModuleDeclarationName}, + ast::{TSModuleDeclaration, TSModuleDeclarationKind, TSModuleDeclarationName}, AstKind, }; use oxc_diagnostics::OxcDiagnostic; @@ -34,16 +34,34 @@ declare_oxc_lint!( fix ); +fn is_nest_module(node: &AstNode, ctx: &LintContext<'_>) -> bool { + ctx.nodes() + .parent_node(node.id()) + .map_or(false, |parent_node| is_valid_module(parent_node, ctx)) +} + +fn is_valid_module(node: &AstNode, _ctx: &LintContext<'_>) -> bool { + matches!(node.kind(), AstKind::TSModuleDeclaration(module) if !is_invalid_module(module)) +} + +fn is_invalid_module(module: &TSModuleDeclaration) -> bool { + module.id.is_string_literal() + || !matches!(module.id, TSModuleDeclarationName::Identifier(_)) + || module.kind != TSModuleDeclarationKind::Module +} + impl Rule for PreferNamespaceKeyword { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::TSModuleDeclaration(module) = node.kind() else { return }; - if module.id.is_string_literal() - || !matches!(module.id, TSModuleDeclarationName::Identifier(_)) - || module.kind != TSModuleDeclarationKind::Module - { + + if is_invalid_module(module) { return; } + if is_nest_module(node, ctx) { + return ctx.diagnostic(prefer_namespace_keyword_diagnostic(module.span)); + } + ctx.diagnostic_with_fix(prefer_namespace_keyword_diagnostic(module.span), |fixer| { let span_size = u32::try_from("module".len()).unwrap_or(6); let span_start = if module.declare { @@ -74,6 +92,7 @@ fn test() { let fail = vec![ "module foo {}", + "module A.B {}", "declare module foo {}", " declare module foo { @@ -88,6 +107,20 @@ fn test() { let fix = vec![ ("module foo {}", "namespace foo {}", None), + ("module A.B {}", "namespace A.B {}", None), + ( + " + declare module A { + declare module B {} + } + ", + " + declare namespace A { + declare namespace B {} + } + ", + None, + ), ("declare module foo {}", "declare namespace foo {}", None), ( " diff --git a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap index ef176c17bffc4..74d1256b98463 100644 --- a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap +++ b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap @@ -8,6 +8,19 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Replace `module` with `namespace`. + ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. + ╭─[prefer_namespace_keyword.tsx:1:1] + 1 │ module A.B {} + · ───────────── + ╰──── + help: Replace `module` with `namespace`. + + ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. + ╭─[prefer_namespace_keyword.tsx:1:10] + 1 │ module A.B {} + · ──── + ╰──── + ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. ╭─[prefer_namespace_keyword.tsx:1:1] 1 │ declare module foo {} From 32966bd10002981e45d36defcc4dc877498ebb64 Mon Sep 17 00:00:00 2001 From: burlinchen Date: Thu, 8 Aug 2024 12:37:28 +0800 Subject: [PATCH 2/9] test(linter): correct nested module handling in prefer_namespace_keyword rule --- .../src/rules/typescript/prefer_namespace_keyword.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index 667d66c20802e..468f881515bd9 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -110,13 +110,13 @@ fn test() { ("module A.B {}", "namespace A.B {}", None), ( " - declare module A { - declare module B {} + module A { + module B {} } ", " - declare namespace A { - declare namespace B {} + namespace A { + namespace B {} } ", None, From 914ffe1e5374928bc7bdef26e62e0f09334f448b Mon Sep 17 00:00:00 2001 From: burlinchen Date: Thu, 8 Aug 2024 16:56:41 +0800 Subject: [PATCH 3/9] chore(linter): Removed unnecessary context parameter from `is_valid_module` function. --- .../src/rules/typescript/prefer_namespace_keyword.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index 468f881515bd9..661da8de50cf3 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -37,10 +37,10 @@ declare_oxc_lint!( fn is_nest_module(node: &AstNode, ctx: &LintContext<'_>) -> bool { ctx.nodes() .parent_node(node.id()) - .map_or(false, |parent_node| is_valid_module(parent_node, ctx)) + .map_or(false, |parent_node| is_valid_module(parent_node)) } -fn is_valid_module(node: &AstNode, _ctx: &LintContext<'_>) -> bool { +fn is_valid_module(node: &AstNode) -> bool { matches!(node.kind(), AstKind::TSModuleDeclaration(module) if !is_invalid_module(module)) } From 264292a97df72211f29b4d51063dcf1b9fd73d90 Mon Sep 17 00:00:00 2001 From: burlinchen Date: Thu, 8 Aug 2024 17:25:13 +0800 Subject: [PATCH 4/9] chore(linter): Refactor `prefer_namespace_keyword` rule in TypeScript linter - Simplified `is_nest_module` function by removing redundant line breaks. --- .../src/rules/typescript/prefer_namespace_keyword.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index 661da8de50cf3..399b6add0d7b7 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -35,9 +35,7 @@ declare_oxc_lint!( ); fn is_nest_module(node: &AstNode, ctx: &LintContext<'_>) -> bool { - ctx.nodes() - .parent_node(node.id()) - .map_or(false, |parent_node| is_valid_module(parent_node)) + ctx.nodes().parent_node(node.id()).map_or(false, |parent_node| is_valid_module(parent_node)) } fn is_valid_module(node: &AstNode) -> bool { From 6ae58c11339b0307dbb911712b5844000ab98215 Mon Sep 17 00:00:00 2001 From: burlinchen Date: Fri, 9 Aug 2024 07:48:25 +0800 Subject: [PATCH 5/9] fix(linter): remove redundant diagnostic return in `prefer_namespace_keyword` rule - Removed an unnecessary return statement in the `prefer_namespace_keyword` rule to ensure proper execution flow and prevent premature diagnostic reporting. --- .../typescript/prefer_namespace_keyword.rs | 30 +++++++++---------- .../snapshots/prefer_namespace_keyword.snap | 6 ---- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index 399b6add0d7b7..9a79591616288 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -57,7 +57,7 @@ impl Rule for PreferNamespaceKeyword { } if is_nest_module(node, ctx) { - return ctx.diagnostic(prefer_namespace_keyword_diagnostic(module.span)); + return; } ctx.diagnostic_with_fix(prefer_namespace_keyword_diagnostic(module.span), |fixer| { @@ -108,29 +108,29 @@ fn test() { ("module A.B {}", "namespace A.B {}", None), ( " - module A { - module B {} + module A { + module B {} } - ", + ", " - namespace A { - namespace B {} + namespace A { + namespace B {} } - ", + ", None, ), ("declare module foo {}", "declare namespace foo {}", None), ( " - declare module foo { - declare module bar {} - } - ", + declare module foo { + declare module bar {} + } + ", " - declare namespace foo { - declare namespace bar {} - } - ", + declare namespace foo { + declare namespace bar {} + } + ", None, ), ]; diff --git a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap index 74d1256b98463..fd8ce43923108 100644 --- a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap +++ b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap @@ -15,12 +15,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Replace `module` with `namespace`. - ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. - ╭─[prefer_namespace_keyword.tsx:1:10] - 1 │ module A.B {} - · ──── - ╰──── - ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. ╭─[prefer_namespace_keyword.tsx:1:1] 1 │ declare module foo {} From d491dc71d28b25ba7ece5443eab148fbb00de78d Mon Sep 17 00:00:00 2001 From: burlinchen Date: Fri, 9 Aug 2024 15:49:16 +0800 Subject: [PATCH 6/9] chore(linter): Refactor module validation functions in `prefer_namespace_keyword.rs` - Renamed `is_invalid_module` to `is_valid_module` and updated its logic to check for valid modules. - Renamed `is_valid_module` to `is_valid_module_node` to reflect its purpose more clearly. - Modified `is_nest_module` to use the updated `is_valid_module_node`. - Updated the `PreferNamespaceKeyword` rule to use the new validation logic, ensuring correct module validation and nesting checks. --- .../typescript/prefer_namespace_keyword.rs | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index 9a79591616288..93105ad19020b 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -35,28 +35,26 @@ declare_oxc_lint!( ); fn is_nest_module(node: &AstNode, ctx: &LintContext<'_>) -> bool { - ctx.nodes().parent_node(node.id()).map_or(false, |parent_node| is_valid_module(parent_node)) + ctx.nodes() + .parent_node(node.id()) + .map_or(false, |parent_node| is_valid_module_node(parent_node)) } -fn is_valid_module(node: &AstNode) -> bool { - matches!(node.kind(), AstKind::TSModuleDeclaration(module) if !is_invalid_module(module)) +fn is_valid_module_node(node: &AstNode) -> bool { + matches!(node.kind(), AstKind::TSModuleDeclaration(module) if is_valid_module(module)) } -fn is_invalid_module(module: &TSModuleDeclaration) -> bool { - module.id.is_string_literal() - || !matches!(module.id, TSModuleDeclarationName::Identifier(_)) - || module.kind != TSModuleDeclarationKind::Module +fn is_valid_module(module: &TSModuleDeclaration) -> bool { + !module.id.is_string_literal() + && matches!(module.id, TSModuleDeclarationName::Identifier(_)) + && module.kind == TSModuleDeclarationKind::Module } impl Rule for PreferNamespaceKeyword { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::TSModuleDeclaration(module) = node.kind() else { return }; - if is_invalid_module(module) { - return; - } - - if is_nest_module(node, ctx) { + if !is_valid_module(module) || is_nest_module(node, ctx) { return; } @@ -110,12 +108,12 @@ fn test() { " module A { module B {} - } + } ", " namespace A { namespace B {} - } + } ", None, ), @@ -125,12 +123,12 @@ fn test() { declare module foo { declare module bar {} } - ", + ", " declare namespace foo { declare namespace bar {} } - ", + ", None, ), ]; From 9c0860e729ce0f93aed31863c48c289f0d13f9a5 Mon Sep 17 00:00:00 2001 From: rzvxa Date: Fri, 9 Aug 2024 11:36:58 +0330 Subject: [PATCH 7/9] fmt --- .../src/rules/typescript/prefer_namespace_keyword.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index 93105ad19020b..f9e155cc52b71 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -90,15 +90,12 @@ fn test() { "module foo {}", "module A.B {}", "declare module foo {}", - " - declare module foo { - declare module bar {} - } - ", + "declare module foo { + declare module bar {} + }", "declare global { module foo {} - } - ", + }", ]; let fix = vec![ From 6fbf7622d2874f6ca01a5db91f9b728145b076ee Mon Sep 17 00:00:00 2001 From: rzvxa Date: Fri, 9 Aug 2024 11:40:40 +0330 Subject: [PATCH 8/9] chore: update snapshot. --- .../snapshots/prefer_namespace_keyword.snap | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap index fd8ce43923108..05139bbe10ed8 100644 --- a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap +++ b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap @@ -23,21 +23,19 @@ source: crates/oxc_linter/src/tester.rs help: Replace `module` with `namespace`. ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. - ╭─[prefer_namespace_keyword.tsx:2:4] - 1 │ - 2 │ ╭─▶ declare module foo { - 3 │ │ declare module bar {} - 4 │ ╰─▶ } - 5 │ + ╭─[prefer_namespace_keyword.tsx:1:1] + 1 │ ╭─▶ declare module foo { + 2 │ │ declare module bar {} + 3 │ ╰─▶ } ╰──── help: Replace `module` with `namespace`. ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. - ╭─[prefer_namespace_keyword.tsx:3:6] - 2 │ declare module foo { - 3 │ declare module bar {} - · ───────────────────── - 4 │ } + ╭─[prefer_namespace_keyword.tsx:2:11] + 1 │ declare module foo { + 2 │ declare module bar {} + · ───────────────────── + 3 │ } ╰──── help: Replace `module` with `namespace`. From e879ea12bdf3284702f2d2f4d78e9f380d68d388 Mon Sep 17 00:00:00 2001 From: rzvxa Date: Fri, 9 Aug 2024 11:42:21 +0330 Subject: [PATCH 9/9] fmt again --- .../typescript/prefer_namespace_keyword.rs | 12 ++++++--- .../snapshots/prefer_namespace_keyword.snap | 26 ++++++++++--------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs index f9e155cc52b71..30107cdabb363 100644 --- a/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs +++ b/crates/oxc_linter/src/rules/typescript/prefer_namespace_keyword.rs @@ -90,12 +90,16 @@ fn test() { "module foo {}", "module A.B {}", "declare module foo {}", - "declare module foo { + " + declare module foo { declare module bar {} - }", - "declare global { + } + ", + " + declare global { module foo {} - }", + } + ", ]; let fix = vec![ diff --git a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap index 05139bbe10ed8..336b624ce33a3 100644 --- a/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap +++ b/crates/oxc_linter/src/snapshots/prefer_namespace_keyword.snap @@ -23,27 +23,29 @@ source: crates/oxc_linter/src/tester.rs help: Replace `module` with `namespace`. ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. - ╭─[prefer_namespace_keyword.tsx:1:1] - 1 │ ╭─▶ declare module foo { - 2 │ │ declare module bar {} - 3 │ ╰─▶ } + ╭─[prefer_namespace_keyword.tsx:2:9] + 1 │ + 2 │ ╭─▶ declare module foo { + 3 │ │ declare module bar {} + 4 │ ╰─▶ } + 5 │ ╰──── help: Replace `module` with `namespace`. ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. - ╭─[prefer_namespace_keyword.tsx:2:11] - 1 │ declare module foo { - 2 │ declare module bar {} + ╭─[prefer_namespace_keyword.tsx:3:11] + 2 │ declare module foo { + 3 │ declare module bar {} · ───────────────────── - 3 │ } + 4 │ } ╰──── help: Replace `module` with `namespace`. ⚠ typescript-eslint(prefer-namespace-keyword): Use 'namespace' instead of 'module' to declare custom TypeScript modules. - ╭─[prefer_namespace_keyword.tsx:2:13] - 1 │ declare global { - 2 │ module foo {} + ╭─[prefer_namespace_keyword.tsx:3:13] + 2 │ declare global { + 3 │ module foo {} · ───────────── - 3 │ } + 4 │ } ╰──── help: Replace `module` with `namespace`.