From 8d511f7f79ac69e2e728a3eccd2a57788fe73678 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Fri, 28 Jul 2023 23:05:26 +0200 Subject: [PATCH 1/2] feat(rome_js_analyze): noUnsafeDeclarationMerging --- CHANGELOG.md | 4 + .../src/categories.rs | 1 + .../src/semantic_analyzers/nursery.rs | 2 + .../nursery/no_unsafe_declaration_merging.rs | 108 ++++++++++++++++++ .../noUnsafeDeclarationMerging/invalid.ts | 17 +++ .../invalid.ts.snap | 101 ++++++++++++++++ .../noUnsafeDeclarationMerging/valid.ts | 29 +++++ .../noUnsafeDeclarationMerging/valid.ts.snap | 39 +++++++ .../src/configuration/linter/rules.rs | 106 ++++++++++------- .../src/configuration/parse/json/rules.rs | 24 ++++ editors/vscode/configuration_schema.json | 7 ++ npm/backend-jsonrpc/src/workspace.ts | 5 + npm/rome/configuration_schema.json | 7 ++ .../components/generated/NumberOfRules.astro | 2 +- website/src/pages/lint/rules/index.mdx | 6 + .../lint/rules/noUnsafeDeclarationMerging.md | 71 ++++++++++++ 16 files changed, 487 insertions(+), 42 deletions(-) create mode 100644 crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unsafe_declaration_merging.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts.snap create mode 100644 website/src/pages/lint/rules/noUnsafeDeclarationMerging.md diff --git a/CHANGELOG.md b/CHANGELOG.md index af278b13a15..938b2d6676c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -142,6 +142,10 @@ if no error diagnostics are emitted. This rule recommends using `Number.isNaN` instead of the global and unsafe `isNaN` that attempts a type coercion. +- Add [`noUnsafeDeclarationMerging`](https://docs.rome.tools/lint/rules/noUnsafeDeclarationMerging/) + + This rule disallows declaration merging between an interface and a class. + - Add [`useArrowFunction`](https://docs.rome.tools/lint/rules/usearrowfunction/) This rule proposes turning function expressions into arrow functions. diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 9b3b5589adf..664c37dcce8 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -94,6 +94,7 @@ define_categories! { "lint/nursery/noRedundantRoles": "https://docs.rome.tools/lint/rules/noRedundantRoles", "lint/nursery/noSelfAssign": "https://docs.rome.tools/lint/rules/noSelfAssign", "lint/nursery/noStaticOnlyClass": "https://docs.rome.tools/lint/rules/noStaticOnlyClass", + "lint/nursery/noUnsafeDeclarationMerging": "https://docs.rome.tools/lint/rules/noUnsafeDeclarationMerging", "lint/nursery/noUselessEmptyExport": "https://docs.rome.tools/lint/rules/noUselessEmptyExport", "lint/nursery/noVoid": "https://docs.rome.tools/lint/rules/noVoid", "lint/nursery/useAriaPropTypes": "https://docs.rome.tools/lint/rules/useAriaPropTypes", diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs index ebb4d4c589b..b801c26369b 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs @@ -8,6 +8,7 @@ pub(crate) mod no_console_log; pub(crate) mod no_constant_condition; pub(crate) mod no_global_is_finite; pub(crate) mod no_global_is_nan; +pub(crate) mod no_unsafe_declaration_merging; pub(crate) mod use_camel_case; pub(crate) mod use_exhaustive_dependencies; pub(crate) mod use_hook_at_top_level; @@ -25,6 +26,7 @@ declare_group! { self :: no_constant_condition :: NoConstantCondition , self :: no_global_is_finite :: NoGlobalIsFinite , self :: no_global_is_nan :: NoGlobalIsNan , + self :: no_unsafe_declaration_merging :: NoUnsafeDeclarationMerging , self :: use_camel_case :: UseCamelCase , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel , diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unsafe_declaration_merging.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unsafe_declaration_merging.rs new file mode 100644 index 00000000000..08a90c99f59 --- /dev/null +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unsafe_declaration_merging.rs @@ -0,0 +1,108 @@ +use crate::semantic_services::Semantic; +use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic}; +use rome_console::markup; +use rome_js_syntax::{ + binding_ext::AnyJsBindingDeclaration, TsDeclareStatement, TsExportDeclareClause, + TsInterfaceDeclaration, +}; +use rome_rowan::{AstNode, TextRange}; + +declare_rule! { + /// Disallow unsafe declaration merging between interfaces and classes. + /// + /// _TypeScript_'s [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html) supports merging separate declarations with the same name. + /// + /// _Declaration merging_ between classes and interfaces is unsafe. + /// The _TypeScript Compiler_ doesn't check whether properties defined in the interface are initialized in the class. + /// This can cause lead to _TypeScript_ not detecting code that will cause runtime errors. + /// + /// Source: https://typescript-eslint.io/rules/no-unsafe-declaration-merging/ + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```ts,expect_diagnostic + /// interface Foo { + /// f(): void + /// } + /// + /// class Foo {} + /// + /// const foo = new Foo(); + /// foo.f(); // Runtime Error: Cannot read properties of undefined. + /// ``` + /// + /// ## Valid + /// + /// ```ts + /// interface Foo {} + /// class Bar implements Foo {} + /// ``` + /// + /// ```ts + /// namespace Baz {} + /// namespace Baz {} + /// enum Baz {} + /// ``` + pub(crate) NoUnsafeDeclarationMerging { + version: "next", + name: "noUnsafeDeclarationMerging", + recommended: true, + } +} + +impl Rule for NoUnsafeDeclarationMerging { + type Query = Semantic; + type State = TextRange; + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let ts_interface = ctx.query(); + let model = ctx.model(); + let interface_binding = ts_interface.id().ok()?; + let interface_name = interface_binding.name_token().ok()?; + let scope = model.scope(ts_interface.syntax()).parent()?; + for binding in scope.bindings() { + if let Some(AnyJsBindingDeclaration::JsClassDeclaration(class)) = + binding.tree().declaration() + { + // This is not unsafe of merging an interface and an ambient class. + if class.parent::().is_none() + && class.parent::().is_none() + { + if let Ok(id) = class.id() { + if let Some(id) = id.as_js_identifier_binding() { + if let Ok(name) = id.name_token() { + if name.text() == interface_name.text() { + return Some(name.text_trimmed_range()); + } + } + } + } + } + } + } + None + } + + fn diagnostic(ctx: &RuleContext, class_range: &Self::State) -> Option { + let ts_interface = ctx.query(); + Some( + RuleDiagnostic::new( + rule_category!(), + class_range, + markup! { + "This ""class"" is unsafely merged with an ""interface""." + }, + ) + .detail(ts_interface.id().ok()?.range(), markup! { + "The ""interface"" is declared here." + }) + .note(markup! { + "The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class." + }), + ) + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts new file mode 100644 index 00000000000..387de34d047 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts @@ -0,0 +1,17 @@ +interface Foo {} +class Foo {} + +class Foo2 {} +interface Foo2 {} + +function f(){ + interface Foo3 {} + class Foo3 {} +} + +{ + interface Foo4 {} + class Foo4 {} +} + +export {} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap new file mode 100644 index 00000000000..ea49b405800 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap @@ -0,0 +1,101 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.ts +--- +# Input +```js +interface Foo {} +class Foo {} + +class Foo2 {} +interface Foo2 {} + +function f(){ + interface Foo3 {} + class Foo3 {} +} + +{ + interface Foo4 {} + class Foo4 {} +} + +export {} + +``` + +# Diagnostics +``` +invalid.ts:2:7 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This class is unsafely merged with an interface. + + 1 │ interface Foo {} + > 2 │ class Foo {} + │ ^^^ + 3 │ + 4 │ class Foo2 {} + + i The interface is declared here. + + > 1 │ interface Foo {} + │ ^^^ + 2 │ class Foo {} + 3 │ + + i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class. + + +``` + +``` +invalid.ts:4:7 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This class is unsafely merged with an interface. + + 2 │ class Foo {} + 3 │ + > 4 │ class Foo2 {} + │ ^^^^ + 5 │ interface Foo2 {} + 6 │ + + i The interface is declared here. + + 4 │ class Foo2 {} + > 5 │ interface Foo2 {} + │ ^^^^ + 6 │ + 7 │ function f(){ + + i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class. + + +``` + +``` +invalid.ts:9:11 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This class is unsafely merged with an interface. + + 7 │ function f(){ + 8 │ interface Foo3 {} + > 9 │ class Foo3 {} + │ ^^^^ + 10 │ } + 11 │ + + i The interface is declared here. + + 7 │ function f(){ + > 8 │ interface Foo3 {} + │ ^^^^ + 9 │ class Foo3 {} + 10 │ } + + i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class. + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts new file mode 100644 index 00000000000..81348d0594e --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts @@ -0,0 +1,29 @@ +interface Foo {} +class Bar implements Foo {} + +namespace Foo2 {} +namespace Foo2 {} + +enum Foo3 {} +namespace Foo3 {} + +namespace Foo4 {} +function Foo4() {} + +const Foo5 = class {}; + +interface Foo6 {} +function f6(){ + class Foo6 {}; +} + +class Foo7 {} +function f7(){ + interface Foo7 {} +} + +interface Foo8 {} +declare class Foo8 {} + +export interface Foo9 {} +export declare class Foo9 {} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts.snap new file mode 100644 index 00000000000..e077dc351d3 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/valid.ts.snap @@ -0,0 +1,39 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.ts +--- +# Input +```js +interface Foo {} +class Bar implements Foo {} + +namespace Foo2 {} +namespace Foo2 {} + +enum Foo3 {} +namespace Foo3 {} + +namespace Foo4 {} +function Foo4() {} + +const Foo5 = class {}; + +interface Foo6 {} +function f6(){ + class Foo6 {}; +} + +class Foo7 {} +function f7(){ + interface Foo7 {} +} + +interface Foo8 {} +declare class Foo8 {} + +export interface Foo9 {} +export declare class Foo9 {} + +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index a6ac8523c89..832ee742741 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -1908,6 +1908,15 @@ pub struct Nursery { #[bpaf(long("no-static-only-class"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] pub no_static_only_class: Option, + #[doc = "Disallow unsafe declaration merging between interfaces and classes."] + #[bpaf( + long("no-unsafe-declaration-merging"), + argument("on|off|warn"), + optional, + hide + )] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_unsafe_declaration_merging: Option, #[doc = "Disallow empty exports that don't change anything in a module file."] #[bpaf( long("no-useless-empty-export"), @@ -2005,7 +2014,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 35] = [ + pub(crate) const GROUP_RULES: [&'static str; 36] = [ "noAccumulatingSpread", "noAriaUnsupportedElements", "noBannedTypes", @@ -2025,6 +2034,7 @@ impl Nursery { "noRedundantRoles", "noSelfAssign", "noStaticOnlyClass", + "noUnsafeDeclarationMerging", "noUselessEmptyExport", "noVoid", "useAriaPropTypes", @@ -2042,7 +2052,7 @@ impl Nursery { "useNamingConvention", "useSimpleNumberKeys", ]; - const RECOMMENDED_RULES: [&'static str; 20] = [ + const RECOMMENDED_RULES: [&'static str; 21] = [ "noAriaUnsupportedElements", "noBannedTypes", "noConstantCondition", @@ -2055,6 +2065,7 @@ impl Nursery { "noRedundantRoles", "noSelfAssign", "noStaticOnlyClass", + "noUnsafeDeclarationMerging", "noUselessEmptyExport", "useArrowFunction", "useExhaustiveDependencies", @@ -2064,7 +2075,7 @@ impl Nursery { "useLiteralEnumMembers", "useLiteralKeys", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 20] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 21] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5]), @@ -2078,15 +2089,16 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 35] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 36] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), @@ -2122,6 +2134,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } @@ -2227,86 +2240,91 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.no_useless_empty_export.as_ref() { + if let Some(rule) = self.no_unsafe_declaration_merging.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.no_void.as_ref() { + if let Some(rule) = self.no_useless_empty_export.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.use_aria_prop_types.as_ref() { + if let Some(rule) = self.no_void.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.use_arrow_function.as_ref() { + if let Some(rule) = self.use_aria_prop_types.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.use_camel_case.as_ref() { + if let Some(rule) = self.use_arrow_function.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { + if let Some(rule) = self.use_camel_case.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); } } - if let Some(rule) = self.use_heading_content.as_ref() { + if let Some(rule) = self.use_grouped_type_import.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26])); } } - if let Some(rule) = self.use_hook_at_top_level.as_ref() { + if let Some(rule) = self.use_heading_content.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.use_hook_at_top_level.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28])); } } - if let Some(rule) = self.use_is_array.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.use_is_nan.as_ref() { + if let Some(rule) = self.use_is_array.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } - if let Some(rule) = self.use_literal_enum_members.as_ref() { + if let Some(rule) = self.use_is_nan.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); } } - if let Some(rule) = self.use_literal_keys.as_ref() { + if let Some(rule) = self.use_literal_enum_members.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); } } - if let Some(rule) = self.use_naming_convention.as_ref() { + if let Some(rule) = self.use_literal_keys.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33])); } } - if let Some(rule) = self.use_simple_number_keys.as_ref() { + if let Some(rule) = self.use_naming_convention.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34])); } } + if let Some(rule) = self.use_simple_number_keys.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2406,86 +2424,91 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.no_useless_empty_export.as_ref() { + if let Some(rule) = self.no_unsafe_declaration_merging.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.no_void.as_ref() { + if let Some(rule) = self.no_useless_empty_export.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.use_aria_prop_types.as_ref() { + if let Some(rule) = self.no_void.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.use_arrow_function.as_ref() { + if let Some(rule) = self.use_aria_prop_types.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.use_camel_case.as_ref() { + if let Some(rule) = self.use_arrow_function.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { + if let Some(rule) = self.use_camel_case.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); } } - if let Some(rule) = self.use_heading_content.as_ref() { + if let Some(rule) = self.use_grouped_type_import.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26])); } } - if let Some(rule) = self.use_hook_at_top_level.as_ref() { + if let Some(rule) = self.use_heading_content.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.use_import_restrictions.as_ref() { + if let Some(rule) = self.use_hook_at_top_level.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28])); } } - if let Some(rule) = self.use_is_array.as_ref() { + if let Some(rule) = self.use_import_restrictions.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.use_is_nan.as_ref() { + if let Some(rule) = self.use_is_array.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } - if let Some(rule) = self.use_literal_enum_members.as_ref() { + if let Some(rule) = self.use_is_nan.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); } } - if let Some(rule) = self.use_literal_keys.as_ref() { + if let Some(rule) = self.use_literal_enum_members.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); } } - if let Some(rule) = self.use_naming_convention.as_ref() { + if let Some(rule) = self.use_literal_keys.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33])); } } - if let Some(rule) = self.use_simple_number_keys.as_ref() { + if let Some(rule) = self.use_naming_convention.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34])); } } + if let Some(rule) = self.use_simple_number_keys.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2494,10 +2517,10 @@ impl Nursery { pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) } - pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 20] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 21] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 35] { Self::ALL_RULES_AS_FILTERS } + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 36] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] pub(crate) fn collect_preset_rules( &self, @@ -2537,6 +2560,7 @@ impl Nursery { "noRedundantRoles" => self.no_redundant_roles.as_ref(), "noSelfAssign" => self.no_self_assign.as_ref(), "noStaticOnlyClass" => self.no_static_only_class.as_ref(), + "noUnsafeDeclarationMerging" => self.no_unsafe_declaration_merging.as_ref(), "noUselessEmptyExport" => self.no_useless_empty_export.as_ref(), "noVoid" => self.no_void.as_ref(), "useAriaPropTypes" => self.use_aria_prop_types.as_ref(), diff --git a/crates/rome_service/src/configuration/parse/json/rules.rs b/crates/rome_service/src/configuration/parse/json/rules.rs index a68c3fc11f4..d83cb280093 100644 --- a/crates/rome_service/src/configuration/parse/json/rules.rs +++ b/crates/rome_service/src/configuration/parse/json/rules.rs @@ -1663,6 +1663,7 @@ impl VisitNode for Nursery { "noRedundantRoles", "noSelfAssign", "noStaticOnlyClass", + "noUnsafeDeclarationMerging", "noUselessEmptyExport", "noVoid", "useAriaPropTypes", @@ -2135,6 +2136,29 @@ impl VisitNode for Nursery { )); } }, + "noUnsafeDeclarationMerging" => match value { + AnyJsonValue::JsonStringValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; + self.no_unsafe_declaration_merging = Some(configuration); + } + AnyJsonValue::JsonObjectValue(_) => { + let mut rule_configuration = RuleConfiguration::default(); + rule_configuration.map_rule_configuration( + &value, + name_text, + "noUnsafeDeclarationMerging", + diagnostics, + )?; + self.no_unsafe_declaration_merging = Some(rule_configuration); + } + _ => { + diagnostics.push(DeserializationDiagnostic::new_incorrect_type( + "object or string", + value.range(), + )); + } + }, "noUselessEmptyExport" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 6c7871c4ffa..97a1f0799b1 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -921,6 +921,13 @@ { "type": "null" } ] }, + "noUnsafeDeclarationMerging": { + "description": "Disallow unsafe declaration merging between interfaces and classes.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noUselessEmptyExport": { "description": "Disallow empty exports that don't change anything in a module file.", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 2c5c41e0a8c..fee46edf71c 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -599,6 +599,10 @@ export interface Nursery { * This rule reports when a class has no non-static members, such as for a class used exclusively as a static namespace. */ noStaticOnlyClass?: RuleConfiguration; + /** + * Disallow unsafe declaration merging between interfaces and classes. + */ + noUnsafeDeclarationMerging?: RuleConfiguration; /** * Disallow empty exports that don't change anything in a module file. */ @@ -1192,6 +1196,7 @@ export type Category = | "lint/nursery/noRedundantRoles" | "lint/nursery/noSelfAssign" | "lint/nursery/noStaticOnlyClass" + | "lint/nursery/noUnsafeDeclarationMerging" | "lint/nursery/noUselessEmptyExport" | "lint/nursery/noVoid" | "lint/nursery/useAriaPropTypes" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 6c7871c4ffa..97a1f0799b1 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -921,6 +921,13 @@ { "type": "null" } ] }, + "noUnsafeDeclarationMerging": { + "description": "Disallow unsafe declaration merging between interfaces and classes.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noUselessEmptyExport": { "description": "Disallow empty exports that don't change anything in a module file.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index ea5c582e0f8..07715a7f29d 100644 --- a/website/src/components/generated/NumberOfRules.astro +++ b/website/src/components/generated/NumberOfRules.astro @@ -1,2 +1,2 @@ -

Rome's linter has a total of 154 rules

\ No newline at end of file +

Rome's linter has a total of 155 rules

\ No newline at end of file diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index e530fef7893..606bb8a785f 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -1011,6 +1011,12 @@ Disallow assignments where both sides are exactly the same. This rule reports when a class has no non-static members, such as for a class used exclusively as a static namespace.

+

+ noUnsafeDeclarationMerging +

+Disallow unsafe declaration merging between interfaces and classes. +
+

noUselessEmptyExport

diff --git a/website/src/pages/lint/rules/noUnsafeDeclarationMerging.md b/website/src/pages/lint/rules/noUnsafeDeclarationMerging.md new file mode 100644 index 00000000000..77ba6c766d1 --- /dev/null +++ b/website/src/pages/lint/rules/noUnsafeDeclarationMerging.md @@ -0,0 +1,71 @@ +--- +title: Lint Rule noUnsafeDeclarationMerging +parent: lint/rules/index +--- + +# noUnsafeDeclarationMerging (since vnext) + +Disallow unsafe declaration merging between interfaces and classes. + +_TypeScript_'s [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html) supports merging separate declarations with the same name. + +_Declaration merging_ between classes and interfaces is unsafe. +The _TypeScript Compiler_ doesn't check whether properties defined in the interface are initialized in the class. +This can cause lead to _TypeScript_ not detecting code that will cause runtime errors. + +Source: https://typescript-eslint.io/rules/no-unsafe-declaration-merging/ + +## Examples + +### Invalid + +```ts +interface Foo { + f(): void +} + +class Foo {} + +const foo = new Foo(); +foo.f(); // Runtime Error: Cannot read properties of undefined. +``` + +
nursery/noUnsafeDeclarationMerging.js:5:7 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━
+
+  ✖ This class is unsafely merged with an interface.
+  
+    3 │ }
+    4 │ 
+  > 5 │ class Foo {}
+      │       ^^^
+    6 │ 
+    7 │ const foo = new Foo();
+  
+  ℹ The interface is declared here.
+  
+  > 1 │ interface Foo {
+      │           ^^^
+    2 │     f(): void
+    3 │ }
+  
+  ℹ The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.
+  
+
+ +## Valid + +```ts +interface Foo {} +class Bar implements Foo {} +``` + +```ts +namespace Baz {} +namespace Baz {} +enum Baz {} +``` + +## Related links + +- [Disable a rule](/linter/#disable-a-lint-rule) +- [Rule options](/linter/#rule-options) From f5220d8ed47462074167d8ea15cea55b9cd26082 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Sat, 29 Jul 2023 00:39:28 +0200 Subject: [PATCH 2/2] fix(rome_js_semantic): do not hoist classes and TS declarations --- .../invalid.ts.snap | 25 +++++++++++++++ crates/rome_js_semantic/src/events.rs | 32 ++++++------------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap index ea49b405800..d0ca656af37 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnsafeDeclarationMerging/invalid.ts.snap @@ -98,4 +98,29 @@ invalid.ts:9:11 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━ ``` +``` +invalid.ts:14:11 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This class is unsafely merged with an interface. + + 12 │ { + 13 │ interface Foo4 {} + > 14 │ class Foo4 {} + │ ^^^^ + 15 │ } + 16 │ + + i The interface is declared here. + + 12 │ { + > 13 │ interface Foo4 {} + │ ^^^^ + 14 │ class Foo4 {} + 15 │ } + + i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class. + + +``` + diff --git a/crates/rome_js_semantic/src/events.rs b/crates/rome_js_semantic/src/events.rs index d24f5a8aa08..690dab01357 100644 --- a/crates/rome_js_semantic/src/events.rs +++ b/crates/rome_js_semantic/src/events.rs @@ -398,9 +398,15 @@ impl SemanticEventExtractor { self.push_binding_into_scope(None, &name_token, &parent_kind); self.export_function_expression(node, &parent); } - JS_CLASS_DECLARATION | JS_CLASS_EXPORT_DEFAULT_DECLARATION => { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(1); - self.push_binding_into_scope(hoisted_scope_id, &name_token, &parent_kind); + JS_CLASS_DECLARATION + | JS_CLASS_EXPORT_DEFAULT_DECLARATION + | TS_ENUM_DECLARATION + | TS_INTERFACE_DECLARATION + | TS_MODULE_DECLARATION + | TS_TYPE_ALIAS_DECLARATION => { + let parent_scope = self.scopes.get(self.scopes.len() - 2); + let parent_scope = parent_scope.map(|scope| scope.scope_id); + self.push_binding_into_scope(parent_scope, &name_token, &parent_kind); self.export_declaration(node, &parent); } JS_CLASS_EXPRESSION => { @@ -437,26 +443,6 @@ impl SemanticEventExtractor { self.export_variable_declarator(node, &possible_declarator); } } - TS_TYPE_ALIAS_DECLARATION => { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(1); - self.push_binding_into_scope(hoisted_scope_id, &name_token, &parent_kind); - self.export_declaration(node, &parent); - } - TS_ENUM_DECLARATION => { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(1); - self.push_binding_into_scope(hoisted_scope_id, &name_token, &parent_kind); - self.export_declaration(node, &parent); - } - TS_INTERFACE_DECLARATION => { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(1); - self.push_binding_into_scope(hoisted_scope_id, &name_token, &parent_kind); - self.export_declaration(node, &parent); - } - TS_MODULE_DECLARATION => { - let hoisted_scope_id = self.scope_index_to_hoist_declarations(1); - self.push_binding_into_scope(hoisted_scope_id, &name_token, &parent_kind); - self.export_declaration(node, &parent); - } _ => { self.push_binding_into_scope(None, &name_token, &parent_kind); }