From c8b59d6fdbfe6bdcc6ecb92e465187f6a4a76894 Mon Sep 17 00:00:00 2001 From: ydah Date: Fri, 1 Mar 2024 19:53:50 +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 | 21 ++++++++++--------- .../cop/rspec/repeated_subject_call_spec.rb | 11 ++++++++++ 4 files changed, 30 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..4a5f18f7d 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,16 +70,11 @@ def on_top_level_group(node) private - def detect_offense(example_node, subject_node) - walker = subject_node + def detect_offense(subject_node) + subject_node.each_ancestor(:block) do |block| + break if subject_node.chained? - while walker.parent? && walker.parent != example_node.body - walker = walker.parent - - if walker.block_type? && walker.method?(:expect) - add_offense(walker) - return - end + add_offense(block) if block.method?(:expect) end end @@ -96,7 +97,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