From 08ef5cf29f19efe5aa35cfea45d306309d11c184 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Wed, 24 Jul 2024 07:45:00 -0500 Subject: [PATCH] Support multiple formatters 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). --- lib/dialyxir/dialyzer.ex | 55 ++++++++++++++++---------------- lib/dialyxir/formatter.ex | 8 +++-- lib/mix/tasks/dialyzer.ex | 4 +-- mix.exs | 4 +-- test/dialyxir/formatter_test.exs | 37 +++++++++++++++------ 5 files changed, 63 insertions(+), 45 deletions(-) diff --git a/lib/dialyxir/dialyzer.ex b/lib/dialyxir/dialyzer.ex index 90e1cc1..2135ea8 100644 --- a/lib/dialyxir/dialyzer.ex +++ b/lib/dialyxir/dialyzer.ex @@ -1,5 +1,6 @@ defmodule Dialyxir.Dialyzer do import Dialyxir.Output + require Logger alias String.Chars alias Dialyxir.Formatter alias Dialyxir.Project @@ -14,40 +15,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 +52,7 @@ defmodule Dialyxir.Dialyzer do result, filterer, filter_map_args, - formatter, + formatters, quiet_with_result? ) do {:ok, formatted_warnings, :no_unused_filters} -> @@ -83,6 +69,19 @@ 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 + Logger.warning("Unrecognized formatter #{unknown} received. Falling back to default.") + @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..8ab134c 100644 --- a/lib/mix/tasks/dialyzer.ex +++ b/lib/mix/tasks/dialyzer.ex @@ -154,7 +154,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 +265,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..e955eda 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,7 @@ 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