Skip to content

Fix problem with unbound type parameters as extension targets #2129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jan 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 50 additions & 21 deletions lib/src/element_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,17 @@ abstract class ElementType extends Privacy {

String get linkedName;

String get name => type.name ?? type.element.name;
String get name;

String get nameWithGenerics;

List<Parameter> get parameters => [];

DartType get instantiatedType;

bool isBoundSupertypeTo(ElementType t);
bool isSubtypeOf(ElementType t);

@override
String toString() => "$type";

Expand All @@ -95,23 +100,37 @@ class UndefinedElementType extends ElementType {
@override
bool get isPublic => true;

@override
String get name {
if (type.isDynamic) {
if (returnedFrom != null &&
(returnedFrom is DefinedElementType &&
(returnedFrom as DefinedElementType).element.isAsynchronous)) {
return 'Future';
} else {
return 'dynamic';
}
}
if (type.isVoid) return 'void';
assert(false, 'Unrecognized type for UndefinedElementType');
return '';
}

@override
String get nameWithGenerics => name;

/// dynamic and void are not allowed to have parameterized types.
/// Assume that undefined elements don't have useful bounds.
@override
String get linkedName {
if (type.isDynamic &&
returnedFrom != null &&
(returnedFrom is DefinedElementType &&
(returnedFrom as DefinedElementType).element.isAsynchronous)) {
return 'Future';
}
return name;
}
DartType get instantiatedType => type;

@override
bool isBoundSupertypeTo(ElementType t) => false;

@override
String get name => type.name ?? '';
bool isSubtypeOf(ElementType t) => type.isBottom && !t.type.isBottom;

@override
String get linkedName => name;
}

/// A FunctionType that does not have an underpinning Element.
Expand Down Expand Up @@ -209,8 +228,7 @@ class TypeParameterElementType extends DefinedElementType {
ClassElement get _boundClassElement => type.element;

@override
// TODO(jcollins-g): This is wrong; bound is not always an InterfaceType.
InterfaceType get _interfaceType => (type as TypeParameterType).bound;
DartType get _bound => (type as TypeParameterType).bound;
}

/// An [ElementType] associated with an [Element].
Expand All @@ -226,6 +244,9 @@ abstract class DefinedElementType extends ElementType {
return _element;
}

@override
String get name => type.element.name;

bool get isParameterType => (type is TypeParameterType);

