Refactor formatters

spec/ameba/formatter/dot_formatter_spec.cr

@@ -0,0 +1,41 @@
+require "../../spec_helper"
+
+module Ameba::Formatter
+  describe DotFormatter do
+    output = IO::Memory.new
+    subject = DotFormatter.new output
+
+    describe "#started" do
+      it "writes started message" do
+        subject.started [Source.new ""]
+        output.to_s.should eq "Inspecting 1 file.\n\n"
+      end
+    end
+
+    describe "#source_finished" do
+      it "writes valid source" do
+        subject.source_finished Source.new ""
+        output.to_s.should contain "."
+      end
+
+      it "writes invalid source" do
+        s = Source.new ""
+        s.error DummyRule.new, 3, "message"
+        subject.source_finished s
+        output.to_s.should contain "F"
+      end
+    end
+
+    describe "#finished" do
+      it "writes a final message" do
+        subject.finished [Source.new ""]
+        output.to_s.should contain "1 inspected, 0 failures."
+      end
+
+      it "writes the elapsed time" do
+        subject.finished [Source.new ""]
+        output.to_s.should contain "Finished in"
+      end
+    end
+  end
+end

src/ameba.cr

@@ -1,24 +1,25 @@
 require "./ameba/*"
 require "./ameba/ast/*"
 require "./ameba/rules/*"
+require "./ameba/formatter/*"
 
 module Ameba
   extend self
 
-  def run(formatter = DotFormatter.new)
+  def run(formatter = Formatter::BaseFormatter.new)
     run Dir["**/*.cr"].reject(&.starts_with? "lib/"), formatter
   end
 
-  def run(files, formatter : Formatter)
+  def run(files, formatter : Formatter::BaseFormatter)
     sources = files.map { |path| Source.new(File.read(path), path) }
 
-    reporter = Reporter.new formatter
+    formatter.started sources
-    reporter.start sources
     sources.each do |source|
+      formatter.source_started source
       catch(source)
-      reporter.report source
+      formatter.source_finished source
     end
-    reporter.try &.finish sources
+    formatter.finished sources
     sources
   end
 

src/ameba/formatter.cr

@@ -1,61 +0,0 @@
-module Ameba
-  abstract class Formatter
-    abstract def before(sources)
-    abstract def format(source : Source)
-    abstract def after(sources)
-  end
-
-  class Reporter
-    property formatter : Formatter
-
-    def initialize(@formatter : Formatter)
-    end
-
-    def start(sources)
-      puts formatter.before sources
-      puts "\n"
-    end
-
-    def report(source)
-      print formatter.format source
-    end
-
-    def finish(sources)
-      puts "\n\n"
-      puts formatter.after sources
-    end
-  end
-
-  class DotFormatter < Formatter
-    def before(sources)
-      if (len = sources.size) == 1
-        "Inspecting 1 file."
-      else
-        "Inspecting #{len} files."
-      end
-    end
-
-    def format(source : Source)
-      source.valid? ? ".".colorize(:green) : "F".colorize(:red)
-    end
-
-    def after(sources)
-      String.build do |mes|
-        failures = sources.select { |s| s.errors.any? }
-        l = failures.map { |f| f.errors.size }.sum
-
-        failures.each do |failure|
-          failure.errors.each do |error|
-            mes << "#{failure.path}:#{error.pos}\n".colorize(:cyan)
-            mes << "#{error.rule.name}: #{error.message}".colorize(:red)
-            mes << "\n\n"
-          end
-        end
-
-        color = l == 0 ? :green : :red
-        mes << "#{sources.size} inspected, #{l} failure#{"s" if l != 1}."
-          .colorize(color)
-      end
-    end
-  end
-end

src/ameba/formatter/base_formatter.cr

@@ -0,0 +1,17 @@
+module Ameba::Formatter
+  class BaseFormatter
+    # allow other IOs
+    getter output : IO::FileDescriptor | IO::Memory
+
+    def initialize(@output = STDOUT)
+    end
+
+    def started(sources); end
+
+    def source_finished(source : Source); end
+
+    def source_started(source : Source); end
+
+    def finished(sources); end
+  end
+end

src/ameba/formatter/dot_formatter.cr

@@ -0,0 +1,75 @@
+module Ameba::Formatter
+  class DotFormatter < BaseFormatter
+    @started_at : Time?
+
+    def started(sources)
+      @started_at = Time.now # Time.monotonic
+
+      output << started_message(sources.size)
+    end
+
+    def source_finished(source : Source)
+      sym = source.valid? ? ".".colorize(:green) : "F".colorize(:red)
+      output << sym
+      output.flush
+    end
+
+    def finished(sources)
+      output << "\n\n"
+      failed_sources = sources.reject { |s| s.valid? }
+
+      failed_sources.each do |source|
+        source.errors.each do |error|
+          output << "#{source.path}:#{error.pos}\n".colorize(:cyan)
+          output << "#{error.rule.name}: #{error.message}\n\n".colorize(:red)
+        end
+      end
+
+      output << finished_in_message(@started_at, Time.now) # Time.monotonic
+      output << final_message(sources, failed_sources)
+    end
+
+    private def started_message(size)
+      if size == 1
+        "Inspecting 1 file.\n\n"
+      else
+        "Inspecting #{size} files.\n\n"
+      end
+    end
+
+    private def finished_in_message(started, finished)
+      if started && finished
+        "Finished in #{to_human(finished - started)} \n\n"
+      end
+    end
+
+    private def to_human(span : Time::Span)
+      total_milliseconds = span.total_milliseconds
+      if total_milliseconds < 1
+        return "#{(span.total_milliseconds * 1_000).round.to_i} microseconds"
+      end
+
+      total_seconds = span.total_seconds
+      if total_seconds < 1
+        return "#{span.total_milliseconds.round(2)} milliseconds"
+      end
+
+      if total_seconds < 60
+        return "#{total_seconds.round(2)} seconds"
+      end
+
+      minutes = span.minutes
+      seconds = span.seconds
+      "#{minutes}:#{seconds < 10 ? "0" : ""}#{seconds} minutes"
+    end
+
+    private def final_message(sources, failed_sources)
+      total = sources.size
+      failures = failed_sources.map { |f| f.errors.size }.sum
+      color = failures == 0 ? :green : :red
+      s = failures != 1 ? "s" : ""
+
+      "#{total} inspected, #{failures} failure#{s}.\n".colorize color
+    end
+  end
+end

src/cli.cr

@@ -1,6 +1,8 @@
 require "option_parser"
 require "./ameba"
 
+formatter = Ameba::Formatter::DotFormatter
+
 OptionParser.parse(ARGV) do |parser|
   parser.banner = "Usage: ameba [options]"
 
@@ -13,6 +15,12 @@ OptionParser.parse(ARGV) do |parser|
     puts parser
     exit 0
   end
+
+  parser.on("-s", "--silent", "Disable output") do
+    formatter = Ameba::Formatter::BaseFormatter
+  end
 end
 
-exit(1) unless Ameba.run.all? &.valid?
+files = Dir["**/*.cr"]
+
+exit(1) unless Ameba.run(files, formatter.new).all? &.valid?