Repository: jeremyjh/dialyxir
Pull request #278 (2018-11-22 16:18:21)

List unnecessary ignore filters
This pull requests implements listing unnecessary ignore filters as proposed in #277.

evnu: I pushed this early to get some more feedback. The current implementation allows enabling/disabling listing the filters with `:list_unused_filters`. It does not yet implement a "warnings as error" flag, as I was not sure were to implement that properly.

Example output with `:list_unused_filters` enabled:

```
Total errors: 1, Skipped: 1, Unnecessary Skips: 1
Unused filters:
{"a/file.ex:17:no_return Function format_long/1 has no local return."}
```

The pull request also misses an update to the README. I will do after the feature is complete.
jeremyjh: This looks good so far.
evnu: 71fad58 is a first attempt at implementing "warnings as errors". By setting `:unused_filters_as_errors`, dialyxir should return with an error code if unused filters are found. @jeremyjh What do you think, is this the right direction?
evnu: > :unused_filters_as_errors

I think a flag like this should also imply that the unused filters are listed. Otherwise, the output is probably confusing.
asummers: This is looking really good. Going to give it another look this evening, but I didn't see anything that stuck out to me with your most recent changes. 
asummers: Seems like Dialyzer isn't honoring the status code, according to Travis. 
evnu: >> Seems like Dialyzer isn't honoring the status code, according to Travis.
> I will check that.

4216113 changes this behaviour. By default, the status code is not changed even if unused filters are present. A user can make this an error with `unused_filters_as_errors`. Is this the behaviour we want here? With that, additionally listing the unnecessary filters might not be needed. Users will encounter output such as this:

```
Total errors: 26, Skipped: 26, Unnecessary Skips: 1
```

They will then wonder what `Unnecessary Skips: 1` means, referring to the README section (TBD) and reading about the two possible options `:list_unused_filters` and `:unused_filters_as_errors`, allowing them to choose how they want to handle this.
evnu: af8f9e4 extends the README.
jeremyjh: I'm struggling to think of an occasion where I'd want a non-zero status for unused filters yet not want to see which filters were unused. In other words I think `unused_filters_as_errors` should imply `list_unused_filters`. 
evnu: With 909904a, `unused_filters_as_errors` implies `list_unused_filters`. This is also tested in `formatter_test.exs`.
evnu: Do you want the commits to be squashed and rebased? If so, should I do that before the review is finished, or afterwards?
jeremyjh: @evnu yes please squash once we approve. you can rebase on master whenever you like. are there any tasks remaining on this PR?
evnu: I rebased the branch and added command line arguments with c6da037. I think with this the TODOs w.r.t. functionality should be finished now.
jeremyjh: Woops, one issue other than the formatting, is that it looks like the CLI arg is getting passed down to dialyzer which complains of an unknown warning:

https://travis-ci.org/jeremyjh/dialyxir/builds/464953658#L864

