From a227dfded9f19569efef410c5ba5817ff6c45f47 Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Thu, 25 Apr 2024 10:51:55 -0400 Subject: [PATCH 1/2] 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 From 199eca42f4f022af52e5addefce1f58721edc4bc Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Sun, 28 Apr 2024 16:45:35 -0400 Subject: [PATCH 2/2] fix formatting I assume this is broken on the default branch, just fixing it so my build with pass --- lib/credo/check/readability/function_names.ex | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/credo/check/readability/function_names.ex b/lib/credo/check/readability/function_names.ex index c6e093782..5c91c44a7 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