Usage highlighting broken in VSCode

[stereotype441]

I recently re-built my SDK, and I'm no longer seeing usages highlighted when clicking on an identifier in VSCode.

I've bisected the issue and it appears to have started with 6f992ae.

CC @bwilkerson @scheglov

[scheglov]

@DanTup What LSP command / handler does VS Code use to highlight usages?

@stereotype441 is it broken for all kinds of elements, e.g. local variables, classes, getters, etc?

[DanTup]

@DanTup What LSP command / handler does VS Code use to highlight usages?

LSP calls these "Document Highlights" and the implementation uses OccurrencesCollectorImpl (which should match the legacy implementation).

In a quick test of class names (running from source as of around 4 hours ago) I do still see them showing up:

[stereotype441]

Hmm, I thought it was broken for everything, but now I can't reproduce the problem at all. I'm not sure what happened.

I will keep an eye out, and if it happens again, I'll post more exact repro instructions.

[stereotype441]

Ok, I have more information. The problem seems to happen only in certain files. For example, in my checkout of the Dart SDK, document highlights work in pkg/analyzer/lib/dart/analysis/analysis_context_collection.dart, but not in pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart.

I played around a bit in pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart, and found that if I modify the AnalysisContextCollectionImpl constructor so that its updateAnalysisOptions2 argument has type void Function()? instead of void Function({required AnalysisOptionsImpl analysisOptions, required ContextRoot contextRoot, required DartSdk sdk})?, then highlights begin to work throughout the file.

Does that help?

[bwilkerson]

I don't see anything obviously wrong with the AST structure created for that constructor. My best guess at this point is that the code might be throwing an exception that's being caught in such a way that it's returning an empty list of references.

[DanTup]

@stereotype441 do you have an exact git hash and example offset where it's not working? I'm browsing the source at 8a28378 and running the server from Dart SDK version: 3.7.0-edge.0c527e1b9f187c823263d19f9a3c2df8a76ccc22 (main) (Tue Dec 10 04:36:59 2024 +0000) on "windows_x64" but it does seem to work in the noted file for me:

[DanTup]

An issue came up at Dart-Code/Dart-Code#5369 that may or may not be related. In this case, we're throwing exceptions, and that causes VS Code to fall back to doing a text search for highlights.

The exceptions are like:

Unsupported operation: This is not a fragment,
#0      FunctionElementImpl.enclosingFragment (package:analyzer/src/dart/element/element.dart:5057:5)
#1      FragmentExtension.withAncestors (package:analysis_server/src/utilities/extensions/element.dart:81:31)
#2      _SyncStarIterator.moveNext (dart:async-patch/async_patch.dart:595:13)
#3      _getUnitElement (package:analysis_server/src/protocol_server.dart:443:49)
#4      newLocation_fromElement (package:analysis_server/src/protocol_server.dart:306:21)
#5      convertElement (package:analysis_server/plugin/protocol/protocol_dart.dart:35:15)
#6      addDartOccurrences.<anonymous closure> (package:analysis_server/src/domains/analysis/occurrences_dart.dart:18:34)
#7      _LinkedHashMapMixin.forEach (dart:_compact_hash:756:13)
#8      addDartOccurrences (package:analysis_server/src/domains/analysis/occurrences_dart.dart:16:27)

And in the case I'm seeing, it's caused by code like this (the function passed to mapResultsSync is where it throws):

class CompletionHandler {

  @override
  Future<ErrorOr<CompletionList>> handle(
    CompletionParams params,
    MessageInfo message,
    CancellationToken token,
  ) async {
  // ...

    var offset = lineInfo.mapResultSync((lineInfo) => toOffset(lineInfo, pos));

I'll try to reproduce in a test, but I'm not certain if it's the same issue, as from the original description here it sounds like nothing was highlighted, rather than all matching names (I guess it's possible there's a setting for the fallback that maybe @stereotype441 has changed though?).