Fix LSP workspace symbols to always have a SymbolKind

Bug: https://github.com/dart-lang/sdk/issues/40636
Change-Id: I28855dac790291a16717c20cf46c33dfae23e9ff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135902
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <danny@tuppeny.com>

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

@@ -14,7 +14,6 @@ import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/protocol_server.dart' show Outline;
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/source/line_info.dart';
-import 'package:analyzer_plugin/protocol/protocol_common.dart' show ElementKind;
 
 // If the client does not provide capabilities.documentSymbol.symbolKind.valueSet
 // then we must never send a kind that's not in this list.
@@ -80,13 +79,8 @@ class DocumentSymbolHandler extends MessageHandler<DocumentSymbolParams,
     LineInfo lineInfo,
     Outline outline,
   ) {
-    final name = outline.element.name != null && outline.element.name != ''
-        ? outline.element.name
-        : (outline.element.kind == ElementKind.EXTENSION
-            ? '<unnamed extension>'
-            : '<unnamed>');
     return DocumentSymbol(
-      name,
+      toElementName(outline.element),
       outline.element.parameters,
       elementKindToSymbolKind(clientSupportedSymbolKinds, outline.element.kind),
       outline.element.isDeprecated,
@@ -108,7 +102,7 @@ class DocumentSymbolHandler extends MessageHandler<DocumentSymbolParams,
     Outline outline,
   ) {
     return SymbolInformation(
-      outline.element.name,
+      toElementName(outline.element),
       elementKindToSymbolKind(clientSupportedSymbolKinds, outline.element.kind),
       outline.element.isDeprecated,
       Location(

pkg/analysis_server/lib/src/lsp/mapping.dart

@@ -160,12 +160,17 @@ lsp.SymbolKind declarationKindToSymbolKind(
       case server.DeclarationKind.VARIABLE:
         return const [lsp.SymbolKind.Variable];
       default:
-        assert(false, 'Unexpected declaration kind $kind');
+        // Assert that we only get here if kind=null. If it's anything else
+        // then we're missing a mapping from above.
+        assert(kind == null, 'Unexpected declaration kind $kind');
         return const [];
     }
   }
 
-  return getKindPreferences().firstWhere(isSupported, orElse: () => null);
+  // LSP requires we specify *some* kind, so in the case where the above code doesn't
+  // match we'll just have to send a value to avoid a crash.
+  return getKindPreferences()
+      .firstWhere(isSupported, orElse: () => lsp.SymbolKind.Obj);
 }
 
 lsp.CompletionItem declarationToCompletionItem(
@@ -383,7 +388,9 @@ lsp.SymbolKind elementKindToSymbolKind(
       case server.ElementKind.UNIT_TEST_TEST:
         return const [lsp.SymbolKind.Method];
       default:
-        assert(false, 'Unexpected element kind $kind');
+        // Assert that we only get here if kind=null. If it's anything else
+        // then we're missing a mapping from above.
+        assert(kind == null, 'Unexpected element kind $kind');
         return const [];
     }
   }
@@ -746,13 +753,21 @@ lsp.Element toElement(server.LineInfo lineInfo, server.Element element) =>
       element.location != null
           ? toRange(lineInfo, element.location.offset, element.location.length)
           : null,
-      element.name,
+      toElementName(element),
       element.kind.name,
       element.parameters,
       element.typeParameters,
       element.returnType,
     );
 
+String toElementName(server.Element element) {
+  return element.name != null && element.name != ''
+      ? element.name
+      : (element.kind == server.ElementKind.EXTENSION
+          ? '<unnamed extension>'
+          : '<unnamed>');
+}
+
 lsp.FlutterOutline toFlutterOutline(
         server.LineInfo lineInfo, server.FlutterOutline outline) =>
     lsp.FlutterOutline(

pkg/analysis_server/test/lsp/document_symbols_test.dart

@@ -24,6 +24,8 @@ class DocumentSymbolsTest extends AbstractLspAnalysisServerTest {
       MyClass(this.myField);
       myMethod() {}
     }
+    extension StringExtensions on String {}
+    extension on String {}
     ''';
     newFile(mainFilePath, content: content);
     await initialize();
@@ -33,7 +35,7 @@ class DocumentSymbolsTest extends AbstractLspAnalysisServerTest {
       (docsymbols) => throw 'Expected SymbolInformations, got DocumentSymbols',
       (symbolInfos) => symbolInfos,
     );
-    expect(symbols, hasLength(5));
+    expect(symbols, hasLength(7));
 
     final topLevel = symbols[0];
     expect(topLevel.name, equals('topLevel'));
@@ -59,6 +61,14 @@ class DocumentSymbolsTest extends AbstractLspAnalysisServerTest {
     expect(method.name, equals('myMethod'));
     expect(method.kind, equals(SymbolKind.Method));
     expect(method.containerName, equals(myClass.name));
+
+    final namedExtension = symbols[5];
+    expect(namedExtension.name, equals('StringExtensions'));
+    expect(namedExtension.containerName, isNull);
+
+    final unnamedExtension = symbols[6];
+    expect(unnamedExtension.name, equals('<unnamed extension>'));
+    expect(unnamedExtension.containerName, isNull);
   }
 
   Future<void> test_hierarchical() async {

pkg/analysis_server/test/lsp/outline_test.dart

@@ -38,6 +38,23 @@ class OutlineTest extends AbstractLspAnalysisServerTest {
     expect(outlineAfterChange.children[0].element.name, equals('B'));
   }
 
+  Future<void> test_extensions() async {
+    final initialContent = '''
+extension StringExtensions on String {}
+extension on String {}
+    ''';
+    await initialize(initializationOptions: {'outline': true});
+
+    final outlineUpdate = waitForOutline(mainFileUri);
+    openFile(mainFileUri, initialContent);
+    final outline = await outlineUpdate;
+
+    expect(outline, isNotNull);
+    expect(outline.children, hasLength(2));
+    expect(outline.children[0].element.name, equals('StringExtensions'));
+    expect(outline.children[1].element.name, equals('<unnamed extension>'));
+  }
+
   Future<void> test_initial() async {
     final content = '''
 /// a

pkg/analysis_server/test/lsp/workspace_symbols_test.dart

@@ -17,6 +17,24 @@ void main() {
 
 @reflectiveTest
 class WorkspaceSymbolsTest extends AbstractLspAnalysisServerTest {
+  Future<void> test_extensions() async {
+    const content = '''
+    extension StringExtensions on String {}
+    extension on String {}
+    ''';
+    newFile(mainFilePath, content: withoutMarkers(content));
+    await initialize();
+
+    final symbols = await getWorkspaceSymbols('S');
+
+    final namedExtensions =
+        symbols.firstWhere((s) => s.name == 'StringExtensions');
+    expect(namedExtensions.kind, equals(SymbolKind.Obj));
+    expect(namedExtensions.containerName, isNull);
+
+    // Unnamed extensions are not returned in Workspace Symbols.
+  }
+
   Future<void> test_fullMatch() async {
     const content = '''
     [[String topLevel = '']];