feat(rome_js_analyze): rule `noDupeArgs`
MODIFY
crates/rome_js_analyze/src/semantic_analyzers/js.rs
crates/rome_js_analyze/src/semantic_analyzers/js.rs
use rome_analyze::declare_group;
mod no_arguments;
mod no_catch_assign;
+mod no_dupe_args;
mod no_function_assign;
mod no_import_assign;
mod no_label_var;
mod no_shouty_constants;
mod no_unused_variables;
mod use_camel_case;
-declare_group! { pub (crate) Js { name : "js" , rules : [self :: no_arguments :: NoArguments , self :: no_catch_assign :: NoCatchAssign , self :: no_function_assign :: NoFunctionAssign , self :: no_import_assign :: NoImportAssign , self :: no_label_var :: NoLabelVar , self :: no_shouty_constants :: NoShoutyConstants , self :: no_unused_variables :: NoUnusedVariables , self :: use_camel_case :: UseCamelCase ,] } }
+declare_group! { pub (crate) Js { name : "js" , rules : [self :: no_arguments :: NoArguments , self :: no_catch_assign :: NoCatchAssign , self :: no_dupe_args :: NoDupeArgs , self :: no_function_assign :: NoFunctionAssign , self :: no_import_assign :: NoImportAssign , self :: no_label_var :: NoLabelVar , self :: no_shouty_constants :: NoShoutyConstants , self :: no_unused_variables :: NoUnusedVariables , self :: use_camel_case :: UseCamelCase ,] } }

