Skip to content
This repository has been archived by the owner on Mar 16, 2023. It is now read-only.

handle correctly nested loops (fixes #178) #182

Closed
wants to merge 1 commit into from

Conversation

jreidinger
Copy link
Contributor

No description provided.

@@ -57,7 +58,7 @@ class LoopKeywords < Base
# @param [RubyLint::AST::Node] node
#
def verify_keyword(keyword, node)
if current_scope.type != :block and !@allow_keyword
if current_scope.type != :block && [0, nil].include?(@allow_keyword)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a nil check here? I'd rather have @allow_keyword always set to a Fixnum so this can just be and @allow_keyword == 0. Also please keep the and instead of &&.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed that, renaming the attribute to loop_nesting, in mvidner@d38dc7d (branch https://github.com/mvidner/ruby-lint/tree/fix_nested_loops).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvidner Please submit that to the same PR (or a new PR containing all the changes), I don't want to pick commits from random places.

@yorickpeterse
Copy link
Owner

Closing in favour of #185

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants