From 9460a611ab410d6bc11df9fd4eaeb218184e8c22 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 6 Sep 2022 11:53:34 +0900 Subject: [PATCH] Fix an incorrect autocorrect for `Minitest/AssertMatch` This PR fixes an incorrect autocorrect for `Minitest/AssertMatch` when `assert` with `match` and RHS is a regexp literal. A regular expression literal must be the first argument to `assert_match`. `TypeError: no implicit conversion of Regexp into String` will occur if it is passed as the second argument. ```ruby assert_match(object, /regexp/) #=> TypeError: no implicit conversion of Regexp into String ``` This issue was found on https://github.com/faker-ruby/faker/pull/2556. --- ...t_autocorrect_for_minitest_assert_match.md | 1 + lib/rubocop/cop/minitest/assert_match.rb | 2 +- lib/rubocop/cop/mixin/minitest_cop_rule.rb | 14 +++++-- .../rubocop/cop/minitest/assert_match_test.rb | 38 +++++++++++++++++++ 4 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 changelog/fix_an_incorrect_autocorrect_for_minitest_assert_match.md diff --git a/changelog/fix_an_incorrect_autocorrect_for_minitest_assert_match.md b/changelog/fix_an_incorrect_autocorrect_for_minitest_assert_match.md new file mode 100644 index 00000000..ee59675d --- /dev/null +++ b/changelog/fix_an_incorrect_autocorrect_for_minitest_assert_match.md @@ -0,0 +1 @@ +* [#181](https://github.com/rubocop/rubocop-minitest/pull/181): Fix an incorrect autocorrect for `Minitest/AssertMatch` when `assert` with `match` and RHS is a regexp literal. ([@koic][]) diff --git a/lib/rubocop/cop/minitest/assert_match.rb b/lib/rubocop/cop/minitest/assert_match.rb index 4b932366..9c917de0 100644 --- a/lib/rubocop/cop/minitest/assert_match.rb +++ b/lib/rubocop/cop/minitest/assert_match.rb @@ -18,7 +18,7 @@ module Minitest class AssertMatch < Base extend MinitestCopRule - define_rule :assert, target_method: :match + define_rule :assert, target_method: :match, inverse: 'regexp_type?' end end end diff --git a/lib/rubocop/cop/mixin/minitest_cop_rule.rb b/lib/rubocop/cop/mixin/minitest_cop_rule.rb index c1c8d2c8..15b61a60 100644 --- a/lib/rubocop/cop/mixin/minitest_cop_rule.rb +++ b/lib/rubocop/cop/mixin/minitest_cop_rule.rb @@ -19,7 +19,8 @@ module MinitestCopRule # autocorrection. The preferred method name that connects # `assertion_method` and `target_method` with `_` is # the default name. - # @param inverse [Boolean] An optional param. Order of arguments replaced by autocorrection. + # @param inverse [Boolean, String] An optional param. Order of arguments replaced by autocorrection. + # If string is passed, it becomes a predicate method for the first argument node. # @api private # def define_rule(assertion_method, target_method:, preferred_method: nil, inverse: false) @@ -78,10 +79,15 @@ def offense_message(arguments) def new_arguments(arguments) receiver = correct_receiver(arguments.first.receiver) - method_argument = arguments.first.arguments.first&.source + method_argument = arguments.first.arguments.first - new_arguments = [receiver, method_argument].compact - new_arguments.reverse! if #{inverse} + new_arguments = [receiver, method_argument&.source].compact + inverse_condition = if %w[true false].include?('#{inverse}') + #{inverse} + else + method_argument.#{inverse} + end + new_arguments.reverse! if inverse_condition new_arguments end diff --git a/test/rubocop/cop/minitest/assert_match_test.rb b/test/rubocop/cop/minitest/assert_match_test.rb index f276cae7..3014c130 100644 --- a/test/rubocop/cop/minitest/assert_match_test.rb +++ b/test/rubocop/cop/minitest/assert_match_test.rb @@ -22,6 +22,44 @@ def test_do_something RUBY end + def test_registers_offense_when_using_assert_with_match_and_lhs_is_regexp_literal + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert(/regexp/.match(object)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_match(/regexp/, object)`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_match(/regexp/, object) + end + end + RUBY + end + + def test_registers_offense_when_using_assert_with_match_and_rhs_is_regexp_literal + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert(object.match(/regexp/)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_match(/regexp/, object)`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_match(/regexp/, object) + end + end + RUBY + end + def test_registers_offense_when_using_assert_with_match_and_message assert_offense(<<~RUBY, @cop) class FooTest < Minitest::Test