diff --git a/lib/brakeman/processors/alias_processor.rb b/lib/brakeman/processors/alias_processor.rb index 975b84d96d21ba69a2b8cd4b375939a586ae1530..249fd55f40da24854b827bdaae00be94c7e23718 100644 --- a/lib/brakeman/processors/alias_processor.rb +++ b/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 diff --git a/test/tests/alias_processor.rb b/test/tests/alias_processor.rb index 35a91e49068a8dfa77649e65f99828bed879e94c..2c6e97dda89ff1b73371f50d8b93439d26cc935b 100644 --- a/test/tests/alias_processor.rb +++ b/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