feat: unused variable code action (#349)

lib/next_ls.ex

@@ -4,6 +4,7 @@ defmodule NextLS do
 
   import NextLS.DB.Query
 
+  alias GenLSP.Enumerations.CodeActionKind
   alias GenLSP.Enumerations.ErrorCodes
   alias GenLSP.Enumerations.TextDocumentSyncKind
   alias GenLSP.ErrorResponse
@@ -16,13 +17,18 @@ defmodule NextLS do
   alias GenLSP.Notifications.WorkspaceDidChangeWorkspaceFolders
   alias GenLSP.Requests.Initialize
   alias GenLSP.Requests.Shutdown
+  alias GenLSP.Requests.TextDocumentCodeAction
   alias GenLSP.Requests.TextDocumentCompletion
   alias GenLSP.Requests.TextDocumentDefinition
   alias GenLSP.Requests.TextDocumentDocumentSymbol
   alias GenLSP.Requests.TextDocumentFormatting
   alias GenLSP.Requests.TextDocumentHover
   alias GenLSP.Requests.TextDocumentReferences
   alias GenLSP.Requests.WorkspaceSymbol
+  alias GenLSP.Structures.CodeActionContext
+  alias GenLSP.Structures.CodeActionOptions
+  alias GenLSP.Structures.CodeActionParams
+  alias GenLSP.Structures.Diagnostic
   alias GenLSP.Structures.DidChangeWatchedFilesParams
   alias GenLSP.Structures.DidChangeWorkspaceFoldersParams
   alias GenLSP.Structures.DidOpenTextDocumentParams
@@ -34,6 +40,7 @@ defmodule NextLS do
   alias GenLSP.Structures.SaveOptions
   alias GenLSP.Structures.ServerCapabilities
   alias GenLSP.Structures.SymbolInformation
+  alias GenLSP.Structures.TextDocumentIdentifier
   alias GenLSP.Structures.TextDocumentItem
   alias GenLSP.Structures.TextDocumentSyncOptions
   alias GenLSP.Structures.TextEdit
@@ -118,6 +125,9 @@ defmodule NextLS do
            save: %SaveOptions{include_text: true},
            change: TextDocumentSyncKind.full()
          },
+         code_action_provider: %CodeActionOptions{
+           code_action_kinds: [CodeActionKind.quick_fix()]
+         },
          completion_provider:
            if init_opts.experimental.completions.enable do
              %GenLSP.Structures.CompletionOptions{
@@ -149,6 +159,26 @@ defmodule NextLS do
      )}
   end
 
+  def handle_request(
+        %TextDocumentCodeAction{
+          params: %CodeActionParams{
+            context: %CodeActionContext{diagnostics: diagnostics},
+            text_document: %TextDocumentIdentifier{uri: uri}
+          }
+        },
+        lsp
+      ) do
+    code_actions =
+      for %Diagnostic{} = diagnostic <- diagnostics,
+          data = %NextLS.CodeActionable.Data{diagnostic: diagnostic, uri: uri, document: lsp.assigns.documents[uri]},
+          namespace = diagnostic.data["namespace"],
+          action <- NextLS.CodeActionable.from(namespace, data) do
+        action
+      end
+
+    {:reply, code_actions, lsp}
+  end
+
   def handle_request(%TextDocumentDefinition{params: %{text_document: %{uri: uri}, position: position}}, lsp) do
     result =
       dispatch(lsp.assigns.registry, :databases, fn entries ->

lib/next_ls/code_actionable.ex

@@ -0,0 +1,29 @@
+defmodule NextLS.CodeActionable do
+  @moduledoc false
+  # A diagnostic can produce 1 or more code actions hence we return a list
+
+  alias GenLSP.Structures.CodeAction
+  alias GenLSP.Structures.Diagnostic
+
+  defmodule Data do
+    @moduledoc false
+    defstruct [:diagnostic, :uri, :document]
+
+    @type t :: %__MODULE__{
+            diagnostic: Diagnostic.t(),
+            uri: String.t(),
+            document: String.t()
+          }
+  end
+
+  @callback from(diagnostic :: Data.t()) :: [CodeAction.t()]
+
+  # TODO: Add support for third party extensions
+  def from("elixir", diagnostic_data) do
+    NextLS.ElixirExtension.CodeAction.from(diagnostic_data)
+  end
+
+  def from("credo", diagnostic_data) do
+    NextLS.CredoExtension.CodeAction.from(diagnostic_data)
+  end
+end

lib/next_ls/extensions/credo_extension.ex

@@ -123,7 +123,7 @@ defmodule NextLS.CredoExtension do
           }
         },
         severity: category_to_severity(issue.category),
