prefer-object-spread: lint more locations, allow first parameter with…

… side effects (#2828) [bugfix] `prefer-object-spread`: allow constructor, function and method calls as first argument to `Object.assign` [enhancement] `prefer-object-spread`: lint more locations where return value is used.
palantir · May 31, 2017 · b38d0e0 · b38d0e0
1 parent 7c2bb49
commit b38d0e0
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 8 deletions.
diff --git a/src/rules/preferObjectSpreadRule.ts b/src/rules/preferObjectSpreadRule.ts
@@ -16,8 +16,8 @@
  */
 
 import {
-    isBinaryExpression, isCallExpression, isIdentifier, isObjectLiteralExpression,
-    isPropertyAccessExpression, isSpreadElement,
+    hasSideEffects, isCallExpression, isIdentifier, isObjectLiteralExpression, isPropertyAccessExpression,
+    isSpreadElement, SideEffectOptions,
 } from "tsutils";
 import * as ts from "typescript";
 
@@ -54,18 +54,35 @@ function walk(ctx: Lint.WalkContext<void>) {
             !node.arguments.some(isSpreadElement)) {
             if (node.arguments[0].kind === ts.SyntaxKind.ObjectLiteralExpression) {
                 ctx.addFailureAtNode(node, Rule.FAILURE_STRING, createFix(node, ctx.sourceFile));
-            } else {
-                const parent = node.parent!;
-                if (parent.kind === ts.SyntaxKind.VariableDeclaration ||
-                    isBinaryExpression(parent) && parent.operatorToken.kind === ts.SyntaxKind.EqualsToken) {
-                    ctx.addFailureAtNode(node, Rule.ASSIGNMENT_FAILURE_STRING, createFix(node, ctx.sourceFile));
-                }
+            } else if (isReturnValueUsed(node) && !hasSideEffects(node.arguments[0], SideEffectOptions.Constructor)) {
+                ctx.addFailureAtNode(node, Rule.ASSIGNMENT_FAILURE_STRING, createFix(node, ctx.sourceFile));
             }
         }
         return ts.forEachChild(node, cb);
     });
 }
 
+function isReturnValueUsed(node: ts.Expression): boolean {
+    const parent = node.parent!;
+    switch (parent.kind) {
+        case ts.SyntaxKind.VariableDeclaration:
+        case ts.SyntaxKind.PropertyAssignment:
+        case ts.SyntaxKind.PropertyDeclaration:
+        case ts.SyntaxKind.ReturnStatement:
+        case ts.SyntaxKind.BindingElement:
+        case ts.SyntaxKind.ArrayLiteralExpression:
+            return true;
+        case ts.SyntaxKind.BinaryExpression:
+            return (parent as ts.BinaryExpression).operatorToken.kind === ts.SyntaxKind.EqualsToken;
+        case ts.SyntaxKind.NewExpression:
+        case ts.SyntaxKind.CallExpression:
+            return (parent as ts.NewExpression | ts.CallExpression).arguments !== undefined &&
+                (parent as ts.NewExpression | ts.CallExpression).arguments!.indexOf(node) !== -1;
+        default:
+            return false;
+    }
+}
+
 function createFix(node: ts.CallExpression, sourceFile: ts.SourceFile): Lint.Fix {
     const args = node.arguments;
     const fix = [

diff --git a/test/rules/prefer-object-spread/test.ts.fix b/test/rules/prefer-object-spread/test.ts.fix
@@ -6,9 +6,18 @@ Object.assign(original, {c: 3});
 var result = {...original, c: 3};
 result = {...original, c: 3};
 var result = {...original, c: 3};
+foo({...original, c: 3});
+[{...original, c: 3}];
+({
+    foo: {...original, c: 3}
+})
 
 {a: 1,};
 {a: 1, ...(foo ? {b: 2} : {c: 3})};
 {a: 1, ...{b: 2}};
 {};
 {};
+
+// allowed
+result = Object.assign(new Foo(), {});
+result = Object.assign(createFoo(), {});
diff --git a/test/rules/prefer-object-spread/test.ts.lint b/test/rules/prefer-object-spread/test.ts.lint
@@ -11,6 +11,14 @@ result = Object.assign(original, {c: 3});
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
 var result = Object.assign({}, original, {c: 3});
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+foo(Object.assign(original, {c: 3}));
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
+[Object.assign(original, {c: 3})];
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
+({
+    foo: Object.assign(original, {c: 3})
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [1]
+})
 
 Object.assign({}, {}, {a: 1,},);
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
@@ -23,5 +31,9 @@ Object.assign({}, {});
 Object.assign({}, {},);
 ~~~~~~~~~~~~~~~~~~~~~~ [0]
 
+// allowed
+result = Object.assign(new Foo(), {});
+result = Object.assign(createFoo(), {});
+
 [0]: Use the object spread operator instead.
 [1]: 'Object.assign' returns the first argument. Prefer object spread if you want a new object.