[analysis_server] Always show type parameters in LSP type hierarchy, not type arguments

The change at 27ba8fce6c (to fix https://github.com/Dart-Code/Dart-Code/issues/4217) added type arguments to the LSP Type Hierarchy (before, neither type args or type params were shown).

Showing type arguments seemed reasonable when looking at supertypes, but behaves oddly for subtypes (and also when invoked on a type with arguments), so this partly reverts that and shows type parameters in all cases instead.

This simplified the code a bit and removed the `TypeHierarchyAnchor` class we were round-tripping in order to preserve the type arguments.

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

Change-Id: I4e0e92d4c73712fde7a9526c51699ecdae0f4ab1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423020
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/computer/computer_lazy_type_hierarchy.dart

@@ -49,36 +49,16 @@ class DartLazyTypeHierarchyComputer {
   }
 
   /// Finds supertypes for the [Element2] at [location].
-  ///
-  /// If [anchor] is provided, it will be used to navigate to the element at
-  /// [location] to preserve type arguments that have been provided along the
-  /// way.
-  ///
-  /// Anchors are included in returned types (where necessary to preserve type
-  /// arguments) that can be used when calling for the next level of types.
   Future<List<TypeHierarchyRelatedItem>?> findSupertypes(
-    ElementLocation location, {
-    TypeHierarchyAnchor? anchor,
-  }) async {
+    ElementLocation location,
+  ) async {
     var targetElement = await _findTargetElement(location);
     if (targetElement == null) {
       return null;
     }
     var targetType = targetElement.thisType;
 
-    // If we were provided an anchor, use it to re-locate the target type so
-    // that any type arguments supplied along the way will be preserved in the
-    // new node.
-    if (anchor != null) {
-      targetType =
-          await _locateTargetFromAnchor(anchor, targetType) ?? targetType;
-    }
-
-    // If we had no existing anchor, create one that starts from this target as
-    // the starting point for the new supertypes.
-    anchor ??= TypeHierarchyAnchor(location: location, path: []);
-
-    return _getSupertypes(targetType, anchor: anchor);
+    return _getSupertypes(targetType);
   }
 
   /// Finds a target for starting type hierarchy navigation at [offset].
@@ -101,7 +81,9 @@ class DartLazyTypeHierarchyComputer {
       }
     }
 
-    return type is InterfaceType ? TypeHierarchyItem.forType(type) : null;
+    return type is InterfaceType
+        ? TypeHierarchyItem.forElement(type.element3)
+        : null;
   }
 
   /// Locate the [Element2] referenced by [location].
