From 0149e4cacea100db7a0ae752b06f06fab3ca89ef Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Sat, 14 Jan 2023 10:37:59 +0900 Subject: [PATCH] Fix a false positive for `RSpec/PredicateMatcher` when multiple arguments and not replaceable method This PR is fix a false positive for `RSpec/PredicateMatcher` when multiple arguments and not replaceable method. following code: ```ruby expect(foo).to include(foo, bar) ``` `RSpec/PredicateMatcher` autocorrects it as follows: ```ruby expect(foo.include?(foo, bar)).to be(true) ``` However, this is a SyntaxError. ``` ArgumentError: wrong number of arguments (given 2, expected 1) ``` --- CHANGELOG.md | 1 + lib/rubocop/cop/rspec/predicate_matcher.rb | 27 ++++++++++++++- .../cop/rspec/predicate_matcher_spec.rb | 33 ++++++++++++++++++- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e3d3ca35..74fdab20a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix a false negative for `RSpec/Pending` when using skipped in metadata is multiline string. ([@ydah]) - Fix a false positive for `RSpec/NoExpectationExample` when using skipped in metadata is multiline string. ([@ydah]) +- Fix a false positive for `RSpec/PredicateMatcher` when multiple arguments and not replaceable method. ([@ydah]) ## 2.17.0 (2023-01-13) diff --git a/lib/rubocop/cop/rspec/predicate_matcher.rb b/lib/rubocop/cop/rspec/predicate_matcher.rb index 62fa497b3..b48bcac78 100644 --- a/lib/rubocop/cop/rspec/predicate_matcher.rb +++ b/lib/rubocop/cop/rspec/predicate_matcher.rb @@ -15,6 +15,8 @@ module InflectedHelper def check_inflected(node) predicate_in_actual?(node) do |predicate, to, matcher| + next unless replaceable_predicate?(predicate) + msg = message_inflected(predicate) add_offense(node, message: msg) do |corrector| remove_predicate(corrector, predicate) @@ -25,6 +27,16 @@ def check_inflected(node) end end + def replaceable_predicate?(predicate) + case predicate.method_name.to_s + when 'is_a?', 'instance_of?', 'include?', + 'respond_to?', 'exist?', 'exists?', /\Ahas_/ + predicate.arguments.count <= 1 + else + true + end + end + # @!method predicate_in_actual?(node) def_node_matcher :predicate_in_actual?, <<-PATTERN (send @@ -118,7 +130,7 @@ def true?(to_symbol, matcher) end # A helper for `explicit` style - module ExplicitHelper + module ExplicitHelper # rubocop:disable Metrics/ModuleLength include RuboCop::RSpec::Language extend NodePattern::Macros @@ -149,12 +161,25 @@ def check_explicit(node) # rubocop:disable Metrics/MethodLength return if part_of_ignored_node?(node) predicate_matcher?(node) do |actual, matcher| + next unless replaceable_matcher?(matcher) + add_offense(node, message: message_explicit(matcher)) do |corrector| corrector_explicit(corrector, node, actual, matcher, matcher) end end end + def replaceable_matcher?(matcher) + case matcher.method_name.to_s + when 'be_a', 'be_an', 'be_a_kind_of', 'a_kind_of', 'be_kind_of', + 'be_an_instance_of', 'be_instance_of', 'an_instance_of', + 'include', 'respond_to', /\Ahave_/ + matcher.arguments.count <= 1 + else + true + end + end + # @!method predicate_matcher?(node) def_node_matcher :predicate_matcher?, <<-PATTERN (send diff --git a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb index ac80e32fc..3ad1982fb 100644 --- a/spec/rubocop/cop/rspec/predicate_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/predicate_matcher_spec.rb @@ -93,6 +93,20 @@ RUBY end + it 'does not register an offense for a predicate method with ' \ + 'multiple arguments and not replaceable method' do + expect_no_offenses(<<~RUBY) + expect(foo.is_a?(foo, bar)).to be_truthy + expect(foo.instance_of? foo, bar).to be_truthy + expect(foo.include?(foo, bar, baz)).to be_truthy + expect(foo.has_key? foo, bar, baz).to be_truthy + expect(foo.respond_to?(foo, bar)).to be_truthy + expect(foo.exist? foo, bar).to be_truthy + expect(foo.exists?(foo, bar, baz)).to be_truthy + expect(foo.has_key? foo, bar, baz).to be_truthy + RUBY + end + it 'registers an offense for a predicate method with a block' do expect_offense(<<~RUBY) expect(foo.all?(&:present?)).to be_truthy @@ -295,7 +309,7 @@ RUBY end - it 'registers an offense for a predicate method with argument' do + it 'registers an offense for a `be_something` with argument' do expect_offense(<<~RUBY) expect(foo).to be_something(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher. @@ -312,6 +326,23 @@ RUBY end + it 'does not register an offense for a predicate method ' \ + 'with multiple arguments and not replaceable method' do + expect_no_offenses(<<~RUBY) + expect(foo).to be_a(foo, bar) + expect(foo).to be_an foo, bar + expect(foo).to be_a_kind_of(foo, bar, baz) + expect(foo).to a_kind_of foo, bar, baz + expect(foo).to be_kind_of(foo, bar) + expect(foo).to be_an_instance_of foo, bar + expect(foo).to be_instance_of(foo, bar, baz) + expect(foo).to an_instance_of foo, bar, baz + expect(foo).to include(foo, bar) + expect(foo).to respond_to foo, bar + expect(foo).to have_key(foo, bar, baz) + RUBY + end + it 'registers an offense for a predicate method with a block' do expect_offense(<<~RUBY) expect(foo).to be_all { |x| x.present? }