Skip to content

Commit

Permalink
Fix: prefer-object-spread false positives with accessors (fixes #12086)…
Browse files Browse the repository at this point in the history
… (#12784)
  • Loading branch information
mdjermanovic authored and btmills committed Jan 17, 2020
1 parent f9774ec commit 16a1c1f
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 2 deletions.
36 changes: 35 additions & 1 deletion lib/rules/prefer-object-spread.js
Expand Up @@ -25,6 +25,36 @@ function hasArraySpread(node) {
return node.arguments.some(arg => arg.type === "SpreadElement");
}

/**
* Determines whether the given node is an accessor property (getter/setter).
* @param {ASTNode} node Node to check.
* @returns {boolean} `true` if the node is a getter or a setter.
*/
function isAccessorProperty(node) {
return node.type === "Property" &&
(node.kind === "get" || node.kind === "set");
}

/**
* Determines whether the given object expression node has accessor properties (getters/setters).
* @param {ASTNode} node `ObjectExpression` node to check.
* @returns {boolean} `true` if the node has at least one getter/setter.
*/
function hasAccessors(node) {
return node.properties.some(isAccessorProperty);
}

/**
* Determines whether the given call expression node has object expression arguments with accessor properties (getters/setters).
* @param {ASTNode} node `CallExpression` node to check.
* @returns {boolean} `true` if the node has at least one argument that is an object expression with at least one getter/setter.
*/
function hasArgumentsWithAccessors(node) {
return node.arguments
.filter(arg => arg.type === "ObjectExpression")
.some(hasAccessors);
}

/**
* 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.
Expand Down Expand Up @@ -249,7 +279,11 @@ module.exports = {
if (
node.arguments.length >= 1 &&
node.arguments[0].type === "ObjectExpression" &&
!hasArraySpread(node)
!hasArraySpread(node) &&
!(
node.arguments.length > 1 &&
hasArgumentsWithAccessors(node)
)
) {
const messageId = node.arguments.length === 1
? "useLiteralMessage"
Expand Down
28 changes: 27 additions & 1 deletion tests/lib/rules/prefer-object-spread.js
Expand Up @@ -59,7 +59,19 @@ ruleTester.run("prefer-object-spread", rule, {
`
import { Object, Array } from 'globals';
Object.assign({ foo: 'bar' });
`
`,

// ignore Object.assign() with > 1 arguments if any of the arguments is an object expression with a getter/setter
"Object.assign({ get a() {} }, {})",
"Object.assign({ set a(val) {} }, {})",
"Object.assign({ get a() {} }, foo)",
"Object.assign({ set a(val) {} }, foo)",
"Object.assign({ foo: 'bar', get a() {}, baz: 'quux' }, quuux)",
"Object.assign({ foo: 'bar', set a(val) {} }, { baz: 'quux' })",
"Object.assign({}, { get a() {} })",
"Object.assign({}, { set a(val) {} })",
"Object.assign({}, { foo: 'bar', get a() {} }, {})",
"Object.assign({ foo }, bar, {}, { baz: 'quux', set a(val) {}, quuux }, {})"
],

invalid: [
Expand Down Expand Up @@ -834,6 +846,20 @@ ruleTester.run("prefer-object-spread", rule, {
column: 1
}
]
},

// report Object.assign() with getters/setters if the function call has only 1 argument
{
code: "Object.assign({ get a() {}, set b(val) {} })",
output: "({get a() {}, set b(val) {}})",
errors: [
{
messageId: "useLiteralMessage",
type: "CallExpression",
line: 1,
column: 1
}
]
}
]
});

0 comments on commit 16a1c1f

Please sign in to comment.