diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index 012342fecd..0fd67e918c 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -166,6 +166,10 @@ class Class extends Container return '${package.baseHref}$filePath'; } + /// Returns the [Class] with the library in which [element] is defined. + Class get definingClass => + ModelElement.from(element, definingLibrary, packageGraph); + /// Returns all the "immediate" public implementors of this class. /// /// If this class has a private implementor, then that is counted as a proxy diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index cf3e1641de..0d33d87c7d 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:collection'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; @@ -79,7 +80,7 @@ class PackageGraph { for (var package in documentedPackages) { package.libraries.sort((a, b) => compareNatural(a.name, b.name)); for (var library in package.libraries) { - library.allClasses.forEach(_addToImplementors); + _addToImplementors(library.allClasses); _extensions.addAll(library.extensions); } } @@ -202,7 +203,9 @@ class PackageGraph { allInheritableElements = {}; /// A mapping of the list of classes which implement each class. - final Map> _implementors = {}; + final Map> _implementors = LinkedHashMap( + equals: (Class a, Class b) => a.definingClass == b.definingClass, + hashCode: (Class class_) => class_.definingClass.hashCode); /// A list of extensions that exist in the package graph. final List _extensions = []; @@ -576,26 +579,46 @@ class PackageGraph { return hrefMap; } - void _addToImplementors(Class class_) { + void _addToImplementors(Iterable classes) { assert(!allImplementorsAdded); - _implementors.putIfAbsent(class_, () => []); - void checkAndAddClass(Class key) { - _implementors.putIfAbsent(key, () => []); - var list = _implementors[key]; - if (!list.any((l) => l.element == class_.element)) { - list.add(class_); + // Private classes may not be included in [classes], but may still be + // necessary links in the implementation chain. They are added here as they + // are found, then processed after [classes]. + var privates = []; + + void checkAndAddClass(Class implemented, Class implementor) { + if (!implemented.isPublic) { + privates.add(implemented); + } + implemented = implemented.canonicalModelElement ?? implemented; + _implementors.putIfAbsent(implemented, () => []); + var list = _implementors[implemented]; + // TODO(srawlins): This would be more efficient if we created a + // SplayTreeSet keyed off of `.element`. + if (!list.any((l) => l.element == implementor.element)) { + list.add(implementor); } } - for (var type in class_.mixins) { - checkAndAddClass(type.element); - } - if (class_.supertype != null) { - checkAndAddClass(class_.supertype.element); + void addImplementor(Class class_) { + for (var type in class_.mixins) { + checkAndAddClass(type.element, class_); + } + if (class_.supertype != null) { + checkAndAddClass(class_.supertype.element, class_); + } + for (var type in class_.interfaces) { + checkAndAddClass(type.element, class_); + } } - for (var type in class_.interfaces) { - checkAndAddClass(type.element); + + classes.forEach(addImplementor); + + // [privates] may grow while processing; use a for loop, rather than a + // for-each loop, to avoid concurrent modification errors. + for (var i = 0; i < privates.length; i++) { + addImplementor(privates[i]); } } diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index ea102afb4c..d116a5af7c 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -3582,7 +3582,7 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, }); test('private classes do not break the implementor chain', () { - var Super1 = fakeLibrary.classes.firstWhere((c) => c.name == 'Super1'); + var Super1 = fakeLibrary.classes.singleWhere((c) => c.name == 'Super1'); var publicImplementors = Super1.publicImplementors.map((i) => i.name); expect(publicImplementors, hasLength(3)); // A direct implementor. @@ -3593,6 +3593,33 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, expect(publicImplementors, contains('Super6')); }); + test( + 'private classes in internal libraries do not break the implementor chain', + () { + var GenericSuperProperty = fakeLibrary.classes + .singleWhere((c) => c.name == 'GenericSuperProperty'); + var publicImplementors = + GenericSuperProperty.publicImplementors.map((i) => i.name); + expect(publicImplementors, hasLength(1)); + // A direct implementor. + expect(publicImplementors, contains('GenericSuperValue')); + + var GenericSuperValue = + fakeLibrary.classes.singleWhere((c) => c.name == 'GenericSuperValue'); + publicImplementors = + GenericSuperValue.publicImplementors.map((i) => i.name); + expect(publicImplementors, hasLength(1)); + // A direct implementor. + expect(publicImplementors, contains('GenericSuperNum')); + + var GenericSuperNum = + fakeLibrary.classes.singleWhere((c) => c.name == 'GenericSuperNum'); + publicImplementors = + GenericSuperNum.publicImplementors.map((i) => i.name); + expect(publicImplementors, hasLength(1)); + expect(publicImplementors, contains('GenericSuperInt')); + }); + test('the first class is Apple', () { expect(apple.name, equals('Apple')); }); diff --git a/testing/test_package/lib/src/tool.dart b/testing/test_package/lib/src/tool.dart index 4cdf0a3118..d08a33dd0d 100644 --- a/testing/test_package/lib/src/tool.dart +++ b/testing/test_package/lib/src/tool.dart @@ -8,3 +8,13 @@ abstract class PrivateLibraryToolUser { /// {@end-tool} void invokeToolPrivateLibrary(); } + +abstract class GenericSuperProperty {} + +abstract class GenericSuperValue extends GenericSuperProperty {} + +class _GenericSuper extends GenericSuperValue {} + +class GenericSuperNum extends _GenericSuper {} + +class GenericSuperInt extends GenericSuperNum {}