Use hash for call locations instead of array

lib/brakeman/call_index.rb

@@ -82,13 +82,13 @@ class Brakeman::CallIndex
   def remove_indexes_by_class classes
     @calls_by_method.each do |name, calls|
       calls.delete_if do |call|
-        call[:location][0] == :class and classes.include? call[:location][1]
+        call[:location][:type] == :class and classes.include? call[:location][:class]
       end
     end
 
     @calls_by_target.each do |name, calls|
       calls.delete_if do |call|
-        call[:location][0] == :class and classes.include? call[:location][1]
+        call[:location][:type] == :class and classes.include? call[:location][:class]
       end
     end
   end
@@ -206,8 +206,8 @@ class Brakeman::CallIndex
   end
 
   def from_template call, template_name
-    return false unless call[:location][0] == :template
+    return false unless call[:location][:type] == :template
     return true if template_name.nil?
-    call[:location][1] == template_name
+    call[:location][:template] == template_name
   end
 end

lib/brakeman/checks/base_check.rb

@@ -31,7 +31,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
 
   #Add result to result list, which is used to check for duplicates
   def add_result result, location = nil
-    location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][1]
+    location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][:class] || result[:location][:template]
     location = location[:name] if location.is_a? Hash
     location = location.to_sym
 
@@ -244,7 +244,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
       raise ArgumentError
     end
 
-    location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][1]
+    location ||= (@current_template && @current_template[:name]) || @current_class || @current_module || @current_set || result[:location][:class] || result[:location][:template]
 
     location = location[:name] if location.is_a? Hash
     location = location.to_sym

lib/brakeman/checks/check_execute.rb

@@ -84,20 +84,12 @@ class Brakeman::CheckExecute < Brakeman::BaseCheck
       user_input = nil
     end
 
-    warning = { :warning_type => "Command Injection",
+    warn :result => result,
+      :warning_type => "Command Injection",
       :warning_code => :command_injection,
       :message => "Possible command injection",
       :code => exp,
       :user_input => user_input,
-      :confidence => confidence }
-
-    if result[:location][0] == :template
-      warning[:template] = result[:location][1]
-    else
-      warning[:class] = result[:location][1]
-      warning[:method] = result[:location][2]
-    end
-
-    warn warning
+      :confidence => confidence
   end
 end

lib/brakeman/checks/check_select_tag.rb

@@ -24,7 +24,7 @@ class Brakeman::CheckSelectTag < Brakeman::BaseCheck
     @message = "Upgrade to Rails #{suggested_version}, #{tracker.config[:rails_version]} select_tag is vulnerable (CVE-2012-3463)"
 
     calls = tracker.find_call(:target => nil, :method => :select_tag).select do |result|
-      result[:location][0] == :template
+      result[:location][:type] == :template
     end
 
     calls.each do |result|

lib/brakeman/checks/check_select_vulnerability.rb

@@ -24,7 +24,7 @@ class Brakeman::CheckSelectVulnerability < Brakeman::BaseCheck
     @message = "Upgrade to Rails #{suggested_version}, #{tracker.config[:rails_version]} select() helper is vulnerable"
 
     calls = tracker.find_call(:target => nil, :method => :select).select do |result|
-      result[:location][0] == :template
+      result[:location][:type] == :template
     end
 
     calls.each do |result|
@@ -47,7 +47,7 @@ class Brakeman::CheckSelectVulnerability < Brakeman::BaseCheck
         confidence = CONFIDENCE[:low]
       end
 
-      warn :template => result[:location][1],
+      warn :template => result[:location][:template],
           :warning_type => "Cross Site Scripting",
           :warning_code => :select_options_vuln,
           :result => result,

lib/brakeman/checks/check_sql.rb

@@ -68,7 +68,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
         if model[:options][:named_scope]
           model[:options][:named_scope].each do |args|
             call = make_call(nil, :named_scope, args).line(args.line)
-            scope_calls << { :call => call, :location => [:class, name ], :method => :named_scope }
+            scope_calls << { :call => call, :location => { :type => :class, :class => name }, :method => :named_scope }
           end
         end
        end
