Skip to content

Commit ae0b50d

Browse files
stereotype441Commit Queue
authored andcommitted
[mini_types] Add a Type.gatherUsedIdentifiers method.
This method will be used by a follow-up CL that adds support for generic functions, since the implementation of `==` and `hashCode` for generic functions needs to perform substitutions in order to recognize that alpha-equivalent types (such as `T Function<T>()` and `U Function<U>`) are equal, and those substitutions need to avoid name collisions with identifier names that already appear in the type. Change-Id: Ifd237a9842791f440e285d89e9d80b898e996411 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395685 Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 922847a commit ae0b50d

File tree

2 files changed

+107
-0
lines changed

2 files changed

+107
-0
lines changed

pkg/_fe_analyzer_shared/test/mini_types.dart

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,25 @@ class FunctionType extends Type
111111
nullabilitySuffix: nullabilitySuffix);
112112
}
113113

114+
@override
115+
void gatherUsedIdentifiers(Set<String> identifiers) {
116+
returnType.gatherUsedIdentifiers(identifiers);
117+
for (var positionalParameter in positionalParameters) {
118+
positionalParameter.gatherUsedIdentifiers(identifiers);
119+
}
120+
for (var typeFormal in typeFormals) {
121+
identifiers.add(typeFormal.name);
122+
}
123+
for (var namedParameter in namedParameters) {
124+
// As explained in the documentation for `Type.gatherUsedIdentifiers`,
125+
// to reduce the risk of confusion, this method is generous in which
126+
// identifiers it reports. So report `namedParameter.name` even though
127+
// it's not strictly necessary.
128+
identifiers.add(namedParameter.name);
129+
namedParameter.type.gatherUsedIdentifiers(identifiers);
130+
}
131+
}
132+
114133
@override
115134
Type? recursivelyDemote({required bool covariant}) {
116135
Type? newReturnType = returnType.recursivelyDemote(covariant: covariant);
@@ -383,6 +402,14 @@ class PrimaryType extends Type {
383402
args: newArgs, nullabilitySuffix: nullabilitySuffix);
384403
}
385404

405+
@override
406+
void gatherUsedIdentifiers(Set<String> identifiers) {
407+
identifiers.add(name);
408+
for (var arg in args) {
409+
arg.gatherUsedIdentifiers(identifiers);
410+
}
411+
}
412+
386413
@override
387414
Type? recursivelyDemote({required bool covariant}) {
388415
List<Type>? newArgs = args.recursivelyDemote(covariant: covariant);
@@ -467,6 +494,21 @@ class RecordType extends Type implements SharedRecordTypeStructure<Type> {
467494
);
468495
}
469496

