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

* Chore: add `getStaticPropertyName` to ast-utils

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

* Chore: use `astUtils.getStaticProperty` for some places.

Author: mysticatea

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/ast-utils.js

@@ -584,5 +584,74 @@ module.exports = {
      */
     isFunction: function(node) {
         return Boolean(node && anyFunctionPattern.test(node.type));
+    },
+
+    /**
+     * Gets the property name of a given node.
+     * The node can be a MemberExpression, a Property, or a MethodDefinition.
+     *
+     * If the name is dynamic, this returns `null`.
+     *
+     * For examples:
+     *
+     *     a.b           // => "b"
+     *     a["b"]        // => "b"
+     *     a['b']        // => "b"
+     *     a[`b`]        // => "b"
+     *     a[100]        // => "100"
+     *     a[b]          // => null
+     *     a["a" + "b"]  // => null
+     *     a[tag`b`]     // => null
+     *     a[`${b}`]     // => null
+     *
+     *     let a = {b: 1}            // => "b"
+     *     let a = {["b"]: 1}        // => "b"
+     *     let a = {['b']: 1}        // => "b"
+     *     let a = {[`b`]: 1}        // => "b"
+     *     let a = {[100]: 1}        // => "100"
+     *     let a = {[b]: 1}          // => null
+     *     let a = {["a" + "b"]: 1}  // => null
+     *     let a = {[tag`b`]: 1}     // => null
+     *     let a = {[`${b}`]: 1}     // => null
+     *
+     * @param {ASTNode} node - The node to get.
+     * @returns {string|null} The property name if static. Otherwise, null.
+     */
+    getStaticPropertyName(node) {
+        let prop;
+
+        switch (node && node.type) {
+            case "Property":
+            case "MethodDefinition":
+                prop = node.key;
+                break;
+
+            case "MemberExpression":
+                prop = node.property;
+                break;
+
+            // no default
+        }
+
+        switch (prop && prop.type) {
+            case "Literal":
+                return String(prop.value);
+
+            case "TemplateLiteral":
+                if (prop.expressions.length === 0 && prop.quasis.length === 1) {
+                    return prop.quasis[0].value.cooked;
+                }
+                break;
+
+            case "Identifier":
+                if (!node.computed) {
+                    return prop.name;
+                }
+                break;
+
+            // no default
+        }
+
+        return null;
     }
 };

lib/rules/array-callback-return.js

@@ -45,38 +45,6 @@ function getLocation(node, sourceCode) {
     return node.id || node;
 }
 
-/**
- * Gets the name of a given node if the node is a Identifier node.
- *
- * @param {ASTNode} node - A node to get.
- * @returns {string} The name of the node, or an empty string.
- */
-function getIdentifierName(node) {
-    return node.type === "Identifier" ? node.name : "";
-}
-
-/**
- * Gets the value of a given node if the node is a Literal node or a
- * TemplateLiteral node.
- *
- * @param {ASTNode} node - A node to get.
- * @returns {string} The value of the node, or an empty string.
- */
-function getConstantStringValue(node) {
-    switch (node.type) {
-        case "Literal":
-            return String(node.value);
-
-        case "TemplateLiteral":
-            return node.expressions.length === 0
-                ? node.quasis[0].value.cooked
-                : "";
-
-        default:
-            return "";
-    }
-}
-
 /**
  * Checks a given node is a MemberExpression node which has the specified name's
  * property.
@@ -88,9 +56,7 @@ function getConstantStringValue(node) {
 function isTargetMethod(node) {
     return (
         node.type === "MemberExpression" &&
-        TARGET_METHODS.test(
-            (node.computed ? getConstantStringValue : getIdentifierName)(node.property)
-        )
+        TARGET_METHODS.test(astUtils.getStaticPropertyName(node) || "")
     );
 }
 

lib/rules/no-alert.js

@@ -4,6 +4,12 @@
  */
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const getPropertyName = require("../ast-utils").getStaticPropertyName;
+
 //------------------------------------------------------------------------------
 // Helpers
 //------------------------------------------------------------------------------
@@ -28,22 +34,6 @@ function report(context, node, identifierName) {
     context.report(node, "Unexpected {{name}}.", { name: identifierName });
 }
 
-/**
- * Returns the property name of a MemberExpression.
- * @param {ASTNode} memberExpressionNode The MemberExpression node.
- * @returns {string|null} Returns the property name if available, null else.
- */
-function getPropertyName(memberExpressionNode) {
-    if (memberExpressionNode.computed) {
-        if (memberExpressionNode.property.type === "Literal") {
-            return memberExpressionNode.property.value;
-        }
-    } else {
-        return memberExpressionNode.property.name;
-    }
-    return null;
-}
-
 /**
  * Finds the escope reference in the given scope.
  * @param {Object} scope The scope to search.

lib/rules/no-extra-bind.js

@@ -4,6 +4,12 @@
  */
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const getPropertyName = require("../ast-utils").getStaticPropertyName;
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -37,31 +43,6 @@ module.exports = {
             });
         }
 
-        /**
-         * Gets the property name of a given node.
-         * If the property name is dynamic, this returns an empty string.
-         *
-         * @param {ASTNode} node - A node to check. This is a MemberExpression.
-         * @returns {string} The property name of the node.
-         */
-        function getPropertyName(node) {
-            if (node.computed) {
-                switch (node.property.type) {
-                    case "Literal":
-                        return String(node.property.value);
-                    case "TemplateLiteral":
-                        if (node.property.expressions.length === 0) {
-                            return node.property.quasis[0].value.cooked;
-                        }
-
-                        // fallthrough
-                    default:
-                        return false;
-                }
-            }
-            return node.property.name;
-        }
-
         /**
          * Checks whether or not a given function node is the callee of `.bind()`
          * method.