Repository: eslint/eslint
Pull request #14152 (2021-03-01 14:32:41)

feat: handle logical assignment in no-self-assign
<!--
    Thank you for contributing!

    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

#### Prerequisites checklist

- [x] I have read the [contributing guidelines](https://github.com/eslint/eslint/blob/master/CONTRIBUTING.md).

#### What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
[ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
[x] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

**What rule do you want to change?**
no-self-assign

**Does this change cause the rule to produce more or fewer warnings?**
more warnings

**How will the change be implemented? (New option, new default behavior, etc.)?**
new default behavior

**Please provide some example code that this change will affect:**

```js
a &= a;  
a |= a;
a &&= a;
a ||= a;
a ??= a;
```

**What does the rule currently do for this code?**
no errors are reported

**What will the rule do after it's changed?**
all lines will be reported.


<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

#### What changes did you make? (Give an overview)
currently, only `=` is handled in `no-self-assign`. But other logical assignment operators work roughly the same as `=`.

`a &&= a` is `a && (a = a)`, which is essentially a noop.

#### Is there anything you'd like reviewers to focus on?



mdjermanovic: Hi @Zzzen, thanks for the PR!

We listed adding support for `&&=`, `||=`, and `??=` operators to the `no-self-assign` rule as _maybe_ in https://github.com/eslint/eslint/issues/13569. We're not yet sure if that change makes sense, so I marked this as "evaluating".

As for `&=` and `|=`, I think that's different as it involves a calculation before the assignment, and it doesn't have to be a noop (should be for integers, but e.g, `1.5 | 1.5` is `1`, `null | null` is `0`, etc.), so those checks probably don't belong to `no-self-assign`.
Zzzen: Ooops, did not notice the issue. IMO, even though `|=` may have side effects on non-integers, `a |= a` is likely a typo and should be disallowed like how object properties are handled.
github-actions[bot]: Oops! It looks like we lost track of this pull request. What do we want to do here? This pull request will auto-close in 7 days without an update.
mdjermanovic: @eslint/eslint-tsc thoughts about this proposal for the `no-self-assign` rule?

* Should it report `a &&= a`, `a ||= a`, and `a ??= a`? I think this makes sense since these are essentially the same as `a = a`.
* Should it report `a &= a`, and `a |= a`? I think this doesn't belong to this rule, for the reasons described in [this comment](https://github.com/eslint/eslint/pull/14152#issuecomment-788062375).


nzakas: @mdjermanovic i agree with your reasoning. 
btmills: @mdjermanovic I agree as well.
linux-foundation-easycla[bot]: <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19114526/11061773/14152/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328" ></a><br/><br /><ul><li><a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19114526/11061773/14152/#/?version=2' target='_blank'>:x:</a> - Zzzen The commit (28715cf15c245bb0b15412ccde9e287fe7f0085d ,2dc996f51f3cc56143500446b268c685f50cad9c) is not authorized under a signed CLA. <a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19114526/11061773/14152/#/?version=2' target='_blank'>Please click here to be authorized</a>. For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul>
Zzzen: 👌  updated

Patch:
diff --git a/docs/rules/no-self-assign.md b/docs/rules/no-self-assign.md
index f7132e2b1..8b90cdfd3 100644
--- a/docs/rules/no-self-assign.md
+++ b/docs/rules/no-self-assign.md
@@ -24,6 +24,10 @@ foo = foo;
 [a, ...b] = [x, ...b];
 
 ({a, b} = {a, x});
+
+foo &&= foo;
+foo ||= foo;
+foo ??= foo;
 ```
 
 Examples of **correct** code for this rule:
@@ -50,6 +54,10 @@ obj[a] = obj["a"];
 obj.a().b = obj.a().b;
 a().b = a().b;
 
+// `&=` and `|=` have an effect on non-integers.
+foo &= foo;
+foo |= foo;
+
 // Known limitation: this does not support computed properties except single literal or single identifier.
 obj[a + b] = obj[a + b];
 obj["a" + "b"] = obj["a" + "b"];
diff --git a/lib/rules/no-self-assign.js b/lib/rules/no-self-assign.js
index d9e261dfa..060cc8e35 100644
--- a/lib/rules/no-self-assign.js
+++ b/lib/rules/no-self-assign.js
@@ -174,7 +174,7 @@ module.exports = {
 
         return {
             AssignmentExpression(node) {
-                if (node.operator === "=") {
+                if (["=", "&&=", "||=", "??="].includes(node.operator)) {
                     eachSelfAssignment(node.left, node.right, props, report);
                 }
             }


Test patch:
diff --git a/tests/lib/rules/no-self-assign.js b/tests/lib/rules/no-self-assign.js
index 04aa2dee4..0fe04040d 100644
--- a/tests/lib/rules/no-self-assign.js
+++ b/tests/lib/rules/no-self-assign.js
@@ -25,6 +25,8 @@ ruleTester.run("no-self-assign", rule, {
         "a += a",
         "a = +a",
         "a = [a]",
+        "a &= a",
+        "a |= a",
         { code: "let a = a", parserOptions: { ecmaVersion: 6 } },
         { code: "const a = a", parserOptions: { ecmaVersion: 6 } },
         { code: "[a] = a", parserOptions: { ecmaVersion: 6 } },
@@ -167,6 +169,23 @@ ruleTester.run("no-self-assign", rule, {
             code: "class C { #field; foo() { [this.#field] = [this.#field]; } }",
             parserOptions: { ecmaVersion: 2022 },
             errors: [{ messageId: "selfAssignment", data: { name: "this.#field" } }]
+        },
+
+        // logical assignment
+        {
+            code: "a &&= a",
+            parserOptions: { ecmaVersion: 2021 },
+            errors: [{ messageId: "selfAssignment", data: { name: "a" } }]
+        },
+        {
+            code: "a ||= a",
+            parserOptions: { ecmaVersion: 2021 },
+            errors: [{ messageId: "selfAssignment", data: { name: "a" } }]
+        },
+        {
+            code: "a ??= a",
+            parserOptions: { ecmaVersion: 2021 },
+            errors: [{ messageId: "selfAssignment", data: { name: "a" } }]
         }
     ]
 });