CLI args need to be captured in the [command options](https://github.com/jeremyjh/dialyxir/blob/master/lib/mix/tasks/dialyzer.ex#L132) so that they are split into a separate list from the dialyzer args (all unknown CLI args are passed to dialyzer with the assumption that they are specific warning flags).

evnu: > LGTM, thanks so much for all your efforts on this!

My pleasure.

> Woops, one issue other than the formatting, is that it looks like the CLI arg is getting passed down to dialyzer which complains of an unknown warning:

Yes, another typo. Should be fixed now.
asummers: @jeremyjh I wonder if we should warn on unknown opts. I’ll make an issue and we can discuss there.
evnu: Squashed into f3c31c3.
jeremyjh: It looks like its not halting with a non-zero exit code when we use --unused-filters-as-errors: 

The WX filter in our own ignore file is apparently unused, and while it prints the warning in red, it still exits with a 0:

https://travis-ci.org/jeremyjh/dialyxir/builds/464988166#L879
evnu: > It looks like its not halting with a non-zero exit code when we use --unused-filters-as-errors

It does abort if `--halt-exit-status` is also used. I will try to figure out why that is currently also needed.
evnu: >  I will try to figure out why that is currently also needed.

The exit status of `Dialyzer.dialyze/3` is ignored if `halt_exit_status` is not set.
jeremyjh: Ah yes, that makes sense. Maybe we should note that in the doc for `unused_filters_as_errors` though?
evnu: >  Maybe we should note that in the doc for unused_filters_as_errors though?

That would work, but I wonder if the option itself makes sense at all if `--halt-exit-status` is needed anyways. I suggest we drop the option and make `list_unused_filters` always result in a warning. That way, `--halt-exit-status` can be used directly together with that option. ea1cc70 implements just that and should fail due to the unused WX filter. What do you think, does this make sene?
evnu: Adding to the previous comment, if `unused_filters_as_errors` is dropped, then `--halt-exit-status` should also imply `list_unused_filters`.
evnu: With 33acdef, `halt-exit-status` now implies `:list_unused_filters`.
jeremyjh: If `--halt-exit-status` without other options means unused filters will result in a non-zero exit then we'll break people's CI builds when they upgrade. Maybe they should be happy to fix them but I'm not sure its worth it, or that there should be no way to opt-out of unused warnings as errors in your CI process (which has to use --halt-exit-status). What *does* make sense to me is that `--list-unused-errors + --halt-exit-status` implies warnings as errors.

Also I noticed the warning isn't highlighted in red anymore: https://travis-ci.org/jeremyjh/dialyxir/builds/465303756#L858


evnu: > What does make sense to me is that --list-unused-errors + --halt-exit-status implies warnings as errors.

Yes, that sounds reasonable. I think it would make sense to derive `:unused_filters_as_errors = --list-unused-errors && --halt-exit-status`, but not expose this as a user-definable variable. So I would go back to the original behaviour from https://github.com/jeremyjh/dialyxir/commit/f3c31c3050b54be1110f881c2a9bff734fa600b6 the squashed commit, remove documentation for `:unused_filters_as_errors ` and derive that value from the other parameters. Does that sound like the right approach?

> Also I noticed the warning isn't highlighted in red anymore:

Ah, yes, I think that is because it is `:warn`, not `:error`. This should be resolved as well with what I wrote further above (going back to the original behaviour etc).
jeremyjh: > So I would go back to the original behaviour from f3c31c3 the squashed commit, remove documentation for :unused_filters_as_errors and derive that value from the other parameters. Does that sound like the right approach?

It does to me. @asummers are you good with this as well?
asummers: Sounds fine. 
evnu: 38ea9b4 derives error from `list-unused-filters + halt-exit-status`. With 612a373, skipped filters are now printed in red if that is an error. For 612a373, I needed to pull printing from `formatter.ex` into `dialyzer.ex`. I am not sure if that solution is pretty, but I did not want to pass color-coding into formatter.
jeremyjh: lgtm

Patch:
diff --git a/.travis.yml b/.travis.yml
index 2d5e42b..5d31c49 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -15,6 +15,8 @@ script:
   - mix format --check-formatted
   - MIX_ENV=prod mix dialyzer --halt-exit-status
   - OUTPUT_TESTS=true mix test
+  - mix dialyzer --list-unused-filters
+  - mix dialyzer --unused-filters-as-errors
 branches:
   except:
     - /^[0-9]\.[0-9]/
diff --git a/README.md b/README.md
index ea137f1..50ebd62 100644
--- a/README.md
+++ b/README.md
@@ -237,3 +237,19 @@ applied to the *short-description* (`mix dialyzer --format short`):
 ```
 
 `:ignore_warnings` works as you may expect with `--halt-exit-status` - by resetting the exit status to 0 if all warnings are filtered.
+
+#### List unused Filters
+
+As filters tend to become obsolete (either because a discrepancy was fixed, or because the location
+for which a filter is needed changes), listing unused filters might be useful. This can be done by
+setting the `:list_unused_filters` option to `true` in `mix.exs`. For example:
+
+```elixir
+dialyzer: [
+    ignore_warnings: "ignore_test.exs",
+    list_unused_filters: true
+]
+```
+
+This option can also be set on the command line with `--list-unused-filters`. When used with
+`--halt-exit-status`, this option will result in an error status code.
diff --git a/lib/dialyxir/dialyzer.ex b/lib/dialyxir/dialyzer.ex
index aaa9db1..3abc78e 100644
--- a/lib/dialyxir/dialyzer.ex
+++ b/lib/dialyxir/dialyzer.ex
@@ -3,11 +3,13 @@ defmodule Dialyxir.Dialyzer do
   alias String.Chars
   alias Dialyxir.Formatter
   alias Dialyxir.Project
+  alias Dialyxir.FilterMap
 
   defmodule Runner do
     def run(args, filterer) do
       try do
-        {split, args} = Keyword.split(args, [:raw, :format])
+        split_args = [:raw, :format, :list_unused_filters, :halt_exit_status]
+        {split, args} = Keyword.split(args, split_args)
 
         formatter =
           cond do
@@ -39,8 +41,19 @@ defmodule Dialyxir.Dialyzer do
         {duration_ms, result} = :timer.tc(&:dialyzer.run/1, [args])
 
         formatted_time_elapsed = Formatter.formatted_time(duration_ms)
-        formatted_warnings = Formatter.format_and_filter(result, filterer, formatter)
-        {:ok, {formatted_time_elapsed, formatted_warnings}}
+
+        filter_map_args = FilterMap.to_args(split)
+
+        case Formatter.format_and_filter(result, filterer, filter_map_args, formatter) do
+          {:ok, formatted_warnings, :no_unused_filters} ->
+            {:ok, {formatted_time_elapsed, formatted_warnings, ""}}
+
+          {:warn, formatted_warnings, {:unused_filters_present, formatted_unnecessary_skips}} ->
+            {:ok, {formatted_time_elapsed, formatted_warnings, formatted_unnecessary_skips}}
+
+          {:error, _formatted_warnings, {:unused_filters_present, formatted_unnecessary_skips}} ->
+            {:error, {"unused filters present", formatted_unnecessary_skips}}
+        end
       catch
         {:dialyzer_error, msg} ->
           {:error, ":dialyzer.run error: " <> Chars.to_string(msg)}
@@ -56,12 +69,23 @@ defmodule Dialyxir.Dialyzer do
 
   def dialyze(args, runner \\ Runner, filterer \\ Project) do
     case runner.run(args, filterer) do
-      {:ok, {time, []}} ->
-        {:ok, @success_return_code, [time, @success_msg]}
+      {:ok, {time, [], formatted_unnecessary_skips}} ->
+        {:ok, @success_return_code, [time, formatted_unnecessary_skips, @success_msg]}
 
-      {:ok, {time, result}} ->
+      {:ok, {time, result, formatted_unnecessary_skips}} ->
         warnings = Enum.map(result, &color(&1, :red))
-        {:warn, @warning_return_code, [time] ++ warnings ++ [@warnings_msg]}
+
+        {:warn, @warning_return_code,
+         [time] ++ warnings ++ [formatted_unnecessary_skips, @warnings_msg]}
+
+      {:warn, {time, result, formatted_unnecessary_skips}} ->
+        warnings = Enum.map(result, &color(&1, :red))
+
+        {:warn, @warning_return_code,
+         [time] ++ warnings ++ [formatted_unnecessary_skips, @warnings_msg]}
+
+      {:error, {msg, formatted_unnecessary_skips}} ->
+        {:error, @error_return_code, [color(formatted_unnecessary_skips, :red), color(msg, :red)]}
 
       {:error, msg} ->
         {:error, @error_return_code, [color(msg, :red)]}
diff --git a/lib/dialyxir/filter_map.ex b/lib/dialyxir/filter_map.ex
new file mode 100644
index 0000000..bf184ff
--- /dev/null
+++ b/lib/dialyxir/filter_map.ex
@@ -0,0 +1,71 @@
+defmodule Dialyxir.FilterMap do
+  @moduledoc """
+  A counters holding warnings to be skipped.
+
+  `:counters` points to a `Map` where the keys are warnings to be skipped and the value indicates
+  how often the warning was skipped.
+  """
+  defstruct list_unused_filters?: false, unused_filters_as_errors?: false, counters: %{}
+
+  @doc """
+  Fill a `FilterMap` from an ignore file.
+  """
+  def from_file(ignore_file, list_unused_filters?, halt_exit_status?) do
+    filter_map = %__MODULE__{
+      list_unused_filters?: list_unused_filters?,
+      unused_filters_as_errors?: list_unused_filters? && halt_exit_status?
+    }
+
+    {ignore, _} =
+      ignore_file
+      |> File.read!()
+      |> Code.eval_string()
+
+    Enum.reduce(ignore, filter_map, fn skip, filter_map ->
+      put_in(filter_map.counters[skip], 0)
+    end)
+  end
+
+  @doc """
+  Remove all non-allowed arguments from `args`.
+  """
+  def to_args(args) do
+    Keyword.take(args, [:list_unused_filters, :halt_exit_status])
+  end
+
+  @doc """
+  Retrieve the filters from a `FilterMap`.
+  """
+  def filters(filter_map) do
+    Map.keys(filter_map.counters)
+  end
+
+  @doc """
+  Increase usage count of a filter in `FilterMap`.
+  """
+  def inc(filter_map, filter) do
+    update_in(filter_map.counters[filter], &(&1 + 1))
+  end
+
+  @doc """
+  List unused filters.
+  """
+  def unused_filters(filter_map) do
+    filter_map.counters
+    |> Enum.filter(&unused?/1)
+    |> Enum.unzip()
+    |> elem(0)
+  end
+
+  @doc """
+  Determine if any filters were not used.
+  """
+  def unused_filters?(filter_map) do
+    Enum.any?(filter_map.counters, &unused?/1)
+  end
+
+  @doc """
+  Check if a `FilterMap` entry is unused.
+  """
+  def unused?({_filter, count}), do: count < 1
+end
diff --git a/lib/dialyxir/formatter.ex b/lib/dialyxir/formatter.ex
index a2bc17b..3e88d8c 100644
--- a/lib/dialyxir/formatter.ex
+++ b/lib/dialyxir/formatter.ex
@@ -7,22 +7,26 @@ defmodule Dialyxir.Formatter do
   """
   import Dialyxir.Output, only: [info: 1]
 
+  alias Dialyxir.FilterMap
+
   def formatted_time(duration_us) do
     minutes = div(duration_us, 60_000_000)
     seconds = (rem(duration_us, 60_000_000) / 1_000_000) |> Float.round(2)
     "done in #{minutes}m#{seconds}s"
   end
 
-  def format_and_filter(warnings, _, :raw) do
+  def format_and_filter(warnings, _, _filter_map_args, :raw) do
     Enum.map(warnings, &inspect/1)
   end
 
-  def format_and_filter(warnings, filterer, :dialyxir) do
+  def format_and_filter(warnings, filterer, filter_map_args, :dialyxir) do
     divider = String.duplicate("_", 80)
+    filter_map = filterer.filter_map(filter_map_args)
+
+    {formatted_warnings, filter_map} = filter_warnings(warnings, filterer, filter_map)
 
     formatted_warnings =
-      warnings
-      |> filter_warnings(filterer)
+      formatted_warnings
       |> filter_legacy_warnings(filterer)
       |> Enum.map(fn warning ->
         message =
@@ -33,15 +37,28 @@ defmodule Dialyxir.Formatter do
         message <> "\n" <> divider
       end)
 
-    show_count_skipped(warnings, formatted_warnings)
+    show_count_skipped(warnings, formatted_warnings, filter_map)
+    formatted_unnecessary_skips = format_unnecessary_skips(filter_map)
 
-    formatted_warnings
+    cond do
+      FilterMap.unused_filters?(filter_map) && filter_map.unused_filters_as_errors? ->
+        {:error, formatted_warnings, {:unused_filters_present, formatted_unnecessary_skips}}
+
+      FilterMap.unused_filters?(filter_map) ->
+        {:warn, formatted_warnings, {:unused_filters_present, formatted_unnecessary_skips}}
+
+      true ->
+        {:ok, formatted_warnings, :no_unused_filters}
+    end
   end
 
-  def format_and_filter(warnings, filterer, :dialyzer) do
+  def format_and_filter(warnings, filterer, filter_map_args, :dialyzer) do
+    filter_map = filterer.filter_map(filter_map_args)
+
+    {filtered_warnings, filter_map} = filter_warnings(warnings, filterer, filter_map)
+
     filtered_warnings =
-      warnings
-      |> filter_warnings(filterer)
+      filtered_warnings
       |> filter_legacy_warnings(filterer)
       |> Enum.map(fn warning ->
         warning
@@ -49,14 +66,17 @@ defmodule Dialyxir.Formatter do
         |> String.replace_trailing("\n", "")
       end)
 
-    show_count_skipped(warnings, filtered_warnings)
+    show_count_skipped(warnings, filtered_warnings, filter_map)
 
     filtered_warnings
   end
 
-  def format_and_filter(warnings, filterer, :short) do
-    warnings
-    |> filter_warnings(filterer)
+  def format_and_filter(warnings, filterer, filter_map_args, :short) do
+    filter_map = filterer.filter_map(filter_map_args)
+
+    {formatted_warnings, _skip_map} = filter_warnings(warnings, filterer, filter_map)
+
+    formatted_warnings
     |> filter_legacy_warnings(filterer)
     |> Enum.map(&format_warning(&1, :short))
   end
@@ -152,15 +172,41 @@ defmodule Dialyxir.Formatter do
     """
   end
 
-  defp show_count_skipped(warnings, filtered_warnings) do
+  defp show_count_skipped(warnings, filtered_warnings, filter_map) do
     warnings_count = Enum.count(warnings)
     filtered_warnings_count = Enum.count(filtered_warnings)
     skipped_count = warnings_count - filtered_warnings_count
-    info("Total errors: #{warnings_count}, Skipped: #{skipped_count}")
+    unnessary_skips_count = count_unnecessary_skips(filter_map)
+
+    info(
+      "Total errors: #{warnings_count}, Skipped: #{skipped_count}, " <>
+        "Unnecessary Skips: #{unnessary_skips_count}"
+    )
 
     :ok
   end
 
+  defp format_unnecessary_skips(filter_map = %FilterMap{list_unused_filters?: true}) do
+    unused_filters = FilterMap.unused_filters(filter_map)
+
+    if Enum.empty?(unused_filters) do
+      ""
+    else
+      unused_filters = Enum.map_join(unused_filters, "\n", &inspect/1)
+      "Unused filters:\n#{unused_filters}"
+    end
+  end
+
+  defp format_unnecessary_skips(_) do
+    ""
+  end
+
+  defp count_unnecessary_skips(filter_map) do
+    filter_map.counters
+    |> Enum.filter(&FilterMap.unused?/1)
+    |> Enum.count()
+  end
+
   defp warning(warning_name) do
     warnings = Dialyxir.Warnings.warnings()
 
@@ -171,13 +217,35 @@ defmodule Dialyxir.Formatter do
     end
   end
 
-  defp filter_warnings(warnings, filterer) do
-    Enum.reject(warnings, fn warning = {_, {file, line}, {warning_type, _}} ->
-      Map.has_key?(Dialyxir.Warnings.warnings(), warning_type) &&
+  defp filter_warnings(warnings, filterer, filter_map) do
+    {warnings, filter_map} =
+      Enum.map_reduce(warnings, filter_map, &filter_warning(filterer, &1, &2))
+
+    warnings = Enum.reject(warnings, &is_nil/1)
+    {warnings, filter_map}
+  end
+
+  defp filter_warning(filterer, warning = {_, {file, line}, {warning_type, _}}, filter_map) do
+    if Map.has_key?(Dialyxir.Warnings.warnings(), warning_type) do
+      {skip?, matching_filters} =
         filterer.filter_warning?(
-          {to_string(file), warning_type, line, format_warning(warning, :short)}
+          {to_string(file), warning_type, line, format_warning(warning, :short)},
+          filter_map
         )
-    end)
+
+      filter_map =
+        Enum.reduce(matching_filters, filter_map, fn filter, filter_map ->
+          FilterMap.inc(filter_map, filter)
+        end)
+
+      if skip? do
+        {nil, filter_map}
+      else
+        {warning, filter_map}
+      end
+    else
+      {warning, filter_map}
+    end
   end
 
   defp filter_legacy_warnings(warnings, filterer) do
diff --git a/lib/dialyxir/output.ex b/lib/dialyxir/output.ex
index 8c39a9a..33be292 100644
--- a/lib/dialyxir/output.ex
+++ b/lib/dialyxir/output.ex
@@ -21,7 +21,9 @@ defmodule Dialyxir.Output do
     end
   end
 
+  def info(""), do: :ok
   def info(text), do: Mix.shell().info(text)
 
+  def error(""), do: :ok
   def error(text), do: Mix.shell().error(text)
 end
diff --git a/lib/dialyxir/project.ex b/lib/dialyxir/project.ex
index ca49c0d..330b16c 100644
--- a/lib/dialyxir/project.ex
+++ b/lib/dialyxir/project.ex
@@ -2,6 +2,8 @@ defmodule Dialyxir.Project do
   @moduledoc false
   import Dialyxir.Output, only: [info: 1, error: 1]
 
+  alias Dialyxir.FilterMap
+
   def plts_list(deps, include_project \\ true, exclude_core \\ false) do
     elixir_apps = [:elixir]
     erlang_apps = [:erts, :kernel, :stdlib, :crypto]
@@ -77,23 +79,27 @@ defmodule Dialyxir.Project do
 
   defp skip?(_, _), do: false
 
-  def filter_warning?({file, warning, line, short_description}) do
+  def filter_warning?({file, warning, line, short_description}, filter_map = %FilterMap{}) do
+    {matching_filters, _non_matching_filters} =
+      filter_map
+      |> FilterMap.filters()
+      |> Enum.split_with(&skip?(&1, {file, warning, line, short_description}))
+
+    {not Enum.empty?(matching_filters), matching_filters}
+  end
+
+  def filter_map(args) do
     cond do
       legacy_ignore_warnings?() ->
-        false
+        %FilterMap{}
 
       dialyzer_ignore_warnings() == nil && !File.exists?(default_ignore_warnings()) ->
-        false
+        %FilterMap{}
 
       true ->
         ignore_file = dialyzer_ignore_warnings() || default_ignore_warnings()
 
-        {ignore, _} =
-          ignore_file
-          |> File.read!()
-          |> Code.eval_string()
-
-        Enum.any?(ignore, &skip?(&1, {file, warning, line, short_description}))
+        FilterMap.from_file(ignore_file, list_unused_filters?(args), halt_exit_status?(args))
     end
   end
 
@@ -149,6 +155,17 @@ defmodule Dialyxir.Project do
     dialyzer_config()[:ignore_warnings]
   end
 
+  defp list_unused_filters?(args) do
+    case Keyword.fetch(args, :list_unused_filters) do
+      :error -> dialyzer_config()[:list_unused_filters]
+      {:ok, list_unused_filters} -> list_unused_filters
+    end
+  end
+
+  defp halt_exit_status?(args) do
+    args[:halt_exit_status]
+  end
+
   def elixir_plt() do
     global_plt("erlang-#{otp_vsn()}_elixir-#{System.version()}")
   end
diff --git a/lib/mix/tasks/dialyzer.ex b/lib/mix/tasks/dialyzer.ex
index c06cb23..69a9ae3 100644
--- a/lib/mix/tasks/dialyzer.ex
+++ b/lib/mix/tasks/dialyzer.ex
@@ -13,6 +13,7 @@ defmodule Mix.Tasks.Dialyzer do
     * `--force_check`      - force PLT check also if lock file is unchanged.
        useful when dealing with local deps.
     * `--halt-exit-status` - exit immediately with same exit status as dialyzer.
+    * `--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.
@@ -134,6 +135,7 @@ defmodule Mix.Tasks.Dialyzer do
     no_check: :boolean,
     force_check: :boolean,
     halt_exit_status: :boolean,
+    list_unused_filters: :boolean,
     plt: :boolean,
     quiet: :boolean,
     raw: :boolean,
@@ -248,7 +250,9 @@ defmodule Mix.Tasks.Dialyzer do
       {:files_rec, Project.dialyzer_paths()},
       {:warnings, dialyzer_warnings(dargs)},
       {:format, opts[:format]},
-      {:raw, opts[:raw]}
+      {:raw, opts[:raw]},
+      {:list_unused_filters, opts[:list_unused_filters]},
+      {:halt_exit_status, opts[:halt_exit_status]}
     ]
 
     {status, exit_status, [time | result]} = Dialyzer.dialyze(args)


