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

Style/For should be marked as unsafe #10427

Closed
diesl opened this issue Feb 19, 2022 · 2 comments · Fixed by #10429
Closed

Style/For should be marked as unsafe #10427

diesl opened this issue Feb 19, 2022 · 2 comments · Fixed by #10429
Labels

Comments

@diesl
Copy link

diesl commented Feb 19, 2022

Cop Style/For should be marked as unsafe, because it can break code

Reason might be that scopes are different, as quoted from Ruby Style Guide

for doesn’t introduce a new scope (unlike each) and variables defined in its block will be visible outside it.


Expected behavior

I expect safe autocorrection not to break my existing ruby code.

Actual behavior

Running a cop marked as safe breaks my existing ruby code.

Steps to reproduce the problem

for n in 1..3 do
  x = "numbers:" if n == 1
  x = x + " " + n.to_s
  puts x
end

# output is
# numbers: 1
# numbers: 1 2
# numbers: 1 2 3

When I run rubocop -a --only Style/IdenticalConditionalBranches, it autocorrects my file to:

(1..3).each do |n|
   x = "numbers:" if n == 1
   x = x + " " + n.to_s
  puts x
end

# output is
# numbers: 1
# NoMethodError (undefined method `+' for nil:NilClass)

RuboCop version

$ [bundle exec] rubocop -V
1.25.1 (using Parser 3.1.0.0, rubocop-ast 1.15.2, running on ruby 2.7.5 x86_64-linux)
  - rubocop-performance 1.13.2
  - rubocop-rails 2.13.2
  - rubocop-rspec 2.8.0
@jonas054
Copy link
Collaborator

jonas054 commented Feb 19, 2022

I think you mean rubocop -a --only Style/For, but you're right about the reason for the failure. It's the change in scope for the x variable in this case.

I agree. Marking it as unsafe is a simple solution.

@koic koic added the bug label Feb 20, 2022
koic added a commit that referenced this issue Feb 21, 2022
[Fix #10427] Style/For: Mark auto-correction as unsafe
@diesl
Copy link
Author

diesl commented Feb 21, 2022

I think you mean rubocop -a --only Style/For

Sure, I just forgot to change this after c&p a similar issue.

Thanks for fast response and fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants