feat: Adds a github formatter (#463)

* refactor: Extracts formatter modules

* refactor: Uses a behaviour over string and atom matching

* feat: Adds a github formatter

* chore: Updates log level and documentation

* fix: Cleans up some warnings

Co-authored-by: Isaac Sanders <isanders@drwholdings.com>

README.md

@@ -49,14 +49,16 @@ mix dialyzer
 
 ### Command line options
 
-  * `--no-compile`         - do not compile even if needed.
-  * `--no-check`           - do not perform (quick) check to see if PLT needs to be updated.
-  * `--ignore-exit-status` - display warnings but do not halt the VM or return an exit status code.
-  *  `--format short`      - format the warnings in a compact format, suitable for ignore file using Elixir term format.
-  *  `--format raw`        - format the warnings in format returned before Dialyzer formatting.
-  *  `--format dialyxir`   - format the warnings in a pretty printed format.
-  *  `--format dialyzer`   - format the warnings in the original Dialyzer format, suitable for ignore file using simple string matches.
-  *  `--quiet`             - suppress all informational messages.
+  * `--no-compile`          - do not compile even if needed.
+  * `--no-check`            - do not perform (quick) check to see if PLT needs to be updated.
+  * `--ignore-exit-status`  - display warnings but do not halt the VM or return an exit status code.
+  *  `--format short`       - format the warnings in a compact format, suitable for ignore file using Elixir term format.
+  *  `--format raw`         - format the warnings in format returned before Dialyzer formatting.
+  *  `--format dialyxir`    - format the warnings in a pretty printed format.
+  *  `--format dialyzer`    - format the warnings in the original Dialyzer format, suitable for ignore file using simple string matches.
+  *  `--format github`      - format the warnings in the Github Actions message format.
+  *  `--format ignore_file` - format the warnings to be suitable for adding to Elixir Format ignore file.
+  *  `--quiet`              - suppress all informational messages.
 
 Warning flags passed to this task are passed on to `:dialyzer` - e.g.
 
@@ -133,7 +135,7 @@ cache:
         run: mix dialyzer --plt
 
       - name: Run dialyzer
-        run: mix dialyzer
+        run: mix dialyzer --format github
 
 ```
 

lib/dialyxir/dialyzer.ex

@@ -20,25 +20,28 @@ defmodule Dialyxir.Dialyzer do
         formatter =
           cond do
             split[:format] == "dialyzer" ->
-              :dialyzer
+              Dialyxir.Formatter.Dialyzer
 
             split[:format] == "dialyxir" ->
-              :dialyxir
+              Dialyxir.Formatter.Dialyxir
+
+            split[:format] == "github" ->
+              Dialyxir.Formatter.Github
 
             split[:format] == "ignore_file" ->
-              :ignore_file
+              Dialyxir.Formatter.IgnoreFile
 
             split[:format] == "raw" ->
-              :raw
+              Dialyxir.Formatter.Raw
 
             split[:format] == "short" ->
-              :short
+              Dialyxir.Formatter.Short
 
             split[:raw] ->
-              :raw
+              Dialyxir.Formatter.Raw
 
             true ->
-              :dialyxir
+              Dialyxir.Formatter.Dialyxir
           end
 
         info("Starting Dialyzer")

lib/dialyxir/formatter.ex

@@ -9,13 +9,19 @@ defmodule Dialyxir.Formatter do
 
   alias Dialyxir.FilterMap
 
+  @type warning() :: {tag :: term(), {file :: Path.t(), line :: pos_integer()}, {atom(), list()}}
+
+  @type t() :: module()
+
+  @callback format(warning()) :: String.t()
+
   def formatted_time(duration_us) do
     minutes = div(duration_us, 60_000_000)
     seconds = (rem(duration_us, 60_000_000) / 1_000_000) |> Float.round(2)
     "done in #{minutes}m#{seconds}s"
   end
 
-  @spec format_and_filter([tuple], module, Keyword.t(), atom) :: tuple
+  @spec format_and_filter([tuple], module, Keyword.t(), t()) :: tuple
   def format_and_filter(warnings, filterer, filter_map_args, formatter) do
     filter_map = filterer.filter_map(filter_map_args)
 
@@ -24,7 +30,7 @@ defmodule Dialyxir.Formatter do
     formatted_warnings =
       filtered_warnings
       |> filter_legacy_warnings(filterer)
-      |> Enum.map(&format_warning(&1, formatter))
+      |> Enum.map(&formatter.format/1)
       |> Enum.uniq()
 
     show_count_skipped(warnings, formatted_warnings, filter_map)
@@ -46,114 +52,6 @@ defmodule Dialyxir.Formatter do
     end
   end
 
-  defp format_warning(warning, :raw) do
-    inspect(warning, limit: :infinity)
-  end
-
-  defp format_warning(warning, :dialyzer) do
-    # OTP 22 uses indented output, but that's incompatible with dialyzer.ignore-warnings format.
-    # Can be disabled, but OTP 21 and older only accept an atom, so only disable on OTP 22+.
-    opts =
-      if String.to_integer(System.otp_release()) < 22,
-        do: :fullpath,
-        else: [{:filename_opt, :fullpath}, {:indent_opt, false}]
-
-    warning
-    |> :dialyzer.format_warning(opts)
-    |> String.Chars.to_string()
-    |> String.replace_trailing("\n", "")
-  end
-
-  defp format_warning({_tag, {file, line}, message}, :short) do
-    {warning_name, arguments} = message
-    base_name = Path.relative_to_cwd(file)
-
-    warning = warning(warning_name)
-    string = warning.format_short(arguments)
-
-    "#{base_name}:#{line}:#{warning_name} #{string}"
-  end
-
-  defp format_warning({_tag, {file, _line}, {warning_name, _arguments}}, :ignore_file) do
-    ~s({"#{file}", :#{warning_name}},)
-  end
-
-  defp format_warning(dialyzer_warning = {_tag, {file, line}, message}, :dialyxir) do
-    {warning_name, arguments} = message
-    base_name = Path.relative_to_cwd(file)
-
-    formatted =
-      try do
-        warning = warning(warning_name)
-        string = warning.format_long(arguments)
-
-        """
-        #{base_name}:#{line}:#{warning_name}
-        #{string}
-        """
-      rescue
-        e ->
-          message = """
-          Unknown error occurred: #{inspect(e)}
-          """
-
-          wrap_error_message(message, dialyzer_warning)
-      catch
-        {:error, :unknown_warning, warning_name} ->
-          message = """
-          Unknown warning:
-          #{inspect(warning_name)}
-          """
-
-          wrap_error_message(message, dialyzer_warning)
-
-        {:error, :lexing, warning} ->
-          message = """
-          Failed to lex warning:
-          #{inspect(warning)}
-          """
-
-          wrap_error_message(message, dialyzer_warning)
-
-        {:error, :parsing, failing_string} ->
-          message = """
-          Failed to parse warning:
-          #{inspect(failing_string)}
-          """
-
-          wrap_error_message(message, dialyzer_warning)
-
-        {:error, :pretty_printing, failing_string} ->
-          message = """
-          Failed to pretty print warning:
-          #{inspect(failing_string)}
-          """
-
-          wrap_error_message(message, dialyzer_warning)
-
-        {:error, :formatting, code} ->
-          message = """
-          Failed to format warning:
-          #{inspect(code)}
-          """
-
-          wrap_error_message(message, dialyzer_warning)
-      end
-
-    formatted <> String.duplicate("_", 80)
-  end
-
-  defp wrap_error_message(message, warning) do
-    """
-    Please file a bug in https://github.com/jeremyjh/dialyxir/issues with this message.
-
-    #{message}
-
-    Legacy warning:
-    #{format_warning(warning, :dialyzer)}
-    """
-  end
-
   defp show_count_skipped(warnings, filtered_warnings, filter_map) do
     warnings_count = Enum.count(warnings)
     filtered_warnings_count = Enum.count(filtered_warnings)
@@ -189,16 +87,6 @@ defmodule Dialyxir.Formatter do
     |> Enum.count()
   end
 
-  defp warning(warning_name) do
-    warnings = Dialyxir.Warnings.warnings()
-
-    if Map.has_key?(warnings, warning_name) do
-      Map.get(warnings, warning_name)
-    else
-      throw({:error, :unknown_warning, warning_name})
-    end
-  end
-
   defp filter_warnings(warnings, filterer, filter_map) do
     {warnings, filter_map} =
       Enum.map_reduce(warnings, filter_map, &filter_warning(filterer, &1, &2))
@@ -212,7 +100,7 @@ defmodule Dialyxir.Formatter do
       {skip?, matching_filters} =
         try do
           filterer.filter_warning?(
-            {to_string(file), warning_type, line, format_warning(warning, :short)},
+            {to_string(file), warning_type, line, Dialyxir.Formatter.Short.format(warning)},
             filter_map
           )
         rescue
@@ -242,7 +130,7 @@ defmodule Dialyxir.Formatter do
     Enum.reject(warnings, fn warning ->
       formatted_warnings =
         warning
-        |> format_warning(:dialyzer)
+        |> Dialyxir.Formatter.Dialyzer.format()
         |> List.wrap()
 
       Enum.empty?(filterer.filter_legacy_warnings(formatted_warnings))

lib/dialyxir/formatter/dialyxir.ex

@@ -0,0 +1,92 @@
+defmodule Dialyxir.Formatter.Dialyxir do
+  @moduledoc false
+
+  @behaviour Dialyxir.Formatter
+
+  @impl Dialyxir.Formatter
+  def format(dialyzer_warning = {_tag, {file, line}, message}) do
+    {warning_name, arguments} = message
+    base_name = Path.relative_to_cwd(file)
+
+    formatted =
+      try do
+        warning = warning(warning_name)
+        string = warning.format_long(arguments)
+
+        """
+        #{base_name}:#{line}:#{warning_name}
+        #{string}
+        """
+      rescue
+        e ->
+          message = """
+          Unknown error occurred: #{inspect(e)}
+          """
+
+          wrap_error_message(message, dialyzer_warning)
+      catch
+        {:error, :unknown_warning, warning_name} ->
+          message = """
+          Unknown warning:
+          #{inspect(warning_name)}
+          """
+
+          wrap_error_message(message, dialyzer_warning)
+
+        {:error, :lexing, warning} ->
+          message = """
+          Failed to lex warning:
+          #{inspect(warning)}
+          """
+
+          wrap_error_message(message, dialyzer_warning)
+
+        {:error, :parsing, failing_string} ->
+          message = """
+          Failed to parse warning:
+          #{inspect(failing_string)}
+          """
+
+          wrap_error_message(message, dialyzer_warning)
+
+        {:error, :pretty_printing, failing_string} ->
+          message = """
+          Failed to pretty print warning:
+          #{inspect(failing_string)}
+          """
+
+          wrap_error_message(message, dialyzer_warning)
+
+        {:error, :formatting, code} ->
+          message = """
+          Failed to format warning:
+          #{inspect(code)}
+          """
+
+          wrap_error_message(message, dialyzer_warning)
+      end
+
+    formatted <> String.duplicate("_", 80)
+  end
+
+  defp wrap_error_message(message, warning) do
+    """
+    Please file a bug in https://github.com/jeremyjh/dialyxir/issues with this message.
+
+    #{message}
+
+    Legacy warning:
+    #{Dialyxir.Formatter.Dialyzer.format(warning)}
+    """
+  end
+
+  defp warning(warning_name) do
+    warnings = Dialyxir.Warnings.warnings()
+
+    if Map.has_key?(warnings, warning_name) do
+      Map.get(warnings, warning_name)
+    else
+      throw({:error, :unknown_warning, warning_name})
+    end
+  end
+end

lib/dialyxir/formatter/dialyzer.ex

@@ -0,0 +1,20 @@
+defmodule Dialyxir.Formatter.Dialyzer do
+  @moduledoc false
+
+  @behaviour Dialyxir.Formatter
+
+  @impl Dialyxir.Formatter
+  def format(warning) do
+    # OTP 22 uses indented output, but that's incompatible with dialyzer.ignore-warnings format.
+    # Can be disabled, but OTP 21 and older only accept an atom, so only disable on OTP 22+.
+    opts =
+      if String.to_integer(System.otp_release()) < 22,
+        do: :fullpath,
+        else: [{:filename_opt, :fullpath}, {:indent_opt, false}]
+
+    warning
+    |> :dialyzer.format_warning(opts)
+    |> String.Chars.to_string()
+    |> String.replace_trailing("\n", "")
+  end
+end

lib/dialyxir/formatter/github.ex

@@ -0,0 +1,25 @@
+defmodule Dialyxir.Formatter.Github do
+  @moduledoc false
+
+  @behaviour Dialyxir.Formatter
+
+  @impl Dialyxir.Formatter
+  def format({_tag, {file, line}, {warning_name, arguments}}) do
+    base_name = Path.relative_to_cwd(file)
+
+    warning = warning(warning_name)
+    string = warning.format_short(arguments)
+
+    "::warning file=#{base_name},line=#{line},title=#{warning_name}::#{string}"
+  end
+
+  defp warning(warning_name) do
+    warnings = Dialyxir.Warnings.warnings()
+
+    if Map.has_key?(warnings, warning_name) do
+      Map.get(warnings, warning_name)
+    else
+      throw({:error, :unknown_warning, warning_name})
+    end
+  end
+end