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

False positive in Lint/ShadowedArgument cop #5561

Closed
mikecmpbll opened this issue Feb 9, 2018 · 2 comments
Closed

False positive in Lint/ShadowedArgument cop #5561

mikecmpbll opened this issue Feb 9, 2018 · 2 comments

Comments

@mikecmpbll
Copy link

The ShadowedArgument cop is triggering where the variable may not be being modified at all, the smallest reproducable example I can get is:

def resource(opts = nil)
  opts = type if foo
  opts ||= {}

  # ...
end

Here, if the resource method is called with an argument and foo is falsey, opts is not being replaced or "shadowed" in this method.


Expected behavior

Should pass.

Actual behavior

rubocop test.rb 
Inspecting 1 file
W

Offenses:

test.rb:1:14: W: Lint/ShadowedArgument: Argument opts was shadowed by a local variable before it was used.
def resource(opts = nil)
             ^^^^^^^^^^

1 file inspected, 1 offense detected

Steps to reproduce the problem

See example above.

RuboCop version

0.52.1 (using Parser 2.4.0.2, running on ruby 2.3.1 x86_64-darwin14)

@mikegee
Copy link
Contributor

mikegee commented Feb 11, 2018

I confirmed the report, including the fact that both of those conditional lines are required to trigger the offense (neither one is sufficient). This implies the cop tries to avoid false reports when the shadowing is conditional, but this example method is more complicated than it can currently handle.

@alberto
Copy link

alberto commented Feb 20, 2018

Same issue here

akhramov pushed a commit to akhramov/rubocop that referenced this issue Mar 21, 2018
…ositive

`Lint/ShadowedArgument` doesn't handle the case of shorthand
assignments and treats them as not using their arguments.

This change adds an additional check for shorthand assignments.
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

3 participants