Test patch:
diff --git a/test/dialyxir/dialyzer_test.exs b/test/dialyxir/dialyzer_test.exs
index 99b2b95..878f133 100644
--- a/test/dialyxir/dialyzer_test.exs
+++ b/test/dialyxir/dialyzer_test.exs
@@ -7,7 +7,7 @@ defmodule Dialyxir.DialyzerTest do
 
     test "formatting with no warnings" do
       defmodule StubSuccess do
-        def run(_, _), do: {:ok, {"time", []}}
+        def run(_, _), do: {:ok, {"time", [], ""}}
       end
 
       {expected_result_code, expected_exit_code, expected_messages} =
@@ -15,12 +15,12 @@ defmodule Dialyxir.DialyzerTest do
 
       assert expected_result_code == :ok
       assert expected_exit_code == 0
-      assert expected_messages == ["time", "\e[32mdone (passed successfully)\e[0m"]
+      assert expected_messages == ["time", "", "\e[32mdone (passed successfully)\e[0m"]
     end
 
     test "formatting with warnings" do
       defmodule StubWarn do
-        def run(_, _), do: {:ok, {"time", ["warning 1", "warning 2"]}}
+        def run(_, _), do: {:ok, {"time", ["warning 1", "warning 2"], ""}}
       end
 
       {expected_result_code, expected_exit_code, expected_messages} = dialyze(nil, StubWarn, nil)
