-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
New Rule: no-lone-blocks (fixes #512) #572
Conversation
function checkForLoners(node) { | ||
// Check for any occurrence of BlockStatement > BlockStatement or | ||
// Program > BlockStatement | ||
node.body.forEach(function (stmt) { |
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.
Instead of triggering on the parent and traversing the children (we try to avoid any additional traversals, even ones this small), trigger on the child (which will always be BlockStatement
) and check the parent.
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.
To add to @michaelficarra, ESLint already does a traversal down, so doing another traversal down inside of a rule is wasteful. You can listen for "BlockStatement", then use context.getAncenstors()
to check the parent.
Aside from the note above, the rest looks really awesome. If you can clean that piece up then I'll be happy to merge it in. |
That makes perfect sense. Updated and squashed. |
@@ -74,6 +74,7 @@ | |||
"no-extend-native": 2, | |||
"no-yoda": 2, | |||
"no-path-concat": 0, | |||
"no-lone-blocks": 1, |
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.
Oops, missed this before. Should be "2" (for an error).
Removed extra down-traversal in favor of looking up one level at ancestors and changed config from warning to error to incorporate PR feedback.
Fixed, now it's an error. |
New Rule: no-lone-blocks (fixes #512)
Implements @michaelficarra's proposal to disallow
BlockStatement > BlockStatement
orProgram > BlockStatement
forms as described in #512.