@@ -153,10 +135,7 @@ class DartLazyTypeHierarchyComputer {
   /// Includes all elements that contribute implementation to the type
   /// such as supertypes and mixins, but not interfaces, constraints or
   /// extended types.
-  List<TypeHierarchyRelatedItem> _getSupertypes(
-    InterfaceType type, {
-    TypeHierarchyAnchor? anchor,
-  }) {
+  List<TypeHierarchyRelatedItem> _getSupertypes(InterfaceType type) {
     var supertype = type.superclass;
     var interfaces = type.interfaces;
     var mixins = type.mixins;
@@ -170,19 +149,6 @@ class DartLazyTypeHierarchyComputer {
           ...mixins.map(TypeHierarchyRelatedItem.mixesIn),
         ].nonNulls.toList();
 
-    if (anchor != null) {
-      for (var (index, item) in supertypes.indexed) {
-        // We only need to carry the anchor along if the supertype has type
-        // arguments that we may be populating.
-        if (item._type.typeArguments.isNotEmpty) {
-          item._anchor = TypeHierarchyAnchor(
-            location: anchor.location,
-            path: [...anchor.path, index],
-          );
-        }
-      }
-    }
-
     return supertypes;
   }
 
@@ -193,29 +159,6 @@ class DartLazyTypeHierarchyComputer {
       declaration is MixinDeclaration ||
       declaration is ExtensionTypeDeclaration ||
       declaration is EnumDeclaration;
-
-  /// Navigate to [target] from [anchor], preserving type arguments supplied
-  /// along the way.
-  Future<InterfaceType?> _locateTargetFromAnchor(
-    TypeHierarchyAnchor anchor,
-    InterfaceType target,
-  ) async {
-    // Start from the anchor.
-    var anchorElement = await _findTargetElement(anchor.location);
-    var anchorPath = anchor.path;
-
-    // Follow the provided path.
-    var type = anchorElement?.thisType;
-    for (int i = 0; i < anchorPath.length && type != null; i++) {
-      var index = anchorPath[i];
-      var supertypes = _getSupertypes(type);
-      type = supertypes.length >= index + 1 ? supertypes[index]._type : null;
-    }
-
-    // Verify the element we arrived at matches the targetElement to guard
-    // against code changes that made the path from the anchor invalid.
-    return type != null && type.element3 == target.element3 ? type : null;
-  }
 }
 
 class TypeHierarchyAnchor {
@@ -241,9 +184,6 @@ class TypeHierarchyItem {
   /// names/identifiers have not changed).
   final ElementLocation location;
 
-  /// The type being displayed.
-  final InterfaceType _type;
-
   /// The file that contains the declaration of this item.
   final String file;
 
@@ -254,28 +194,26 @@ class TypeHierarchyItem {
   final SourceRange codeRange;
 
   TypeHierarchyItem({
-    required InterfaceType type,
     required this.displayName,
     required this.location,
     required this.file,
     required this.nameRange,
     required this.codeRange,
-  }) : _type = type;
+  });
 
-  TypeHierarchyItem._forType({
-    required InterfaceType type,
+  TypeHierarchyItem._forElement({
+    required InterfaceElement2 element,
     required this.location,
-  }) : _type = type,
-       displayName = _displayNameForType(type),
-       nameRange = _nameRangeForElement(type.element3),
-       codeRange = _codeRangeForElement(type.element3),
-       file = type.element3.firstFragment.libraryFragment.source.fullName;
+  }) : displayName = _displayNameForElement(element),
+       nameRange = _nameRangeForElement(element),
+       codeRange = _codeRangeForElement(element),
+       file = element.firstFragment.libraryFragment.source.fullName;
 
-  static TypeHierarchyItem? forType(InterfaceType type) {
-    var location = ElementLocation.forElement(type.element3);
+  static TypeHierarchyItem? forElement(InterfaceElement2 element) {
+    var location = ElementLocation.forElement(element);
     if (location == null) return null;
 
-    return TypeHierarchyItem._forType(type: type, location: location);
+    return TypeHierarchyItem._forElement(element: element, location: location);
   }
 
   /// Returns the [SourceRange] of the code for [element].
@@ -285,9 +223,9 @@ class TypeHierarchyItem {
     return SourceRange(firstFragment.codeOffset!, firstFragment.codeLength!);
   }
 
-  /// Returns a name to display in the hierarchy for [type].
-  static String _displayNameForType(InterfaceType type) {
-    return type.getDisplayString();
+  /// Returns a name to display in the hierarchy for [element].
+  static String _displayNameForElement(InterfaceElement2 element) {
+    return element.baseElement.thisType.getDisplayString();
   }
 
   /// Returns the [SourceRange] of the name for [element].
@@ -316,51 +254,48 @@ class TypeHierarchyRelatedItem extends TypeHierarchyItem {
   /// The relationship this item has with the target item.
   final TypeHierarchyItemRelationship relationship;
 
-  TypeHierarchyAnchor? _anchor;
-
-  TypeHierarchyRelatedItem({
-    required super.type,
-    required this.relationship,
-    required super.displayName,
-    required super.location,
-    required super.file,
-    required super.nameRange,
-    required super.codeRange,
-  });
-
-  TypeHierarchyRelatedItem.forType({
-    required super.type,
+  TypeHierarchyRelatedItem.forElement({
+    required super.element,
     required this.relationship,
     required super.location,
-  }) : super._forType();
-
-  /// An optional anchor element used to preserve type args.
-  TypeHierarchyAnchor? get anchor => _anchor;
+  }) : super._forElement();
 
   static TypeHierarchyRelatedItem? constrainedTo(InterfaceType type) =>
-      _forType(type, relationship: TypeHierarchyItemRelationship.constrainedTo);
+      _forElement(
+        type.element3,
+        relationship: TypeHierarchyItemRelationship.constrainedTo,
+      );
 
-  static TypeHierarchyRelatedItem? extends_(InterfaceType type) =>
-      _forType(type, relationship: TypeHierarchyItemRelationship.extends_);
+  static TypeHierarchyRelatedItem? extends_(InterfaceType type) => _forElement(
+    type.element3,
+    relationship: TypeHierarchyItemRelationship.extends_,
+  );
 
   static TypeHierarchyRelatedItem? implements(InterfaceType type) =>
-      _forType(type, relationship: TypeHierarchyItemRelationship.implements);
+      _forElement(
+        type.element3,
+        relationship: TypeHierarchyItemRelationship.implements,
+      );
 
-  static TypeHierarchyRelatedItem? mixesIn(InterfaceType type) =>
-      _forType(type, relationship: TypeHierarchyItemRelationship.mixesIn);
+  static TypeHierarchyRelatedItem? mixesIn(InterfaceType type) => _forElement(
+    type.element3,
+    relationship: TypeHierarchyItemRelationship.mixesIn,
+  );
 
-  static TypeHierarchyRelatedItem? unknown(InterfaceType type) =>
-      _forType(type, relationship: TypeHierarchyItemRelationship.unknown);
+  static TypeHierarchyRelatedItem? unknown(InterfaceType type) => _forElement(
+    type.element3,
+    relationship: TypeHierarchyItemRelationship.unknown,
+  );
 
-  static TypeHierarchyRelatedItem? _forType(
-    InterfaceType type, {
+  static TypeHierarchyRelatedItem? _forElement(
+    InterfaceElement2 element, {
     required TypeHierarchyItemRelationship relationship,
   }) {
-    var location = ElementLocation.forElement(type.element3);
+    var location = ElementLocation.forElement(element);
     if (location == null) return null;
 
-    return TypeHierarchyRelatedItem.forType(
-      type: type,
+    return TypeHierarchyRelatedItem.forElement(
+      element: element,
       relationship: relationship,
       location: location,
     );

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

@@ -190,26 +190,11 @@ class TypeHierarchySupertypesHandler
       }
 
       var location = ElementLocation.decode(data.ref);
-      var anchor = _toServerAnchor(data);
-      var calls = await computer.findSupertypes(location, anchor: anchor);
+      var calls = await computer.findSupertypes(location);
       var results = calls != null ? _convertItems(unit, calls) : null;
       return success(results);
     });
   }
-
-  /// Reads the anchor from [data] (if available) and converts it to a server
-  /// [type_hierarchy.TypeHierarchyAnchor].
-  type_hierarchy.TypeHierarchyAnchor? _toServerAnchor(
-    TypeHierarchyItemInfo data,
-  ) {
-    var anchor = data.anchor;
-    return anchor != null
-        ? type_hierarchy.TypeHierarchyAnchor(
-          location: ElementLocation.decode(anchor.ref),
-          path: anchor.path,
-        )
-        : null;
-  }
 }
 
 /// Utility methods used by all Type Hierarchy handlers.
@@ -224,24 +209,13 @@ mixin _TypeHierarchyUtils on HandlerHelperMixin<AnalysisServer> {
     type_hierarchy.TypeHierarchyItem item,
     LineInfo lineInfo,
   ) {
-    var anchor =
-        item is type_hierarchy.TypeHierarchyRelatedItem ? item.anchor : null;
     return TypeHierarchyItem(
       name: item.displayName,
       kind: SymbolKind.Class,
       uri: uriConverter.toClientUri(item.file),
       range: sourceRangeToRange(lineInfo, item.codeRange),
       selectionRange: sourceRangeToRange(lineInfo, item.nameRange),
-      data: TypeHierarchyItemInfo(
-        ref: item.location.encoding,
-        anchor:
-            anchor != null
-                ? TypeHierarchyAnchor(
-                  ref: anchor.location.encoding,
-                  path: anchor.path,
-                )
-                : null,
-      ),
+      data: TypeHierarchyItemInfo(ref: item.location.encoding),
     );
   }
 

pkg/analysis_server/test/lsp/type_hierarchy_test.dart

@@ -418,8 +418,8 @@ extension type E^2(A a) implements B, E1 {}
     );
   }
 
-  /// Ensure that type arguments flow across multiple levels of the tree.
-  Future<void> test_generics_typeArgsFlow() async {
+  /// Ensure that we always show type parameters and not type arguments.
+  Future<void> test_generics_typeParameters() async {
     var content = '''
 class A<T1, T2> {}
 class B<T1, T2> extends A<T1, T2> {}
@@ -442,14 +442,7 @@ class ^E extends D {}
     }
 
     // Check for substituted type args.
-    expect(names, [
-      'E',
-      'D',
-      'C<int>',
-      'B<int, String>',
-      'A<int, String>',
-      'Object',
-    ]);
+    expect(names, ['E', 'D', 'C<T1>', 'B<T1, T2>', 'A<T1, T2>', 'Object']);
   }
 
   Future<void> test_implements() async {

pkg/analysis_server/test/src/computer/type_hierarchy_computer_test.dart

@@ -292,10 +292,9 @@ class TypeHierarchyComputerFindSupertypesTest
   ) async {
     var file = getFile(target.file);
     var result = await getResolvedUnit(file);
-    var anchor = target is TypeHierarchyRelatedItem ? target.anchor : null;
     return DartLazyTypeHierarchyComputer(
       result,
-    ).findSupertypes(target.location, anchor: anchor);
+    ).findSupertypes(target.location);
   }
 
   /// Test that if the file is modified between fetching a target and it's
@@ -338,7 +337,7 @@ class ^MyClass2<T1> implements MyClass1<T1, String> {}
     expect(supertypes, [
       _isObject,
       _isRelatedItem(
-        'MyClass1<T1, String>',
+        'MyClass1<T1, T2>',
         testFile.path,
         relationship: TypeHierarchyItemRelationship.implements,
         codeRange: parsedTestCode.ranges[0].sourceRange,
@@ -347,8 +346,8 @@ class ^MyClass2<T1> implements MyClass1<T1, String> {}
     ]);
   }
 
-  /// Ensure that type arguments flow across multiple levels of the tree.
-  Future<void> test_class_generic_typeArgsFlow() async {
+  /// Ensure that type parameters are shown instead of type arguments.
+  Future<void> test_class_generic_typeParameters() async {
     var content = '''
 class A<T1, T2> {}
 class B<T1, T2> extends A<T1, T2> {}
@@ -371,15 +370,7 @@ class ^E extends D {}
               : null;
     }
 
-    // Check for substituted type args.
-    expect(names, [
-      'E',
-      'D',
-      'C<int>',
-      'B<int, String>',
-      'A<int, String>',
-      'Object',
-    ]);
+    expect(names, ['E', 'D', 'C<T1>', 'B<T1, T2>', 'A<T1, T2>', 'Object']);
   }
 
   Future<void> test_class_interfaces() async {
@@ -741,7 +732,8 @@ int? b;
     );
   }
 
-  /// Ensure invocations directly on a type with type args retain those args.
+  /// Ensure invocations directly on a type with type arguments show the type
+  /// parameters.
   Future<void> test_typeReference_generic() async {
     var content = '''
 /*[0*/class /*[1*/MyClass1/*1]*/<T1, T2> {}/*0]*/
@@ -751,7 +743,7 @@ MyCl^ass1<String, String>? a;
     addTestSource(content);
     await expectTarget(
       _isItem(
-        'MyClass1<String, String>?',
+        'MyClass1<T1, T2>',
         testFile.path,
         codeRange: parsedTestCode.ranges[0].sourceRange,
         nameRange: parsedTestCode.ranges[1].sourceRange,

pkg/analysis_server/tool/lsp_spec/generate_all.dart

@@ -388,19 +388,6 @@ List<LspEntity> getCustomClasses() {
       field('valid', type: 'boolean'),
       field('message', type: 'string', canBeUndefined: true),
     ]),
-    interface('TypeHierarchyAnchor', [
-      field(
-        'ref',
-        type: 'string',
-        comment: 'The ElementLocation for this anchor element.',
-      ),
-      field(
-        'path',
-        type: 'int',
-        array: true,
-        comment: 'Indices used to navigate from this anchor to the element.',
-      ),
-    ]),
     interface('TypeHierarchyItemInfo', [
       field(
         'ref',
@@ -410,14 +397,6 @@ List<LspEntity> getCustomClasses() {
             'element when subtypes/supertypes are '
             'fetched later.',
       ),
-      field(
-        'anchor',
-        type: 'TypeHierarchyAnchor',
-        comment:
-            'An anchor element that can be used to navigate to this element '
-            'preserving type arguments.',
-        canBeUndefined: true,
-      ),
     ]),
     interface('EditableArguments', [
       field('textDocument', type: 'TextDocumentIdentifier'),

pkg/analyzer/api.txt

@@ -3883,6 +3883,7 @@ package:analyzer/dart/element/element2.dart:
   HideElementCombinator (see above)
   InstanceElement2 (class extends Object implements TypeDefiningElement2, TypeParameterizedElement2, HasSinceSdkVersion):
     new (constructor: InstanceElement2 Function())
+    baseElement (getter: InstanceElement2)
     enclosingElement2 (getter: LibraryElement2)
     fields2 (getter: List<FieldElement2>)
     firstFragment (getter: InstanceFragment)

pkg/analyzer/lib/dart/element/element2.dart

@@ -1280,6 +1280,9 @@ abstract class InstanceElement2
         TypeDefiningElement2,
         TypeParameterizedElement2,
         HasSinceSdkVersion {
+  @override
+  InstanceElement2 get baseElement;
+
   @override
   LibraryElement2 get enclosingElement2;
 

third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart

@@ -343,32 +343,6 @@ bool _canParseListFlutterOutlineAttribute(
   return true;
 }
 
-bool _canParseListInt(
-    Map<String, Object?> map, LspJsonReporter reporter, String fieldName,
-    {required bool allowsUndefined, required bool allowsNull}) {
-  reporter.push(fieldName);
-  try {
-    if (!allowsUndefined && !map.containsKey(fieldName)) {
-      reporter.reportError('must not be undefined');
-      return false;
-    }
-    final value = map[fieldName];
-    final nullCheck = allowsNull || allowsUndefined;
-    if (!nullCheck && value == null) {
-      reporter.reportError('must not be null');
-      return false;
-    }
-    if ((!nullCheck || value != null) &&
-        (value is! List<Object?> || value.any((item) => item is! int))) {
-      reporter.reportError('must be of type List<int>');
-      return false;
-    }
-  } finally {
-    reporter.pop();
-  }
-  return true;
-}
-
 bool _canParseListOutline(
     Map<String, Object?> map, LspJsonReporter reporter, String fieldName,
     {required bool allowsUndefined, required bool allowsNull}) {
@@ -651,31 +625,6 @@ bool _canParseTextDocumentIdentifier(
   return true;
 }
 
-bool _canParseTypeHierarchyAnchor(
-    Map<String, Object?> map, LspJsonReporter reporter, String fieldName,
-    {required bool allowsUndefined, required bool allowsNull}) {
-  reporter.push(fieldName);
-  try {
-    if (!allowsUndefined && !map.containsKey(fieldName)) {
-      reporter.reportError('must not be undefined');
-      return false;
-    }
-    final value = map[fieldName];
-    final nullCheck = allowsNull || allowsUndefined;
-    if (!nullCheck && value == null) {
-      reporter.reportError('must not be null');
-      return false;
-    }
-    if ((!nullCheck || value != null) &&
-        !TypeHierarchyAnchor.canParse(value, reporter)) {
-      return false;
-    }
-  } finally {
-    reporter.pop();
-  }
-  return true;
-}
-
 bool _canParseUri(
     Map<String, Object?> map, LspJsonReporter reporter, String fieldName,
     {required bool allowsUndefined, required bool allowsNull}) {
@@ -3414,112 +3363,33 @@ class SnippetTextEdit implements TextEdit, ToJsonable {
   }
 }
 
-class TypeHierarchyAnchor implements ToJsonable {
-  static const jsonHandler = LspJsonHandler(
-    TypeHierarchyAnchor.canParse,
-    TypeHierarchyAnchor.fromJson,
-  );
-
-  /// Indices used to navigate from this anchor to the element.
-  final List<int> path;
-
-  /// The ElementLocation for this anchor element.
-  final String ref;
-
-  TypeHierarchyAnchor({
-    required this.path,
-    required this.ref,
-  });
-  @override
-  int get hashCode => Object.hash(
-        lspHashCode(path),
-        ref,
-      );
-
-  @override
-  bool operator ==(Object other) {
-    return other is TypeHierarchyAnchor &&
-        other.runtimeType == TypeHierarchyAnchor &&
-        const DeepCollectionEquality().equals(path, other.path) &&
-        ref == other.ref;
-  }
-
-  @override
-  Map<String, Object?> toJson() {
-    var result = <String, Object?>{};
-    result['path'] = path;
-    result['ref'] = ref;
-    return result;
-  }
-
-  @override
-  String toString() => jsonEncoder.convert(toJson());
-
-  static bool canParse(Object? obj, LspJsonReporter reporter) {
-    if (obj is Map<String, Object?>) {
-      if (!_canParseListInt(obj, reporter, 'path',
-          allowsUndefined: false, allowsNull: false)) {
-        return false;
-      }
-      return _canParseString(obj, reporter, 'ref',
-          allowsUndefined: false, allowsNull: false);
-    } else {
-      reporter.reportError('must be of type TypeHierarchyAnchor');
-      return false;
-    }
-  }
-
-  static TypeHierarchyAnchor fromJson(Map<String, Object?> json) {
-    final pathJson = json['path'];
-    final path =
-        (pathJson as List<Object?>).map((item) => item as int).toList();
-    final refJson = json['ref'];
-    final ref = refJson as String;
-    return TypeHierarchyAnchor(
-      path: path,
-      ref: ref,
-    );
-  }
-}
-
 class TypeHierarchyItemInfo implements ToJsonable {
   static const jsonHandler = LspJsonHandler(
     TypeHierarchyItemInfo.canParse,
     TypeHierarchyItemInfo.fromJson,
   );
 
-  /// An anchor element that can be used to navigate to this element preserving
-  /// type arguments.
-  final TypeHierarchyAnchor? anchor;
-
   /// The ElementLocation for this element, used to re-locate the element when
   /// subtypes/supertypes are fetched later.
   final String ref;
 
   TypeHierarchyItemInfo({
-    this.anchor,
     required this.ref,
   });
+
   @override
-  int get hashCode => Object.hash(
-        anchor,
-        ref,
-      );
+  int get hashCode => ref.hashCode;
 
   @override
   bool operator ==(Object other) {
     return other is TypeHierarchyItemInfo &&
         other.runtimeType == TypeHierarchyItemInfo &&
-        anchor == other.anchor &&
         ref == other.ref;
   }
 
   @override
   Map<String, Object?> toJson() {
     var result = <String, Object?>{};
-    if (anchor != null) {
-      result['anchor'] = anchor?.toJson();
-    }
     result['ref'] = ref;
     return result;
   }
@@ -3529,10 +3399,6 @@ class TypeHierarchyItemInfo implements ToJsonable {
 
   static bool canParse(Object? obj, LspJsonReporter reporter) {
     if (obj is Map<String, Object?>) {
-      if (!_canParseTypeHierarchyAnchor(obj, reporter, 'anchor',
-          allowsUndefined: true, allowsNull: false)) {
-        return false;
-      }
       return _canParseString(obj, reporter, 'ref',
           allowsUndefined: false, allowsNull: false);
     } else {
@@ -3542,14 +3408,9 @@ class TypeHierarchyItemInfo implements ToJsonable {
   }
 
   static TypeHierarchyItemInfo fromJson(Map<String, Object?> json) {
-    final anchorJson = json['anchor'];
-    final anchor = anchorJson != null
-        ? TypeHierarchyAnchor.fromJson(anchorJson as Map<String, Object?>)
-        : null;
     final refJson = json['ref'];
     final ref = refJson as String;
     return TypeHierarchyItemInfo(
-      anchor: anchor,
       ref: ref,
     );
   }