Merge pull request #2191 from rrosenblum/move_next_offense

Move next offense

CHANGELOG.md

@@ -17,6 +17,7 @@
 * [#2146](https://github.com/bbatsov/rubocop/pull/2146): Add STDIN support. ([@caseywebdev][])
 * [#2175](https://github.com/bbatsov/rubocop/pull/2175): Files that are excluded from a cop (e.g. using the `Exclude:` config option) are no longer being processed by that cop. ([@bquorning][])
 * `Rails/ActionFilter` now handles complete list of methods found in the Rails 4.2 [release notes](https://github.com/rails/rails/blob/4115a12da1409c753c747fd4bab6e612c0c6e51a/guides/source/4_2_release_notes.md#notable-changes-1). ([@MGerrior][])
+* [*2138](https://github.com/bbatsov/rubocop/issues/2138): Change the offense in `Style/Next` to highlight the condition instead of the iteration. ([@rrosenblum][])
 
 ### Bug Fixes
 

lib/rubocop/cop/style/next.rb

@@ -23,54 +23,59 @@ class Next < Cop
         include ConfigurableEnforcedStyle
         include MinBodyLength
 
-        MSG = 'Use `next` to skip iteration.'
+        MSG = 'Use `next` to skip iteration.'.freeze
+        EXIT_TYPES = [:break, :return].freeze
+        EACH_ = 'each_'.freeze
         ENUMERATORS = [:collect, :detect, :downto, :each, :find, :find_all,
                        :inject, :loop, :map!, :map, :reduce, :reverse_each,
-                       :select, :times, :upto]
+                       :select, :times, :upto].freeze
 
         def on_block(node)
           block_owner, _, body = *node
-          return unless block_owner.type == :send
-          return if body.nil?
+          return unless block_owner.send_type?
+          return unless body && ends_with_condition?(body)
 
           _, method_name = *block_owner
           return unless enumerator?(method_name)
-          return unless ends_with_condition?(body)
 
-          add_offense(block_owner, :selector, MSG)
+          offense_node = offense_node(body)
+          add_offense(offense_node, offense_location(offense_node), MSG)
         end
 
         def on_while(node)
           _, body = *node
           return unless body && ends_with_condition?(body)
 
-          add_offense(node, :keyword, MSG)
+          offense_node = offense_node(body)
+          add_offense(offense_node, offense_location(offense_node), MSG)
         end
         alias_method :on_until, :on_while
 
         def on_for(node)
           _, _, body = *node
           return unless body && ends_with_condition?(body)
 
-          add_offense(node, :keyword, MSG)
+          offense_node = offense_node(body)
+          add_offense(offense_node, offense_location(offense_node), MSG)
         end
 
         private
 
         def enumerator?(method_name)
-          ENUMERATORS.include?(method_name) || /\Aeach_/.match(method_name)
+          ENUMERATORS.include?(method_name) ||
+            method_name.to_s.start_with?(EACH_)
         end
 
         def ends_with_condition?(body)
           return true if simple_if_without_break?(body)
 
-          body.type == :begin && simple_if_without_break?(body.children.last)
+          body.begin_type? && simple_if_without_break?(body.children.last)
         end
 
         def simple_if_without_break?(node)
+          return false unless node.if_type?
           return false if ternary_op?(node)
           return false if if_else?(node)
-          return false unless node.type == :if
           return false if style == :skip_modifier_ifs && modifier_if?(node)
           return false if !modifier_if?(node) && !min_body_length?(node)
 
@@ -79,7 +84,18 @@ def simple_if_without_break?(node)
           _conditional, if_body, _else_body = *node
           return true unless if_body
 
-          ![:break, :return].include?(if_body.type)
+          !EXIT_TYPES.include?(if_body.type)
+        end
+
+        def offense_node(body)
+          *_, condition = *body
+          (condition && condition.if_type?) ? condition : body
+        end
+
+        def offense_location(offense_node)
+          condition_expression, = *offense_node
+          offense_begin_pos = offense_node.loc.expression.begin
+          offense_begin_pos.join(condition_expression.loc.expression)
         end
       end
     end