no-shadow: allow a declaration's body to reuse the declared variable's name

[philipahlberg]

What rule do you want to change?
no-shadow

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

How will the change be implemented? (New option, new default behavior, etc.)?
New default behavior

Please provide some example code that this change will affect:

const person = people.find(person => person.name === "John");

What does the rule currently do for this code?
It currently produces a warning, claiming that the right-hand side person shadows the left-hand side person.

What will the rule do after it's changed?
It will not produce warnings when the purportedly-shadowed variable has not been declared.

I believe the current behavior is incorrect, as the variable has not (semantically) been declared at the point where the warning is generated, and so it can not be shadowed.

Are you willing to submit a pull request to implement this change?
Yes, though I would need some pointers on how to get started.

[kaicataldo]

This makes sense to me. As far as I understand it, there's no danger here because it hasn't been declared yet and so cannot be overwritten.

The following example leads to a ReferenceError:

let user = ['philipahlberg', 'kaicataldo'].find(p => user === 'philipahlberg');

ReferenceError: can't access lexical declaration `user' before initialization

[mysticatea]

There are similar patterns:

// variable declarations
const a = [].find(a => true)

// assignment patterns in variable declarations or parameters
const [b = [].find(b => true)] = dummy
const { c = [].find(c => true) } = dummy
function func(d = [].find(d => true)) {}

// for-in / for-of with variable declarations
for (const e in [].find(e => true)) {}
for (const f of [].find(f => true)) {}

Online demo

Those a to f should not be warned as same.

[philipahlberg]

It seems like it's sort of checked already, but perhaps it fails because the initialized variable is inserted into the outer scope before evaluating the rule or something similar?

[mysticatea]

@philipahlberg That check is to avoid reporting function expression names:

const a = function a() {}

[philipahlberg]

Any updates on this?

[kaicataldo]

I think this could be classified as a bug since the rule currently has false negatives. @mysticatea Thoughts?

[philipahlberg]

It seems to me like you generally agree that it should be implemented/fixed. Is that correct?

In that case, I wouldn't mind looking into it myself, assuming you don't have time to do so. I'm not familiar with the ESLint codebase though, so any help with getting started would be appreciated. Perhaps you have a forum/discord/irc where I can ask, if anything comes up?

[kaicataldo]

Sorry for the radio silence on our end, and thanks for being willing to work on this! It looks like both @mysticatea and myself agree that this is a false-positive bug and I'm therefore going to mark it as accepted. We would gladly accept a PR for this!

We have some documentation around contributing that can be found here. If you have any questions, feel free to comment here (or on a work in progress pull request, if that's easier), or stop by our Gitter chat.

We're happy to help :)