Repository: rrrene/credo
Pull request #1126 (2024-04-25 14:52:15)

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".



Patch:
diff --git a/lib/credo/check/readability/function_names.ex b/lib/credo/check/readability/function_names.ex
index c6e09378..5c91c44a 100644
--- a/lib/credo/check/readability/function_names.ex
+++ b/lib/credo/check/readability/function_names.ex
@@ -149,7 +149,14 @@ defmodule Credo.Check.Readability.FunctionNames do
     issues
   end
 
-  defp issues_for_name("sigil_" <> sigil_letters = name, args, meta, issues, issue_meta, _allow_acronyms?) do
+  defp issues_for_name(
+         "sigil_" <> sigil_letters = name,
+         args,
+         meta,
+         issues,
+         issue_meta,
+         _allow_acronyms?
+       ) do
     multi_letter_sigil? = String.match?(sigil_letters, ~r/^[A-Z]+$/)
 
     if multi_letter_sigil? do
diff --git a/lib/credo/check/warning/dbg.ex b/lib/credo/check/warning/dbg.ex
index 7c60313e..20fdd867 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 c2a3528d..7853bb4d 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


Test patch:
diff --git a/test/credo/check/warning/dbg_test.exs b/test/credo/check/warning/dbg_test.exs
index d5d94bda..8311a62d 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 258c0b1f..cabf2a57 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