eslint: prefer-object-spread invalid autofix with accessors

Tell us about your environment

  • ESLint Version: 6.1.0
  • Node Version: 10.16.0
  • npm Version: 6.9.0

What parser (default, Babel-ESLint, etc.) are you using?

default

Please show your full configuration:

Configuration
module.exports = {
  parserOptions: {
    ecmaVersion: 2018,
  },
};

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

Example #1

/*eslint prefer-object-spread: "error"*/

const foo = { a:1 };

const bar = Object.assign(
  { 
    get a() { return this._a }, 
    set a(v) { this._a = v + 1 } 
  }, 
  foo
)

console.log(bar.a) // 2

Example #2

/*eslint prefer-object-spread: "error"*/

const foo = Object.assign(
  {}, 
  { 
    get a() { return this._a }, 
    set a(v) { this._a = v + 1 } 
  }
);

foo.a = 1;
console.log(foo.a) // 1
eslint index.js

What did you expect to happen?

Not to change behavior.

What actually happened? Please include the actual, raw output from ESLint.

Example #1

/*eslint prefer-object-spread: "error"*/

const foo = { a:1 };

const bar = {
  get a() { return this._a }, 
  set a(v) { this._a = v + 1 }, 
  ...foo
}

console.log(bar.a) // 1

Example #2

/*eslint prefer-object-spread: "error"*/

const foo = {
  get a() { return this._a }, 
  set a(v) { this._a = v + 1 }
};

foo.a = 1;
console.log(foo.a) // 2

Are you willing to submit a pull request to fix this bug?

Yes, just to define what should be done (no warning, warning without the fix or something else).

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 16 (14 by maintainers)

Commits related to this issue

Most upvoted comments

After thinking about this again, I believe that prefer-object-spread shouldn’t even report in any of these cases.

Even without the autofix, the rule would suggest using object literal/object spread instead, which is wrong as it isn’t equivalent. Maybe there could be another rule to warn about using getters/setters with Object.assign (to disallow that style if it’s intentional or find errors if it isn’t).

Yes, fixing x = Object.assign({}, { get a(){} }) to x = { get a(){} } is certainly wrong.

I’ll make a PR to ignore Object.assign if there are any getters/setters inside (as described in this comment), that seems like the most appropriate solution for this edge case.

Semi-related, __proto__ or super in source objects would cause a similar bug, but it’s better to open a separate issue for these.

🤦‍♂ Reopened now.

Assigning to myself so it doesn’t get autoclosed again, but please feel free to assign yourself @mdjermanovic.

I think this should be reopened?