497+
@override
498+
void gatherUsedIdentifiers(Set<String> identifiers) {
499+
for (var type in positionalTypes) {
500+
type.gatherUsedIdentifiers(identifiers);
501+
}
502+
for (var namedType in namedTypes) {
503+
// As explained in the documentation for `Type.gatherUsedIdentifiers`,
504+
// to reduce the risk of confusion, this method is generous in which
505+
// identifiers it reports. So report `namedType.name` even though it's
506+
// not strictly necessary.
507+
identifiers.add(namedType.name);
508+
namedType.type.gatherUsedIdentifiers(identifiers);
509+
}
510+
}
511+
470512
@override
471513
Type? recursivelyDemote({required bool covariant}) {
472514
List<Type>? newPositional;
@@ -581,6 +623,21 @@ abstract class Type implements SharedTypeStructure<Type>, _Substitutable<Type> {
581623
/// `Never`).
582624
Type? closureWithRespectToUnknown({required bool covariant});
583625

626+
/// Recursively visits `this`, gathering up all the identifiers that appear in
627+
/// it, and adds them to the set [identifiers].
628+
///
629+
/// This method is intended to aid in choosing safe names for substitutions.
630+
/// For example, it can be used to determine that in a type like
631+
/// `T Function<U>(U)`, it's not safe to rename the type variable `U` to `T`,
632+
/// since that would conflict with an existing use of `T`.
633+
///
634+
/// To lower the risk of confusion, it is generous in which identifiers it
635+
/// reports. For example, in the type `void Function<T>({T X})`, it reports
636+
/// `X` as a used identifier. This is because even though it would technically
637+
/// be safe to rename the type variable `T` to `X`, to do so would be result
638+
/// in a confusing type.
639+
void gatherUsedIdentifiers(Set<String> identifiers);
640+
584641
@override
585642
String getDisplayString() => type;
586643

@@ -724,6 +781,12 @@ class TypeParameterType extends Type {
724781
promotion: newPromotion, nullabilitySuffix: nullabilitySuffix);
725782
}
726783

784+
@override
785+
void gatherUsedIdentifiers(Set<String> identifiers) {
786+
identifiers.add(typeParameter.name);
787+
promotion?.gatherUsedIdentifiers(identifiers);
788+
}
789+
727790
@override
728791
Type? recursivelyDemote({required bool covariant}) {
729792
if (!covariant) {
@@ -1474,6 +1537,9 @@ class UnknownType extends Type implements SharedUnknownTypeStructure<Type> {
14741537
Type? closureWithRespectToUnknown({required bool covariant}) =>
14751538
covariant ? Type('Object?') : NeverType.instance;
14761539

1540+
@override
1541+
void gatherUsedIdentifiers(Set<String> identifiers) {}
1542+
14771543
@override
14781544
Type? recursivelyDemote({required bool covariant}) => null;
14791545

pkg/_fe_analyzer_shared/test/mini_types_test.dart

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,47 @@ main() {
10211021
});
10221022
});
10231023

1024+
group('gatherUsedIdentifiers:', () {
1025+
Set<String> queryUsedIdentifiers(Type t) {
1026+
var identifiers = <String>{};
1027+
t.gatherUsedIdentifiers(identifiers);
1028+
return identifiers;
1029+
}
1030+
1031+
test('FunctionType', () {
1032+
expect(queryUsedIdentifiers(Type('int Function(String, {bool b})')),
1033+
unorderedEquals({'int', 'String', 'bool', 'b'}));
1034+
});
1035+
1036+
test('PrimaryType', () {
1037+
expect(queryUsedIdentifiers(Type('Map<String, int>')),
1038+
unorderedEquals({'Map', 'String', 'int'}));
1039+
expect(
1040+
queryUsedIdentifiers(Type('dynamic')), unorderedEquals({'dynamic'}));
1041+
expect(queryUsedIdentifiers(Type('error')), unorderedEquals({'error'}));
1042+
expect(queryUsedIdentifiers(Type('Never')), unorderedEquals({'Never'}));
1043+
expect(queryUsedIdentifiers(Type('Null')), unorderedEquals({'Null'}));
1044+
expect(queryUsedIdentifiers(Type('void')), unorderedEquals({'void'}));
1045+
expect(queryUsedIdentifiers(Type('FutureOr<int>')),
1046+
unorderedEquals({'FutureOr', 'int'}));
1047+
});
1048+
1049+
test('RecordType', () {
1050+
expect(queryUsedIdentifiers(Type('(int, {String s})')),
1051+
unorderedEquals({'int', 'String', 's'}));
1052+
});
1053+
1054+
test('TypeParameterType', () {
1055+
expect(queryUsedIdentifiers(Type('T')), unorderedEquals({'T'}));
1056+
expect(
1057+
queryUsedIdentifiers(Type('T&int')), unorderedEquals({'T', 'int'}));
1058+
});
1059+
1060+
test('UnknownType', () {
1061+
expect(queryUsedIdentifiers(Type('_')), isEmpty);
1062+
});
1063+
});
1064+
10241065
group('substitute:', () {
10251066
test('FunctionType', () {
10261067
expect(Type('int Function(int, {int i})').substitute({t: Type('String')}),

0 commit comments

Comments
 (0)