Merge pull request #960 from rubocop-hq/optimize-leading-subject

Don't spend CPU finding the same node twice

Author: bquorning

lib/rubocop/cop/rspec/leading_subject.rb

@@ -43,35 +43,34 @@ def on_block(node)
         end
 
         def check_previous_nodes(node)
-          node.parent.each_child_node do |sibling|
-            if offending?(sibling)
-              msg = format(MSG, offending: sibling.method_name)
-              add_offense(node, message: msg) do |corrector|
-                autocorrect(corrector, node)
-              end
+          offending_node(node) do |offender|
+            msg = format(MSG, offending: offender.method_name)
+            add_offense(node, message: msg) do |corrector|
+              autocorrect(corrector, node, offender)
             end
-
-            break if offending?(sibling) || sibling.equal?(node)
           end
         end
 
         private
 
-        def autocorrect(corrector, node)
-          first_node = find_first_offending_node(node)
+        def offending_node(node)
+          node.parent.each_child_node.find do |sibling|
+            break if sibling.equal?(node)
+
+            yield sibling if offending?(sibling)
+          end
+        end
+
+        def autocorrect(corrector, node, sibling)
           RuboCop::RSpec::Corrector::MoveNode.new(
             node, corrector, processed_source
-          ).move_before(first_node)
+          ).move_before(sibling)
         end
 
         def offending?(node)
           let?(node) || hook?(node) || example?(node)
         end
 
-        def find_first_offending_node(node)
-          node.parent.children.find { |sibling| offending?(sibling) }
-        end
-
         def in_spec_block?(node)
           node.each_ancestor(:block).any? do |ancestor|
             example?(ancestor)

spec/rubocop/cop/rspec/leading_subject_spec.rb

@@ -135,4 +135,29 @@
       end
     RUBY
   end
+
+  it 'checks also when subject is below a non-offending node' do
+    expect_offense(<<~RUBY)
+      RSpec.describe do
+        def helper_method
+        end
+
+        it { is_expected.to be_present }
+
+        subject { described_class.new }
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Declare `subject` above any other `it` declarations.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      RSpec.describe do
+        def helper_method
+        end
+
+        subject { described_class.new }
+        it { is_expected.to be_present }
+
+      end
+    RUBY
+  end
 end