Lint in parallel (#118)

* Lint in parallel

* Synced output for dot/flycheck formatters

* Re-raise exceptions raised in fibers

* Add readme instructions

README.md

@@ -15,6 +15,22 @@
   </p>
 </p>
 
+- [About](#about)
+- [Usage](#usage)
+  * [Run in parallel](#run-in-parallel)
+- [Installation](#installation)
+  * [As a project dependency:](#as-a-project-dependency)
+  * [OS X](#os-x)
+  * [Docker](#docker)
+  * [From sources](#from-sources)
+- [Configuration](#configuration)
+  * [Only/Except](#onlyexcept)
+  * [Explanation](#explanation)
+  * [Inline disabling](#inline-disabling)
+- [Editor integration](#editor-integration)
+- [Credits & inspirations](#credits--inspirations)
+- [Contributors](#contributors)
+
 ## About
 
 Ameba is a static code analysis tool for the Crystal language.
@@ -49,6 +65,26 @@ Finished in 542.64 milliseconds
 
 ```
 
+### Run in parallel
+
+Starting from 0.31.0 Crystal [supports parallelism](https://crystal-lang.org/2019/09/06/parallelism-in-crystal.html).
+It allows to run linting in parallel too.
+In order to take advantage of this feature you need to build ameba with preview_mt support:
+
+```
+$ crystal build src/cli.cr -Dpreview_mt -o bin/ameba
+$ make install
+```
+
+Some quick benchmark results measured while running Ameba on Crystal repo:
+
+```
+$ CRYSTAL_WORKERS=1 ameba #=> 29.11 seconds
+$ CRYSTAL_WORKERS=2 ameba #=> 19.49 seconds
+$ CRYSTAL_WORKERS=4 ameba #=> 13.48 seconds
+$ CRYSTAL_WORKERS=8 ameba #=> 10.14 seconds
+```
+
 ## Installation
 
 ### As a project dependency:

spec/ameba/runner_spec.cr

@@ -53,6 +53,19 @@ module Ameba
         Runner.new(all_rules, [source], formatter, default_severity).run.success?.should be_true
       end
 
+      context "exception in rule" do
+        it "raises an exception raised in fiber while running a rule" do
+          rule = RaiseRule.new
+          rule.should_raise = true
+          rules = [rule] of Rule::Base
+          source = Source.new "", "source.cr"
+
+          expect_raises(Exception, "something went wrong") do
+            Runner.new(rules, [source], formatter, default_severity).run
+          end
+        end
+      end
+
       context "invalid syntax" do
         it "reports a syntax error" do
           rules = [Rule::Lint::Syntax.new] of Rule::Base

spec/spec_helper.cr

@@ -81,6 +81,15 @@ module Ameba
     end
   end
 
+  # A rule that always raises an error
+  struct RaiseRule < Rule::Base
+    property should_raise = false
+
+    def test(source)
+      should_raise && raise "something went wrong"
+    end
+  end
+
   class DummyFormatter < Formatter::BaseFormatter
     property started_sources : Array(Source)?
     property finished_sources : Array(Source)?

src/ameba/formatter/dot_formatter.cr

@@ -7,6 +7,7 @@ module Ameba::Formatter
     include Util
 
     @started_at : Time?
+    @mutex = Thread::Mutex.new
 
     # Reports a message when inspection is started.
     def started(sources)
@@ -18,12 +19,12 @@ module Ameba::Formatter
     # Reports a result of the inspection of a corresponding source.
     def source_finished(source : Source)
       sym = source.valid? ? ".".colorize(:green) : "F".colorize(:red)
-      output << sym
+      @mutex.synchronize { output << sym }
-      output.flush
     end
 
     # Reports a message when inspection is finished.
     def finished(sources)
+      output.flush
       output << "\n\n"
 
       show_affected_code = !config[:without_affected_code]?
@@ -38,7 +39,7 @@ module Ameba::Formatter
           output << "[#{issue.rule.severity.symbol}] #{issue.rule.name}: #{issue.message}\n".colorize(:red)
 
           if show_affected_code && (code = affected_code(source, location))
-            output << code
+            output << code.colorize(:default)
           end
 
           output << "\n"
@@ -51,15 +52,15 @@ module Ameba::Formatter
 
     private def started_message(size)
       if size == 1
-        "Inspecting 1 file.\n\n"
+        "Inspecting 1 file.\n\n".colorize(:default)
       else
-        "Inspecting #{size} files.\n\n"
+        "Inspecting #{size} files.\n\n".colorize(:default)
       end
     end
 
     private def finished_in_message(started, finished)
       if started && finished
-        "Finished in #{to_human(finished - started)} \n\n"
+        "Finished in #{to_human(finished - started)} \n\n".colorize(:default)
       end
     end
 

src/ameba/formatter/flycheck_formatter.cr

@@ -1,12 +1,16 @@
 module Ameba::Formatter
   class FlycheckFormatter < BaseFormatter
+    @mutex = Mutex.new
+
     def source_finished(source : Source)
       source.issues.each do |e|
         next if e.disabled?
         if loc = e.location
-          output.printf "%s:%d:%d: %s: [%s] %s\n",
+          @mutex.synchronize do
-            source.path, loc.line_number, loc.column_number, e.rule.severity.symbol,
+            output.printf "%s:%d:%d: %s: [%s] %s\n",
-            e.rule.name, e.message.gsub("\n", " ")
+              source.path, loc.line_number, loc.column_number, e.rule.severity.symbol,
+              e.rule.name, e.message.gsub("\n", " ")
+          end
         end
       end
     end

src/ameba/runner.cr

@@ -68,24 +68,42 @@ module Ameba
     #
     def run
       @formatter.started @sources
-      @sources.each do |source|
+      channels = @sources.map { Channel(Exception?).new }
-        @formatter.source_started source
+      @sources.each_with_index do |source, idx|
-
+        channel = channels[idx]
-        if @syntax_rule.catch(source).valid?
+        spawn do
-          @rules.each do |rule|
+          run_source(source)
-            next if rule.excluded?(source)
+        rescue e
-            rule.test(source)
+          channel.send(e)
-          end
+        else
-          check_unneeded_directives(source)
+          channel.send(nil)
         end
-
-        @formatter.source_finished source
       end
+
+      channels.each do |c|
+        e = c.receive
+        raise e unless e.nil?
+      end
+
       self
     ensure
       @formatter.finished @sources
     end
 
+    private def run_source(source)
+      @formatter.source_started source
+
+      if @syntax_rule.catch(source).valid?
+        @rules.each do |rule|
+          next if rule.excluded?(source)
+          rule.test(source)
+        end
+        check_unneeded_directives(source)
+      end
+
+      @formatter.source_finished source
+    end
+
     # Explains an issue at a specified *location*.
     #
     # Runner should perform inspection before doing the explain.