ADD
crates/rome_js_analyze/src/semantic_analyzers/js/no_dupe_args.rs
+use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleCategory, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{
+ JsAnyArrayBindingPatternElement, JsAnyBinding, JsAnyBindingPattern, JsAnyFormalParameter,
+ JsAnyObjectBindingPatternMember, JsAnyParameter, JsArrowFunctionExpression,
+ JsFunctionDeclaration, JsFunctionExportDefaultDeclaration, JsFunctionExpression,
+ JsIdentifierBinding, JsMethodClassMember, JsMethodObjectMember,
+};
+use rome_rowan::{declare_node_union, AstNode};
+use rustc_hash::FxHashSet;
+
+declare_rule! {
+ /// Disallow duplicate function arguments name.
+ ///
+ /// ## Examples
+ ///
+ /// ### Invalid
+ ///
+ /// ```js,expect_diagnostic
+ /// var f = function(a, b, b) {}
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// function b(a, b, b) {}
+ /// ```
+ ///
+ /// ### Valid
+ ///
+ /// ```js
+ /// function i(i, b, c) {}
+ /// var j = function (j, b, c) {};
+ /// function k({ k, b }, { c, d }) {}
+ /// function l([, l]) {}
+ /// function foo([[a, b], [c, d]]) {}
+ /// ```
+ pub(crate) NoDupeArgs {
+ version: "0.9.0",
+ name: "noDupeArgs",
+ recommended: true,
+ }
+}
+
+impl Rule for NoDupeArgs {
+ const CATEGORY: RuleCategory = RuleCategory::Lint;
+
+ type Query = Ast<JsAnyFunctionAndMethod>;
+ type State = JsIdentifierBinding;
+ type Signals = Option<Self::State>;
+
+ fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
+ let function = ctx.query();
+ let args = match function {
+ JsAnyFunctionAndMethod::JsArrowFunctionExpression(func) => {
+ func.parameters().ok()?.as_js_parameters()?.clone()
+ }
+ JsAnyFunctionAndMethod::JsFunctionDeclaration(func) => func.parameters().ok()?,
+ JsAnyFunctionAndMethod::JsFunctionExportDefaultDeclaration(func) => {
+ func.parameters().ok()?
+ }
+ JsAnyFunctionAndMethod::JsFunctionExpression(func) => func.parameters().ok()?,
+ JsAnyFunctionAndMethod::JsMethodClassMember(member) => member.parameters().ok()?,
+ JsAnyFunctionAndMethod::JsMethodObjectMember(member) => member.parameters().ok()?,
+ };
+ let mut set = FxHashSet::default();
+ // Traversing the parameters of the function in preorder and checking for duplicates,
+ args.items().into_iter().find_map(|parameter| {
+ let parameter = parameter.ok()?;
+ traverse_parameter(parameter, &mut set)
+ })
+ }
+
+ fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+ let binding_syntax_node = state;
+ Some(RuleDiagnostic::new(
+ binding_syntax_node.syntax().text_trimmed_range(),
+ markup! {
+ "Duplicate argument name"
+ },
+ ))
+ }
+}
+
+/// Traverse the parameter recursively and check if it is duplicated.
+/// Return `Some(JsIdentifierBinding)` if it is duplicated.
+fn traverse_parameter(
+ parameter: JsAnyParameter,
+ tracked_bindings: &mut FxHashSet<String>,
+) -> Option<JsIdentifierBinding> {
+ match parameter {
+ JsAnyParameter::JsAnyFormalParameter(p) => match p {
+ JsAnyFormalParameter::JsFormalParameter(parameter) => {
+ traverse_binding(parameter.binding().ok()?, tracked_bindings)
+ }
+ JsAnyFormalParameter::JsUnknownParameter(_) => None,
+ },
+ JsAnyParameter::JsRestParameter(rest_parameter) => {
+ traverse_binding(rest_parameter.binding().ok()?, tracked_bindings)
+ }
+ JsAnyParameter::TsThisParameter(_) => None,
+ }
+}
+
+/// Traverse a [JsAnyBindingPattern] in preorder and check if the name of [JsIdentifierBinding] has seem before.
+/// If true then add the [JsIdentifierBinding] to the [duplicated_arguments].
+/// If false then add the [JsIdentifierBinding] to the [tracked_bindings], mark it name as seen.
+/// If it is not a [JsIdentifierBinding] then recursively call [traverse_binding] on its children.
+fn traverse_binding(
+ binding: JsAnyBindingPattern,
+ tracked_bindings: &mut FxHashSet<String>,
+) -> Option<JsIdentifierBinding> {
+ match binding {
+ JsAnyBindingPattern::JsAnyBinding(inner_binding) => match inner_binding {
+ JsAnyBinding::JsIdentifierBinding(id_binding) => {
+ if track_binding(&id_binding, tracked_bindings) {
+ return Some(id_binding);
+ }
+ }
+ JsAnyBinding::JsUnknownBinding(_) => {}
+ },
+ JsAnyBindingPattern::JsArrayBindingPattern(inner_binding) => {
+ return inner_binding.elements().into_iter().find_map(|element| {
+ let element = element.ok()?;
+ match element {
+ JsAnyArrayBindingPatternElement::JsAnyBindingPattern(pattern) => {
+ traverse_binding(pattern, tracked_bindings)
+ }
+ JsAnyArrayBindingPatternElement::JsArrayBindingPatternRestElement(
+ binding_rest,
+ ) => {
+ let binding_pattern = binding_rest.pattern().ok()?;
+ traverse_binding(binding_pattern, tracked_bindings)
+ }
+ JsAnyArrayBindingPatternElement::JsArrayHole(_) => None,
+ JsAnyArrayBindingPatternElement::JsBindingPatternWithDefault(
+ binding_with_default,
+ ) => {
+ let pattern = binding_with_default.pattern().ok()?;
+ traverse_binding(pattern, tracked_bindings)
+ }
+ }
+ })
+ }
+
+ JsAnyBindingPattern::JsObjectBindingPattern(pattern) => {
+ return pattern.properties().into_iter().find_map(|prop| {
+ let prop = prop.ok()?;
+ match prop {
+ JsAnyObjectBindingPatternMember::JsIdentifierBinding(id_binding) => {
+ track_binding(&id_binding, tracked_bindings).then(|| id_binding)
+ }
+ JsAnyObjectBindingPatternMember::JsObjectBindingPatternProperty(pattern) => {
+ let pattern = pattern.pattern().ok()?;
+ traverse_binding(pattern, tracked_bindings)
+ }
+ JsAnyObjectBindingPatternMember::JsObjectBindingPatternRest(rest) => {
+ let pattern = rest.binding().ok()?;
+ match pattern {
+ JsAnyBinding::JsIdentifierBinding(binding) => {
+ track_binding(&binding, tracked_bindings).then(|| binding)
+ }
+ JsAnyBinding::JsUnknownBinding(_) => None,
+ }
+ }
+ JsAnyObjectBindingPatternMember::JsObjectBindingPatternShorthandProperty(
+ shorthand_binding,
+ ) => match shorthand_binding.identifier().ok()? {
+ JsAnyBinding::JsIdentifierBinding(id_binding) => {
+ track_binding(&id_binding, tracked_bindings).then(|| id_binding)
+ }
+ JsAnyBinding::JsUnknownBinding(_) => None,
+ },
+ JsAnyObjectBindingPatternMember::JsUnknownBinding(_) => None,
+ }
+ })
+ }
+ }
+ None
+}
+
+#[inline]
+/// If the name of binding has been seen in set, then we push the `JsIdentifierBinding` into `identifier_vec`.
+/// Else we mark the name of binding as seen.
+fn track_binding(
+ id_binding: &JsIdentifierBinding,
+ tracked_bindings: &mut FxHashSet<String>,
+) -> bool {
+ let binding_text = id_binding.text();
+ if tracked_bindings.contains(&binding_text) {
+ true
+ } else {
+ tracked_bindings.insert(binding_text);
+ false
+ }
+}
+
+declare_node_union! {
+ /// A union of all possible FunctionLike `JsAstNode` in the JS grammar.
+ pub(crate) JsAnyFunctionAndMethod = JsArrowFunctionExpression| JsFunctionDeclaration| JsFunctionExportDefaultDeclaration | JsFunctionExpression | JsMethodClassMember | JsMethodObjectMember
+}

