Skip to content

Commit fb158ba

Browse files
committed
Recover the LSP analysis context after a workspace build
Why: Projects opened before their first build lack generated metadata. Recover on compiler-log events without requiring an editor restart. Context modified (compiler-log creation and change notifications): Before (text): Refresh diagnostics using the existing analysis and package context. After (text): Discover fresh analysis tables and package configuration. Replace the context if the workspace package is available. Otherwise retain the previous context without repeated error messages. Preserve open buffers, settings and diagnostics, then refresh diagnostics. Diagnostic modified (initial package discovery failure): Before (text): Try building the project then restart the server After (text): Try building the project. The server will retry when a compiler log changes. Add snapshot tests for Created and Changed events, early retries, compiled-file lookup and preservation of unsaved buffers. Document the automatic recovery behavior. Validation: dune build and eight recovery/diagnostic tests pass. The full LSP suite still has snapshot differences in hover_jsx.ml and code_action.ml. Signed-off-by: Pedro Castro <aspeddro@gmail.com>
1 parent 58789fc commit fb158ba

3 files changed

Lines changed: 167 additions & 31 deletions

File tree

‎lsp/lsp.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ notifications. Some are out of scope because they don't make sense.
114114
generated with `build_root` field for each subpackage.
115115
- [x] Add the warning number to the diagnostic message.
116116
- [x] `workspace/didChangeWatchedFiles` - client notification
117+
- When a `.compiler.log` is created or changed, the server rediscovers packages
118+
and refreshes its analysis context and compiler configuration. A workspace
119+
opened before its first build can recover without restarting the server.
120+
Failed discovery leaves the previous context intact; open buffers, settings
121+
and diagnostic ownership are preserved.
117122
- The server uses this notification to detect changes to generated
118123
`.compiler.log` files, which usually means a ReScript build has finished.
119124
When a watched log changes, the server re-reads every known compiler log and

‎lsp/src/rescript_language_server.ml‎

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -128,30 +128,22 @@ let publish_diagnostics ?(force_publish_uris = []) (state : State.t) diagnostics
128128
function unsafe in the usual functional sense: callers must provide a fresh
129129
per-server analysis state or accept that repeated calls append/overwrite
130130
package roots as a side effect. *)
131-
let discover_subpackages_and_populate ~workspace_root
131+
let discover_subpackages_and_populate ~notify_errors ~workspace_root
132132
~(analysis_state : Analysis.Shared_types.state) ~server =
133133
let root_package =
134134
let root_path = workspace_root |> Uri.to_path in
135135
match Analysis.Packages.new_bs_package ~root_path with
136136
| Some p -> Some p
137137
| None ->
138-
(* TODO: Decide how the server should recover when the workspace has not
139-
been built yet, for example right after cloning a repository. Package
140-
discovery depends on generated build metadata, so most analysis
141-
features cannot work until that metadata exists.
142-
Options to evaluate:
143-
- Keep the server running and show a window message. In this case user should restart the server after build.
144-
- Stop the server after explaining that a build is required.
145-
- Re-initialize `analysis_state` after DidChangeWatchedFiles reports a
146-
new `.compiler.log` change.
147-
*)
148138
let message =
149139
Printf.sprintf
150140
"Failed to initialize the context for the project at %s. Try \
151-
building the project then restart the server"
141+
building the project. The server will retry when a compiler log \
142+
changes."
152143
root_path
153144
in
154-
Server.show_message_notification ~kind:MessageType.Error message server;
145+
if notify_errors then
146+
Server.show_message_notification ~kind:MessageType.Error message server;
155147
None
156148
in
157149

@@ -171,7 +163,7 @@ let discover_subpackages_and_populate ~workspace_root
171163
| exception Failure err -> Either.Right err)
172164
dependencies
173165
in
174-
if List.length deps_errors > 0 then
166+
if notify_errors && List.length deps_errors > 0 then
175167
Server.show_message_notification ~kind:MessageType.Error
176168
(Printf.sprintf "Failed to resolve dependencies. %s"
177169
(String.concat ", " deps_errors))
@@ -205,27 +197,18 @@ let discover_subpackages_and_populate ~workspace_root
205197
| None -> ())
206198
|> ignore
207199

208-
let on_initialize (params : InitializeParams.t) (server : State.t Server.t) =
209-
let state = Server.state server in
210-
211-
let diagnostics =
212-
Diagnostics.make ~diagnostics:(Diagnostics.empty ())
213-
~send:(fun publish_diagnostics_params ->
214-
Server.notification
215-
(Server_notification.PublishDiagnostics publish_diagnostics_params)
216-
server)
217-
in
218-
200+
let discover_project_context ~notify_errors ~workspace_root
201+
(server : State.t Server.t) =
219202
let analysis_state = Analysis.Shared_types.create_state () in
220-
221-
let workspace_root = Helpers.workspace_root_uri_of_initialize_params params in
222-
223-
discover_subpackages_and_populate ~workspace_root ~analysis_state ~server;
203+
discover_subpackages_and_populate ~notify_errors ~workspace_root
204+
~analysis_state ~server;
224205

225206
let compiler_config =
226207
analysis_state.packages_by_root |> Hashtbl.to_seq
227208
|> Seq.filter_map (fun (root_path, _) ->
228-
match Compiler_config.parse ~root:root_path ~fs:state.fs with
209+
match
210+
Compiler_config.parse ~root:root_path ~fs:(Server.state server).fs
211+
with
229212
| Ok config -> Some (Uri.of_path root_path, config)
230213
| Error _ ->
231214
(* NOTE: Surface malformed package configs to the client. We ignore
@@ -234,6 +217,21 @@ let on_initialize (params : InitializeParams.t) (server : State.t Server.t) =
234217
None)
235218
|> Compiler_config.Uri_map.of_seq
236219
in
220+
(analysis_state, compiler_config)
221+
222+
let on_initialize (params : InitializeParams.t) (server : State.t Server.t) =
223+
let state = Server.state server in
224+
let diagnostics =
225+
Diagnostics.make ~diagnostics:(Diagnostics.empty ())
226+
~send:(fun publish_diagnostics_params ->
227+
Server.notification
228+
(Server_notification.PublishDiagnostics publish_diagnostics_params)
229+
server)
230+
in
231+
let workspace_root = Helpers.workspace_root_uri_of_initialize_params params in
232+
let analysis_state, compiler_config =
233+
discover_project_context ~notify_errors:true ~workspace_root server
234+
in
237235

238236
let state =
239237
State.initialize state ~params ~diagnostics ~analysis_state ~compiler_config
@@ -626,7 +624,34 @@ let on_notification notification (server : State.t Server.t) =
626624
Server.request (Server_request.ClientRegisterCapability params) server;
627625

628626
state
629-
| DidChangeWatchedFiles _ -> (
627+
| DidChangeWatchedFiles {changes} -> (
628+
let state =
629+
if
630+
List.exists
631+
(fun (event : FileEvent.t) ->
632+
Filename.basename (Uri.to_path event.uri) = Constants.compiler_log
633+
&& (event.type_ = FileChangeType.Created
634+
|| event.type_ = FileChangeType.Changed))
635+
changes
636+
then
637+
let workspace_root = State.workspace_root state in
638+
let analysis_state, compiler_config =
639+
discover_project_context ~notify_errors:false ~workspace_root server
640+
in
641+
(* A workspace opened before its first build has no package metadata.
642+
Rediscover into fresh tables so failed lookups and old CMT caches
643+
cannot survive the build. Keep the previous context if metadata is
644+
still unavailable, and preserve buffers, settings and diagnostics. *)
645+
if
646+
Hashtbl.mem analysis_state.packages_by_root
647+
(Uri.to_path workspace_root)
648+
then
649+
State.initialize state ~params:(State.params state)
650+
~diagnostics:(State.diagnostics state) ~analysis_state
651+
~compiler_config
652+
else state
653+
else state
654+
in
630655
(* Do not limit diagnostics to the path reported by
631656
DidChangeWatchedFilesParams. In monorepos, a build in one subpackage
632657
can change diagnostics that should be shown for files in another
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
open Lsp
2+
open Types
3+
4+
let notify_log ~root ~type_ client =
5+
Test.Client.send_notification client
6+
(Client_notification.DidChangeWatchedFiles
7+
(DidChangeWatchedFilesParams.create
8+
~changes:
9+
[
10+
FileEvent.create ~type_
11+
~uri:(Uri.of_path (Filename.concat root "lib/bs/.compiler.log"));
12+
]))
13+
14+
let test_recovery type_ =
15+
Test.with_project ~name:"first-build" ~run_build:false
16+
~text:{|let value = 1
17+
|} (fun ~root ~doc client ->
18+
let hover =
19+
Client_request.TextDocumentHover
20+
(HoverParams.create ~textDocument:(Test.text_document doc)
21+
~position:(Test.position (0, 5))
22+
())
23+
in
24+
Test.snapshot ~root client hover;
25+
ignore (Test.Client.take_notifications client);
26+
(* An early log event can arrive before build metadata exists. It must
27+
leave the session usable and must not repeat initialization errors. *)
28+
notify_log ~root ~type_ client;
29+
Test.synchronize client;
30+
let messages =
31+
Test.Client.take_notifications client
32+
|> List.filter (fun (n : Jsonrpc.Notification.t) ->
33+
n.method_ = "window/showMessage")
34+
in
35+
Printf.printf "repeated initialization messages: %d\n"
36+
(List.length messages);
37+
Test.run_rescript_build ~root;
38+
notify_log ~root ~type_ client;
39+
Test.snapshot ~root client hover;
40+
(* This request uses compiler_config, not only the analysis package cache. *)
41+
Test.snapshot ~root client
42+
(Client_request.UnknownRequest
43+
{
44+
meth = "textDocument/openCompiled";
45+
params =
46+
Some
47+
(Jsonrpc.Structured.t_of_yojson
48+
(TextDocumentIdentifier.yojson_of_t (Test.text_document doc)));
49+
});
50+
Test.change_document client doc ~version:1 ~text:{|let unsaved = 2
51+
|};
52+
notify_log ~root ~type_:FileChangeType.Changed client;
53+
let symbols =
54+
Client_request.DocumentSymbol
55+
(DocumentSymbolParams.create ~textDocument:(Test.text_document doc) ())
56+
in
57+
(* The unsaved buffer survives rediscovery; disk still has a valid value. *)
58+
Test.snapshot ~root client symbols)
59+
60+
let%expect_test "recovers after the first compiler log is created" =
61+
test_recovery FileChangeType.Created;
62+
[%expect
63+
{|
64+
null
65+
repeated initialization messages: 0
66+
{ "contents": { "kind": "markdown", "value": "```rescript\nint\n```" } }
67+
{ "uri": "file:///workspace/lib/es6/./Test.res.js" }
68+
[
69+
{
70+
"kind": 16,
71+
"name": "unsaved",
72+
"range": {
73+
"end": { "character": 15, "line": 0 },
74+
"start": { "character": 0, "line": 0 }
75+
},
76+
"selectionRange": {
77+
"end": { "character": 15, "line": 0 },
78+
"start": { "character": 0, "line": 0 }
79+
}
80+
}
81+
]
82+
|}]
83+
84+
let%expect_test "recovers when the client reports the first build as a change" =
85+
test_recovery FileChangeType.Changed;
86+
[%expect
87+
{|
88+
null
89+
repeated initialization messages: 0
90+
{ "contents": { "kind": "markdown", "value": "```rescript\nint\n```" } }
91+
{ "uri": "file:///workspace/lib/es6/./Test.res.js" }
92+
[
93+
{
94+
"kind": 16,
95+
"name": "unsaved",
96+
"range": {
97+
"end": { "character": 15, "line": 0 },
98+
"start": { "character": 0, "line": 0 }
99+
},
100+
"selectionRange": {
101+
"end": { "character": 15, "line": 0 },
102+
"start": { "character": 0, "line": 0 }
103+
}
104+
}
105+
]
106+
|}]

0 commit comments

Comments
 (0)