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

Shadow lint error going too far #379

Closed
hueniverse opened this issue Jun 22, 2015 · 3 comments
Assignees
Labels
bug
Milestone

Comments

@hueniverse
Copy link
Member

@hueniverse hueniverse commented Jun 22, 2015

See: https://travis-ci.org/hapijs/wreck/jobs/67856616

Basically, it is fine to shadow some variables like err in callback. If you didn't handle it at the top of the method you clearly don't care. I have never seen bugs because of err argument shadows. I am going to go over the warning in hapi and see which one I find useful and fix those. I'll be back here with my findings but just heads up that I am going to want to see this rule changed at least for some cases.

@hueniverse hueniverse added the bug label Jun 22, 2015
@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Jun 23, 2015

I think you are correct. This change has improved my code, but in some cases with err it is likely overkill.

@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Jun 24, 2015

I created https://www.npmjs.com/package/no-shadow-relaxed

We can allow certain variables to be shadowed, like err, done, res... any others?

@hueniverse

This comment has been minimized.

Copy link
Member Author

@hueniverse hueniverse commented Jun 25, 2015

I think just err and done for now. res is an special one in tests so probably better to require doing res1, res2...

@geek geek self-assigned this Jun 25, 2015
@geek geek added this to the 5.11.1 milestone Jun 25, 2015
@arb arb closed this in #385 Jun 25, 2015
jagoda added a commit to jagoda/vision that referenced this issue Jul 7, 2015
Lab enabled the [no-shadow rule](http://eslint.org/docs/rules/no-shadow.html)
in hapijs/lab#371 (with exceptions provided by hapijs/lab#379).
This change corrects lint errors created by variable shadowing.
@jagoda jagoda mentioned this issue Jul 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.