MODIFY
crates/rome_js_analyze/src/utils/rename.rs
crates/rome_js_analyze/src/utils/rename.rs
@@ -198,8 +198,7 @@ impl RenameSymbolExtensions for BatchMutation<JsLanguage> {
let scope = reference.scope();
if scope
.ancestors()
- .filter_map(|scope| scope.get_binding(new_name))
- .next()
+ .find_map(|scope| scope.get_binding(new_name))
.is_some()
{
return false;

ADD
crates/rome_js_analyze/tests/specs/js/noDupeArgs.js
+// valid
+function a(a, b, c) {}
+var j = function (j, b, c) {};
+function k({ k, b }, { c, d }) {}
+function l([, l]) {}
+function foo([[a, b], [c, d]]) {}
+function test(a = function (a) {}) {}
+// invalid
+function b(a, b, b) {}
+
+function c(a, a, a) {}
+
+const d = (a, b, a) => {};
+
+function e(a, b, a, b) {}
+
+var f = function (a, b, b) {};
+
+class G {
+ ggg(a, a, a) {}
+}
+
+let objectMethods = {
+ method(a, b, c, c) {
+
+ }
+}
+
+var h = function (a, b, a) {};
+
+export default function (a, b, a, a) {}
+function f({ test: res = 3 }, res) {}
+
+export function f2(a, b, c = (a, b, b) => {}) {}

MODIFY
crates/rome_js_semantic/src/tests/scopes.rs
crates/rome_js_semantic/src/tests/scopes.rs
@@ -22,6 +22,7 @@ assert_semantics! {
ok_scope_function_with_export_default, ";export default function/*START A*/ f() {}/*END A*/",
ok_scope_function_expression, ";var a = function/*START A*/ f() {}/*END A*/",
ok_scope_arrow_function, ";(/*START A*/) => {}/*END A*/",
+ ok_scope_js_function_export_default_declaration, "export default function/*START A*/ () {}/*END A*/",
}
// Classes

MODIFY
crates/rome_service/src/configuration/linter/rules.rs
crates/rome_service/src/configuration/linter/rules.rs
@@ -220,7 +220,7 @@ pub struct Js {
}
impl Js {
const CATEGORY_NAME: &'static str = "js";
- pub(crate) const CATEGORY_RULES: [&'static str; 29] = [
+ pub(crate) const CATEGORY_RULES: [&'static str; 30] = [
"noArguments",
"noAsyncPromiseExecutor",
"noCatchAssign",
@@ -229,6 +229,7 @@ impl Js {
"noDebugger",
"noDelete",
"noDoubleEquals",
+ "noDupeArgs",
"noEmptyPattern",
"noExtraBooleanCast",
"noFunctionAssign",
@@ -251,7 +252,7 @@ impl Js {
"useValidTypeof",
"useWhile",
];
- const RECOMMENDED_RULES: [&'static str; 27] = [
+ const RECOMMENDED_RULES: [&'static str; 28] = [
"noArguments",
"noAsyncPromiseExecutor",
"noCatchAssign",
@@ -259,6 +260,7 @@ impl Js {
"noDebugger",
"noDelete",
"noDoubleEquals",
+ "noDupeArgs",
"noEmptyPattern",
"noExtraBooleanCast",
"noFunctionAssign",
@@ -280,7 +282,7 @@ impl Js {
"useValidTypeof",
"useWhile",
];
- const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 27] = [
+ const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 28] = [
RuleFilter::Rule("js", Self::CATEGORY_RULES[0]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[1]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[2]),
@@ -302,12 +304,13 @@ impl Js {
RuleFilter::Rule("js", Self::CATEGORY_RULES[19]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[20]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[21]),
- RuleFilter::Rule("js", Self::CATEGORY_RULES[23]),
+ RuleFilter::Rule("js", Self::CATEGORY_RULES[22]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[24]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[25]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[26]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[27]),
RuleFilter::Rule("js", Self::CATEGORY_RULES[28]),
+ RuleFilter::Rule("js", Self::CATEGORY_RULES[29]),
];
pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) }
pub(crate) fn get_enabled_rules(&self) -> IndexSet<RuleFilter> {
@@ -334,7 +337,7 @@ impl Js {
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>; 27] {
+ pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 28] {
Self::RECOMMENDED_RULES_AS_FILTERS
}
}

MODIFY
website/src/docs/lint/rules/index.md
website/src/docs/lint/rules/index.md
@@ -77,6 +77,14 @@ Disallow the use of the <code>delete</code> operator
Require the use of <code>===</code> and <code>!==</code>
</div>
<div class="rule">
+<h3 data-toc-exclude id="noDupeArgs">
+ <a href="/docs/lint/rules/noDupeArgs">noDupeArgs (since v0.9.0)</a>
+ <a class="header-anchor" href="#noDupeArgs"></a>
+ <span class="recommended">recommended</span>
+</h3>
+Disallow duplicate function arguments name.
+</div>
+<div class="rule">
<h3 data-toc-exclude id="noEmptyPattern">
<a href="/docs/lint/rules/noEmptyPattern">noEmptyPattern (since v0.7.0)</a>
<a class="header-anchor" href="#noEmptyPattern"></a>

feat(rome_js_analyze): rule `noDupeArgs` (#2972)