Skip to content

Commit

Permalink
Support multiple formatters (#538)
Browse files Browse the repository at this point in the history
Fixes #530

One scenario where multiple formatters is helpful is on CI where you want to use the GitHub
formatter along with a more verbose formatter to see more about the specific failures (especially
when viewing the CI logs directly).
  • Loading branch information
axelson authored Sep 24, 2024
1 parent d8cb107 commit 4d76fe9
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 69 deletions.
30 changes: 15 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,21 @@ 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.
* `--list-unused-filters` - list unused ignore filters useful for CI. do
not use with `mix do`.
* `--plt` - only build the required PLT(s) and exit
* `--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. (default)
* `--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 in {file, warning} format for Elixir Format ignore file.
* `--format ignore_file_strict` - format the warnings in {file, short_description} format for Elixir Format ignore file.
* `--quiet` - suppress all informational messages.
* `--quiet-with-result` - suppress all informational messages except for the final result message
* `--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.
* `--list-unused-filters` - list unused ignore filters useful for CI. do not use with `mix do`.
* `--plt` - only build the requir ed PLT(s) and exit.
* `--format <name>` - Specify the format for the warnings, can be specified multiple times to print warnings multiple times in different output formats. Defaults to `dialyxir`.
* `--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. (default)
* `--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 in {file, warning} format for Elixir Format ignore file.
* `--format ignore_file_strict` - format the warnings in {file, short_description} format for Elixir Format ignore file.
* `--quiet` - suppress all informational messages.
* `--quiet-with-result` - suppress all informational messages except for the final result message.

Warning flags passed to this task are passed on to `:dialyzer` - e.g.

Expand Down
7 changes: 5 additions & 2 deletions docs/github_actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ steps:
priv/plts
- name: Run dialyzer
run: mix dialyzer --format github
# Two formats are included for ease of debugging and it is lightly recommended to use both, see https://github.com/jeremyjh/dialyxir/issues/530 for reasoning
# --format github is helpful to print the warnings in a way that GitHub understands and can place on the /files page of a PR
# --format dialyxir allows the raw GitHub actions logs to be useful because they have the full warning printed
run: mix dialyzer --format github --format dialyxir

# ...
```
```
59 changes: 31 additions & 28 deletions lib/dialyxir/dialyzer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,25 @@ defmodule Dialyxir.Dialyzer do
:quiet_with_result
]

@default_formatter Dialyxir.Formatter.Dialyxir

def run(args, filterer) do
try do
{split, args} = Keyword.split(args, @dialyxir_args)

quiet_with_result? = split[:quiet_with_result]

formatter =
cond do
split[:format] == "dialyzer" ->
Dialyxir.Formatter.Dialyzer

split[:format] == "dialyxir" ->
Dialyxir.Formatter.Dialyxir

split[:format] == "github" ->
Dialyxir.Formatter.Github

split[:format] == "ignore_file" ->
Dialyxir.Formatter.IgnoreFile

split[:format] == "ignore_file_strict" ->
Dialyxir.Formatter.IgnoreFileStrict

split[:format] == "raw" ->
Dialyxir.Formatter.Raw

split[:format] == "short" ->
Dialyxir.Formatter.Short

split[:raw] ->
Dialyxir.Formatter.Raw
raw_formatters =
if split[:raw] do
Enum.uniq(split[:format] ++ ["raw"])
else
split[:format]
end

true ->
Dialyxir.Formatter.Dialyxir
formatters =
case raw_formatters do
[] -> [@default_formatter]
raw_formatters -> Enum.map(raw_formatters, &parse_formatter/1)
end

info("Starting Dialyzer")
Expand All @@ -66,7 +51,7 @@ defmodule Dialyxir.Dialyzer do
result,
filterer,
filter_map_args,
formatter,
formatters,
quiet_with_result?
) do
{:ok, formatted_warnings, :no_unused_filters} ->
Expand All @@ -83,6 +68,24 @@ defmodule Dialyxir.Dialyzer do
{:error, ":dialyzer.run error: " <> Chars.to_string(msg)}
end
end

defp parse_formatter("dialyzer"), do: Dialyxir.Formatter.Dialyzer
defp parse_formatter("dialyxir"), do: Dialyxir.Formatter.Dialyxir
defp parse_formatter("github"), do: Dialyxir.Formatter.Github
defp parse_formatter("ignore_file"), do: Dialyxir.Formatter.IgnoreFile
defp parse_formatter("ignore_file_string"), do: Dialyxir.Formatter.IgnoreFileStrict
defp parse_formatter("raw"), do: Dialyxir.Formatter.Raw
defp parse_formatter("short"), do: Dialyxir.Formatter.Short

defp parse_formatter(unknown) do
warning("""
Unrecognized formatter #{unknown} received. \
Known formatters are dialyzer, dialyxir, github, ignore_file, ignore_file_string, raw, and short. \
Falling back to dialyxir.
""")

@default_formatter
end
end

@success_return_code 0
Expand Down
8 changes: 5 additions & 3 deletions lib/dialyxir/formatter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ defmodule Dialyxir.Formatter do
"done in #{minutes}m#{seconds}s"
end

@spec format_and_filter([tuple], module, Keyword.t(), t(), boolean()) :: tuple
@spec format_and_filter([tuple], module, Keyword.t(), [t()], boolean()) :: tuple
def format_and_filter(
warnings,
filterer,
Expand All @@ -31,15 +31,17 @@ defmodule Dialyxir.Formatter do
quiet_with_result? \\ false
)

def format_and_filter(warnings, filterer, filter_map_args, formatter, quiet_with_result?) do
def format_and_filter(warnings, filterer, filter_map_args, formatters, quiet_with_result?) do
filter_map = filterer.filter_map(filter_map_args)

{filtered_warnings, filter_map} = filter_warnings(warnings, filterer, filter_map)

formatted_warnings =
filtered_warnings
|> filter_legacy_warnings(filterer)
|> Enum.map(&formatter.format/1)
|> Enum.flat_map(fn legacy_warning ->
Enum.map(formatters, & &1.format(legacy_warning))
end)
|> Enum.uniq()

show_count_skipped(warnings, formatted_warnings, filter_map, quiet_with_result?)
Expand Down
19 changes: 10 additions & 9 deletions lib/mix/tasks/dialyzer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ defmodule Mix.Tasks.Dialyzer do
* `--list-unused-filters` - list unused ignore filters useful for CI. do
not use with `mix do`.
* `--plt` - only build the required PLT(s) and exit
* `--format short` - format the warnings in a compact 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
* `--format github` - format the warnings in the Github Actions message format
* `--format ignore_file` - format the warnings in {file, warning} format for Elixir Format ignore file
* `--format ignore_file_strict` - format the warnings in {file, short_description} format for Elixir Format ignore file.
* `--format <name>` - Specify the format for the warnings, can be specified multiple times to print warnings multiple times in different output formats. Defaults to `dialyxir`.
* `--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 (default)
* `--format dialyzer` - format the warnings in the original Dialyzer format
* `--format github` - format the warnings in the Github Actions message format
* `--format ignore_file` - format the warnings in {file, warning} format for Elixir Format ignore file
* `--format ignore_file_strict` - format the warnings in {file, short_description} format for Elixir Format ignore file.
* `--quiet` - suppress all informational messages
* `--quiet-with-result` - suppress all informational messages except for the final result message
Expand Down Expand Up @@ -154,7 +155,7 @@ defmodule Mix.Tasks.Dialyzer do
quiet: :boolean,
quiet_with_result: :boolean,
raw: :boolean,
format: :string
format: [:string, :keep]
)