-        data: %{check: issue.check, file: issue.filename},
+        data: %{check: issue.check, file: issue.filename, namespace: :credo},
         source: "credo",
         code: Macro.to_string(issue.check),
         code_description: %CodeDescription{

lib/next_ls/extensions/credo_extension/code_action.ex

@@ -0,0 +1,10 @@
+defmodule NextLS.CredoExtension.CodeAction do
+  @moduledoc false
+
+  @behaviour NextLS.CodeActionable
+
+  alias NextLS.CodeActionable.Data
+
+  @impl true
+  def from(%Data{} = _data), do: []
+end

lib/next_ls/extensions/elixir_extension.ex

@@ -44,7 +44,8 @@ defmodule NextLS.ElixirExtension do
         severity: severity(d.severity),
         message: IO.iodata_to_binary(d.message),
         source: d.compiler_name,
-        range: range(d.position, Map.get(d, :span))
+        range: range(d.position, Map.get(d, :span)),
+        data: metadata(d)
       })
     end
 
@@ -111,4 +112,17 @@ defmodule NextLS.ElixirExtension do
   end
 
   def clamp(line), do: max(line, 0)
+
+  @unused_variable ~r/variable\s\"[^\"]+\"\sis\sunused/
+  defp metadata(diagnostic) do
+    base = %{"namespace" => "elixir"}
+
+    cond do
+      is_binary(diagnostic.message) and diagnostic.message =~ @unused_variable ->
+        Map.put(base, "type", "unused_variable")
+
+      true ->
+        base
+    end
+  end
 end

lib/next_ls/extensions/elixir_extension/code_action.ex

@@ -0,0 +1,19 @@
+defmodule NextLS.ElixirExtension.CodeAction do
+  @moduledoc false
+
+  @behaviour NextLS.CodeActionable
+
+  alias NextLS.CodeActionable.Data
+  alias NextLS.ElixirExtension.CodeAction.UnusedVariable
+
+  @impl true
+  def from(%Data{} = data) do
+    case data.diagnostic.data do
+      %{"type" => "unused_variable"} ->
+        UnusedVariable.new(data.diagnostic, data.document, data.uri)
+
+      _ ->
+        []
+    end
+  end
+end

lib/next_ls/extensions/elixir_extension/code_action/unused_variable.ex

@@ -0,0 +1,33 @@
+defmodule NextLS.ElixirExtension.CodeAction.UnusedVariable do
+  @moduledoc false
+
+  alias GenLSP.Structures.CodeAction
+  alias GenLSP.Structures.Diagnostic
+  alias GenLSP.Structures.Range
+  alias GenLSP.Structures.TextEdit
+  alias GenLSP.Structures.WorkspaceEdit
+
+  def new(diagnostic, _text, uri) do
+    %Diagnostic{range: %{start: start}} = diagnostic
+
+    [
+      %CodeAction{
+        title: "Underscore unused variable",
+        diagnostics: [diagnostic],
+        edit: %WorkspaceEdit{
+          changes: %{
+            uri => [
+              %TextEdit{
+                new_text: "_",
+                range: %Range{
+                  start: start,
+                  end: start
+                }
+              }
+            ]
+          }
+        }
+      }
+    ]
+  end
+end

test/next_ls/diagnostics_test.exs

@@ -100,7 +100,8 @@ defmodule NextLS.DiagnosticsTest do
             "range" => %{
               "start" => %{"line" => 3, "character" => ^char},
               "end" => %{"line" => 3, "character" => 14}
-            }
+            },
+            "data" => %{"type" => "unused_variable", "namespace" => "elixir"}
           }
         ]
       }

test/next_ls/extensions/elixir_extension/code_action/unused_variable_test.exs

