Merge pull request #526 from maennchen/jm/fix_file_location_with_column

Fix formatting of file locations including column

Author: jeremyjh

Diff for: lib/dialyxir/formatter.ex

@@ -9,7 +9,8 @@ defmodule Dialyxir.Formatter do
 
   alias Dialyxir.FilterMap
 
-  @type warning() :: {tag :: term(), {file :: Path.t(), line :: pos_integer()}, {atom(), list()}}
+  @type warning() ::
+          {tag :: term(), {file :: Path.t(), location :: :erl_anno.location()}, {atom(), list()}}
 
   @type t() :: module()
 

Diff for: lib/dialyxir/formatter/dialyxir.ex

@@ -6,7 +6,7 @@ defmodule Dialyxir.Formatter.Dialyxir do
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
-  def format(dialyzer_warning = {_tag, {file, line}, message}) do
+  def format(dialyzer_warning = {_tag, {file, location}, message}) do
     {warning_name, arguments} = message
     base_name = Path.relative_to_cwd(file)
 
@@ -16,7 +16,7 @@ defmodule Dialyxir.Formatter.Dialyxir do
         string = warning.format_long(arguments)
 
         """
-        #{base_name}:#{line}:#{warning_name}
+        #{base_name}:#{Utils.format_location(location)}:#{warning_name}
         #{string}
         """
       rescue

Diff for: lib/dialyxir/formatter/github.ex

@@ -6,12 +6,18 @@ defmodule Dialyxir.Formatter.Github do
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
-  def format({_tag, {file, line}, {warning_name, arguments}}) do
+  def format({_tag, {file, location}, {warning_name, arguments}}) do
     base_name = Path.relative_to_cwd(file)
 
     warning = Utils.warning(warning_name)
     string = warning.format_short(arguments)
 
-    "::warning file=#{base_name},line=#{line},title=#{warning_name}::#{string}"
+    case location do
+      {line, col} ->
+        "::warning file=#{base_name},line=#{line},col=#{col},title=#{warning_name}::#{string}"
+
+      line ->
+        "::warning file=#{base_name},line=#{line},title=#{warning_name}::#{string}"
+    end
   end
 end

Diff for: lib/dialyxir/formatter/ignore_file.ex

@@ -4,7 +4,7 @@ defmodule Dialyxir.Formatter.IgnoreFile do
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
-  def format({_tag, {file, _line}, {warning_name, _arguments}}) do
+  def format({_tag, {file, _location}, {warning_name, _arguments}}) do
     ~s({"#{file}", :#{warning_name}},)
   end
 end

Diff for: lib/dialyxir/formatter/ignore_file_strict.ex

@@ -6,7 +6,7 @@ defmodule Dialyxir.Formatter.IgnoreFileStrict do
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
-  def format({_tag, {file, _line}, {warning_name, arguments}}) do
+  def format({_tag, {file, _location}, {warning_name, arguments}}) do
     warning = Utils.warning(warning_name)
     string = warning.format_short(arguments)
 

Diff for: lib/dialyxir/formatter/short.ex

@@ -6,12 +6,12 @@ defmodule Dialyxir.Formatter.Short do
   @behaviour Dialyxir.Formatter
 
   @impl Dialyxir.Formatter
-  def format({_tag, {file, line}, {warning_name, arguments}}) do
+  def format({_tag, {file, location}, {warning_name, arguments}}) do
     base_name = Path.relative_to_cwd(file)
 
     warning = Utils.warning(warning_name)
     string = warning.format_short(arguments)
 
-    "#{base_name}:#{line}:#{warning_name} #{string}"
+    "#{base_name}:#{Utils.format_location(location)}:#{warning_name} #{string}"
   end
 end

Diff for: lib/dialyxir/formatter/utils.ex

@@ -8,4 +8,10 @@ defmodule Dialyxir.Formatter.Utils do
       throw({:error, :unknown_warning, warning_name})
     end
   end
+
+  @doc false
+  @spec format_location(:erl_anno.location()) :: String.t()
+  def format_location(location)
+  def format_location({line, column}), do: "#{line}:#{column}"
+  def format_location(line), do: "#{line}"
 end

Diff for: test/dialyxir/formatter_test.exs

@@ -14,6 +14,46 @@ defmodule Dialyxir.FormatterTest do
     Mix.Project.in_project(app, "test/fixtures/#{Atom.to_string(app)}", fn _ -> f.() end)
   end
 
+  describe "formats dialyzer warning" do
+    if System.otp_release() >= "24" do
+      for {formatter, message} <- %{
+            Formatter.Dialyxir =>
+              "lib/file/warning_type/line.ex:19:4:no_return\nFunction format_long/1 has no local return.",
+            Formatter.Dialyzer =>
+              "lib/file/warning_type/line.ex:19:4: Function format_long/1 has no local return",
+            Formatter.Github =>
+              "::warning file=lib/file/warning_type/line.ex,line=19,col=4,title=no_return::Function format_long/1 has no local return.",
+            Formatter.IgnoreFileStrict =>
+              ~s|{"lib/file/warning_type/line.ex", "Function format_long/1 has no local return."},|,
+            Formatter.IgnoreFile => ~s|{"lib/file/warning_type/line.ex", :no_return},|,
+            # TODO: Remove if once only Elixir ~> 1.15 is supported
+            Formatter.Raw =>
+              if Version.match?(System.version(), "<= 1.15.0") do
+                ~s|{:warn_return_no_exit, {'lib/file/warning_type/line.ex', {19, 4}}, {:no_return, [:only_normal, :format_long, 1]}}|
+              else
+                ~s|{:warn_return_no_exit, {~c"lib/file/warning_type/line.ex", {19, 4}}, {:no_return, [:only_normal, :format_long, 1]}}|
+              end,
+            Formatter.Short =>
+              "lib/file/warning_type/line.ex:19:4:no_return Function format_long/1 has no local return."
+          } do
+        test "file location including column for #{formatter} formatter" do
+          assert {:warn, [message], _unused_filters} =
+                   Formatter.format_and_filter(
+                     [
+                       {:warn_return_no_exit, {~c"lib/file/warning_type/line.ex", {19, 4}},
+                        {:no_return, [:only_normal, :format_long, 1]}}
+                     ],
+                     Project,
+                     [],
+                     unquote(formatter)
+                   )
+
+          assert message =~ unquote(message)
+        end
+      end
+    end
+  end
+
   describe "exs ignore" do
     test "evaluates an ignore file and ignores warnings matching the pattern" do
       warnings = [