diff --git a/CHANGELOG.md b/CHANGELOG.md index edd43d761b03..916153c2eb2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Linter +#### New features + +- Add [noThisInStatic](https://biomejs.dev/linter/rules/no-this-in-static) rule. Contributed by @ditorodev and @Conaclos + #### Bug fixes - Fix [#548](https://github.com/biomejs/biome/issues/548) which made [noSelfAssign](https://biomejs.dev/linter/rules/no-self-assign) panic. diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs index 211bdec7c654..b9ef7da4a84d 100644 --- a/crates/biome_diagnostics_categories/src/categories.rs +++ b/crates/biome_diagnostics_categories/src/categories.rs @@ -100,6 +100,7 @@ define_categories! { "lint/nursery/noInvalidNewBuiltin": "https://biomejs.dev/lint/rules/no-invalid-new-builtin", "lint/nursery/noMisleadingInstantiator": "https://biomejs.dev/linter/rules/no-misleading-instantiator", "lint/nursery/noMisrefactoredShorthandAssign": "https://biomejs.dev/lint/rules/no-misrefactored-shorthand-assign", + "lint/nursery/noThisInStatic": "https://biomejs.dev/lint/rules/no-this-in-static", "lint/nursery/noUnusedImports": "https://biomejs.dev/lint/rules/no-unused-imports", "lint/nursery/noUselessElse": "https://biomejs.dev/lint/rules/no-useless-else", "lint/nursery/noUselessLoneBlockStatements": "https://biomejs.dev/lint/rules/no-useless-lone-block-statements", diff --git a/crates/biome_js_analyze/src/semantic_analyzers/nursery.rs b/crates/biome_js_analyze/src/semantic_analyzers/nursery.rs index 0167ce1893bf..23e88c3097a2 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/nursery.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/nursery.rs @@ -3,6 +3,7 @@ use biome_analyze::declare_group; pub(crate) mod no_invalid_new_builtin; +pub(crate) mod no_this_in_static; pub(crate) mod no_unused_imports; declare_group! { @@ -10,6 +11,7 @@ declare_group! { name : "nursery" , rules : [ self :: no_invalid_new_builtin :: NoInvalidNewBuiltin , + self :: no_this_in_static :: NoThisInStatic , self :: no_unused_imports :: NoUnusedImports , ] } diff --git a/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_this_in_static.rs b/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_this_in_static.rs new file mode 100644 index 000000000000..a119b2ffc7f7 --- /dev/null +++ b/crates/biome_js_analyze/src/semantic_analyzers/nursery/no_this_in_static.rs @@ -0,0 +1,193 @@ +use biome_analyze::{ + context::RuleContext, declare_rule, ActionCategory, Ast, FixKind, Rule, RuleDiagnostic, +}; +use biome_console::markup; +use biome_diagnostics::Applicability; +use biome_js_factory::make; +use biome_js_syntax::{ + AnyJsClass, AnyJsClassMember, AnyJsExpression, JsArrowFunctionExpression, JsSuperExpression, + JsSyntaxToken, JsThisExpression, +}; +use biome_rowan::{declare_node_union, AstNode, AstNodeList, BatchMutationExt, SyntaxResult}; + +use crate::{control_flow::AnyJsControlFlowRoot, JsRuleAction}; + +declare_rule! { + /// Disallow `this` and `super` in `static` contexts. + /// + /// In JavaScript, the `this` keyword in static contexts refers to the class (the constructor) instance, + /// not an instance of the class. This can be confusing for developers coming from other languages where + /// `this` typically refers to an instance of the class, not the class itself. + /// + /// Similarly, `super` in static contexts refers to the parent class, not an instance of the class. + /// This can lead to unexpected behavior if not properly understood. + /// + /// This rule enforces the use of the class name itself to access static methods, + /// which can make the code clearer and less prone to errors. It helps to prevent + /// misunderstandings and bugs that can arise from the unique behavior of `this` and `super` in static contexts. + /// + /// Source: https://github.com/mysticatea/eslint-plugin/blob/master/docs/rules/no-this-in-static.md + /// + /// ## Example + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// class A { + /// static CONSTANT = 0; + /// + /// static foo() { + /// this.CONSTANT; + /// } + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// class B extends A { + /// static bar() { + /// super.CONSTANT; + /// } + /// } + /// ``` + /// + /// ### Valid + /// + /// ```js + /// class B extends A { + /// static ANOTHER_CONSTANT = A.CONSTANT + 1; + /// + /// static foo() { + /// A.CONSTANT; + /// B.ANOTHER_CONSTANT; + /// } + /// + /// bar() { + /// this.property; + /// } + /// } + /// ``` + /// + /// ```js + /// class A { + /// static foo() { + /// doSomething() + /// } + /// + /// bar() { + /// A.foo() + /// } + /// } + /// ``` + /// + pub(crate) NoThisInStatic { + version: "next", + name: "noThisInStatic", + recommended: false, + fix_kind: FixKind::Unsafe, + } +} + +impl Rule for NoThisInStatic { + type Query = Ast; + type State = (); + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let this_super_expression = ctx.query(); + let static_method = this_super_expression + .syntax() + .ancestors() + .find(|x| { + AnyJsControlFlowRoot::can_cast(x.kind()) + && !JsArrowFunctionExpression::can_cast(x.kind()) + }) + .and_then(AnyJsClassMember::cast) + .filter(|member| match member { + AnyJsClassMember::JsGetterClassMember(member) => member + .modifiers() + .iter() + .any(|modifier| modifier.as_js_static_modifier().is_some()), + AnyJsClassMember::JsMethodClassMember(member) => member + .modifiers() + .iter() + .any(|modifier| modifier.as_js_static_modifier().is_some()), + AnyJsClassMember::JsSetterClassMember(member) => member + .modifiers() + .iter() + .any(|modifier| modifier.as_js_static_modifier().is_some()), + AnyJsClassMember::JsPropertyClassMember(member) => member + .modifiers() + .iter() + .any(|modifier| modifier.as_js_static_modifier().is_some()), + AnyJsClassMember::JsStaticInitializationBlockClassMember(_) => true, + _ => false, + }); + static_method.is_some().then_some(()) + } + + fn diagnostic(ctx: &RuleContext, _: &Self::State) -> Option { + let this_super_expression = ctx.query(); + let this_super_token = this_super_expression.token().ok()?; + let text = this_super_token.text_trimmed(); + let note = if let JsThisSuperExpression::JsSuperExpression(_) = this_super_expression { + markup! { + "super"" refers to a parent class." + } + } else { + markup! { + "this"" refers to the class." + } + }; + Some(RuleDiagnostic::new( + rule_category!(), + this_super_expression.range(), + markup! { + "Using "{text}" in a ""static"" context can be confusing." + }, + ).note(note)) + } + + fn action(ctx: &RuleContext, _: &Self::State) -> Option { + let this_super_expression = ctx.query(); + let class = this_super_expression + .syntax() + .ancestors() + .find_map(AnyJsClass::cast)?; + let suggested_class_name = if let JsThisSuperExpression::JsSuperExpression(_) = + this_super_expression + { + let extends_clause = class.extends_clause()?; + let super_class_name = extends_clause.super_class().ok()?; + let AnyJsExpression::JsIdentifierExpression(super_class_name) = super_class_name else { + return None; + }; + super_class_name + } else { + let class_name = class.id()?.as_js_identifier_binding()?.name_token().ok()?; + make::js_identifier_expression(make::js_reference_identifier(class_name)) + }; + let expr = AnyJsExpression::cast_ref(this_super_expression.syntax())?; + let mut mutation = ctx.root().begin(); + mutation.replace_node(expr, suggested_class_name.into()); + Some(JsRuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + message: markup! { "Use the class name instead." }.to_owned(), + mutation, + }) + } +} + +declare_node_union! { + pub(crate) JsThisSuperExpression = JsSuperExpression | JsThisExpression +} + +impl JsThisSuperExpression { + fn token(&self) -> SyntaxResult { + match self { + JsThisSuperExpression::JsSuperExpression(expr) => expr.super_token(), + JsThisSuperExpression::JsThisExpression(expr) => expr.this_token(), + } + } +} diff --git a/crates/biome_js_analyze/src/semantic_analyzers/suspicious/no_class_assign.rs b/crates/biome_js_analyze/src/semantic_analyzers/suspicious/no_class_assign.rs index 779c5485ae47..feeec25ca059 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/suspicious/no_class_assign.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/suspicious/no_class_assign.rs @@ -82,7 +82,7 @@ impl Rule for NoClassAssign { let node = ctx.query(); let model = ctx.model(); - if let Ok(Some(id)) = node.id() { + if let Some(id) = node.id() { if let Some(id_binding) = id.as_js_identifier_binding() { return id_binding.all_writes(model).collect(); } @@ -94,8 +94,7 @@ impl Rule for NoClassAssign { fn diagnostic(ctx: &RuleContext, reference: &Self::State) -> Option { let binding = ctx .query() - .id() - .ok()?? + .id()? .as_js_identifier_binding()? .name_token() .ok()?; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/invalid.js b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/invalid.js new file mode 100644 index 000000000000..890b683454b1 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/invalid.js @@ -0,0 +1,39 @@ +export default class B extends A { + static { this.CONSTANT += super.foo(); } + + static CONSTANT = this.OTHER_CONSTANT; + static OTHER_CONSTANT = super.ANOTHER_CONSTANT; + + static get property() { + /*before*/this/*after*/; + return /*before*/super/*after*/.x; + } + + static set property(x) { + () => this; + () => super.x = x; + } + + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + +class C extends A { + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + +const D = class D extends f() { + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + + +const E = class extends f() { + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/invalid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/invalid.js.snap new file mode 100644 index 000000000000..1f86c6c13814 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/invalid.js.snap @@ -0,0 +1,379 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```js +export default class B extends A { + static { this.CONSTANT += super.foo(); } + + static CONSTANT = this.OTHER_CONSTANT; + static OTHER_CONSTANT = super.ANOTHER_CONSTANT; + + static get property() { + /*before*/this/*after*/; + return /*before*/super/*after*/.x; + } + + static set property(x) { + () => this; + () => super.x = x; + } + + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + +class C extends A { + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + +const D = class D extends f() { + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + + +const E = class extends f() { + static method() { + return this.CONSTANT + super.ANOTHER_CONSTANT; + } +} + +``` + +# Diagnostics +``` +invalid.js:2:14 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 1 │ export default class B extends A { + > 2 │ static { this.CONSTANT += super.foo(); } + │ ^^^^ + 3 │ + 4 │ static CONSTANT = this.OTHER_CONSTANT; + + i this refers to the class. + + i Unsafe fix: Use the class name instead. + + 1 1 │ export default class B extends A { + 2 │ - ····static·{·this.CONSTANT·+=·super.foo();·} + 2 │ + ····static·{·B.CONSTANT·+=·super.foo();·} + 3 3 │ + 4 4 │ static CONSTANT = this.OTHER_CONSTANT; + + +``` + +``` +invalid.js:2:31 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 1 │ export default class B extends A { + > 2 │ static { this.CONSTANT += super.foo(); } + │ ^^^^^ + 3 │ + 4 │ static CONSTANT = this.OTHER_CONSTANT; + + i super refers to a parent class. + + i Unsafe fix: Use the class name instead. + + 1 1 │ export default class B extends A { + 2 │ - ····static·{·this.CONSTANT·+=·super.foo();·} + 2 │ + ····static·{·this.CONSTANT·+=·A.foo();·} + 3 3 │ + 4 4 │ static CONSTANT = this.OTHER_CONSTANT; + + +``` + +``` +invalid.js:8:19 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 7 │ static get property() { + > 8 │ /*before*/this/*after*/; + │ ^^^^ + 9 │ return /*before*/super/*after*/.x; + 10 │ } + + i this refers to the class. + + i Unsafe fix: Use the class name instead. + + 6 6 │ + 7 7 │ static get property() { + 8 │ - ········/*before*/this/*after*/; + 8 │ + ········/*before*/B/*after*/; + 9 9 │ return /*before*/super/*after*/.x; + 10 10 │ } + + +``` + +``` +invalid.js:9:26 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 7 │ static get property() { + 8 │ /*before*/this/*after*/; + > 9 │ return /*before*/super/*after*/.x; + │ ^^^^^ + 10 │ } + 11 │ + + i super refers to a parent class. + + i Unsafe fix: Use the class name instead. + + 7 7 │ static get property() { + 8 8 │ /*before*/this/*after*/; + 9 │ - ········return·/*before*/super/*after*/.x; + 9 │ + ········return·/*before*/A/*after*/.x; + 10 10 │ } + 11 11 │ + + +``` + +``` +invalid.js:13:15 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 12 │ static set property(x) { + > 13 │ () => this; + │ ^^^^ + 14 │ () => super.x = x; + 15 │ } + + i this refers to the class. + + i Unsafe fix: Use the class name instead. + + 11 11 │ + 12 12 │ static set property(x) { + 13 │ - ········()·=>·this; + 13 │ + ········()·=>·B; + 14 14 │ () => super.x = x; + 15 15 │ } + + +``` + +``` +invalid.js:14:15 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 12 │ static set property(x) { + 13 │ () => this; + > 14 │ () => super.x = x; + │ ^^^^^ + 15 │ } + 16 │ + + i super refers to a parent class. + + i Unsafe fix: Use the class name instead. + + 12 12 │ static set property(x) { + 13 13 │ () => this; + 14 │ - ········()·=>·super.x·=·x; + 14 │ + ········()·=>·A.x·=·x; + 15 15 │ } + 16 16 │ + + +``` + +``` +invalid.js:18:16 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 17 │ static method() { + > 18 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^ + 19 │ } + 20 │ } + + i this refers to the class. + + i Unsafe fix: Use the class name instead. + + 16 16 │ + 17 17 │ static method() { + 18 │ - ········return·this.CONSTANT·+·super.ANOTHER_CONSTANT; + 18 │ + ········return·B.CONSTANT·+·super.ANOTHER_CONSTANT; + 19 19 │ } + 20 20 │ } + + +``` + +``` +invalid.js:18:32 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 17 │ static method() { + > 18 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^^ + 19 │ } + 20 │ } + + i super refers to a parent class. + + i Unsafe fix: Use the class name instead. + + 16 16 │ + 17 17 │ static method() { + 18 │ - ········return·this.CONSTANT·+·super.ANOTHER_CONSTANT; + 18 │ + ········return·this.CONSTANT·+·A.ANOTHER_CONSTANT; + 19 19 │ } + 20 20 │ } + + +``` + +``` +invalid.js:24:16 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 22 │ class C extends A { + 23 │ static method() { + > 24 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^ + 25 │ } + 26 │ } + + i this refers to the class. + + i Unsafe fix: Use the class name instead. + + 22 22 │ class C extends A { + 23 23 │ static method() { + 24 │ - ········return·this.CONSTANT·+·super.ANOTHER_CONSTANT; + 24 │ + ········return·C.CONSTANT·+·super.ANOTHER_CONSTANT; + 25 25 │ } + 26 26 │ } + + +``` + +``` +invalid.js:24:32 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 22 │ class C extends A { + 23 │ static method() { + > 24 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^^ + 25 │ } + 26 │ } + + i super refers to a parent class. + + i Unsafe fix: Use the class name instead. + + 22 22 │ class C extends A { + 23 23 │ static method() { + 24 │ - ········return·this.CONSTANT·+·super.ANOTHER_CONSTANT; + 24 │ + ········return·this.CONSTANT·+·A.ANOTHER_CONSTANT; + 25 25 │ } + 26 26 │ } + + +``` + +``` +invalid.js:30:16 lint/nursery/noThisInStatic FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 28 │ const D = class D extends f() { + 29 │ static method() { + > 30 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^ + 31 │ } + 32 │ } + + i this refers to the class. + + i Unsafe fix: Use the class name instead. + + 28 28 │ const D = class D extends f() { + 29 29 │ static method() { + 30 │ - ········return·this.CONSTANT·+·super.ANOTHER_CONSTANT; + 30 │ + ········return·D.CONSTANT·+·super.ANOTHER_CONSTANT; + 31 31 │ } + 32 32 │ } + + +``` + +``` +invalid.js:30:32 lint/nursery/noThisInStatic ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 28 │ const D = class D extends f() { + 29 │ static method() { + > 30 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^^ + 31 │ } + 32 │ } + + i super refers to a parent class. + + +``` + +``` +invalid.js:37:16 lint/nursery/noThisInStatic ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using this in a static context can be confusing. + + 35 │ const E = class extends f() { + 36 │ static method() { + > 37 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^ + 38 │ } + 39 │ } + + i this refers to the class. + + +``` + +``` +invalid.js:37:32 lint/nursery/noThisInStatic ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Using super in a static context can be confusing. + + 35 │ const E = class extends f() { + 36 │ static method() { + > 37 │ return this.CONSTANT + super.ANOTHER_CONSTANT; + │ ^^^^^ + 38 │ } + 39 │ } + + i super refers to a parent class. + + +``` + + diff --git a/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/valid.js b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/valid.js new file mode 100644 index 000000000000..7cbee2e6f84c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/valid.js @@ -0,0 +1,5 @@ +function foo() { this } +() => { this } +class A { constructor() { this } } +class A { foo() { this } } +class A { static foo() { function foo() { this } } } \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/valid.js.snap b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/valid.js.snap new file mode 100644 index 000000000000..73f0d2fea9c5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noThisInStatic/valid.js.snap @@ -0,0 +1,14 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +function foo() { this } +() => { this } +class A { constructor() { this } } +class A { foo() { this } } +class A { static foo() { function foo() { this } } } +``` + + diff --git a/crates/biome_js_formatter/src/utils/format_class.rs b/crates/biome_js_formatter/src/utils/format_class.rs index 55bcd7ef3c98..a8fe5a151b87 100644 --- a/crates/biome_js_formatter/src/utils/format_class.rs +++ b/crates/biome_js_formatter/src/utils/format_class.rs @@ -8,7 +8,7 @@ pub struct FormatClass<'a> { impl FormatClass<'_> { fn should_group(&self, comments: &JsComments) -> FormatResult { - if let Some(id) = self.class.id()? { + if let Some(id) = self.class.id() { if comments.has_trailing_comments(id.syntax()) { return Ok(true); } @@ -44,7 +44,7 @@ impl Format for FormatClass<'_> { fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { let decorators = self.class.decorators(); let abstract_token = self.class.abstract_token(); - let id = self.class.id()?; + let id = self.class.id(); let extends = self.class.extends_clause(); let implements_clause = self.class.implements_clause(); let type_parameters = self.class.type_parameters(); diff --git a/crates/biome_js_syntax/src/union_ext.rs b/crates/biome_js_syntax/src/union_ext.rs index 326fff420050..3b644b67994c 100644 --- a/crates/biome_js_syntax/src/union_ext.rs +++ b/crates/biome_js_syntax/src/union_ext.rs @@ -31,11 +31,11 @@ impl AnyJsClass { } } - pub fn id(&self) -> SyntaxResult> { + pub fn id(&self) -> Option { match self { - AnyJsClass::JsClassDeclaration(declaration) => declaration.id().map(Some), - AnyJsClass::JsClassExpression(expression) => Ok(expression.id()), - AnyJsClass::JsClassExportDefaultDeclaration(declaration) => Ok(declaration.id()), + AnyJsClass::JsClassDeclaration(declaration) => declaration.id().ok(), + AnyJsClass::JsClassExpression(expression) => expression.id(), + AnyJsClass::JsClassExportDefaultDeclaration(declaration) => declaration.id(), } } diff --git a/crates/biome_service/src/configuration/linter/rules.rs b/crates/biome_service/src/configuration/linter/rules.rs index 301939fc2cef..2a716db9bf3e 100644 --- a/crates/biome_service/src/configuration/linter/rules.rs +++ b/crates/biome_service/src/configuration/linter/rules.rs @@ -2302,6 +2302,10 @@ pub struct Nursery { )] #[serde(skip_serializing_if = "Option::is_none")] pub no_misrefactored_shorthand_assign: Option, + #[doc = "Disallow this and super in static contexts."] + #[bpaf(long("no-this-in-static"), argument("on|off|warn"), optional, hide)] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_this_in_static: Option, #[doc = "Disallow unused imports."] #[bpaf(long("no-unused-imports"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] @@ -2366,7 +2370,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 17] = [ + pub(crate) const GROUP_RULES: [&'static str; 18] = [ "noApproximativeNumericConstant", "noDuplicateJsonKeys", "noEmptyBlockStatements", @@ -2375,6 +2379,7 @@ impl Nursery { "noInvalidNewBuiltin", "noMisleadingInstantiator", "noMisrefactoredShorthandAssign", + "noThisInStatic", "noUnusedImports", "noUselessElse", "noUselessLoneBlockStatements", @@ -2400,12 +2405,12 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[3]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 17] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 18] = [ 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]), @@ -2423,6 +2428,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14]), 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]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { @@ -2479,51 +2485,56 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.no_unused_imports.as_ref() { + if let Some(rule) = self.no_this_in_static.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_useless_else.as_ref() { + if let Some(rule) = self.no_unused_imports.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_useless_lone_block_statements.as_ref() { + if let Some(rule) = self.no_useless_else.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.use_aria_activedescendant_with_tabindex.as_ref() { + if let Some(rule) = self.no_useless_lone_block_statements.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.use_arrow_function.as_ref() { + if let Some(rule) = self.use_aria_activedescendant_with_tabindex.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.use_as_const_assertion.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[13])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_as_const_assertion.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_import_restrictions.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[15])); } } - if let Some(rule) = self.use_shorthand_assign.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[16])); } } + if let Some(rule) = self.use_shorthand_assign.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2568,51 +2579,56 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.no_unused_imports.as_ref() { + if let Some(rule) = self.no_this_in_static.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_useless_else.as_ref() { + if let Some(rule) = self.no_unused_imports.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_useless_lone_block_statements.as_ref() { + if let Some(rule) = self.no_useless_else.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.use_aria_activedescendant_with_tabindex.as_ref() { + if let Some(rule) = self.no_useless_lone_block_statements.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.use_arrow_function.as_ref() { + if let Some(rule) = self.use_aria_activedescendant_with_tabindex.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.use_as_const_assertion.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[13])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_as_const_assertion.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_import_restrictions.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[15])); } } - if let Some(rule) = self.use_shorthand_assign.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[16])); } } + if let Some(rule) = self.use_shorthand_assign.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2626,7 +2642,7 @@ impl Nursery { pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 8] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 17] { + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 18] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] @@ -2659,6 +2675,7 @@ impl Nursery { "noInvalidNewBuiltin" => self.no_invalid_new_builtin.as_ref(), "noMisleadingInstantiator" => self.no_misleading_instantiator.as_ref(), "noMisrefactoredShorthandAssign" => self.no_misrefactored_shorthand_assign.as_ref(), + "noThisInStatic" => self.no_this_in_static.as_ref(), "noUnusedImports" => self.no_unused_imports.as_ref(), "noUselessElse" => self.no_useless_else.as_ref(), "noUselessLoneBlockStatements" => self.no_useless_lone_block_statements.as_ref(), diff --git a/crates/biome_service/src/configuration/parse/json/rules.rs b/crates/biome_service/src/configuration/parse/json/rules.rs index 5bd8f11bfeaf..245243a8dc2e 100644 --- a/crates/biome_service/src/configuration/parse/json/rules.rs +++ b/crates/biome_service/src/configuration/parse/json/rules.rs @@ -792,6 +792,7 @@ impl VisitNode for Nursery { "noInvalidNewBuiltin", "noMisleadingInstantiator", "noMisrefactoredShorthandAssign", + "noThisInStatic", "noUnusedImports", "noUselessElse", "noUselessLoneBlockStatements", @@ -884,6 +885,11 @@ impl VisitNode for Nursery { )?; self.no_misrefactored_shorthand_assign = Some(configuration); } + "noThisInStatic" => { + let mut configuration = RuleConfiguration::default(); + configuration.map_rule_configuration(&value, "noThisInStatic", diagnostics)?; + self.no_this_in_static = Some(configuration); + } "noUnusedImports" => { let mut configuration = RuleConfiguration::default(); configuration.map_rule_configuration(&value, "noUnusedImports", diagnostics)?; diff --git a/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap b/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap index 218e0d76d413..579f2259cfa6 100644 --- a/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap +++ b/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap @@ -25,6 +25,7 @@ hooks_incorrect_options.json:6:5 deserialize ━━━━━━━━━━━ - noInvalidNewBuiltin - noMisleadingInstantiator - noMisrefactoredShorthandAssign + - noThisInStatic - noUnusedImports - noUselessElse - noUselessLoneBlockStatements diff --git a/crates/biome_service/tests/invalid/hooks_missing_name.json.snap b/crates/biome_service/tests/invalid/hooks_missing_name.json.snap index e6d6aa47a446..b67aeb71517a 100644 --- a/crates/biome_service/tests/invalid/hooks_missing_name.json.snap +++ b/crates/biome_service/tests/invalid/hooks_missing_name.json.snap @@ -25,6 +25,7 @@ hooks_missing_name.json:6:5 deserialize ━━━━━━━━━━━━━ - noInvalidNewBuiltin - noMisleadingInstantiator - noMisrefactoredShorthandAssign + - noThisInStatic - noUnusedImports - noUselessElse - noUselessLoneBlockStatements diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index a9f87316ae37..84b0a407e17f 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -1086,6 +1086,13 @@ { "type": "null" } ] }, + "noThisInStatic": { + "description": "Disallow this and super in static contexts.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noUnusedImports": { "description": "Disallow unused imports.", "anyOf": [ diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index efcbd14f2438..b1179da242d5 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -764,6 +764,10 @@ export interface Nursery { * Disallow shorthand assign when variable appears on both sides. */ noMisrefactoredShorthandAssign?: RuleConfiguration; + /** + * Disallow this and super in static contexts. + */ + noThisInStatic?: RuleConfiguration; /** * Disallow unused imports. */ @@ -1432,6 +1436,7 @@ export type Category = | "lint/nursery/noInvalidNewBuiltin" | "lint/nursery/noMisleadingInstantiator" | "lint/nursery/noMisrefactoredShorthandAssign" + | "lint/nursery/noThisInStatic" | "lint/nursery/noUnusedImports" | "lint/nursery/noUselessElse" | "lint/nursery/noUselessLoneBlockStatements" diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index a9f87316ae37..84b0a407e17f 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -1086,6 +1086,13 @@ { "type": "null" } ] }, + "noThisInStatic": { + "description": "Disallow this and super in static contexts.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noUnusedImports": { "description": "Disallow unused imports.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 6b9300bc0694..ddca54d93573 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 170 rules

