New: Adds prefer-object-spread rule (refs: #7230) (#9955)

* New: Adds prefer-object-spread rule (refs: #7230)

* Adds exception for `Object.assign` with one object literal argument

This was an exception to this rule that we use internally, in the case that an
`Object.assign` call is made with an object literal as the only argument. The
`Object.assign` call is not needed and we can just use the object literal
directly.

* Refactors and adds multiline object fixes

* Many fixes and improvements:
- handles nested object literals
- handles various comment types
- places comma after arguments in a smarter way

* Fixes typo and improves wording in docs

* include string.prototype.matchall in package.json

* Several improvements:

- adds line and column numbers to tests
- warn on cases where argument is a spread element,
- use getCommentsInside instead of regex
- fix example in doc

* use spread instead of apply

* Check if the native `Object` is being overwritten, if it is, do not warn.

Bug fix to ensure that an we're warning on an `Object.assign` for the literal case.

* make messages consistent

conf/eslint-recommended.js

@@ -230,6 +230,7 @@ module.exports = {
         "prefer-const": "off",
         "prefer-destructuring": "off",
         "prefer-numeric-literals": "off",
+        "prefer-object-spread": "off",
         "prefer-promise-reject-errors": "off",
         "prefer-reflect": "off",
         "prefer-rest-params": "off",

docs/rules/prefer-object-spread.md

@@ -0,0 +1,47 @@
+# Prefer use of an object spread over `Object.assign` (prefer-object-spread)
+
+When Object.assign is called using an object literal as the first argument, this rule requires using the object spread syntax instead. This rule also warns on cases where an `Object.assign` call is made using a single argument that is an object literal, in this case, the `Object.assign` call is not needed.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+
+Object.assign({}, foo)
+
+Object.assign({}, {foo: 'bar'})
+
+Object.assign({ foo: 'bar'}, baz)
+
+Object.assign({ foo: 'bar' }, Object.assign({ bar: 'foo' }))
+
+Object.assign({}, { foo, bar, baz })
+
+Object.assign({}, { ...baz })
+
+// Object.assign with a single argument that is an object literal
+Object.assign({});
+
+Object.assign({ foo: bar });
+```
+
+Examples of **correct** code for this rule:
+
+```js
+
+Object.assign(...foo);
+
+// Any Object.assign call without an object literal as the first argument
+Object.assign(foo, { bar: baz });
+
+Object.assign(foo, Object.assign(bar));
+
+Object.assign(foo, { bar, baz })
+
+Object.assign(foo, { ...baz });
+```
+
+## When Not To Use It
+
+When you don't care about syntactic sugar added by the object spread property.

lib/rules/prefer-object-spread.js

@@ -0,0 +1,278 @@
+/**
+ * @fileoverview Prefers object spread property over Object.assign
+ * @author Sharmila Jesupaul
+ * See LICENSE file in root directory for full license.
+ */
+
+"use strict";
+
+const matchAll = require("string.prototype.matchall");
+
+/**
+ * Helper that checks if the node is an Object.assign call
+ * @param {ASTNode} node - The node that the rule warns on
+ * @returns {boolean} - Returns true if the node is an Object.assign call
+ */
+function isObjectAssign(node) {
+    return (
+        node.callee &&
+        node.callee.type === "MemberExpression" &&
+        node.callee.object.name === "Object" &&
+        node.callee.property.name === "assign"
+    );
+}
+
+/**
+ * Helper that checks if the node needs parentheses to be valid JS.
+ * The default is to wrap the node in parentheses to avoid parsing errors.
+ * @param {ASTNode} node - The node that the rule warns on
+ * @returns {boolean} - Returns true if the node needs parentheses
+ */
+function needsParens(node) {
+    const parent = node.parent;
+
+    if (!parent || !node.type) {
+        return true;
+    }
+
+    switch (parent.type) {
+        case "VariableDeclarator":
+        case "ArrayExpression":
+        case "ReturnStatement":
+        case "CallExpression":
+        case "Property":
+            return false;
+        default:
+            return true;
+    }
+}
+
+/**
+ * Determines if an argument needs parentheses. The default is to not add parens.
+ * @param {ASTNode} node - The node to be checked.
+ * @returns {boolean} True if the node needs parentheses
+ */
+function argNeedsParens(node) {
+    if (!node.type) {
+        return false;
+    }
+
+    switch (node.type) {
+        case "AssignmentExpression":
+        case "ArrowFunctionExpression":
+        case "ConditionalExpression":
+            return true;
+        default:
+            return false;
+    }
+}
+
+/**
+ * Helper that adds a comma after the last non-whitespace character that is not a part of a comment.
+ * @param {string} formattedArg - String of argument text
+ * @param {array} comments - comments inside the argument
+ * @returns {string} - argument with comma at the end of it
+ */
+function addComma(formattedArg, comments) {
+    const nonWhitespaceCharacterRegex = /[^\s\\]/g;
+    const nonWhitespaceCharacters = Array.from(matchAll(formattedArg, nonWhitespaceCharacterRegex));
+    const commentRanges = comments.map(comment => comment.range);
+    const validWhitespaceMatches = [];
+
+    // Create a list of indexes where non-whitespace characters exist.
+    nonWhitespaceCharacters.forEach(match => {
+        const insertIndex = match.index + match[0].length;
+
+        if (!commentRanges.length) {
+            validWhitespaceMatches.push(insertIndex);
+        }
+
+        // If comment ranges are found make sure that the non whitespace characters are not part of the comment.
+        commentRanges.forEach(arr => {
+            const commentStart = arr[0];
+            const commentEnd = arr[1];
+
+            if (insertIndex < commentStart || insertIndex > commentEnd) {
+                validWhitespaceMatches.push(insertIndex);
+            }
+        });
+    });
+    const insertPos = Math.max(...validWhitespaceMatches);
+    const regex = new RegExp(`^((?:.|[^/s/S]){${insertPos}}) *`);
+
+    return formattedArg.replace(regex, "$1, ");
+}
+
+/**
+ * Helper formats an argument by either removing curlies or adding a spread operator
+ * @param {ASTNode|null} arg - ast node representing argument to format
+ * @param {boolean} isLast - true if on the last element of the array
+ * @param {Object} sourceCode - in context sourcecode object
+ * @param {array} comments - comments inside checked node
+ * @returns {string} - formatted argument
+ */
+function formatArg(arg, isLast, sourceCode, comments) {
+    const text = sourceCode.getText(arg);
+    const parens = argNeedsParens(arg);
+    const spread = arg.type === "SpreadElement" ? "" : "...";
+
+    if (arg.type === "ObjectExpression" && arg.properties.length === 0) {
+        return "";
+    }
+
+    if (arg.type === "ObjectExpression") {
+
+        /**
+         * This regex finds the opening curly brace and any following spaces and replaces it with whatever
+         * exists before the curly brace. It also does the same for the closing curly brace. This is to avoid
+         * having multiple spaces around the object expression depending on how the object properties are spaced.
+         */
+        const formattedObjectLiteral = text.replace(/^(.*){ */, "$1").replace(/ *}([^}]*)$/, "$1");
+
+        return isLast ? formattedObjectLiteral : addComma(formattedObjectLiteral, comments);
+    }
+
+    if (isLast) {
+        return parens ? `${spread}(${text})` : `${spread}${text}`;
+    }
+
+    return parens ? addComma(`${spread}(${text})`, comments) : `${spread}${addComma(text, comments)}`;
+}
+
+/**
+ * Autofixes the Object.assign call to use an object spread instead.
+ * @param {ASTNode|null} node - The node that the rule warns on, i.e. the Object.assign call
+ * @param {string} sourceCode - sourceCode of the Object.assign call
+ * @returns {Function} autofixer - replaces the Object.assign with a spread object.
+ */
+function autofixSpread(node, sourceCode) {
+    return fixer => {
+        const args = node.arguments;
+        const firstArg = args[0];
+        const lastArg = args[args.length - 1];
+        const parens = needsParens(node);
+        const comments = sourceCode.getCommentsInside(node);
+        const replaceObjectAssignStart = fixer.replaceTextRange(
+            [node.range[0], firstArg.range[0]],
+            `${parens ? "({" : "{"}`
+        );
+
+        const handleArgs = args
+            .map((arg, i, arr) => formatArg(arg, i + 1 >= arr.length, sourceCode, comments))
+            .filter(arg => arg !== "," && arg !== "");
+
+        const insertBody = fixer.replaceTextRange([firstArg.range[0], lastArg.range[1]], handleArgs.join(""));
+        const replaceObjectAssignEnd = fixer.replaceTextRange([lastArg.range[1], node.range[1]], `${parens ? "})" : "}"}`);
+
+        return [
+            replaceObjectAssignStart,
+            insertBody,
+            replaceObjectAssignEnd
+        ];
+    };
+}
+
+/**
+ * Autofixes the Object.assign call with a single object literal as an argument
+ * @param {ASTNode|null} node - The node that the rule warns on, i.e. the Object.assign call
+ * @param {string} sourceCode - sourceCode of the Object.assign call
+ * @returns {Function} autofixer - replaces the Object.assign with a object literal.
+ */
+function autofixObjectLiteral(node, sourceCode) {
+    return fixer => {
+        const argument = node.arguments[0];
+        const parens = needsParens(node);
+
+        return fixer.replaceText(node, `${parens ? "(" : ""}${sourceCode.text.slice(argument.range[0], argument.range[1])}${parens ? ")" : ""}`);
+    };
+}
+
+
+module.exports = {
+    meta: {
+        docs: {
+            description:
+                "disallow using Object.assign with an object literal as the first argument and prefer the use of object spread instead.",
+            category: "Stylistic Issues",
+            recommended: false,
+            url: "https://eslint.org/docs/rules/prefer-object-spread"
+        },
+        schema: [],
+        fixable: "code",
+        messages: {
+            useSpreadMessage: "Use an object spread instead of `Object.assign` eg: `{ ...foo }`",
+            useLiteralMessage: "Use an object literal instead of `Object.assign`. eg: `{ foo: bar }`"
+        }
+    },
+
+    create: function rule(context) {
+        const sourceCode = context.getSourceCode();
+        let overWritingNativeObject = false;
+
+        return {
+            VariableDeclaration(node) {
+
+                /*
+                 * If a declared variable is overwriting the native `Object`, eg. `const Object = 'something'`
+                 * we want to skip warning on them since the behavior might be different.
+                 */
+                if (node.declarations.length) {
+                    const declarationNames = node.declarations.map(dec => dec.id);
+                    const nativeObjectOverride = declarationNames.filter(identifier => identifier.name === "Object");
+
+                    if (nativeObjectOverride.length) {
+                        overWritingNativeObject = true;
+                    }
+                }
+            },
+            AssignmentExpression(node) {
+
+                /*
+                 * If an assignment is reassigning the native `Object`, eg. `Object = 'something'`
+                 * we want to skip warning on them since the behavior might be different.
+                 */
+                if (node.left.name === "Object") {
+                    overWritingNativeObject = true;
+                }
+            },
+            CallExpression(node) {
+
+                /*
+                 * The condition below is cases where Object.assign has a single argument and
+                 * that argument is an object literal. e.g. `Object.assign({ foo: bar })`.
+                 * For now, we will warn on this case and autofix it.
+                 */
+                if (
+                    !overWritingNativeObject &&
+                    node.arguments.length === 1 &&
+                    node.arguments[0].type === "ObjectExpression" &&
+                    isObjectAssign(node)
+                ) {
+                    context.report({
+                        node,
+                        messageId: "useLiteralMessage",
+                        fix: autofixObjectLiteral(node, sourceCode)
+                    });
+                }
+
+                /*
+                 * The condition below warns on `Object.assign` calls that that have
+                 * an object literal as the first argument and have a second argument
+                 * that can be spread. e.g `Object.assign({ foo: bar }, baz)`
+                 */
+                if (
+                    !overWritingNativeObject &&
+                    node.arguments.length > 1 &&
+                    node.arguments[0].type === "ObjectExpression" &&
+                    isObjectAssign(node)
+                ) {
+                    context.report({
+                        node,
+                        messageId: "useSpreadMessage",
+                        fix: autofixSpread(node, sourceCode)
+                    });
+                }
+            }
+        };
+    }
+};

package.json

@@ -67,6 +67,7 @@
     "regexpp": "^1.1.0",
     "require-uncached": "^1.0.3",
     "semver": "^5.5.0",
+    "string.prototype.matchall": "^2.0.0",
     "strip-ansi": "^4.0.0",
     "strip-json-comments": "^2.0.1",
     "table": "^4.0.3",