Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

prefer-object-spread: lint more locations, allow first parameter with side effects #2828

Merged
merged 1 commit into from May 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 25 additions & 8 deletions src/rules/preferObjectSpreadRule.ts
Expand Up @@ -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";

Expand Down Expand Up @@ -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 = [
Expand Down
9 changes: 9 additions & 0 deletions test/rules/prefer-object-spread/test.ts.fix
Expand Up @@ -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(), {});
12 changes: 12 additions & 0 deletions test/rules/prefer-object-spread/test.ts.lint
Expand Up @@ -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]
Expand All @@ -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.