From 29bfdf81a6bb5d9642f3fc5528b4ad40244af8ae Mon Sep 17 00:00:00 2001 From: kaioduarte Date: Sat, 10 Dec 2022 23:15:52 +0000 Subject: [PATCH 1/4] feat(rome_js_analyze): `noClassAssign` rule --- .../src/categories.rs | 1 + .../rome_js_analyze/src/analyzers/nursery.rs | 3 +- .../src/analyzers/nursery/no_class_assign.rs | 115 +++++++ .../tests/specs/nursery/noClassAssign.js | 111 +++++++ .../tests/specs/nursery/noClassAssign.js.snap | 293 ++++++++++++++++++ .../src/configuration/linter/rules.rs | 23 +- editors/vscode/configuration_schema.json | 11 + npm/backend-jsonrpc/src/workspace.ts | 5 + npm/rome/configuration_schema.json | 11 + website/src/pages/lint/rules/index.mdx | 6 + website/src/pages/lint/rules/noClassAssign.md | 141 +++++++++ 11 files changed, 710 insertions(+), 10 deletions(-) create mode 100644 crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap create mode 100644 website/src/pages/lint/rules/noClassAssign.md diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 5edd251a949..24a4862b146 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -46,6 +46,7 @@ define_dategories! { "lint/nursery/noAssignInExpressions": "https://docs.rome.tools/lint/rules/noAssignInExpressions", "lint/nursery/noWith": "https://docs.rome.tools/lint/rules/noWith", "lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes", + "lint/nursery/noClassAssign": "https://docs.rome.tools/lint/rules/noClassAssign", "lint/nursery/noCommaOperator": "https://docs.rome.tools/lint/rules/noCommaOperator", "lint/nursery/noConstEnum": "https://docs.rome.tools/lint/rules/noConstEnum", "lint/nursery/noConstructorReturn": "https://docs.rome.tools/lint/rules/noConstructorReturn", diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs index 47b25368635..1d5ecb6f2e5 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery.rs @@ -4,6 +4,7 @@ use rome_analyze::declare_group; mod no_access_key; mod no_assign_in_expressions; mod no_banned_types; +mod no_class_assign; mod no_comma_operator; mod no_const_enum; mod no_constructor_return; @@ -29,4 +30,4 @@ mod use_default_switch_clause_last; mod use_enum_initializers; mod use_exponentiation_operator; mod use_numeric_literals; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_assign_in_expressions :: NoAssignInExpressions , self :: no_banned_types :: NoBannedTypes , self :: no_comma_operator :: NoCommaOperator , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_duplicate_object_keys :: NoDuplicateObjectKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_alt :: NoRedundantAlt , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_self_compare :: NoSelfCompare , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_useless_switch_case :: NoUselessSwitchCase , self :: no_void_type_return :: NoVoidTypeReturn , self :: no_with :: NoWith , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_numeric_literals :: UseNumericLiterals ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_assign_in_expressions :: NoAssignInExpressions , self :: no_banned_types :: NoBannedTypes , self :: no_class_assign :: NoClassAssign , self :: no_comma_operator :: NoCommaOperator , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_duplicate_object_keys :: NoDuplicateObjectKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_alt :: NoRedundantAlt , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_self_compare :: NoSelfCompare , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_useless_switch_case :: NoUselessSwitchCase , self :: no_void_type_return :: NoVoidTypeReturn , self :: no_with :: NoWith , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_numeric_literals :: UseNumericLiterals ,] } } diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs new file mode 100644 index 00000000000..603a7138895 --- /dev/null +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs @@ -0,0 +1,115 @@ +use rome_analyze::context::RuleContext; +use rome_analyze::{declare_rule, Rule, RuleDiagnostic}; +use rome_js_semantic::{Reference, ReferencesExtensions}; +use rome_js_syntax::{JsClassDeclaration, JsClassExpression, JsIdentifierBinding}; +use rome_rowan::{declare_node_union, AstNode}; + +use crate::semantic_services::Semantic; + +declare_rule! { + /// Disallow reassigning class members. + /// + /// A class declaration creates a variable that we can modify, however, the modification is a mistake in most cases. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// class A {} + /// A = 0; + /// ``` + /// + /// ```js,expect_diagnostic + /// A = 0; + /// class A {} + /// ``` + /// + /// ```js,expect_diagnostic + /// class A { + /// b() { + /// A = 0; + /// } + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// let A = class A { + /// b() { + /// A = 0; + /// // `let A` is shadowed by the class name. + /// } + /// } + /// ``` + /// + /// ### Valid + /// + /// ```js + /// let A = class A {} + /// A = 0; // A is a variable. + /// ``` + /// + /// ```js + /// let A = class { + /// b() { + /// A = 0; // A is a variable. + /// } + /// } + /// ``` + /// + /// ```js + /// class A { + /// b(A) { + /// A = 0; // A is a parameter. + /// } + /// } + /// ``` + /// + pub(crate) NoClassAssign { + version: "12.0.0", + name: "noClassAssign", + recommended: true, + } +} + +declare_node_union! { + pub(crate) AnyClass = JsClassDeclaration | JsClassExpression +} + +impl Rule for NoClassAssign { + type Query = Semantic; + type State = Vec; + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let binding = ctx.query(); + let model = ctx.model(); + + // ensures that we only verifies class bindings + binding.parent::()?; + + let all_writes: Vec = binding.all_writes(model).collect(); + + if all_writes.is_empty() { + None + } else { + Some(all_writes) + } + } + + fn diagnostic(ctx: &RuleContext, references: &Self::State) -> Option { + let mut diagnostic = RuleDiagnostic::new( + rule_category!(), + ctx.query().range(), + "Don't reassign classes", + ); + + for reference in references.iter() { + diagnostic = + diagnostic.detail(reference.syntax().text_trimmed_range(), "Reassigned here."); + } + + Some(diagnostic) + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js new file mode 100644 index 00000000000..dcf1f39daee --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js @@ -0,0 +1,111 @@ +/* Valid */ +function case1() { + class A { } + foo(A); +} + +function case2() { + let A = class A { }; + foo(A); +} + +function case3() { + class A { + b(A) { + A = 0; + } + } +} + +function case4() { + class A { + b() { + let A; + A = 0; + } + } +} + +function case5() { + let A = class { + b() { + A = 0; + } + } +} + +// /* Ignores non class. */ +function case6() { + var x = 0; + x = 1; +} + +function case7() { + let x = 0; + x = 1; +} + +function case8() { + const x = 0; + x = 1; +} + +function case9() { + function x() {} + x = 1; +} + +function case10(x) { + x = 1; +} + +function case11() { + try {} + catch (x) { + x = 1; + } +} + +// /* Invalid */ + +function case12() { + class A { } + A = 0; +} + +function case13() { + class B { } + ({B} = 0); +} + +function case14() { + class C { } + ({b: C = 0} = {}); +} + +function case15() { + D = 0; + class D { } +} + +function case16() { + class E { + b() { + E = 0; + } + } +} + +function case17() { + let F = class F { + b() { + F = 0; + } + } +} + +function case18() { + class G { } + G = 0; + G = 1; +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap new file mode 100644 index 00000000000..8fb52390a25 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap @@ -0,0 +1,293 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 92 +expression: noClassAssign.js +--- +# Input +```js +/* Valid */ +function case1() { + class A { } + foo(A); +} + +function case2() { + let A = class A { }; + foo(A); +} + +function case3() { + class A { + b(A) { + A = 0; + } + } +} + +function case4() { + class A { + b() { + let A; + A = 0; + } + } +} + +function case5() { + let A = class { + b() { + A = 0; + } + } +} + +// /* Ignores non class. */ +function case6() { + var x = 0; + x = 1; +} + +function case7() { + let x = 0; + x = 1; +} + +function case8() { + const x = 0; + x = 1; +} + +function case9() { + function x() {} + x = 1; +} + +function case10(x) { + x = 1; +} + +function case11() { + try {} + catch (x) { + x = 1; + } +} + +// /* Invalid */ + +function case12() { + class A { } + A = 0; +} + +function case13() { + class B { } + ({B} = 0); +} + +function case14() { + class C { } + ({b: C = 0} = {}); +} + +function case15() { + D = 0; + class D { } +} + +function case16() { + class E { + b() { + E = 0; + } + } +} + +function case17() { + let F = class F { + b() { + F = 0; + } + } +} + +function case18() { + class G { } + G = 0; + G = 1; +} + +``` + +# Diagnostics +``` +noClassAssign.js:72:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 71 │ function case12() { + > 72 │ class A { } + │ ^ + 73 │ A = 0; + 74 │ } + + i Reassigned here. + + 71 │ function case12() { + 72 │ class A { } + > 73 │ A = 0; + │ ^ + 74 │ } + 75 │ + + +``` + +``` +noClassAssign.js:77:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 76 │ function case13() { + > 77 │ class B { } + │ ^ + 78 │ ({B} = 0); + 79 │ } + + i Reassigned here. + + 76 │ function case13() { + 77 │ class B { } + > 78 │ ({B} = 0); + │ ^ + 79 │ } + 80 │ + + +``` + +``` +noClassAssign.js:82:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 81 │ function case14() { + > 82 │ class C { } + │ ^ + 83 │ ({b: C = 0} = {}); + 84 │ } + + i Reassigned here. + + 81 │ function case14() { + 82 │ class C { } + > 83 │ ({b: C = 0} = {}); + │ ^ + 84 │ } + 85 │ + + +``` + +``` +noClassAssign.js:88:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 86 │ function case15() { + 87 │ D = 0; + > 88 │ class D { } + │ ^ + 89 │ } + 90 │ + + i Reassigned here. + + 86 │ function case15() { + > 87 │ D = 0; + │ ^ + 88 │ class D { } + 89 │ } + + +``` + +``` +noClassAssign.js:92:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 91 │ function case16() { + > 92 │ class E { + │ ^ + 93 │ b() { + 94 │ E = 0; + + i Reassigned here. + + 92 │ class E { + 93 │ b() { + > 94 │ E = 0; + │ ^ + 95 │ } + 96 │ } + + +``` + +``` +noClassAssign.js:100:16 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 99 │ function case17() { + > 100 │ let F = class F { + │ ^ + 101 │ b() { + 102 │ F = 0; + + i Reassigned here. + + 100 │ let F = class F { + 101 │ b() { + > 102 │ F = 0; + │ ^ + 103 │ } + 104 │ } + + +``` + +``` +noClassAssign.js:108:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Don't reassign classes. + + 107 │ function case18() { + > 108 │ class G { } + │ ^ + 109 │ G = 0; + 110 │ G = 1; + + i Reassigned here. + + 107 │ function case18() { + 108 │ class G { } + > 109 │ G = 0; + │ ^ + 110 │ G = 1; + 111 │ } + + i Reassigned here. + + 108 │ class G { } + 109 │ G = 0; + > 110 │ G = 1; + │ ^ + 111 │ } + 112 │ + + +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 852f812f328..7ce52e1a462 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -729,6 +729,8 @@ struct NurserySchema { no_assign_in_expressions: Option, #[doc = "Disallow certain types."] no_banned_types: Option, + #[doc = "Disallow reassigning class members."] + no_class_assign: Option, #[doc = "Disallow comma operator."] no_comma_operator: Option, #[doc = "Disallow TypeScript const enum"] @@ -798,10 +800,11 @@ struct NurserySchema { } impl Nursery { const CATEGORY_NAME: &'static str = "nursery"; - pub(crate) const CATEGORY_RULES: [&'static str; 36] = [ + pub(crate) const CATEGORY_RULES: [&'static str; 37] = [ "noAccessKey", "noAssignInExpressions", "noBannedTypes", + "noClassAssign", "noCommaOperator", "noConstEnum", "noConstructorReturn", @@ -836,9 +839,10 @@ impl Nursery { "useHookAtTopLevel", "useNumericLiterals", ]; - const RECOMMENDED_RULES: [&'static str; 27] = [ + const RECOMMENDED_RULES: [&'static str; 28] = [ "noAssignInExpressions", "noBannedTypes", + "noClassAssign", "noCommaOperator", "noConstEnum", "noConstructorReturn", @@ -865,7 +869,7 @@ impl Nursery { "useExhaustiveDependencies", "useNumericLiterals", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 27] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 28] = [ RuleFilter::Rule("nursery", Self::CATEGORY_RULES[1]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[2]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[3]), @@ -877,8 +881,8 @@ impl Nursery { RuleFilter::Rule("nursery", Self::CATEGORY_RULES[9]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[10]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[11]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[14]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[17]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[12]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[15]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[18]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[19]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[20]), @@ -886,13 +890,14 @@ impl Nursery { RuleFilter::Rule("nursery", Self::CATEGORY_RULES[22]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[23]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[24]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[26]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[28]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[25]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[27]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[29]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[30]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[31]), RuleFilter::Rule("nursery", Self::CATEGORY_RULES[32]), - RuleFilter::Rule("nursery", Self::CATEGORY_RULES[35]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[33]), + RuleFilter::Rule("nursery", Self::CATEGORY_RULES[36]), ]; pub(crate) fn is_recommended(&self) -> bool { !matches!(self.recommended, Some(false)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { @@ -919,7 +924,7 @@ 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>; 27] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 28] { Self::RECOMMENDED_RULES_AS_FILTERS } } diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 5afcf83f6e5..56c699ae22b 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -610,6 +610,17 @@ } ] }, + "noClassAssign": { + "description": "Disallow reassigning class members.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "noCommaOperator": { "description": "Disallow comma operator.", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index f7bf2a1acbb..5463b2a33e0 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -297,6 +297,10 @@ export interface Nursery { * Disallow certain types. */ noBannedTypes?: RuleConfiguration; + /** + * Disallow reassigning class members. + */ + noClassAssign?: RuleConfiguration; /** * Disallow comma operator. */ @@ -706,6 +710,7 @@ export type Category = | "lint/nursery/noAssignInExpressions" | "lint/nursery/noWith" | "lint/nursery/noBannedTypes" + | "lint/nursery/noClassAssign" | "lint/nursery/noCommaOperator" | "lint/nursery/noConstEnum" | "lint/nursery/noConstructorReturn" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 5afcf83f6e5..56c699ae22b 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -610,6 +610,17 @@ } ] }, + "noClassAssign": { + "description": "Disallow reassigning class members.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "noCommaOperator": { "description": "Disallow comma operator.", "anyOf": [ diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index b91d9919f18..0ebdb9c92d3 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -492,6 +492,12 @@ Disallow assignments in expressions. Disallow certain types.
+

+ noClassAssign +

+Disallow reassigning class members. +
+

noCommaOperator

diff --git a/website/src/pages/lint/rules/noClassAssign.md b/website/src/pages/lint/rules/noClassAssign.md new file mode 100644 index 00000000000..c648079be5b --- /dev/null +++ b/website/src/pages/lint/rules/noClassAssign.md @@ -0,0 +1,141 @@ +--- +title: Lint Rule noClassAssign +parent: lint/rules/index +--- + +# noClassAssign (since v12.0.0) + +Disallow reassigning class members. + +A class declaration creates a variable that we can modify, however, the modification is a mistake in most cases. + +## Examples + +### Invalid + +```jsx +class A {} +A = 0; +``` + +
nursery/noClassAssign.js:1:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Don't reassign classes
+  
+  > 1 │ class A {}
+      │       ^
+    2 │ A = 0;
+    3 │ 
+  
+  ℹ Reassigned here.
+  
+    1 │ class A {}
+  > 2 │ A = 0;
+      │ ^
+    3 │ 
+  
+
+ +```jsx +A = 0; +class A {} +``` + +
nursery/noClassAssign.js:2:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Don't reassign classes
+  
+    1 │ A = 0;
+  > 2 │ class A {}
+      │       ^
+    3 │ 
+  
+  ℹ Reassigned here.
+  
+  > 1 │ A = 0;
+      │ ^
+    2 │ class A {}
+    3 │ 
+  
+
+ +```jsx +class A { + b() { + A = 0; + } +} +``` + +
nursery/noClassAssign.js:1:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Don't reassign classes
+  
+  > 1 │ class A {
+      │       ^
+    2 │ 	b() {
+    3 │ 		A = 0;
+  
+  ℹ Reassigned here.
+  
+    1 │ class A {
+    2 │ 	b() {
+  > 3 │ 		A = 0;
+      │ 		^
+    4 │ 	}
+    5 │ }
+  
+
+ +```jsx +let A = class A { + b() { + A = 0; + // `let A` is shadowed by the class name. + } +} +``` + +
nursery/noClassAssign.js:1:15 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ Don't reassign classes
+  
+  > 1 │ let A = class A {
+      │               ^
+    2 │ 	b() {
+    3 │ 		A = 0;
+  
+  ℹ Reassigned here.
+  
+    1 │ let A = class A {
+    2 │ 	b() {
+  > 3 │ 		A = 0;
+      │ 		^
+    4 │ 		// `let A` is shadowed by the class name.
+    5 │ 	}
+  
+
+ +### Valid + +```jsx +let A = class A {} +A = 0; // A is a variable. +``` + +```jsx +let A = class { + b() { + A = 0; // A is a variable. + } +} +``` + +```jsx +class A { + b(A) { + A = 0; // A is a parameter. + } +} +``` + From 4c164d4c3ef08e7568c1a64a2dd8e2594519a09a Mon Sep 17 00:00:00 2001 From: kaioduarte Date: Sat, 10 Dec 2022 23:19:46 +0000 Subject: [PATCH 2/4] chore: move rule to semantic_analyzers folder --- crates/rome_js_analyze/src/analyzers/nursery.rs | 3 +-- crates/rome_js_analyze/src/semantic_analyzers/nursery.rs | 3 ++- .../nursery/no_class_assign.rs | 2 +- website/src/pages/lint/rules/noClassAssign.md | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) rename crates/rome_js_analyze/src/{analyzers => semantic_analyzers}/nursery/no_class_assign.rs (98%) diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs index 1d5ecb6f2e5..47b25368635 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery.rs @@ -4,7 +4,6 @@ use rome_analyze::declare_group; mod no_access_key; mod no_assign_in_expressions; mod no_banned_types; -mod no_class_assign; mod no_comma_operator; mod no_const_enum; mod no_constructor_return; @@ -30,4 +29,4 @@ mod use_default_switch_clause_last; mod use_enum_initializers; mod use_exponentiation_operator; mod use_numeric_literals; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_assign_in_expressions :: NoAssignInExpressions , self :: no_banned_types :: NoBannedTypes , self :: no_class_assign :: NoClassAssign , self :: no_comma_operator :: NoCommaOperator , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_duplicate_object_keys :: NoDuplicateObjectKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_alt :: NoRedundantAlt , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_self_compare :: NoSelfCompare , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_useless_switch_case :: NoUselessSwitchCase , self :: no_void_type_return :: NoVoidTypeReturn , self :: no_with :: NoWith , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_numeric_literals :: UseNumericLiterals ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_assign_in_expressions :: NoAssignInExpressions , self :: no_banned_types :: NoBannedTypes , self :: no_comma_operator :: NoCommaOperator , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_duplicate_object_keys :: NoDuplicateObjectKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_alt :: NoRedundantAlt , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_self_compare :: NoSelfCompare , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_useless_switch_case :: NoUselessSwitchCase , self :: no_void_type_return :: NoVoidTypeReturn , self :: no_with :: NoWith , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_numeric_literals :: UseNumericLiterals ,] } } diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs index 0af250e65c8..853f29d4933 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs @@ -1,10 +1,11 @@ //! Generated file, do not edit by hand, see `xtask/codegen` use rome_analyze::declare_group; +mod no_class_assign; mod no_restricted_globals; mod no_var; mod use_camel_case; mod use_const; mod use_exhaustive_dependencies; mod use_hook_at_top_level; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_restricted_globals :: NoRestrictedGlobals , self :: no_var :: NoVar , self :: use_camel_case :: UseCamelCase , self :: use_const :: UseConst , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_class_assign :: NoClassAssign , self :: no_restricted_globals :: NoRestrictedGlobals , self :: no_var :: NoVar , self :: use_camel_case :: UseCamelCase , self :: use_const :: UseConst , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel ,] } } diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs similarity index 98% rename from crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs rename to crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs index 603a7138895..5f24daf052d 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/no_class_assign.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs @@ -102,7 +102,7 @@ impl Rule for NoClassAssign { let mut diagnostic = RuleDiagnostic::new( rule_category!(), ctx.query().range(), - "Don't reassign classes", + "Don't reassign classes.", ); for reference in references.iter() { diff --git a/website/src/pages/lint/rules/noClassAssign.md b/website/src/pages/lint/rules/noClassAssign.md index c648079be5b..8bc6d1de9fa 100644 --- a/website/src/pages/lint/rules/noClassAssign.md +++ b/website/src/pages/lint/rules/noClassAssign.md @@ -20,7 +20,7 @@ A = 0; 
nursery/noClassAssign.js:1:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes
+  ✖ Don't reassign classes.
   
   > 1 │ class A {}
       │       ^
@@ -43,7 +43,7 @@ class A {}
 
 
nursery/noClassAssign.js:2:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes
+  ✖ Don't reassign classes.
   
     1 │ A = 0;
   > 2 │ class A {}
@@ -69,7 +69,7 @@ class A {
 
 
nursery/noClassAssign.js:1:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes
+  ✖ Don't reassign classes.
   
   > 1 │ class A {
       │       ^
@@ -98,7 +98,7 @@ let A = class A {
 
 
nursery/noClassAssign.js:1:15 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes
+  ✖ Don't reassign classes.
   
   > 1 │ let A = class A {
       │               ^

From 76110b6620c5ced962153edfc0999921f2f136c0 Mon Sep 17 00:00:00 2001
From: kaioduarte 
Date: Mon, 12 Dec 2022 23:07:23 +0000
Subject: [PATCH 3/4] chore: address PR feedback

---
 .../nursery/no_class_assign.rs                |  44 ++----
 .../tests/specs/nursery/noClassAssign.js      |   1 -
 .../tests/specs/nursery/noClassAssign.js.snap | 145 +++++-------------
 website/src/pages/lint/rules/noClassAssign.md |  32 ----
 4 files changed, 52 insertions(+), 170 deletions(-)

diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs
index 5f24daf052d..abb5cd117fb 100644
--- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs
+++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs
@@ -1,8 +1,8 @@
 use rome_analyze::context::RuleContext;
 use rome_analyze::{declare_rule, Rule, RuleDiagnostic};
-use rome_js_semantic::{Reference, ReferencesExtensions};
-use rome_js_syntax::{JsClassDeclaration, JsClassExpression, JsIdentifierBinding};
-use rome_rowan::{declare_node_union, AstNode};
+use rome_js_semantic::ReferencesExtensions;
+use rome_js_syntax::{AnyJsClass, JsIdentifierBinding};
+use rome_rowan::AstNode;
 
 use crate::semantic_services::Semantic;
 
@@ -72,44 +72,30 @@ declare_rule! {
     }
 }
 
-declare_node_union! {
-    pub(crate) AnyClass = JsClassDeclaration | JsClassExpression
-}
-
 impl Rule for NoClassAssign {
-    type Query = Semantic;
-    type State = Vec;
+    type Query = Semantic;
+    type State = JsIdentifierBinding;
     type Signals = Option;
     type Options = ();
 
     fn run(ctx: &RuleContext) -> Self::Signals {
-        let binding = ctx.query();
+        let node = ctx.query();
         let model = ctx.model();
+        let binding = node.id().ok()??;
+        let binding = binding.as_js_identifier_binding()?;
 
-        // ensures that we only verifies class bindings
-        binding.parent::()?;
-
-        let all_writes: Vec = binding.all_writes(model).collect();
-
-        if all_writes.is_empty() {
-            None
+        if binding.all_writes(model).count() > 0 {
+            Some(binding.clone())
         } else {
-            Some(all_writes)
+            None
         }
     }
 
-    fn diagnostic(ctx: &RuleContext, references: &Self::State) -> Option {
-        let mut diagnostic = RuleDiagnostic::new(
+    fn diagnostic(_: &RuleContext, class_id: &Self::State) -> Option {
+        Some(RuleDiagnostic::new(
             rule_category!(),
-            ctx.query().range(),
+            class_id.range(),
             "Don't reassign classes.",
-        );
-
-        for reference in references.iter() {
-            diagnostic =
-                diagnostic.detail(reference.syntax().text_trimmed_range(), "Reassigned here.");
-        }
-
-        Some(diagnostic)
+        ))
     }
 }
diff --git a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js
index dcf1f39daee..410ef0e3135 100644
--- a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js
+++ b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js
@@ -67,7 +67,6 @@ function case11() {
 }
 
 // /* Invalid  */
-
 function case12() {
 	class A { }
 	A = 0;
diff --git a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap
index 8fb52390a25..039617df040 100644
--- a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap
+++ b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap
@@ -74,7 +74,6 @@ function case11() {
 }
 
 // /* Invalid  */
-
 function case12() {
 	class A { }
 	A = 0;
@@ -121,171 +120,101 @@ function case18() {
 
 # Diagnostics
 ```
-noClassAssign.js:72:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:71:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-    71 │ function case12() {
-  > 72 │ 	class A { }
+    69 │ // /* Invalid  */
+    70 │ function case12() {
+  > 71 │ 	class A { }
        │ 	      ^
-    73 │ 	A = 0;
-    74 │ }
-  
-  i Reassigned here.
-  
-    71 │ function case12() {
-    72 │ 	class A { }
-  > 73 │ 	A = 0;
-       │ 	^
-    74 │ }
-    75 │ 
+    72 │ 	A = 0;
+    73 │ }
   
 
 ```
 
 ```
-noClassAssign.js:77:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:76:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-    76 │ function case13() {
-  > 77 │ 	class B { }
+    75 │ function case13() {
+  > 76 │ 	class B { }
        │ 	      ^
-    78 │ 	({B} = 0);
-    79 │ }
-  
-  i Reassigned here.
-  
-    76 │ function case13() {
-    77 │ 	class B { }
-  > 78 │ 	({B} = 0);
-       │ 	  ^
-    79 │ }
-    80 │ 
+    77 │ 	({B} = 0);
+    78 │ }
   
 
 ```
 
 ```
-noClassAssign.js:82:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:81:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-    81 │ function case14() {
-  > 82 │ 	class C { }
+    80 │ function case14() {
+  > 81 │ 	class C { }
        │ 	      ^
-    83 │ 	({b: C = 0} = {});
-    84 │ }
-  
-  i Reassigned here.
-  
-    81 │ function case14() {
-    82 │ 	class C { }
-  > 83 │ 	({b: C = 0} = {});
-       │ 	     ^
-    84 │ }
-    85 │ 
+    82 │ 	({b: C = 0} = {});
+    83 │ }
   
 
 ```
 
 ```
-noClassAssign.js:88:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:87:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-    86 │ function case15() {
-    87 │ 	D = 0;
-  > 88 │ 	class D { }
+    85 │ function case15() {
+    86 │ 	D = 0;
+  > 87 │ 	class D { }
        │ 	      ^
-    89 │ }
-    90 │ 
-  
-  i Reassigned here.
-  
-    86 │ function case15() {
-  > 87 │ 	D = 0;
-       │ 	^
-    88 │ 	class D { }
-    89 │ }
+    88 │ }
+    89 │ 
   
 
 ```
 
 ```
-noClassAssign.js:92:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:91:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-    91 │ function case16() {
-  > 92 │ 	class E {
+    90 │ function case16() {
+  > 91 │ 	class E {
        │ 	      ^
-    93 │ 		b() {
-    94 │ 			E = 0;
-  
-  i Reassigned here.
-  
-    92 │ 	class E {
-    93 │ 		b() {
-  > 94 │ 			E = 0;
-       │ 			^
-    95 │ 		}
-    96 │ 	}
+    92 │ 		b() {
+    93 │ 			E = 0;
   
 
 ```
 
 ```
-noClassAssign.js:100:16 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:99:16 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-     99 │ function case17() {
-  > 100 │ 	let F = class F {
+     98 │ function case17() {
+   > 99 │ 	let F = class F {
         │ 	              ^
-    101 │ 		b() {
-    102 │ 			F = 0;
-  
-  i Reassigned here.
-  
-    100 │ 	let F = class F {
-    101 │ 		b() {
-  > 102 │ 			F = 0;
-        │ 			^
-    103 │ 		}
-    104 │ 	}
+    100 │ 		b() {
+    101 │ 			F = 0;
   
 
 ```
 
 ```
-noClassAssign.js:108:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+noClassAssign.js:107:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
   ! Don't reassign classes.
   
-    107 │ function case18() {
-  > 108 │ 	class G { }
+    106 │ function case18() {
+  > 107 │ 	class G { }
         │ 	      ^
-    109 │ 	G = 0;
-    110 │ 	G = 1;
-  
-  i Reassigned here.
-  
-    107 │ function case18() {
-    108 │ 	class G { }
-  > 109 │ 	G = 0;
-        │ 	^
-    110 │ 	G = 1;
-    111 │ }
-  
-  i Reassigned here.
-  
-    108 │ 	class G { }
-    109 │ 	G = 0;
-  > 110 │ 	G = 1;
-        │ 	^
-    111 │ }
-    112 │ 
+    108 │ 	G = 0;
+    109 │ 	G = 1;
   
 
 ```
diff --git a/website/src/pages/lint/rules/noClassAssign.md b/website/src/pages/lint/rules/noClassAssign.md
index 8bc6d1de9fa..892298a0336 100644
--- a/website/src/pages/lint/rules/noClassAssign.md
+++ b/website/src/pages/lint/rules/noClassAssign.md
@@ -27,13 +27,6 @@ A = 0;
     2 │ A = 0;
     3 │ 
   
-  ℹ Reassigned here.
-  
-    1 │ class A {}
-  > 2 │ A = 0;
-      │ ^
-    3 │ 
-  
 

 
 ```jsx
@@ -50,13 +43,6 @@ class A {}
       │       ^
     3 │ 
   
-  ℹ Reassigned here.
-  
-  > 1 │ A = 0;
-      │ ^
-    2 │ class A {}
-    3 │ 
-  
 

 
 ```jsx
@@ -76,15 +62,6 @@ class A {
     2 │ 	b() {
     3 │ 		A = 0;
   
-  ℹ Reassigned here.
-  
-    1 │ class A {
-    2 │ 	b() {
-  > 3 │ 		A = 0;
-      │ 		^
-    4 │ 	}
-    5 │ }
-  
 

 
 ```jsx
@@ -105,15 +82,6 @@ let A = class A {
     2 │ 	b() {
     3 │ 		A = 0;
   
-  ℹ Reassigned here.
-  
-    1 │ let A = class A {
-    2 │ 	b() {
-  > 3 │ 		A = 0;
-      │ 		^
-    4 │ 		// `let A` is shadowed by the class name.
-    5 │ 	}
-
### Valid From 0deb4d7e064d69995ea8a84aeae7ad521e7ae5dd Mon Sep 17 00:00:00 2001 From: kaioduarte Date: Tue, 13 Dec 2022 19:54:54 +0000 Subject: [PATCH 4/4] chore: respond PR feedback --- .../nursery/no_class_assign.rs | 49 +++++--- .../tests/specs/nursery/noClassAssign.js.snap | 113 +++++++++++++++--- website/src/pages/lint/rules/noClassAssign.md | 48 ++++++-- 3 files changed, 171 insertions(+), 39 deletions(-) diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs index abb5cd117fb..1719b1951bd 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_class_assign.rs @@ -1,8 +1,8 @@ use rome_analyze::context::RuleContext; use rome_analyze::{declare_rule, Rule, RuleDiagnostic}; -use rome_js_semantic::ReferencesExtensions; -use rome_js_syntax::{AnyJsClass, JsIdentifierBinding}; -use rome_rowan::AstNode; +use rome_console::markup; +use rome_js_semantic::{Reference, ReferencesExtensions}; +use rome_js_syntax::AnyJsClass; use crate::semantic_services::Semantic; @@ -74,28 +74,43 @@ declare_rule! { impl Rule for NoClassAssign { type Query = Semantic; - type State = JsIdentifierBinding; - type Signals = Option; + type State = Reference; + type Signals = Vec; type Options = (); fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); let model = ctx.model(); - let binding = node.id().ok()??; - let binding = binding.as_js_identifier_binding()?; - if binding.all_writes(model).count() > 0 { - Some(binding.clone()) - } else { - None + if let Ok(Some(id)) = node.id() { + if let Some(id_binding) = id.as_js_identifier_binding() { + return id_binding.all_writes(model).collect(); + } } + + Vec::new() } - fn diagnostic(_: &RuleContext, class_id: &Self::State) -> Option { - Some(RuleDiagnostic::new( - rule_category!(), - class_id.range(), - "Don't reassign classes.", - )) + fn diagnostic(ctx: &RuleContext, reference: &Self::State) -> Option { + let binding = ctx + .query() + .id() + .ok()?? + .as_js_identifier_binding()? + .name_token() + .ok()?; + let class_name = binding.text_trimmed(); + + Some( + RuleDiagnostic::new( + rule_category!(), + reference.syntax().text_trimmed_range(), + markup! {"'"{class_name}"' is a class."}, + ) + .detail( + binding.text_trimmed_range(), + markup! {"'"{class_name}"' is defined here."}, + ), + ) } } diff --git a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap index 039617df040..732003cf06b 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noClassAssign.js.snap @@ -120,9 +120,18 @@ function case18() { # Diagnostics ``` -noClassAssign.js:71:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:72:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'A' is a class. + + 70 │ function case12() { + 71 │ class A { } + > 72 │ A = 0; + │ ^ + 73 │ } + 74 │ + + i 'A' is defined here. 69 │ // /* Invalid */ 70 │ function case12() { @@ -135,9 +144,18 @@ noClassAssign.js:71:8 lint/nursery/noClassAssign ━━━━━━━━━━ ``` ``` -noClassAssign.js:76:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:77:4 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'B' is a class. + + 75 │ function case13() { + 76 │ class B { } + > 77 │ ({B} = 0); + │ ^ + 78 │ } + 79 │ + + i 'B' is defined here. 75 │ function case13() { > 76 │ class B { } @@ -149,9 +167,18 @@ noClassAssign.js:76:8 lint/nursery/noClassAssign ━━━━━━━━━━ ``` ``` -noClassAssign.js:81:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:82:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'C' is a class. + + 80 │ function case14() { + 81 │ class C { } + > 82 │ ({b: C = 0} = {}); + │ ^ + 83 │ } + 84 │ + + i 'C' is defined here. 80 │ function case14() { > 81 │ class C { } @@ -163,9 +190,17 @@ noClassAssign.js:81:8 lint/nursery/noClassAssign ━━━━━━━━━━ ``` ``` -noClassAssign.js:87:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:86:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'D' is a class. + + 85 │ function case15() { + > 86 │ D = 0; + │ ^ + 87 │ class D { } + 88 │ } + + i 'D' is defined here. 85 │ function case15() { 86 │ D = 0; @@ -178,9 +213,18 @@ noClassAssign.js:87:8 lint/nursery/noClassAssign ━━━━━━━━━━ ``` ``` -noClassAssign.js:91:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:93:4 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'E' is a class. + + 91 │ class E { + 92 │ b() { + > 93 │ E = 0; + │ ^ + 94 │ } + 95 │ } + + i 'E' is defined here. 90 │ function case16() { > 91 │ class E { @@ -192,9 +236,18 @@ noClassAssign.js:91:8 lint/nursery/noClassAssign ━━━━━━━━━━ ``` ``` -noClassAssign.js:99:16 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:101:4 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'F' is a class. + + 99 │ let F = class F { + 100 │ b() { + > 101 │ F = 0; + │ ^ + 102 │ } + 103 │ } + + i 'F' is defined here. 98 │ function case17() { > 99 │ let F = class F { @@ -206,9 +259,41 @@ noClassAssign.js:99:16 lint/nursery/noClassAssign ━━━━━━━━━━ ``` ``` -noClassAssign.js:107:8 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +noClassAssign.js:108:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! 'G' is a class. + + 106 │ function case18() { + 107 │ class G { } + > 108 │ G = 0; + │ ^ + 109 │ G = 1; + 110 │ } + + i 'G' is defined here. + + 106 │ function case18() { + > 107 │ class G { } + │ ^ + 108 │ G = 0; + 109 │ G = 1; + + +``` + +``` +noClassAssign.js:109:2 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! Don't reassign classes. + ! 'G' is a class. + + 107 │ class G { } + 108 │ G = 0; + > 109 │ G = 1; + │ ^ + 110 │ } + 111 │ + + i 'G' is defined here. 106 │ function case18() { > 107 │ class G { } diff --git a/website/src/pages/lint/rules/noClassAssign.md b/website/src/pages/lint/rules/noClassAssign.md index 892298a0336..72092990167 100644 --- a/website/src/pages/lint/rules/noClassAssign.md +++ b/website/src/pages/lint/rules/noClassAssign.md @@ -18,9 +18,16 @@ class A {} A = 0; ``` -
nursery/noClassAssign.js:1:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
nursery/noClassAssign.js:2:1 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes.
+  ✖ 'A' is a class.
+  
+    1 │ class A {}
+  > 2 │ A = 0;
+      │ ^
+    3 │ 
+  
+  ℹ 'A' is defined here.
   
   > 1 │ class A {}
       │       ^
@@ -34,9 +41,16 @@ A = 0;
 class A {}
 ```
 
-
nursery/noClassAssign.js:2:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
nursery/noClassAssign.js:1:1 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes.
+  ✖ 'A' is a class.
+  
+  > 1 │ A = 0;
+      │ ^
+    2 │ class A {}
+    3 │ 
+  
+  ℹ 'A' is defined here.
   
     1 │ A = 0;
   > 2 │ class A {}
@@ -53,9 +67,18 @@ class A {
 }
 ```
 
-
nursery/noClassAssign.js:1:7 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
nursery/noClassAssign.js:3:3 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes.
+  ✖ 'A' is a class.
+  
+    1 │ class A {
+    2 │ 	b() {
+  > 3 │ 		A = 0;
+      │ 		^
+    4 │ 	}
+    5 │ }
+  
+  ℹ 'A' is defined here.
   
   > 1 │ class A {
       │       ^
@@ -73,9 +96,18 @@ let A = class A {
 }
 ```
 
-
nursery/noClassAssign.js:1:15 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
nursery/noClassAssign.js:3:3 lint/nursery/noClassAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ Don't reassign classes.
+  ✖ 'A' is a class.
+  
+    1 │ let A = class A {
+    2 │ 	b() {
+  > 3 │ 		A = 0;
+      │ 		^
+    4 │ 		// `let A` is shadowed by the class name.
+    5 │ 	}
+  
+  ℹ 'A' is defined here.
   
   > 1 │ let A = class A {
       │               ^