[analysis_server] Handle requests for resolved units in handlers between initialization and analysis roots being set
If you open VS Code and have some Dart files open, it may send requests to the analysis server before we have (asynchronously) set up the analysis roots. This often results in "File not analyzed" errors in the console, but until now I had believed these errors to not cause any visible issues.
However, https://github.com/Dart-Code/Dart-Code/issues/5030 had a good repro for where this is visible - the Outline view would never populate for these open files.
We had some existing code to wait for in-progress context rebuilds to complete when fetching a resolved unit in a request handler, but requests could still get in before that started.
This change also waits for the client to send Initialized, which is when we start the analysis root build, so this bridges that gap and ensures the initial analysis roots are set before any handlers get to run.
Fixes https://github.com/Dart-Code/Dart-Code/issues/5030
Change-Id: I4b2d936f533f0696e3e363972faa82a2304276c0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356722
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index c0532fd..d61320b 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -210,6 +210,10 @@
/// Starts completed and will be replaced each time a context rebuild starts.
Completer<void> analysisContextRebuildCompleter = Completer()..complete();
+ /// A completer for tracking LSP client initialization
+ /// (see [lspClientInitialized]).
+ final Completer<void> _lspClientInitializedCompleter = Completer();
+
/// The workspace for rename refactorings.
late final refactoringWorkspace =
RefactoringWorkspace(driverMap.values, searchEngine);
@@ -367,6 +371,19 @@
/// by the client.
LspClientConfiguration get lspClientConfiguration;
+ /// A [Future] that completes once the client has initialized.
+ ///
+ /// For the LSP server, this happens when the client sends the `initialized`
+ /// notification. For LSP-over-Legacy this happens when the first LSP request
+ /// triggers initializetion.
+ ///
+ /// This future can be used by handlers requiring unit results to wait for
+ /// complete initialization even if the client sends the requests before
+ /// analysis roots have been initialized (for example because of async
+ /// requests to get configuration back from the client).
+ Future<void> get lspClientInitialized =>
+ _lspClientInitializedCompleter.future;
+
/// Returns the function that can send `openUri` request to the client.
/// Returns `null` is the client does not support it.
OpenUriNotificationSender? get openUriNotificationSender;
@@ -451,6 +468,14 @@
}
}
+ /// Completes [lspClientInitialized], signalling that LSP has finished
+ /// initializing.
+ void completeLspInitialization() {
+ if (!_lspClientInitializedCompleter.isCompleted) {
+ _lspClientInitializedCompleter.complete();
+ }
+ }
+
/// If the state location can be accessed, return the file byte store,
/// otherwise return the memory byte store.
ByteStore createByteStore(ResourceProvider resourceProvider) {
diff --git a/pkg/analysis_server/lib/src/handler/legacy/lsp_over_legacy_handler.dart b/pkg/analysis_server/lib/src/handler/legacy/lsp_over_legacy_handler.dart
index 620630b..e2a20a1 100644
--- a/pkg/analysis_server/lib/src/handler/legacy/lsp_over_legacy_handler.dart
+++ b/pkg/analysis_server/lib/src/handler/legacy/lsp_over_legacy_handler.dart
@@ -38,7 +38,7 @@
@override
Future<void> handle() async {
- server.sendLspNotifications = true;
+ server.initializeLsp();
final params = LspHandleParams.fromRequest(request);
final lspMessageJson = params.lspMessage;
final reporter = LspJsonReporter();
diff --git a/pkg/analysis_server/lib/src/legacy_analysis_server.dart b/pkg/analysis_server/lib/src/legacy_analysis_server.dart
index f22ed84..6e492d4 100644
--- a/pkg/analysis_server/lib/src/legacy_analysis_server.dart
+++ b/pkg/analysis_server/lib/src/legacy_analysis_server.dart
@@ -447,9 +447,9 @@
if (capabilities.supportsUris ?? false) {
// URI support implies LSP, as that's the only way to access (and get
// change notifications for) custom-scheme files.
- sendLspNotifications = true;
uriConverter = ClientUriConverter.withVirtualFileSupport(
resourceProvider.pathContext);
+ initializeLsp();
} else {
uriConverter = ClientUriConverter.noop(resourceProvider.pathContext);
}
@@ -632,6 +632,15 @@
}
}
+ /// Initializes LSP support for the legacy server.
+ ///
+ /// This method is called when the client sends an LSP request, or indicates
+ /// that it will use LSP-overy-Legacy via client capabilities.
+ void initializeLsp() {
+ sendLspNotifications = true;
+ completeLspInitialization();
+ }
+
/// Return `true` if the [path] is both absolute and normalized.
bool isAbsoluteAndNormalized(String path) {
var pathContext = resourceProvider.pathContext;
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_initialized.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_initialized.dart
index 5b94ce3..e4ff91e 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_initialized.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_initialized.dart
@@ -37,6 +37,10 @@
await server.updateWorkspaceFolders(openWorkspacePaths, const []);
}
+ // Mark initialization as done so that handlers that want to wait on this
+ // can continue.
+ server.completeLspInitialization();
+
return success(null);
}
}
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handlers.dart b/pkg/analysis_server/lib/src/lsp/handlers/handlers.dart
index 00c62f8..b94537b 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handlers.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handlers.dart
@@ -79,6 +79,11 @@
ClientUriConverter get uriConverter => server.uriConverter;
+ /// A [Future] that completes when both the client has finished initializing
+ /// and any in-progress context rebuilds are complete.
+ Future<void> get _initializedWithContexts =>
+ server.lspClientInitialized.then((_) => server.analysisContextsRebuilt);
+
ErrorOr<T> analysisFailedError<T>(String path) => error<T>(
ServerErrorCodes.FileAnalysisFailed, 'Analysis failed for file', path);
@@ -183,7 +188,7 @@
if (result == null) {
// Handle retry if allowed.
if (waitForInProgressContextRebuilds) {
- await server.analysisContextsRebuilt;
+ await _initializedWithContexts;
return requireResolvedLibrary(path,
waitForInProgressContextRebuilds: false);
}
@@ -211,7 +216,7 @@
if (result == null) {
// Handle retry if allowed.
if (waitForInProgressContextRebuilds) {
- await server.analysisContextsRebuilt;
+ await _initializedWithContexts;
return requireResolvedUnit(path,
waitForInProgressContextRebuilds: false);
}
@@ -237,7 +242,7 @@
if (result == null) {
// Handle retry if allowed.
if (waitForInProgressContextRebuilds) {
- await server.analysisContextsRebuilt;
+ await _initializedWithContexts;
return requireUnresolvedUnit(path,
waitForInProgressContextRebuilds: false);
}
diff --git a/pkg/analysis_server/test/lsp/initialization_test.dart b/pkg/analysis_server/test/lsp/initialization_test.dart
index 7958505..0a65890 100644
--- a/pkg/analysis_server/test/lsp/initialization_test.dart
+++ b/pkg/analysis_server/test/lsp/initialization_test.dart
@@ -820,6 +820,28 @@
expect(server.contextManager.excludedPaths, equals([excludedFolderPath]));
}
+ /// Tests that requests that requires a unit result are handled correctly even
+ /// if sent immediately after the `initialized` notification and do not result
+ /// in "File not Analyzed"-style errors because roots are set asynchronously.
+ Future<void> test_immediateRequests() async {
+ newFile(mainFilePath, 'void f() {}');
+ late Future<Either2<List<DocumentSymbol>, List<SymbolInformation>>> result;
+ await provideConfig(
+ () => initialize(
+ immediatelyAfterInitialized: () {
+ result = getDocumentSymbols(mainFileUri);
+ },
+ ),
+ {},
+ );
+
+ final symbols = (await result).map(
+ (docSymbols) => docSymbols,
+ (symbolInfos) => symbolInfos,
+ );
+ expect(symbols, hasLength(1));
+ }
+
Future<void> test_initialize() async {
final response = await initialize();
expect(response, isNotNull);
diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart
index a6385ba..fc88ad9 100644
--- a/pkg/analysis_server/test/lsp/server_abstract.dart
+++ b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -1054,6 +1054,7 @@
bool allowEmptyRootUri = false,
bool failTestOnAnyErrorNotification = true,
bool includeClientRequestTime = false,
+ void Function()? immediatelyAfterInitialized,
}) async {
this.includeClientRequestTime = includeClientRequestTime;
@@ -1118,7 +1119,10 @@
final notification =
makeNotification(Method.initialized, InitializedParams());
- await sendNotificationToServer(notification);
+
+ final initializedNotification = sendNotificationToServer(notification);
+ immediatelyAfterInitialized?.call();
+ await initializedNotification;
await pumpEventQueue();
} else if (throwOnFailure) {
throw 'Error during initialize request: '
