diff --git a/lib/next_ls/extensions/credo_extension/code_action.ex b/lib/next_ls/extensions/credo_extension/code_action.ex index 0a7e6303..3edf79c1 100644 --- a/lib/next_ls/extensions/credo_extension/code_action.ex +++ b/lib/next_ls/extensions/credo_extension/code_action.ex @@ -4,7 +4,23 @@ defmodule NextLS.CredoExtension.CodeAction do @behaviour NextLS.CodeActionable alias NextLS.CodeActionable.Data + alias NextLS.CredoExtension.CodeAction.RemoveDebugger + @debug_checks ~w( + Elixir.Credo.Check.Warning.Dbg + Elixir.Credo.Check.Warning.IExPry + Elixir.Credo.Check.Warning.IoInspect + Elixir.Credo.Check.Warning.IoPuts + Elixir.Credo.Check.Warning.MixEnv + ) @impl true - def from(%Data{} = _data), do: [] + def from(%Data{} = data) do + case data.diagnostic.data do + %{"check" => check} when check in @debug_checks -> + RemoveDebugger.new(data.diagnostic, data.document, data.uri) + + _ -> + [] + end + end end diff --git a/lib/next_ls/extensions/credo_extension/code_action/remove_debugger.ex b/lib/next_ls/extensions/credo_extension/code_action/remove_debugger.ex new file mode 100644 index 00000000..19ce29fc --- /dev/null +++ b/lib/next_ls/extensions/credo_extension/code_action/remove_debugger.ex @@ -0,0 +1,149 @@ +defmodule NextLS.CredoExtension.CodeAction.RemoveDebugger do + @moduledoc false + + alias GenLSP.Structures.CodeAction + alias GenLSP.Structures.Diagnostic + alias GenLSP.Structures.Position + alias GenLSP.Structures.Range + alias GenLSP.Structures.TextEdit + alias GenLSP.Structures.WorkspaceEdit + alias NextLS.EditHelpers + alias Sourceror.Zipper, as: Z + + @line_length 121 + + def new(%Diagnostic{} = diagnostic, text, uri) do + range = diagnostic.range + + with {:ok, ast, comments} <- parse(text), + {:ok, debugger_node} <- find_debugger(ast, range) do + indent = EditHelpers.get_indent(text, range.start.line) + ast_without_debugger = remove_debugger(debugger_node) + range = make_range(debugger_node) + + comments = + Enum.filter(comments, fn comment -> + comment.line > range.start.line && comment.line <= range.end.line + end) + + to_algebra_opts = [comments: comments] + doc = Code.quoted_to_algebra(ast_without_debugger, to_algebra_opts) + formatted = doc |> Inspect.Algebra.format(@line_length) |> IO.iodata_to_binary() + + [ + %CodeAction{ + title: make_title(debugger_node), + diagnostics: [diagnostic], + edit: %WorkspaceEdit{ + changes: %{ + uri => [ + %TextEdit{ + new_text: EditHelpers.add_indent_to_edit(formatted, indent), + range: range + } + ] + } + } + } + ] + else + _ -> + [] + end + end + + defp find_debugger(ast, range) do + pos = %{ + start: [line: range.start.line + 1, column: range.start.character + 1], + end: [line: range.end.line + 1, column: range.end.character + 1] + } + + {_, results} = + ast + |> Z.zip() + |> Z.traverse([], fn tree, acc -> + node = Z.node(tree) + range = Sourceror.get_range(node) + + # range.start <= diagnostic_pos.start <= diagnostic_pos.end <= range.end + if (matches_debug?(node) || matches_pipe?(node)) && range && + Sourceror.compare_positions(range.start, pos.start) in [:lt, :eq] && + Sourceror.compare_positions(range.end, pos.end) in [:gt, :eq] do + {tree, [node | acc]} + else + {tree, acc} + end + end) + + result = + Enum.min_by( + results, + fn node -> + range = Sourceror.get_range(node) + + pos.start[:column] - range.start[:column] + range.end[:column] - pos.end[:column] + end, + fn -> nil end + ) + + result = + Enum.find(results, result, fn + {:|>, _, [_first, ^result]} -> true + _ -> false + end) + + case result do + nil -> {:error, "could find a debugger to remove"} + node -> {:ok, node} + end + end + + defp parse(lines) do + lines + |> Enum.join("\n") + |> Spitfire.parse_with_comments(literal_encoder: &{:ok, {:__block__, &2, [&1]}}) + |> case do + {:error, ast, comments, _errors} -> + {:ok, ast, comments} + + other -> + other + end + end + + defp make_range(original_ast) do + range = Sourceror.get_range(original_ast) + + %Range{ + start: %Position{line: range.start[:line] - 1, character: range.start[:column] - 1}, + end: %Position{line: range.end[:line] - 1, character: range.end[:column] - 1} + } + end + + defp matches_pipe?({:|>, _, [_, arg]}), do: matches_debug?(arg) + defp matches_pipe?(_), do: false + + defp matches_debug?({:dbg, _, _}), do: true + + defp matches_debug?({{:., _, [{:__aliases__, _, [:IO]}, f]}, _, _}) when f in [:puts, :inspect], do: true + + defp matches_debug?({{:., _, [{:__aliases__, _, [:IEx]}, :pry]}, _, _}), do: true + defp matches_debug?({{:., _, [{:__aliases__, _, [:Mix]}, :env]}, _, _}), do: true + defp matches_debug?({{:., _, [{:__aliases__, _, [:Kernel]}, :dbg]}, _, _}), do: true + defp matches_debug?(_), do: false + + defp remove_debugger({:|>, _, [arg, _function]}), do: arg + defp remove_debugger({{:., _, [{:__aliases__, _, [:IO]}, :inspect]}, _, [arg | _]}), do: arg + defp remove_debugger({{:., _, [{:__aliases__, _, [:Kernel]}, :dbg]}, _, [arg | _]}), do: arg + defp remove_debugger({:dbg, _, [arg | _]}), do: arg + defp remove_debugger(_node), do: {:__block__, [], []} + + defp make_title({_, ctx, _} = node), do: "Remove `#{format_node(node)}` #{ctx[:line]}:#{ctx[:column]}" + defp format_node({:|>, _, [_arg, function]}), do: format_node(function) + + defp format_node({{:., _, [{:__aliases__, _, [module]}, function]}, _, args}), + do: "&#{module}.#{function}/#{Enum.count(args)}" + + defp format_node({:dbg, _, args}), do: "&dbg/#{Enum.count(args)}" + defp format_node(node), do: Macro.to_string(node) +end diff --git a/mix.exs b/mix.exs index 44d844eb..90c1c351 100644 --- a/mix.exs +++ b/mix.exs @@ -76,7 +76,7 @@ defmodule NextLS.MixProject do {:burrito, "~> 1.0", only: [:dev, :prod]}, {:bypass, "~> 2.1", only: :test}, {:dialyxir, ">= 0.0.0", only: [:dev, :test], runtime: false}, - {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, + {:credo, github: "rrrene/credo", branch: "master", only: [:dev, :test], runtime: false}, {:ex_doc, ">= 0.0.0", only: :dev}, {:styler, "~> 0.8", only: :dev} ] diff --git a/mix.lock b/mix.lock index b4c863cd..bd9d3256 100644 --- a/mix.lock +++ b/mix.lock @@ -9,7 +9,7 @@ "cowboy": {:hex, :cowboy, "2.10.0", "ff9ffeff91dae4ae270dd975642997afe2a1179d94b1887863e43f681a203e26", [:make, :rebar3], [{:cowlib, "2.12.1", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "3afdccb7183cc6f143cb14d3cf51fa00e53db9ec80cdcd525482f5e99bc41d6b"}, "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"}, "cowlib": {:hex, :cowlib, "2.12.1", "a9fa9a625f1d2025fe6b462cb865881329b5caff8f1854d1cbc9f9533f00e1e1", [:make, :rebar3], [], "hexpm", "163b73f6367a7341b33c794c4e88e7dbfe6498ac42dcd69ef44c5bc5507c8db0"}, - "credo": {:hex, :credo, "1.7.5", "643213503b1c766ec0496d828c90c424471ea54da77c8a168c725686377b9545", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "f799e9b5cd1891577d8c773d245668aa74a2fcd15eb277f51a0131690ebfb3fd"}, + "credo": {:git, "https://github.com/rrrene/credo.git", "3612dc2417455c2487043bf947b048b90a882b47", [branch: "master"]}, "ctx": {:hex, :ctx, "0.6.0", "8ff88b70e6400c4df90142e7f130625b82086077a45364a78d208ed3ed53c7fe", [:rebar3], [], "hexpm", "a14ed2d1b67723dbebbe423b28d7615eb0bdcba6ff28f2d1f1b0a7e1d4aa5fc2"}, "db_connection": {:hex, :db_connection, "2.5.0", "bb6d4f30d35ded97b29fe80d8bd6f928a1912ca1ff110831edcd238a1973652c", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c92d5ba26cd69ead1ff7582dbb860adeedfff39774105a4f1c92cbb654b55aa2"}, "dialyxir": {:hex, :dialyxir, "1.4.3", "edd0124f358f0b9e95bfe53a9fcf806d615d8f838e2202a9f430d59566b6b53b", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "bf2cfb75cd5c5006bec30141b131663299c661a864ec7fbbc72dfa557487a986"}, diff --git a/test/next_ls/extensions/credo_extension/remove_debugger_test.exs b/test/next_ls/extensions/credo_extension/remove_debugger_test.exs new file mode 100644 index 00000000..64987012 --- /dev/null +++ b/test/next_ls/extensions/credo_extension/remove_debugger_test.exs @@ -0,0 +1,346 @@ +defmodule NextLS.CredoExtension.CodeAction.RemoveDebuggerTest do + use ExUnit.Case, async: true + + import NextLS.Support.Utils, only: [assert_is_text_edit: 3] + + alias GenLSP.Structures.CodeAction + alias GenLSP.Structures.Position + alias GenLSP.Structures.Range + alias GenLSP.Structures.TextEdit + alias GenLSP.Structures.WorkspaceEdit + alias NextLS.CredoExtension.CodeAction.RemoveDebugger + + @uri "file:///home/owner/my_project/hello.ex" + + test "removes debugger checks" do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + IO.inspect(arg) + foo(arg) + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + arg + foo(arg) + end + end + """) + + start = %Position{character: 4, line: 2} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + assert is_struct(code_action, CodeAction) + assert [diagnostic] == code_action.diagnostics + assert code_action.title == "Remove `&IO.inspect/1` 3:8" + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + test "works for all credo checks" do + checks = [ + {"Elixir.Credo.Check.Warning.Dbg", "dbg()", "dbg/0", ""}, + {"Elixir.Credo.Check.Warning.Dbg", "dbg(arg)", "dbg/1", "arg"}, + {"Elixir.Credo.Check.Warning.Dbg", "Kernel.dbg()", "Kernel.dbg/0", ""}, + {"Elixir.Credo.Check.Warning.IExPry", "IEx.pry()", "IEx.pry/0", ""}, + {"Elixir.Credo.Check.Warning.IoInspect", "IO.inspect(foo, label: ~s/bar/)", "IO.inspect/2", "foo"}, + {"Elixir.Credo.Check.Warning.IoPuts", "IO.puts(arg)", "IO.puts/1", ""}, + {"Elixir.Credo.Check.Warning.MixEnv", "Mix.env()", "Mix.env/0", ""} + ] + + for {check, code, title, edit} <- checks do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + #{code} + arg + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + #{edit} + arg + end + end + """) + + start = %Position{character: 4, line: 2} + diagnostic = get_diagnostic(start, check: check, code: code) + title = "Remove `&#{title}` #{start.line + 1}" + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + assert code_action.title =~ title + + assert %WorkspaceEdit{ + changes: %{ + @uri => [%TextEdit{} = edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + end + + test "works on multiple expressions on one line" do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + IO.inspect(arg, label: "Debugging"); world(arg) + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + arg; world(arg) + end + end + """) + + start = %Position{character: 4, line: 2} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + test "handles pipe calls in the middle" do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + arg + |> Enum.map(&(&1 * &1)) + |> IO.inspect(label: "FOO") + |> Enum.sum() + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + arg + |> Enum.map(&(&1 * &1)) + |> Enum.sum() + end + end + """) + + start = %Position{character: 10, line: 4} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + test "handles pipe calls at the end" do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + arg + |> Enum.map(&(&1 * &1)) + |> Enum.sum() + |> IO.inspect() + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + arg + |> Enum.map(&(&1 * &1)) + |> Enum.sum() + end + end + """) + + start = %Position{character: 10, line: 5} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + test "handles pipe calls after an expr" do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + arg |> IO.inspect() + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + arg + end + end + """) + + start = %Position{character: 10, line: 2} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + test "handles functions with only one expression" do + text = + String.split( + """ + defmodule Test.Debug do + def hello(arg) do + IO.inspect(arg, label: "DEBUG") + end + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + def hello(arg) do + arg + end + end + """) + + start = %Position{character: 4, line: 2} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + test "handles inspects in module bodies" do + text = + String.split( + """ + defmodule Test.Debug do + @attr "foo" + IO.inspect(@attr) + end + """, + "\n" + ) + + expected = + String.trim(""" + defmodule Test.Debug do + @attr "foo" + @attr + end + """) + + start = %Position{character: 4, line: 2} + diagnostic = get_diagnostic(start) + + assert [code_action] = RemoveDebugger.new(diagnostic, text, @uri) + + assert %WorkspaceEdit{ + changes: %{ + @uri => [edit] + } + } = code_action.edit + + assert_is_text_edit(text, edit, expected) + end + + defp get_diagnostic(start, opts \\ []) do + check = Keyword.get(opts, :check, "Elixir.Credo.Check.Warning.IoInspect") + code = Keyword.get(opts, :code, "IO.inspect/2") + + %GenLSP.Structures.Diagnostic{ + data: %{"namespace" => "credo", "check" => check}, + message: "There should be no calls to `#{code}`", + source: "Elixir", + range: %Range{ + start: start, + end: start + } + } + end +end diff --git a/test/next_ls/extensions/elixir_extension/code_action_test.exs b/test/next_ls/extensions/elixir_extension/code_action_test.exs index 09fd4846..58e86d9b 100644 --- a/test/next_ls/extensions/elixir_extension/code_action_test.exs +++ b/test/next_ls/extensions/elixir_extension/code_action_test.exs @@ -14,6 +14,7 @@ defmodule NextLS.Extensions.ElixirExtension.CodeActionTest do cwd = Path.join(tmp_dir, "my_proj") foo_path = Path.join(cwd, "lib/foo.ex") + bar_path = Path.join(cwd, "lib/bar.ex") foo = """ defmodule MyProj.Foo do @@ -28,9 +29,20 @@ defmodule NextLS.Extensions.ElixirExtension.CodeActionTest do end """ + bar = """ + defmodule MyProj.Bar do + def foo() do + a = :bar + foo(dbg(a), IO.inspect(a)) + a + end + end + """ + File.write!(foo_path, foo) + File.write!(bar_path, bar) - [foo: foo, foo_path: foo_path] + [foo: foo, foo_path: foo_path, bar: bar, bar_path: bar_path] end setup :with_lsp @@ -42,6 +54,7 @@ defmodule NextLS.Extensions.ElixirExtension.CodeActionTest do assert_notification "$/progress", %{"value" => %{"kind" => "end", "message" => "Finished indexing!"}} did_open(context.client, context.foo_path, context.foo) + did_open(context.client, context.bar_path, context.bar) context end @@ -60,7 +73,7 @@ defmodule NextLS.Extensions.ElixirExtension.CodeActionTest do "data" => %{"namespace" => "elixir", "type" => "unused_variable"}, "message" => "variable \"foo\" is unused (if the variable is not meant to be used, prefix it with an underscore)", - "range" => %{"end" => %{"character" => 999, "line" => 2}, "start" => %{"character" => 4, "line" => 2}}, + "range" => %{"end" => %{"character" => 999, "line" => 3}, "start" => %{"character" => 4, "line" => 3}}, "severity" => 2, "source" => "Elixir" } @@ -122,4 +135,50 @@ defmodule NextLS.Extensions.ElixirExtension.CodeActionTest do }, 500 end + + test "sends back a remove inspect action", %{client: client, bar_path: bar} do + bar_uri = uri(bar) + id = 1 + + request client, %{ + method: "textDocument/codeAction", + id: id, + jsonrpc: "2.0", + params: %{ + context: %{ + "diagnostics" => [ + %{ + "data" => %{"namespace" => "credo", "check" => "Elixir.Credo.Check.Warning.Dbg"}, + "message" => "There should be no calls to `dbg/1`.", + "range" => %{"end" => %{"character" => 13, "line" => 3}, "start" => %{"character" => 8, "line" => 3}}, + "severity" => 2, + "source" => "Elixir" + }, + %{ + "data" => %{"namespace" => "credo", "check" => "Elixir.Credo.Check.Warning.IoInspect"}, + "message" => "There should be no calls to `IO.inspect/1`.", + "range" => %{"end" => %{"character" => 28, "line" => 3}, "start" => %{"character" => 20, "line" => 3}}, + "severity" => 2, + "source" => "Elixir" + } + ] + }, + range: %{start: %{line: 0, character: 0}, end: %{line: 7, character: 999}}, + textDocument: %{uri: bar_uri} + } + } + + assert_receive %{ + "jsonrpc" => "2.0", + "id" => 1, + "result" => [ + %{"edit" => %{"changes" => %{^bar_uri => [%{"newText" => "a", "range" => range1}]}}}, + %{"edit" => %{"changes" => %{^bar_uri => [%{"newText" => "a", "range" => range2}]}}} + ] + }, + 500 + + assert %{"start" => %{"character" => 8, "line" => 3}, "end" => %{"character" => 14, "line" => 3}} == range1 + assert %{"start" => %{"character" => 16, "line" => 3}, "end" => %{"character" => 29, "line" => 3}} == range2 + end end diff --git a/test/support/utils.ex b/test/support/utils.ex index fe063d22..c470c3f7 100644 --- a/test/support/utils.ex +++ b/test/support/utils.ex @@ -4,6 +4,10 @@ defmodule NextLS.Support.Utils do import ExUnit.Callbacks import GenLSP.Test + alias GenLSP.Structures.Position + alias GenLSP.Structures.Range + alias GenLSP.Structures.TextEdit + def mix_exs do """ defmodule Project.MixProject do @@ -172,4 +176,32 @@ defmodule NextLS.Support.Utils do }) end end + + def apply_edit(code, edit) when is_binary(code), do: apply_edit(String.split(code, "\n"), edit) + + def apply_edit(lines, %TextEdit{} = edit) when is_list(lines) do + text = edit.new_text + %Range{start: %Position{line: startl, character: startc}, end: %Position{line: endl, character: endc}} = edit.range + + startl_text = Enum.at(lines, startl) + prefix = String.slice(startl_text, 0, startc) + + endl_text = Enum.at(lines, endl) + suffix = String.slice(endl_text, endc, String.length(endl_text) - endc) + + replacement = prefix <> text <> suffix + + new_lines = Enum.slice(lines, 0, startl) ++ [replacement] ++ Enum.slice(lines, endl + 1, Enum.count(lines)) + + new_lines + |> Enum.join("\n") + |> String.trim() + end + + defmacro assert_is_text_edit(code, edit, expected) do + quote do + actual = unquote(__MODULE__).apply_edit(unquote(code), unquote(edit)) + assert actual == unquote(expected) + end + end end