@@ -32,6 +32,7 @@ defmodule Dialyxir.DialyzerTest do
                "time",
                "\e[31mwarning 1\e[0m",
                "\e[31mwarning 2\e[0m",
+               "",
                "\e[33mdone (warnings were emitted)\e[0m"
              ]
     end
diff --git a/test/dialyxir/formatter_test.exs b/test/dialyxir/formatter_test.exs
index 5ceac97..bbafc4c 100644
--- a/test/dialyxir/formatter_test.exs
+++ b/test/dialyxir/formatter_test.exs
@@ -1,5 +1,6 @@
 defmodule Dialyxir.FormatterTest do
   use ExUnit.Case
+
   alias Dialyxir.Formatter
   alias Dialyxir.Project
 
@@ -13,7 +14,7 @@ defmodule Dialyxir.FormatterTest do
         {:warn_return_no_exit, {'a/file.ex', 17}, {:no_return, [:only_normal, :format_long, 1]}}
 
       in_project(:ignore, fn ->
-        remaining = Formatter.format_and_filter([warning], Project, :short)
+        remaining = Formatter.format_and_filter([warning], Project, [], :short)
         assert remaining == []
       end)
     end
@@ -24,7 +25,7 @@ defmodule Dialyxir.FormatterTest do
          {:no_return, [:only_normal, :format_long, 1]}}
 
       in_project(:ignore, fn ->
-        [remaining] = Formatter.format_and_filter([warning], Project, :short)
+        [remaining] = Formatter.format_and_filter([warning], Project, [], :short)
         assert remaining =~ ~r/different_file.* no local return/
       end)
     end
