Merge pull request #55 from codeclimate/will/rescue-less

rescue less

Author: wfleming

lib/cc/engine/analyzers/analyzer_base.rb

@@ -1,16 +1,26 @@
+require "cc/engine/analyzers/parser_error"
+
 module CC
   module Engine
     module Analyzers
       class Base
+        RESCUABLE_ERRORS = [
+          ::CC::Engine::Analyzers::ParserError,
+          ::Racc::ParseError,
+        ]
+
         def initialize(engine_config:)
           @engine_config = engine_config
         end
 
         def run(file)
           process_file(file)
+        rescue *RESCUABLE_ERRORS => ex
+          $stderr.puts("Skipping file #{file} due to exception:")
+          $stderr.puts("(#{ex.class}) #{ex.message} #{ex.backtrace.join("\n")}")
         rescue => ex
-          $stderr.puts "Skipping file #{file} due to exception"
-          $stderr.puts "(#{ex.class}) #{ex.message} #{ex.backtrace.join("\n")}"
+          $stderr.puts("#{ex.class} error occurred processing file #{file}: aborting.")
+          raise ex
         end
 
         def files

lib/cc/engine/analyzers/command_line_runner.rb

@@ -0,0 +1,45 @@
+require "open3"
+require "timeout"
+
+module CC
+  module Engine
+    module Analyzers
+      class CommandLineRunner
+        DEFAULT_TIMEOUT = 300
+
+        def initialize(command, timeout = DEFAULT_TIMEOUT)
+          @command = command
+          @timeout = timeout
+        end
+
+        def run(input)
+          Timeout.timeout(timeout) do
+            Open3.popen3 command, "r+" do |stdin, stdout, stderr, wait_thr|
+              stdin.puts input
+              stdin.close
+
+              exit_code = wait_thr.value
+
+              output = stdout.gets
+              stdout.close
+
+              err_output = stderr.gets
+              stderr.close
+
+              if 0 == exit_code
+                yield output
+              else
+                raise ::CC::Engine::Analyzers::ParserError, "Python parser exited with code #{exit_code}:\n#{err_output}"
+              end
+            end
+          end
+        end
+
+        private
+
+        attr_reader :command, :timeout
+      end
+    end
+  end
+end
+

lib/cc/engine/analyzers/javascript/parser.rb

@@ -1,4 +1,4 @@
-require 'timeout'
+require "cc/engine/analyzers/command_line_runner"
 
 module CC
   module Engine
@@ -13,7 +13,7 @@ def initialize(code, filename)
           end
 
           def parse
-            runner = CommandLineRunner.new(js_command, self)
+            runner = CommandLineRunner.new(js_command)
             runner.run(strip_shebang(code)) do |ast|
               json_ast = JSON.parse(ast)
               @syntax_tree = json_ast
@@ -37,36 +37,6 @@ def strip_shebang(code)
             end
           end
         end
-
-        class CommandLineRunner
-          attr_reader :command, :delegate
-
-          DEFAULT_TIMEOUT = 20
-          EXCEPTIONS = [
-            StandardError,
-            Timeout::Error,
-            SystemStackError
-          ]
-
-          def initialize(command, delegate)
-            @command = command
-            @delegate = delegate
-          end
-
-          def run(input, timeout = DEFAULT_TIMEOUT)
-            Timeout.timeout(timeout) do
-              IO.popen command, 'r+' do |io|
-                io.puts input
-                io.close_write
-
-                output = io.gets
-                io.close
-
-                yield output if $?.to_i == 0
-              end
-            end
-          end
-        end
       end
     end
   end

lib/cc/engine/analyzers/parser_error.rb

@@ -0,0 +1,7 @@
+module CC
+  module Engine
+    module Analyzers
+      ParserError = Class.new(StandardError)
+    end
+  end
+end

lib/cc/engine/analyzers/php/parser.rb

@@ -1,5 +1,6 @@
-require 'cc/engine/analyzers/php/ast'
-require 'cc/engine/analyzers/php/nodes'
+require "cc/engine/analyzers/command_line_runner"
+require "cc/engine/analyzers/php/ast"
+require "cc/engine/analyzers/php/nodes"
 
 module CC
   module Engine
