fix: references of Object.assign in prefer-object-spread

[Tanujkanti4441]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What rule do you want to change?
prefer-object-spread

What change do you want to make (place an "X" next to just one item)?

[ ] Generate more warnings
[x] Generate fewer warnings
[ ] Implement autofix
[ ] Implement suggestions

How will the change be implemented (place an "X" next to just one item)?

[ ] A new option
[ ] A new default behavior
[x] Other

Please provide some example code that this change will affect:

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

var doSomething;

if (foo) {
    doSomething = Object.assign;
} else {
    doSomething = somethingElse;
}

var bar = doSomething({}, a, b); 

What does the rule currently do for this code?
Rule reports and fixes the doSomething({}, a, b) as it's a reference of Object.assign

What will the rule do after it's changed?
Rule will not report doSomething({}, a, b) and only report the Object.assign({}, a) and GlobalThis.Object.assign({}, a)

Is there anything you'd like reviewers to focus on?

Fixes #12826

[netlify]

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	769f997
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/65db030e16669a0008749bd5
😎 Deploy Preview	https://deploy-preview-18148--docs-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[snitin315]

I feel this should be reported because foo has never been reassigned.

Incorrect

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

let doSomething;

doSomething = Object.assign;

var bar = doSomething({}, a, b); // Error

Correct

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

let doSomething;

if (foo) {

doSomething = Object.assign;
} else {
doSomething = somethingElse
}
var bar = doSomething({}, a, b); // OK

[Tanujkanti4441]

seems like i have understood the issue wrong but what about autofixes is it also correct

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

let doSomething;

doSomething = Object.assign;

var bar = { ...a, ...b}; 

[snitin315]

Yes, I believe the auto fix is also correct, doSomething will be reported by the no-unused-vars rule.

@mdjermanovic Can you once confirm the expected behavior?

[mdjermanovic]

The problem is that ReferenceTracker returns both certain references (e.g., const x = Object.assign; x();) and potential references (those that are references only in some code paths, e.g., const x = foo ? Object.assign : bar; x();), which is okay for some rules (e.g., for no-obj-calls, because it's a bug if JSON() is called in any code paths) but not for this one.

We currently don't have a utility that would help find only certain references. The solution could be:

1. Add an option to ReferenceTracker to not return potential references.
2. Make another utility.
3. Limit this rule to check only explicit Object.assign calls. Maybe also <global object>.Object.assign (like no-eval).

[Tanujkanti4441]

So, changes in this PR falls in 3rd point so what should we do now, move forward or look for another approaches (point 1st and 2nd)?
the reason i chose 3rd one is i noticed in this rule's previous version is that references was used so that it allows the Object.assign if Object is importing from a module, and i didn't find any tests which doesn't call Object.assign directly.

[Tanujkanti4441]

I am still doubtful that this fix is correct and doesn't lead to bugs.

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

let doSomething;

doSomething = Object.assign;

var bar = { ...a, ...b}; 

[mdjermanovic]

Perhaps we could first ask at https://github.com/eslint-community/eslint-utils if option 1 would be possible. If not, then I think option 3 is fine.

[github-actions]

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.