From a227dfded9f19569efef410c5ba5817ff6c45f47 Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Thu, 25 Apr 2024 10:51:55 -0400 Subject: [PATCH] Fix incorrect columns for dbg and IO.inspect Previously, this was backfilling the column based on analyzing the source code line, but the column is already available in the AST, so we instead take it from there. This allows IDE type tools to accurately create refactor "code actions". --- lib/credo/check/warning/dbg.ex | 17 +++++++++-------- lib/credo/check/warning/io_inspect.ex | 11 ++++++----- test/credo/check/warning/dbg_test.exs | 18 ++++++++++++++++++ test/credo/check/warning/io_inspect_test.exs | 19 +++++++++++++++++++ 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/lib/credo/check/warning/dbg.ex b/lib/credo/check/warning/dbg.ex index 7c60313e6..20fdd867f 100644 --- a/lib/credo/check/warning/dbg.ex +++ b/lib/credo/check/warning/dbg.ex @@ -32,7 +32,7 @@ defmodule Credo.Check.Warning.Dbg do issues, issue_meta ) do - {ast, [issue_for(issue_meta, meta[:line]) | issues]} + {ast, [issue_for(issue_meta, meta) | issues]} end defp traverse( @@ -40,7 +40,7 @@ defmodule Credo.Check.Warning.Dbg do issues, issue_meta ) do - {ast, [issue_for(issue_meta, meta[:line]) | issues]} + {ast, [issue_for(issue_meta, meta) | issues]} end defp traverse( @@ -48,7 +48,7 @@ defmodule Credo.Check.Warning.Dbg do issues, issue_meta ) do - {ast, [issue_for(issue_meta, meta[:line]) | issues]} + {ast, [issue_for(issue_meta, meta) | issues]} end defp traverse( @@ -56,7 +56,7 @@ defmodule Credo.Check.Warning.Dbg do issues, issue_meta ) do - {ast, [issue_for(issue_meta, meta[:line]) | issues]} + {ast, [issue_for(issue_meta, meta) | issues]} end defp traverse( @@ -64,7 +64,7 @@ defmodule Credo.Check.Warning.Dbg do issues, issue_meta ) do - {ast, [issue_for(issue_meta, meta[:line]) | issues]} + {ast, [issue_for(issue_meta, meta) | issues]} end defp traverse( @@ -72,19 +72,20 @@ defmodule Credo.Check.Warning.Dbg do issues, issue_meta ) do - {ast, [issue_for(issue_meta, meta[:line]) | issues]} + {ast, [issue_for(issue_meta, meta) | issues]} end defp traverse(ast, issues, _issue_meta) do {ast, issues} end - defp issue_for(issue_meta, line_no) do + defp issue_for(issue_meta, meta) do format_issue( issue_meta, message: "There should be no calls to `dbg/1`.", trigger: "dbg", - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/lib/credo/check/warning/io_inspect.ex b/lib/credo/check/warning/io_inspect.ex index c2a3528d9..7853bb4d6 100644 --- a/lib/credo/check/warning/io_inspect.ex +++ b/lib/credo/check/warning/io_inspect.ex @@ -23,7 +23,7 @@ defmodule Credo.Check.Warning.IoInspect do end defp traverse( - {{:., _, [{:__aliases__, _, [:"Elixir", :IO]}, :inspect]}, meta, _arguments} = ast, + {{:., _, [{:__aliases__, meta, [:"Elixir", :IO]}, :inspect]}, _, _arguments} = ast, issues, issue_meta ) do @@ -31,7 +31,7 @@ defmodule Credo.Check.Warning.IoInspect do end defp traverse( - {{:., _, [{:__aliases__, _, [:IO]}, :inspect]}, meta, _arguments} = ast, + {{:., _, [{:__aliases__, meta, [:IO]}, :inspect]}, _meta, _arguments} = ast, issues, issue_meta ) do @@ -43,15 +43,16 @@ defmodule Credo.Check.Warning.IoInspect do end defp issues_for_call(meta, issues, issue_meta) do - [issue_for(issue_meta, meta[:line], @call_string) | issues] + [issue_for(issue_meta, meta, @call_string) | issues] end - defp issue_for(issue_meta, line_no, trigger) do + defp issue_for(issue_meta, meta, trigger) do format_issue( issue_meta, message: "There should be no calls to `IO.inspect/1`.", trigger: trigger, - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/test/credo/check/warning/dbg_test.exs b/test/credo/check/warning/dbg_test.exs index d5d94bdab..8311a62d8 100644 --- a/test/credo/check/warning/dbg_test.exs +++ b/test/credo/check/warning/dbg_test.exs @@ -71,6 +71,24 @@ defmodule Credo.Check.Warning.DbgTest do |> assert_issue() end + test "it should report a violation with two on the same line" do + """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + dbg(parameter1) + dbg(parameter2) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issues(fn [one, two] -> + assert one.line_no == 3 + assert one.column == 23 + assert two.line_no == 3 + assert two.column == 5 + end) + end + test "it should report a violation /2" do """ defmodule CredoSampleModule do diff --git a/test/credo/check/warning/io_inspect_test.exs b/test/credo/check/warning/io_inspect_test.exs index 258c0b1f6..cabf2a575 100644 --- a/test/credo/check/warning/io_inspect_test.exs +++ b/test/credo/check/warning/io_inspect_test.exs @@ -37,6 +37,25 @@ defmodule Credo.Check.Warning.IoInspectTest do |> assert_issue() end + test "it should report a violation with two on the same line" do + """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + foo(IO.inspect(parameter1), parameter2) |> IO.inspect() + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issues(fn [first, second] -> + assert first.line_no == 3 + assert first.column == 48 + + assert second.line_no == 3 + assert second.column == 9 + end) + end + test "it should report a violation /2" do """ defmodule CredoSampleModule do