Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support multiple formatters #538

Merged
merged 6 commits into from
Sep 24, 2024
Merged

Support multiple formatters #538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c329c1b
Support multiple formatters
axelson Jul 24, 2024
4c3297c
Update docs
axelson Sep 20, 2024
804c271
Don't use Logger
axelson Sep 20, 2024
baaa24d
Add test for warning output
axelson Sep 20, 2024
42c0570
formatting
axelson Sep 20, 2024
173db23
Merge branch 'master' into multiple-formatters
jeremyjh Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 27 additions & 28 deletions lib/dialyxir/dialyzer.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
defmodule Dialyxir.Dialyzer do
import Dialyxir.Output
require Logger
alias String.Chars
alias Dialyxir.Formatter
alias Dialyxir.Project
Expand All @@ -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")
Expand All @@ -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} ->
Expand All @@ -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.")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an issue with depending on :logger, but we already use the Mix.shell functions for text output (through Dialyxir.Output module) and Output.warning should color the text - is there a particular reason to use Logger instead ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, updated to use Dialyxir.Output.warning in baaa24d

@default_formatter
end
end

@success_return_code 0
Expand Down
8 changes: 5 additions & 3 deletions lib/dialyxir/formatter.ex
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions lib/mix/tasks/dialyzer.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]},
Expand Down
7 changes: 5 additions & 2 deletions mix.exs
View file
Open in desktop
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
View file
Open in desktop
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
Loading