[analysis_server] Avoid using stale LineInfos immediately after overlay updates

This fixes a bug where a semantic tokens request could use an incorrect LineInfo for mappings if sent immediately after an overlay update because:

a) we computed a LineInfo from the source instead of using one consistent with the resolved unit
b) we read the file state in a way that did not take into account the latest overlay updates if the analysis driver hadn't handled the changed file

The second issue could affect other requests too because server.getLineInfo() is used in a number of places (to support plugins). This change reverts reading the file state directly, but also updates semantic tokens to prefer using the LineInfo from the resolved unit when we have one (only falling back to computing one from current sources for non-Dart files, which can be handled by plugins).

Fixes https://github.com/dart-lang/sdk/issues/55084

Change-Id: I77ab6ad63a78ebd062b264d901a9ae8568b9096e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370021
Reviewed-by: Sam Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/analysis_server.dart

@@ -618,21 +618,22 @@ abstract class AnalysisServer {
   /// This method supports non-Dart files but uses the current content of the
   /// file which may not be the latest analyzed version of the file if it was
   /// recently modified, so using the LineInfo from an analyzed result may be
-  /// preferable.
+  /// preferable. This method exists mainly to support plugins which do not
+  /// provide us a matching [LineInfo] for the content they used.
   LineInfo? getLineInfo(String path) {
     try {
-      // First try to get from the File if it's an analyzed Dart file.
+      // First try from the overlay because it may be more up-to-date than
+      // the file state.
+      var content = resourceProvider.getFile(path).readAsStringSync();
+      return LineInfo.fromContent(content);
+    } on FileSystemException {
+      // If the file does not exist or cannot be read, try the file state
+      // because this could be something like a macro-generated file.
       var result = getAnalysisDriver(path)?.getFileSync(path);
       if (result is FileResult) {
         return result.lineInfo;
       }
 
-      // Fall back to reading from the resource provider.
-      var content = resourceProvider.getFile(path).readAsStringSync();
-      return LineInfo.fromContent(content);
-    } on FileSystemException {
-      // If the file does not exist or cannot be read, return null to allow
-      // the caller to decide how to handle this.
       return null;
     }
   }

pkg/analysis_server/lib/src/lsp/handlers/handler_definition.dart

@@ -51,19 +51,16 @@ class DefinitionHandler extends LspMessageHandler<TextDocumentPositionParams,
         .toList();
   }
 
