-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Update: no-inner-declarations false negative in non-block (fixes #12222) #13062
Conversation
These should be also allowed: /*eslint no-inner-declarations: ["error", "both"]*/
export var foo;
export function bar() {}
export default function baz() {} |
I changed the validation condition, now its allowing those named and default exports |
Thoughts about simplifying the code? A declaration is okay if its parent is either of:
so maybe we could just check these simple conditions, instead of calculating the distance and then analyzing the result. |
@mdjermanovic I simplified the code as per your suggestion and it is working as intended ! that while loop was needed as we need to find the correct immediate ancestor where the declaration should be moved for the error message. let me know if it needs some more changes 👍 |
You're right! I missed the fact that the ancestor is used for the error message as well. |
@mdjermanovic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, I left just some minor suggestions.
We should also update the documentation with some examples.
@mdjermanovic I updated the docs with some examples |
@mdjermanovic done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! The change is implemented as intended.
Leaving the "needs bikeshedding" label since it isn't definitely confirmed that this should be the behavior of this rule.
Removing the "needs bikeshedding" label as per this comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
fixes #12222
What changes did you make? (Give an overview)
Currently whenever the distance is more than 1, it was not showing error cause it was considering ancestor as any (
Program
included).it will be valid only if ancestor is not
Program
for distance equal to 2example
this will result in distance 1 for the ancestor
Program
This will be distance 2 for the ancestor
Program
.here both calculations for distance and ancestor are correct but not valid. (the
master
is considering it as valid)So in this PR, whenever distance is 2, it should not check if from
Program
.Is there anything you'd like reviewers to focus on?
#12222