/// This type is a public type if the underlying, canonical element is public.
Expand Down Expand Up @@ -274,32 +295,40 @@ abstract class DefinedElementType extends ElementType {
Class get boundClass =>
ModelElement.fromElement(_boundClassElement, packageGraph);

InterfaceType get _interfaceType => type;
DartType get _bound => type;

InterfaceType _instantiatedType;
DartType _instantiatedType;

/// Return this type, instantiated to bounds if it isn't already.
@override
DartType get instantiatedType {
if (_instantiatedType == null) {
if (!_interfaceType.typeArguments.every((t) => t is InterfaceType)) {
if (_bound is InterfaceType &&
!(_bound as InterfaceType)
.typeArguments
.every((t) => t is InterfaceType)) {
var typeSystem = library.element.typeSystem as TypeSystemImpl;
_instantiatedType = typeSystem.instantiateToBounds(_interfaceType);
// TODO(jcollins-g): convert to ClassElement.instantiateToBounds
// dart-lang/dartdoc#2135
_instantiatedType = typeSystem.instantiateToBounds(_bound);
} else {
_instantiatedType = _interfaceType;
_instantiatedType = _bound;
}
}
return _instantiatedType;
}

/// The instantiated to bounds type of this type is a subtype of
/// [t].
bool isSubtypeOf(DefinedElementType t) =>
@override
bool isSubtypeOf(ElementType t) =>
library.typeSystem.isSubtypeOf(instantiatedType, t.instantiatedType);

/// Returns true if at least one supertype (including via mixins and
/// interfaces) is equivalent to or a subtype of [this] when
/// instantiated to bounds.
bool isBoundSupertypeTo(DefinedElementType t) =>
@override
bool isBoundSupertypeTo(ElementType t) =>
_isBoundSupertypeTo(t.instantiatedType, HashSet());

bool _isBoundSupertypeTo(DartType superType, HashSet<DartType> visited) {
Expand Down
22 changes: 9 additions & 13 deletions lib/src/model/extension.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,23 @@ class Extension extends Container

/// Detect if this extension applies to every object.
bool get alwaysApplies =>
extendedType.type.isDynamic ||
extendedType.type.isVoid ||
extendedType.type.isObject;
extendedType.instantiatedType.isDynamic ||
extendedType.instantiatedType.isVoid ||
extendedType.instantiatedType.isObject;

bool couldApplyTo<T extends ExtensionTarget>(T c) =>
_couldApplyTo(c.modelType);

/// Return true if this extension could apply to [t].
bool _couldApplyTo(DefinedElementType t) {
if (extendedType is UndefinedElementType) {
assert(extendedType.type.isDynamic || extendedType.type.isVoid);
if (extendedType.instantiatedType.isDynamic ||
extendedType.instantiatedType.isVoid) {
return true;
}
{
DefinedElementType extendedType = this.extendedType;
return t.instantiatedType == extendedType.instantiatedType ||
(t.instantiatedType.element ==
extendedType.instantiatedType.element &&
extendedType.isSubtypeOf(t)) ||
extendedType.isBoundSupertypeTo(t);
}
return t.instantiatedType == extendedType.instantiatedType ||
(t.instantiatedType.element == extendedType.instantiatedType.element &&
extendedType.isSubtypeOf(t)) ||
extendedType.isBoundSupertypeTo(t);
}

@override
Expand Down
8 changes: 7 additions & 1 deletion test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1867,22 +1867,28 @@ void main() {
});

test('extensions on special types work', () {
Extension extensionOnDynamic, extensionOnVoid, extensionOnNull;
Extension extensionOnDynamic,
extensionOnVoid,
extensionOnNull,
extensionOnTypeParameter;
Class object = packageGraph.specialClasses[SpecialClass.object];
Extension getExtension(String name) =>
fakeLibrary.extensions.firstWhere((e) => e.name == name);

extensionOnDynamic = getExtension('ExtensionOnDynamic');
extensionOnNull = getExtension('ExtensionOnNull');
extensionOnVoid = getExtension('ExtensionOnVoid');
extensionOnTypeParameter = getExtension('ExtensionOnTypeParameter');

expect(extensionOnDynamic.couldApplyTo(object), isTrue);
expect(extensionOnVoid.couldApplyTo(object), isTrue);
expect(extensionOnNull.couldApplyTo(object), isFalse);
expect(extensionOnTypeParameter.couldApplyTo(object), isTrue);

expect(extensionOnDynamic.alwaysApplies, isTrue);
expect(extensionOnVoid.alwaysApplies, isTrue);
expect(extensionOnNull.alwaysApplies, isFalse);
expect(extensionOnTypeParameter.alwaysApplies, isTrue);

// Even though it does have extensions that could apply to it,
// extensions that apply to [Object] should always be hidden from
Expand Down
5 changes: 4 additions & 1 deletion testing/test_package/lib/fake.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,6 @@ extension DoSomething2X<A, B, R> on Function1<A, Function1<B, R>> {
Function2<A, B, R> something() => (A first, B second) => this(first)(second);
}


/// Extensions might exist on types defined by the language.
extension ExtensionOnDynamic on dynamic {
void youCanAlwaysCallMe() {}
Expand All @@ -1151,3 +1150,7 @@ extension ExtensionOnNull on Null {
void youCanOnlyCallMeOnNulls() {}
}

/// Extensions might exist on unbound type parameters.
extension ExtensionOnTypeParameter<T> on T {
T aFunctionReturningT(T other) => other;
}