Convert more Sexp#[] to method calls

lib/brakeman/checks/base_check.rb

@@ -69,7 +69,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
       @has_user_input = Match.new(:cookies, exp)
     elsif request_env? target
       @has_user_input = Match.new(:request, exp)
-    elsif sexp? target and model_name? target[1]
+    elsif sexp? target and model_name? target[1] #TODO: Can this be target.target?
       @has_user_input = Match.new(:model, exp)
     end
 

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -240,7 +240,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
 
       #exp[0] = :ignore #should not be necessary
       @matched = false
-    elsif sexp? target and model_name? target[1]
+    elsif sexp? target and model_name? target[1] #TODO: use method call?
       @matched = Match.new(:model, exp)
     elsif cookies? exp
       @matched = Match.new(:cookies, exp)
@@ -285,9 +285,8 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
 
   #Ignore condition in if Sexp
   def process_if exp
-    exp[2..-1].each do |e|
-      process e if sexp? e
-    end
+    process exp.then_clause if sexp? exp.then_clause
+    process exp.else_clause if sexp? exp.else_clause
     exp
   end
 

lib/brakeman/checks/check_execute.rb

@@ -30,12 +30,12 @@ class Brakeman::CheckExecute < Brakeman::BaseCheck
   #Processes results from Tracker#find_call.
   def process_result result
     call = result[:call]
-
-    args = process call[3]
+    args = call.arglist
+    first_arg = call.first_arg
 
     case call.method
     when :system, :exec
-      failure = include_user_input?(args[1]) || include_interp?(args[1])
+      failure = include_user_input?(first_arg) || include_interp?(first_arg)
     else
       failure = include_user_input?(args) || include_interp?(args)
     end

lib/brakeman/checks/check_link_to.rb

@@ -82,7 +82,7 @@ class Brakeman::CheckLinkTo < Brakeman::CheckCrossSiteScripting
         :link_path => "link_to"
 
     elsif not tracker.options[:ignore_model_output] and match = has_immediate_model?(arg)
-      method = match[2]
+      method = match.method
 
       unless IGNORE_MODEL_METHODS.include? method
         add_result result

lib/brakeman/checks/check_sql.rb

@@ -226,8 +226,8 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
         :confidence => confidence
     end
 
-    if check_for_limit_or_offset_vulnerability args[-1]
-      if include_user_input? args[-1]
+    if check_for_limit_or_offset_vulnerability args.last
+      if include_user_input? args.last
         confidence = CONFIDENCE[:high]
       else
         confidence = CONFIDENCE[:low]
@@ -263,7 +263,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
     return unless node_type? args, :arglist
 
     if node_type? args[2], :iter
-      unsafe_sql? args[2][-1]
+      unsafe_sql? args[2].block
     else
       unsafe_sql? args[2]
     end
@@ -457,7 +457,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
   def check_hash_values exp
     hash_iterate(exp) do |key, value|
       if symbol? key
-        unsafe = case key[1]
+        unsafe = case key.value
                  when :conditions, :having, :select
                    check_query_arguments value
                  when :order, :group

lib/brakeman/checks/check_validation_regex.rb

@@ -53,8 +53,9 @@ class Brakeman::CheckValidationRegex < Brakeman::BaseCheck
   #Get the name of the attribute being validated.
   def get_name validator
     name = validator[1]
+
     if sexp? name
-      name[1]
+      name.value
     else
       name
     end

lib/brakeman/processors/alias_processor.rb

@@ -107,7 +107,6 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
 
     target = exp.target
     method = exp.method
-    args = exp[3]
     first_arg = exp.first_arg
 
     #See if it is possible to simplify some basic cases
@@ -154,7 +153,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
         temp_exp = process_array_access target, exp.args
         exp = temp_exp if temp_exp
       elsif hash? target
-        temp_exp = process_hash_access target, exp.args
+        temp_exp = process_hash_access target, first_arg
         exp = temp_exp if temp_exp
       end
     when :merge!, :update
@@ -231,8 +230,10 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
 
     if @inside_if and val = env[local]
       #avoid setting to value it already is (e.g. "1 or 1")
-      if val != exp.rhs and val[1] != exp.rhs and val[2] != exp.rhs
-        env[local] = Sexp.new(:or, val, exp.rhs).line(exp.line || -2)
+      if val != exp.rhs
+        unless node_type?(val, :or) and (val.rhs == exp.rhs or val.lhs == exp.rhs)
+          env[local] = Sexp.new(:or, val, exp.rhs).line(exp.line || -2)
+        end
       end
     else
       env[local] = exp.rhs
@@ -392,8 +393,10 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     exp
   end
 
+  #This is the right hand side value of a multiple assignment,
+  #like `x = y, z`
   def process_svalue exp
-    exp[1]
+    exp.value
   end
 
   #Constant assignments like
@@ -423,9 +426,9 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     if true? condition
       exps = [exp.then_clause]
     elsif false? condition
-      exps = exp[3..-1]
+      exps = [exp.else_clause]
     else
-      exps = exp[2..-1]
+      exps = [exp.then_clause, exp.else_clause]
     end
 
     was_inside = @inside_if
@@ -530,8 +533,8 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
 
   #Finds the inner most call target which is not the target of a call to <<
   def find_push_target exp
-    if call? exp and exp[2] == :<<
-      find_push_target exp[1]
+    if call? exp and exp.method == :<<
+      find_push_target exp.target
     else
       exp
     end

lib/brakeman/processors/erb_template_processor.rb

@@ -16,14 +16,14 @@ class Brakeman::ErbTemplateProcessor < Brakeman::TemplateProcessor
     if node_type? target, :lvar and target.value == :_erbout
       if method == :concat
         @inside_concat = true
-        args = exp.arglist = process(exp.arglist)
+        exp.arglist = process(exp.arglist)
         @inside_concat = false
 
-        if args.length > 2
+        if exp.args.length > 2
           raise Exception.new("Did not expect more than a single argument to _erbout.concat")
         end
 
-        arg = args[1]
+        arg = exp.first_arg
 
         if arg.node_type == :call and arg.method == :to_s #erb always calls to_s on output
           arg = arg.target
@@ -63,7 +63,7 @@ class Brakeman::ErbTemplateProcessor < Brakeman::TemplateProcessor
         process e
       end
       @inside_concat = true
-      process exp[-1]
+      process exp.last
     else
       exp.map! do |e|
         res = process e

lib/brakeman/processors/lib/processor_helper.rb

@@ -45,7 +45,7 @@ module Brakeman::ProcessorHelper
       when :lvar
         exp.value.to_sym
       when :colon2
-        "#{class_name(exp[1])}::#{exp[2]}".to_sym
+        "#{class_name(exp.lhs)}::#{exp.rhs}".to_sym
       when :colon3
         "::#{exp.value}".to_sym
       when :call

lib/brakeman/processors/model_processor.rb

@@ -77,7 +77,7 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
         when :attr_accessible
           @model[:attr_accessible] ||= []
           args = args.map do |e|
-            e[1]
+            e.value
           end
 
           @model[:attr_accessible].concat args