From 36ce10c745ccf43923b740465ccc0abf891c28d4 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 15 Jan 2020 10:04:40 -0800 Subject: [PATCH 1/5] Fix problem with unbound type parameters as extension targets --- lib/src/element_type.dart | 15 +++++++++------ lib/src/package_meta.dart | 3 ++- test/model_test.dart | 8 +++++++- testing/test_package/lib/fake.dart | 5 ++++- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 9d9bb3c248..3b2120251c 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -210,7 +210,7 @@ class TypeParameterElementType extends DefinedElementType { @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]. @@ -274,18 +274,21 @@ 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. 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); + _instantiatedType = typeSystem.instantiateToBounds(_bound); } else { - _instantiatedType = _interfaceType; + _instantiatedType = _bound; } } return _instantiatedType; diff --git a/lib/src/package_meta.dart b/lib/src/package_meta.dart index c6ba1bb66c..19d922d567 100644 --- a/lib/src/package_meta.dart +++ b/lib/src/package_meta.dart @@ -317,7 +317,8 @@ class _FilePackageMeta extends PackageMeta { @override bool get requiresFlutter => - _pubspec['environment']?.containsKey('flutter') == true; + _pubspec['environment']?.containsKey('flutter') == true || + _pubspec['dependencies']?.containsKey('flutter') == true; @override FileContents getReadmeContents() { diff --git a/test/model_test.dart b/test/model_test.dart index c1da301d1c..99a21fabb4 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -1867,7 +1867,10 @@ 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); @@ -1875,14 +1878,17 @@ void main() { extensionOnDynamic = getExtension('ExtensionOnDynamic'); extensionOnNull = getExtension('ExtensionOnNull'); extensionOnVoid = getExtension('ExtensionOnVoid'); + extensionOnVoid = 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 diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 9f1586d37e..22b1a1d73f 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1137,7 +1137,6 @@ extension DoSomething2X on Function1> { Function2 something() => (A first, B second) => this(first)(second); } - /// Extensions might exist on types defined by the language. extension ExtensionOnDynamic on dynamic { void youCanAlwaysCallMe() {} @@ -1151,3 +1150,7 @@ extension ExtensionOnNull on Null { void youCanOnlyCallMeOnNulls() {} } +/// Extensions might exist on unbound type parameters. +extension ExtensionOnTypeParameter on T { + T aFunctionReturningT(T other) => other; +} From e9115893301d5649c8c92ab270ede0f89a7b28ed Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 16 Jan 2020 14:16:34 -0800 Subject: [PATCH 2/5] Eliminate type related deprecation warnings and fix bugs in changed type hierarchy --- bin/dartdoc.dart | 1 - lib/src/element_type.dart | 50 +++++++++++++++++++++++--------- lib/src/html/html_generator.dart | 2 -- lib/src/html/template_data.dart | 1 - lib/src/model/extension.dart | 22 ++++++-------- test/model_test.dart | 2 +- test/src/utils.dart | 1 - 7 files changed, 46 insertions(+), 33 deletions(-) diff --git a/bin/dartdoc.dart b/bin/dartdoc.dart index bd6bad48c5..f76c82549a 100644 --- a/bin/dartdoc.dart +++ b/bin/dartdoc.dart @@ -9,7 +9,6 @@ import 'dart:io'; import 'package:args/args.dart'; import 'package:dartdoc/dartdoc.dart'; -import 'package:dartdoc/src/html/html_generator.dart'; import 'package:dartdoc/src/logging.dart'; import 'package:dartdoc/src/tool_runner.dart'; import 'package:stack_trace/stack_trace.dart'; diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 3b2120251c..9491c5b25b 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -71,12 +71,17 @@ abstract class ElementType extends Privacy { String get linkedName; - String get name => type.name ?? type.element.name; + String get name => type.element.name; String get nameWithGenerics; List get parameters => []; + DartType get instantiatedType; + + bool isBoundSupertypeTo(ElementType t); + bool isSubtypeOf(ElementType t); + @override String toString() => "$type"; @@ -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 + bool isSubtypeOf(ElementType t) => type.isBottom && !t.type.isBottom; @override - String get name => type.name ?? ''; + String get linkedName => name; } /// A FunctionType that does not have an underpinning Element. @@ -279,6 +298,7 @@ abstract class DefinedElementType extends ElementType { DartType _instantiatedType; /// Return this type, instantiated to bounds if it isn't already. + @override DartType get instantiatedType { if (_instantiatedType == null) { if (_bound is InterfaceType && @@ -296,13 +316,15 @@ abstract class DefinedElementType extends ElementType { /// 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 visited) { diff --git a/lib/src/html/html_generator.dart b/lib/src/html/html_generator.dart index 29be73c776..43142e9b59 100644 --- a/lib/src/html/html_generator.dart +++ b/lib/src/html/html_generator.dart @@ -6,10 +6,8 @@ library dartdoc.html_generator; import 'dart:async' show Future, StreamController, Stream; import 'dart:io' show Directory, File; -import 'dart:isolate'; import 'package:dartdoc/dartdoc.dart'; -import 'package:dartdoc/src/empty_generator.dart'; import 'package:dartdoc/src/generator.dart'; import 'package:dartdoc/src/html/html_generator_instance.dart'; import 'package:dartdoc/src/html/template_data.dart'; diff --git a/lib/src/html/template_data.dart b/lib/src/html/template_data.dart index 464bff60aa..593e07e928 100644 --- a/lib/src/html/template_data.dart +++ b/lib/src/html/template_data.dart @@ -2,7 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'package:dartdoc/src/render/template_renderer.dart'; import 'package:dartdoc/src/model/model.dart'; abstract class HtmlOptions { diff --git a/lib/src/model/extension.dart b/lib/src/model/extension.dart index 457a2081bf..f32af551b8 100644 --- a/lib/src/model/extension.dart +++ b/lib/src/model/extension.dart @@ -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 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 diff --git a/test/model_test.dart b/test/model_test.dart index 99a21fabb4..5f4efdef7b 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -1878,7 +1878,7 @@ void main() { extensionOnDynamic = getExtension('ExtensionOnDynamic'); extensionOnNull = getExtension('ExtensionOnNull'); extensionOnVoid = getExtension('ExtensionOnVoid'); - extensionOnVoid = getExtension('ExtensionOnTypeParameter'); + extensionOnTypeParameter = getExtension('ExtensionOnTypeParameter'); expect(extensionOnDynamic.couldApplyTo(object), isTrue); expect(extensionOnVoid.couldApplyTo(object), isTrue); diff --git a/test/src/utils.dart b/test/src/utils.dart index b5d8a793df..8fbdf9f4d5 100644 --- a/test/src/utils.dart +++ b/test/src/utils.dart @@ -10,7 +10,6 @@ import 'dart:io'; import 'package:async/async.dart'; import 'package:dartdoc/dartdoc.dart'; -import 'package:dartdoc/src/html/html_generator.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/package_meta.dart'; import 'package:path/path.dart' as path; From 69db87813ed6acee2d5ab32417c7c78c8cf836e8 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 16 Jan 2020 14:20:31 -0800 Subject: [PATCH 3/5] Remove TODO that was implemented --- lib/src/element_type.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 9491c5b25b..26976be26c 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -228,7 +228,6 @@ class TypeParameterElementType extends DefinedElementType { ClassElement get _boundClassElement => type.element; @override - // TODO(jcollins-g): This is wrong; bound is not always an InterfaceType. DartType get _bound => (type as TypeParameterType).bound; } From 7fab9737a3feeb80e41ffd3761242e38624151fb Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 16 Jan 2020 14:29:51 -0800 Subject: [PATCH 4/5] Revert accidentally included change from #2130 --- lib/src/package_meta.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/package_meta.dart b/lib/src/package_meta.dart index 19d922d567..c6ba1bb66c 100644 --- a/lib/src/package_meta.dart +++ b/lib/src/package_meta.dart @@ -317,8 +317,7 @@ class _FilePackageMeta extends PackageMeta { @override bool get requiresFlutter => - _pubspec['environment']?.containsKey('flutter') == true || - _pubspec['dependencies']?.containsKey('flutter') == true; + _pubspec['environment']?.containsKey('flutter') == true; @override FileContents getReadmeContents() { From 9205293862f42de4177ac91aa4dd80f8479d4bda Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 21 Jan 2020 08:11:01 -0800 Subject: [PATCH 5/5] Review comments --- lib/src/element_type.dart | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 26976be26c..f8118c784d 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -71,7 +71,7 @@ abstract class ElementType extends Privacy { String get linkedName; - String get name => type.element.name; + String get name; String get nameWithGenerics; @@ -244,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. @@ -305,6 +308,8 @@ abstract class DefinedElementType extends ElementType { .typeArguments .every((t) => t is InterfaceType)) { var typeSystem = library.element.typeSystem as TypeSystemImpl; + // TODO(jcollins-g): convert to ClassElement.instantiateToBounds + // dart-lang/dartdoc#2135 _instantiatedType = typeSystem.instantiateToBounds(_bound); } else { _instantiatedType = _bound;