From e4ba94ef316d62cf05c63bf60f73f5f1b802547d Mon Sep 17 00:00:00 2001 From: Scott Ming Date: Fri, 26 Apr 2024 18:45:19 +0800 Subject: [PATCH] Rename file when renaming module (#651) * Rename the file while renaming the module. It first determines whether the module meets certain hard constraints, such as having no parent, then performs some conventional checks. If both are satisfied, it proceeds to return the file renaming. * Maybe insert special folder for PhoenixWeb's module * Apply some code review changes for using `Document.Changes` Use `Document.Changes` instead of `CodeMod.Rename.DocumentChanges` Make `Document.Changes.RenameFile` a struct and use `root_module?` instead of `has_silibings?` and `has_parent?` * Use `rename progress marker` instead of suspending build * Use `file_changed` instead of `file_compile_requested` for reindexing * `uri_with_operations_counts` by client name * Apply some code review suggestions for `rename/file` * Apply suggestions from code review Co-authored-by: Steve Cohen * Do not need to care about the `DidClose` message * Apply some code review suggestions 1. Move the `if-else` logic to the `Commands.Rename` module 2. Add `file_opened` message type 3. modify the format error message * Shouldn't returns `RenameFile` if the file name not changed * Change `uri_with_operation_counts` to `uri_with_expected_operation` and special some message type for emacs --------- Co-authored-by: Steve Cohen --- .../lib/lexical/remote_control/api.ex | 26 +- .../lexical/remote_control/api/messages.ex | 8 + .../lib/lexical/remote_control/application.ex | 1 + .../lib/lexical/remote_control/build.ex | 17 ++ .../code_intelligence/entity.ex | 24 +- .../lexical/remote_control/code_mod/format.ex | 12 +- .../lexical/remote_control/code_mod/rename.ex | 50 +++- .../remote_control/code_mod/rename/file.ex | 154 ++++++++++++ .../remote_control/code_mod/rename/module.ex | 30 ++- .../remote_control/commands/reindex.ex | 19 +- .../lexical/remote_control/commands/rename.ex | 74 ++++++ .../dispatch/handlers/indexing.ex | 15 +- .../remote_control/code_mod/format_test.exs | 2 + .../remote_control/code_mod/rename_test.exs | 235 ++++++++++++++++-- .../remote_control/commands/rename_test.exs | 42 ++++ .../dispatch/handlers/indexer_test.exs | 6 +- .../server/lib/lexical/server/provider/env.ex | 7 +- .../server/provider/handlers/rename.ex | 51 +++- apps/server/lib/lexical/server/state.ex | 10 +- .../server/provider/handlers/rename_test.exs | 53 ++-- .../lib/lexical/document/changes.ex | 22 +- 21 files changed, 770 insertions(+), 88 deletions(-) create mode 100644 apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex create mode 100644 apps/remote_control/lib/lexical/remote_control/commands/rename.ex create mode 100644 apps/remote_control/test/lexical/remote_control/commands/rename_test.exs diff --git a/apps/remote_control/lib/lexical/remote_control/api.ex b/apps/remote_control/lib/lexical/remote_control/api.ex index ebdf336ce..d97924533 100644 --- a/apps/remote_control/lib/lexical/remote_control/api.ex +++ b/apps/remote_control/lib/lexical/remote_control/api.ex @@ -16,6 +16,21 @@ defmodule Lexical.RemoteControl.Api do defdelegate schedule_compile(project, force?), to: Build defdelegate compile_document(project, document), to: Build + def maybe_schedule_compile(project, triggered_file_uri, triggered_message) do + RemoteControl.call(project, Build, :maybe_schedule_compile, [ + triggered_file_uri, + triggered_message + ]) + end + + def maybe_compile_document(project, document, triggered_message) do + RemoteControl.call(project, Build, :maybe_compile_document, [ + project, + document, + triggered_message + ]) + end + def expand_alias( %Project{} = project, segments_or_module, @@ -55,11 +70,18 @@ defmodule Lexical.RemoteControl.Api do RemoteControl.call(project, CodeMod.Rename, :prepare, [analysis, position]) end - def rename(%Project{} = project, %Analysis{} = analysis, %Position{} = position, new_name) do + def rename( + %Project{} = project, + %Analysis{} = analysis, + %Position{} = position, + new_name, + client_name + ) do RemoteControl.call(project, CodeMod.Rename, :rename, [ analysis, position, - new_name + new_name, + client_name ]) end diff --git a/apps/remote_control/lib/lexical/remote_control/api/messages.ex b/apps/remote_control/lib/lexical/remote_control/api/messages.ex index fa1baaad7..861b2d7d0 100644 --- a/apps/remote_control/lib/lexical/remote_control/api/messages.ex +++ b/apps/remote_control/lib/lexical/remote_control/api/messages.ex @@ -15,6 +15,8 @@ defmodule Lexical.RemoteControl.Api.Messages do defrecord :file_changed, uri: nil, from_version: nil, to_version: nil, open?: false + defrecord :file_opened, uri: nil, version: nil + defrecord :file_compile_requested, project: nil, build_number: 0, uri: nil defrecord :file_compiled, @@ -79,6 +81,12 @@ defmodule Lexical.RemoteControl.Api.Messages do open?: boolean() ) + @type file_opened :: + record(:file_opened, + uri: Lexical.uri(), + version: non_neg_integer() + ) + @type file_compile_requested :: record(:file_compile_requested, project: Lexical.Project.t(), diff --git a/apps/remote_control/lib/lexical/remote_control/application.ex b/apps/remote_control/lib/lexical/remote_control/application.ex index b7e364f64..7ca4f2f12 100644 --- a/apps/remote_control/lib/lexical/remote_control/application.ex +++ b/apps/remote_control/lib/lexical/remote_control/application.ex @@ -14,6 +14,7 @@ defmodule Lexical.RemoteControl.Application do if RemoteControl.project_node?() do [ {RemoteControl.Commands.Reindex, nil}, + RemoteControl.Commands.Rename, RemoteControl.Module.Loader, {RemoteControl.Dispatch, progress: true}, RemoteControl.ModuleMappings, diff --git a/apps/remote_control/lib/lexical/remote_control/build.ex b/apps/remote_control/lib/lexical/remote_control/build.ex index 2d01d61fb..1819ff3d0 100644 --- a/apps/remote_control/lib/lexical/remote_control/build.ex +++ b/apps/remote_control/lib/lexical/remote_control/build.ex @@ -4,6 +4,7 @@ defmodule Lexical.RemoteControl.Build do alias Lexical.RemoteControl alias Lexical.RemoteControl.Build.Document.Compilers.HEEx alias Lexical.RemoteControl.Build.State + alias Lexical.RemoteControl.Commands.Rename alias Lexical.VM.Versions require Logger @@ -35,6 +36,22 @@ defmodule Lexical.RemoteControl.Build do :ok end + def maybe_schedule_compile(triggered_file_uri, message) do + if Rename.in_progress?() do + Rename.update_progress(triggered_file_uri, message) + else + GenServer.cast(__MODULE__, {:compile, false}) + end + end + + def maybe_compile_document(%Project{} = project, %Document{} = document, message) do + if Rename.in_progress?() do + Rename.update_progress(document.uri, message) + else + compile_document(project, document) + end + end + # this is for testing def force_compile_document(%Project{} = project, %Document{} = document) do with false <- Path.absname(document.path) == "mix.exs", diff --git a/apps/remote_control/lib/lexical/remote_control/code_intelligence/entity.ex b/apps/remote_control/lib/lexical/remote_control/code_intelligence/entity.ex index 1b5a09f94..470b58e89 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_intelligence/entity.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_intelligence/entity.ex @@ -52,6 +52,22 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Entity do ) end + @spec phoenix_controller_module?(module()) :: boolean() + def phoenix_controller_module?(module) do + function_exists?(module, :call, 2) and function_exists?(module, :action, 2) + end + + @spec phoenix_liveview_module?(module()) :: boolean() + def phoenix_liveview_module?(module) do + function_exists?(module, :mount, 3) and function_exists?(module, :render, 1) + end + + @spec phoenix_component_module?(module()) :: boolean() + def phoenix_component_module?(module) do + function_exists?(module, :__components__, 0) and + function_exists?(module, :__phoenix_component_verify__, 1) + end + defp check_commented(%Analysis{} = analysis, %Position{} = position) do if Analysis.commented?(analysis, position) do :error @@ -247,14 +263,6 @@ defmodule Lexical.RemoteControl.CodeIntelligence.Entity do end end - defp phoenix_controller_module?(module) do - function_exists?(module, :call, 2) and function_exists?(module, :action, 2) - end - - defp phoenix_liveview_module?(module) do - function_exists?(module, :mount, 3) and function_exists?(module, :render, 1) - end - # Take only the segments at and before the cursor, e.g. # Foo|.Bar.Baz -> Foo # Foo.|Bar.Baz -> Foo.Bar diff --git a/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex b/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex index dcaabd7dc..3aeb07032 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_mod/format.ex @@ -5,6 +5,7 @@ defmodule Lexical.RemoteControl.CodeMod.Format do alias Lexical.RemoteControl alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.CodeMod.Diff + alias Lexical.RemoteControl.Commands.Rename require Logger @@ -12,13 +13,22 @@ defmodule Lexical.RemoteControl.CodeMod.Format do @spec edits(Project.t(), Document.t()) :: {:ok, Changes.t()} | {:error, any} def edits(%Project{} = project, %Document{} = document) do - with :ok <- Build.compile_document(project, document), + with :ok <- ensure_not_renaming(), + :ok <- Build.compile_document(project, document), {:ok, formatted} <- do_format(project, document) do edits = Diff.diff(document, formatted) {:ok, Changes.new(document, edits)} end end + defp ensure_not_renaming do + if Rename.in_progress?() do + {:error, :rename_in_progress} + else + :ok + end + end + defp do_format(%Project{} = project, %Document{} = document) do project_path = Project.project_path(project) diff --git a/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex b/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex index 506f199c7..d3d03df42 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex @@ -1,8 +1,12 @@ defmodule Lexical.RemoteControl.CodeMod.Rename do alias Lexical.Ast.Analysis - alias Lexical.Document.Edit + alias Lexical.Document alias Lexical.Document.Position alias Lexical.Document.Range + alias Lexical.Protocol.Notifications.DidChange + alias Lexical.Protocol.Notifications.DidSave + alias Lexical.RemoteControl.Commands + alias __MODULE__ @spec prepare(Analysis.t(), Position.t()) :: @@ -11,12 +15,48 @@ defmodule Lexical.RemoteControl.CodeMod.Rename do @rename_mapping %{module: Rename.Module} - @spec rename(Analysis.t(), Position.t(), String.t()) :: - {:ok, %{Lexical.uri() => [Edit.t()]}} | {:error, term()} - def rename(%Analysis{} = analysis, %Position{} = position, new_name) do + @spec rename(Analysis.t(), Position.t(), String.t(), String.t() | nil) :: + {:ok, [Document.Changes.t()]} | {:error, term()} + def rename(%Analysis{} = analysis, %Position{} = position, new_name, client_name) do with {:ok, {renamable, entity}, range} <- Rename.Prepare.resolve(analysis, position) do rename_module = @rename_mapping[renamable] - {:ok, rename_module.rename(range, new_name, entity)} + results = rename_module.rename(range, new_name, entity) + set_rename_progress(results, client_name) + {:ok, results} end end + + defp set_rename_progress(document_changes_list, client_name) do + client_name + |> uri_with_expected_operation(document_changes_list) + |> Commands.Rename.set_rename_progress() + end + + defp uri_with_expected_operation(client_name, document_changes_list) + when client_name in ["Visual Studio Code", "emacs"] do + document_changes_list + |> Enum.flat_map(fn %Document.Changes{document: document, rename_file: rename_file} -> + if rename_file do + # when the file is renamed, we won't receive `DidSave` for the old file + [{rename_file.old_uri, DidChange}, {rename_file.new_uri, DidSave}] + else + [{document.uri, DidSave}] + end + end) + |> Map.new() + end + + defp uri_with_expected_operation(_, document_changes_list) do + document_changes_list + |> Enum.flat_map(fn %Document.Changes{document: document, rename_file: rename_file} -> + if rename_file do + [{document.uri, DidSave}] + else + # Some editors do not directly save the file after renaming, such as *neovim*. + # when the file is not renamed, we'll only received `DidChange` for the old file + [{document.uri, DidChange}] + end + end) + |> Map.new() + end end diff --git a/apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex b/apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex new file mode 100644 index 000000000..f0c5023d1 --- /dev/null +++ b/apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex @@ -0,0 +1,154 @@ +defmodule Lexical.RemoteControl.CodeMod.Rename.File do + alias Lexical.Ast + alias Lexical.Document + alias Lexical.ProcessCache + alias Lexical.Project + alias Lexical.RemoteControl + alias Lexical.RemoteControl.CodeIntelligence.Entity + alias Lexical.RemoteControl.Search.Indexer + alias Lexical.RemoteControl.Search.Indexer.Entry + + @spec maybe_rename(Document.t(), Entry.t(), String.t()) :: Document.Changes.rename_file() + def maybe_rename(%Document{} = document, %Entry{} = entry, new_suffix) do + if root_module?(entry, document) do + rename_file(document, entry, new_suffix) + end + end + + defp root_module?(entry, document) do + entries = + ProcessCache.trans("#{document.uri}-entries", 50, fn -> + with {:ok, entries} <- + Indexer.Source.index_document(document, [Indexer.Extractors.Module]) do + entries + end + end) + + case Enum.filter(entries, &(&1.block_id == :root)) do + [root_module] -> + root_module.subject == entry.subject and root_module.block_range == entry.block_range + + _ -> + false + end + end + + defp rename_file(document, entry, new_suffix) do + root_path = root_path() + relative_path = relative_path(entry.path, root_path) + + with {:ok, prefix} <- fetch_conventional_prefix(relative_path), + {:ok, new_name} <- fetch_new_name(document, entry, new_suffix) do + extname = Path.extname(entry.path) + + suffix = + new_name + |> Macro.underscore() + |> maybe_insert_special_phoenix_folder(entry.subject, relative_path) + + new_path = Path.join([root_path, prefix, "#{suffix}#{extname}"]) + new_uri = Document.Path.ensure_uri(new_path) + + if document.uri != new_uri do + Document.Changes.RenameFile.new(document.uri, new_uri) + end + else + _ -> nil + end + end + + defp relative_path(path, root_path) do + Path.relative_to(path, root_path) + end + + defp root_path do + Project.root_path(RemoteControl.get_project()) + end + + defp fetch_new_name(document, entry, new_suffix) do + text_edits = [Document.Edit.new(new_suffix, entry.range)] + + with {:ok, edited_document} <- + Document.apply_content_changes(document, document.version + 1, text_edits), + {:ok, %{context: {:alias, alias}}} <- + Ast.surround_context(edited_document, entry.range.start) do + {:ok, to_string(alias)} + else + _ -> :error + end + end + + defp fetch_conventional_prefix(path) do + # To obtain the new relative path, we can't directly convert from the *new module* name. + # We also need a part of the prefix, and Elixir has some conventions in this regard, + # For example: + # + # in umbrella projects, the prefix is `Path.join(["apps", app_name, "lib"])` + # in non-umbrella projects, most file's prefix is `"lib"` + # + # ## Examples + # + # iex> fetch_conventional_prefix("apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex") + # {:ok, "apps/remote_control/lib"} + result = + path + |> Path.split() + |> Enum.chunk_every(2, 2) + |> Enum.reduce([], fn + ["apps", app_name], _ -> + [app_name, "apps"] + + ["lib", _follow_element], prefix -> + ["lib" | prefix] + + ["test", _follow_element], prefix -> + ["test" | prefix] + + _remain, prefix -> + prefix + end) + + case result do + [] -> + :error + + prefix -> + prefix = prefix |> Enum.reverse() |> Path.join() + {:ok, prefix} + end + end + + defp maybe_insert_special_phoenix_folder(suffix, subject, relative_path) do + insertions = + cond do + Entity.phoenix_controller_module?(subject) -> + "controllers" + + Entity.phoenix_liveview_module?(subject) -> + "live" + + Entity.phoenix_component_module?(subject) -> + "components" + + true -> + nil + end + + # In some cases, users prefer to include the `insertions` in the module name, + # such as `DemoWeb.Components.Icons`. + # In this case, we should not insert the prefix in a nested manner. + prefer_to_include_insertions? = insertions && insertions in Path.split(suffix) + old_path_contains_insertions? = insertions in Path.split(relative_path) + + if not is_nil(insertions) and old_path_contains_insertions? and + not prefer_to_include_insertions? do + suffix + |> Path.split() + |> List.insert_at(1, insertions) + |> Enum.reject(&(&1 == "")) + |> Path.join() + else + suffix + end + end +end diff --git a/apps/remote_control/lib/lexical/remote_control/code_mod/rename/module.ex b/apps/remote_control/lib/lexical/remote_control/code_mod/rename/module.ex index 616e7d8a9..79f4866be 100644 --- a/apps/remote_control/lib/lexical/remote_control/code_mod/rename/module.ex +++ b/apps/remote_control/lib/lexical/remote_control/code_mod/rename/module.ex @@ -6,21 +6,23 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do alias Lexical.Document.Position alias Lexical.Document.Range alias Lexical.RemoteControl.CodeIntelligence.Entity + alias Lexical.RemoteControl.CodeMod.Rename alias Lexical.RemoteControl.Search.Store require Logger import Line - @spec rename(Range.t(), String.t(), atom()) :: %{Lexical.uri() => [Edit.t()]} + @spec rename(Range.t(), String.t(), atom()) :: [Document.Changes.t()] def rename(%Range{} = old_range, new_name, entity) do {old_suffix, new_suffix} = old_range |> range_text() |> diff(new_name) results = exacts(entity, old_suffix) ++ descendants(entity, old_suffix) - Enum.group_by( - results, - &Document.Path.ensure_uri(&1.path), - &Edit.new(new_suffix, &1.range) - ) + for {uri, entries} <- Enum.group_by(results, &Document.Path.ensure_uri(&1.path)), + result = to_document_changes(uri, entries, new_suffix), + match?({:ok, _}, result) do + {:ok, document_changes} = result + document_changes + end end @spec resolve(Analysis.t() | Lexical.path(), Position.t()) :: @@ -77,6 +79,13 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do |> adjust_range_for_descendants(entity, old_suffix) end + defp maybe_rename_file(document, entries, new_suffix) do + entries + |> Enum.map(&Rename.File.maybe_rename(document, &1, new_suffix)) + # every group should have only one `rename_file` + |> Enum.find(&(not is_nil(&1))) + end + defp entry_matching?(entry, old_suffix) do entry.range |> range_text() |> String.contains?(old_suffix) end @@ -161,4 +170,13 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do |> put_in([:start, :character], start_character) |> put_in([:end, :character], end_character) end + + defp to_document_changes(uri, entries, new_suffix) do + edits = Enum.map(entries, &Edit.new(new_suffix, &1.range)) + + with {:ok, document} <- Document.Store.open_temporary(uri) do + rename_file = maybe_rename_file(document, entries, new_suffix) + {:ok, Document.Changes.new(document, edits, rename_file)} + end + end end diff --git a/apps/remote_control/lib/lexical/remote_control/commands/reindex.ex b/apps/remote_control/lib/lexical/remote_control/commands/reindex.ex index d9763ffde..0f2f29d43 100644 --- a/apps/remote_control/lib/lexical/remote_control/commands/reindex.ex +++ b/apps/remote_control/lib/lexical/remote_control/commands/reindex.ex @@ -1,5 +1,6 @@ defmodule Lexical.RemoteControl.Commands.Reindex do defmodule State do + alias Lexical.Ast alias Lexical.Ast.Analysis alias Lexical.Document alias Lexical.ProcessCache @@ -54,8 +55,7 @@ defmodule Lexical.RemoteControl.Commands.Reindex do end defp entries_for_uri(uri) do - with {:ok, %Document{} = document, %Analysis{} = analysis} <- - Document.Store.fetch(uri, :analysis), + with {:ok, %Document{} = document, %Analysis{} = analysis} <- ensure_document_open(uri), {:ok, entries} <- Indexer.Quoted.index_with_cleanup(analysis) do {:ok, document.path, entries} else @@ -64,6 +64,21 @@ defmodule Lexical.RemoteControl.Commands.Reindex do error end end + + defp ensure_document_open(uri) do + case Document.Store.fetch(uri, :analysis) do + {:ok, %Document{} = document, analysis} -> + {:ok, document, analysis} + + {:error, :not_open} -> + # Sometimes, some operations are received long after the reindex is triggered, + # such as new files after a batch rename + with {:ok, document} <- Document.Store.open_temporary(uri) do + analysis = Ast.analyze(document) + {:ok, document, analysis} + end + end + end end @moduledoc """ diff --git a/apps/remote_control/lib/lexical/remote_control/commands/rename.ex b/apps/remote_control/lib/lexical/remote_control/commands/rename.ex new file mode 100644 index 000000000..7fc8f704b --- /dev/null +++ b/apps/remote_control/lib/lexical/remote_control/commands/rename.ex @@ -0,0 +1,74 @@ +defmodule Lexical.RemoteControl.Commands.Rename do + @moduledoc """ + We are unable to accurately determine the process of renaming, + because after the rename, there will be a series of operations such as + `DidChange`, `DidSave`, etc., which will trigger expensive actions like compiling the entire project. + Therefore, we need this module to make some markings to determine whether it is currently in the process of renaming. + """ + defmodule State do + defstruct uri_with_expected_operation: %{} + + def new(uri_with_expected_operation) do + %__MODULE__{uri_with_expected_operation: uri_with_expected_operation} + end + + def update_progress(%__MODULE__{} = state, uri, operation_message) do + new_uri_with_expected_operation = + maybe_pop_expected_operation(state.uri_with_expected_operation, uri, operation_message) + + %__MODULE__{state | uri_with_expected_operation: new_uri_with_expected_operation} + end + + def in_progress?(%__MODULE__{} = state) do + state.uri_with_expected_operation != %{} + end + + def maybe_pop_expected_operation(uri_to_operation, uri, %operation{}) do + case uri_to_operation do + %{^uri => ^operation} -> Map.delete(uri_to_operation, uri) + _ -> uri_to_operation + end + end + end + + use GenServer + + def start_link(_) do + GenServer.start_link(__MODULE__, %State{}, name: __MODULE__) + end + + @impl true + def init(state) do + {:ok, state} + end + + @spec set_rename_progress(%{Lexical.uri() => atom()}) :: :ok + def set_rename_progress(uri_with_expected_operation) do + GenServer.cast(__MODULE__, {:set_rename_progress, uri_with_expected_operation}) + end + + def update_progress(uri, operation_message) do + GenServer.cast(__MODULE__, {:update_progress, uri, operation_message}) + end + + def in_progress? do + GenServer.call(__MODULE__, :in_progress?) + end + + @impl true + def handle_call(:in_progress?, _from, state) do + {:reply, State.in_progress?(state), state} + end + + @impl true + def handle_cast({:set_rename_progress, uri_with_expected_operation}, _state) do + new_state = State.new(uri_with_expected_operation) + {:noreply, new_state} + end + + @impl true + def handle_cast({:update_progress, uri, message}, state) do + new_state = State.update_progress(state, uri, message) + {:noreply, new_state} + end +end diff --git a/apps/remote_control/lib/lexical/remote_control/dispatch/handlers/indexing.ex b/apps/remote_control/lib/lexical/remote_control/dispatch/handlers/indexing.ex index f20ec4bd3..b7c569168 100644 --- a/apps/remote_control/lib/lexical/remote_control/dispatch/handlers/indexing.ex +++ b/apps/remote_control/lib/lexical/remote_control/dispatch/handlers/indexing.ex @@ -8,9 +8,20 @@ defmodule Lexical.RemoteControl.Dispatch.Handlers.Indexing do require Logger import Messages - use Dispatch.Handler, [file_compile_requested(), filesystem_event()] + use Dispatch.Handler, [filesystem_event(), file_changed(), file_opened()] - def on_event(file_compile_requested(uri: uri), state) do + def on_event(file_changed(uri: uri, open?: true), state) do + reindex(uri) + {:ok, state} + end + + def on_event(file_changed(), state) do + {:ok, state} + end + + def on_event(file_opened(uri: uri), state) do + # When renaming occurs, new files are only *opened* and *closed*, + # so we need to index the new files. reindex(uri) {:ok, state} end diff --git a/apps/remote_control/test/lexical/remote_control/code_mod/format_test.exs b/apps/remote_control/test/lexical/remote_control/code_mod/format_test.exs index 7eb0f369e..bfaa170d0 100644 --- a/apps/remote_control/test/lexical/remote_control/code_mod/format_test.exs +++ b/apps/remote_control/test/lexical/remote_control/code_mod/format_test.exs @@ -6,6 +6,7 @@ defmodule Lexical.RemoteControl.CodeMod.FormatTest do alias Lexical.RemoteControl.Api.Messages alias Lexical.RemoteControl.Build alias Lexical.RemoteControl.CodeMod.Format + alias Lexical.RemoteControl.Commands use Lexical.Test.CodeMod.Case, enable_ast_conversion: false use Patch @@ -66,6 +67,7 @@ defmodule Lexical.RemoteControl.CodeMod.FormatTest do end setup do + start_supervised!(Commands.Rename) project = project() {:ok, project: project} end diff --git a/apps/remote_control/test/lexical/remote_control/code_mod/rename_test.exs b/apps/remote_control/test/lexical/remote_control/code_mod/rename_test.exs index 98af234d0..2a5e66923 100644 --- a/apps/remote_control/test/lexical/remote_control/code_mod/rename_test.exs +++ b/apps/remote_control/test/lexical/remote_control/code_mod/rename_test.exs @@ -1,7 +1,7 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do alias Lexical.Document - alias Lexical.Project alias Lexical.RemoteControl + alias Lexical.RemoteControl.CodeIntelligence.Entity alias Lexical.RemoteControl.CodeMod.Rename alias Lexical.RemoteControl.Search alias Lexical.RemoteControl.Search.Store.Backends @@ -15,6 +15,7 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do import Fixtures use ExUnit.Case + use Patch setup_all do project = project() @@ -40,16 +41,6 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do {:ok, project: project} end - setup %{project: project} do - uri = subject_uri(project) - - on_exit(fn -> - Document.Store.close(uri) - end) - - %{uri: uri} - end - describe "prepare/2" do test "returns the module name" do {:ok, result, _} = @@ -71,7 +62,7 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do assert result == "TopLevel.Foo" end - test "returns the whole module name even if the cusor is not at the end" do + test "returns the whole module name even if the cursor is not at the end" do {:ok, result, _} = ~q[ defmodule Top|Level.Foo do @@ -217,7 +208,7 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do end end - describe "rename descendants" do + describe "rename module descendants" do test "rename the descendants" do {:ok, result} = ~q[ defmodule TopLevel.|Module do @@ -341,21 +332,210 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do end end - defp rename(%Project{} = project \\ project(), source, new_name) do - uri = subject_uri(project) + describe "rename file" do + setup do + patch(Entity, :function_exists?, false) + :ok + end + + test "it shouldn't rename file if the module has parent module within that file" do + {:ok, {_applied, nil}} = + ~q[ + defmodule FooServer do + defmodule |State do + end + end + ] |> rename("Renamed", "lib/foo_server.ex") + end + + test "it shouldn't rename file if the module has any siblings within that file" do + assert {:ok, {_applied, nil}} = + ~q[ + defmodule |Foo do + end + + defmodule Bar do + end + ] |> rename("Renamed", "lib/foo.ex") + end + + test "it shouldn't rename file if the path doesn't match the any convensions" do + assert {:ok, {_applied, nil}} = + ~q[ + defmodule |Foo.Mix do + end + ] |> rename("Renamed", "mix.ex") + end + + test "succeeds when the path matching the `lib/*` convension", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |Foo do + end + ] |> rename("Renamed", "lib/foo.ex") + + assert rename_file.new_uri == subject_uri(project, "lib/renamed.ex") + end + + test "it shouldn't rename file if just uppercased the module name" do + assert {:ok, {_applied, nil}} = + ~q[ + defmodule |Foo do + end + ] |> rename("FOO", "lib/foo.ex") + end + + test "succeeds when the path matching the `apps/*` convension", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |FooApp.Bar do + end + ] |> rename("FooApp.Renamed", "apps/foo_app/lib/foo_app/bar.ex") + + assert rename_file.new_uri == subject_uri(project, "apps/foo_app/lib/foo_app/renamed.ex") + end + + test "succeeds when the path matching the `apps/*` convension with nested folders", %{ + project: project + } do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |Lexical.RemoteControl do + end + ] |> rename("Lexical.RemoteChaos", "apps/remote_control/lib/lexical/remote_control.ex") + + assert rename_file.new_uri == + subject_uri(project, "apps/remote_control/lib/lexical/remote_chaos.ex") + end + + test "succeeds when the path matching the `test/*` convension", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |FooTest do + end + ] |> rename("RenamedTest", "test/foo_test.exs") + + assert rename_file.new_uri == subject_uri(project, "test/renamed_test.exs") + end + + test "leaves the `components` folder as is when renaming the live view", %{project: project} do + patch(Entity, :phoenix_component_module?, fn DemoWeb.FooComponent -> true end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule DemoWeb.|FooComponent do + end + ] |> rename("DemoWeb.RenamedComponent", "lib/demo_web/components/foo_component.ex") + + assert rename_file.new_uri == + subject_uri(project, "lib/demo_web/components/renamed_component.ex") + end + + test "leaves the `components` folder as is when renaming a component", %{project: project} do + patch(Entity, :phoenix_component_module?, fn DemoWeb.SomeContext.FooComponent -> true end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule DemoWeb.SomeContext.|FooComponent do + end + ] + |> rename( + "DemoWeb.SomeContext.RenamedComponent", + "lib/demo_web/components/some_context/foo_component.ex" + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/demo_web/components/some_context/renamed_component.ex") + end + + test "leaves the `components` folder as is when the user prefers to include the `Components` in the module name", + %{ + project: project + } do + patch(Entity, :phoenix_component_module?, fn DemoWeb.Components.Icons -> true end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule DemoWeb.Components.|Icons do + end + ] |> rename("DemoWeb.Components.RenamedIcons", "lib/demo_web/components/icons.ex") + + assert rename_file.new_uri == + subject_uri(project, "lib/demo_web/components/renamed_icons.ex") + end + + test "leaves the `controllers` folder as is when renaming the controller", %{project: project} do + patch(Entity, :phoenix_controller_module?, fn DemoWeb.FooController -> true end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule DemoWeb.|FooController do + end + ] |> rename("DemoWeb.RenamedController", "lib/demo_web/controllers/foo_controller.ex") + + assert rename_file.new_uri == + subject_uri(project, "lib/demo_web/controllers/renamed_controller.ex") + end + + test "leaves the `controller` folder as is when renaming the `JSON` module", %{ + project: project + } do + patch(Entity, :phoenix_controller_module?, fn DemoWeb.FooController.JSON -> true end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule DemoWeb.FooController.|JSON do + end + ] + |> rename( + "DemoWeb.FooController.RenamedJSON", + "lib/demo_web/controllers/foo_controller/json.ex" + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/demo_web/controllers/foo_controller/renamed_json.ex") + end + + test "leaves the `live` folder as is when renaming the live view", %{project: project} do + patch(Entity, :phoenix_liveview_module?, fn DemoWeb.FooLive -> true end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule DemoWeb.|FooLive do + end + ] |> rename("DemoWeb.RenamedLive", "lib/demo_web/live/foo_live.ex") + + assert rename_file.new_uri == subject_uri(project, "lib/demo_web/live/renamed_live.ex") + end + end + + defp rename(source, new_name, path \\ nil) do + project = project() + uri = subject_uri(project, path) with {position, text} <- pop_cursor(source), {:ok, document} <- open_document(uri, text), {:ok, entries} <- Search.Indexer.Source.index(document.path, text), :ok <- Search.Store.replace(entries), analysis = Lexical.Ast.analyze(document), - {:ok, uri_with_changes} <- Rename.rename(analysis, position, new_name) do - changes = uri_with_changes |> Map.values() |> List.flatten() - {:ok, apply_edits(document, changes)} + {:ok, document_changes} <- Rename.rename(analysis, position, new_name, nil) do + changes = document_changes |> Enum.map(& &1.edits) |> List.flatten() + applied = apply_edits(document, changes) + + result = + if path do + rename_file = document_changes |> Enum.map(& &1.rename_file) |> List.first() + {applied, rename_file} + else + applied + end + + {:ok, result} end end - defp prepare(project \\ project(), code) do + defp prepare(code) do + project = project() uri = subject_uri(project) with {position, text} <- pop_cursor(code), @@ -368,10 +548,19 @@ defmodule Lexical.RemoteControl.CodeMod.RenameTest do end end - defp subject_uri(project) do - project - |> file_path(Path.join("lib", "project.ex")) - |> Document.Path.ensure_uri() + defp subject_uri(project, path \\ nil) do + path = path || Path.join("wont_rename_file_folder", "project.ex") + + uri = + project + |> file_path(path) + |> Document.Path.ensure_uri() + + on_exit(fn -> + Document.Store.close(uri) + end) + + uri end defp open_document(uri, content) do diff --git a/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs b/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs new file mode 100644 index 000000000..5a7e285a7 --- /dev/null +++ b/apps/remote_control/test/lexical/remote_control/commands/rename_test.exs @@ -0,0 +1,42 @@ +defmodule Lexical.RemoteControl.Commands.RenameTest do + alias Lexical.RemoteControl.Commands.Rename + + defmodule FakeDidChange do + defstruct [:field] + end + + defmodule FakeDidSave do + defstruct [:field] + end + + use ExUnit.Case + use Patch + + setup do + start_supervised!(Rename) + :ok + end + + test "it should mark the `in_progress` as `true` when a rename is in progress." do + assert :ok = Rename.set_rename_progress(%{"file://file.ex" => FakeDidChange}) + assert Rename.in_progress?() + end + + test "it should mark the `in_progress` as false when a rename is done" do + file_uri = "file://file.ex" + Rename.set_rename_progress(%{file_uri => FakeDidSave}) + Rename.update_progress(file_uri, %FakeDidSave{}) + + refute Rename.in_progress?() + end + + test "it should still in progress if there are files yet to be saved." do + uri1 = "file://file1.ex" + uri2 = "file://file2.ex" + + Rename.set_rename_progress(%{uri1 => FakeDidChange, uri2 => FakeDidSave}) + Rename.update_progress(uri1, %FakeDidChange{}) + + assert Rename.in_progress?() + end +end diff --git a/apps/remote_control/test/lexical/remote_control/dispatch/handlers/indexer_test.exs b/apps/remote_control/test/lexical/remote_control/dispatch/handlers/indexer_test.exs index c9670e431..3898960c0 100644 --- a/apps/remote_control/test/lexical/remote_control/dispatch/handlers/indexer_test.exs +++ b/apps/remote_control/test/lexical/remote_control/dispatch/handlers/indexer_test.exs @@ -60,7 +60,7 @@ defmodule Lexical.RemoteControl.Dispatch.Handlers.IndexingTest do ] |> set_document!() - assert {:ok, _} = Indexing.on_event(file_compile_requested(uri: uri), state) + assert {:ok, _} = Indexing.on_event(file_changed(uri: uri, open?: true), state) assert_eventually [entry] = Search.Store.exact("NewModule", []) @@ -84,7 +84,7 @@ defmodule Lexical.RemoteControl.Dispatch.Handlers.IndexingTest do ] |> set_document!() - assert {:ok, _} = Indexing.on_event(file_compile_requested(uri: uri), state) + assert {:ok, _} = Indexing.on_event(file_changed(uri: uri, open?: true), state) assert_eventually [entry] = Search.Store.exact("UpdatedModule", []) assert entry.subject == UpdatedModule @@ -102,7 +102,7 @@ defmodule Lexical.RemoteControl.Dispatch.Handlers.IndexingTest do ] |> set_document!() - assert {:ok, _} = Indexing.on_event(file_compile_requested(uri: uri), state) + assert {:ok, _} = Indexing.on_event(file_changed(uri: uri, open?: true), state) assert [] = Search.Store.exact("Stale", []) end end diff --git a/apps/server/lib/lexical/server/provider/env.ex b/apps/server/lib/lexical/server/provider/env.ex index f728c6ba7..6960bb363 100644 --- a/apps/server/lib/lexical/server/provider/env.ex +++ b/apps/server/lib/lexical/server/provider/env.ex @@ -8,10 +8,11 @@ defmodule Lexical.Server.Provider.Env do alias Lexical.Project alias Lexical.Server.Configuration - defstruct [:project] + defstruct [:project, :client_name] @type t :: %__MODULE__{ - project: Project.t() + project: Project.t(), + client_name: String.t() | nil } def new do @@ -19,7 +20,7 @@ defmodule Lexical.Server.Provider.Env do end def from_configuration(%Configuration{} = config) do - %__MODULE__{project: config.project} + %__MODULE__{project: config.project, client_name: config.client_name} end def project_name(%__MODULE__{} = env) do diff --git a/apps/server/lib/lexical/server/provider/handlers/rename.ex b/apps/server/lib/lexical/server/provider/handlers/rename.ex index 31abbaced..a81468624 100644 --- a/apps/server/lib/lexical/server/provider/handlers/rename.ex +++ b/apps/server/lib/lexical/server/provider/handlers/rename.ex @@ -3,7 +3,9 @@ defmodule Lexical.Server.Provider.Handlers.Rename do alias Lexical.Document alias Lexical.Protocol.Requests.Rename alias Lexical.Protocol.Responses - alias Lexical.Protocol.Types.Workspace.Edit + alias Lexical.Protocol.Types.RenameFile + alias Lexical.Protocol.Types.TextDocument + alias Lexical.Protocol.Types.Workspace alias Lexical.RemoteControl.Api alias Lexical.Server.Provider.Env require Logger @@ -11,7 +13,14 @@ defmodule Lexical.Server.Provider.Handlers.Rename do def handle(%Rename{} = request, %Env{} = env) do case Document.Store.fetch(request.document.uri, :analysis) do {:ok, _document, %Ast.Analysis{valid?: true} = analysis} -> - rename(env.project, analysis, request.position, request.new_name, request.id) + rename( + env.project, + analysis, + request.position, + request.new_name, + request.id, + env.client_name + ) _ -> {:reply, @@ -19,14 +28,26 @@ defmodule Lexical.Server.Provider.Handlers.Rename do end end - defp rename(project, analysis, position, new_name, id) do - case Api.rename(project, analysis, position, new_name) do - {:ok, results} when results == %{} -> + defp rename(project, analysis, position, new_name, id, client_name) do + case Api.rename(project, analysis, position, new_name, client_name) do + {:ok, []} -> {:reply, nil} {:ok, results} -> - edit = Edit.new(changes: results) - {:reply, Responses.Rename.new(id, edit)} + text_document_edits = + Enum.map(results, fn %Document.Changes{edits: edits, document: document} -> + new_text_document_edit(document.uri, edits) + end) + + rename_files = + results + |> Stream.map(& &1.rename_file) + |> Stream.reject(&(&1 == nil)) + |> Enum.map(&new_rename_file/1) + + workspace_edit = Workspace.Edit.new(document_changes: text_document_edits ++ rename_files) + + {:reply, Responses.Rename.new(id, workspace_edit)} {:error, {:unsupported_entity, entity}} -> Logger.info("Unrenameable entity: #{inspect(entity)}") @@ -36,4 +57,20 @@ defmodule Lexical.Server.Provider.Handlers.Rename do {:reply, Responses.Rename.error(id, :request_failed, inspect(reason))} end end + + defp new_text_document_edit(uri, edits) do + text_document = TextDocument.OptionalVersioned.Identifier.new(uri: uri, version: 0) + TextDocument.Edit.new(edits: edits, text_document: text_document) + end + + defp new_rename_file(%Document.Changes.RenameFile{} = rename_file) do + options = RenameFile.Options.new(overwrite: true) + + RenameFile.new( + kind: "rename", + new_uri: rename_file.new_uri, + old_uri: rename_file.old_uri, + options: options + ) + end end diff --git a/apps/server/lib/lexical/server/state.ex b/apps/server/lib/lexical/server/state.ex index 95b8a8b39..2aa19150f 100644 --- a/apps/server/lib/lexical/server/state.ex +++ b/apps/server/lib/lexical/server/state.ex @@ -143,7 +143,7 @@ defmodule Lexical.Server.State do {:ok, state} end - def apply(%__MODULE__{} = state, %DidChange{lsp: event}) do + def apply(%__MODULE__{} = state, %DidChange{lsp: event} = message) do uri = event.text_document.uri version = event.text_document.version project = state.configuration.project @@ -162,7 +162,7 @@ defmodule Lexical.Server.State do ) Api.broadcast(project, updated_message) - Api.compile_document(state.configuration.project, updated_source) + Api.maybe_compile_document(state.configuration.project, updated_source, message) {:ok, state} error -> @@ -181,6 +181,8 @@ defmodule Lexical.Server.State do case Document.Store.open(uri, text, version, language_id) do :ok -> Logger.info("opened #{uri}") + project = state.configuration.project + Api.broadcast(project, file_opened(uri: uri, version: version)) {:ok, state} error -> @@ -205,12 +207,12 @@ defmodule Lexical.Server.State do end end - def apply(%__MODULE__{} = state, %DidSave{lsp: event}) do + def apply(%__MODULE__{} = state, %DidSave{lsp: event} = message) do uri = event.text_document.uri case Document.Store.save(uri) do :ok -> - Api.schedule_compile(state.configuration.project, false) + Api.maybe_schedule_compile(state.configuration.project, uri, message) {:ok, state} error -> diff --git a/apps/server/test/lexical/server/provider/handlers/rename_test.exs b/apps/server/test/lexical/server/provider/handlers/rename_test.exs index f4cd62703..6160f6759 100644 --- a/apps/server/test/lexical/server/provider/handlers/rename_test.exs +++ b/apps/server/test/lexical/server/provider/handlers/rename_test.exs @@ -62,8 +62,8 @@ defmodule Lexical.Server.Provider.Handlers.RenameTest do {:ok, nil, %Ast.Analysis{valid?: true}} end) - patch(RemoteControl.Api, :rename, fn ^project, _analysis, _position, _new_name -> - {:ok, %{}} + patch(RemoteControl.Api, :rename, fn ^project, _analysis, _position, _new_name, _ -> + {:ok, []} end) {:ok, request} = build_request(uri, 1, 5) @@ -73,34 +73,47 @@ defmodule Lexical.Server.Provider.Handlers.RenameTest do end test "returns edit when there are changes", %{project: project, uri: uri} do + document = %Document{uri: uri, version: 0} + patch(Document.Store, :fetch, fn ^uri, :analysis -> {:ok, nil, %Ast.Analysis{valid?: true}} end) - patch(RemoteControl.Api, :rename, fn ^project, _analysis, _position, _new_name -> + patch(RemoteControl.Api, :rename, fn ^project, _analysis, _position, _new_name, _ -> {:ok, - %{ - "file:///path/to/file.ex" => [ - %{ - new_text: "new_text", - range: %{start: %{line: 1, character: 5}, end: %{line: 1, character: 10}} - } - ] - }} + [ + Document.Changes.new( + document, + [ + %{ + new_text: "new_text", + range: %{start: %{line: 1, character: 5}, end: %{line: 1, character: 10}} + } + ], + Document.Changes.RenameFile.new( + document.uri, + "file:///path/to/new_text.ex" + ) + ) + ]} end) {:ok, request} = build_request(uri, 1, 5) assert {:reply, response} = handle(request, project) - - assert response.result.changes == %{ - "file:///path/to/file.ex" => [ - %{ - new_text: "new_text", - range: %{end: %{character: 10, line: 1}, start: %{character: 5, line: 1}} - } - ] - } + [edit, rename_file] = response.result.document_changes + + assert edit.edits == [ + %{ + new_text: "new_text", + range: %{end: %{character: 10, line: 1}, start: %{character: 5, line: 1}} + } + ] + + assert edit.text_document.uri == document.uri + assert edit.text_document.version == 0 + assert rename_file.old_uri == document.uri + assert rename_file.new_uri == "file:///path/to/new_text.ex" end end end diff --git a/projects/lexical_shared/lib/lexical/document/changes.ex b/projects/lexical_shared/lib/lexical/document/changes.ex index fc848452f..f6ea4c6fa 100644 --- a/projects/lexical_shared/lib/lexical/document/changes.ex +++ b/projects/lexical_shared/lib/lexical/document/changes.ex @@ -9,15 +9,28 @@ defmodule Lexical.Document.Changes do Using this struct allows efficient conversions at the language server border, as the document doesn't have to be looked up (and possibly read off the filesystem) by the language server. """ - defstruct [:document, :edits] + defmodule RenameFile do + @type t :: %__MODULE__{old_uri: Lexical.uri(), new_uri: Lexical.uri()} + + defstruct [:old_uri, :new_uri] + + @spec new(Lexical.uri(), Lexical.uri()) :: t() + def new(old_uri, new_uri) do + %__MODULE__{old_uri: old_uri, new_uri: new_uri} + end + end + + defstruct [:document, :edits, :rename_file] alias Lexical.Document use Lexical.StructAccess @type edits :: Document.Edit.t() | [Document.Edit.t()] + @type rename_file :: nil | RenameFile.t() @type t :: %__MODULE__{ document: Document.t(), - edits: edits + edits: edits, + rename_file: rename_file() } @doc """ @@ -28,4 +41,9 @@ defmodule Lexical.Document.Changes do def new(document, edits) do %__MODULE__{document: document, edits: edits} end + + @spec new(Document.t(), edits(), rename_file()) :: t() + def new(document, edits, rename_file) do + %__MODULE__{document: document, edits: edits, rename_file: rename_file} + end end