From e1b1076b28b9fdcac56fcdfa20644990cad39ecd Mon Sep 17 00:00:00 2001 From: ydah Date: Fri, 1 Mar 2024 23:13:52 +0900 Subject: [PATCH] Fix a false positive for `RSpec/RepeatedSubjectCall` Fix: #1821 --- CHANGELOG.md | 2 ++ docs/modules/ROOT/pages/cops_rspec.adoc | 6 +++++ .../cop/rspec/repeated_subject_call.rb | 24 +++++++++++-------- .../cop/rspec/repeated_subject_call_spec.rb | 11 +++++++++ 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8de0a471..6cda77667 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +- Fix a false positive for `RSpec/RepeatedSubjectCall` when `subject.method_call`. ([@ydah]) + ## 2.27.0 (2024-03-01) - Add new `RSpec/IsExpectedSpecify` cop. ([@ydah]) diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index a818c0d94..c2fb78a3e 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -4921,6 +4921,12 @@ it do expect { my_method }.to change { A.count } expect { my_method }.to not_change { A.count } end + +# also good +it do + expect { subject.a }.to change { A.count } + expect { subject.b }.to not_change { A.count } +end ---- === References diff --git a/lib/rubocop/cop/rspec/repeated_subject_call.rb b/lib/rubocop/cop/rspec/repeated_subject_call.rb index 8d2267e0e..faa430718 100644 --- a/lib/rubocop/cop/rspec/repeated_subject_call.rb +++ b/lib/rubocop/cop/rspec/repeated_subject_call.rb @@ -23,6 +23,12 @@ module RSpec # expect { my_method }.to not_change { A.count } # end # + # # also good + # it do + # expect { subject.a }.to change { A.count } + # expect { subject.b }.to not_change { A.count } + # end + # class RepeatedSubjectCall < Base include TopLevelGroup @@ -64,17 +70,15 @@ def on_top_level_group(node) private - def detect_offense(example_node, subject_node) - walker = subject_node + def detect_offense(subject_node) + return if subject_node.chained? + return unless (block_node = expect_block(subject_node)) - while walker.parent? && walker.parent != example_node.body - walker = walker.parent + add_offense(block_node) + end - if walker.block_type? && walker.method?(:expect) - add_offense(walker) - return - end - end + def expect_block(node) + node.each_ancestor(:block).find { |block| block.method?(:expect) } end def detect_offenses_in_block(node, subject_names = []) @@ -96,7 +100,7 @@ def detect_offenses_in_example(node, subject_names) subject_calls(node.body, Set[*subject_names, :subject]).each do |call| if subjects_used[call.method_name] - detect_offense(node, call) + detect_offense(call) else subjects_used[call.method_name] = true end diff --git a/spec/rubocop/cop/rspec/repeated_subject_call_spec.rb b/spec/rubocop/cop/rspec/repeated_subject_call_spec.rb index 883d955cf..514c9893c 100644 --- a/spec/rubocop/cop/rspec/repeated_subject_call_spec.rb +++ b/spec/rubocop/cop/rspec/repeated_subject_call_spec.rb @@ -102,4 +102,15 @@ end RUBY end + + it 'registers no offenses for `subject.method_call`' do + expect_no_offenses(<<~RUBY) + RSpec.describe Foo do + it do + expect { subject.a }.to change { A.count } + expect { subject.b }.to not_change { A.count } + end + end + RUBY + end end