diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_self_assign.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_self_assign.rs index d316ecb9139..0d202a9d179 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/no_self_assign.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_self_assign.rs @@ -2,11 +2,11 @@ use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic use rome_console::markup; use rome_js_syntax::{ AnyJsArrayAssignmentPatternElement, AnyJsArrayElement, AnyJsAssignment, AnyJsAssignmentPattern, - AnyJsExpression, AnyJsName, AnyJsObjectAssignmentPatternMember, AnyJsObjectMember, - JsAssignmentExpression, JsAssignmentOperator, JsComputedMemberAssignment, - JsComputedMemberExpression, JsIdentifierAssignment, JsIdentifierExpression, JsLanguage, JsName, - JsPrivateName, JsReferenceIdentifier, JsStaticMemberAssignment, JsStaticMemberExpression, - JsSyntaxToken, + AnyJsExpression, AnyJsLiteralExpression, AnyJsName, AnyJsObjectAssignmentPatternMember, + AnyJsObjectMember, JsAssignmentExpression, JsAssignmentOperator, JsCallExpression, + JsComputedMemberAssignment, JsComputedMemberExpression, JsIdentifierAssignment, + JsIdentifierExpression, JsLanguage, JsName, JsPrivateName, JsReferenceIdentifier, + JsStaticMemberAssignment, JsStaticMemberExpression, JsSyntaxToken, }; use rome_rowan::{ declare_node_union, AstNode, AstSeparatedList, AstSeparatedListNodesIterator, SyntaxError, @@ -46,6 +46,14 @@ declare_rule! { /// a[b] = a[b]; /// ``` /// + /// ```js,expect_diagnostic + /// a[b].foo = a[b].foo; + /// ``` + /// + /// ```js,expect_diagnostic + /// a['b'].foo = a['b'].foo; + /// ``` + /// /// ## Valid /// /// ```js @@ -304,9 +312,10 @@ impl SameIdentifiers { left: &mut AnyJsAssignmentExpressionLikeIterator, right: &mut AnyJsAssignmentExpressionLikeIterator, ) -> Option { - if let (Some(left), Some(right)) = (left.next(), right.next()) { - let (left_name, left_reference) = left; - let (right_name, right_reference) = right; + if let (Some(left_item), Some(right_item)) = (left.next(), right.next()) { + let (left_name, left_reference) = left_item; + let (right_name, right_reference) = right_item; + if let Ok(identifier_like) = IdentifiersLike::try_from((left_name, right_name)) { if with_same_identifiers(&identifier_like).is_some() { if let (Some(left_reference), Some(right_reference)) = @@ -318,8 +327,15 @@ impl SameIdentifiers { )) .is_some() { - return Some(AnyAssignmentLike::Identifiers(identifier_like)); + let source_identifier = IdentifiersLike::try_from(( + left.source_member.clone(), + right.source_member.clone(), + )) + .ok()?; + return Some(AnyAssignmentLike::Identifiers(source_identifier)); } + } else { + return Self::next_static_expression(left, right); } } } @@ -457,8 +473,21 @@ impl Iterator for AnyJsAssignmentExpressionLikeIterator { self.drained = true; Some(identifier.name().ok()?) } + AnyJsExpression::JsCallExpression(call_expression) => { + self.current_member_expression = Some( + AnyAssignmentExpressionLike::JsCallExpression(call_expression), + ); + None + } + AnyJsExpression::JsComputedMemberExpression(computed_expression) => { + self.current_member_expression = Some( + AnyAssignmentExpressionLike::JsComputedMemberExpression(computed_expression), + ); + None + } _ => return None, }; + Some((name, reference)) } } @@ -510,11 +539,11 @@ enum AnyAssignmentLike { } declare_node_union! { - pub(crate) AnyNameLike = AnyJsName | JsReferenceIdentifier | JsIdentifierExpression + pub(crate) AnyNameLike = AnyJsName | JsReferenceIdentifier | JsIdentifierExpression | AnyJsLiteralExpression } declare_node_union! { - pub(crate) AnyAssignmentExpressionLike = JsStaticMemberExpression | JsComputedMemberExpression + pub(crate) AnyAssignmentExpressionLike = JsStaticMemberExpression | JsComputedMemberExpression | JsCallExpression } impl AnyAssignmentExpressionLike { @@ -525,11 +554,22 @@ impl AnyAssignmentExpressionLike { } AnyAssignmentExpressionLike::JsComputedMemberExpression(node) => { node.member().ok().and_then(|node| { - Some(AnyNameLike::from( - node.as_js_identifier_expression()?.name().ok()?, - )) + Some(match node { + AnyJsExpression::JsIdentifierExpression(node) => node.name().ok()?.into(), + AnyJsExpression::AnyJsLiteralExpression(node) => node.into(), + _ => return None, + }) }) } + AnyAssignmentExpressionLike::JsCallExpression(node) => node + .callee() + .ok() + .and_then(|callee| callee.as_js_static_member_expression().cloned()) + .and_then(|callee| callee.member().ok()) + .and_then(|member| { + let js_name = member.as_js_name()?; + Some(AnyNameLike::from(AnyJsName::JsName(js_name.clone()))) + }), } } @@ -537,6 +577,12 @@ impl AnyAssignmentExpressionLike { match self { AnyAssignmentExpressionLike::JsStaticMemberExpression(node) => node.object().ok(), AnyAssignmentExpressionLike::JsComputedMemberExpression(node) => node.object().ok(), + AnyAssignmentExpressionLike::JsCallExpression(node) => node + .callee() + .ok()? + .as_js_static_member_expression()? + .object() + .ok(), } } } @@ -641,6 +687,13 @@ pub(crate) enum IdentifiersLike { /// a.#b = a.#b; /// ``` PrivateName(JsPrivateName, JsPrivateName), + /// To store identifiers found in code like: + /// + /// ```js + /// a['b'].d = a['b'].d + /// a[3].d = a[4].d + /// ``` + Literal(AnyJsLiteralExpression, AnyJsLiteralExpression), } impl TryFrom<(AnyNameLike, AnyNameLike)> for IdentifiersLike { @@ -667,6 +720,11 @@ impl TryFrom<(AnyNameLike, AnyNameLike)> for IdentifiersLike { AnyNameLike::JsIdentifierExpression(right), ) => Ok(Self::References(left.name()?, right.name()?)), + ( + AnyNameLike::AnyJsLiteralExpression(left), + AnyNameLike::AnyJsLiteralExpression(right), + ) => Ok(Self::Literal(left, right)), + _ => unreachable!("you should map the correct references"), } } @@ -679,6 +737,7 @@ impl IdentifiersLike { IdentifiersLike::Name(left, _) => left.range(), IdentifiersLike::PrivateName(left, _) => left.range(), IdentifiersLike::References(left, _) => left.range(), + IdentifiersLike::Literal(left, _) => left.range(), } } @@ -688,6 +747,7 @@ impl IdentifiersLike { IdentifiersLike::Name(_, right) => right.range(), IdentifiersLike::PrivateName(_, right) => right.range(), IdentifiersLike::References(_, right) => right.range(), + IdentifiersLike::Literal(_, right) => right.range(), } } @@ -697,6 +757,7 @@ impl IdentifiersLike { IdentifiersLike::Name(_, right) => right.value_token().ok(), IdentifiersLike::PrivateName(_, right) => right.value_token().ok(), IdentifiersLike::References(_, right) => right.value_token().ok(), + IdentifiersLike::Literal(_, right) => right.value_token().ok(), } } } @@ -724,6 +785,27 @@ fn with_same_identifiers(identifiers_like: &IdentifiersLike) -> Option<()> { let right_value = right.value_token().ok()?; (left_value, right_value) } + IdentifiersLike::Literal(left, right) => match (left, right) { + ( + AnyJsLiteralExpression::JsStringLiteralExpression(left), + AnyJsLiteralExpression::JsStringLiteralExpression(right), + ) => { + let left_value = left.value_token().ok()?; + let right_value = right.value_token().ok()?; + (left_value, right_value) + } + + ( + AnyJsLiteralExpression::JsNumberLiteralExpression(left), + AnyJsLiteralExpression::JsNumberLiteralExpression(right), + ) => { + let left_value = left.value_token().ok()?; + let right_value = right.value_token().ok()?; + (left_value, right_value) + } + + _ => return None, + }, }; if left_value.text_trimmed() == right_value.text_trimmed() { diff --git a/crates/rome_js_analyze/src/lib.rs b/crates/rome_js_analyze/src/lib.rs index 1ed4a702b61..398e4cb58d6 100644 --- a/crates/rome_js_analyze/src/lib.rs +++ b/crates/rome_js_analyze/src/lib.rs @@ -179,12 +179,7 @@ mod tests { String::from_utf8(buffer).unwrap() } - const SOURCE: &str = r#"function f() { - if (true){ - useMyEffect(() => {}, []); - - } - }"#; + const SOURCE: &str = r#"a[f].c = a[f].c;"#; let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default()); @@ -195,7 +190,7 @@ mod tests { closure_index: Some(0), dependencies_index: Some(1), }; - let rule_filter = RuleFilter::Rule("nursery", "useHookAtTopLevel"); + let rule_filter = RuleFilter::Rule("nursery", "noSelfAssign"); options.configuration.rules.push_rule( RuleKey::new("nursery", "useHookAtTopLevel"), RuleOptions::new(HooksOptions { hooks: vec![hook] }), diff --git a/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js index d05f87c4cd3..d8df9a581dc 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js +++ b/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js @@ -17,5 +17,10 @@ a = a; ({a, b} = {a, ...x, b}); a.b = a.b; a.#b = a.#b; -a.c.b = a.ZZ.b; a[b] = a[b]; +a.b().c = a.b().c; +a.b.c = a.b.c; +({a} = {a}); +a['b'].bar = a['b'].bar; +a[foobar].b = a[foobar].b; +a[10].b = a[10].b; diff --git a/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js.snap index aa9c1b4c9cc..2ba479f2370 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noSelfAssign/invalid.js.snap @@ -1,6 +1,5 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs -assertion_line: 96 expression: invalid.js --- # Input @@ -24,8 +23,13 @@ a = a; ({a, b} = {a, ...x, b}); a.b = a.b; a.#b = a.#b; -a.c.b = a.ZZ.b; a[b] = a[b]; +a.b().c = a.b().c; +a.b.c = a.b.c; +({a} = {a}); +a['b'].bar = a['b'].bar; +a[foobar].b = a[foobar].b; +a[10].b = a[10].b; ``` @@ -682,7 +686,7 @@ invalid.js:18:9 lint/nursery/noSelfAssign ━━━━━━━━━━━━ > 18 │ a.b = a.b; │ ^ 19 │ a.#b = a.#b; - 20 │ a.c.b = a.ZZ.b; + 20 │ a[b] = a[b]; i This is where is assigned. @@ -691,7 +695,7 @@ invalid.js:18:9 lint/nursery/noSelfAssign ━━━━━━━━━━━━ > 18 │ a.b = a.b; │ ^ 19 │ a.#b = a.#b; - 20 │ a.c.b = a.ZZ.b; + 20 │ a[b] = a[b]; ``` @@ -705,8 +709,8 @@ invalid.js:19:10 lint/nursery/noSelfAssign ━━━━━━━━━━━━ 18 │ a.b = a.b; > 19 │ a.#b = a.#b; │ ^^ - 20 │ a.c.b = a.ZZ.b; - 21 │ a[b] = a[b]; + 20 │ a[b] = a[b]; + 21 │ a.b().c = a.b().c; i This is where is assigned. @@ -714,30 +718,174 @@ invalid.js:19:10 lint/nursery/noSelfAssign ━━━━━━━━━━━━ 18 │ a.b = a.b; > 19 │ a.#b = a.#b; │ ^^ - 20 │ a.c.b = a.ZZ.b; - 21 │ a[b] = a[b]; + 20 │ a[b] = a[b]; + 21 │ a.b().c = a.b().c; ``` ``` -invalid.js:21:10 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.js:20:10 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! b is assigned to itself. + 18 │ a.b = a.b; 19 │ a.#b = a.#b; - 20 │ a.c.b = a.ZZ.b; - > 21 │ a[b] = a[b]; + > 20 │ a[b] = a[b]; │ ^ - 22 │ + 21 │ a.b().c = a.b().c; + 22 │ a.b.c = a.b.c; i This is where is assigned. + 18 │ a.b = a.b; 19 │ a.#b = a.#b; - 20 │ a.c.b = a.ZZ.b; - > 21 │ a[b] = a[b]; + > 20 │ a[b] = a[b]; │ ^ - 22 │ + 21 │ a.b().c = a.b().c; + 22 │ a.b.c = a.b.c; + + +``` + +``` +invalid.js:21:17 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! c is assigned to itself. + + 19 │ a.#b = a.#b; + 20 │ a[b] = a[b]; + > 21 │ a.b().c = a.b().c; + │ ^ + 22 │ a.b.c = a.b.c; + 23 │ ({a} = {a}); + + i This is where is assigned. + + 19 │ a.#b = a.#b; + 20 │ a[b] = a[b]; + > 21 │ a.b().c = a.b().c; + │ ^ + 22 │ a.b.c = a.b.c; + 23 │ ({a} = {a}); + + +``` + +``` +invalid.js:22:13 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! c is assigned to itself. + + 20 │ a[b] = a[b]; + 21 │ a.b().c = a.b().c; + > 22 │ a.b.c = a.b.c; + │ ^ + 23 │ ({a} = {a}); + 24 │ a['b'].bar = a['b'].bar; + + i This is where is assigned. + + 20 │ a[b] = a[b]; + 21 │ a.b().c = a.b().c; + > 22 │ a.b.c = a.b.c; + │ ^ + 23 │ ({a} = {a}); + 24 │ a['b'].bar = a['b'].bar; + + +``` + +``` +invalid.js:23:9 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! a is assigned to itself. + + 21 │ a.b().c = a.b().c; + 22 │ a.b.c = a.b.c; + > 23 │ ({a} = {a}); + │ ^ + 24 │ a['b'].bar = a['b'].bar; + 25 │ a[foobar].b = a[foobar].b; + + i This is where is assigned. + + 21 │ a.b().c = a.b().c; + 22 │ a.b.c = a.b.c; + > 23 │ ({a} = {a}); + │ ^ + 24 │ a['b'].bar = a['b'].bar; + 25 │ a[foobar].b = a[foobar].b; + + +``` + +``` +invalid.js:24:21 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! bar is assigned to itself. + + 22 │ a.b.c = a.b.c; + 23 │ ({a} = {a}); + > 24 │ a['b'].bar = a['b'].bar; + │ ^^^ + 25 │ a[foobar].b = a[foobar].b; + 26 │ a[10].b = a[10].b; + + i This is where is assigned. + + 22 │ a.b.c = a.b.c; + 23 │ ({a} = {a}); + > 24 │ a['b'].bar = a['b'].bar; + │ ^^^ + 25 │ a[foobar].b = a[foobar].b; + 26 │ a[10].b = a[10].b; + + +``` + +``` +invalid.js:25:25 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! b is assigned to itself. + + 23 │ ({a} = {a}); + 24 │ a['b'].bar = a['b'].bar; + > 25 │ a[foobar].b = a[foobar].b; + │ ^ + 26 │ a[10].b = a[10].b; + 27 │ + + i This is where is assigned. + + 23 │ ({a} = {a}); + 24 │ a['b'].bar = a['b'].bar; + > 25 │ a[foobar].b = a[foobar].b; + │ ^ + 26 │ a[10].b = a[10].b; + 27 │ + + +``` + +``` +invalid.js:26:17 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! b is assigned to itself. + + 24 │ a['b'].bar = a['b'].bar; + 25 │ a[foobar].b = a[foobar].b; + > 26 │ a[10].b = a[10].b; + │ ^ + 27 │ + + i This is where is assigned. + + 24 │ a['b'].bar = a['b'].bar; + 25 │ a[foobar].b = a[foobar].b; + > 26 │ a[10].b = a[10].b; + │ ^ + 27 │ ``` diff --git a/website/src/pages/lint/rules/noSelfAssign.md b/website/src/pages/lint/rules/noSelfAssign.md index ee0cd837475..f9522ca7c8f 100644 --- a/website/src/pages/lint/rules/noSelfAssign.md +++ b/website/src/pages/lint/rules/noSelfAssign.md @@ -115,6 +115,46 @@ a[b] = a[b]; +```jsx +a[b].foo = a[b].foo; +``` + +
nursery/noSelfAssign.js:1:17 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ foo is assigned to itself.
+  
+  > 1 │ a[b].foo = a[b].foo;
+      │                 ^^^
+    2 │ 
+  
+  ℹ This is where is assigned.
+  
+  > 1 │ a[b].foo = a[b].foo;
+      │      ^^^
+    2 │ 
+  
+
+ +```jsx +a['b'].foo = a['b'].foo; +``` + +
nursery/noSelfAssign.js:1:21 lint/nursery/noSelfAssign ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ foo is assigned to itself.
+  
+  > 1 │ a['b'].foo = a['b'].foo;
+      │                     ^^^
+    2 │ 
+  
+  ℹ This is where is assigned.
+  
+  > 1 │ a['b'].foo = a['b'].foo;
+      │        ^^^
+    2 │ 
+  
+
+ ## Valid ```jsx