Minitest/LiteralAsActualArgument infinitely loops when both args are literals.

[amomchilov]

I ran across a piece of code in which someone was trying to assert that a block would never be called. It looks like they didn't know about #flunk, so they used this instead:

assert_equal 0, 1, "This block should not have run"

While this is odd code that should be fixed, it shouldn't crash the cop.

---

Expected behavior

Several behaviours seem reasonable to me:

1. Do nothing
2. Swap the two literals
3. Ideally, compare the two literals for equality and either:

• Suggest to use flunk if they're unequal
• Delete the assertion if they're equal

Actual behavior

Infinite loop detected in .../file.rb and caused by Minitest/LiteralAsActualArgument -> Minitest/LiteralAsActualArgument
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:298:in `check_for_infinite_loop'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:281:in `block in iterate_until_no_changes'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:280:in `loop'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:280:in `iterate_until_no_changes'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:249:in `do_inspection_loop'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:130:in `block in file_offenses'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:155:in `file_offense_cache'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:129:in `file_offenses'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:120:in `process_file'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:101:in `block in each_inspected_file'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:100:in `each'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:100:in `reduce'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:100:in `each_inspected_file'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:86:in `inspect_files'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/runner.rb:47:in `run'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli/command/execute_runner.rb:26:in `block in execute_runner'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli/command/execute_runner.rb:52:in `with_redirect'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli/command/execute_runner.rb:25:in `execute_runner'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli/command/execute_runner.rb:17:in `run'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli/command.rb:11:in `run'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli/environment.rb:18:in `run'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli.rb:71:in `run_command'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli.rb:78:in `execute_runners'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/lib/rubocop/cli.rb:47:in `run'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/exe/rubocop:12:in `block in <top (required)>'
/opt/rubies/2.7.2/lib/ruby/2.7.0/benchmark.rb:308:in `realtime'
/Users/Alex/.gem/ruby/2.7.2/gems/rubocop-1.21.0/exe/rubocop:12:in `<top (required)>'
/Users/Alex/.gem/ruby/2.7.2/bin/rubocop:23:in `load'
/Users/Alex/.gem/ruby/2.7.2/bin/rubocop:23:in `<main>'

Steps to reproduce the problem

On a file that contains an assertion with two literal args (like shown above), run:

rubocop --auto-correct-all --only Minitest/LiteralAsActualArgument

RuboCop version

$ [bundle exec] rubocop -V
1.21.0 (using Parser 3.0.2.0, rubocop-ast 1.11.0, running on ruby 2.7.2 x86_64-darwin20)
  - rubocop-minitest 0.15.0
  - rubocop-rails 2.11.3
  - rubocop-sorbet 0.6.2

[andyw8]

Swap the two literals

Not sure I understand, what would be the benefit in that?

Delete the assertion if they're equal

It's probably not feasible to determine the author's intention so I'd say that's not safe.

[andyw8]

(Thanks for the report!)

[amomchilov]

Not sure I understand, what would be the benefit in that?

None-what-so-ever, but it wouldn't be a crasher :)

It's probably not feasible to determine the author's intention so I'd say that's not safe.

If someone wrote assert_equal 1, 1, what reason might they have against deleting it? I can't think of any. (not to say people won't come up with some ... creative code)