@@ -35,9 +36,52 @@ defmodule Dialyxir.FormatterTest do
          {:no_return, [:only_normal, :format_long, 1]}}
 
       in_project(:ignore, fn ->
-        remaining = Formatter.format_and_filter([warning], Project, :short)
+        remaining = Formatter.format_and_filter([warning], Project, [], :short)
         assert remaining == []
       end)
     end
+
+    test "lists unnecessary skips" do
+      warning =
+        {:warn_return_no_exit, {'a/regex_file.ex', 17},
+         {:no_return, [:only_normal, :format_long, 1]}}
+
+      in_project(:ignore, fn ->
+        assert {:warn, [], {:unused_filters_present, warning}} =
+                 Formatter.format_and_filter([warning], Project, [], :dialyxir)
+
+        assert warning =~ "Unused filters:"
+      end)
+    end
+
+    test "error on unnecessary skips with halt_exit_status" do
+      warning =
+        {:warn_return_no_exit, {'a/regex_file.ex', 17},
+         {:no_return, [:only_normal, :format_long, 1]}}
+
+      filter_args = [{:halt_exit_status, true}]
+
+      in_project(:ignore, fn ->
+        {:error, [], {:unused_filters_present, error}} =
+          Formatter.format_and_filter([warning], Project, filter_args, :dialyxir)
+
+        assert error =~ "Unused filters:"
+      end)
+    end
+
+    test "overwrite ':list_unused_filters_present'" do
+      warning =
+        {:warn_return_no_exit, {'a/regex_file.ex', 17},
+         {:no_return, [:only_normal, :format_long, 1]}}
+
+      filter_args = [{:list_unused_filters, false}]
+
+      in_project(:ignore, fn ->
+        assert {:warn, [], {:unused_filters_present, warning}} =
+                 Formatter.format_and_filter([warning], Project, filter_args, :dialyxir)
+
+        refute warning =~ "Unused filters:"
+      end)
+    end
   end
 end
diff --git a/test/fixtures/ignore/mix.exs b/test/fixtures/ignore/mix.exs
index a56c5a5..2204b0e 100644
--- a/test/fixtures/ignore/mix.exs
+++ b/test/fixtures/ignore/mix.exs
@@ -2,6 +2,13 @@ defmodule Ignore.Mixfile do
   use Mix.Project
 
   def project do
-    [app: :ignore, version: "0.1.0", dialyzer: [ignore_warnings: "ignore_test.exs"]]
+    [
+      app: :ignore,
+      version: "0.1.0",
+      dialyzer: [
+        ignore_warnings: "ignore_test.exs",
+        list_unused_filters: true
+      ]
+    ]
   end
 end