diff --git a/apps/engine/lib/engine.ex b/apps/engine/lib/engine.ex index 643e636d..416365a2 100644 --- a/apps/engine/lib/engine.ex +++ b/apps/engine/lib/engine.ex @@ -63,6 +63,14 @@ defmodule Engine do defdelegate workspace_symbols(query), to: CodeIntelligence.Symbols, as: :for_workspace + defdelegate prepare_rename(analysis, position), to: Engine.CodeMod.Rename, as: :prepare + + defdelegate rename(analysis, position, new_name, client_name), to: Engine.CodeMod.Rename + + defdelegate maybe_update_rename_progress(triggered_message), + to: Engine.Commands.Rename, + as: :update_progress + def list_apps do for {app, _, _} <- :application.loaded_applications(), not Forge.Namespace.Module.prefixed?(app), diff --git a/apps/engine/lib/engine/application.ex b/apps/engine/lib/engine/application.ex index 014aaf60..431fd847 100644 --- a/apps/engine/lib/engine/application.ex +++ b/apps/engine/lib/engine/application.ex @@ -10,6 +10,7 @@ defmodule Engine.Application do [ Engine.Api.Proxy, Engine.Commands.Reindex, + Engine.Commands.RenameSupervisor, Engine.Module.Loader, Engine.Dispatch, Engine.ModuleMappings, diff --git a/apps/engine/lib/engine/code_mod/rename.ex b/apps/engine/lib/engine/code_mod/rename.ex new file mode 100644 index 00000000..def5d6bd --- /dev/null +++ b/apps/engine/lib/engine/code_mod/rename.ex @@ -0,0 +1,128 @@ +defmodule Engine.CodeMod.Rename do + @moduledoc """ + Entry point for rename operations. + + This module provides the main API for renaming entities (currently modules) + in Elixir code. It coordinates between the preparation phase and the actual + rename execution. + """ + alias Engine.CodeMod.Rename + alias Engine.Commands + alias Engine.Progress + alias Forge.Ast.Analysis + alias Forge.Document + alias Forge.Document.Position + alias Forge.Document.Range + + import Forge.EngineApi.Messages + + @doc """ + Prepares a rename operation at the given position. + + Returns `{:ok, entity_name, range}` if the entity can be renamed, + `{:ok, nil}` if at an unsupported location, + or `{:error, reason}` if renaming is not possible. + """ + @spec prepare(Analysis.t(), Position.t()) :: + {:ok, String.t(), Range.t()} | {:ok, nil} | {:error, term()} + defdelegate prepare(analysis, position), to: Rename.Prepare + + @rename_mappings %{module: Rename.Module} + + @doc """ + Executes a rename operation. + + Renames the entity at the given position to `new_name`, returning a list + of document changes that should be applied. + + The `client_name` parameter is used to determine client-specific behavior + for progress tracking (e.g., VSCode sends different events than Neovim). + """ + @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 = Map.fetch!(@rename_mappings, renamable) + 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 + # Progress tracking is optional - if the infrastructure isn't running + # (e.g., in tests), we just skip it silently + try do + do_set_rename_progress(document_changes_list, client_name) + rescue + _ -> :ok + catch + :exit, _ -> :ok + end + end + + defp do_set_rename_progress(document_changes_list, client_name) do + uri_to_expected_operation = + uri_to_expected_operation(client_name, document_changes_list) + + {paths_to_delete, paths_to_reindex} = + for %Document.Changes{rename_file: rename_file, document: document} <- document_changes_list do + if rename_file do + {rename_file.old_uri, rename_file.new_uri} + else + {nil, document.uri} + end + end + |> Enum.unzip() + + paths_to_delete = Enum.reject(paths_to_delete, &is_nil/1) + renaming_operation_count = Enum.count(uri_to_expected_operation) + + total_operation_count = + renaming_operation_count + length(paths_to_delete) + length(paths_to_reindex) + + {on_update_progress, on_complete} = + Progress.begin_percent("Renaming", total_operation_count) + + Commands.RenameSupervisor.start_renaming( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) + end + + # VSCode sends both file_changed and file_saved events + defp uri_to_expected_operation(client_name, document_changes_list) + when client_name in ["Visual Studio Code"] 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, file_changed(uri: rename_file.old_uri)}, + {rename_file.new_uri, file_saved(uri: rename_file.new_uri)} + ] + else + [{document.uri, file_saved(uri: document.uri)}] + end + end) + |> Map.new() + end + + # Other editors (like Neovim) may only send file_changed events + defp uri_to_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 + [{rename_file.new_uri, file_saved(uri: rename_file.new_uri)}] + 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, file_changed(uri: document.uri)}] + end + end) + |> Map.new() + end +end diff --git a/apps/engine/lib/engine/code_mod/rename/entry.ex b/apps/engine/lib/engine/code_mod/rename/entry.ex new file mode 100644 index 00000000..14e64a09 --- /dev/null +++ b/apps/engine/lib/engine/code_mod/rename/entry.ex @@ -0,0 +1,46 @@ +defmodule Engine.CodeMod.Rename.Entry do + @moduledoc """ + An entry wrapper for search indexer entries used in rename operations. + + When renaming, we rely on the `Forge.Search.Indexer.Entry`, + and we also need some other fields used exclusively for renaming, such as `edit_range`. + """ + alias Forge.Document.Range + alias Forge.Search.Indexer.Entry, as: IndexerEntry + + @type t :: %__MODULE__{ + id: IndexerEntry.entry_id(), + path: Forge.path(), + subject: IndexerEntry.subject(), + block_range: Range.t() | nil, + range: Range.t(), + edit_range: Range.t(), + subtype: IndexerEntry.entry_subtype() + } + + defstruct [ + :id, + :path, + :subject, + :block_range, + :range, + :edit_range, + :subtype + ] + + @doc """ + Creates a new Entry from an IndexerEntry. + """ + @spec new(IndexerEntry.t()) :: t() + def new(%IndexerEntry{} = indexer_entry) do + %__MODULE__{ + id: indexer_entry.id, + path: indexer_entry.path, + subject: indexer_entry.subject, + subtype: indexer_entry.subtype, + block_range: indexer_entry.block_range, + range: indexer_entry.range, + edit_range: indexer_entry.range + } + end +end diff --git a/apps/engine/lib/engine/code_mod/rename/file.ex b/apps/engine/lib/engine/code_mod/rename/file.ex new file mode 100644 index 00000000..4637f13d --- /dev/null +++ b/apps/engine/lib/engine/code_mod/rename/file.ex @@ -0,0 +1,165 @@ +defmodule Engine.CodeMod.Rename.File do + @moduledoc """ + Handles file renaming logic during module renaming operations. + + Determines if a file should be renamed when its containing module is renamed, + based on conventions and constraints. + """ + alias Engine.CodeMod.Rename.Entry + alias Engine.Search.Indexer + alias Forge.Ast + alias Forge.Document + alias Forge.ProcessCache + alias Forge.Project + alias Forge.Search.Indexer.Entry, as: IndexerEntry + + @doc """ + Determines if a file should be renamed when renaming a module. + + Returns a `Forge.Document.Changes.RenameFile` struct if the file should be renamed, + or `nil` if no file rename is needed. + """ + @spec maybe_rename(Document.t(), Entry.t(), String.t()) :: Document.Changes.RenameFile.t() | nil + 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{} = 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 + [%IndexerEntry{} = root_module] -> + root_module.subject == entry.subject and root_module.block_range == entry.block_range + + _ -> + false + end + end + + defp rename_file(document, %Entry{} = entry, new_suffix) do + root_path = root_path() + relative_path = Path.relative_to(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 root_path do + Project.root_path(Engine.get_project()) + end + + defp fetch_new_name(document, %Entry{} = entry, new_suffix) do + text_edits = [Document.Edit.new(new_suffix, entry.edit_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.edit_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"} + segments = + case Path.split(path) do + ["apps", app_name, "lib" | _] -> ["apps", app_name, "lib"] + ["apps", app_name, "test" | _] -> ["apps", app_name, "test"] + ["lib" | _] -> ["lib"] + ["test" | _] -> ["test"] + _ -> nil + end + + if segments do + {:ok, Path.join(segments)} + else + :error + end + end + + defp maybe_insert_special_phoenix_folder(suffix, subject, relative_path) do + insertions = + cond do + phoenix_controller_module?(subject) -> + "controllers" + + phoenix_liveview_module?(subject) -> + "live" + + 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 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) + |> Path.join() + else + suffix + 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 + + defp phoenix_component_module?(module) do + function_exists?(module, :__components__, 0) or + function_exists?(module, :__live__, 0) + end + + defp function_exists?(module, function, arity) do + function_exported?(module, function, arity) + end +end diff --git a/apps/engine/lib/engine/code_mod/rename/module.ex b/apps/engine/lib/engine/code_mod/rename/module.ex new file mode 100644 index 00000000..cd5a58b5 --- /dev/null +++ b/apps/engine/lib/engine/code_mod/rename/module.ex @@ -0,0 +1,261 @@ +defmodule Engine.CodeMod.Rename.Module do + @moduledoc """ + Handles module renaming logic. + + This module is responsible for: + - Recognizing if a position is at a module definition + - Preparing the rename range + - Executing the rename across all references + """ + alias Engine.CodeIntelligence.Entity + alias Engine.CodeMod.Rename + alias Engine.CodeMod.Rename.Entry + alias Engine.CodeMod.Rename.Module + alias Engine.Search.Store + alias Forge.Ast + alias Forge.Ast.Analysis + alias Forge.Document + alias Forge.Document.Edit + alias Forge.Document.Line + alias Forge.Document.Position + alias Forge.Document.Range + alias Forge.Formats + + require Logger + import Line + + @doc """ + Checks if the position is at a module that can be renamed. + """ + @spec recognizes?(Analysis.t(), Position.t()) :: boolean() + def recognizes?(%Analysis{} = analysis, %Position{} = position) do + case resolve(analysis, position) do + {:ok, _, _} -> + true + + _ -> + false + end + end + + @doc """ + Prepares the rename operation by returning the module and the range to be renamed. + """ + @spec prepare(Analysis.t(), Position.t()) :: + {:ok, {:module, atom()}, Range.t()} | {:error, tuple() | atom()} + def prepare(%Analysis{} = analysis, %Position{} = position) do + with {:ok, {:module, _module}, _range} <- resolve(analysis, position) do + {module, range} = surround_the_whole_module(analysis, position) + + if cursor_at_declaration?(module, range) do + {:ok, {:module, module}, range} + else + {:error, {:unsupported_location, :module}} + end + end + end + + @doc """ + Executes the rename operation, returning a list of document changes. + """ + @spec rename(Range.t(), String.t(), atom()) :: [Document.Changes.t()] + def rename(%Range{} = old_range, new_name, module) do + {to_be_renamed, replacement} = old_range |> range_text() |> Module.Diff.diff(new_name) + results = exacts(module, to_be_renamed) ++ descendants(module, to_be_renamed) + + for {uri, entries} <- Enum.group_by(results, &Document.Path.ensure_uri(&1.path)), + result = to_document_changes(uri, entries, replacement), + match?({:ok, _}, result) do + {:ok, document_changes} = result + document_changes + end + end + + defp resolve(%Analysis{} = analysis, %Position{} = position) do + case Entity.resolve(analysis, position) do + {:ok, {module_or_struct, module}, range} when module_or_struct in [:struct, :module] -> + {:ok, {:module, module}, range} + + _ -> + {:error, :not_a_module} + end + end + + defp resolve(path, %Position{} = position) do + uri = Document.Path.ensure_uri(path) + + with {:ok, _} <- Document.Store.open_temporary(uri), + {:ok, _document, analysis} <- Document.Store.fetch(uri, :analysis) do + resolve(analysis, position) + end + end + + defp cursor_at_declaration?(module, rename_range) do + case Store.exact(module, type: :module, subtype: :definition) do + {:ok, [definition]} -> + rename_range == definition.range + + _ -> + false + end + end + + defp surround_the_whole_module(analysis, position) do + # When renaming occurs, we want users to be able to choose any place in the defining module, + # not just the last local module, like: `defmodule |Foo.Bar do` also works. + {:ok, %{end: {_end_line, end_character}}} = Ast.surround_context(analysis, position) + end_position = %{position | character: end_character - 1} + {:ok, {:module, module}, range} = resolve(analysis, end_position) + {module, range} + end + + defp exacts(module, to_be_renamed) do + module + |> query_for_exacts() + |> Enum.filter(&entry_matching?(&1, to_be_renamed)) + |> adjust_range_for_exacts(to_be_renamed) + end + + defp descendants(module, to_be_renamed) do + module + |> query_for_descendants() + |> Enum.filter(&(entry_matching?(&1, to_be_renamed) and has_dots_in_range?(&1))) + |> adjust_range_for_descendants(module, to_be_renamed) + end + + defp query_for_exacts(module) do + module_string = Formats.module(module) + + case Store.exact(module_string, type: :module) do + {:ok, entries} -> Enum.map(entries, &Entry.new/1) + {:error, _} -> [] + end + end + + defp query_for_descendants(module) do + module_string = Formats.module(module) + prefix = "#{module_string}." + + case Store.prefix(prefix, type: :module) do + {:ok, entries} -> Enum.map(entries, &Entry.new/1) + {:error, _} -> [] + end + end + + defp maybe_rename_file(document, entries, replacement) do + entries + |> Enum.map(&Rename.File.maybe_rename(document, &1, replacement)) + # every group should have only one `rename_file` + |> Enum.find(&(not is_nil(&1))) + end + + defp entry_matching?(entry, to_be_renamed) do + entry.range |> range_text() |> String.contains?(to_be_renamed) + end + + defp has_dots_in_range?(entry) do + entry.range |> range_text() |> String.contains?(".") + end + + defp adjust_range_for_exacts(entries, to_be_renamed) do + old_suffix_length = String.length(to_be_renamed) + + for %Entry{} = entry <- entries do + start_character = entry.edit_range.end.character - old_suffix_length + put_in(entry.edit_range.start.character, start_character) + end + end + + defp adjust_range_for_descendants(entries, module, to_be_renamed) do + for %Entry{} = entry <- entries, + range_text = range_text(entry.edit_range), + matches = matches(range_text, to_be_renamed), + result = resolve_module_range(entry, module, matches), + match?({:ok, _}, result) do + {_, range} = result + %{entry | edit_range: range} + end + end + + defp range_text(range) do + line(text: text) = range.end.context_line + String.slice(text, range.start.character - 1, range.end.character - range.start.character) + end + + defp resolve_module_range(_entry, _module, []) do + {:error, :not_found} + end + + defp resolve_module_range(entry, module, [[{start, length}]]) do + range = adjust_range_characters(entry.edit_range, {start, length}) + + with {:ok, {:module, ^module}, _} <- resolve(entry.path, range.end) do + {:ok, range} + end + end + + defp resolve_module_range(entry, module, [[{start, length}] | tail] = _matches) do + # This function is mainly for the duplicated suffixes + # For example, if we have a module named `Foo.Bar.Foo.Bar` and we want to rename it to `Foo.Bar.Baz` + # The `Foo.Bar` will be duplicated in the range text, so we need to resolve the correct range + # and only rename the second occurrence of `Foo.Bar` + start_character = entry.edit_range.start.character + start + position = %{entry.edit_range.start | character: start_character} + + with {:ok, {:module, result}, range} <- resolve(entry.path, position) do + if result == module do + range = adjust_range_characters(range, {start, length}) + {:ok, range} + else + resolve_module_range(entry, module, tail) + end + end + end + + defp matches(range_text, "") do + # When expanding a module, the to_be_renamed is an empty string, + # so we need to scan the module before the period + for [{start, length}] <- Regex.scan(~r/\w+(?=\.)/, range_text, return: :index) do + [{start + length, 0}] + end + end + + defp matches(range_text, to_be_renamed) do + Regex.scan(~r/#{to_be_renamed}/, range_text, return: :index) + end + + defp adjust_range_characters(%Range{} = range, {start, length} = _matched_old_suffix) do + start_character = range.start.character + start + end_character = start_character + length + + range + |> put_in([:start, :character], start_character) + |> put_in([:end, :character], end_character) + end + + defp to_document_changes(uri, entries, replacement) do + with {:ok, document} <- Document.Store.open_temporary(uri) do + rename_file = maybe_rename_file(document, entries, replacement) + + edits = + Enum.map(entries, fn entry -> + reference? = entry.subtype == :reference + + if reference? and not ancestor_is_alias?(document, entry.edit_range.start) do + replacement = replacement |> String.split(".") |> Enum.at(-1) + Edit.new(replacement, entry.edit_range) + else + Edit.new(replacement, entry.edit_range) + end + end) + + {:ok, Document.Changes.new(document, edits, rename_file)} + end + end + + defp ancestor_is_alias?(%Document{} = document, %Position{} = position) do + document + |> Ast.cursor_path(position) + |> Enum.any?(&match?({:alias, _, _}, &1)) + end +end diff --git a/apps/engine/lib/engine/code_mod/rename/module/diff.ex b/apps/engine/lib/engine/code_mod/rename/module/diff.ex new file mode 100644 index 00000000..68a02280 --- /dev/null +++ b/apps/engine/lib/engine/code_mod/rename/module/diff.ex @@ -0,0 +1,50 @@ +defmodule Engine.CodeMod.Rename.Module.Diff do + @moduledoc """ + A module for computing the diff between old and new module names during renaming. + """ + + @doc """ + Computes the parts of the module name that need to be renamed and their replacements. + + Returns a tuple `{to_be_renamed, replacement}` where: + - `to_be_renamed` is the suffix of the old name that needs to be changed + - `replacement` is what it should be changed to + + ## Examples + + iex> diff("TopLevel.Parent.Child", "TopLevel.Renamed.Child") + {"Parent", "Renamed"} + + iex> diff("TopLevel.Parent.Child", "TopLevel.Child") + {"Parent.Child", "Child"} + + iex> diff("Foo.Bar", "Baz.Qux") + {"Foo.Bar", "Baz.Qux"} + """ + @spec diff(String.t(), String.t()) :: {String.t(), String.t()} + def diff(old_name, new_name) do + with [{:eq, eq} | _] <- String.myers_difference(old_name, new_name), + equal_segment <- trim_last_dot_part(eq), + true <- not is_nil(equal_segment) do + to_be_renamed = replace_leading_eq(old_name, equal_segment) + replacement = replace_leading_eq(new_name, equal_segment) + {to_be_renamed, replacement} + else + _ -> + {old_name, new_name} + end + end + + defp trim_last_dot_part(module) do + split = module |> String.reverse() |> String.split(".", parts: 2) + + if length(split) == 2 do + [_, rest] = split + rest |> String.reverse() + end + end + + defp replace_leading_eq(module, eq) do + module |> String.replace(~r"^#{eq}", "") |> String.trim_leading(".") + end +end diff --git a/apps/engine/lib/engine/code_mod/rename/prepare.ex b/apps/engine/lib/engine/code_mod/rename/prepare.ex new file mode 100644 index 00000000..7f976fef --- /dev/null +++ b/apps/engine/lib/engine/code_mod/rename/prepare.ex @@ -0,0 +1,70 @@ +defmodule Engine.CodeMod.Rename.Prepare do + @moduledoc """ + Handles the preparation phase of rename operations. + + The preparation phase determines: + - Whether the entity at the cursor can be renamed + - What the current name is + - What range should be replaced + """ + alias Engine.CodeIntelligence.Entity + alias Engine.CodeMod.Rename + alias Forge.Ast.Analysis + alias Forge.Document.Position + alias Forge.Document.Range + alias Forge.Formats + + require Logger + + @renaming_modules [Rename.Module] + + @doc """ + Prepares a rename operation at the given position. + + Returns `{:ok, module_name_string, range}` if the entity can be renamed, + or an appropriate error. + """ + @spec prepare(Analysis.t(), Position.t()) :: + {:ok, String.t(), Range.t()} | {:ok, nil} | {:error, term()} + def prepare(%Analysis{} = analysis, %Position{} = position) do + case resolve(analysis, position) do + {:ok, {:module, module}, range} -> + {:ok, Formats.module(module), range} + + {:error, {:unsupported_location, _}} -> + {:ok, nil} + + {:error, {:unsupported_entity, entity_type}} -> + {:error, "Renaming #{inspect(entity_type)} is not supported for now"} + + {:error, error} -> + {:error, error} + end + end + + @doc """ + Resolves the entity at the given position for renaming. + + Returns `{:ok, {entity_type, entity}, range}` if the entity can be renamed, + or an error tuple. + """ + @spec resolve(Analysis.t(), Position.t()) :: + {:ok, {atom(), atom()}, Range.t()} | {:error, tuple() | atom()} + def resolve(%Analysis{} = analysis, %Position{} = position) do + prepare_result = + Enum.find_value(@renaming_modules, fn module -> + if module.recognizes?(analysis, position) do + module.prepare(analysis, position) + end + end) + + prepare_result || handle_unsupported_entity(analysis, position) + end + + defp handle_unsupported_entity(analysis, position) do + with {:ok, other, _range} <- Entity.resolve(analysis, position) do + Logger.info("Unsupported entity for renaming: #{inspect(other)}") + {:error, {:unsupported_entity, elem(other, 0)}} + end + end +end diff --git a/apps/engine/lib/engine/commands/reindex.ex b/apps/engine/lib/engine/commands/reindex.ex index 1a9d8a9a..1a0d726d 100644 --- a/apps/engine/lib/engine/commands/reindex.ex +++ b/apps/engine/lib/engine/commands/reindex.ex @@ -54,8 +54,7 @@ defmodule Engine.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_open(uri), {:ok, entries} <- Indexer.Quoted.index_with_cleanup(analysis) do {:ok, document.path, entries} else @@ -64,6 +63,20 @@ defmodule Engine.Commands.Reindex do error end end + + defp ensure_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.Store.open_temporary(uri) do + Document.Store.fetch(uri, :analysis) + end + end + end end @moduledoc """ diff --git a/apps/engine/lib/engine/commands/rename.ex b/apps/engine/lib/engine/commands/rename.ex new file mode 100644 index 00000000..e8149624 --- /dev/null +++ b/apps/engine/lib/engine/commands/rename.ex @@ -0,0 +1,211 @@ +defmodule Engine.Commands.Rename do + @moduledoc """ + Tracks rename progress and triggers reindexing after rename operations complete. + + This GenServer is started when a rename operation begins. It tracks expected file + operations (file_changed, file_saved) and when all operations are received, it + reindexes the modified files and clears old entries from the search index. + + This is necessary because after a rename, the search index still contains the old + module names. Without reindexing, subsequent renames won't find the new entries. + """ + + alias Engine.Commands.Reindex + alias Engine.Search.Store + alias Forge.EngineApi.Messages + + require Logger + import Messages + + use GenServer + + defmodule State do + @moduledoc false + + @type uri_to_expected_operation :: %{ + Forge.uri() => Messages.file_changed() | Messages.file_saved() + } + + @type t :: %__MODULE__{ + uri_to_expected_operation: uri_to_expected_operation(), + paths_to_reindex: list(Forge.uri()), + paths_to_delete: list(Forge.uri()), + on_update_progress: (integer(), String.t() -> :ok), + on_complete: (-> :ok) + } + + defstruct uri_to_expected_operation: %{}, + paths_to_reindex: [], + paths_to_delete: [], + on_update_progress: nil, + on_complete: nil + + def new( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) do + %__MODULE__{ + uri_to_expected_operation: uri_to_expected_operation, + paths_to_reindex: paths_to_reindex, + paths_to_delete: paths_to_delete, + on_update_progress: on_update_progress, + on_complete: on_complete + } + end + + def update_progress(%__MODULE__{} = state, file_changed(uri: uri)) do + update_progress(state, uri, file_changed(uri: uri)) + end + + def update_progress(%__MODULE__{} = state, file_saved(uri: uri)) do + update_progress(state, uri, file_saved(uri: uri)) + end + + defp update_progress(%__MODULE__{} = state, uri, message) do + new_uri_with_expected_operation = + maybe_pop_expected_operation( + state.uri_to_expected_operation, + uri, + message, + state.on_update_progress + ) + + if Enum.empty?(new_uri_with_expected_operation) do + reindex_all_modified_files(state) + state.on_complete.() + end + + %__MODULE__{state | uri_to_expected_operation: new_uri_with_expected_operation} + end + + def in_progress?(%__MODULE__{} = state) do + state.uri_to_expected_operation != %{} + end + + defp maybe_pop_expected_operation(uri_to_operation, uri, message, on_update_progress) do + case uri_to_operation do + %{^uri => ^message} -> + on_update_progress.(1, "") + Map.delete(uri_to_operation, uri) + + _ -> + uri_to_operation + end + end + + defp reindex_all_modified_files(%__MODULE__{} = state) do + Enum.each(state.paths_to_reindex, fn uri -> + Reindex.uri(uri) + state.on_update_progress.(1, "reindexing") + end) + + Enum.each(state.paths_to_delete, fn uri -> + Store.clear(uri) + state.on_update_progress.(1, "deleting old index") + end) + end + end + + @spec child_spec( + %{Forge.uri() => Messages.file_changed() | Messages.file_saved()}, + list(Forge.uri()), + list(Forge.uri()), + (integer(), String.t() -> :ok), + (-> :ok) + ) :: Supervisor.child_spec() + def child_spec( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) do + %{ + id: __MODULE__, + start: + {__MODULE__, :start_link, + [ + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ]}, + restart: :transient + } + end + + def start_link( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) do + Logger.info( + "Starting rename tracking: #{map_size(uri_to_expected_operation)} operations, " <> + "#{length(paths_to_reindex)} to reindex, #{length(paths_to_delete)} to delete" + ) + + state = + State.new( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) + + GenServer.start_link(__MODULE__, state, name: __MODULE__) + end + + @impl true + def init(state) do + {:ok, state, {:continue, :start_buffering}} + end + + @doc """ + Updates the rename progress with a file operation message. + + This should be called when files are changed or saved during rename. + Returns `:ok` if the message was processed, `{:error, :not_in_rename_progress}` + if no rename is in progress. + """ + @spec update_progress(Messages.file_changed() | Messages.file_saved()) :: + :ok | {:error, :not_in_rename_progress} + def update_progress(message) do + pid = Process.whereis(__MODULE__) + + if pid && Process.alive?(pid) do + GenServer.cast(__MODULE__, {:update_progress, message}) + else + {:error, :not_in_rename_progress} + end + end + + @impl true + def handle_continue(:start_buffering, state) do + Engine.Api.Proxy.start_buffering() + {:noreply, state} + end + + @impl true + def handle_call(:in_progress?, _from, state) do + {:reply, State.in_progress?(state), state} + end + + @impl true + def handle_cast({:update_progress, message}, state) do + new_state = State.update_progress(state, message) + + if State.in_progress?(new_state) do + {:noreply, new_state} + else + Logger.info("Rename process completed.") + {:stop, :normal, new_state} + end + end +end diff --git a/apps/engine/lib/engine/commands/rename_supervisor.ex b/apps/engine/lib/engine/commands/rename_supervisor.ex new file mode 100644 index 00000000..7a72fdcb --- /dev/null +++ b/apps/engine/lib/engine/commands/rename_supervisor.ex @@ -0,0 +1,58 @@ +defmodule Engine.Commands.RenameSupervisor do + @moduledoc """ + DynamicSupervisor for managing rename progress tracking GenServers. + + Each rename operation spawns a transient child that tracks progress + and shuts down when the rename is complete. + """ + + alias Engine.Commands.Rename + + use DynamicSupervisor + + def child_spec(_) do + %{ + id: __MODULE__, + start: {__MODULE__, :start_link, []} + } + end + + def start_link do + DynamicSupervisor.start_link(__MODULE__, [], name: __MODULE__) + end + + @doc """ + Starts a new rename progress tracker. + + ## Parameters + + - `uri_to_expected_operation` - Map of URIs to expected messages (file_changed/file_saved) + - `paths_to_reindex` - List of URIs that need to be reindexed after rename + - `paths_to_delete` - List of URIs whose entries should be deleted from the index + - `on_update_progress` - Callback function receiving (increment, message) + - `on_complete` - Callback function called when rename is complete + """ + def start_renaming( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) do + DynamicSupervisor.start_child( + __MODULE__, + Rename.child_spec( + uri_to_expected_operation, + paths_to_reindex, + paths_to_delete, + on_update_progress, + on_complete + ) + ) + end + + @impl true + def init(_init_arg) do + DynamicSupervisor.init(strategy: :one_for_one) + end +end diff --git a/apps/engine/lib/engine/progress.ex b/apps/engine/lib/engine/progress.ex index 98a5640f..3c4f4a02 100644 --- a/apps/engine/lib/engine/progress.ex +++ b/apps/engine/lib/engine/progress.ex @@ -7,6 +7,14 @@ defmodule Engine.Progress do alias Engine.Dispatch + import Forge.EngineApi.Messages + + @type label :: String.t() + @type message :: String.t() + @type delta :: pos_integer() + @type on_complete_callback :: (-> any()) + @type report_progress_callback :: (delta(), message() -> any()) + @impl true def begin(title, opts \\ []) when is_list(opts) do Dispatch.erpc_call(Expert.Progress, :begin, [title, opts]) @@ -29,4 +37,39 @@ defmodule Engine.Progress do Dispatch.erpc_cast(Expert.Progress, :complete, [token, opts]) :ok end + + @doc """ + Begins a percent-based progress operation. + + Returns a tuple of `{report_progress_callback, on_complete_callback}` that can + be used to report progress and signal completion. + + ## Parameters + + - `label` - The label/title for the progress + - `max` - The maximum value (total number of operations) + + ## Example + + {report_progress, on_complete} = Progress.begin_percent("Renaming", 10) + report_progress.(1, "Processing file...") + on_complete.() + """ + @spec begin_percent(label(), pos_integer()) :: + {report_progress_callback(), on_complete_callback()} + def begin_percent(label, max) do + Engine.broadcast(percent_progress(label: label, max: max, stage: :begin)) + + report_progress = fn delta, message -> + Engine.broadcast( + percent_progress(label: label, message: message, delta: delta, stage: :report) + ) + end + + complete = fn -> + Engine.broadcast(percent_progress(label: label, stage: :complete)) + end + + {report_progress, complete} + end end diff --git a/apps/engine/test/engine/code_mod/rename/module/diff_test.exs b/apps/engine/test/engine/code_mod/rename/module/diff_test.exs new file mode 100644 index 00000000..d5cc2258 --- /dev/null +++ b/apps/engine/test/engine/code_mod/rename/module/diff_test.exs @@ -0,0 +1,27 @@ +defmodule Engine.CodeMod.Rename.Module.DiffTest do + alias Engine.CodeMod.Rename.Module.Diff + + use ExUnit.Case, async: true + + describe "diff/2" do + test "returns the local module pair if only the local name is changed" do + assert Diff.diff("A.B.C", "A.B.D") == {"C", "D"} + end + + test "returns the local module pair even if the part of local name is changed" do + assert Diff.diff("A.B.CD", "A.B.CC") == {"CD", "CC"} + end + + test "returns the suffix when extending the middle part" do + assert Diff.diff("Foo.Bar", "Foo.Baz.Bar") == {"Bar", "Baz.Bar"} + end + + test "returns the suffix if the middle part is removed" do + assert Diff.diff("Foo.Baz.Bar", "Foo.Bar") == {"Baz.Bar", "Bar"} + end + + test "returns the entire module pair if the change starts from the first module" do + assert Diff.diff("Foo.Bar", "Foa.Bar") == {"Foo.Bar", "Foa.Bar"} + end + end +end diff --git a/apps/engine/test/engine/code_mod/rename_test.exs b/apps/engine/test/engine/code_mod/rename_test.exs new file mode 100644 index 00000000..48a9a48f --- /dev/null +++ b/apps/engine/test/engine/code_mod/rename_test.exs @@ -0,0 +1,908 @@ +defmodule Engine.CodeMod.RenameTest do + alias Engine.CodeMod.Rename + alias Engine.Search + alias Engine.Search.Store.Backends + alias Forge.Document + + use ExUnit.Case, async: false + use Patch + + import Forge.Test.CodeSigil + import Forge.Test.CursorSupport + import Forge.Test.Fixtures + import Forge.Test.EventualAssertions + + setup do + project = project() + + Backends.Ets.destroy_all(project) + Engine.set_project(project) + + start_supervised!({Document.Store, derive: [analysis: &Forge.Ast.analyze/1]}) + start_supervised!(Engine.Dispatch) + start_supervised!(Backends.Ets) + + start_supervised!( + {Search.Store, [project, fn _ -> {:ok, []} end, fn _, _ -> {:ok, [], []} end, Backends.Ets]} + ) + + Search.Store.enable() + assert_eventually(Search.Store.loaded?(), 1500) + + on_exit(fn -> + Backends.Ets.destroy_all(project) + end) + + {:ok, project: project} + end + + describe "prepare/2" do + test "returns module name" do + {:ok, result, _} = + ~q[ + defmodule |Users do + end + ] + |> prepare() + + assert result == "Users" + end + + test "returns full module name" do + {:ok, result, _} = + ~q[ + defmodule MyApp.|Users do + end + ] + |> prepare() + + assert result == "MyApp.Users" + end + + test "returns full module name when cursor is in the middle" do + {:ok, result, _} = + ~q[ + defmodule My|App.Users do + end + ] + |> prepare() + + assert result == "MyApp.Users" + end + + test "returns nil for module reference" do + assert {:ok, nil} = + ~q[ + defmodule MyApp.Users do + end + + defmodule MyApp.Accounts do + alias |MyApp.Users + end + ] + |> prepare() + end + + test "returns error for unsupported entity" do + assert {:error, "Renaming :variable is not supported for now"} = + ~q[ + x = 1 + |x + ] + |> prepare() + end + end + + describe "rename/4 basic" do + test "renames at definition" do + {:ok, result} = + ~q[ + defmodule |Users do + end + ] + |> rename("Accounts") + + assert result =~ ~S[defmodule Accounts do] + end + + test "fails at alias reference" do + assert {:error, {:unsupported_location, :module}} == + ~q[ + defmodule MyApp.Accounts do + alias |MyApp.Users + end + ] + |> rename("Members") + end + + test "fails at module call reference" do + assert {:error, {:unsupported_location, :module}} == + ~q[ + defmodule MyApp.Accounts do + alias MyApp.Users + Use|rs.list() + end + ] + |> rename("Members") + end + + test "renames multi-part module" do + {:ok, result} = + ~q[ + defmodule MyApp.Auth.|Session do + end + ] + |> rename("MyApp.Auth.Token") + + assert result =~ ~S[defmodule MyApp.Auth.Token do] + end + + test "renames middle segment" do + {:ok, result} = + ~q[ + defmodule MyApp.Auth.|Session do + end + ] + |> rename("MyApp.Identity.Session") + + assert result =~ ~S[defmodule MyApp.Identity.Session do] + end + + test "simplifies module name" do + {:ok, result} = + ~q[ + defmodule MyApp.Auth.|Session do + end + ] + |> rename("MyApp.Session") + + assert result =~ ~S[defmodule MyApp.Session do] + end + + test "renames nested module definition" do + {:ok, result} = + ~q[ + defmodule MyApp.Users do + defmodule |Query do + end + end + + defmodule MyApp.UsersTest do + alias MyApp.Users.Query + end + ] + |> rename("Filter") + + assert result == ~q[ + defmodule MyApp.Users do + defmodule Filter do + end + end + + defmodule MyApp.UsersTest do + alias MyApp.Users.Filter + end + ] + end + + test "renames multi-alias syntax" do + {:ok, result} = + ~q[ + defmodule MyApp.|Accounts do + end + + defmodule MyApp.Web do + alias MyApp.{ + Users, Accounts, + Auth.Session + } + end + ] + |> rename("MyApp.Members") + + assert result =~ ~S[ Users, Members,] + end + end + + describe "rename/4 with references" do + test "does not rename similar module names" do + {:ok, result} = + ~q[ + defmodule |MyApp.Users do + end + + defmodule MyApp.UsersTest do + end + ] + |> rename("MyApp.Accounts") + + assert result =~ ~S[defmodule MyApp.UsersTest do] + end + + test "renames local references when local name changes" do + {:ok, result} = + ~q[ + defmodule |MyApp.Users do + end + + defmodule MyApp.UsersTest do + alias MyApp.Users + Users.list() + end + ] + |> rename("MyApp.Accounts") + + assert result =~ ~S[alias MyApp.Accounts] + assert result =~ ~S[ Accounts.list()] + end + + test "keeps local reference when local name unchanged" do + {:ok, result} = + ~q[ + defmodule |MyApp.Users do + end + + defmodule MyApp.Admin do + alias MyApp.Users + + Users.list() # no change + end + ] + |> rename("MyApp.Core.Users") + + assert result =~ ~S[defmodule MyApp.Core.Users do] + assert result =~ ~S[alias MyApp.Core.Users] + assert result =~ ~S[ Users.list() # no change] + end + + test "keeps local reference when only prefix changes" do + {:ok, result} = + ~q[ + defmodule |MyApp.Core.Utils do + end + + defmodule MyApp.Web do + alias MyApp.Core.Utils + + Utils.format() # no change + end + ] + |> rename("MyApp.Shared.Utils") + + assert result =~ ~S[defmodule MyApp.Shared.Utils do] + assert result =~ ~S[alias MyApp.Shared.Utils] + assert result =~ ~S[ Utils.format() # no change] + end + end + + describe "rename/4 descendants" do + test "renames descendants" do + {:ok, result} = + ~q[ + defmodule MyApp.|Users do + end + + defmodule MyApp.Users.Query do + end + ] + |> rename("MyApp.Accounts") + + assert result =~ ~S[defmodule MyApp.Accounts.Query] + assert result =~ ~S[defmodule MyApp.Accounts do] + end + + test "renames descendants when expanding name" do + {:ok, result} = + ~q[ + defmodule MyApp.|Users do + alias MyApp.Users.Query + end + + defmodule MyApp.Users.Query do + end + ] + |> rename("MyApp.Core.Users") + + assert result =~ ~S[defmodule MyApp.Core.Users] + assert result =~ ~S[alias MyApp.Core.Users.Query] + assert result =~ ~S[defmodule MyApp.Core.Users.Query do] + end + + test "renames descendants with multi-part expansion" do + {:ok, result} = + ~q[ + defmodule MyApp.|Auth do + end + + defmodule MyApp.Auth.Session do + end + + defmodule MyApp.AuthTest do + alias MyApp.Auth + alias MyApp.Auth.Session + end + ] + |> rename("MyApp.Auth.OAuth") + + assert result =~ ~S[defmodule MyApp.Auth.OAuth do] + assert result =~ ~S[alias MyApp.Auth.OAuth] + assert result =~ ~S[alias MyApp.Auth.OAuth.Session] + end + + test "handles repeated module name in path" do + {:ok, result} = + ~q[ + defmodule MyApp.Core.MyApp.|Core do + end + + defmodule MyApp.Core.MyApp.Core.Utils do + end + + defmodule MyApp.Web do + alias MyApp.Core.MyApp.Core.Utils + end + ] + |> rename("MyApp.Core.MyApp.Shared") + + assert result =~ ~S[defmodule MyApp.Core.MyApp.Shared do] + assert result =~ ~S[defmodule MyApp.Core.MyApp.Shared.Utils do] + assert result =~ ~S[alias MyApp.Core.MyApp.Shared.Utils] + end + + test "skips same-named nested modules" do + {:ok, result} = + ~q[ + defmodule MyApp.|Users do + defmodule Users do # skip this + end + end + + defmodule MyApp.Admin do + alias MyApp.Users.Users + end + ] + |> rename("MyApp.Accounts") + + assert result =~ ~S[defmodule MyApp.Accounts do] + assert result =~ ~S[defmodule Users do # skip this] + assert result =~ ~S[alias MyApp.Accounts.Users] + end + + test "renames when removing middle segment" do + {:ok, result} = + ~q[ + defmodule |MyApp.Core.Users do + end + + defmodule MyApp.Core.Users.Query do + alias MyApp.Core.Users.Query + end + ] + |> rename("MyApp.Users") + + assert result =~ ~S[defmodule MyApp.Users.Query do] + assert result =~ ~S[alias MyApp.Users.Query] + end + + test "does not rename modules containing old suffix as substring" do + {:ok, result} = + ~q[ + defmodule |MyApp.Ast do + end + + defmodule MyApp.Parser do + alias MyApp.Ast.Detection + + Detection.Bitstring.detected?() # Bitstring contains the old suffix: `st` + end + ] + |> rename("MyApp.AST") + + refute result =~ ~S[Detection.BitSTring.detected?()] + end + end + + describe "rename/4 struct" do + test "renames struct references" do + {:ok, result} = + ~q[ + defmodule |MyApp.User do + defstruct name: nil, email: nil + end + + defmodule MyApp.Accounts do + def new do + %MyApp.User{} + end + end + ] + |> rename("MyApp.Account") + + assert result =~ ~S[defmodule MyApp.Account do] + assert result =~ ~S[%MyApp.Account{}] + end + + test "renames struct in pattern matching" do + {:ok, result} = + ~q[ + defmodule |MyApp.User do + defstruct name: nil, email: nil + end + + defmodule MyApp.Accounts do + def get_name(%MyApp.User{name: name}), do: name + end + ] + |> rename("MyApp.Account") + + assert result =~ ~S[def get_name(%MyApp.Account{name: name})] + end + + test "renames struct update syntax" do + {:ok, result} = + ~q[ + defmodule |MyApp.User do + defstruct name: nil, email: nil + end + + defmodule MyApp.Accounts do + def update(user, name) do + %MyApp.User{user | name: name} + end + end + ] + |> rename("MyApp.Account") + + assert result =~ ~S[%MyApp.Account{user | name: name}] + end + end + + describe "rename/4 edge cases" do + test "does not rename module with similar prefix" do + # MyApp.User should not affect MyApp.UserProfile + {:ok, result} = + ~q[ + defmodule |MyApp.User do + end + + defmodule MyApp.UserProfile do + end + + defmodule MyApp.Consumer do + alias MyApp.User + alias MyApp.UserProfile + end + ] + |> rename("MyApp.Account") + + assert result =~ ~S[defmodule MyApp.Account do] + # UserProfile should NOT be renamed + assert result =~ ~S[defmodule MyApp.UserProfile do] + assert result =~ ~S[alias MyApp.UserProfile] + end + + test "does not rename module that contains old name as substring" do + # Renaming MyApp.API should not affect MyApp.SomeAPIService + {:ok, result} = + ~q[ + defmodule |MyApp.API do + end + + defmodule MyApp.SomeAPIService do + end + ] + |> rename("MyApp.Gateway") + + assert result =~ ~S[defmodule MyApp.Gateway do] + # SomeAPIService should NOT be renamed to SomeGatewayService + assert result =~ ~S[defmodule MyApp.SomeAPIService do] + end + + test "correctly renames when old and new names share common prefix" do + # MyApp.User -> MyApp.UserAccount (extending the name) + {:ok, result} = + ~q[ + defmodule |MyApp.User do + end + + defmodule MyApp.Consumer do + alias MyApp.User + end + ] + |> rename("MyApp.UserAccount") + + assert result =~ ~S[defmodule MyApp.UserAccount do] + assert result =~ ~S[alias MyApp.UserAccount] + end + + test "correctly renames when new name is shorter" do + # MyApp.UserAccount -> MyApp.User (shortening the name) + {:ok, result} = + ~q[ + defmodule |MyApp.UserAccount do + end + + defmodule MyApp.Consumer do + alias MyApp.UserAccount + end + ] + |> rename("MyApp.User") + + assert result =~ ~S[defmodule MyApp.User do] + assert result =~ ~S[alias MyApp.User] + end + + test "handles renaming module with single segment name" do + {:ok, result} = + ~q[ + defmodule |Users do + end + + defmodule Consumer do + alias Users + end + ] + |> rename("Accounts") + + assert result =~ ~S[defmodule Accounts do] + assert result =~ ~S[alias Accounts] + end + + test "handles renaming to completely different module path" do + # MyApp.Users -> OtherApp.Accounts (different prefix entirely) + {:ok, result} = + ~q[ + defmodule |MyApp.Users do + end + + defmodule MyApp.Consumer do + alias MyApp.Users + end + ] + |> rename("OtherApp.Accounts") + + assert result =~ ~S[defmodule OtherApp.Accounts do] + assert result =~ ~S[alias OtherApp.Accounts] + end + + test "renames deeply nested module correctly" do + {:ok, result} = + ~q[ + defmodule |MyApp.Core.Domain.Users.Query do + end + + defmodule MyApp.Web do + alias MyApp.Core.Domain.Users.Query + end + ] + |> rename("MyApp.Core.Domain.Users.Filter") + + assert result =~ ~S[defmodule MyApp.Core.Domain.Users.Filter do] + assert result =~ ~S[alias MyApp.Core.Domain.Users.Filter] + end + + test "does not corrupt nearby code when renaming" do + {:ok, result} = + ~q[ + defmodule |MyApp.Users do + @moduledoc "Users module" + + def list, do: :ok + end + + defmodule MyApp.UsersTest do + use ExUnit.Case + end + ] + |> rename("MyApp.Accounts") + + # Verify the module is renamed + assert result =~ ~S[defmodule MyApp.Accounts do] + # Verify other code is preserved + assert result =~ ~S[@moduledoc "Users module"] + assert result =~ ~S[def list, do: :ok] + # UsersTest should NOT be renamed (different module) + assert result =~ ~S[defmodule MyApp.UsersTest do] + end + end + + describe "rename/4 file renaming" do + setup do + patch(Engine.CodeMod.Rename.File, :function_exists?, false) + :ok + end + + test "does not rename file with parent module", %{project: project} do + {:ok, {_applied, nil}} = + ~q[ + defmodule MyApp.Server do + defmodule |State do + end + end + ] + |> rename_with_file("Config", "lib/my_app/server.ex", project) + end + + test "does not rename file with sibling modules", %{project: project} do + assert {:ok, {_applied, nil}} = + ~q[ + defmodule |MyApp.Users do + end + + defmodule MyApp.Accounts do + end + ] + |> rename_with_file("Members", "lib/my_app/users.ex", project) + end + + test "does not rename file for non-conventional path", %{project: project} do + assert {:ok, {_applied, nil}} = + ~q[ + defmodule |MyApp.MixProject do + end + ] + |> rename_with_file("MyApp.Renamed", "mix.exs", project) + end + + test "renames lib file", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |MyApp.Users do + end + ] + |> rename_with_file("MyApp.Accounts", "lib/my_app/users.ex", project) + + assert rename_file.new_uri == subject_uri(project, "lib/my_app/accounts.ex") + end + + test "does not rename file when only case changes", %{project: project} do + assert {:ok, {_applied, nil}} = + ~q[ + defmodule |MyApp.Users do + end + ] + |> rename_with_file("MyApp.USERS", "lib/my_app/users.ex", project) + end + + test "renames umbrella app file", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |MyApp.Users do + end + ] + |> rename_with_file("MyApp.Accounts", "apps/my_app/lib/my_app/users.ex", project) + + assert rename_file.new_uri == subject_uri(project, "apps/my_app/lib/my_app/accounts.ex") + end + + test "renames umbrella app nested file", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |Engine.CodeMod do + end + ] + |> rename_with_file( + "Engine.Refactor", + "apps/engine/lib/engine/code_mod.ex", + project + ) + + assert rename_file.new_uri == + subject_uri(project, "apps/engine/lib/engine/refactor.ex") + end + + test "renames test file", %{project: project} do + {:ok, {_applied, rename_file}} = + ~q[ + defmodule |MyApp.UsersTest do + end + ] + |> rename_with_file("MyApp.AccountsTest", "test/my_app/users_test.exs", project) + + assert rename_file.new_uri == subject_uri(project, "test/my_app/accounts_test.exs") + end + + test "preserves components folder for phoenix component", %{project: project} do + patch(Engine.CodeMod.Rename.File, :phoenix_component_module?, fn MyAppWeb.IconComponent -> + true + end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule MyAppWeb.|IconComponent do + end + ] + |> rename_with_file( + "MyAppWeb.BadgeComponent", + "lib/my_app_web/components/icon_component.ex", + project + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/my_app_web/components/badge_component.ex") + end + + test "preserves components folder for nested component", %{project: project} do + patch( + Engine.CodeMod.Rename.File, + :phoenix_component_module?, + fn MyAppWeb.Admin.IconComponent -> true end + ) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule MyAppWeb.Admin.|IconComponent do + end + ] + |> rename_with_file( + "MyAppWeb.Admin.BadgeComponent", + "lib/my_app_web/components/admin/icon_component.ex", + project + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/my_app_web/components/admin/badge_component.ex") + end + + test "preserves components folder when Components in module name", %{project: project} do + patch( + Engine.CodeMod.Rename.File, + :phoenix_component_module?, + fn MyAppWeb.Components.Icons -> + true + end + ) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule MyAppWeb.Components.|Icons do + end + ] + |> rename_with_file( + "MyAppWeb.Components.Badges", + "lib/my_app_web/components/icons.ex", + project + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/my_app_web/components/badges.ex") + end + + test "preserves controllers folder", %{project: project} do + patch( + Engine.CodeMod.Rename.File, + :phoenix_controller_module?, + fn MyAppWeb.UserController -> true end + ) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule MyAppWeb.|UserController do + end + ] + |> rename_with_file( + "MyAppWeb.AccountController", + "lib/my_app_web/controllers/user_controller.ex", + project + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/my_app_web/controllers/account_controller.ex") + end + + test "preserves controllers folder for JSON module", %{project: project} do + patch( + Engine.CodeMod.Rename.File, + :phoenix_controller_module?, + fn MyAppWeb.UserController.JSON -> true end + ) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule MyAppWeb.UserController.|JSON do + end + ] + |> rename_with_file( + "MyAppWeb.UserController.API", + "lib/my_app_web/controllers/user_controller/json.ex", + project + ) + + assert rename_file.new_uri == + subject_uri(project, "lib/my_app_web/controllers/user_controller/api.ex") + end + + test "preserves live folder", %{project: project} do + patch(Engine.CodeMod.Rename.File, :phoenix_liveview_module?, fn MyAppWeb.UserLive -> + true + end) + + {:ok, {_applied, rename_file}} = + ~q[ + defmodule MyAppWeb.|UserLive do + end + ] + |> rename_with_file("MyAppWeb.AccountLive", "lib/my_app_web/live/user_live.ex", project) + + assert rename_file.new_uri == subject_uri(project, "lib/my_app_web/live/account_live.ex") + end + end + + # Helpers + + defp prepare(code) do + with {position, code} <- pop_cursor(code), + {:ok, _document, analysis} <- index(code) do + Rename.prepare(analysis, position) + end + end + + defp rename(code, new_name) do + with {position, code} <- pop_cursor(code), + {:ok, document, analysis} <- index(code), + {:ok, results} <- Rename.rename(analysis, position, new_name, nil) do + case results do + [%Document.Changes{edits: edits, document: doc}] -> + {:ok, edited_doc} = + Document.apply_content_changes(doc, doc.version + 1, edits) + + {:ok, Document.to_string(edited_doc)} + + [] -> + {:ok, Document.to_string(document)} + end + end + end + + defp rename_with_file(code, new_name, path, project) do + uri = subject_uri(project, path) + + with {position, text} <- pop_cursor(code), + {:ok, document} <- open_document(uri, text), + {:ok, entries} <- Engine.Search.Indexer.Source.index_document(document), + :ok <- Search.Store.replace(entries), + {:ok, _document, analysis} <- Document.Store.fetch(uri, :analysis), + {: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) + rename_file = document_changes |> Enum.map(& &1.rename_file) |> List.first() + + {:ok, {applied, rename_file}} + end + end + + defp index(code) do + project = project() + uri = module_uri(project) + + with :ok <- Document.Store.open(uri, code, 1), + {:ok, document, analysis} <- Document.Store.fetch(uri, :analysis), + {:ok, entries} <- Engine.Search.Indexer.Quoted.index(analysis) do + Search.Store.replace(entries) + {:ok, document, analysis} + end + end + + defp module_uri(project) do + project + |> file_path(Path.join("lib", "my_module.ex")) + |> Document.Path.ensure_uri() + end + + defp subject_uri(project, path) do + project + |> file_path(path) + |> Document.Path.ensure_uri() + end + + defp open_document(uri, content) do + with :ok <- Document.Store.open(uri, content, 0) do + Document.Store.fetch(uri) + end + end + + defp apply_edits(document, text_edits) do + {:ok, edited_document} = Document.apply_content_changes(document, 1, text_edits) + Document.to_string(edited_document) + end +end diff --git a/apps/engine/test/engine/commands/rename_test.exs b/apps/engine/test/engine/commands/rename_test.exs new file mode 100644 index 00000000..1f5d3162 --- /dev/null +++ b/apps/engine/test/engine/commands/rename_test.exs @@ -0,0 +1,152 @@ +defmodule Engine.Commands.RenameTest do + alias Engine.Api.Proxy + alias Engine.Commands.Rename + alias Engine.Commands.RenameSupervisor + alias Engine.Search.Store + + import Forge.EngineApi.Messages + import Forge.Test.EventualAssertions + + use ExUnit.Case + use Patch + + setup do + start_supervised!(RenameSupervisor) + :ok + end + + setup do + pid = self() + + on_report_progress = fn delta, message -> update_progress(pid, delta, message) end + on_complete = fn -> complete_progress(pid) end + + patch(Proxy, :start_buffering, :ok) + %{on_report_progress: on_report_progress, on_complete: on_complete} + end + + test "it should start buffering when a rename is in progress", %{ + on_report_progress: on_report_progress, + on_complete: on_complete + } do + uri = "file://file.ex" + uri_with_expected_operation = %{uri => file_changed(uri: uri)} + paths_to_reindex = [uri] + + {:ok, _pid} = + RenameSupervisor.start_renaming( + uri_with_expected_operation, + paths_to_reindex, + [], + on_report_progress, + on_complete + ) + + assert_called(Proxy.start_buffering()) + end + + test "it should complete and shutdown the process when rename is done", %{ + on_report_progress: on_report_progress, + on_complete: on_complete + } do + uri = "file://file.ex" + paths_to_reindex = [uri] + + {:ok, _pid} = + RenameSupervisor.start_renaming( + %{uri => file_saved(uri: uri)}, + paths_to_reindex, + [], + on_report_progress, + on_complete + ) + + Rename.update_progress(file_saved(uri: uri)) + + assert_receive {:update_progress, 1, ""} + assert_receive :complete_progress + + refute_eventually Process.whereis(Rename) + end + + test "it should still be in progress if there are files yet to be saved", %{ + on_report_progress: on_report_progress, + on_complete: on_complete + } do + uri1 = "file://file1.ex" + old_uri = "file://file2.ex" + new_uri = "file://file3.ex" + + uri_with_expected_operation = %{ + uri1 => file_changed(uri: uri1), + new_uri => file_saved(uri: new_uri) + } + + paths_to_reindex = [uri1, new_uri] + paths_to_delete = [old_uri] + + {:ok, _pid} = + RenameSupervisor.start_renaming( + uri_with_expected_operation, + paths_to_reindex, + paths_to_delete, + on_report_progress, + on_complete + ) + + Rename.update_progress(file_changed(uri: uri1)) + assert_receive {:update_progress, 1, ""} + refute_receive :complete_progress + end + + test "it should reindex new files and delete old entries when all files are modified", %{ + on_report_progress: on_report_progress, + on_complete: on_complete + } do + patch(Store, :clear, :ok) + old_uri = "file://old_file.ex" + new_uri = "file://new_file.ex" + another_uri = "file://file.ex" + + uri_with_expected_operation = %{ + old_uri => file_changed(uri: old_uri), + new_uri => file_saved(uri: new_uri) + } + + paths_to_reindex = [new_uri, another_uri] + paths_to_delete = [old_uri] + + {:ok, _pid} = + RenameSupervisor.start_renaming( + uri_with_expected_operation, + paths_to_reindex, + paths_to_delete, + on_report_progress, + on_complete + ) + + Rename.update_progress(file_changed(uri: old_uri)) + assert_receive {:update_progress, 1, ""} + refute_receive :complete_progress + + Rename.update_progress(file_saved(uri: new_uri)) + assert_receive {:update_progress, 1, ""} + + assert_receive {:update_progress, 1, "reindexing"} + assert_receive {:update_progress, 1, "deleting old index"} + assert_receive :complete_progress + end + + test "it should return :error when updating progress if no rename is in progress" do + assert {:error, :not_in_rename_progress} = + Rename.update_progress(file_changed(uri: "file://file.ex")) + end + + defp update_progress(pid, delta, message) do + send(pid, {:update_progress, delta, message}) + end + + defp complete_progress(pid) do + send(pid, :complete_progress) + end +end diff --git a/apps/expert/lib/expert.ex b/apps/expert/lib/expert.ex index dc193d73..a207c860 100644 --- a/apps/expert/lib/expert.ex +++ b/apps/expert/lib/expert.ex @@ -274,6 +274,12 @@ defmodule Expert do %GenLSP.Requests.WorkspaceSymbol{} -> {:ok, Handlers.WorkspaceSymbol} + %GenLSP.Requests.TextDocumentPrepareRename{} -> + {:ok, Handlers.PrepareRename} + + %GenLSP.Requests.TextDocumentRename{} -> + {:ok, Handlers.Rename} + %request_module{} -> {:error, {:unhandled, request_module}} end diff --git a/apps/expert/lib/expert/engine_api.ex b/apps/expert/lib/expert/engine_api.ex index 46aa28e3..43f4f31f 100644 --- a/apps/expert/lib/expert/engine_api.ex +++ b/apps/expert/lib/expert/engine_api.ex @@ -147,5 +147,43 @@ defmodule Expert.EngineApi do call(project, Engine, :workspace_symbols, [query]) end + def prepare_rename(%Project{} = project, %Analysis{} = analysis, %Position{} = position) do + call(project, Engine, :prepare_rename, [analysis, position]) + end + + def rename( + %Project{} = project, + %Analysis{} = analysis, + %Position{} = position, + new_name, + client_name + ) do + call(project, Engine, :rename, [ + analysis, + position, + new_name, + client_name + ]) + end + + @doc """ + Updates the rename progress with a triggered message. + + This should be called when files are changed or saved during a rename operation. + The message is used to track which file operations have been completed. + + This function is intentionally fire-and-forget - failures don't affect + the main document change/save flow. + """ + def maybe_update_rename_progress(%Project{} = project, message) do + try do + call(project, Engine, :maybe_update_rename_progress, [message]) + rescue + _ -> {:error, :not_in_rename_progress} + catch + :exit, _ -> {:error, :not_in_rename_progress} + end + end + defdelegate stop(project), to: EngineNode end diff --git a/apps/expert/lib/expert/provider/handlers/prepare_rename.ex b/apps/expert/lib/expert/provider/handlers/prepare_rename.ex new file mode 100644 index 00000000..5d988562 --- /dev/null +++ b/apps/expert/lib/expert/provider/handlers/prepare_rename.ex @@ -0,0 +1,68 @@ +defmodule Expert.Provider.Handlers.PrepareRename do + @moduledoc """ + Handler for textDocument/prepareRename requests. + + This handler determines if the entity at the cursor can be renamed + and returns the range and placeholder text for the rename operation. + """ + alias Expert.Configuration + alias Expert.EngineApi + alias Forge.Ast + alias Forge.Document + alias GenLSP.Structures + + require Logger + + def handle( + %GenLSP.Requests.TextDocumentPrepareRename{ + params: %Structures.PrepareRenameParams{} = params + }, + %Configuration{} = config + ) do + document = Forge.Document.Container.context_document(params, nil) + + case Document.Store.fetch(document.uri, :analysis) do + {:ok, _document, %Ast.Analysis{valid?: true} = analysis} -> + prepare_rename(config.project, analysis, params.position) + + _ -> + {:error, :request_failed, "Document cannot be analyzed"} + end + end + + defp prepare_rename(project, analysis, position) do + case EngineApi.prepare_rename(project, analysis, position) do + {:ok, placeholder, range} when is_binary(placeholder) -> + # PrepareRenameResult is a type alias in GenLSP, return as map + result = %{ + placeholder: placeholder, + range: to_lsp_range(range) + } + + {:ok, result} + + {:ok, nil} -> + {:ok, nil} + + {:error, error} when is_binary(error) -> + {:error, :request_failed, error} + + {:error, error} -> + {:error, :request_failed, inspect(error)} + end + end + + defp to_lsp_range(%Forge.Document.Range{} = range) do + %Structures.Range{ + start: to_lsp_position(range.start), + end: to_lsp_position(range.end) + } + end + + defp to_lsp_position(%Forge.Document.Position{} = position) do + %Structures.Position{ + line: position.line - 1, + character: position.character - 1 + } + end +end diff --git a/apps/expert/lib/expert/provider/handlers/rename.ex b/apps/expert/lib/expert/provider/handlers/rename.ex new file mode 100644 index 00000000..8b489d47 --- /dev/null +++ b/apps/expert/lib/expert/provider/handlers/rename.ex @@ -0,0 +1,107 @@ +defmodule Expert.Provider.Handlers.Rename do + @moduledoc """ + Handler for textDocument/rename requests. + + This handler executes the rename operation and returns the workspace edit + containing all the text edits and file renames needed. + """ + alias Expert.Configuration + alias Expert.EngineApi + alias Forge.Ast + alias Forge.Document + alias Forge.Document.Changes + alias GenLSP.Structures + + require Logger + + def handle( + %GenLSP.Requests.TextDocumentRename{ + params: %Structures.RenameParams{} = params + }, + %Configuration{} = config + ) do + document = Forge.Document.Container.context_document(params, nil) + + case Document.Store.fetch(document.uri, :analysis) do + {:ok, _document, %Ast.Analysis{valid?: true} = analysis} -> + rename(config, analysis, params.position, params.new_name) + + _ -> + {:error, :request_failed, "Document cannot be analyzed"} + end + end + + defp rename(%Configuration{} = config, analysis, position, new_name) do + %Configuration{project: project, client_name: client_name} = config + + case EngineApi.rename(project, analysis, position, new_name, client_name) do + {:ok, []} -> + {:ok, nil} + + {:ok, results} -> + document_changes = + Enum.flat_map(results, fn + %Changes{rename_file: %Changes.RenameFile{}} = changes -> + [to_text_document_edit(changes), to_rename_file(changes.rename_file)] + + %Changes{} = changes -> + [to_text_document_edit(changes)] + end) + + workspace_edit = %Structures.WorkspaceEdit{document_changes: document_changes} + {:ok, workspace_edit} + + {:error, {:unsupported_entity, entity}} -> + Logger.info("Cannot rename entity: #{inspect(entity)}") + {:ok, nil} + + {:error, reason} -> + {:error, :request_failed, inspect(reason)} + end + end + + defp to_text_document_edit(%Changes{} = changes) do + %Changes{document: document, edits: edits} = changes + + text_document = + %Structures.OptionalVersionedTextDocumentIdentifier{ + uri: document.uri, + version: document.version + } + + %Structures.TextDocumentEdit{ + edits: Enum.map(edits, &to_text_edit/1), + text_document: text_document + } + end + + defp to_text_edit(%Document.Edit{} = edit) do + %Structures.TextEdit{ + new_text: edit.text, + range: to_lsp_range(edit.range) + } + end + + defp to_lsp_range(%Document.Range{} = range) do + %Structures.Range{ + start: to_lsp_position(range.start), + end: to_lsp_position(range.end) + } + end + + defp to_lsp_position(%Document.Position{} = position) do + %Structures.Position{ + line: position.line - 1, + character: position.character - 1 + } + end + + defp to_rename_file(%Changes.RenameFile{} = rename_file) do + %Structures.RenameFile{ + kind: "rename", + new_uri: rename_file.new_uri, + old_uri: rename_file.old_uri, + options: %Structures.RenameFileOptions{overwrite: true} + } + end +end diff --git a/apps/expert/lib/expert/state.ex b/apps/expert/lib/expert/state.ex index 2ba810b4..1814b75c 100644 --- a/apps/expert/lib/expert/state.ex +++ b/apps/expert/lib/expert/state.ex @@ -109,6 +109,7 @@ defmodule Expert.State do EngineApi.broadcast(project, updated_message) EngineApi.compile_document(state.configuration.project, updated_source) + EngineApi.maybe_update_rename_progress(project, updated_message) {:ok, state} error -> @@ -155,10 +156,20 @@ defmodule Expert.State do def apply(%__MODULE__{} = state, %GenLSP.Notifications.TextDocumentDidSave{params: params}) do uri = params.text_document.uri + project = state.configuration.project case Document.Store.save(uri) do :ok -> - EngineApi.schedule_compile(state.configuration.project, false) + EngineApi.schedule_compile(project, false) + EngineApi.maybe_update_rename_progress(project, file_saved(uri: uri)) + {:ok, state} + + {:error, :not_open} -> + # File was closed before save notification was processed. + # This can happen during batch renames when didClose and didSave + # arrive nearly simultaneously. Still update rename progress tracking. + Logger.debug("Save received for already-closed file: #{uri}") + EngineApi.maybe_update_rename_progress(project, file_saved(uri: uri)) {:ok, state} error -> @@ -221,6 +232,11 @@ defmodule Expert.State do trigger_characters: CodeIntelligence.Completion.trigger_characters() } + rename_options = + %Structures.RenameOptions{ + prepare_provider: true + } + server_capabilities = %Structures.ServerCapabilities{ code_action_provider: code_action_options, @@ -232,6 +248,7 @@ defmodule Expert.State do execute_command_provider: command_options, hover_provider: true, references_provider: true, + rename_provider: rename_options, text_document_sync: sync_options, workspace_symbol_provider: true } diff --git a/apps/expert/test/expert/state/rename_progress_test.exs b/apps/expert/test/expert/state/rename_progress_test.exs new file mode 100644 index 00000000..8b091e2b --- /dev/null +++ b/apps/expert/test/expert/state/rename_progress_test.exs @@ -0,0 +1,231 @@ +defmodule Expert.State.RenameProgressTest do + @moduledoc """ + Integration tests for rename progress tracking with different editors. + + Different editors send different events after rename operations: + - VSCode: sends both file_changed and file_saved events for renamed files + - Neovim: sends only file_changed for non-renamed files, file_saved for renamed files + + These tests verify the full rename flow from State → EngineApi → Engine. + """ + + alias Expert.Configuration + alias Expert.EngineApi + alias Expert.State + alias Engine.Commands.Rename + alias Engine.Commands.RenameSupervisor + alias Forge.Document + + import Forge.EngineApi.Messages + import Forge.Test.Fixtures + + use ExUnit.Case, async: false + use Patch + + setup do + start_supervised!(Expert.Application.document_store_child_spec()) + start_supervised!(RenameSupervisor) + + patch(Engine.Api.Proxy, :start_buffering, :ok) + patch(Engine.Commands.Reindex, :uri, fn _uri -> :ok end) + patch(Engine.Search.Store, :clear, fn _uri -> :ok end) + + if pid = Process.whereis(Rename) do + Process.exit(pid, :kill) + Process.sleep(10) + end + + :ok + end + + describe "VSCode editor - non-file-rename (edits only)" do + test "reindex triggers after file_saved (VSCode expects save after edit)" do + uri = "file:///test/lib/foo.ex" + editor = vscode_editor() + open_document(uri, "defmodule Foo do\nend") + expect_events_before_reindex(%{uri => file_saved(uri: uri)}, reindex: [uri]) + + simulate_did_change(editor, uri) + refute_reindex_triggered() + + simulate_did_save(editor, uri) + assert_reindex_triggered(reindex: [uri], delete: []) + end + end + + describe "race condition - didClose before didSave" do + test "rename progress is updated even when file is already closed" do + # This can happen during batch renames when didClose and didSave + # arrive nearly simultaneously and didClose is processed first + uri = "file:///test/lib/race.ex" + editor = vscode_editor() + open_document(uri, "defmodule Race do\nend") + expect_events_before_reindex(%{uri => file_saved(uri: uri)}, reindex: [uri]) + + # Simulate didClose being processed before didSave + :ok = Document.Store.close(uri) + + # didSave should still succeed and update rename progress + simulate_did_save(editor, uri) + assert_reindex_triggered(reindex: [uri], delete: []) + end + end + + describe "Neovim editor - non-file-rename (edits only)" do + test "reindex triggers after file_changed only (Neovim doesn't auto-save)" do + uri = "file:///test/lib/foo.ex" + editor = neovim_editor() + open_document(uri, "defmodule Foo do\nend") + expect_events_before_reindex(%{uri => file_changed(uri: uri)}, reindex: [uri]) + + simulate_did_change(editor, uri) + + assert_reindex_triggered(reindex: [uri], delete: []) + end + end + + describe "VSCode editor - file rename" do + test "reindex triggers after receiving file_changed for old + file_saved for new" do + old_uri = "file:///test/lib/old_module.ex" + new_uri = "file:///test/lib/new_module.ex" + editor = vscode_editor() + + open_document(old_uri, "defmodule OldModule do\nend") + open_document(new_uri, "defmodule NewModule do\nend") + + expect_events_before_reindex( + %{old_uri => file_changed(uri: old_uri), new_uri => file_saved(uri: new_uri)}, + reindex: [new_uri], + delete: [old_uri] + ) + + simulate_did_change(editor, old_uri) + refute_reindex_triggered() + + simulate_did_save(editor, new_uri) + assert_reindex_triggered(reindex: [new_uri], delete: [old_uri]) + end + end + + describe "Neovim editor - file rename" do + test "reindex triggers after file_saved for new file only" do + old_uri = "file:///test/lib/old_neovim.ex" + new_uri = "file:///test/lib/new_neovim.ex" + editor = neovim_editor() + + open_document(new_uri, "defmodule NewNeovim do\nend") + + expect_events_before_reindex( + %{new_uri => file_saved(uri: new_uri)}, + reindex: [new_uri], + delete: [old_uri] + ) + + simulate_did_save(editor, new_uri) + + assert_reindex_triggered(reindex: [new_uri], delete: [old_uri]) + end + end + + # ============================================================================ + # Test DSL - Editor simulation + # ============================================================================ + + defp vscode_editor do + build_editor("Visual Studio Code") + end + + defp neovim_editor do + build_editor("Neovim") + end + + defp build_editor(client_name) do + project = project() + config = Configuration.new(project: project, client_name: client_name) + state = %State{configuration: config, initialized?: true} + + patch(EngineApi, :broadcast, fn ^project, _msg -> :ok end) + patch(EngineApi, :compile_document, fn ^project, _doc -> :ok end) + patch(EngineApi, :schedule_compile, fn ^project, _ -> :ok end) + + patch(EngineApi, :maybe_update_rename_progress, fn ^project, msg -> + Rename.update_progress(msg) + end) + + %{state: state, version: 1} + end + + defp open_document(uri, content) do + :ok = Document.Store.open(uri, content, 1) + end + + defp simulate_did_change(editor, uri) do + new_version = editor.version + 1 + notification = build_did_change(uri, new_version) + {:ok, new_state} = State.apply(editor.state, notification) + %{editor | state: new_state, version: new_version} + end + + defp simulate_did_save(editor, uri) do + notification = build_did_save(uri) + {:ok, new_state} = State.apply(editor.state, notification) + %{editor | state: new_state} + end + + # ============================================================================ + # Test DSL - Expectations + # ============================================================================ + + defp expect_events_before_reindex(uri_to_expected, opts) do + paths_to_reindex = Keyword.get(opts, :reindex, []) + paths_to_delete = Keyword.get(opts, :delete, []) + test_pid = self() + + on_complete = fn -> + send(test_pid, {:rename_complete, paths_to_reindex, paths_to_delete}) + end + + {:ok, _} = + RenameSupervisor.start_renaming( + uri_to_expected, + paths_to_reindex, + paths_to_delete, + fn _delta, _msg -> :ok end, + on_complete + ) + end + + defp assert_reindex_triggered(opts) do + reindex = Keyword.fetch!(opts, :reindex) + delete = Keyword.fetch!(opts, :delete) + assert_receive {:rename_complete, ^reindex, ^delete} + end + + defp refute_reindex_triggered do + refute_receive {:rename_complete, _, _}, 50 + end + + # ============================================================================ + # LSP notification builders + # ============================================================================ + + defp build_did_change(uri, version) do + %GenLSP.Notifications.TextDocumentDidChange{ + params: %GenLSP.Structures.DidChangeTextDocumentParams{ + text_document: %GenLSP.Structures.VersionedTextDocumentIdentifier{ + uri: uri, + version: version + }, + content_changes: [%{text: "defmodule Renamed do\nend"}] + } + } + end + + defp build_did_save(uri) do + %GenLSP.Notifications.TextDocumentDidSave{ + params: %GenLSP.Structures.DidSaveTextDocumentParams{ + text_document: %GenLSP.Structures.TextDocumentIdentifier{uri: uri} + } + } + end +end diff --git a/apps/forge/lib/forge/document/changes.ex b/apps/forge/lib/forge/document/changes.ex index 2911dd80..d7fedf9c 100644 --- a/apps/forge/lib/forge/document/changes.ex +++ b/apps/forge/lib/forge/document/changes.ex @@ -9,23 +9,42 @@ defmodule Forge.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] + defstruct [:document, :edits, :rename_file] alias Forge.Document use Forge.StructAccess + defmodule RenameFile do + @moduledoc """ + Represents a file rename operation during module renaming. + """ + defstruct [:old_uri, :new_uri] + + @type t :: %__MODULE__{ + old_uri: Forge.uri(), + new_uri: Forge.uri() + } + + @spec new(Forge.uri(), Forge.uri()) :: t() + def new(old_uri, new_uri) do + %__MODULE__{old_uri: old_uri, new_uri: new_uri} + end + end + @type edits :: Document.Edit.t() | [Document.Edit.t()] + @type rename_file :: RenameFile.t() | nil @type t :: %__MODULE__{ document: Document.t(), - edits: edits + edits: edits, + rename_file: rename_file } @doc """ Creates a new Changes struct given a document and edits. """ - @spec new(Document.t(), edits()) :: t() - def new(document, edits) do - %__MODULE__{document: document, edits: edits} + @spec new(Document.t(), edits(), rename_file()) :: t() + def new(document, edits, rename_file \\ nil) do + %__MODULE__{document: document, edits: edits, rename_file: rename_file} end end diff --git a/apps/forge/lib/forge/engine_api/messages.ex b/apps/forge/lib/forge/engine_api/messages.ex index 1adfb96a..b90d302b 100644 --- a/apps/forge/lib/forge/engine_api/messages.ex +++ b/apps/forge/lib/forge/engine_api/messages.ex @@ -27,6 +27,8 @@ defmodule Forge.EngineApi.Messages do defrecord :file_deleted, project: nil, uri: nil + defrecord :file_saved, uri: nil + defrecord :module_updated, file: nil, name: nil, functions: [], macros: [], struct: nil defrecord :project_diagnostics, project: nil, build_number: 0, diagnostics: [] @@ -41,6 +43,10 @@ defmodule Forge.EngineApi.Messages do defrecord :project_reindexed, project: nil, elapsed_ms: 0, status: :success + defrecord :project_progress, label: nil, stage: :begin + + defrecord :percent_progress, label: nil, message: nil, delta: 0, max: 0, stage: :begin + @type compile_status :: :successful | :error @type name_and_arity :: {atom, non_neg_integer} @type field_list :: Keyword.t() | [atom] @@ -91,6 +97,8 @@ defmodule Forge.EngineApi.Messages do elapsed_ms: non_neg_integer ) + @type file_saved :: record(:file_saved, uri: Forge.uri()) + @type module_updated :: record(:module_updated, name: module(), @@ -125,4 +133,21 @@ defmodule Forge.EngineApi.Messages do elapsed_ms: non_neg_integer(), status: :success | {:error, term()} ) + + @type progress_stage :: :begin | :report | :complete + + @type project_progress :: + record(:project_progress, + label: String.t(), + stage: progress_stage() + ) + + @type percent_progress :: + record(:percent_progress, + label: String.t(), + message: String.t() | nil, + delta: non_neg_integer(), + max: non_neg_integer(), + stage: progress_stage() + ) end