-  Future<AnalysisNavigationParams> getServerResult(
-      bool supportsLocationLink, String path, int offset) async {
+  Future<AnalysisNavigationParams> getServerResult(ResolvedUnitResult result,
+      String path, bool supportsLocationLink, int offset) async {
     var collector = NavigationCollectorImpl();
 
-    var result = await server.getResolvedUnit(path);
-    if (result != null) {
-      computeDartNavigation(
-          server.resourceProvider, collector, result, offset, 0);
-      if (supportsLocationLink) {
-        await _updateTargetsWithCodeLocations(collector);
-      }
-      collector.createRegions();
+    computeDartNavigation(
+        server.resourceProvider, collector, result, offset, 0);
+    if (supportsLocationLink) {
+      await _updateTargetsWithCodeLocations(collector);
     }
+    collector.createRegions();
 
     return AnalysisNavigationParams(
         path, collector.regions, collector.targets, collector.files);
@@ -86,7 +83,10 @@ class DefinitionHandler extends LspMessageHandler<TextDocumentPositionParams,
     var path = pathOfDoc(params.textDocument);
 
     return path.mapResult((path) async {
-      var lineInfo = server.getLineInfo(path);
+      // Always prefer a LineInfo from a resolved unit than server.getLineInfo.
+      var resolvedUnit = (await requireResolvedUnit(path)).resultOrNull;
+      var lineInfo = resolvedUnit?.lineInfo ?? server.getLineInfo(path);
+
       // If there is no lineInfo, the request cannot be translated from LSP line/col
       // to server offset/length.
       if (lineInfo == null) {
@@ -97,7 +97,9 @@ class DefinitionHandler extends LspMessageHandler<TextDocumentPositionParams,
 
       return offset.mapResult((offset) async {
         var allResults = [
-          await getServerResult(supportsLocationLink, path, offset),
+          if (resolvedUnit != null)
+            await getServerResult(
+                resolvedUnit, path, supportsLocationLink, offset),
           ...await getPluginResults(path, offset),
         ];
 

pkg/analysis_server/lib/src/lsp/handlers/handler_semantic_tokens.dart

@@ -13,6 +13,7 @@ import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/lsp/registration/feature_registration.dart';
 import 'package:analysis_server/src/lsp/semantic_tokens/encoder.dart';
 import 'package:analysis_server/src/lsp/semantic_tokens/legend.dart';
+import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/source/source_range.dart';
 import 'package:analyzer/src/utilities/extensions/collection.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
@@ -31,15 +32,9 @@ abstract class AbstractSemanticTokensHandler<T>
   }
 
   Future<List<SemanticTokenInfo>> getServerResult(
-      String path, SourceRange? range) async {
-    var result = await requireResolvedUnit(path);
-    return result.map(
-      (_) => [], // Error, return nothing.
-      (unit) {
-        var computer = DartUnitHighlightsComputer(unit.unit, range: range);
-        return computer.computeSemanticTokens();
-      },
-    );
+      CompilationUnit unit, SourceRange? range) async {
+    var computer = DartUnitHighlightsComputer(unit, range: range);
+    return computer.computeSemanticTokens();
   }
 
   Iterable<SemanticTokenInfo> _filter(
@@ -59,7 +54,10 @@ abstract class AbstractSemanticTokensHandler<T>
     var path = pathOfDoc(textDocument);
 
     return path.mapResult((path) async {
-      var lineInfo = server.getLineInfo(path);
+      // Always prefer a LineInfo from a resolved unit than server.getLineInfo.
+      var resolvedUnit = (await requireResolvedUnit(path)).resultOrNull;
+      var lineInfo = resolvedUnit?.lineInfo ?? server.getLineInfo(path);
+
       // If there is no lineInfo, the request cannot be translated from LSP
       // line/col to server offset/length.
       if (lineInfo == null) {
@@ -67,7 +65,9 @@ abstract class AbstractSemanticTokensHandler<T>
       }
 
       return toSourceRangeNullable(lineInfo, range).mapResult((range) async {
-        var serverTokens = await getServerResult(path, range);
+        var serverTokens = resolvedUnit != null
+            ? await getServerResult(resolvedUnit.unit, range)
+            : <SemanticTokenInfo>[];
         var pluginHighlightRegions = getPluginResults(path).flattenedToList2;
 
         if (token.isCancellationRequested) {

pkg/analysis_server/test/lsp/semantic_tokens_test.dart

@@ -802,6 +802,36 @@ extension type E(int i) {}
     expect(decoded, equals(expected));
   }
 
+  /// Verify that sending a semantic token request immediately after an overlay
+  /// update (with no delay) does not result in corrupt semantic tokens because
+  /// the previous file content was used.
+  ///
+  /// https://github.com/dart-lang/sdk/issues/55084
+  Future<void> test_immediatelyAfterUpdate() async {
+    const initialContent = 'class A {}\nclass B {}';
+    const updatedContent = 'class Aaaaa {}\nclass Bbbbb {}';
+
+    newFile(mainFilePath, initialContent);
+    await initialize();
+
+    await openFile(mainFileUri, initialContent);
+
+    // Send an edit (don't await), then fetch the tokens and verify the results
+    // were correct for the final content. If the bug occurs, the strings won't
+    // match up because the offsets will have been mapped incorrectly.
+    unawaited(replaceFile(2, mainFileUri, updatedContent));
+    var tokens = await getSemanticTokens(mainFileUri);
+    var decoded = _decodeSemanticTokens(updatedContent, tokens);
+    expect(decoded, [
+      _Token('class', SemanticTokenTypes.keyword),
+      _Token('Aaaaa', SemanticTokenTypes.class_,
+          [SemanticTokenModifiers.declaration]),
+      _Token('class', SemanticTokenTypes.keyword),
+      _Token('Bbbbb', SemanticTokenTypes.class_,
+          [SemanticTokenModifiers.declaration])
+    ]);
+  }
+
   Future<void> test_invalidSyntax() async {
     failTestOnErrorDiagnostic = false;