From b2663eadb4338588ad3f1705a53cdcb45ea8b45f Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 2 Jan 2020 15:30:51 -0800 Subject: [PATCH 1/4] Fix FunctionTypeImpl crash --- lib/src/element_type.dart | 17 +++++++++++------ lib/src/model/class.dart | 20 +++----------------- lib/src/model/extension.dart | 11 +++++++---- lib/src/model/extension_target.dart | 26 ++++++++++++++++++++++++++ lib/src/model/package_graph.dart | 10 +++++++++- testing/test_package/lib/fake.dart | 15 +++++++++++++++ 6 files changed, 71 insertions(+), 28 deletions(-) create mode 100644 lib/src/model/extension_target.dart diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 3e9e618f8a..a5ed703eb5 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -205,10 +205,10 @@ class TypeParameterElementType extends DefinedElementType { String get nameWithGenerics => name; @override - ClassElement get _boundClassElement => interfaceType.element; + ClassElement get _boundClassElement => type.element; @override - InterfaceType get interfaceType => (type as TypeParameterType).bound; + InterfaceType get _interfaceType => (type as TypeParameterType).bound; } /// An [ElementType] associated with an [Element]. @@ -271,18 +271,19 @@ abstract class DefinedElementType extends ElementType { ClassElement get _boundClassElement => (element.element as ClassElement); Class get boundClass => ModelElement.fromElement(_boundClassElement, packageGraph); - InterfaceType get interfaceType => type; + + InterfaceType get _interfaceType => type; InterfaceType _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 (!_interfaceType.typeArguments.every((t) => t is InterfaceType)) { var typeSystem = library.element.typeSystem as TypeSystemImpl; - _instantiatedType = typeSystem.instantiateToBounds(interfaceType); + _instantiatedType = typeSystem.instantiateToBounds(_interfaceType); } else { - _instantiatedType = interfaceType; + _instantiatedType = _interfaceType; } } return _instantiatedType; @@ -413,4 +414,8 @@ class CallableGenericTypeAliasElementType extends ParameterizedElementType } return _returnType; } + + /// Return this type, instantiated to bounds if it isn't already. + @override + DartType get instantiatedType => type; } diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index a647d29c48..cf9d1d5e6c 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -5,12 +5,13 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:dartdoc/src/element_type.dart'; +import 'package:dartdoc/src/model/extension_target.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/model_utils.dart' as model_utils; import 'package:quiver/iterables.dart' as quiver; class Class extends Container - with TypeParameters, Categorization + with TypeParameters, Categorization, ExtensionTarget implements EnclosedElement { List mixins; DefinedElementType supertype; @@ -51,22 +52,6 @@ class Class extends Container return _defaultConstructor; } - bool get hasPotentiallyApplicableExtensions => - potentiallyApplicableExtensions.isNotEmpty; - - List _potentiallyApplicableExtensions; - - Iterable get potentiallyApplicableExtensions { - if (_potentiallyApplicableExtensions == null) { - _potentiallyApplicableExtensions = model_utils - .filterNonDocumented(packageGraph.extensions) - .where((e) => e.couldApplyTo(this)) - .toList(growable: false) - ..sort(byName); - } - return _potentiallyApplicableExtensions; - } - Iterable get allInstanceMethods => quiver.concat([instanceMethods, inheritedMethods]); @@ -223,6 +208,7 @@ class Class extends Container bool get hasPublicMixins => publicMixins.isNotEmpty; + @override bool get hasModifiers => hasPublicMixins || hasAnnotations || diff --git a/lib/src/model/extension.dart b/lib/src/model/extension.dart index dd9bd06968..546eb08585 100644 --- a/lib/src/model/extension.dart +++ b/lib/src/model/extension.dart @@ -8,6 +8,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/src/dart/element/element.dart'; import 'package:dartdoc/src/element_type.dart'; +import 'package:dartdoc/src/model/extension_target.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:quiver/iterables.dart' as quiver; @@ -24,7 +25,8 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - bool couldApplyTo(Class c) => _couldApplyTo(c.modelType); + bool couldApplyTo(T c) => + _couldApplyTo(c.modelType); /// Return true if this extension could apply to [t]. bool _couldApplyTo(DefinedElementType t) { @@ -40,13 +42,14 @@ class Extension extends Container .isSubtypeOf(extendedType.instantiatedType, t.instantiatedType); bool isBoundSupertypeTo(DefinedElementType t) => - _isBoundSupertypeTo(t.type, HashSet()); + _isBoundSupertypeTo(t.instantiatedType, HashSet()); /// Returns true if at least one supertype (including via mixins and /// interfaces) is equivalent to or a subtype of [extendedType] when /// instantiated to bounds. - bool _isBoundSupertypeTo( - InterfaceType superType, HashSet visited) { + bool _isBoundSupertypeTo(DartType superType, HashSet visited) { + // Only InterfaceTypes can have superTypes. + if (superType is! InterfaceType) return false; ClassElement superClass = superType?.element; if (visited.contains(superType)) return false; visited.add(superType); diff --git a/lib/src/model/extension_target.dart b/lib/src/model/extension_target.dart new file mode 100644 index 0000000000..9930f39071 --- /dev/null +++ b/lib/src/model/extension_target.dart @@ -0,0 +1,26 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// 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/model/model.dart'; + +// TODO(jcollins-g): Mix-in ExtensionTarget on Method, ModelFunction, Typedef, +// and other possible documented symbols that could be extended. +mixin ExtensionTarget on ModelElement { + bool get hasModifiers; + + bool get hasPotentiallyApplicableExtensions => + potentiallyApplicableExtensions.isNotEmpty; + + List _potentiallyApplicableExtensions; + + Iterable get potentiallyApplicableExtensions { + if (_potentiallyApplicableExtensions == null) { + _potentiallyApplicableExtensions = packageGraph.documentedExtensions + .where((e) => e.couldApplyTo(this)) + .toList(growable: false) + ..sort(byName); + } + return _potentiallyApplicableExtensions; + } +} diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 1025e27861..bcb3771c65 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -152,13 +152,21 @@ class PackageGraph { return _implementors; } + List _documentedExtensions; + Iterable get documentedExtensions { + if (_documentedExtensions == null) { + _documentedExtensions = + utils.filterNonDocumented(extensions).toList(growable: false); + } + return _documentedExtensions; + } + Iterable get extensions { assert(allExtensionsAdded); return _extensions; } Map> _findRefElementCache; - Map> get findRefElementCache { if (_findRefElementCache == null) { assert(packageGraph.allLibrariesAdded); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index a195bfb0c1..94ff87e57b 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1117,3 +1117,18 @@ abstract class CanonicalPrivateInheritedToolUser print('hello, tool world'); } } + +/* + * Complex extension methods case. + * + * TODO(jcollins-g): add unit tests around behavior when #2701 is implemented. + * Until #2701 is fixed we mostly are testing that we don't crash because + * DoSomething2X is declared. + */ + +typedef R Function1(A a); +typedef R Function2(A a, B b); + +extension DoSomething2X on Function1> { + Function2 something() => (A first, B second) => this(first)(second); +} \ No newline at end of file From 7511f3cf804c5c367496fa76274409d30fc4e299 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 2 Jan 2020 15:33:38 -0800 Subject: [PATCH 2/4] Add bug number to comment --- lib/src/model/extension_target.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/model/extension_target.dart b/lib/src/model/extension_target.dart index 9930f39071..a88c0e5a59 100644 --- a/lib/src/model/extension_target.dart +++ b/lib/src/model/extension_target.dart @@ -5,7 +5,7 @@ import 'package:dartdoc/src/model/model.dart'; // TODO(jcollins-g): Mix-in ExtensionTarget on Method, ModelFunction, Typedef, -// and other possible documented symbols that could be extended. +// and other possible documented symbols that could be extended (#2701). mixin ExtensionTarget on ModelElement { bool get hasModifiers; From 98254621462771da4584b3d175362e910dbef246 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 2 Jan 2020 15:37:14 -0800 Subject: [PATCH 3/4] Remove unnecessary comment --- 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 a5ed703eb5..39750bdefa 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -415,7 +415,6 @@ class CallableGenericTypeAliasElementType extends ParameterizedElementType return _returnType; } - /// Return this type, instantiated to bounds if it isn't already. @override DartType get instantiatedType => type; } From 365e0349fded06c8bf893550f46108a274e44eb0 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 3 Jan 2020 08:01:54 -0800 Subject: [PATCH 4/4] Add TODO for bad cast --- lib/src/element_type.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 39750bdefa..86df0c75ea 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -208,6 +208,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; }