This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 659
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(rome_js_analyze): noParameterAssign (#4264)
- Loading branch information
Showing
15 changed files
with
1,249 additions
and
72 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.
115 changes: 115 additions & 0 deletions
115
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_parameter_assign.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,115 @@ | ||
use crate::semantic_services::Semantic; | ||
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic}; | ||
use rome_console::markup; | ||
use rome_js_semantic::{AllBindingWriteReferencesIter, Reference, ReferencesExtensions}; | ||
use rome_js_syntax::{AnyJsBindingPattern, AnyJsFormalParameter, AnyJsParameter}; | ||
use rome_rowan::AstNode; | ||
|
||
declare_rule! { | ||
/// Disallow reassigning `function` parameters. | ||
/// | ||
/// Assignment to a `function` parameters can be misleading and confusing, | ||
/// as modifying parameters will also mutate the `arguments` object. | ||
/// It is often unintended and indicative of a programmer error. | ||
/// | ||
/// Source: https://eslint.org/docs/latest/rules/no-param-reassign | ||
/// | ||
/// In contrast to the _ESLint_ rule, this rule cannot be configured to report | ||
/// assignments to a property of a parameter. | ||
/// | ||
/// ## Examples | ||
/// | ||
/// ### Invalid | ||
/// | ||
/// ```js,expect_diagnostic | ||
/// function f(param) { | ||
/// param = 13; | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```js,expect_diagnostic | ||
/// function f(param) { | ||
/// param++; | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```js,expect_diagnostic | ||
/// function f(param) { | ||
/// for (param of arr) {} | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// class C { | ||
/// constructor(readonly prop: number) { | ||
/// prop++ | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ## Valid | ||
/// | ||
/// ```js | ||
/// function f(param) { | ||
/// let local = param; | ||
/// } | ||
/// ``` | ||
/// | ||
pub(crate) NoParameterAssign { | ||
version: "next", | ||
name: "noParameterAssign", | ||
recommended: true, | ||
} | ||
} | ||
|
||
impl Rule for NoParameterAssign { | ||
type Query = Semantic<AnyJsParameter>; | ||
type State = Reference; | ||
type Signals = AllBindingWriteReferencesIter; | ||
type Options = (); | ||
|
||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
let param = ctx.query(); | ||
let model = ctx.model(); | ||
if let Some(binding) = binding_of(param) { | ||
if let Some(binding) = binding.as_any_js_binding() { | ||
return binding.all_writes(model); | ||
} | ||
} | ||
// Empty iterator that conforms to `AllBindingWriteReferencesIter` type. | ||
std::iter::successors(None, |_| None) | ||
} | ||
|
||
fn diagnostic(ctx: &RuleContext<Self>, reference: &Self::State) -> Option<RuleDiagnostic> { | ||
let param = ctx.query(); | ||
Some( | ||
RuleDiagnostic::new( | ||
rule_category!(), | ||
reference.syntax().text_trimmed_range(), | ||
markup! { | ||
"Reassigning a "<Emphasis>"function parameter"</Emphasis>" is confusing." | ||
}, | ||
) | ||
.detail( | ||
param.syntax().text_trimmed_range(), | ||
markup! { | ||
"The "<Emphasis>"parameter"</Emphasis>" is declared here:" | ||
}, | ||
) | ||
.note(markup! { | ||
"Use a local variable instead." | ||
}), | ||
) | ||
} | ||
} | ||
|
||
fn binding_of(param: &AnyJsParameter) -> Option<AnyJsBindingPattern> { | ||
match param { | ||
AnyJsParameter::AnyJsFormalParameter(formal_param) => match &formal_param { | ||
AnyJsFormalParameter::JsBogusParameter(_) => None, | ||
AnyJsFormalParameter::JsFormalParameter(param) => param.binding().ok(), | ||
}, | ||
AnyJsParameter::JsRestParameter(param) => param.binding().ok(), | ||
AnyJsParameter::TsThisParameter(_) => None, | ||
} | ||
} |
21 changes: 21 additions & 0 deletions
21
crates/rome_js_analyze/tests/specs/nursery/noParameterAssign/invalid.jsonc
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,21 @@ | ||
[ | ||
"function foo(bar) { bar = 13; }", | ||
"function foo(bar) { bar += 13; }", | ||
"function foo(bar) { (function() { bar = 13; })(); }", | ||
"function foo(bar) { ++bar; }", | ||
"function foo(bar) { ++bar; }", | ||
"function foo(bar) { --bar; }", | ||
"function foo(bar) { bar--; }", | ||
"function foo(bar) { bar--; }", | ||
"function foo(bar) { bar--; }", | ||
"function foo(bar) { ({bar} = {}); }", | ||
"function foo(bar) { ({x: [, bar = 0]} = {}); }", | ||
"function foo(bar) { for (bar in baz); }", | ||
"function foo(bar) { for (bar of baz); }", | ||
"function foo(a) { ({a} = obj); }", | ||
"function foo(a) { ([...a] = obj); }", | ||
"function foo(a) { ({...a} = obj); }", | ||
"function foo(a) { a &&= b; }", | ||
"function foo(a) { a ||= b; }", | ||
"function foo(a) { a ??= b; }" | ||
] |
Oops, something went wrong.