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

AssertMatch/RefuteMatch autofix can be incorrect #310

Open
movermeyer opened this issue Jun 6, 2024 · 1 comment
Open

AssertMatch/RefuteMatch autofix can be incorrect #310

movermeyer opened this issue Jun 6, 2024 · 1 comment

Comments

@movermeyer
Copy link

movermeyer commented Jun 6, 2024

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
@movermeyer movermeyer changed the title Minitest/AssertMatch autofix can be incorrect AssertMatch/RefuteMatch autofix can be incorrect Jun 6, 2024
@sambostock
Copy link
Contributor

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants