Skip to content

Commit 10afb29

Browse files
committed
fix(rename): add progress tracking to trigger reindex after rename
Without reindexing after a rename operation, the search index still contains old module names. This causes subsequent renames to fail - e.g., renaming `Dummy.Accounts` to `Dummy.Accounts2` works, but renaming back to `Dummy.Accounts` fails to update `Dummy.Accounts2.User` because the index doesn't know about the new module names. Add Commands.Rename GenServer to track file_changed/file_saved events and trigger reindexing once all rename operations complete.
1 parent a38652e commit 10afb29

File tree

10 files changed

+584
-4
lines changed

10 files changed

+584
-4
lines changed

apps/engine/lib/engine.ex

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ defmodule Engine do
6767

6868
defdelegate rename(analysis, position, new_name, client_name), to: Engine.CodeMod.Rename
6969

70+
defdelegate maybe_update_rename_progress(triggered_message),
71+
to: Engine.Commands.Rename,
72+
as: :update_progress
73+
7074
def list_apps do
7175
for {app, _, _} <- :application.loaded_applications(),
7276
not Forge.Namespace.Module.prefixed?(app),

apps/engine/lib/engine/application.ex

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ defmodule Engine.Application do
1010
[
1111
Engine.Api.Proxy,
1212
Engine.Commands.Reindex,
13+
Engine.Commands.RenameSupervisor,
1314
Engine.Module.Loader,
1415
Engine.Dispatch,
1516
Engine.ModuleMappings,

apps/engine/lib/engine/code_mod/rename.ex

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,15 @@ defmodule Engine.CodeMod.Rename do
77
rename execution.
88
"""
99
alias Engine.CodeMod.Rename
10+
alias Engine.Commands
11+
alias Engine.Progress
1012
alias Forge.Ast.Analysis
1113
alias Forge.Document
1214
alias Forge.Document.Position
1315
alias Forge.Document.Range
1416

17+
import Forge.EngineApi.Messages
18+
1519
@doc """
1620
Prepares a rename operation at the given position.
1721
@@ -31,16 +35,94 @@ defmodule Engine.CodeMod.Rename do
3135
Renames the entity at the given position to `new_name`, returning a list
3236
of document changes that should be applied.
3337
34-
The `client_name` parameter is currently unused but reserved for future
35-
client-specific behavior (e.g., different progress tracking for VSCode vs Neovim).
38+
The `client_name` parameter is used to determine client-specific behavior
39+
for progress tracking (e.g., VSCode sends different events than Neovim).
3640
"""
3741
@spec rename(Analysis.t(), Position.t(), String.t(), String.t() | nil) ::
3842
{:ok, [Document.Changes.t()]} | {:error, term()}
39-
def rename(%Analysis{} = analysis, %Position{} = position, new_name, _client_name) do
43+
def rename(%Analysis{} = analysis, %Position{} = position, new_name, client_name) do
4044
with {:ok, {renamable, entity}, range} <- Rename.Prepare.resolve(analysis, position) do
4145
rename_module = Map.fetch!(@rename_mappings, renamable)
4246
results = rename_module.rename(range, new_name, entity)
47+
set_rename_progress(results, client_name)
4348
{:ok, results}
4449
end
4550
end
51+
52+
defp set_rename_progress(document_changes_list, client_name) do
53+
# Progress tracking is optional - if the infrastructure isn't running
54+
# (e.g., in tests), we just skip it
55+
try do
56+
do_set_rename_progress(document_changes_list, client_name)
57+
rescue
58+
_ -> :ok
59+
catch
60+
:exit, _ -> :ok
61+
end
62+
end
63+
64+
defp do_set_rename_progress(document_changes_list, client_name) do
65+
uri_to_expected_operation =
66+
uri_to_expected_operation(client_name, document_changes_list)
67+
68+
{paths_to_delete, paths_to_reindex} =
69+
for %Document.Changes{rename_file: rename_file, document: document} <- document_changes_list do
70+
if rename_file do
71+
{rename_file.old_uri, rename_file.new_uri}
72+
else
73+
{nil, document.uri}
74+
end
75+
end
76+
|> Enum.unzip()
77+
78+
paths_to_delete = Enum.reject(paths_to_delete, &is_nil/1)
79+
renaming_operation_count = Enum.count(uri_to_expected_operation)
80+
81+
total_operation_count =
82+
renaming_operation_count + length(paths_to_delete) + length(paths_to_reindex)
83+
84+
{on_update_progress, on_complete} =
85+
Progress.begin_percent("Renaming", total_operation_count)
86+
87+
Commands.RenameSupervisor.start_renaming(
88+
uri_to_expected_operation,
89+
paths_to_reindex,
90+
paths_to_delete,
91+
on_update_progress,
92+
on_complete
93+
)
94+
end
95+
96+
# VSCode sends both file_changed and file_saved events
97+
defp uri_to_expected_operation(client_name, document_changes_list)
98+
when client_name in ["Visual Studio Code"] do
99+
document_changes_list
100+
|> Enum.flat_map(fn %Document.Changes{document: document, rename_file: rename_file} ->
101+
if rename_file do
102+
# when the file is renamed, we won't receive `DidSave` for the old file
103+
[
104+
{rename_file.old_uri, file_changed(uri: rename_file.old_uri)},
105+
{rename_file.new_uri, file_saved(uri: rename_file.new_uri)}
106+
]
107+
else
108+
[{document.uri, file_saved(uri: document.uri)}]
109+
end
110+
end)
111+
|> Map.new()
112+
end
113+
114+
# Other editors (like Neovim) may only send file_changed events
115+
defp uri_to_expected_operation(_, document_changes_list) do
116+
document_changes_list
117+
|> Enum.flat_map(fn %Document.Changes{document: document, rename_file: rename_file} ->
118+
if rename_file do
119+
[{rename_file.new_uri, file_saved(uri: rename_file.new_uri)}]
120+
else
121+
# Some editors do not directly save the file after renaming, such as *neovim*.
122+
# when the file is not renamed, we'll only received `DidChange` for the old file
123+
[{document.uri, file_changed(uri: document.uri)}]
124+
end
125+
end)
126+
|> Map.new()
127+
end
46128
end
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
defmodule Engine.Commands.Rename do
2+
@moduledoc """
3+
Tracks rename progress and triggers reindexing after rename operations complete.
4+
5+
This GenServer is started when a rename operation begins. It tracks expected file
6+
operations (file_changed, file_saved) and when all operations are received, it
7+
reindexes the modified files and clears old entries from the search index.
8+
9+
This is necessary because after a rename, the search index still contains the old
10+
module names. Without reindexing, subsequent renames won't find the new entries.
11+
"""
12+
13+
alias Engine.Commands.Reindex
14+
alias Engine.Search.Store
15+
alias Forge.EngineApi.Messages
16+
17+
require Logger
18+
import Messages
19+
20+
use GenServer
21+
22+
defmodule State do
23+
@moduledoc false
24+
25+
@type uri_to_expected_operation :: %{
26+
Forge.uri() => Messages.file_changed() | Messages.file_saved()
27+
}
28+
29+
@type t :: %__MODULE__{
30+
uri_to_expected_operation: uri_to_expected_operation(),
31+
paths_to_reindex: list(Forge.uri()),
32+
paths_to_delete: list(Forge.uri()),
33+
on_update_progress: (integer(), String.t() -> :ok),
34+
on_complete: (-> :ok)
35+
}
36+
37+
defstruct uri_to_expected_operation: %{},
38+
paths_to_reindex: [],
39+
paths_to_delete: [],
40+
on_update_progress: nil,
41+
on_complete: nil
42+
43+
def new(
44+
uri_to_expected_operation,
45+
paths_to_reindex,
46+
paths_to_delete,
47+
on_update_progress,
48+
on_complete
49+
) do
50+
%__MODULE__{
51+
uri_to_expected_operation: uri_to_expected_operation,
52+
paths_to_reindex: paths_to_reindex,
53+
paths_to_delete: paths_to_delete,
54+
on_update_progress: on_update_progress,
55+
on_complete: on_complete
56+
}
57+
end
58+
59+
def update_progress(%__MODULE__{} = state, file_changed(uri: uri)) do
60+
update_progress(state, uri, file_changed(uri: uri))
61+
end
62+
63+
def update_progress(%__MODULE__{} = state, file_saved(uri: uri)) do
64+
update_progress(state, uri, file_saved(uri: uri))
65+
end
66+
67+
defp update_progress(%__MODULE__{} = state, uri, message) do
68+
new_uri_with_expected_operation =
69+
maybe_pop_expected_operation(
70+
state.uri_to_expected_operation,
71+
uri,
72+
message,
73+
state.on_update_progress
74+
)
75+
76+
if Enum.empty?(new_uri_with_expected_operation) do
77+
reindex_all_modified_files(state)
78+
state.on_complete.()
79+
end
80+
81+
%__MODULE__{state | uri_to_expected_operation: new_uri_with_expected_operation}
82+
end
83+
84+
def in_progress?(%__MODULE__{} = state) do
85+
state.uri_to_expected_operation != %{}
86+
end
87+
88+
defp maybe_pop_expected_operation(uri_to_operation, uri, message, on_update_progress) do
89+
case uri_to_operation do
90+
%{^uri => ^message} ->
91+
on_update_progress.(1, "")
92+
Map.delete(uri_to_operation, uri)
93+
94+
_ ->
95+
uri_to_operation
96+
end
97+
end
98+
99+
defp reindex_all_modified_files(%__MODULE__{} = state) do
100+
Enum.each(state.paths_to_reindex, fn uri ->
101+
Reindex.uri(uri)
102+
state.on_update_progress.(1, "reindexing")
103+
end)
104+
105+
Enum.each(state.paths_to_delete, fn uri ->
106+
Store.clear(uri)
107+
state.on_update_progress.(1, "deleting old index")
108+
end)
109+
end
110+
end
111+
112+
@spec child_spec(
113+
%{Forge.uri() => Messages.file_changed() | Messages.file_saved()},
114+
list(Forge.uri()),
115+
list(Forge.uri()),
116+
(integer(), String.t() -> :ok),
117+
(-> :ok)
118+
) :: Supervisor.child_spec()
119+
def child_spec(
120+
uri_to_expected_operation,
121+
paths_to_reindex,
122+
paths_to_delete,
123+
on_update_progress,
124+
on_complete
125+
) do
126+
%{
127+
id: __MODULE__,
128+
start:
129+
{__MODULE__, :start_link,
130+
[
131+
uri_to_expected_operation,
132+
paths_to_reindex,
133+
paths_to_delete,
134+
on_update_progress,
135+
on_complete
136+
]},
137+
restart: :transient
138+
}
139+
end
140+
141+
def start_link(
142+
uri_to_expected_operation,
143+
paths_to_reindex,
144+
paths_to_delete,
145+
on_update_progress,
146+
on_complete
147+
) do
148+
state =
149+
State.new(
150+
uri_to_expected_operation,
151+
paths_to_reindex,
152+
paths_to_delete,
153+
on_update_progress,
154+
on_complete
155+
)
156+
157+
GenServer.start_link(__MODULE__, state, name: __MODULE__)
158+
end
159+
160+
@impl true
161+
def init(state) do
162+
{:ok, state, {:continue, :start_buffering}}
163+
end
164+
165+
@doc """
166+
Updates the rename progress with a file operation message.
167+
168+
This should be called when files are changed or saved during rename.
169+
Returns `:ok` if the message was processed, `{:error, :not_in_rename_progress}`
170+
if no rename is in progress.
171+
"""
172+
@spec update_progress(Messages.file_changed() | Messages.file_saved()) ::
173+
:ok | {:error, :not_in_rename_progress}
174+
def update_progress(message) do
175+
pid = Process.whereis(__MODULE__)
176+
177+
if pid && Process.alive?(pid) do
178+
GenServer.cast(__MODULE__, {:update_progress, message})
179+
else
180+
{:error, :not_in_rename_progress}
181+
end
182+
end
183+
184+
@impl true
185+
def handle_continue(:start_buffering, state) do
186+
Engine.Api.Proxy.start_buffering()
187+
{:noreply, state}
188+
end
189+
190+
@impl true
191+
def handle_call(:in_progress?, _from, state) do
192+
{:reply, State.in_progress?(state), state}
193+
end
194+
195+
@impl true
196+
def handle_cast({:update_progress, message}, state) do
197+
new_state = State.update_progress(state, message)
198+
199+
if State.in_progress?(new_state) do
200+
{:noreply, new_state}
201+
else
202+
Logger.info("Rename process completed.")
203+
{:stop, :normal, new_state}
204+
end
205+
end
206+
end
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
defmodule Engine.Commands.RenameSupervisor do
2+
@moduledoc """
3+
DynamicSupervisor for managing rename progress tracking GenServers.
4+
5+
Each rename operation spawns a transient child that tracks progress
6+
and shuts down when the rename is complete.
7+
"""
8+
9+
alias Engine.Commands.Rename
10+
11+
use DynamicSupervisor
12+
13+
def child_spec(_) do
14+
%{
15+
id: __MODULE__,
16+
start: {__MODULE__, :start_link, []}
17+
}
18+
end
19+
20+
def start_link do
21+
DynamicSupervisor.start_link(__MODULE__, [], name: __MODULE__)
22+
end
23+
24+
@doc """
25+
Starts a new rename progress tracker.
26+
27+
## Parameters
28+
29+
- `uri_to_expected_operation` - Map of URIs to expected messages (file_changed/file_saved)
30+
- `paths_to_reindex` - List of URIs that need to be reindexed after rename
31+
- `paths_to_delete` - List of URIs whose entries should be deleted from the index
32+
- `on_update_progress` - Callback function receiving (increment, message)
33+
- `on_complete` - Callback function called when rename is complete
34+
"""
35+
def start_renaming(
36+
uri_to_expected_operation,
37+
paths_to_reindex,
38+
paths_to_delete,
39+
on_update_progress,
40+
on_complete
41+
) do
42+
DynamicSupervisor.start_child(
43+
__MODULE__,
44+
Rename.child_spec(
45+
uri_to_expected_operation,
46+
paths_to_reindex,
47+
paths_to_delete,
48+
on_update_progress,
49+
on_complete
50+
)
51+
)
52+
end
53+
54+
@impl true
55+
def init(_init_arg) do
56+
DynamicSupervisor.init(strategy: :one_for_one)
57+
end
58+
end

0 commit comments

Comments
 (0)