@@ -0,0 +1,52 @@
+defmodule NextLS.ElixirExtension.UnusedVariableTest do
+  use ExUnit.Case, async: true
+
+  alias GenLSP.Structures.CodeAction
+  alias GenLSP.Structures.Position
+  alias GenLSP.Structures.Range
+  alias GenLSP.Structures.TextEdit
+  alias GenLSP.Structures.WorkspaceEdit
+  alias NextLS.ElixirExtension.CodeAction.UnusedVariable
+
+  test "adds an underscore to unused variables" do
+    text = """
+    defmodule Test.Unused do
+      def hello() do
+        foo = 3
+        :world
+      end
+    end
+    """
+
+    start = %Position{character: 4, line: 3}
+
+    diagnostic = %GenLSP.Structures.Diagnostic{
+      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)",
+      source: "Elixir",
+      range: %GenLSP.Structures.Range{
+        start: start,
+        end: %{start | character: 999}
+      }
+    }
+
+    uri = "file:///home/owner/my_project/hello.ex"
+
+    assert [code_action] = UnusedVariable.new(diagnostic, text, uri)
+    assert is_struct(code_action, CodeAction)
+    assert [diagnostic] == code_action.diagnostics
+
+    # We insert a single underscore character at the start position of the unused variable
+    # Hence the start and end positions are matching the original start position in the diagnostic
+    assert %WorkspaceEdit{
+             changes: %{
+               ^uri => [
+                 %TextEdit{
+                   new_text: "_",
+                   range: %Range{start: ^start, end: ^start}
+                 }
+               ]
+             }
+           } = code_action.edit
+  end
+end

test/next_ls/extensions/elixir_extension/code_action_test.exs

@@ -0,0 +1,77 @@
+defmodule NextLS.Extensions.ElixirExtension.CodeActionTest do
+  use ExUnit.Case, async: true
+
+  import GenLSP.Test
+  import NextLS.Support.Utils
+
+  @moduletag :tmp_dir
+  @moduletag root_paths: ["my_proj"]
+
+  setup %{tmp_dir: tmp_dir} do
+    File.mkdir_p!(Path.join(tmp_dir, "my_proj/lib"))
+    File.write!(Path.join(tmp_dir, "my_proj/mix.exs"), mix_exs())
+
+    cwd = Path.join(tmp_dir, "my_proj")
+
+    foo_path = Path.join(cwd, "lib/foo.ex")
+
+    foo = """
+    defmodule MyProj.Foo do
+      def hello() do
+        foo = :bar
+        :world
+      end
+    end
+    """
+
+    File.write!(foo_path, foo)
+
+    [foo: foo, foo_path: foo_path]
+  end
+
+  setup :with_lsp
+
+  setup context do
+    assert :ok == notify(context.client, %{method: "initialized", jsonrpc: "2.0", params: %{}})
+    assert_is_ready(context, "my_proj")
+    assert_compiled(context, "my_proj")
+    assert_notification "$/progress", %{"value" => %{"kind" => "end", "message" => "Finished indexing!"}}
+
+    did_open(context.client, context.foo_path, context.foo)
+    context
+  end
+
+  test "sends back a list of code actions", %{client: client, foo_path: foo} do
+    foo_uri = uri(foo)
+    id = 1
+
+    request client, %{
+      method: "textDocument/codeAction",
+      id: id,
+      jsonrpc: "2.0",
+      params: %{
+        context: %{
+          "diagnostics" => [
+            %{
+              "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}},
+              "severity" => 2,
+              "source" => "Elixir"
+            }
+          ]
+        },
+        range: %{start: %{line: 2, character: 4}, end: %{line: 2, character: 999}},
+        textDocument: %{uri: foo_uri}
+      }
+    }
+
+    assert_receive %{
+                     "jsonrpc" => "2.0",
+                     "id" => 1,
+                     "result" => [%{"edit" => %{"changes" => %{^foo_uri => [%{"newText" => "_"}]}}}]
+                   },
+                   500
+  end
+end

test/next_ls/extensions/elixir_extension_test.exs

@@ -65,6 +65,7 @@ defmodule NextLS.ElixirExtensionTest do
     assert %{
              with_iodata.file => [
                %GenLSP.Structures.Diagnostic{
+                 data: %{"namespace" => "elixir"},
                  severity: 2,
                  message:
                    "ElixirExtension.foo/0" <>
@@ -84,6 +85,7 @@ defmodule NextLS.ElixirExtensionTest do
              ],
              only_line.file => [
                %GenLSP.Structures.Diagnostic{
+                 data: %{"namespace" => "elixir"},
                  severity: 2,
                  message: "kind of bad",
                  source: "Elixir",
@@ -101,6 +103,7 @@ defmodule NextLS.ElixirExtensionTest do
              ],
              line_and_col.file => [
                %GenLSP.Structures.Diagnostic{
+                 data: %{"namespace" => "elixir"},
                  severity: 1,
                  message: "nothing works",
                  source: "Elixir",
@@ -118,6 +121,7 @@ defmodule NextLS.ElixirExtensionTest do
              ],
              start_and_end.file => [
                %GenLSP.Structures.Diagnostic{
+                 data: %{"namespace" => "elixir"},
                  severity: 4,
                  message: "here's a hint",
                  source: "Elixir",