def run(args) do
Expand Down Expand Up @@ -265,7 +266,7 @@ defmodule Mix.Tasks.Dialyzer do
{:init_plt, String.to_charlist(Project.plt_file())},
{:files, Project.dialyzer_files()},
{:warnings, dialyzer_warnings(dargs)},
{:format, opts[:format]},
{:format, Keyword.get_values(opts, :format)},
{:raw, opts[:raw]},
{:list_unused_filters, opts[:list_unused_filters]},
{:ignore_exit_status, opts[:ignore_exit_status]},
Expand Down
7 changes: 5 additions & 2 deletions mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ defmodule Dialyxir.Mixfile do
deps: deps(),
aliases: [test: "test --no-start"],
dialyzer: [
plt_apps: [:dialyzer, :elixir, :kernel, :mix, :stdlib, :erlex],
plt_apps: [:dialyzer, :elixir, :kernel, :mix, :stdlib, :erlex, :logger],
ignore_warnings: ".dialyzer_ignore.exs",
flags: [:unmatched_returns, :error_handling, :underspecs]
],
Expand All @@ -33,7 +33,10 @@ defmodule Dialyxir.Mixfile do
end

def application do
[mod: {Dialyxir, []}, extra_applications: [:dialyzer, :crypto, :mix, :erts, :syntax_tools]]
[
mod: {Dialyxir, []},
extra_applications: [:dialyzer, :crypto, :mix, :erts, :syntax_tools, :logger]
]
end

defp description do
Expand Down
37 changes: 27 additions & 10 deletions test/dialyxir/formatter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ defmodule Dialyxir.FormatterTest do
alias Dialyxir.Formatter
alias Dialyxir.Formatter.Dialyxir, as: DialyxirFormatter
alias Dialyxir.Formatter.Dialyzer, as: DialyzerFormatter
alias Dialyxir.Formatter.Github, as: GithubFormatter
alias Dialyxir.Formatter.Short, as: ShortFormatter
alias Dialyxir.Formatter.IgnoreFileStrict, as: IgnoreFileStrictFormatter
alias Dialyxir.Project
Expand Down Expand Up @@ -45,7 +46,7 @@ defmodule Dialyxir.FormatterTest do
],
Project,
[],
unquote(formatter)
[unquote(formatter)]
)

