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.

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

Support multiple formatters #538

Merged
merged 6 commits into from
Sep 24, 2024
Merged

Conversation

axelson
Copy link
Contributor

@axelson axelson commented Jul 24, 2024

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).

I add :logger to give the user a warning message if they provide an invalid name for a formatter but I can remove the warning if there's a specific reason to not depend on logger.

Example run:

> mix dialyzer --format github --format dialyxir
Finding suitable PLTs
Checking PLT...
[:compiler, :crypto, :dialyxir, :dialyzer, :elixir, :erlex, :erts, :kernel, :logger, :mix, :stdlib, :syntax_tools, :weather]
PLT is up to date!
No :ignore_warnings opt specified in mix.exs and default does not exist.

Starting Dialyzer
[
  check_plt: false,
  init_plt: ~c"/Users/jasonaxelson/dev/tmp/weather/_build/dev/dialyxir_erlang-26.2.5_elixir-1.17.2_deps-dev.plt",
  files: [~c"/Users/jasonaxelson/dev/tmp/weather/_build/dev/lib/weather/ebin/Elixir.BusinessLogic.beam",
   ~c"/Users/jasonaxelson/dev/tmp/weather/_build/dev/lib/weather/ebin/Elixir.Weather.beam",
   ~c"/Users/jasonaxelson/dev/tmp/weather/_build/dev/lib/weather/ebin/Elixir.WeatherAPI.beam",
   ~c"/Users/jasonaxelson/dev/tmp/weather/_build/dev/lib/weather/ebin/Elixir.WeatherImpl.beam"],
  warnings: [:unknown]
]
Total errors: 2, Skipped: -2, Unnecessary Skips: 0
done in 0m3.44s
::warning file=lib/weather_impl.ex,line=5,title=callback_type_mismatch::Type mismatch for @callback get_weather.
lib/weather_impl.ex:5:callback_type_mismatch
Type mismatch for @callback get_weather/1 in WeatherAPI behaviour.

Expected type:
{:error, binary()} | {:ok, map()}

Actual type:
:ok

________________________________________________________________________________
::warning file=lib/weather_impl.ex,line=11,title=callback_type_mismatch::Type mismatch for @callback fetch_weather.
lib/weather_impl.ex:11:callback_type_mismatch
Type mismatch for @callback fetch_weather/1 in WeatherAPI behaviour.

Expected type:
{:error, binary()} | {:ok, map()}

Actual type:
:ok

________________________________________________________________________________
done (warnings were emitted)
Halting VM with exit status 2

Updated docs in README:
Screenshot 2024-09-20 06-37-43@2x

Fixes jeremyjh#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).
@axelson
Copy link
Contributor Author

axelson commented Sep 9, 2024

@jeremyjh Ping! Do you have any thoughts on this PR?

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

@jeremyjh
Copy link
Owner

@axelson Sorry, was just very busy and then forgot to get back to this. I really appreciate the contribution! Would you mind adding an update to the docs for this?

@axelson
Copy link
Contributor Author

axelson commented Sep 11, 2024

No worries! I've definitely taken significantly longer to respond to PRs 😅

Yeah I'll take a first crack at adding docs for this 👍

@axelson
Copy link
Contributor Author

axelson commented Sep 20, 2024

Okay I've added docs in baaa24d

Also let me know if you want me to clean up the git history

@jeremyjh jeremyjh enabled auto-merge (squash) September 24, 2024 11:23
@jeremyjh jeremyjh merged commit 4d76fe9 into jeremyjh:master Sep 24, 2024
18 checks passed
@jeremyjh
Copy link
Owner

@axelson Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to output multiple formats
2 participants