@@ -84,10 +84,10 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
               process_scope_with_block name, args
             elsif second_arg.node_type == :call
               call = second_arg
-              scope_calls << { :call => call, :location => [:class, name ], :method => call.method }
+              scope_calls << { :call => call, :location => { :type => :class, :class => name }, :method => call.method }
             else
               call = make_call(nil, :scope, args).line(args.line)
-              scope_calls << { :call => call, :location => [:class, name ], :method => :scope }
+              scope_calls << { :call => call, :location => { :type => :class, :class => name }, :method => :scope }
             end
           end
         end
@@ -174,7 +174,8 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
         end
       end
     elsif block.node_type == :call
-      process_result :target => block.target, :method => block.method, :call => block, :location => [:class, model_name, scope_name]
+      process_result :target => block.target, :method => block.method, :call => block,
+        :location => { :type => :class, :class => model_name, :method => scope_name }
     end
   end
 

lib/brakeman/processors/lib/find_all_calls.rb

@@ -49,15 +49,12 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor
     method = exp.method
     process_call_args exp
 
-    call = { :target => target, :method => method, :call => exp, :nested => @in_target, :chain => get_chain(exp) }
-    
-    if @current_template
-      call[:location] = [:template, @current_template]
-    else
-      call[:location] = [:class, @current_class, @current_method]
-    end
-
-    @calls << call
+    @calls << { :target => target,
+                :method => method,
+                :call => exp,
+                :nested => @in_target,
+                :chain => get_chain(exp),
+                :location => make_location }
 
     exp
   end
@@ -67,15 +64,11 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor
   def process_render exp
     process exp.last if sexp? exp.last
 
-    call = { :target => nil, :method => :render, :call => exp, :nested => false }
-
-    if @current_template
-      call[:location] = [:template, @current_template]
-    else
-      call[:location] = [:class, @current_class, @current_method]
-    end
-
-    @calls << call
+    @calls << { :target => nil,
+                :method => :render,
+                :call => exp,
+                :nested => false,
+                :location => make_location }
 
     exp
   end
@@ -85,15 +78,11 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor
   def process_dxstr exp
     process exp.last if sexp? exp.last
 
-    call = { :target => nil, :method => :`, :call => exp, :nested => false }
-
-    if @current_template
-      call[:location] = [:template, @current_template]
-    else
-      call[:location] = [:class, @current_class, @current_method]
-    end
-
-    @calls << call
+    @calls << { :target => nil,
+                :method => :`,
+                :call => exp,
+                :nested => false,
+                :location => make_location }
 
     exp
   end
@@ -102,15 +91,11 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor
   def process_dsym exp
     exp.each { |arg| process arg if sexp? arg }
 
-    call = { :target => nil, :method => :literal_to_sym, :call => exp, :nested => false }
-
-    if @current_template
-      call[:location] = [:template, @current_template]
-    else
-      call[:location] = [:class, @current_class, @current_method]
-    end
-
-    @calls << call
+    @calls << { :target => nil,
+                :method => :literal_to_sym,
+                :call => exp,
+                :nested => false,
+                :location => make_location }
 
     exp
   end
@@ -156,4 +141,18 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor
       [get_target(call)]
     end
   end
+
+  def make_location
+    if @current_template
+      { :type => :template,
+        :template => @current_template,
+        :file => @current_file }
+    else
+      { :type => :class,
+        :class => @current_class,
+        :method => @current_method,
+        :file => @current_file }
+    end
+
+  end
 end

lib/brakeman/warning.rb

@@ -24,13 +24,14 @@ class Brakeman::Warning
 
     result = options[:result]
     if result
-      if result[:location][0] == :template #template result
-        @template ||= result[:location][1]
-        @code ||= result[:call]
+      @code ||= result[:call]
+      @file ||= result[:file]
+
+      if result[:location][:type] == :template #template result
+        @template ||= result[:location][:template]
       else
-        @class ||= result[:location][1]
-        @method ||= result[:location][2]
-        @code ||= result[:call]
+        @class ||= result[:location][:class]
+        @method ||= result[:location][:method]
       end
     end