Skip to content

Commit

Permalink
Fix autocorrection for some Minitest cops
Browse files Browse the repository at this point in the history
Follow up rails/rails#49236 (comment)

This PR fixes autocorrection for `Minitest/LiteralAsActualArgument`,
`Minitest/AssertPathExists`, and `Minitest/RefutePathExists` cops
to preserve the presence or absence of argument parentheses.
  • Loading branch information
koic committed Sep 12, 2023
1 parent ba71716 commit 8a1bf39
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#259](https://github.com/rubocop/rubocop-minitest/pull/259): Fix autocorrection for `Minitest/LiteralAsActualArgument`, `Minitest/AssertPathExists`, and `Minitest/RefutePathExists` cops to preserve the presence or absence of argument parentheses. ([@koic][])
13 changes: 9 additions & 4 deletions lib/rubocop/cop/minitest/assert_path_exists.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,25 @@ class AssertPathExists < Base
def on_send(node)
assert_file_exists(node) do |path, failure_message|
failure_message = failure_message.first
good_method = build_good_method(path, failure_message)
good_method = build_good_method(node, path, failure_message)
message = format(MSG, good_method: good_method)

add_offense(node, message: message) do |corrector|
corrector.replace(node, good_method)
corrector.replace(node.loc.selector, 'assert_path_exists')
corrector.replace(node.first_argument, path.source)
end
end
end

private

def build_good_method(path, message)
def build_good_method(node, path, message)
args = [path.source, message&.source].compact.join(', ')
"assert_path_exists(#{args})"
if node.parenthesized?
"assert_path_exists(#{args})"
else
"assert_path_exists #{args}"
end
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/minitest/literal_as_actual_argument.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,24 @@ class LiteralAsActualArgument < Base
def on_send(node)
return unless node.method?(:assert_equal)

expected, actual, message = *node.arguments
expected, actual, _message = *node.arguments
return unless actual&.recursive_basic_literal?
return if expected.recursive_basic_literal?

add_offense(all_arguments_range(node)) do |corrector|
autocorrect(corrector, node, expected, actual, message)
autocorrect(corrector, expected, actual)
end
end

def autocorrect(corrector, node, expected, actual, message)
def autocorrect(corrector, expected, actual)
new_actual_source = if actual.hash_type? && !actual.braces?
"{#{actual.source}}"
else
actual.source
end
arguments = [new_actual_source, expected.source, message&.source].compact.join(', ')

corrector.replace(node, "assert_equal(#{arguments})")
corrector.replace(expected, new_actual_source)
corrector.replace(actual, expected.source)
end
end
end
Expand Down
13 changes: 9 additions & 4 deletions lib/rubocop/cop/minitest/refute_path_exists.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,25 @@ class RefutePathExists < Base
def on_send(node)
refute_file_exists(node) do |path, failure_message|
failure_message = failure_message.first
good_method = build_good_method(path, failure_message)
good_method = build_good_method(node, path, failure_message)
message = format(MSG, good_method: good_method)

add_offense(node, message: message) do |corrector|
corrector.replace(node, good_method)
corrector.replace(node.loc.selector, 'refute_path_exists')
corrector.replace(node.first_argument, path.source)
end
end
end

private

def build_good_method(path, message)
def build_good_method(node, path, message)
args = [path.source, message&.source].compact.join(', ')
"refute_path_exists(#{args})"
if node.parenthesized?
"refute_path_exists(#{args})"
else
"refute_path_exists #{args}"
end
end
end
end
Expand Down
38 changes: 38 additions & 0 deletions test/rubocop/cop/minitest/assert_path_exists_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,44 @@ def test_do_something
RUBY
end

def test_registers_offense_when_using_assert_file_exist_without_without_parentheses
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert File.exist?(path)
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_path_exists path`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert_path_exists path
end
end
RUBY
end

def test_registers_offense_when_using_assert_file_exist_and_message_without_parentheses
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert File.exist?(path), 'message'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_path_exists path, 'message'`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert_path_exists path, 'message'
end
end
RUBY
end

def test_does_not_register_offense_when_using_assert_path_exists_method
assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
Expand Down
19 changes: 19 additions & 0 deletions test/rubocop/cop/minitest/literal_as_actual_argument_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,25 @@ def test_do_something
RUBY
end

def test_registers_offense_when_actual_is_basic_literal_without_parentheses
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert_equal foo, 2
^^^^^^ Replace the literal with the first argument.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert_equal 2, foo
end
end
RUBY
end

def test_registers_offense_when_actual_is_recursive_basic_literal
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
Expand Down
38 changes: 38 additions & 0 deletions test/rubocop/cop/minitest/refute_path_exists_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,44 @@ def test_do_something
RUBY
end

def test_registers_offense_when_using_refute_file_exist_without_parentheses
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute File.exist?(path)
^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_path_exists path`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute_path_exists path
end
end
RUBY
end

def test_registers_offense_when_using_refute_file_exist_and_message_without_parentheses
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute File.exist?(path), 'message'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_path_exists path, 'message'`.
end
end
RUBY

assert_correction(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
refute_path_exists path, 'message'
end
end
RUBY
end

def test_does_not_register_offense_when_using_refute_path_exists_method
assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
Expand Down

0 comments on commit 8a1bf39

Please sign in to comment.