\ No newline at end of file +

Biome's linter has a total of 171 rules

\ No newline at end of file diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index bfe2e686ec72..847f00e162a3 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -73,6 +73,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Linter +#### New features + +- Add [noThisInStatic](https://biomejs.dev/linter/rules/no-this-in-static) rule. Contributed by @ditorodev and @Conaclos + #### Bug fixes - Fix [#548](https://github.com/biomejs/biome/issues/548) which made [noSelfAssign](https://biomejs.dev/linter/rules/no-self-assign) panic. diff --git a/website/src/content/docs/linter/rules/index.mdx b/website/src/content/docs/linter/rules/index.mdx index ab5a1482ab17..08c2e69d2b45 100644 --- a/website/src/content/docs/linter/rules/index.mdx +++ b/website/src/content/docs/linter/rules/index.mdx @@ -223,6 +223,7 @@ Rules that belong to this group are not subject to semantic versionnew operators with global non-constructor functions. | ⚠️ | | [noMisleadingInstantiator](/linter/rules/no-misleading-instantiator) | Enforce proper usage of new and constructor. | | | [noMisrefactoredShorthandAssign](/linter/rules/no-misrefactored-shorthand-assign) | Disallow shorthand assign when variable appears on both sides. | ⚠️ | +| [noThisInStatic](/linter/rules/no-this-in-static) | Disallow this and super in static contexts. | ⚠️ | | [noUnusedImports](/linter/rules/no-unused-imports) | Disallow unused imports. | 🔧 | | [noUselessElse](/linter/rules/no-useless-else) | Disallow else block when the if block breaks early. | ⚠️ | | [noUselessLoneBlockStatements](/linter/rules/no-useless-lone-block-statements) | Disallow unnecessary nested block statements. | ⚠️ | diff --git a/website/src/content/docs/linter/rules/no-this-in-static.md b/website/src/content/docs/linter/rules/no-this-in-static.md new file mode 100644 index 000000000000..1165c8931a81 --- /dev/null +++ b/website/src/content/docs/linter/rules/no-this-in-static.md @@ -0,0 +1,127 @@ +--- +title: noThisInStatic (since vnext) +--- + +**Diagnostic Category: `lint/nursery/noThisInStatic`** + +:::caution +This rule is part of the [nursery](/linter/rules/#nursery) group. +::: + +Disallow `this` and `super` in `static` contexts. + +In JavaScript, the `this` keyword in static contexts refers to the class (the constructor) instance, +not an instance of the class. This can be confusing for developers coming from other languages where +`this` typically refers to an instance of the class, not the class itself. + +Similarly, `super` in static contexts refers to the parent class, not an instance of the class. +This can lead to unexpected behavior if not properly understood. + +This rule enforces the use of the class name itself to access static methods, +which can make the code clearer and less prone to errors. It helps to prevent +misunderstandings and bugs that can arise from the unique behavior of `this` and `super` in static contexts. + +Source: https://github.com/mysticatea/eslint-plugin/blob/master/docs/rules/no-this-in-static.md + +## Example + +### Invalid + +```jsx + class A { + static CONSTANT = 0; + + static foo() { + this.CONSTANT; + } + } +``` + +

