From 4d76fe921ca2660ecfebb683051d6676e2e6c829 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Tue, 24 Sep 2024 01:24:24 -1000 Subject: [PATCH] Support multiple formatters (#538) 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). --- README.md | 30 ++++++++-------- docs/github_actions.md | 7 ++-- lib/dialyxir/dialyzer.ex | 59 +++++++++++++++++--------------- lib/dialyxir/formatter.ex | 8 +++-- lib/mix/tasks/dialyzer.ex | 19 +++++----- mix.exs | 7 ++-- test/dialyxir/formatter_test.exs | 37 ++++++++++++++------ test/mix/tasks/dialyzer_test.exs | 10 ++++++ 8 files changed, 108 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index 53934f3..25e88ee 100644 --- a/README.md +++ b/README.md @@ -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 ` - 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. diff --git a/docs/github_actions.md b/docs/github_actions.md index dc863fc..45cba27 100644 --- a/docs/github_actions.md +++ b/docs/github_actions.md @@ -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 # ... -``` \ No newline at end of file +``` diff --git a/lib/dialyxir/dialyzer.ex b/lib/dialyxir/dialyzer.ex index 90e1cc1..9171249 100644 --- a/lib/dialyxir/dialyzer.ex +++ b/lib/dialyxir/dialyzer.ex @@ -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") @@ -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} -> @@ -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 diff --git a/lib/dialyxir/formatter.ex b/lib/dialyxir/formatter.ex index e1c9a47..6c9bac3 100644 --- a/lib/dialyxir/formatter.ex +++ b/lib/dialyxir/formatter.ex @@ -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, @@ -31,7 +31,7 @@ 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) @@ -39,7 +39,9 @@ defmodule Dialyxir.Formatter do 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?) diff --git a/lib/mix/tasks/dialyzer.ex b/lib/mix/tasks/dialyzer.ex index 768935f..b545aaf 100644 --- a/lib/mix/tasks/dialyzer.ex +++ b/lib/mix/tasks/dialyzer.ex @@ -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 ` - 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 @@ -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 @@ -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]}, diff --git a/mix.exs b/mix.exs index ab06c9b..baddc93 100644 --- a/mix.exs +++ b/mix.exs @@ -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] ], @@ -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 diff --git a/test/dialyxir/formatter_test.exs b/test/dialyxir/formatter_test.exs index 030e89c..bb75883 100644 --- a/test/dialyxir/formatter_test.exs +++ b/test/dialyxir/formatter_test.exs @@ -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 @@ -45,7 +46,7 @@ defmodule Dialyxir.FormatterTest do ], Project, [], - unquote(formatter) + [unquote(formatter)] ) assert message =~ unquote(message) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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}, @@ -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 diff --git a/test/mix/tasks/dialyzer_test.exs b/test/mix/tasks/dialyzer_test.exs index 4500eb0..01b2434 100644 --- a/test/mix/tasks/dialyzer_test.exs +++ b/test/mix/tasks/dialyzer_test.exs @@ -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