Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): noParameterAssign #4264

Merged
merged 1 commit into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ define_categories! {
"lint/nursery/noPrototypeBuiltins": "https://docs.rome.tools/lint/rules/noPrototypeBuiltins",
"lint/nursery/noSvgWithoutTitle": "https://docs.rome.tools/lint/rules/noSvgWithoutTitle",
"lint/nursery/noUselessCatch": "https://docs.rome.tools/lint/rules/noUselessCatch",
"lint/nursery/noParameterAssign": "https://docs.rome.tools/lint/rules/noParameterAssign",
// Insert new nursery rule here
"lint/nursery/noRedeclaration": "https://docs.rome.tools/lint/rules/noRedeclaration",

Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/semantic_analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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,
}
}
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; }"
]
Loading