nursery/noThisInStatic.js:5:9 lint/nursery/noThisInStatic  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+   Using this in a static context can be confusing.
+  
+    4 │     static foo() {
+  > 5 │         this.CONSTANT;
+           ^^^^
+    6 │     }
+    7 │  }
+  
+   this refers to the class.
+  
+   Unsafe fix: Use the class name instead.
+  
+    3 3  
+    4 4      static foo() {
+    5  - ········this.CONSTANT;
+      5+ ········A.CONSTANT;
+    6 6      }
+    7 7   }
+  
+
+ +```jsx + class B extends A { + static bar() { + super.CONSTANT; + } + } +``` + +
nursery/noThisInStatic.js:3:9 lint/nursery/noThisInStatic  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+   Using super in a static context can be confusing.
+  
+    1 │  class B extends A {
+    2 │     static bar() {
+  > 3 │         super.CONSTANT;
+           ^^^^^
+    4 │     }
+    5 │  }
+  
+   super refers to a parent class.
+  
+   Unsafe fix: Use the class name instead.
+  
+    1 1   class B extends A {
+    2 2      static bar() {
+    3  - ········super.CONSTANT;
+      3+ ········A.CONSTANT;
+    4 4      }
+    5 5   }
+  
+
+ +### Valid + +```jsx +class B extends A { + static ANOTHER_CONSTANT = A.CONSTANT + 1; + + static foo() { + A.CONSTANT; + B.ANOTHER_CONSTANT; + } + + bar() { + this.property; + } +} +``` + +```jsx +class A { + static foo() { + doSomething() + } + + bar() { + A.foo() + } +} +``` + +## Related links + +- [Disable a rule](/linter/#disable-a-lint-rule) +- [Rule options](/linter/#rule-options)