Proof of concept for configurable formatters

USAGE.md

@@ -249,6 +249,32 @@ You can prefix local files with a dot to define them relative to `packwerk.yml`,
 You can also reference the name of a gem.
 
 ## Examples
+### Custom `OffensesFormatter`
+While `packwerk` ships with its own `OffensesFormatter`, the extension system provides a mechanism to configure your own to be used when `bin/packwerk check` is run.
+
+Firstly, you'll need to create an `OffensesFormatter` that implements the `Packwerk::OffensesFormatter` interface. You can use [`Packwerk::Formatters::OffensesFormatter`](lib/packwerk/formatters/offenses_formatter.rb) as a model for this. Then, in the `require` directive described above, you'll want to tell `packwerk` about it:
+```ruby
+# ./path/to/file.rb
+class MyOffensesFormatter
+  include Packwerk::OffensesFormatter
+  # implement the `OffensesFormatter` interface
+
+  def identifier
+    'my_offenses_formatter'
+  end
+end
+```
+
+Then in `packwerk.yml`, you can set the `formatter`:
+```yml
+formatter: my_offenses_formatter
+```
+
+You can also specify the formatter on the command line, as so:
+```
+bin/packwerk check --format my_offenses_formatter
+```
+
 ### Privacy Checker
 
 [`packwerk-extensions`](https://github.com/rubyatscale/packwerk-extensions) (originally extracted from `packwerk`) can be used to help define and enforce public API boundaries of a package. See the README.md for more details. To use this, add it to your `Gemfile` and then require it via `packwerk.yml`:

lib/packwerk.rb

@@ -65,6 +65,7 @@ module Formatters
     extend ActiveSupport::Autoload
 
     autoload :OffensesFormatter
+    autoload :OffensesFormatterPlain
     autoload :ProgressFormatter
   end
 

lib/packwerk/cli.rb

@@ -5,6 +5,10 @@
 
 module Packwerk
   # A command-line interface to Packwerk.
+  # Would it be better to refactor this to use `thor`, which is already in the Gemfile.lock
+  # via railties and other gems?
+  # That would allow the `format` option to be simpler and would help us not need to reinvent the wheel
+  # submit a PR for this?
   class Cli
     extend T::Sig
 
@@ -33,7 +37,7 @@ def initialize(
       @configuration = T.let(configuration || Configuration.from_path, Configuration)
       @progress_formatter = T.let(Formatters::ProgressFormatter.new(@out, style: style), Formatters::ProgressFormatter)
       @offenses_formatter = T.let(
-        offenses_formatter || Formatters::OffensesFormatter.new(style: @style),
+        offenses_formatter || @configuration.offenses_formatter,
         OffensesFormatter
       )
     end

lib/packwerk/configuration.rb

@@ -6,6 +6,8 @@
 
 module Packwerk
   class Configuration
+    extend T::Sig
+
     class << self
       def from_path(path = Dir.pwd)
         raise ArgumentError, "#{File.expand_path(path)} does not exist" unless File.exist?(path)
@@ -19,6 +21,14 @@ def from_path(path = Dir.pwd)
         end
       end
 
+      def load_default_formatters
+        # Ensure default offenses formatters are loaded
+        # All this is done is explicitly reference these so that the `included` hook is
+        # called on Packwerk::OffensesFormatter so that these classes are registered
+        Formatters::OffensesFormatter # rubocop:disable Lint/Void
+        Formatters::OffensesFormatterPlain
+      end
+
       private
 
       def from_packwerk_config(path)
@@ -49,6 +59,9 @@ def initialize(configs = {}, config_path: nil)
       @cache_directory = Pathname.new(configs["cache_directory"] || "tmp/cache/packwerk")
       @config_path = config_path
 
+      self.class.load_default_formatters
+      @offenses_formatter_identifier = configs["formatter"] || Formatters::OffensesFormatter::IDENTIFIER
+
       if configs.key?("require")
         configs["require"].each do |require_directive|
           ExtensionLoader.load(require_directive, @root_path)
@@ -64,6 +77,11 @@ def parallel?
       @parallel
     end
 
+    sig { returns(OffensesFormatter) }
+    def offenses_formatter
+      OffensesFormatter.find(@offenses_formatter_identifier)
+    end
+
     def cache_enabled?
       @cache_enabled
     end

lib/packwerk/formatters/offenses_formatter.rb

@@ -6,11 +6,13 @@ module Formatters
     class OffensesFormatter
       include Packwerk::OffensesFormatter
 
+      IDENTIFIER = T.let("default", String)
+
       extend T::Sig
 
-      sig { params(style: OutputStyle).void }
-      def initialize(style: OutputStyles::Plain.new)
-        @style = style
+      sig { returns(OutputStyle) }
+      def style
+        @style ||= T.let(Packwerk::OutputStyles::Coloured.new, T.nilable(Packwerk::OutputStyles::Coloured))
       end
 
       sig { override.params(offenses: T::Array[T.nilable(Offense)]).returns(String) }
@@ -32,13 +34,18 @@ def show_stale_violations(offense_collection, fileset)
         end
       end
 
+      sig { override.returns(String) }
+      def identifier
+        IDENTIFIER
+      end
+
       private
 
       sig { params(offenses: T::Array[T.nilable(Offense)]).returns(String) }
       def offenses_list(offenses)
         offenses
           .compact
-          .map { |offense| offense.to_s(@style) }
+          .map { |offense| offense.to_s(style) }
           .join("\n")
       end
 

lib/packwerk/formatters/offenses_formatter_plain.rb

@@ -0,0 +1,23 @@
+# typed: strict
+# frozen_string_literal: true
+
+module Packwerk
+  module Formatters
+    class OffensesFormatterPlain < OffensesFormatter
+      include Packwerk::OffensesFormatter
+      IDENTIFIER = T.let("plain", String)
+
+      extend T::Sig
+
+      sig { returns(OutputStyle) }
+      def style
+        @style ||= T.let(Packwerk::OutputStyles::Plain.new, T.nilable(Packwerk::OutputStyle))
+      end
+
+      sig { override.returns(String) }
+      def identifier
+        IDENTIFIER
+      end
+    end
+  end
+end

lib/packwerk/offenses_formatter.rb

@@ -6,7 +6,39 @@ module OffensesFormatter
     extend T::Sig
     extend T::Helpers
 
-    interface!
+    abstract!
+
+    class << self
+      extend T::Sig
+
+      sig { params(base: Class).void }
+      def included(base)
+        @formatters ||= T.let(@formatters, T.nilable(T::Array[Class]))
+        @formatters ||= []
+        @formatter_by_identifier = nil
+        @formatters << base
+      end
+
+      sig { returns(T::Array[OffensesFormatter]) }
+      def all
+        T.unsafe(@formatters).map(&:new)
+      end
+
+      sig { params(identifier: String).returns(OffensesFormatter) }
+      def find(identifier)
+        formatter_by_identifier(identifier)
+      end
+
+      private
+
+      sig { params(name: String).returns(OffensesFormatter) }
+      def formatter_by_identifier(name)
+        @formatter_by_identifier ||= T.let(@formatter_by_identifier,
+          T.nilable(T::Hash[String, T.nilable(OffensesFormatter)]))
+        @formatter_by_identifier ||= OffensesFormatter.all.map { |c| [c.identifier, c] }.to_h
+        T.must(@formatter_by_identifier[name])
+      end
+    end
 
     sig { abstract.params(offenses: T::Array[T.nilable(Offense)]).returns(String) }
     def show_offenses(offenses)
@@ -15,5 +47,9 @@ def show_offenses(offenses)
     sig { abstract.params(offense_collection: Packwerk::OffenseCollection, for_files: T::Set[String]).returns(String) }
     def show_stale_violations(offense_collection, for_files)
     end
+
+    sig { abstract.returns(String) }
+    def identifier
+    end
   end
 end

lib/packwerk/parse_run.rb

@@ -16,19 +16,20 @@ class ParseRun
       params(
         relative_file_set: FilesForProcessing::RelativeFileSet,
         configuration: Configuration,
+        offenses_formatter: T.nilable(OffensesFormatter),
         progress_formatter: Formatters::ProgressFormatter,
-        offenses_formatter: OffensesFormatter,
       ).void
     end
     def initialize(
       relative_file_set:,
       configuration:,
-      progress_formatter: Formatters::ProgressFormatter.new(StringIO.new),
-      offenses_formatter: Formatters::OffensesFormatter.new
+      offenses_formatter: nil,
+      progress_formatter: Formatters::ProgressFormatter.new(StringIO.new)
     )
+
       @configuration = configuration
       @progress_formatter = progress_formatter
-      @offenses_formatter = offenses_formatter
+      @offenses_formatter = T.let(offenses_formatter || configuration.offenses_formatter, Packwerk::OffensesFormatter)
       @relative_file_set = relative_file_set
     end
 

test/fixtures/extended/config/my_local_extension.rb

@@ -1,2 +1,18 @@
 module MyLocalExtension
 end
+
+class MyOffensesFormatter
+  include Packwerk::OffensesFormatter
+
+  def show_offenses(offenses)
+    ["hi i am a custom offense formatter", *offenses].join("\n")
+  end
+
+  def show_stale_violations(_offense_collection, _fileset)
+    "stale violations report"
+  end
+
+  def identifier
+    'my_offenses_formatter'
+  end
+end

test/fixtures/extended/packwerk.yml

@@ -2,3 +2,4 @@ parallel: false
 require:
   - ./config/my_local_extension
   - my_gem_extension
+formatter: my_offenses_formatter

test/fixtures/minimal/packwerk.yml

@@ -1,2 +1,4 @@
 include:
   - "**/*.rb"
+
+formatter: plain

test/support/application_fixture_helper.rb

@@ -13,6 +13,10 @@ def setup_application_fixture
     @old_working_dir = Dir.pwd
   end
 
+  def remove_extensions
+    Object.send(:remove_const, :MyLocalExtension) if defined?(MyLocalExtension)
+  end
+
   def teardown_application_fixture
     Dir.chdir(@old_working_dir)
     FileUtils.remove_entry(@app_dir, true) if using_template?

test/unit/cli_test.rb

@@ -139,6 +139,10 @@ def show_offenses(offenses)
         def show_stale_violations(_offense_collection, _fileset)
           "stale violations report"
         end
+
+        def identifier
+          "custom"
+        end
       end
 
       file_path = "path/of/exile.rb"
@@ -168,5 +172,37 @@ def show_stale_violations(_offense_collection, _fileset)
 
       refute success
     end
+
+    test "#execute_command using a custom offenses class loaded in via packwerk.yml" do
+      use_template(:extended)
+
+      file_path = "path/of/exile.rb"
+      violation_message = "This is a violation of code health."
+      offense = Offense.new(file: file_path, message: violation_message)
+
+      RunContext.any_instance.stubs(:process_file)
+        .returns([offense])
+
+      cli = nil
+      string_io = StringIO.new
+      mock_require_method = ->(required_thing) do
+        next unless required_thing.include?("my_local_extension")
+
+        require required_thing
+      end
+      ExtensionLoader.stub(:require, mock_require_method) do
+        cli = ::Packwerk::Cli.new(out: string_io)
+      end
+
+      ::Packwerk::FilesForProcessing.stubs(fetch: Set.new([file_path]))
+
+      success = cli.execute_command(["check", file_path])
+
+      assert_includes string_io.string, "hi i am a custom offense formatter"
+      assert_includes string_io.string, "stale violations report"
+      assert_includes string_io.string, violation_message
+
+      refute success
+    end
   end
 end

test/unit/configuration_test.rb

@@ -13,7 +13,6 @@ class ConfigurationTest < Minitest::Test
 
     teardown do
       teardown_application_fixture
-      Object.send(:remove_const, :MyLocalExtension) if defined?(MyLocalExtension)
     end
 
     test ".from_path raises ArgumentError if path does not exist" do
@@ -77,6 +76,8 @@ class ConfigurationTest < Minitest::Test
         Configuration.from_path
       end
       assert defined?(MyLocalExtension)
+
+      remove_extensions
     end
 
     test "require works when referencing a gem" do
@@ -91,6 +92,8 @@ class ConfigurationTest < Minitest::Test
       end
 
       assert_includes(required_things, "my_gem_extension")
+
+      remove_extensions
     end
   end
 end

test/unit/formatters/offenses_formatter_test.rb

@@ -7,7 +7,7 @@ module Packwerk
   module Formatters
     class OffensesFormatterTest < Minitest::Test
       setup do
-        @offenses_formatter = OffensesFormatter.new
+        @offenses_formatter = OffensesFormatterPlain.new
       end
 
       test "#show_offenses prints No offenses detected when there are no offenses" do

test/unit/parse_run_test.rb

@@ -342,9 +342,10 @@ def something
       YML
 
       out = StringIO.new
+      Configuration.load_default_formatters
       parse_run = Packwerk::ParseRun.new(
         relative_file_set: Set.new([file_to_check]),
-        configuration: Configuration.new({ "parallel" => false }),
+        configuration: Configuration.new({ "parallel" => false, "formatter" => "plain" }),
         progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out)
       )