From 095025ec17f7bdda9f86d5145ab0584eb74b6b58 Mon Sep 17 00:00:00 2001 From: Benjamin Quorning Date: Sat, 29 Mar 2025 11:18:10 +0100 Subject: [PATCH 1/2] Use map lookup in PredicateMatcher This prepares for adding more mathcer options. --- lib/rubocop/cop/rspec/predicate_matcher.rb | 54 +++++++++++++--------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/rubocop/cop/rspec/predicate_matcher.rb b/lib/rubocop/cop/rspec/predicate_matcher.rb index fd7830ec4..cead1c710 100644 --- a/lib/rubocop/cop/rspec/predicate_matcher.rb +++ b/lib/rubocop/cop/rspec/predicate_matcher.rb @@ -69,18 +69,21 @@ def message_inflected(predicate) matcher_name: to_predicate_matcher(predicate.method_name)) end + TO_PREDICATE_MATCHER_MAP = { + exist?: 'exist', + exists?: 'exist', + include?: 'include', + instance_of?: 'be_an_instance_of', + is_a?: 'be_a', + respond_to?: 'respond_to' + }.freeze + private_constant :TO_PREDICATE_MATCHER_MAP + def to_predicate_matcher(name) - case name = name.to_s - when 'is_a?' - 'be_a' - when 'instance_of?' - 'be_an_instance_of' - when 'include?', 'respond_to?' - name[0..-2] - when 'exist?', 'exists?' - 'exist' - when /\Ahas_/ - name.sub('has_', 'have_')[0..-2] + if TO_PREDICATE_MATCHER_MAP.key?(name) + TO_PREDICATE_MATCHER_MAP.fetch(name) + elsif name.start_with?('has_') + name.to_s.sub('has_', 'have_')[0..-2] else "be_#{name[0..-2]}" end @@ -241,18 +244,25 @@ def move_predicate(corrector, actual, matcher, block_child) corrector.insert_after(actual, ".#{predicate}" + args + block) end + TO_PREDICATE_METHOD_MAP = { + a_kind_of: 'is_a?', + an_instance_of: 'instance_of?', + be_a: 'is_a?', + be_a_kind_of: 'is_a?', + be_an: 'is_a?', + be_an_instance_of: 'instance_of?', + be_instance_of: 'instance_of?', + be_kind_of: 'is_a?', + include: 'include?', + respond_to: 'respond_to?' + }.freeze + private_constant :TO_PREDICATE_METHOD_MAP + def to_predicate_method(matcher) - case matcher = matcher.to_s - when 'be_a', 'be_an', 'be_a_kind_of', 'a_kind_of', 'be_kind_of' - 'is_a?' - when 'be_an_instance_of', 'be_instance_of', 'an_instance_of' - 'instance_of?' - when 'include' - 'include?' - when 'respond_to' - 'respond_to?' - when /\Ahave_(.+)/ - "has_#{Regexp.last_match(1)}?" + if TO_PREDICATE_METHOD_MAP.key?(matcher) + TO_PREDICATE_METHOD_MAP.fetch(matcher) + elsif (subject = matcher[/\Ahave_(.+)/, 1]) + "has_#{subject}?" else "#{matcher[/\Abe_(.+)/, 1]}?" end From 411875d7b2bf67b79679257d98a90bc323e08656 Mon Sep 17 00:00:00 2001 From: Benjamin Quorning Date: Sat, 29 Mar 2025 11:35:55 +0100 Subject: [PATCH 2/2] Improve PredicateMatcher recommendations PredicateMatcher should not recommend changing `#key?` to `be_key`, but will now instead suggest using `have_key`. --- CHANGELOG.md | 1 + lib/rubocop/cop/rspec/predicate_matcher.rb | 1 + spec/rubocop/cop/rspec/predicate_matcher_spec.rb | 3 +++ 3 files changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b878922fd..cf22b6cdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Fix false positive in `RSpec/Pending`, where it would mark the default block `it` as an offense. ([@bquorning]) - Fix issue when `Style/ContextWording` is configured with a Prefix being interpreted as a boolean, like `on`. ([@sakuro]) - Add new `RSpec/IncludeExamples` cop to enforce using `it_behaves_like` over `include_examples`. ([@dvandersluis]) +- Fix `RSpec/PredicateMatcher` sometimes suggesting the `be_key` matcher instead of `have_key`. ([@bquorning]) ## 3.5.0 (2025-02-16) diff --git a/lib/rubocop/cop/rspec/predicate_matcher.rb b/lib/rubocop/cop/rspec/predicate_matcher.rb index cead1c710..7f8f0b187 100644 --- a/lib/rubocop/cop/rspec/predicate_matcher.rb +++ b/lib/rubocop/cop/rspec/predicate_matcher.rb @@ -75,6 +75,7 @@ def message_inflected(predicate) include?: 'include', instance_of?: 'be_an_instance_of', is_a?: 'be_a', + key?: 'have_key', respond_to?: 'respond_to' }.freeze private_constant :TO_PREDICATE_MATCHER_MAP diff --git a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb index 18ad4013a..58a2f8031 100644 --- a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb @@ -91,6 +91,8 @@ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_something` matcher over `something?`. expect(foo.has_key?('foo')).to be_truthy ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `have_key` matcher over `has_key?`. + expect(foo.key?('foo')).to be_truthy + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `have_key` matcher over `key?`. expect(foo.is_a?(Array)).to be_truthy ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `be_a` matcher over `is_a?`. expect(foo.instance_of?(Array)).to be_truthy @@ -103,6 +105,7 @@ expect(foo).to be_something('foo', 'bar') expect(foo).to be_something 1, 2 expect(foo).to have_key('foo') + expect(foo).to have_key('foo') expect(foo).to be_a(Array) expect(foo).to be_an_instance_of(Array) RUBY