[analysis_server] Always show type parameters in LSP type hierarchy, not type arguments
The change at 27ba8fce6c73f23e221d8437529c73716938ce94 (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>
diff --git a/pkg/analysis_server/lib/src/computer/computer_lazy_type_hierarchy.dart b/pkg/analysis_server/lib/src/computer/computer_lazy_type_hierarchy.dart
index 4069e9e..d73c483 100644
--- a/pkg/analysis_server/lib/src/computer/computer_lazy_type_hierarchy.dart
+++ b/pkg/analysis_server/lib/src/computer/computer_lazy_type_hierarchy.dart
@@ -49,36 +49,16 @@
}
/// 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 @@
}
}
- 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 @@
/// 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 @@
...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 @@
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 @@
/// 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 @@
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 @@
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 @@
/// 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,
);
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_type_hierarchy.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_type_hierarchy.dart
index c66e055..2836ecd 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_type_hierarchy.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_type_hierarchy.dart
@@ -190,26 +190,11 @@
}
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 @@
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),
);
}
diff --git a/pkg/analysis_server/test/lsp/type_hierarchy_test.dart b/pkg/analysis_server/test/lsp/type_hierarchy_test.dart
index b12671f..6b35c2d 100644
--- a/pkg/analysis_server/test/lsp/type_hierarchy_test.dart
+++ b/pkg/analysis_server/test/lsp/type_hierarchy_test.dart
@@ -418,8 +418,8 @@
);
}
- /// 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 @@
}
// 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 {
diff --git a/pkg/analysis_server/test/src/computer/type_hierarchy_computer_test.dart b/pkg/analysis_server/test/src/computer/type_hierarchy_computer_test.dart
index bc640e9..28db9a7 100644
--- a/pkg/analysis_server/test/src/computer/type_hierarchy_computer_test.dart
+++ b/pkg/analysis_server/test/src/computer/type_hierarchy_computer_test.dart
@@ -292,10 +292,9 @@
) 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 @@
expect(supertypes, [
_isObject,
_isRelatedItem(
- 'MyClass1<T1, String>',
+ 'MyClass1<T1, T2>',
testFile.path,
relationship: TypeHierarchyItemRelationship.implements,
codeRange: parsedTestCode.ranges[0].sourceRange,
@@ -347,8 +346,8 @@
]);
}
- /// 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 @@
: 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 @@
);
}
- /// 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 @@
addTestSource(content);
await expectTarget(
_isItem(
- 'MyClass1<String, String>?',
+ 'MyClass1<T1, T2>',
testFile.path,
codeRange: parsedTestCode.ranges[0].sourceRange,
nameRange: parsedTestCode.ranges[1].sourceRange,
diff --git a/pkg/analysis_server/tool/lsp_spec/generate_all.dart b/pkg/analysis_server/tool/lsp_spec/generate_all.dart
index 68b711a..de8c249f 100644
--- a/pkg/analysis_server/tool/lsp_spec/generate_all.dart
+++ b/pkg/analysis_server/tool/lsp_spec/generate_all.dart
@@ -388,19 +388,6 @@
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 @@
'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'),
diff --git a/pkg/analyzer/api.txt b/pkg/analyzer/api.txt
index e31ba3c..2bf6011 100644
--- a/pkg/analyzer/api.txt
+++ b/pkg/analyzer/api.txt
@@ -3883,6 +3883,7 @@
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)
diff --git a/pkg/analyzer/lib/dart/element/element2.dart b/pkg/analyzer/lib/dart/element/element2.dart
index 16b74b9..a2bf0d9 100644
--- a/pkg/analyzer/lib/dart/element/element2.dart
+++ b/pkg/analyzer/lib/dart/element/element2.dart
@@ -1281,6 +1281,9 @@
TypeParameterizedElement2,
HasSinceSdkVersion {
@override
+ InstanceElement2 get baseElement;
+
+ @override
LibraryElement2 get enclosingElement2;
/// The fields declared in this element.
diff --git a/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart b/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart
index 30bdfcc..94c6d698 100644
--- a/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart
+++ b/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart
@@ -343,32 +343,6 @@
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 @@
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 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 @@
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 @@
}
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,
);
}
