rubocop / rubocop-performance

An extension of RuboCop focused on code performance checks.
https://docs.rubocop.org/rubocop-performance
MIT License
670 stars 79 forks source link

`Naming/BlockForwarding` and `Performance/BlockGivenWithExplicitBlock` combined safe autocorrect results in invalid code #444

Open a-lavis opened 7 months ago

a-lavis commented 7 months ago

When code violates both Naming/BlockForwarding and Performance/BlockGivenWithExplicitBlock, performing a safe autocorrect on that code can result in incorrect code.


Expected behavior

When running rubocop -a foo.rb where foo.rb is the following file:

# frozen_string_literal: true

def bar
  puts 'bar!'
  yield
end

def foo(&block)
  if block_given?
    bar(&block)
  else
    puts 'no block'
  end
end

foo do
  puts 'the block'
end

foo

And where your .rubocop.yml is as follows:

require:
  - rubocop-performance

AllCops:
  NewCops: enable
  TargetRubyVersion: 3.2

I expect either the Naming/BlockForwarding or the Performance/BlockGivenWithExplicitBlock safe autocorrect to be applied, but not both.

Actual behavior

It safe autocorrects to:

# frozen_string_literal: true

def bar
  puts 'bar!'
  yield
end

def foo(&)
  if block
    bar(&)
  else
    puts 'no block'
  end
end

foo do
  puts 'the block'
end

foo

If you try running this code using ruby foo.rb, it will fail. The Performance/BlockGivenWithExplicitBlock cop safe autocorrect was applied, so block_given? is changed to just block. The Naming/BlockForwarding cop safe autocorrect was applied, so usage of &block is changed to just &. This means that where we are referencing block, we are referencing an undefined variable. When you run the code you encounter the following error:

foo.rb:9:in `foo': undefined local variable or method `block' for main:Object (NameError)

  if block
     ^^^^^
    from foo.rb:16:in `<main>'

Steps to reproduce the problem

  1. Set your Ruby version to 3.2.
  2. Create an empty directory.
  3. Put the above foo.rb and .rubocop.yml files into that directory.
  4. Run ruby foo.rb, and observe that it runs without failing.
  5. Run rubocop -a foo.rb.
  6. Run ruby foo.rb, and observe that it fails.

RuboCop version

> rubocop -V
1.60.2 (using Parser 3.3.0.5, rubocop-ast 1.30.0, running on ruby 3.2.2) [arm64-darwin22]
  - rubocop-performance 1.20.2
corsonknowles commented 1 week ago

This seems important. Thanks for submitting a PR to fix it! https://github.com/rubocop/rubocop-performance/pull/445