Rubocop: Incorrect shadow argument check for local variable in a block

[steventen]

Hi, I noticed that rubocop raises Lint/ShadowedArgument warning for local variable in block.

A simple example code could be like:

numbers = [1, 2, 3]
numbers.each do |i; j|
  j = i * 2
  puts j
end

Where j is a local variable in the inner block.

---

Expected behavior

rubocop should not raise Lint/ShadowedArgument warning, since it is not a shadow variable.

Actual behavior

W: Lint/ShadowedArgument: Argument j was shadowed by a local variable before it was used.
  j = i * 2
  ^^^^^^^^^

RuboCop version

$ rubocop -V
0.58.2 (using Parser 2.5.1.2, running on ruby 2.3.7 x86_64-linux)

[asherkach]

How is this a bug? The variable j is declared by the do block, so the assignment to i * 2 is shadowing. Granted, that j is part of the |i; j| block is worth discussing (flagging)...

2.5.1 :001 > numbers = [1, 2, 3]
 => [1, 2, 3] 
2.5.1 :002 > numbers.each do |i; j|
2.5.1 :003 >     puts j
2.5.1 :004?>   end



 => [1, 2, 3] 
2.5.1 :005 > numbers.each do |i|
2.5.1 :006 >     puts j
2.5.1 :007?>   end
Traceback (most recent call last):
        4: from /Users/asherkach/.rvm/rubies/ruby-2.5.1/bin/irb:11:in `<main>'
        3: from (irb):5
        2: from (irb):5:in `each'
        1: from (irb):6:in `block in irb_binding'
NameError (undefined local variable or method `j' for main:Object)

[steventen]

I guess it really depends on what kind of shadowing we want to flag.

My understanding is that this lint should check local variable that shadows or potentially shadows variable from outer scope, which could cause bugs. But for the example I gave, I would argue that it should be fine from that kind of issue.

[mikegee]

I see your point. We don't call this shadowing:

local_var = nil
local_var = :not_nil

The j block param in your example wasn't from an outside scope, so it wasn't shadowed.

These Rubocop issues can be tough sometimes when the bug is real, but I'd rather say: "just don't write the code that way". 😝

[jonas054]

My reasoning for marking it as a bug was that Ruby itself warns for block local variables:

# file.rb 
numbers = [1, 2, 3]
j = 'hi'
numbers.each do |i; j|
  j = i * 2
  puts j
end

puts j

$ ruby -vw file.rb 
ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux]
file.rb:3: warning: shadowing outer local variable - j
2
4
6
hi

[asherkach]

Ah, that is a very different example, with j existing outside the scope of the block... In the most recent example, I agree, there is argument shadowing.

[jonas054]

I think I confused matters with my example. Sorry about that. Reporting the declaration of the block local variable is the responsibility of the Lint/ShadowingOuterLocalVariable cop, and it works as expected, reporting the same as ruby -w.

What's relevant to this discussion is that rubocop currently reports j = i * 2 as a Lint/ShadowedArgument offense, but ruby -w does not warn. That's a bug in the Lint/ShadowedArgument cop.