-
-
Notifications
You must be signed in to change notification settings - Fork 514
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
15 changed files
with
663 additions
and
31 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
178 changes: 178 additions & 0 deletions
178
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_useless_this_alias.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,178 @@ | ||
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<JsVariableDeclarator>; | ||
type State = JsIdentifierBinding; | ||
type Signals = Option<Self::State>; | ||
type Options = (); | ||
|
||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
let declarator = ctx.query(); | ||
let model = ctx.model(); | ||
let mut is_this_alias = if let Some(initializer) = declarator.initializer() { | ||
let initializer = initializer.expression().ok()?.omit_parentheses(); | ||
if !JsThisExpression::can_cast(initializer.syntax().kind()) { | ||
return None; | ||
} | ||
true | ||
} else { | ||
false | ||
}; | ||
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; | ||
} | ||
// This cehck is useful when the loop is not executed (no write). | ||
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>, _: &Self::State) -> Option<RuleDiagnostic> { | ||
let declarator = ctx.query(); | ||
Some( | ||
RuleDiagnostic::new( | ||
rule_category!(), | ||
declarator.range(), | ||
markup! { | ||
"This aliasing of "<Emphasis>"this"</Emphasis>" is unnecessary." | ||
}, | ||
) | ||
.note(markup! { | ||
"Arrow functions inherits `this` from their enclosing scope." | ||
}), | ||
) | ||
} | ||
|
||
fn action(ctx: &RuleContext<Self>, id: &Self::State) -> Option<JsRuleAction> { | ||
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 "<Emphasis>"this"</Emphasis>" instead of an alias." | ||
} | ||
.to_owned(), | ||
mutation, | ||
}) | ||
} | ||
} |
25 changes: 25 additions & 0 deletions
25
crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); | ||
} | ||
} |
166 changes: 166 additions & 0 deletions
166
crates/rome_js_analyze/tests/specs/nursery/noUselessThisAlias/invalid.js.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 Arrow functions inherits `this` from their enclosing scope. | ||
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 Arrow functions inherits `this` from their enclosing scope. | ||
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 Arrow functions inherits `this` from their enclosing scope. | ||
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 Arrow functions inherits `this` from their enclosing scope. | ||
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 Arrow functions inherits `this` from their enclosing scope. | ||
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 │ } | ||
``` | ||
|
||
|
Oops, something went wrong.