assert message =~ unquote(message)
Expand All @@ -67,7 +68,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore, fn ->
{:error, remaining, _unused_filters_present} =
Formatter.format_and_filter(warnings, Project, [], ShortFormatter)
Formatter.format_and_filter(warnings, Project, [], [ShortFormatter])

assert remaining == []
end)
Expand All @@ -85,7 +86,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore_strict, fn ->
{:ok, remaining, :no_unused_filters} =
Formatter.format_and_filter(warnings, Project, [], IgnoreFileStrictFormatter)
Formatter.format_and_filter(warnings, Project, [], [IgnoreFileStrictFormatter])

assert remaining == []
end)
Expand All @@ -98,7 +99,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore, fn ->
{:error, [remaining], _} =
Formatter.format_and_filter([warning], Project, [], ShortFormatter)
Formatter.format_and_filter([warning], Project, [], [ShortFormatter])

assert remaining =~ ~r/different_file.* no local return/
end)
Expand All @@ -111,7 +112,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore, fn ->
{:error, remaining, _unused_filters_present} =
Formatter.format_and_filter([warning], Project, [], ShortFormatter)
Formatter.format_and_filter([warning], Project, [], [ShortFormatter])

assert remaining == []
end)
Expand All @@ -126,7 +127,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore, fn ->
assert {:warn, [], {:unused_filters_present, warning}} =
Formatter.format_and_filter([warning], Project, filter_args, :dialyxir)
Formatter.format_and_filter([warning], Project, filter_args, [:dialyxir])

assert warning =~ "Unused filters:"
end)
Expand All @@ -141,7 +142,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore, fn ->
{:error, [], {:unused_filters_present, error}} =
Formatter.format_and_filter([warning], Project, filter_args, :dialyxir)
Formatter.format_and_filter([warning], Project, filter_args, [:dialyxir])

assert error =~ "Unused filters:"
end)
Expand All @@ -156,7 +157,7 @@ defmodule Dialyxir.FormatterTest do

in_project(:ignore, fn ->
assert {:warn, [], {:unused_filters_present, warning}} =
Formatter.format_and_filter([warning], Project, filter_args, :dialyxir)
Formatter.format_and_filter([warning], Project, filter_args, [:dialyxir])

refute warning =~ "Unused filters:"
end)
Expand All @@ -169,12 +170,28 @@ defmodule Dialyxir.FormatterTest do
{:warn_matching, {~c"a/file.ex", 17}, {:pattern_match, [~c"pattern 'ok'", ~c"'error'"]}}

in_project(:ignore_string, fn ->
assert Formatter.format_and_filter([warning], Project, [], :dialyzer) ==
assert Formatter.format_and_filter([warning], Project, [], [:dialyzer]) ==
{:ok, [], :no_unused_filters}
end)
end
end

describe "multiple formatters" do
test "short and github" do
warning =
{:warn_return_no_exit, {~c"a/different_file.ex", 17},
{:no_return, [:only_normal, :format_long, 1]}}

in_project(:ignore, fn ->
{:error, [short_formatted, github_formatted], _} =
Formatter.format_and_filter([warning], Project, [], [ShortFormatter, GithubFormatter])

assert short_formatted =~ ~r/different_file.* no local return/
assert github_formatted =~ ~r/^::warning file=a\/different_file\.ex.* no local return/
end)
end
end

test "listing unused filter behaves the same for different formats" do
warnings = [
{:warn_return_no_exit, {~c"a/regex_file.ex", 17},
Expand All @@ -194,7 +211,7 @@ defmodule Dialyxir.FormatterTest do
for format <- [ShortFormatter, DialyxirFormatter, DialyzerFormatter] do
in_project(:ignore, fn ->
capture_io(fn ->
result = Formatter.format_and_filter(warnings, Project, filter_args, format)
result = Formatter.format_and_filter(warnings, Project, filter_args, [format])

assert {:error, [warning], {:unused_filters_present, unused}} = result
assert warning =~ expected_warning
Expand Down
10 changes: 10 additions & 0 deletions test/mix/tasks/dialyzer_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,14 @@ defmodule Mix.Tasks.DialyzerTest do
assert result =~
~r/Total errors: ., Skipped: ., Unnecessary Skips: .\ndone \(passed successfully\)\n/
end

@tag :output_tests
test "Warning is printed when unknown format is requested" do
args = ["dialyzer", "--format", "foo"]
env = [{"MIX_ENV", "prod"}]
{result, 0} = System.cmd("mix", args, env: env)

assert result =~
"Unrecognized formatter foo received. Known formatters are dialyzer, dialyxir, github, ignore_file, ignore_file_string, raw, and short. Falling back to dialyxir."
end
end

0 comments on commit 4d76fe9

Please sign in to comment.