Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ReferenceTracker returns potential references #12826

Open
mdjermanovic opened this issue Jan 25, 2020 · 6 comments · May be fixed by #18148
Open

ReferenceTracker returns potential references #12826

mdjermanovic opened this issue Jan 25, 2020 · 6 comments · May be fixed by #18148
Assignees
@mdjermanovic
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly needs design Important details about this change need to be discussed rule Relates to ESLint's core rules

Comments

@mdjermanovic
Copy link
Member

Tell us about your environment

  • ESLint Version: 7.0.0-alpha.0
  • Node Version: v12.14.0
  • npm Version: v6.13.4

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

default

Please show your full configuration:

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

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

Online Demo Link

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

var doSomething;

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

var bar = doSomething({}, a, b);
eslint index.js --fix

What did you expect to happen?

I guess this shouldn't be reported and auto-fixed.

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

Auto-fixed to:

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

var doSomething;

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

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

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

Yes, though this would need some design first.

The problem is: eslint-utils.ReferenceTracker returns references that may be the targeted global in some code paths, but may be something else in other code paths. This feature might be not suitable for some rules.
@mdjermanovic mdjermanovic added bug ESLint is working incorrectly rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jan 25, 2020
@kaicataldo kaicataldo added 3rd party plugin This is an issue related to a 3rd party plugin, config, or parser needs design Important details about this change need to be discussed accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion 3rd party plugin This is an issue related to a 3rd party plugin, config, or parser labels Jan 27, 2020
@kaicataldo
Copy link
Member

I have verified this bug. Agreed this should be fixed!

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Apr 26, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@kaicataldo kaicataldo removed the auto closed The bot closed this issue label Apr 27, 2020
@kaicataldo kaicataldo reopened this Apr 27, 2020
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label May 29, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@kaicataldo kaicataldo removed the auto closed The bot closed this issue label May 29, 2020
@kaicataldo kaicataldo reopened this May 29, 2020
@kaicataldo
Copy link
Member

Reopening this because this is a bug that results in the autofixer changing the execution of code.

@aggmoulik
Copy link

Hy @kaicataldo I am looking into this issue. Can we explain a bit more how can I proceed ?

@mdjermanovic
Copy link
Member Author

We could add an option in ReferenceTracker to return only certain references, or maybe change the rule to not use ReferenceTracker and manually find a smaller set of references.

@mysticatea what do you think about this?

@Tanujkanti4441 Tanujkanti4441 linked a pull request Feb 25, 2024 that will close this issue
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdjermanovic mdjermanovic

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly needs design Important details about this change need to be discussed rule Relates to ESLint's core rules
Projects
Triage
Status: Implementing
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: references of Object.assign in prefer-object-spread Tanujkanti4441/eslint
3 participants
@kaicataldo @aggmoulik @mdjermanovic