Add new Performance/BlockGivenWithExplicitBlock cop

[fatkodima]

This cop identifies unnecessary use of a block_given? where explicit check of block argument would suffice.

# bad
def method(&block)
  do_something if block_given?
end

# good
def method(&block)
  do_something if block
end

Benchmark

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  gem 'benchmark-ips'
end

def if_block(&block)
  if block
  end
end

def if_block_given(&block)
  if block_given?
  end
end

Benchmark.ips do |x|
  x.report('if block')         { if_block }
  x.report('if block_given?')  { if_block_given }
  x.compare!
end

Results

Warming up --------------------------------------
            if block     1.039M i/100ms
     if block_given?   844.838k i/100ms
Calculating -------------------------------------
            if block     10.382M (± 1.1%) i/s -     51.960M in   5.005227s
     if block_given?      8.443M (± 1.1%) i/s -     42.242M in   5.003794s

Comparison:
            if block: 10382417.4 i/s
     if block_given?:  8443073.9 i/s - 1.23x  (± 0.00) slower

[marcandre]

Oh, cool, I had to do just that for ostruct and was wondering if that would trigger block capturing or not (and be slower). Glad it doesn't.

I'd mark the autocorrection as unsafe, even it's quite unlikely to be:

def foo(&block)
  block ||= ->(x) { ...}
  warn "Using default ..." unless block_given?
  foo(&block)
end

Or you bail of the autocorrection if the variable is reassigned?

[fatkodima]

Good point. Updated to not trigger an offense when the block arg is reassigned.

[marcandre]

Sorry I forgot to check this. Don't hesitate to ping me after 2-3 days.

[fatkodima]

Updated.

[marcandre]

LGTM 👍

[koic]

Thanks!