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.

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

Feature request: [no-shadow] It would really help, if the error message would show where the original identifier has been defined #13646

Assignees
Labels
accepted archived due to age enhancement rule

Comments

@doberkofler
Copy link
Contributor

doberkofler commented Sep 1, 2020

What rule do you want to change?
no-shadow

Does this change cause the rule to produce more or fewer warnings?
The same

How will the change be implemented? (New option, new default behavior, etc.)?
Just a change in the error message

Please provide some example code that this change will affect:

In my experience, the no-shadow rule could greatly benefit from an error message that also describes where the identifier has already been defined and not only where the redefinition has occurred.

var a = 3;
function b() {
    var a = 10;
}

What does the rule currently do for this code?

The error 3:9 - 'a' is already declared in the upper scope. (no-shadow) explains that a has already be declared somewhere but not exactly where the original declaration has been made.

What will the rule do after it's changed?

An improved error like 3:9 - 'a' is already declared in the upper scope 1:5. (no-shadow) would additionally inform that the original definition is in line 1 column 5 and greatly improved the experience.

Are you willing to submit a pull request to implement this change?
Not sure if I have the required skills.

Versions

package version
ESLint 7.8.0
node 14.8.0
@doberkofler doberkofler added enhancement rule triage labels Sep 1, 2020
@anikethsaha anikethsaha added evaluating and removed triage labels Sep 1, 2020
@anikethsaha
Copy link
Member

anikethsaha commented Sep 1, 2020

Thanks for the issue

Make sense to me.

@mdjermanovic
Copy link
Member

mdjermanovic commented Sep 4, 2020

Makes sense to me, too. I'll champion this.

@mdjermanovic mdjermanovic self-assigned this Sep 4, 2020
@t-mangoe
Copy link
Contributor

t-mangoe commented Nov 14, 2020

I have worked on this issue.
I will open pull request, if you like, please check and review my codes.

t-mangoe added a commit to t-mangoe/eslint that referenced this issue Nov 21, 2020
@btmills btmills added accepted and removed evaluating labels Jan 10, 2021
@btmills
Copy link
Member

btmills commented Jan 10, 2021

This is a nice improvement 👍 marking as accepted since this has support from three team members including a champion.

t-mangoe added a commit to t-mangoe/eslint that referenced this issue Jan 16, 2021
btmills pushed a commit that referenced this issue Jan 30, 2021
…) (#13841)

* Update: show the original identifier place (refs #13646)

* Update: show message when the variable is global (refs #13646)

* Update: fix code according to review.
This was referenced Mar 17, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jul 30, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age label Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.