Ignore branches when assigning via +=

lib/brakeman/processors/alias_processor.rb

@@ -257,12 +257,18 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   #Local assignment
   # x = 1
   def process_lasgn exp
+    self_assign = self_assign?(exp.lhs, exp.rhs)
     exp.rhs = process exp.rhs if sexp? exp.rhs
     return exp if exp.rhs.nil?
 
     local = Sexp.new(:lvar, exp.lhs).line(exp.line || -2)
 
-    set_value local, exp.rhs
+    if self_assign
+      # Skip branching
+      env[local] = exp.rhs
+    else
+      set_value local, exp.rhs
+    end
 
     exp
   end
@@ -270,10 +276,19 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   #Instance variable assignment
   # @x = 1
   def process_iasgn exp
+    self_assign = self_assign?(exp.lhs, exp.rhs)
     exp.rhs = process exp.rhs
     ivar = Sexp.new(:ivar, exp.lhs).line(exp.line)
 
-    set_value ivar, exp.rhs
+    if self_assign
+      if env[ivar].nil? and @meth_env
+        @meth_env[ivar] = exp.rhs
+      else
+        env[ivar] = exp.rhs
+      end
+    else
+      set_value ivar, exp.rhs
+    end
 
     exp
   end
@@ -727,6 +742,14 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     end
   end
 
+  #Return true if for x += blah or @x += blah
+  def self_assign? var, value
+    call? value and
+    value.method == :+ and
+    node_type? value.target, :lvar, :ivar and
+    value.target.value == var
+  end
+
   def value_from_if exp
     if block? exp.else_clause or block? exp.then_clause
       #If either clause is more than a single expression, just use entire

test/tests/alias_processor.rb

@@ -255,7 +255,7 @@ class AliasProcessorTests < Test::Unit::TestCase
   end
 
   def test_simple_or_operation_compaction
-    assert_alias "[0, ((1 || 2) || 3), (4 || 8)]", <<-RUBY
+    assert_alias "[0, 4, (4 || 8)]", <<-RUBY
     x = 1
 
     if z
@@ -546,4 +546,45 @@ class AliasProcessorTests < Test::Unit::TestCase
       end
     OUTPUT
   end
+
+  def test_no_branch_for_plus_equals_with_string
+    assert_alias '"abc"', <<-INPUT
+      x = "a"
+      x += "b" if something
+      x += "c" if something_else
+      x
+    INPUT
+  end
+
+  def test_no_branch_for_plus_equals_with_string_in_ivar
+    assert_alias '"abc"', <<-INPUT
+      @x = "a"
+      @x += "b" if something
+      @x += "c" if something_else
+      @x
+    INPUT
+  end
+
+  #We could do better, but this prevents some Sexp explosions and retains
+  #information about the values
+  def test_no_branch_for_plus_equals_with_interpolated_string
+    assert_alias '"a" + "#{b}" + "c"', <<-'INPUT'
+      x = "a"
+      x += "#{b}" if something
+      x += "c" if something_else
+      x
+    INPUT
+  end
+
+  #Unfortunate to go to this behavior which loses information
+  #but I can't think of a scenario in which the several integers
+  #in ORs would be handled right anyway
+  def test_no_branch_for_plus_equals_with_number
+    assert_alias '6', <<-INPUT
+      x = 1
+      x += 2 if something
+      x += 3 if something_else
+      x
+    INPUT
+  end
 end