Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add {Assert,Refute}Match OnlyRegexpLiteral config #323

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/new_add_assert_refute_match.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#323](https://github.com/rubocop/rubocop-minitest/pull/323): Add `{Assert,Refute}Match` `OnlyRegexpLiteral` config. ([@sambostock][])
2 changes: 2 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Minitest/AssertMatch:
StyleGuide: 'https://minitest.rubystyle.guide#assert-match'
Enabled: true
VersionAdded: '0.6'
OnlyRegexpLiteral: false

Minitest/AssertNil:
Description: 'This cop enforces the test to use `assert_nil` instead of using `assert_equal(nil, something)` or `assert(something.nil?)`.'
Expand Down Expand Up @@ -266,6 +267,7 @@ Minitest/RefuteMatch:
StyleGuide: 'https://minitest.rubystyle.guide#refute-match'
Enabled: true
VersionAdded: '0.6'
OnlyRegexpLiteral: false

Minitest/RefuteNil:
Description: 'This cop enforces the test to use `refute_nil` instead of using `refute_equal(nil, something)` or `refute(something.nil?)`.'
Expand Down
54 changes: 28 additions & 26 deletions lib/rubocop/cop/minitest/assert_match.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,50 +18,52 @@ module Minitest
# assert_match(regex, string)
# assert_match(matcher, string, 'message')
#
# @example OnlyRegexpLiteral: false (default)
# # bad
# assert /.../.match?(object)
# assert object.match?(/.../)
# assert matcher.match?(object)
#
# # good
# assert_match(/.../, object)
# assert_match(matcher, object)
#
# @example OnlyRegexpLiteral: true
# # bad
# assert /.../.match?(object)
# assert object.match?(/.../)
#
# # good
# assert_match(/.../, object)
# assert_match(matcher, object)
#
class AssertMatch < Base
include ArgumentRangeHelper
include AssertRefuteMatchHelper
extend AutoCorrector

MSG = 'Prefer using `assert_match(%<preferred>s)`.'
RESTRICT_ON_SEND = %i[assert assert_operator].freeze

def_node_matcher :assert_match, <<~PATTERN
def_node_matcher :match_assertion, <<~PATTERN
{
(send nil? :assert (send $_ {:match :match? :=~} $_) $...)
(send nil? :assert_operator $_ (sym :=~) $_ $...)
}
PATTERN

# rubocop:disable Metrics/AbcSize
def on_send(node)
assert_match(node) do |expected, actual, rest_args|
basic_arguments = order_expected_and_actual(expected, actual)
preferred = (message_arg = rest_args.first) ? "#{basic_arguments}, #{message_arg.source}" : basic_arguments
message = format(MSG, preferred: preferred)

add_offense(node, message: message) do |corrector|
corrector.replace(node.loc.selector, 'assert_match')

range = if node.method?(:assert)
node.first_argument
else
node.first_argument.source_range.begin.join(node.arguments[2].source_range.end)
end

corrector.replace(range, basic_arguments)
end
end
check_match_assertion(node)
end
# rubocop:enable Metrics/AbcSize

private

def order_expected_and_actual(expected, actual)
if actual.regexp_type?
[actual, expected]
else
[expected, actual]
end.map(&:source).join(', ')
def basic_preferred_assertion_method_name
:assert
end

def preferred_assertion_method_name
:assert_match
end
end
end
Expand Down
53 changes: 27 additions & 26 deletions lib/rubocop/cop/minitest/refute_match.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,52 @@ module Minitest
# refute_match(matcher, string)
# refute_match(matcher, string, 'message')
#
# @example OnlyRegexpLiteral: false (default)
# # bad
# refute /.../.match?(object)
# refute object.match?(/.../)
# refute matcher.match?(object)
#
# # good
# refute_match(/.../, object)
# refute_match(matcher, object)
#
# @example OnlyRegexpLiteral: true
# # bad
# refute /.../.match?(object)
# refute object.match?(/.../)
#
# # good
# refute_match(/.../, object)
# refute_match(matcher, object)
class RefuteMatch < Base
include ArgumentRangeHelper
include AssertRefuteMatchHelper
extend AutoCorrector

MSG = 'Prefer using `refute_match(%<preferred>s)`.'
RESTRICT_ON_SEND = %i[refute refute_operator assert_operator].freeze

def_node_matcher :refute_match, <<~PATTERN
def_node_matcher :match_assertion, <<~PATTERN
{
(send nil? :refute (send $_ {:match :match? :=~} $_) $...)
(send nil? :refute_operator $_ (sym :=~) $_ $...)
(send nil? :assert_operator $_ (sym :!~) $_ $...)
}
PATTERN

# rubocop:disable Metrics/AbcSize
def on_send(node)
refute_match(node) do |expected, actual, rest_args|
basic_arguments = order_expected_and_actual(expected, actual)
preferred = (message_arg = rest_args.first) ? "#{basic_arguments}, #{message_arg.source}" : basic_arguments
message = format(MSG, preferred: preferred)

