Update: add props option to no-self-assign rule (fixes #6718)

docs/rules/no-self-assign.md

@@ -41,6 +41,52 @@ let foo = foo;
 [foo = 1] = [foo];
 ```
 
+## Options
+
+This rule has the option to check properties as well.
+
+```json
+{
+    "no-self-assign": ["error", {"props": false}]
+}
+```
+
+- `props` - if this is `true`, `no-self-assign` rule warns self-assignments of properties. Default is `false`.
+
+### props
+
+Examples of **incorrect** code for the `{ "props": true }` option:
+
+```js
+/*eslint no-self-assign: [error, {props: true}]*/
+
+// self-assignments with properties.
+obj.a = obj.a;
+obj.a.b = obj.a.b;
+obj["a"] = obj["a"];
+obj[a] = obj[a];
+```
+
+Examples of **correct** code for the `{ "props": true }` option:
+
+```js
+/*eslint no-self-assign: [error, {props: true}]*/
+
+// non-self-assignments with properties.
+obj.a = obj.b;
+obj.a.b = obj.c.b;
+obj.a.b = obj.a.c;
+obj[a] = obj["a"]
+
+// This ignores if there is a function call.
+obj.a().b = obj.a().b
+a().b = a().b
+
+// 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"];
+```
+
 ## When Not To Use It
 
 If you don't want to notify about self assignments, then it's safe to disable this rule.

lib/rules/no-self-assign.js

@@ -5,23 +5,79 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
 //------------------------------------------------------------------------------
 // Helpers
 //------------------------------------------------------------------------------
 
+const SPACES = /\s+/g;
+
+/**
+ * Checks whether the property of 2 given member expression nodes are the same
+ * property or not.
+ *
+ * @param {ASTNode} left - A member expression node to check.
+ * @param {ASTNode} right - Another member expression node to check.
+ * @returns {boolean} `true` if the member expressions have the same property.
+ */
+function isSameProperty(left, right) {
+    if (left.property.type === "Identifier" &&
+        left.property.type === right.property.type &&
+        left.property.name === right.property.name &&
+        left.computed === right.computed
+    ) {
+        return true;
+    }
+
+    const lname = astUtils.getStaticPropertyName(left);
+    const rname = astUtils.getStaticPropertyName(right);
+
+    return lname !== null && lname === rname;
+}
+
+/**
+ * Checks whether 2 given member expression nodes are the reference to the same
+ * property or not.
+ *
+ * @param {ASTNode} left - A member expression node to check.
+ * @param {ASTNode} right - Another member expression node to check.
+ * @returns {boolean} `true` if the member expressions are the reference to the
+ *  same property or not.
+ */
+function isSameMember(left, right) {
+    if (!isSameProperty(left, right)) {
+        return false;
+    }
+
+    const lobj = left.object;
+    const robj = right.object;
+
+    if (lobj.type !== robj.type) {
+        return false;
+    }
+    if (lobj.type === "MemberExpression") {
+        return isSameMember(lobj, robj);
+    }
+    return lobj.type === "Identifier" && lobj.name === robj.name;
+}
+
 /**
  * Traverses 2 Pattern nodes in parallel, then reports self-assignments.
  *
  * @param {ASTNode|null} left - A left node to traverse. This is a Pattern or
  *      a Property.
  * @param {ASTNode|null} right - A right node to traverse. This is a Pattern or
  *      a Property.
+ * @param {boolean} props - The flag to check member expressions as well.
  * @param {Function} report - A callback function to report.
  * @returns {void}
  */
-function eachSelfAssignment(left, right, report) {
-    let i, j;
-
+function eachSelfAssignment(left, right, props, report) {
     if (!left || !right) {
 
         // do nothing
@@ -37,10 +93,10 @@ function eachSelfAssignment(left, right, report) {
     ) {
         const end = Math.min(left.elements.length, right.elements.length);
 
-        for (i = 0; i < end; ++i) {
+        for (let i = 0; i < end; ++i) {
             const rightElement = right.elements[i];
 
-            eachSelfAssignment(left.elements[i], rightElement, report);
+            eachSelfAssignment(left.elements[i], rightElement, props, report);
 
             // After a spread element, those indices are unknown.
             if (rightElement && rightElement.type === "SpreadElement") {
@@ -51,7 +107,7 @@ function eachSelfAssignment(left, right, report) {
         left.type === "RestElement" &&
         right.type === "SpreadElement"
     ) {
-        eachSelfAssignment(left.argument, right.argument, report);
+        eachSelfAssignment(left.argument, right.argument, props, report);
     } else if (
         left.type === "ObjectPattern" &&
         right.type === "ObjectExpression" &&
@@ -62,18 +118,19 @@ function eachSelfAssignment(left, right, report) {
         // It's possible to overwrite properties followed by it.
         let startJ = 0;
 
-        for (i = right.properties.length - 1; i >= 0; --i) {
+        for (let i = right.properties.length - 1; i >= 0; --i) {
             if (right.properties[i].type === "ExperimentalSpreadProperty") {
                 startJ = i + 1;
                 break;
             }
         }
 
-        for (i = 0; i < left.properties.length; ++i) {
-            for (j = startJ; j < right.properties.length; ++j) {
+        for (let i = 0; i < left.properties.length; ++i) {
+            for (let j = startJ; j < right.properties.length; ++j) {
                 eachSelfAssignment(
                     left.properties[i],
                     right.properties[j],
+                    props,
                     report
                 );
             }
@@ -87,7 +144,14 @@ function eachSelfAssignment(left, right, report) {
         !right.method &&
         left.key.name === right.key.name
     ) {
-        eachSelfAssignment(left.value, right.value, report);
+        eachSelfAssignment(left.value, right.value, props, report);
+    } else if (
+        props &&
+        left.type === "MemberExpression" &&
+        right.type === "MemberExpression" &&
+        isSameMember(left, right)
+    ) {
+        report(right);
     }
 }
 
@@ -103,10 +167,23 @@ module.exports = {
             recommended: true
         },
 
-        schema: []
+        schema: [
+            {
+                type: "object",
+                properties: {
+                    props: {
+                        type: "boolean"
+                    }
+                },
+                additionalProperties: false
+            }
+        ]
     },
 
     create: function(context) {
+        const sourceCode = context.getSourceCode();
+        const options = context.options[0];
+        const props = Boolean(options && options.props);
 
         /**
          * Reports a given node as self assignments.
@@ -118,14 +195,16 @@ module.exports = {
             context.report({
                 node: node,
                 message: "'{{name}}' is assigned to itself.",
-                data: node
+                data: {
+                    name: sourceCode.getText(node).replace(SPACES, "")
+                }
             });
         }
 
         return {
             AssignmentExpression: function(node) {
                 if (node.operator === "=") {
-                    eachSelfAssignment(node.left, node.right, report);
+                    eachSelfAssignment(node.left, node.right, props, report);
                 }
             }
         };

tests/lib/rules/no-self-assign.js

@@ -39,7 +39,19 @@ ruleTester.run("no-self-assign", rule, {
         {code: "({a} = {a: b})", parserOptions: {ecmaVersion: 6}},
         {code: "({a} = {a() {}})", parserOptions: {ecmaVersion: 6}},
         {code: "({a} = {[a]: a})", parserOptions: {ecmaVersion: 6}},
-        {code: "({a, ...b} = {a, ...b})", parserOptions: {ecmaVersion: 6, ecmaFeatures: {experimentalObjectRestSpread: true}}}
+        {code: "({a, ...b} = {a, ...b})", parserOptions: {ecmaVersion: 6, ecmaFeatures: {experimentalObjectRestSpread: true}}},
+        {code: "a.b = a.c", options: [{props: true}]},
+        {code: "a.b = c.b", options: [{props: true}]},
+        {code: "a.b = a[b]", options: [{props: true}]},
+        {code: "a[b] = a.b", options: [{props: true}]},
+        {code: "a.b().c = a.b().c", options: [{props: true}]},
+        {code: "b().c = b().c", options: [{props: true}]},
+        {code: "a[b + 1] = a[b + 1]", options: [{props: true}]},  // it ignores non-simple computed properties.
+        {code: "a.b = a.b"},
+        {code: "a.b.c = a.b.c"},
+        {code: "a[b] = a[b]"},
+        {code: "a['b'] = a['b']"},
+        {code: "a[\n    'b'\n] = a[\n    'b'\n]"},
     ],
     invalid: [
         {code: "a = a", errors: ["'a' is assigned to itself."]},
@@ -55,6 +67,11 @@ ruleTester.run("no-self-assign", rule, {
         {code: "({a, b} = {b, a})", parserOptions: {ecmaVersion: 6}, errors: ["'b' is assigned to itself.", "'a' is assigned to itself."]},
         {code: "({a, b} = {c, a})", parserOptions: {ecmaVersion: 6}, errors: ["'a' is assigned to itself."]},
         {code: "({a: {b}, c: [d]} = {a: {b}, c: [d]})", parserOptions: {ecmaVersion: 6}, errors: ["'b' is assigned to itself.", "'d' is assigned to itself."]},
-        {code: "({a, b} = {a, ...x, b})", parserOptions: {ecmaVersion: 6, ecmaFeatures: {experimentalObjectRestSpread: true}}, errors: ["'b' is assigned to itself."]}
+        {code: "({a, b} = {a, ...x, b})", parserOptions: {ecmaVersion: 6, ecmaFeatures: {experimentalObjectRestSpread: true}}, errors: ["'b' is assigned to itself."]},
+        {code: "a.b = a.b", options: [{props: true}], errors: ["'a.b' is assigned to itself."]},
+        {code: "a.b.c = a.b.c", options: [{props: true}], errors: ["'a.b.c' is assigned to itself."]},
+        {code: "a[b] = a[b]", options: [{props: true}], errors: ["'a[b]' is assigned to itself."]},
+        {code: "a['b'] = a['b']", options: [{props: true}], errors: ["'a['b']' is assigned to itself."]},
+        {code: "a[\n    'b'\n] = a[\n    'b'\n]", options: [{props: true}], errors: ["'a['b']' is assigned to itself."]},
     ]
 });