From b59f59afd3fdd5a2a09e131465ce65d78bd41b84 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Mon, 7 Aug 2023 18:24:38 +0200 Subject: [PATCH] feat(lint): noUselessThisAlias --- .../src/categories.rs | 1 + .../src/semantic_analyzers/nursery.rs | 2 + .../nursery/no_useless_this_alias.rs | 176 ++++++++++++++++++ .../nursery/noUselessThisAlias/invalid.js | 25 +++ .../noUselessThisAlias/invalid.js.snap | 166 +++++++++++++++++ .../specs/nursery/noUselessThisAlias/valid.js | 54 ++++++ .../nursery/noUselessThisAlias/valid.js.snap | 64 +++++++ .../src/configuration/linter/rules.rs | 79 +++++--- .../src/configuration/parse/json/rules.rs | 24 +++ editors/vscode/configuration_schema.json | 7 + .../@biomejs/backend-jsonrpc/src/workspace.ts | 5 + .../@biomejs/biome/configuration_schema.json | 7 + .../components/generated/NumberOfRules.astro | 2 +- website/src/pages/lint/rules/index.mdx | 6 + .../pages/lint/rules/noUselessThisAlias.md | 74 ++++++++ 15 files changed, 661 insertions(+), 31 deletions(-) create mode 100644 crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_this_alias.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js.snap create mode 100644 website/src/pages/lint/rules/noUselessThisAlias.md diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 2d040b2de1ce..404a8988d7c6 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -98,6 +98,7 @@ define_categories! { "lint/nursery/noStaticOnlyClass": "https://biomejs.dev/lint/rules/noStaticOnlyClass", "lint/nursery/noUnsafeDeclarationMerging": "https://biomejs.dev/lint/rules/noUnsafeDeclarationMerging", "lint/nursery/noUselessEmptyExport": "https://biomejs.dev/lint/rules/noUselessEmptyExport", + "lint/nursery/noUselessThisAlias": "https://biomejs.dev/lint/rules/noUselessThisAlias", "lint/nursery/noVoid": "https://biomejs.dev/lint/rules/noVoid", "lint/nursery/useAriaPropTypes": "https://biomejs.dev/lint/rules/useAriaPropTypes", "lint/nursery/useArrowFunction": "https://biomejs.dev/lint/rules/useArrowFunction", diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs index 94627848a228..9b795b863e68 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_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 no_useless_this_alias; pub(crate) mod use_exhaustive_dependencies; pub(crate) mod use_hook_at_top_level; pub(crate) mod use_is_array; @@ -23,6 +24,7 @@ declare_group! { self :: no_global_is_finite :: NoGlobalIsFinite , self :: no_global_is_nan :: NoGlobalIsNan , self :: no_unsafe_declaration_merging :: NoUnsafeDeclarationMerging , + self :: no_useless_this_alias :: NoUselessThisAlias , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel , self :: use_is_array :: UseIsArray , diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_this_alias.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_this_alias.rs new file mode 100644 index 000000000000..fff95d1d30c3 --- /dev/null +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_this_alias.rs @@ -0,0 +1,176 @@ +use crate::{control_flow::AnyJsControlFlowRoot, semantic_services::Semantic, JsRuleAction}; +use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic}; +use rome_console::markup; +use rome_diagnostics::Applicability; +use rome_js_factory::make; +use rome_js_semantic::ReferencesExtensions; +use rome_js_syntax::{ + AnyJsBinding, AnyJsBindingPattern, AnyJsExpression, JsArrowFunctionExpression, + JsAssignmentExpression, JsExpressionStatement, JsIdentifierBinding, JsIdentifierExpression, + JsThisExpression, JsVariableDeclaration, JsVariableDeclarator, T, +}; +use rome_rowan::{AstNode, AstSeparatedList, BatchMutationExt}; + +declare_rule! { + /// Disallow useless `this` aliasing. + /// + /// Arrow functions inherits `this` from their enclosing scope. + /// This makes `this` aliasing useless in this situation. + /// + /// Credits: https://typescript-eslint.io/rules/no-this-alias/ + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// class A { + /// method() { + /// const self = this; + /// return () => { + /// return self; + /// } + /// } + /// } + /// ``` + /// + /// ## Valid + /// + /// ```js + /// class A { + /// method() { + /// const self = this; + /// return function() { + /// this.g(); + /// return self; + /// } + /// } + /// } + /// ``` + /// + pub(crate) NoUselessThisAlias { + version: "next", + name: "noUselessThisAlias", + recommended: true, + } +} + +impl Rule for NoUselessThisAlias { + type Query = Semantic; + type State = JsIdentifierBinding; + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let declarator = ctx.query(); + let model = ctx.model(); + let mut is_this_alias = false; + if let Some(initializer) = declarator.initializer() { + let initializer = initializer.expression().ok()?.omit_parentheses(); + if !JsThisExpression::can_cast(initializer.syntax().kind()) { + return None; + } + is_this_alias = true; + }; + let Ok(AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding(id))) = declarator.id() else { + // Ignore destructuring + return None; + }; + let this_scope = declarator + .syntax() + .ancestors() + .find_map(AnyJsControlFlowRoot::cast)?; + for write in id.all_writes(model) { + let assign = JsAssignmentExpression::cast(write.syntax().parent()?)?; + let assign_right = assign.right().ok()?.omit_parentheses(); + if !JsThisExpression::can_cast(assign_right.syntax().kind()) { + return None; + } + is_this_alias = true; + } + if !is_this_alias { + return None; + } + for reference in id.all_references(model) { + let current_this_scope = reference + .syntax() + .ancestors() + .filter(|x| !JsArrowFunctionExpression::can_cast(x.kind())) + .find_map(AnyJsControlFlowRoot::cast)?; + if this_scope != current_this_scope { + // The aliasing is required because they have not the same `this` scope. + return None; + } + } + Some(id) + } + + fn diagnostic(ctx: &RuleContext, _: &Self::State) -> Option { + let declarator = ctx.query(); + Some( + RuleDiagnostic::new( + rule_category!(), + declarator.range(), + markup! { + "This aliasing of ""this"" is unnecessary." + }, + ) + .note(markup! { + "This note will give you more information." + }), + ) + } + + fn action(ctx: &RuleContext, id: &Self::State) -> Option { + let declarator = ctx.query(); + let model = ctx.model(); + let Some(var_decl) = declarator.syntax().ancestors().find_map(JsVariableDeclaration::cast) else { + return None; + }; + let mut mutation = ctx.root().begin(); + let this_expr = AnyJsExpression::from(make::js_this_expression(make::token(T![this]))); + for read in id.all_reads(model) { + let syntax = read.syntax(); + let syntax = syntax.parent()?; + let Some(expr) = JsIdentifierExpression::cast(syntax) else { + return None; + }; + mutation.replace_node(expr.into(), this_expr.clone()); + } + for write in id.all_writes(model) { + let syntax = write.syntax(); + let syntax = syntax.parent()?; + let Some(statement) = JsExpressionStatement::cast(syntax.parent()?) else { + return None; + }; + mutation.remove_node(statement); + } + let var_declarator_list = var_decl.declarators(); + if var_declarator_list.len() == 1 { + mutation.remove_node(var_decl); + } else { + let mut deleted_comma = None; + for (current_declarator, current_comma) in var_declarator_list + .iter() + .zip(var_declarator_list.separators()) + { + deleted_comma = current_comma.ok(); + let current_declarator = current_declarator.ok()?; + if ¤t_declarator == declarator { + break; + } + } + mutation.remove_node(declarator.clone()); + mutation.remove_token(deleted_comma?); + } + Some(JsRuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::Always, + message: markup! { + "Use ""this"" instead of an alias." + } + .to_owned(), + mutation, + }) + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js new file mode 100644 index 000000000000..a6390f22049d --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js @@ -0,0 +1,25 @@ +const self = this, v = 0, /*u*/ u = 2, self2 = this; + +function f() { + // assignment comment + const self = this; + return () => { + /*a*/self/*b*/.g(); + } +} + +function f() { + let self = this; + return () => { + self.g(); + } +} + +function f() { + var self; + self = this; + self = this; + return () => { + self.g(); + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js.snap new file mode 100644 index 000000000000..43d5d42eb994 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js.snap @@ -0,0 +1,166 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```js +const self = this, v = 0, /*u*/ u = 2, self2 = this; + +function f() { + // assignment comment + const self = this; + return () => { + /*a*/self/*b*/.g(); + } +} + +function f() { + let self = this; + return () => { + self.g(); + } +} + +function f() { + var self; + self = this; + self = this; + return () => { + self.g(); + } +} + +``` + +# Diagnostics +``` +invalid.js:1:7 lint/nursery/noUselessThisAlias FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This aliasing of this is unnecessary. + + > 1 │ const self = this, v = 0, /*u*/ u = 2, self2 = this; + │ ^^^^^^^^^^^ + 2 │ + 3 │ function f() { + + i This note will give you more information. + + i Safe fix: Use this instead of an alias. + + 1 │ const·self·=·this,·v·=·0,·/*u*/·u·=·2,·self2·=·this; + │ ------------- + +``` + +``` +invalid.js:1:40 lint/nursery/noUselessThisAlias FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This aliasing of this is unnecessary. + + > 1 │ const self = this, v = 0, /*u*/ u = 2, self2 = this; + │ ^^^^^^^^^^^^ + 2 │ + 3 │ function f() { + + i This note will give you more information. + + i Safe fix: Use this instead of an alias. + + 1 │ const·self·=·this,·v·=·0,·/*u*/·u·=·2,·self2·=·this; + │ -------------- + +``` + +``` +invalid.js:5:11 lint/nursery/noUselessThisAlias FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This aliasing of this is unnecessary. + + 3 │ function f() { + 4 │ // assignment comment + > 5 │ const self = this; + │ ^^^^^^^^^^^ + 6 │ return () => { + 7 │ /*a*/self/*b*/.g(); + + i This note will give you more information. + + i Safe fix: Use this instead of an alias. + + 1 1 │ const self = this, v = 0, /*u*/ u = 2, self2 = this; + 2 2 │ + 3 │ - function·f()·{ + 4 │ - ····//·assignment·comment + 5 │ - ····const·self·=·this; + 3 │ + function·f()·{; + 6 4 │ return () => { + 7 │ - ········/*a*/self/*b*/.g(); + 5 │ + ········/*a*/this/*b*/.g(); + 8 6 │ } + 9 7 │ } + + +``` + +``` +invalid.js:12:9 lint/nursery/noUselessThisAlias FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This aliasing of this is unnecessary. + + 11 │ function f() { + > 12 │ let self = this; + │ ^^^^^^^^^^^ + 13 │ return () => { + 14 │ self.g(); + + i This note will give you more information. + + i Safe fix: Use this instead of an alias. + + 9 9 │ } + 10 10 │ + 11 │ - function·f()·{ + 12 │ - ····let·self·=·this; + 11 │ + function·f()·{; + 13 12 │ return () => { + 14 │ - ········self.g(); + 13 │ + ········this.g(); + 15 14 │ } + 16 15 │ } + + +``` + +``` +invalid.js:19:9 lint/nursery/noUselessThisAlias FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This aliasing of this is unnecessary. + + 18 │ function f() { + > 19 │ var self; + │ ^^^^ + 20 │ self = this; + 21 │ self = this; + + i This note will give you more information. + + i Safe fix: Use this instead of an alias. + + 16 16 │ } + 17 17 │ + 18 │ - function·f()·{ + 19 │ - ····var·self; + 20 │ - ····self·=·this; + 21 │ - ····self·=·this; + 22 │ - ····return·()·=>·{ + 23 │ - ········self.g(); + 18 │ + function·f()·{; + 19 │ + ····return·()·=>·{ + 20 │ + ········this.g(); + 24 21 │ } + 25 22 │ } + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js new file mode 100644 index 000000000000..471354185838 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js @@ -0,0 +1,54 @@ +const { a, b } = this; +const [c, d] = this; +const property = this.property; +const firstItem = this[0]; +const object = { property: this }; +foo.bar = this; + +function f() { + const self = this; + return function distinctThisScope() { + self.g(); + } +} + +function f() { + const self = this; + function f() { + self.g(); + } + return () => { + self.g(); + } +} + +function f() { + let self = this; + self = {} + return () => { + self.g(); + } +} + +function f() { + let self; + return () => { + self.g(); + } +} + +class Class { + a = this; + #priv = this; + + constructor() { + this.b = this; + this.c = [this]; + } + + act(self = this) { + self.f() + } + + f() {} +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js.snap new file mode 100644 index 000000000000..d53c58544715 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/valid.js.snap @@ -0,0 +1,64 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +const { a, b } = this; +const [c, d] = this; +const property = this.property; +const firstItem = this[0]; +const object = { property: this }; +foo.bar = this; + +function f() { + const self = this; + return function distinctThisScope() { + self.g(); + } +} + +function f() { + const self = this; + function f() { + self.g(); + } + return () => { + self.g(); + } +} + +function f() { + let self = this; + self = {} + return () => { + self.g(); + } +} + +function f() { + let self; + return () => { + self.g(); + } +} + +class Class { + a = this; + #priv = this; + + constructor() { + this.b = this; + this.c = [this]; + } + + act(self = this) { + self.f() + } + + f() {} +} + +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 2ed1f8f94009..23e7c76e3abd 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -2005,6 +2005,10 @@ pub struct Nursery { )] #[serde(skip_serializing_if = "Option::is_none")] pub no_useless_empty_export: Option, + #[doc = "Disallow useless this aliasing."] + #[bpaf(long("no-useless-this-alias"), argument("on|off|warn"), optional, hide)] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_useless_this_alias: Option, #[doc = "Disallow the use of void operators, which is not a familiar operator."] #[bpaf(long("no-void"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] @@ -2072,7 +2076,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 29] = [ + pub(crate) const GROUP_RULES: [&'static str; 30] = [ "noAccumulatingSpread", "noAriaUnsupportedElements", "noBannedTypes", @@ -2091,6 +2095,7 @@ impl Nursery { "noStaticOnlyClass", "noUnsafeDeclarationMerging", "noUselessEmptyExport", + "noUselessThisAlias", "noVoid", "useAriaPropTypes", "useArrowFunction", @@ -2103,7 +2108,7 @@ impl Nursery { "useLiteralEnumMembers", "useNamingConvention", ]; - const RECOMMENDED_RULES: [&'static str; 19] = [ + const RECOMMENDED_RULES: [&'static str; 20] = [ "noAriaUnsupportedElements", "noBannedTypes", "noConstantCondition", @@ -2117,6 +2122,7 @@ impl Nursery { "noStaticOnlyClass", "noUnsafeDeclarationMerging", "noUselessEmptyExport", + "noUselessThisAlias", "useArrowFunction", "useExhaustiveDependencies", "useGetterReturn", @@ -2124,7 +2130,7 @@ impl Nursery { "useIsArray", "useLiteralEnumMembers", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 19] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 20] = [ 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[4]), @@ -2138,14 +2144,15 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 29] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 30] = [ 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]), @@ -2175,6 +2182,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } @@ -2275,61 +2283,66 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.no_void.as_ref() { + if let Some(rule) = self.no_useless_this_alias.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - 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[19])); } } - 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[20])); } } - if let Some(rule) = self.use_exhaustive_dependencies.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[21])); } } - if let Some(rule) = self.use_getter_return.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[22])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_getter_return.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_hook_at_top_level.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[24])); } } - 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[25])); } } - 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[26])); } } - if let Some(rule) = self.use_literal_enum_members.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[27])); } } - if let Some(rule) = self.use_naming_convention.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[28])); } } + 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[29])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2424,61 +2437,66 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.no_void.as_ref() { + if let Some(rule) = self.no_useless_this_alias.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - 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[19])); } } - 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[20])); } } - if let Some(rule) = self.use_exhaustive_dependencies.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[21])); } } - if let Some(rule) = self.use_getter_return.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[22])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_getter_return.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_hook_at_top_level.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[24])); } } - 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[25])); } } - 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[26])); } } - if let Some(rule) = self.use_literal_enum_members.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[27])); } } - if let Some(rule) = self.use_naming_convention.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[28])); } } + 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[29])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2487,10 +2505,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>; 19] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 20] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 29] { Self::ALL_RULES_AS_FILTERS } + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 30] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] pub(crate) fn collect_preset_rules( &self, @@ -2529,6 +2547,7 @@ impl Nursery { "noStaticOnlyClass" => self.no_static_only_class.as_ref(), "noUnsafeDeclarationMerging" => self.no_unsafe_declaration_merging.as_ref(), "noUselessEmptyExport" => self.no_useless_empty_export.as_ref(), + "noUselessThisAlias" => self.no_useless_this_alias.as_ref(), "noVoid" => self.no_void.as_ref(), "useAriaPropTypes" => self.use_aria_prop_types.as_ref(), "useArrowFunction" => self.use_arrow_function.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 b2a5a352df41..6be5e38f1eb8 100644 --- a/crates/rome_service/src/configuration/parse/json/rules.rs +++ b/crates/rome_service/src/configuration/parse/json/rules.rs @@ -1782,6 +1782,7 @@ impl VisitNode for Nursery { "noStaticOnlyClass", "noUnsafeDeclarationMerging", "noUselessEmptyExport", + "noUselessThisAlias", "noVoid", "useAriaPropTypes", "useArrowFunction", @@ -2226,6 +2227,29 @@ impl VisitNode for Nursery { )); } }, + "noUselessThisAlias" => match value { + AnyJsonValue::JsonStringValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; + self.no_useless_this_alias = Some(configuration); + } + AnyJsonValue::JsonObjectValue(_) => { + let mut rule_configuration = RuleConfiguration::default(); + rule_configuration.map_rule_configuration( + &value, + name_text, + "noUselessThisAlias", + diagnostics, + )?; + self.no_useless_this_alias = Some(rule_configuration); + } + _ => { + diagnostics.push(DeserializationDiagnostic::new_incorrect_type( + "object or string", + value.range(), + )); + } + }, "noVoid" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 5004a403c8a4..0e917376e6ce 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -957,6 +957,13 @@ { "type": "null" } ] }, + "noUselessThisAlias": { + "description": "Disallow useless this aliasing.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noVoid": { "description": "Disallow the use of void operators, which is not a familiar operator.", "anyOf": [ diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index c36b21899df0..971e71151714 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -639,6 +639,10 @@ export interface Nursery { * Disallow empty exports that don't change anything in a module file. */ noUselessEmptyExport?: RuleConfiguration; + /** + * Disallow useless this aliasing. + */ + noUselessThisAlias?: RuleConfiguration; /** * Disallow the use of void operators, which is not a familiar operator. */ @@ -1224,6 +1228,7 @@ export type Category = | "lint/nursery/noStaticOnlyClass" | "lint/nursery/noUnsafeDeclarationMerging" | "lint/nursery/noUselessEmptyExport" + | "lint/nursery/noUselessThisAlias" | "lint/nursery/noVoid" | "lint/nursery/useAriaPropTypes" | "lint/nursery/useArrowFunction" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 5004a403c8a4..0e917376e6ce 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -957,6 +957,13 @@ { "type": "null" } ] }, + "noUselessThisAlias": { + "description": "Disallow useless this aliasing.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noVoid": { "description": "Disallow the use of void operators, which is not a familiar operator.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 9e15dd027b25..93f856432bf4 100644 --- a/website/src/components/generated/NumberOfRules.astro +++ b/website/src/components/generated/NumberOfRules.astro @@ -1,2 +1,2 @@ -

Biome's linter has a total of 155 rules

\ No newline at end of file +

Biome's linter has a total of 156 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 39d49c43eb35..e24d0ad9f9f2 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -1054,6 +1054,12 @@ Disallow unsafe declaration merging between interfaces and classes. Disallow empty exports that don't change anything in a module file.

+

+ noUselessThisAlias +

+Disallow useless this aliasing. +
+

noVoid

diff --git a/website/src/pages/lint/rules/noUselessThisAlias.md b/website/src/pages/lint/rules/noUselessThisAlias.md new file mode 100644 index 000000000000..a385df90912c --- /dev/null +++ b/website/src/pages/lint/rules/noUselessThisAlias.md @@ -0,0 +1,74 @@ +--- +title: Lint Rule noUselessThisAlias +parent: lint/rules/index +--- + +# noUselessThisAlias (since vnext) + +Disallow useless `this` aliasing. + +Arrow functions inherits `this` from their enclosing scope. +This makes `this` aliasing useless in this situation. + +Credits: https://typescript-eslint.io/rules/no-this-alias/ + +## Examples + +### Invalid + +```jsx +class A { + method() { + const self = this; + return () => { + return self; + } + } +} +``` + +
nursery/noUselessThisAlias.js:3:15 lint/nursery/noUselessThisAlias  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ This aliasing of this is unnecessary.
+  
+    1 │ class A {
+    2 │     method() {
+  > 3 │         const self = this;
+      │               ^^^^^^^^^^^
+    4 │         return () => {
+    5 │             return self;
+  
+  ℹ This note will give you more information.
+  
+  ℹ Safe fix: Use this instead of an alias.
+  
+    1 1 │   class A {
+    2   │ - ····method()·{
+    3   │ - ········const·self·=·this;
+      2 │ + ····method()·{;
+    4 3 │           return () => {
+    5   │ - ············return·self;
+      4 │ + ············return·this;
+    6 5 │           }
+    7 6 │       }
+  
+
+ +## Valid + +```jsx +class A { + method() { + const self = this; + return function() { + this.g(); + return self; + } + } +} +``` + +## Related links + +- [Disable a rule](/linter/#disable-a-lint-rule) +- [Rule options](/linter/#rule-options)