add_offense(node, message: message) do |corrector|
corrector.replace(node.loc.selector, 'refute_match')

range = if node.method?(:refute)
node.first_argument
else
node.first_argument.source_range.begin.join(node.arguments[2].source_range.end)
end

corrector.replace(range, basic_arguments)
end
end
check_match_assertion(node)
end
# rubocop:enable Metrics/AbcSize

private

def order_expected_and_actual(expected, actual)
if actual.regexp_type?
[actual, expected]
else
[expected, actual]
end.map(&:source).join(', ')
def basic_preferred_assertion_method_name
:refute
end

def preferred_assertion_method_name
:refute_match
end
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/minitest_cops.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative 'mixin/argument_range_helper'
require_relative 'mixin/assert_refute_match_helper'
require_relative 'mixin/in_delta_mixin'
require_relative 'mixin/instance_of_assertion_handleable'
require_relative 'mixin/minitest_cop_rule'
Expand Down
54 changes: 54 additions & 0 deletions lib/rubocop/cop/mixin/assert_refute_match_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Common implementation for `AssertMatch` and `RefuteMatch` cops.
module AssertRefuteMatchHelper
private

def check_match_assertion(node)
match_assertion(node) do |expected, actual, rest_args|
basic_argument_nodes = order_expected_and_actual(expected, actual)
next if allowed_matcher?(basic_argument_nodes.first)

basic_arguments = basic_argument_nodes.map(&:source).join(', ')

add_offense(node, message: format_message(basic_arguments, rest_args)) do |corrector|
corrector.replace(node.loc.selector, preferred_assertion_method_name)
corrector.replace(assertion_arguments_range(node), basic_arguments)
end
end
end

def allowed_matcher?(matcher_node)
if cop_config['OnlyRegexpLiteral']
!matcher_node.regexp_type?
else
false
end
end

def format_message(basic_arguments, rest_args)
preferred = (message_arg = rest_args.first) ? "#{basic_arguments}, #{message_arg.source}" : basic_arguments

format(self.class::MSG, preferred: preferred)
end

def assertion_arguments_range(node)
if node.method?(basic_preferred_assertion_method_name)
node.first_argument
else
node.first_argument.source_range.begin.join(node.arguments[2].source_range.end)
end
end

def order_expected_and_actual(expected, actual)
if actual.regexp_type?
[actual, expected]
else
[expected, actual]
end
end
end
end
end
72 changes: 72 additions & 0 deletions test/rubocop/cop/minitest/assert_match_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,76 @@ def test_do_something
end
RUBY
end

def test_does_not_register_offense_when_neither_receiver_nor_first_argument_is_a_regexp_and_only_regexp_is_enabled
enable_only_regexp

assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert matcher.match?(object)
end
end
RUBY
end

def test_does_not_register_offense_when_receiver_and_arguments_are_string_literals_and_only_regexp_is_enabled
enable_only_regexp

assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert "...".match?("...")
end
end
RUBY
end

def test_does_register_offense_when_receiver_is_a_regexp_literal_and_only_regexp_is_enabled
enable_only_regexp

assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert /.../.match?(object)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_match(/.../, object)`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert_match /.../, object
end
end
RUBY
end

def test_does_register_offense_when_first_argument_is_a_regexp_literal_and_only_regexp_is_enabled
enable_only_regexp

assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert object.match?(/.../)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_match(/.../, object)`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert_match /.../, object
end
end
RUBY
end

private

def enable_only_regexp
@configuration = RuboCop::Config.new({ 'Minitest/AssertMatch' => { 'OnlyRegexpLiteral' => true } })
end
end
72 changes: 72 additions & 0 deletions test/rubocop/cop/minitest/refute_match_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,76 @@ def test_do_something
end
RUBY
end

def test_does_not_register_offense_when_neither_receiver_nor_first_argument_is_a_regexp_and_only_regexp_is_enabled
enable_only_regexp

assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute matcher.match?(object)
end
end
RUBY
end

def test_does_not_register_offense_when_receiver_and_arguments_are_string_literals_and_only_regexp_is_enabled
enable_only_regexp

assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute "...".match?("...")
end
end
RUBY
end

def test_does_register_offense_when_receiver_is_a_regexp_literal_and_only_regexp_is_enabled
enable_only_regexp

assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute /.../.match?(object)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_match(/.../, object)`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute_match /.../, object
end
end
RUBY
end

def test_does_register_offense_when_first_argument_is_a_regexp_literal_and_only_regexp_is_enabled
enable_only_regexp

assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute object.match?(/.../)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_match(/.../, object)`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute_match /.../, object
end
end
RUBY
end

private

def enable_only_regexp
@configuration = RuboCop::Config.new({ 'Minitest/RefuteMatch' => { 'OnlyRegexpLiteral' => true } })
end
end