@@ -36,30 +37,6 @@ def parser_path
             )
           end
         end
-
-        class CommandLineRunner
-          attr_reader :command, :delegate
-
-          DEFAULT_TIMEOUT = 20
-
-          def initialize(command)
-            @command = command
-          end
-
-          def run(input, timeout = DEFAULT_TIMEOUT)
-            Timeout.timeout(timeout) do
-              IO.popen command, 'r+' do |io|
-                io.puts input
-                io.close_write
-
-                output = io.gets
-                io.close
-
-                yield output if $?.to_i == 0
-              end
-            end
-          end
-        end
       end
     end
   end

lib/cc/engine/analyzers/python/parser.rb

@@ -1,5 +1,6 @@
-require 'timeout'
-require 'json'
+require "cc/engine/analyzers/command_line_runner"
+require "timeout"
+require "json"
 
 module CC
   module Engine
@@ -14,7 +15,7 @@ def initialize(code, filename)
           end
 
           def parse
-            runner = CommandLineRunner.new(python_command, self)
+            runner = CommandLineRunner.new(python_command)
             runner.run(code) do |ast|
               json_ast = JSON.parse(ast)
               @syntax_tree = json_ast
@@ -28,31 +29,6 @@ def python_command
             "python #{file}"
           end
         end
-
-        class CommandLineRunner
-          DEFAULT_TIMEOUT = 20
-
-          attr_reader :command, :delegate
-
-          def initialize(command, delegate)
-            @command = command
-            @delegate = delegate
-          end
-
-          def run(input, timeout = DEFAULT_TIMEOUT)
-            Timeout.timeout(timeout) do
-              IO.popen command, "r+" do |io|
-                io.puts input
-                io.close_write
-
-                output = io.gets
-                io.close
-
-                yield output if $?.to_i == 0
-              end
-            end
-          end
-        end
       end
     end
   end

lib/cc/engine/analyzers/ruby/main.rb

@@ -19,7 +19,7 @@ class Main < CC::Engine::Analyzers::Base
           ]
           DEFAULT_MASS_THRESHOLD = 18
           BASE_POINTS = 10_000
-          TIMEOUT = 10
+          TIMEOUT = 300
 
           private
 

spec/cc/engine/analyzers/javascript/main_spec.rb

@@ -36,6 +36,16 @@
       expect(json["content"]["body"]).to match /This issue has a mass of `99`/
       expect(json["fingerprint"]).to eq("55ae5d0990647ef496e9e0d315f9727d")
     end
+
+    it "skips unparsable files" do
+      create_source_file("foo.js", <<-EOJS)
+        function () { do(); // missing closing brace
+      EOJS
+
+      expect {
+        expect(run_engine(engine_conf)).to eq("")
+      }.to output(/Skipping file/).to_stderr
+    end
   end
 
   it "does not flag duplicate comments" do

spec/cc/engine/analyzers/php/main_spec.rb

@@ -48,6 +48,16 @@
       expect(json["content"]["body"]).to match /This issue has a mass of `44`/
       expect(json["fingerprint"]).to eq("667da0e2bab866aa2fe9d014a65d57d9")
     end
+
+    it "skips unparsable files" do
+      create_source_file("foo.php", <<-EOPHP)
+        <?php blorb &; "fee
+      EOPHP
+
+      expect {
+        expect(run_engine(engine_conf)).to eq("")
+      }.to output(/Skipping file/).to_stderr
+    end
   end
 
   def printed_issue

spec/cc/engine/analyzers/python/main_spec.rb

@@ -36,6 +36,16 @@
       expect(json["content"]["body"]).to match /This issue has a mass of `54`/
       expect(json["fingerprint"]).to eq("42b832387c997f54a2012efb2159aefc")
     end
+
+    it "skips unparsable files" do
+      create_source_file("foo.py", <<-EOPY)
+        ---
+      EOPY
+
+      expect {
+        expect(run_engine(engine_conf)).to eq("")
+      }.to output(/Skipping file/).to_stderr
+    end
   end
 
   def engine_conf