From 2bc9d232a07ddfaeaf2b0b37bc8cf8fa976ce466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20M=C3=A4nnchen?= Date: Thu, 7 Dec 2023 21:35:27 +0100 Subject: [PATCH] Fix formatting of file locations including column Fixes #466 Fixes #458 Fixes #444 Fixes #435 --- lib/dialyxir/formatter.ex | 3 +- lib/dialyxir/formatter/dialyxir.ex | 4 +- lib/dialyxir/formatter/github.ex | 10 ++++- lib/dialyxir/formatter/ignore_file.ex | 2 +- lib/dialyxir/formatter/ignore_file_strict.ex | 2 +- lib/dialyxir/formatter/short.ex | 4 +- lib/dialyxir/formatter/utils.ex | 6 +++ test/dialyxir/formatter_test.exs | 40 ++++++++++++++++++++ 8 files changed, 62 insertions(+), 9 deletions(-) diff --git a/lib/dialyxir/formatter.ex b/lib/dialyxir/formatter.ex index f5a6140..e1c9a47 100644 --- a/lib/dialyxir/formatter.ex +++ b/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 --git a/lib/dialyxir/formatter/dialyxir.ex b/lib/dialyxir/formatter/dialyxir.ex index 241a936..8136bff 100644 --- a/lib/dialyxir/formatter/dialyxir.ex +++ b/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 --git a/lib/dialyxir/formatter/github.ex b/lib/dialyxir/formatter/github.ex index da306d6..c99a1a1 100644 --- a/lib/dialyxir/formatter/github.ex +++ b/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 --git a/lib/dialyxir/formatter/ignore_file.ex b/lib/dialyxir/formatter/ignore_file.ex index b1bf6cc..c173c79 100644 --- a/lib/dialyxir/formatter/ignore_file.ex +++ b/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 --git a/lib/dialyxir/formatter/ignore_file_strict.ex b/lib/dialyxir/formatter/ignore_file_strict.ex index 49aeb10..6633f45 100644 --- a/lib/dialyxir/formatter/ignore_file_strict.ex +++ b/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 --git a/lib/dialyxir/formatter/short.ex b/lib/dialyxir/formatter/short.ex index 5c5691c..636272b 100644 --- a/lib/dialyxir/formatter/short.ex +++ b/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 --git a/lib/dialyxir/formatter/utils.ex b/lib/dialyxir/formatter/utils.ex index 8f3fd54..c0bee9a 100644 --- a/lib/dialyxir/formatter/utils.ex +++ b/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 --git a/test/dialyxir/formatter_test.exs b/test/dialyxir/formatter_test.exs index cbc7bdf..030e89c 100644 --- a/test/dialyxir/formatter_test.exs +++ b/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 = [