diff --git a/extensions/vscode/src/extension.ts b/extensions/vscode/src/extension.ts index 00d3453..9ef5bbd 100644 --- a/extensions/vscode/src/extension.ts +++ b/extensions/vscode/src/extension.ts @@ -1,5 +1,10 @@ -import { LanguageClient } from "vscode-languageclient/node"; -import { workspace, ExtensionContext } from "vscode"; +import { + CancellationToken, + LanguageClient, + ResolveCodeActionSignature, +} from "vscode-languageclient/node"; +import { ExtensionContext, CodeAction, window, workspace } from "vscode"; + import { ChildProcess, exec } from "child_process"; import * as path from "path"; import * as net from "net"; @@ -40,6 +45,7 @@ export async function activate(context: ExtensionContext) { synchronize: { fileEvents: workspace.createFileSystemWatcher("**/.clientrc"), }, + middleware: { resolveCodeAction }, } ); @@ -142,3 +148,36 @@ function connect( doConnect(); }); } + +async function resolveCodeAction( + item: CodeAction, + token: CancellationToken, + next: ResolveCodeActionSignature +) { + if (willNameNewResource(item)) { + const resource = item.title.split(" ").pop(); + + const newName = await window.showInputBox({ + title: item.title, + prompt: `Choose a name for new ${resource}`, + }); + + // user canceled the input box, so we + // are also canceling the code action + if (!newName) return undefined; + + (item as any).data.new_name = newName; + } + + return next(item, token); +} + +function willNameNewResource(item: CodeAction) { + return [ + "Extract anonymous function", + "Extract constant", + "Extract guard", + "Extract function", + "Extract variable", + ].includes(item.title); +} diff --git a/lib/refactorex.ex b/lib/refactorex.ex index b060113..9fa0ac9 100644 --- a/lib/refactorex.ex +++ b/lib/refactorex.ex @@ -20,6 +20,7 @@ defmodule Refactorex do alias __MODULE__.{ Diff, Logger, + NameCache, Parser, Refactor, Response @@ -118,9 +119,10 @@ defmodule Refactorex do end def do_handle_request(%CodeActionResolve{params: %{data: data}}, lsp) do - %{module: module, uri: uri, range: range} = Parser.parse_metadata(data) + %{module: module, uri: uri, range: range} = metadata = Parser.parse_metadata(data) original = lsp.assigns.documents[uri] + NameCache.store_name(metadata[:new_name]) with {:ok, zipper, selection_or_line} <- Parser.parse_inputs(original, range) do { diff --git a/lib/refactorex/application.ex b/lib/refactorex/application.ex index eb8aed0..b726e4b 100644 --- a/lib/refactorex/application.ex +++ b/lib/refactorex/application.ex @@ -12,6 +12,7 @@ defmodule Refactorex.Application do [communication: {GenLSP.Communication.TCP, [port: port()]}] }, {Refactorex.Logger, []}, + {Refactorex.NameCache, []}, {Refactorex, []} ], strategy: :one_for_one, diff --git a/lib/refactorex/name_cache.ex b/lib/refactorex/name_cache.ex new file mode 100644 index 0000000..a1996a6 --- /dev/null +++ b/lib/refactorex/name_cache.ex @@ -0,0 +1,17 @@ +defmodule Refactorex.NameCache do + use Agent + + def start_link(_), do: Agent.start_link(fn -> nil end, name: __MODULE__) + + def store_name(new_name) when is_bitstring(new_name) do + new_name + |> String.replace(~r/[^a-zA-Z0-9_?!]/, "") + |> String.to_atom() + |> store_name() + end + + def store_name(new_name), do: Agent.update(__MODULE__, fn _ -> new_name end) + + def consume_name_or(namer_fn), + do: Agent.get_and_update(__MODULE__, &{&1 || namer_fn.(), nil}) +end diff --git a/lib/refactorex/refactor/constant/extract_constant.ex b/lib/refactorex/refactor/constant/extract_constant.ex index 40846b6..3aca59f 100644 --- a/lib/refactorex/refactor/constant/extract_constant.ex +++ b/lib/refactorex/refactor/constant/extract_constant.ex @@ -4,6 +4,8 @@ defmodule Refactorex.Refactor.Constant.ExtractConstant do kind: "refactor.extract", works_on: :selection + alias Refactorex.NameCache + alias Refactorex.Refactor.{ Module, Variable @@ -44,12 +46,14 @@ defmodule Refactorex.Refactor.Constant.ExtractConstant do end def next_available_constant_name(zipper) do - Module.next_available_name( - zipper, - @constant_name, - &match?({:@, _, _}, &1), - fn {_, _, [{name, _, _}]} -> name end - ) + NameCache.consume_name_or(fn -> + Module.next_available_name( + zipper, + @constant_name, + &match?({:@, _, _}, &1), + fn {_, _, [{name, _, _}]} -> name end + ) + end) end defp after_used_constants(nodes, node) do diff --git a/lib/refactorex/refactor/function.ex b/lib/refactorex/refactor/function.ex index 5fbd567..dca478f 100644 --- a/lib/refactorex/refactor/function.ex +++ b/lib/refactorex/refactor/function.ex @@ -1,5 +1,6 @@ defmodule Refactorex.Refactor.Function do alias Sourceror.Zipper, as: Z + alias Refactorex.NameCache alias Refactorex.Refactor.Module import Sourceror.Identifier @@ -70,12 +71,14 @@ defmodule Refactorex.Refactor.Function do end def next_available_function_name(zipper, base_name) do - Module.next_available_name( - zipper, - base_name, - &definition?/1, - fn {_, _, [{name, _, _}, _]} -> name end - ) + NameCache.consume_name_or(fn -> + Module.next_available_name( + zipper, + base_name, + &definition?/1, + fn {_, _, [{name, _, _}, _]} -> name end + ) + end) end def go_to_block(zipper) do diff --git a/lib/refactorex/refactor/guard/extract_guard.ex b/lib/refactorex/refactor/guard/extract_guard.ex index 0faeba2..840b681 100644 --- a/lib/refactorex/refactor/guard/extract_guard.ex +++ b/lib/refactorex/refactor/guard/extract_guard.ex @@ -4,6 +4,8 @@ defmodule Refactorex.Refactor.Guard.ExtractGuard do kind: "refactor.extract", works_on: :selection + alias Refactorex.NameCache + alias Refactorex.Refactor.{ Guard, Module, @@ -38,11 +40,13 @@ defmodule Refactorex.Refactor.Guard.ExtractGuard do end defp next_available_guard_name(zipper) do - Module.next_available_name( - zipper, - @guard_name, - &Guard.definition?/1, - fn {_, _, [{:when, _, [{name, _, _} | _]}]} -> name end - ) + NameCache.consume_name_or(fn -> + Module.next_available_name( + zipper, + @guard_name, + &Guard.definition?/1, + fn {_, _, [{:when, _, [{name, _, _} | _]}]} -> name end + ) + end) end end diff --git a/lib/refactorex/refactor/variable/extract_variable.ex b/lib/refactorex/refactor/variable/extract_variable.ex index 4a7bdcd..ca9faa6 100644 --- a/lib/refactorex/refactor/variable/extract_variable.ex +++ b/lib/refactorex/refactor/variable/extract_variable.ex @@ -4,6 +4,8 @@ defmodule Refactorex.Refactor.Variable.ExtractVariable do kind: "refactor.extract", works_on: :selection + alias Refactorex.NameCache + alias Refactorex.Refactor.{ Function, IfElse, @@ -103,31 +105,33 @@ defmodule Refactorex.Refactor.Variable.ExtractVariable do end defp next_available_name(zipper) do - zipper - |> Z.top() - |> Z.traverse([0], fn - %{node: {id, _, nil}} = zipper, used_numbers when is_atom(id) -> - { - zipper, - case Regex.run(~r/#{@variable_name}(\d*)/, Atom.to_string(id)) do - [_, ""] -> - [1 | used_numbers] - - [_, i] -> - [String.to_integer(i) | used_numbers] - - _ -> - used_numbers - end - } - - zipper, used_numbers -> - {zipper, used_numbers} + NameCache.consume_name_or(fn -> + zipper + |> Z.top() + |> Z.traverse([0], fn + %{node: {id, _, nil}} = zipper, used_numbers when is_atom(id) -> + { + zipper, + case Regex.run(~r/#{@variable_name}(\d*)/, Atom.to_string(id)) do + [_, ""] -> + [1 | used_numbers] + + [_, i] -> + [String.to_integer(i) | used_numbers] + + _ -> + used_numbers + end + } + + zipper, used_numbers -> + {zipper, used_numbers} + end) + |> elem(1) + |> Enum.max() + |> then(&"#{@variable_name}#{if &1 == 0, do: "", else: &1 + 1}") + |> String.to_atom() end) - |> elem(1) - |> Enum.max() - |> then(&"#{@variable_name}#{if &1 == 0, do: "", else: &1 + 1}") - |> String.to_atom() end defp replace_selection_by_variable(statement, selection, variable) do diff --git a/test/refactorex_test.exs b/test/refactorex_test.exs index 653172a..0076c23 100644 --- a/test/refactorex_test.exs +++ b/test/refactorex_test.exs @@ -131,6 +131,51 @@ defmodule RefactorexTest do ) end + test "allows some Extract CodeActions to name the new resource", %{ + client: client, + file_uri: file_uri + } do + :ok = + request_code_actions(client, file_uri, %{ + # selecting arg usage + start: %{line: 2, character: 4}, + end: %{line: 2, character: 7} + }) + + assert_result(2, code_actions, @timeout) + + assert extract_variable = Enum.find(code_actions, &(&1["title"] == "Extract variable")) + + :ok = + request(client, %{ + method: "codeAction/resolve", + id: 3, + jsonrpc: "2.0", + params: put_in(extract_variable, ~w(data new_name), "foo") + }) + + assert_result( + 3, + %{ + "title" => "Extract variable", + "edit" => %{ + "changes" => %{ + ^file_uri => [ + %{ + "newText" => " foo = arg\n foo", + "range" => %{ + "start" => %{"line" => 2, "character" => 0}, + "end" => %{"line" => 2, "character" => 7} + } + } + ] + } + } + }, + @timeout + ) + end + test "responds no CodeActions for syntactically broken file", %{ client: client, file_uri: file_uri @@ -223,17 +268,21 @@ defmodule RefactorexTest do ) end - defp request_code_actions(client, file_uri) do + defp request_code_actions( + client, + file_uri, + range \\ %{ + start: %{line: 1, character: 4}, + end: %{line: 1, character: 4} + } + ) do request(client, %{ method: "textDocument/codeAction", id: 2, jsonrpc: "2.0", params: %{ textDocument: %{uri: file_uri}, - range: %{ - start: %{line: 1, character: 4}, - end: %{line: 1, character: 4} - }, + range: range, context: %{ diagnostics: [], triggerKind: 1