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

Fix assignments in if expressions for like the 5th time #297

Merged
merged 7 commits into from Apr 1, 2013

Conversation

Projects
None yet
2 participants
@presidentbeef
Copy link
Owner

presidentbeef commented Mar 21, 2013

The latest issue is this:

x = 1

if something
  x = 2
  x = 3 # x should be 3
else
  x = 4
  x = 5 # x should be 5
end

x # x should be (1 or 3 or 5)

What Brakeman was actually doing:

x = 1

if something
  x = 2
  x = 3 # x is (1 or 3)
else
  x = 4
  x = 5 # x is (1 or 3 or 5)
end

x # x is (1 or 3 or 5)

Oops! Besides being wrong, this causes a lot of unnecessary duplication of values. On top of that, the previous fix was really complicated and confusing, whereas this is a bit simpler.

process e
if @ignore_ifs or no_branch
exps.each_with_index do |branch, i|
exp[2 + i] = process_if_branch branch

This comment has been minimized.

Copy link
@oreoshake

oreoshake Mar 21, 2013

Contributor

Magic number?

This comment has been minimized.

Copy link
@presidentbeef

presidentbeef Mar 21, 2013

Author Owner

I just knew you were going to call me out on that 🐕

@presidentbeef

This comment has been minimized.

Copy link
Owner Author

presidentbeef commented Apr 1, 2013

Just can't reproduce the Travis failure at all 🚒

@presidentbeef presidentbeef merged commit 620e1a7 into master Apr 1, 2013

1 check failed

default The Travis build failed
Details

Repository owner locked and limited conversation to collaborators Feb 16, 2016

@presidentbeef presidentbeef deleted the assignments_in_else_branch branch Jul 22, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.