rubocop / rubocop-minitest

Code style checking for Minitest files.
https://docs.rubocop.org/rubocop-minitest
MIT License
144 stars 44 forks source link

`AssertMatch`/`RefuteMatch` autofix can be incorrect #310

Open movermeyer opened 6 months ago

movermeyer commented 6 months ago

When applying Minitest/AssertMatch (or Minitest/RefuteMatch)'s auto-fix to the following the working code:

require "minitest/autorun"

class AssertMatchTest < Minitest::Test
  def test_assert_match_autofix
    hello = "Hello World!"
    greeting = %r{Hello}
    assert hello.match?(%r{World})
    assert hello.match? greeting
  end
end

Expected behavior

It would auto-correct assert hello.match? greeting to assert_match greeting, hello. Indeed, it works correctly for assert hello.match?(%r{World}).

Actual behavior

It auto-corrects assert hello.match? greeting to assert_match hello, greeting (note the order of parameters), which causes the test to error:

TypeError: no implicit conversion of Regexp into String

since it didn't swap the order of the parameters.

Steps to reproduce the problem

  1. Save the above code into a file (foo_test.rb)
  2. Run the test to verify that it passes (ruby foo_test.rb)
  3. Autoformat the code (rubocop -a foo_test.rb)
  4. Run the test again (ruby foo_test.rb)

See the test error.

RuboCop version

➜ bundle exec rubocop -V
1.64.1 (using Parser 3.3.0.5, rubocop-ast 1.31.2, running on ruby 3.1.4) [arm64-darwin23]
  - rubocop-minitest 0.35.0
  - rubocop-performance 1.21.0
sambostock commented 2 months ago

323 introduces an OnlyRegexpLiteral option, which if enabled, resolves this issue. Unfortunately, it does so by simply allowing the offense, because the cop would have to follow the local variable to determine it's a Regexp.