Add new format for ignore_file_strict: {file, warning_description} (#493

)

* Add format for ignore_file_strict

README.md

@@ -36,16 +36,17 @@ 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.
-  *  `--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.
-  *  `--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 to be suitable for adding to Elixir Format ignore file.
-  *  `--quiet`              - suppress all informational messages.
+  * `--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.
+  *  `--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.
 
 Warning flags passed to this task are passed on to `:dialyzer` - e.g.
 
@@ -280,6 +281,8 @@ applied to the *short-description* format of Dialyzer output (`mix dialyzer --fo
   {":0:unknown_function Function :erl_types.t_to_string/1 does not exist.", :unknown_function, 0},
   # {file, warning_type, line}
   {"lib/dialyxir/pretty_print.ex", :no_return, 100},
+  # {file, warning_description}
+  {"lib/dialyxir/warning_helpers.ex", "Function :erl_types.t_to_string/1 does not exist."},
   # {file, warning_type}
   {"lib/dialyxir/warning_helpers.ex", :no_return},
   # {file}
@@ -289,8 +292,42 @@ applied to the *short-description* format of Dialyzer output (`mix dialyzer --fo
 ]
 ```
 
-Entries for existing warnings can be generated with `mix dialyzer --format ignore_file`.
+_Note that `short_description` contains additional information that `warning_description` does not._
 
+Entries for existing warnings can be generated with one of the following:
+- `mix dialyzer --format ignore_file`
+- `mix dialyzer --format ignore_file_strict` (recommended)
+
+For example, if `mix dialyzer --format short` gives you a result like:
+```
+lib/something.ex:15:no_return Function init/1 has no local return.
+lib/something.ex:36:no_return Function refresh/0 has no local return.
+lib/something.ex:45:no_return Function create/2 has no local return.
+lib/something.ex:26:no_return Function update/2 has no local return.
+lib/something.ex:49:no_return Function delete/1 has no local return.
+```
+
+If you had used `--format ignore_file`, you'd be given a single file ignore line for all five warnings:
+```elixir
+# .dialyzer_ignore.exs
+[
+  # {file, warning_type}
+  {"lib/something.ex", :no_return},
+]
+```
+
+If you had used `--format ignore_file_strict`, you'd be given more granular ignore lines:
+```elixir
+# .dialyzer_ignore.exs
+[
+  # {file, warning_description}
+  {"lib/something.ex", "Function init/1 has no local return."},
+  {"lib/something.ex", "Function refresh/0 has no local return."},
+  {"lib/something.ex", "Function create/2 has no local return."},
+  {"lib/something.ex", "Function update/2 has no local return."},
+  {"lib/something.ex", "Function delete/1 has no local return."},
+]
+```
 
 #### List unused Filters
 

lib/dialyxir/dialyzer.ex

@@ -31,6 +31,9 @@ defmodule Dialyxir.Dialyzer do
             split[:format] == "ignore_file" ->
               Dialyxir.Formatter.IgnoreFile
 
+            split[:format] == "ignore_file_strict" ->
+              Dialyxir.Formatter.IgnoreFileStrict
+
             split[:format] == "raw" ->
               Dialyxir.Formatter.Raw
 

lib/dialyxir/formatter.ex

@@ -95,14 +95,11 @@ defmodule Dialyxir.Formatter do
     {warnings, filter_map}
   end
 
-  defp filter_warning(filterer, warning = {_, {file, line}, {warning_type, _}}, filter_map) do
+  defp filter_warning(filterer, {_, {_file, _line}, {warning_type, _args}} = warning, filter_map) do
     if Map.has_key?(Dialyxir.Warnings.warnings(), warning_type) do
       {skip?, matching_filters} =
         try do
-          filterer.filter_warning?(
-            {to_string(file), warning_type, line, Dialyxir.Formatter.Short.format(warning)},
-            filter_map
-          )
+          filterer.filter_warning?(warning, filter_map)
         rescue
           _ ->
             {false, []}

lib/dialyxir/formatter/dialyxir.ex

@@ -1,6 +1,8 @@
 defmodule Dialyxir.Formatter.Dialyxir do
   @moduledoc false
 
+  alias Dialyxir.Formatter.Utils
+
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
@@ -10,7 +12,7 @@ defmodule Dialyxir.Formatter.Dialyxir do
 
     formatted =
       try do
-        warning = warning(warning_name)
+        warning = Utils.warning(warning_name)
         string = warning.format_long(arguments)
 
         """
@@ -79,14 +81,4 @@ defmodule Dialyxir.Formatter.Dialyxir do
     #{Dialyxir.Formatter.Dialyzer.format(warning)}
     """
   end
-
-  defp warning(warning_name) do
-    warnings = Dialyxir.Warnings.warnings()
-
-    if Map.has_key?(warnings, warning_name) do
-      Map.get(warnings, warning_name)
-    else
-      throw({:error, :unknown_warning, warning_name})
-    end
-  end
 end

lib/dialyxir/formatter/github.ex

@@ -1,25 +1,17 @@
 defmodule Dialyxir.Formatter.Github do
   @moduledoc false
 
+  alias Dialyxir.Formatter.Utils
+
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
   def format({_tag, {file, line}, {warning_name, arguments}}) do
     base_name = Path.relative_to_cwd(file)
 
-    warning = warning(warning_name)
+    warning = Utils.warning(warning_name)
     string = warning.format_short(arguments)
 
     "::warning file=#{base_name},line=#{line},title=#{warning_name}::#{string}"
   end
-
-  defp warning(warning_name) do
-    warnings = Dialyxir.Warnings.warnings()
-
-    if Map.has_key?(warnings, warning_name) do
-      Map.get(warnings, warning_name)
-    else
-      throw({:error, :unknown_warning, warning_name})
-    end
-  end
 end

lib/dialyxir/formatter/ignore_file_strict.ex

@@ -0,0 +1,15 @@
+defmodule Dialyxir.Formatter.IgnoreFileStrict do
+  @moduledoc false
+
+  alias Dialyxir.Formatter.Utils
+
+  @behaviour Dialyxir.Formatter
+
+  @impl Dialyxir.Formatter
+  def format({_tag, {file, _line}, {warning_name, arguments}}) do
+    warning = Utils.warning(warning_name)
+    string = warning.format_short(arguments)
+
+    ~s({"#{file}", "#{string}"},)
+  end
+end

lib/dialyxir/formatter/short.ex

@@ -1,25 +1,17 @@
 defmodule Dialyxir.Formatter.Short do
   @moduledoc false
 
+  alias Dialyxir.Formatter.Utils
+
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
   def format({_tag, {file, line}, {warning_name, arguments}}) do
     base_name = Path.relative_to_cwd(file)
 
-    warning = warning(warning_name)
+    warning = Utils.warning(warning_name)
     string = warning.format_short(arguments)
 
     "#{base_name}:#{line}:#{warning_name} #{string}"
   end
-
-  defp warning(warning_name) do
-    warnings = Dialyxir.Warnings.warnings()
-
-    if Map.has_key?(warnings, warning_name) do
-      Map.get(warnings, warning_name)
-    else
-      throw({:error, :unknown_warning, warning_name})
-    end
-  end
 end

lib/dialyxir/formatter/utils.ex

@@ -0,0 +1,11 @@
+defmodule Dialyxir.Formatter.Utils do
+  def warning(warning_name) do
+    warnings = Dialyxir.Warnings.warnings()
+
+    if Map.has_key?(warnings, warning_name) do
+      Map.get(warnings, warning_name)
+    else
+      throw({:error, :unknown_warning, warning_name})
+    end
+  end
+end

lib/dialyxir/project.ex

@@ -3,6 +3,8 @@ defmodule Dialyxir.Project do
   import Dialyxir.Output, only: [info: 1, error: 1]
 
   alias Dialyxir.FilterMap
+  alias Dialyxir.Formatter.Short
+  alias Dialyxir.Formatter.Utils
 
   def plts_list(deps, include_project \\ true, exclude_core \\ false) do
     elixir_apps = [:elixir]
@@ -116,24 +118,40 @@ defmodule Dialyxir.Project do
     end
   end
 
-  defp skip?({file, warning, line}, {file, warning, line, _}), do: true
-  defp skip?({file, warning}, {file, warning, _, _}), do: true
-  defp skip?({file}, {file, _, _, _}), do: true
-  defp skip?({short_description, warning, line}, {_, warning, line, short_description}), do: true
-  defp skip?({short_description, warning}, {_, warning, _, short_description}), do: true
-  defp skip?({short_description}, {_, _, _, short_description}), do: true
+  defp skip?({file, warning, line}, {file, warning, line, _, _}), do: true
 
-  defp skip?(%Regex{} = pattern, {_, _, _, short_description}) do
+  defp skip?({file, warning_description}, {file, _, _, _, warning_description})
+       when is_binary(warning_description),
+       do: true
+
+  defp skip?({file, warning}, {file, warning, _, _, _}) when is_atom(warning), do: true
+  defp skip?({file}, {file, _, _, _, _}), do: true
+
+  defp skip?({short_description, warning, line}, {_, warning, line, short_description, _}),
+    do: true
+
+  defp skip?({short_description, warning}, {_, warning, _, short_description, _}), do: true
+  defp skip?({short_description}, {_, _, _, short_description, _}), do: true
+
+  defp skip?(%Regex{} = pattern, {_, _, _, short_description, _}) do
     Regex.match?(pattern, short_description)
   end
 
   defp skip?(_, _), do: false
 
-  def filter_warning?({file, warning, line, short_description}, filter_map = %FilterMap{}) do
+  def filter_warning?(
+        {_, {file, line}, {warning_type, args}} = warning,
+        filter_map = %FilterMap{}
+      ) do
+    short_description = Short.format(warning)
+    warning_description = Utils.warning(warning_type).format_short(args)
+
     {matching_filters, _non_matching_filters} =
       filter_map
       |> FilterMap.filters()
-      |> Enum.split_with(&skip?(&1, {file, warning, line, short_description}))
+      |> Enum.split_with(
+        &skip?(&1, {to_string(file), warning_type, line, short_description, warning_description})
+      )
 
     {not Enum.empty?(matching_filters), matching_filters}
   end

test/dialyxir/formatter_test.exs

@@ -7,6 +7,7 @@ defmodule Dialyxir.FormatterTest do
   alias Dialyxir.Formatter.Dialyxir, as: DialyxirFormatter
   alias Dialyxir.Formatter.Dialyzer, as: DialyzerFormatter
   alias Dialyxir.Formatter.Short, as: ShortFormatter
+  alias Dialyxir.Formatter.IgnoreFileStrict, as: IgnoreFileStrictFormatter
   alias Dialyxir.Project
 
   defp in_project(app, f) when is_atom(app) do
@@ -32,6 +33,24 @@ defmodule Dialyxir.FormatterTest do
       end)
     end
 
+    test "evaluates an ignore file of the form {file, short_description} and ignores warnings matching the pattern" do
+      warnings = [
+        {:warn_return_no_exit, {'lib/poorly_written_code.ex', 10},
+         {:no_return, [:only_normal, :do_a_thing, 1]}},
+        {:warn_return_no_exit, {'lib/poorly_written_code.ex', 20},
+         {:no_return, [:only_normal, :do_something_else, 2]}},
+        {:warn_return_no_exit, {'lib/poorly_written_code.ex', 30},
+         {:no_return, [:only_normal, :do_many_things, 3]}}
+      ]
+
+      in_project(:ignore_strict, fn ->
+        {:ok, remaining, :no_unused_filters} =
+          Formatter.format_and_filter(warnings, Project, [], IgnoreFileStrictFormatter)
+
+        assert remaining == []
+      end)
+    end
+
     test "does not filter lines not matching the pattern" do
       warning =
         {:warn_return_no_exit, {'a/different_file.ex', 17},
@@ -58,7 +77,7 @@ defmodule Dialyxir.FormatterTest do
       end)
     end
 
-    test "lists unnecessary skips as warnings if ignoring exit status " do
+    test "lists unnecessary skips as warnings if ignoring exit status" do
       warning =
         {:warn_return_no_exit, {'a/regex_file.ex', 17},
          {:no_return, [:only_normal, :format_long, 1]}}

test/fixtures/ignore_strict/ignore_strict_test.exs

@@ -0,0 +1,5 @@
+[
+  {"lib/poorly_written_code.ex", "Function do_a_thing/1 has no local return."},
+  {"lib/poorly_written_code.ex", "Function do_something_else/2 has no local return."},
+  {"lib/poorly_written_code.ex", "Function do_many_things/3 has no local return."}
+]

test/fixtures/ignore_strict/mix.exs

@@ -0,0 +1,14 @@
+defmodule IgnoreStrict.Mixfile do
+  use Mix.Project
+
+  def project do
+    [
+      app: :ignore_strict,
+      version: "0.1.0",
+      dialyzer: [
+        ignore_warnings: "ignore_strict_test.exs",
+        list_unused_filters: true
+      ]
+    ]
+  end
+end