Skip to content

Commit

Permalink
Fix problem with unbound type parameters as extension targets (#2129)
Browse files Browse the repository at this point in the history
* Fix problem with unbound type parameters as extension targets

* Eliminate type related deprecation warnings and fix bugs in changed type hierarchy

* Remove TODO that was implemented

* Revert accidentally included change from #2130

* Review comments
  • Loading branch information
jcollins-g committed Jan 22, 2020
1 parent 465f1dc commit b5e341e
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 36 deletions.
71 changes: 50 additions & 21 deletions lib/src/element_type.dart
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
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
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
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;
}

0 comments on commit b5e341e

Please sign in to comment.