From 1a155fc67021f9d52f139e7ed3c9d16098251f42 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 29 Oct 2019 15:05:34 -0700 Subject: [PATCH 01/10] Fix discovery --- lib/src/model.dart | 256 +++++++++++++++++++++------------------------ 1 file changed, 118 insertions(+), 138 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index d5b6d623c7..36a3c0c84a 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -38,8 +38,7 @@ import 'package:analyzer/src/dart/element/member.dart' import 'package:analyzer/src/dart/sdk/sdk.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/java_io.dart'; -import 'package:analyzer/src/generated/resolver.dart' - show Namespace, NamespaceBuilder; +import 'package:analyzer/src/generated/resolver.dart' show NamespaceBuilder; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/source_io.dart'; @@ -153,11 +152,14 @@ mixin Extendable on ContainerMember { mixin ContainerMember on ModelElement implements EnclosedElement { /// True if this [ContainerMember] is inherited from a different class. bool get isInherited; + /// True if this [ContainerMember] is overriding a superclass. bool get isOverride; + /// True if this [ContainerMember] has a parameter whose type is overridden /// by a subtype. bool get isCovariant; + /// True if this [ContainerMember] is from an applicable [Extension]. /// False otherwise, including if this [ContainerMember]'s [enclosingElement] /// is the extension it was declared in. @@ -391,8 +393,8 @@ class ContainerAccessor extends Accessor with ContainerMember, Inheritable { PackageGraph packageGraph) : super(element, library, packageGraph, null); - ContainerAccessor.inherited(PropertyAccessorElement element, - Library library, PackageGraph packageGraph, this._enclosingElement, + ContainerAccessor.inherited(PropertyAccessorElement element, Library library, + PackageGraph packageGraph, this._enclosingElement, {Member originalMember}) : super(element, library, packageGraph, originalMember) { _isInherited = true; @@ -539,7 +541,6 @@ class Accessor extends ModelElement implements EnclosedElement { @override bool get isCanonical => enclosingCombo.isCanonical; - @override String get href { return enclosingCombo.href; @@ -999,8 +1000,8 @@ class Class extends Container }).toSet(); for (ExecutableElement e in inheritedMethodElements) { - Method m = - ModelElement.from(e, library, packageGraph, enclosingContainer: this); + Method m = ModelElement.from(e, library, packageGraph, + enclosingContainer: this); _inheritedMethods.add(m); } _inheritedMethods.sort(byName); @@ -1025,8 +1026,8 @@ class Class extends Container !operatorNames.contains(e.name)); }).toSet(); for (ExecutableElement e in inheritedOperatorElements) { - Operator o = - ModelElement.from(e, library, packageGraph, enclosingContainer: this); + Operator o = ModelElement.from(e, library, packageGraph, + enclosingContainer: this); _inheritedOperators.add(o); } _inheritedOperators.sort(byName); @@ -1357,7 +1358,8 @@ class Extension extends Container if (f.setter != null) { setter = ContainerAccessor(f.setter, library, packageGraph); } - return ModelElement.from(f, library, packageGraph, getter: getter, setter: setter) as Field; + return ModelElement.from(f, library, packageGraph, + getter: getter, setter: setter) as Field; }).toList(growable: false) ..sort(byName); @@ -2347,7 +2349,7 @@ class _HashableChildLibraryElementVisitor class Library extends ModelElement with Categorization, TopLevelContainer { List _variables; - Namespace _exportedNamespace; + List _exportedAndLocalElements; String _name; factory Library(LibraryElement element, PackageGraph packageGraph) { @@ -2367,11 +2369,38 @@ class Library extends ModelElement with Categorization, TopLevelContainer { _HashableChildLibraryElementVisitor((Element e) => packageGraph._populateModelNodeFor(e, _compilationUnitMap)) .visitElement(element); - _exportedNamespace = - NamespaceBuilder().createExportNamespaceForLibrary(element); + + // Initialize the list of elements defined in this library and + // exported via its export directives. + Set exportedAndLocalElements = NamespaceBuilder() + .createExportNamespaceForLibrary(element) + .definedNames + .values + .toSet(); + exportedAndLocalElements + .addAll(getDefinedElements(_libraryElement.definingCompilationUnit)); + for (CompilationUnitElement cu in _libraryElement.parts) { + exportedAndLocalElements.addAll(getDefinedElements(cu)); + } + _exportedAndLocalElements = exportedAndLocalElements.toList(); + _package._allLibraries.add(this); } + static Iterable getDefinedElements( + CompilationUnitElement compilationUnit) { + return quiver.concat([ + compilationUnit.accessors, + compilationUnit.enums, + compilationUnit.extensions, + compilationUnit.functions, + compilationUnit.functionTypeAliases, + compilationUnit.mixins, + compilationUnit.topLevelVariables, + compilationUnit.types, + ]); + } + List _allOriginalModelElementNames; final Package _package; @@ -2418,14 +2447,16 @@ class Library extends ModelElement with Categorization, TopLevelContainer { @override CharacterLocation get characterLocation { if (element.nameOffset == -1) { - assert(isAnonymous, 'Only anonymous libraries are allowed to have no declared location'); + assert(isAnonymous, + 'Only anonymous libraries are allowed to have no declared location'); return CharacterLocation(1, 1); } return super.characterLocation; } @override - CompilationUnitElement get compilationUnitElement => (element as LibraryElement).definingCompilationUnit; + CompilationUnitElement get compilationUnitElement => + (element as LibraryElement).definingCompilationUnit; @override Iterable get classes => allClasses.where((c) => !c.isErrorOrException); @@ -2433,24 +2464,11 @@ class Library extends ModelElement with Categorization, TopLevelContainer { @override Iterable get extensions { if (_extensions == null) { - // De-dupe extensions coming from multiple exported libraries at once. - Set extensionElements = Set(); - extensionElements.addAll(_libraryElement.definingCompilationUnit.extensions); - for (CompilationUnitElement cu in _libraryElement.parts) { - extensionElements.addAll(cu.extensions); - } - for (LibraryElement le in _libraryElement.exportedLibraries) { - extensionElements.addAll(le.definingCompilationUnit.extensions - .where((t) => _exportedNamespace.definedNames.values.contains(t.name))); - } - - extensionElements.addAll(_exportedNamespace.definedNames.values - .whereType()); - - _extensions = extensionElements + _extensions = _exportedAndLocalElements + .whereType() .map((e) => ModelElement.from(e, this, packageGraph) as Extension) .toList(growable: false) - ..sort(byName); + ..sort(byName); } return _extensions; } @@ -2605,40 +2623,37 @@ class Library extends ModelElement with Categorization, TopLevelContainer { @override List get enums { - if (_enums != null) return _enums; - List enumClasses = []; - enumClasses.addAll(_exportedNamespace.definedNames.values - .whereType() - .where((element) => element.isEnum)); - _enums = enumClasses - .map((e) => ModelElement.from(e, this, packageGraph) as Enum) - .toList(growable: false) - ..sort(byName); - + if (_enums == null) { + _enums = _exportedAndLocalElements + .whereType() + .where((element) => element.isEnum) + .map((e) => ModelElement.from(e, this, packageGraph) as Enum) + .toList(growable: false) + ..sort(byName); + } return _enums; } @override List get mixins { - if (_mixins != null) return _mixins; - - /// Can not be [MixinElementImpl] because [ClassHandle]s are sometimes - /// returned from _exportedNamespace. - List mixinClasses = []; - mixinClasses.addAll(_exportedNamespace.definedNames.values - .whereType() - .where((ClassElement c) => c.isMixin)); - _mixins = mixinClasses - .map((e) => ModelElement.from(e, this, packageGraph) as Mixin) - .toList(growable: false) - ..sort(byName); + if (_mixins == null) { + /// Can not be [MixinElementImpl] because [ClassHandle]s are sometimes + /// returned from _exportedElements. + _mixins = _exportedAndLocalElements + .whereType() + .where((ClassElement c) => c.isMixin) + .map((e) => ModelElement.from(e, this, packageGraph) as Mixin) + .toList(growable: false) + ..sort(byName); + } return _mixins; } @override List get exceptions { if (_exceptions == null) { - _exceptions = allClasses.where((c) => c.isErrorOrException).toList(growable: false); + _exceptions = + allClasses.where((c) => c.isErrorOrException).toList(growable: false); } return _exceptions; } @@ -2648,21 +2663,13 @@ class Library extends ModelElement with Categorization, TopLevelContainer { @override List get functions { - if (_functions != null) return _functions; - - Set elements = Set(); - elements.addAll(_libraryElement.definingCompilationUnit.functions); - for (CompilationUnitElement cu in _libraryElement.parts) { - elements.addAll(cu.functions); + if (_functions == null) { + _functions = + _exportedAndLocalElements.whereType().map((e) { + return ModelElement.from(e, this, packageGraph) as ModelFunction; + }).toList(growable: false) + ..sort(byName); } - elements.addAll( - _exportedNamespace.definedNames.values.whereType()); - - _functions = elements.map((e) { - return ModelElement.from(e, this, packageGraph) as ModelFunction; - }).toList(growable: false) - ..sort(byName); - return _functions; } @@ -2742,49 +2749,25 @@ class Library extends ModelElement with Categorization, TopLevelContainer { @override List get typedefs { - if (_typedefs != null) return _typedefs; - - Set elements = Set(); - elements - .addAll(_libraryElement.definingCompilationUnit.functionTypeAliases); - for (CompilationUnitElement cu in _libraryElement.parts) { - elements.addAll(cu.functionTypeAliases); + if (_typedefs == null) { + _typedefs = _exportedAndLocalElements + .whereType() + .map((e) => ModelElement.from(e, this, packageGraph) as Typedef) + .toList(growable: false) + ..sort(byName); } - - elements.addAll(_exportedNamespace.definedNames.values - .whereType()); - _typedefs = elements - .map((e) => ModelElement.from(e, this, packageGraph) as Typedef) - .toList(growable: false) - ..sort(byName); - return _typedefs; } List get allClasses { - if (_classes != null) return _classes; - - // De-dupe classes coming from multiple exported libraries at once. - Set types = Set(); - types.addAll(_libraryElement.definingCompilationUnit.types); - for (CompilationUnitElement cu in _libraryElement.parts) { - types.addAll(cu.types); - } - for (LibraryElement le in _libraryElement.exportedLibraries) { - types.addAll(le.definingCompilationUnit.types - .where((t) => _exportedNamespace.definedNames.values.contains(t.name))); + if (_classes == null) { + _classes = _exportedAndLocalElements + .whereType() + .where((e) => !e.isMixin && !e.isEnum) + .map((e) => ModelElement.from(e, this, packageGraph) as Class) + .toList(growable: false) + ..sort(byName); } - - types.addAll(_exportedNamespace.definedNames.values - .whereType() - .where((e) => !e.isMixin && !e.isEnum)); - - _classes = types - .map((e) => ModelElement.from(e, this, packageGraph) as Class) - .toList(growable: false) - ..sort(byName); - - assert(!_classes.any((Class c) => c is Mixin)); return _classes; } @@ -2795,34 +2778,29 @@ class Library extends ModelElement with Categorization, TopLevelContainer { } List _getVariables() { - if (_variables != null) return _variables; - - Set elements = Set(); - elements.addAll(_libraryElement.definingCompilationUnit.topLevelVariables); - for (CompilationUnitElement cu in _libraryElement.parts) { - elements.addAll(cu.topLevelVariables); - } - _exportedNamespace.definedNames.values.forEach((element) { - if (element is PropertyAccessorElement) { - elements.add(element.variable); - } - }); - _variables = []; - for (TopLevelVariableElement element in elements) { - Accessor getter; - if (element.getter != null) { - getter = ModelElement.from(element.getter, this, packageGraph); - } - Accessor setter; - if (element.setter != null) { - setter = ModelElement.from(element.setter, this, packageGraph); + if (_variables == null) { + Set elements = _exportedAndLocalElements + .whereType() + .toSet(); + elements.addAll(_exportedAndLocalElements + .whereType() + .map((a) => a.variable)); + _variables = []; + for (TopLevelVariableElement element in elements) { + Accessor getter; + if (element.getter != null) { + getter = ModelElement.from(element.getter, this, packageGraph); + } + Accessor setter; + if (element.setter != null) { + setter = ModelElement.from(element.setter, this, packageGraph); + } + ModelElement me = ModelElement.from(element, this, packageGraph, + getter: getter, setter: setter); + _variables.add(me); } - ModelElement me = ModelElement.from(element, this, packageGraph, - getter: getter, setter: setter); - _variables.add(me); + _variables.sort(byName); } - - _variables.sort(byName); return _variables; } @@ -2973,8 +2951,8 @@ class Method extends ModelElement _calcTypeParameters(); } - Method.inherited(MethodElement element, this._enclosingContainer, Library library, - PackageGraph packageGraph, + Method.inherited(MethodElement element, this._enclosingContainer, + Library library, PackageGraph packageGraph, {Member originalMember}) : super(element, library, packageGraph, originalMember) { _isInherited = true; @@ -3835,8 +3813,10 @@ abstract class ModelElement extends Canonicalization if (!_characterLocationIsSet) { LineInfo lineInfo = compilationUnitElement.lineInfo; _characterLocationIsSet = true; - assert(element.nameOffset >= 0, 'Invalid location data for element: $fullyQualifiedName'); - assert(lineInfo != null, 'No lineInfo data available for element: $fullyQualifiedName'); + assert(element.nameOffset >= 0, + 'Invalid location data for element: $fullyQualifiedName'); + assert(lineInfo != null, + 'No lineInfo data available for element: $fullyQualifiedName'); if (element.nameOffset >= 0) { _characterLocation = lineInfo?.getLocation(element.nameOffset); } @@ -3844,7 +3824,8 @@ abstract class ModelElement extends Canonicalization return _characterLocation; } - CompilationUnitElement get compilationUnitElement => element.getAncestor((e) => e is CompilationUnitElement); + CompilationUnitElement get compilationUnitElement => + element.getAncestor((e) => e is CompilationUnitElement); bool get hasAnnotations => annotations.isNotEmpty; @@ -5347,8 +5328,7 @@ class PackageGraph { List messageParts = [warningMessage]; if (warnable != null) { - messageParts - .add("$warnablePrefix $warnableName: ${warnable.location}"); + messageParts.add("$warnablePrefix $warnableName: ${warnable.location}"); } if (referredFrom != null) { for (Locatable referral in referredFrom) { From 3604602e5ea43f7b19bf6575b343d50f00e1c514 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 29 Oct 2019 15:30:33 -0700 Subject: [PATCH 02/10] Add tests --- test/dartdoc_integration_test.dart | 15 +-- test/dartdoc_test.dart | 4 +- test/model_test.dart | 109 ++++++++++++------ testing/test_package/lib/reexport_three.dart | 6 + testing/test_package/lib/reexport_two.dart | 4 +- testing/test_package/lib/src/shadow_lib.dart | 7 ++ .../test_package/lib/src/shadowing_lib.dart | 7 ++ testing/test_package/lib/src/somelib.dart | 1 - 8 files changed, 102 insertions(+), 51 deletions(-) create mode 100644 testing/test_package/lib/reexport_three.dart create mode 100644 testing/test_package/lib/src/shadow_lib.dart create mode 100644 testing/test_package/lib/src/shadowing_lib.dart diff --git a/test/dartdoc_integration_test.dart b/test/dartdoc_integration_test.dart index bc289d041a..1007c41a34 100644 --- a/test/dartdoc_integration_test.dart +++ b/test/dartdoc_integration_test.dart @@ -232,26 +232,21 @@ void main() { }, timeout: Timeout.factor(2)); test('--footer-text excludes version', () async { - String _testPackagePath = - path.fromUri(_currentFileUri.resolve('../testing/test_package_options')); + String _testPackagePath = path + .fromUri(_currentFileUri.resolve('../testing/test_package_options')); - var args = [ - dartdocPath, - '--output', - tempDir.path - ]; + var args = [dartdocPath, '--output', tempDir.path]; await subprocessLauncher.runStreamed(Platform.resolvedExecutable, args, workingDirectory: _testPackagePath); File outFile = File(path.join(tempDir.path, 'index.html')); - RegExp footerRegex = RegExp('
(.*\s*?\n?)+?
', multiLine: true); + RegExp footerRegex = + RegExp('
(.*\s*?\n?)+?
', multiLine: true); // get footer, check for version number RegExpMatch m = footerRegex.firstMatch(outFile.readAsStringSync()); RegExp version = RegExp(r'(\d+\.)?(\d+\.)?(\*|\d+)'); expect(version.hasMatch(m.group(0)), false); }); - }, timeout: Timeout.factor(4)); } - diff --git a/test/dartdoc_test.dart b/test/dartdoc_test.dart index 078d3e8d70..d049533348 100644 --- a/test/dartdoc_test.dart +++ b/test/dartdoc_test.dart @@ -258,7 +258,7 @@ void main() { expect(p.name, 'test_package'); expect(p.hasDocumentationFile, isTrue); // Total number of public libraries in test_package. - expect(packageGraph.defaultPackage.publicLibraries, hasLength(15)); + expect(packageGraph.defaultPackage.publicLibraries, hasLength(16)); expect(packageGraph.localPackages.length, equals(1)); }); @@ -327,7 +327,7 @@ void main() { PackageGraph p = results.packageGraph; expect(p.defaultPackage.name, 'test_package'); expect(p.defaultPackage.hasDocumentationFile, isTrue); - expect(p.localPublicLibraries, hasLength(14)); + expect(p.localPublicLibraries, hasLength(15)); expect(p.localPublicLibraries.map((lib) => lib.name).contains('fake'), isFalse); }); diff --git a/test/model_test.dart b/test/model_test.dart index c2a076e218..42317ce34b 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -494,7 +494,7 @@ void main() { expect( packageGraph .localPackages.first.defaultCategory.publicLibraries.length, - equals(8)); + equals(9)); }); test('Verify libraries with multiple categories show up in multiple places', @@ -518,7 +518,7 @@ void main() { expect( packageGraph .localPackages.first.defaultCategory.publicLibraries.length, - equals(8)); + equals(9)); }); }); @@ -588,7 +588,7 @@ void main() { }); test('libraries', () { - expect(packageGraph.localPublicLibraries, hasLength(13)); + expect(packageGraph.localPublicLibraries, hasLength(14)); expect(interceptorsLib.isPublic, isFalse); }); @@ -603,7 +603,7 @@ void main() { Package package = packageGraph.localPackages.first; expect(package.name, 'test_package'); - expect(package.publicLibraries, hasLength(13)); + expect(package.publicLibraries, hasLength(14)); }); test('multiple packages, sorted default', () { @@ -696,7 +696,8 @@ void main() { // If EventTarget really does start implementing hashCode, this will // fail. expect(hashCode.href, equals('dart-core/Object/hashCode.html')); - expect(hashCode.canonicalEnclosingContainer, equals(objectModelElement)); + expect( + hashCode.canonicalEnclosingContainer, equals(objectModelElement)); expect( EventTarget.publicSuperChainReversed .any((et) => et.name == 'Interceptor'), @@ -720,13 +721,17 @@ void main() { isDeprecated, someLib, reexportOneLib, - reexportTwoLib; - Class SomeClass, SomeOtherClass, YetAnotherClass, AUnicornClass; + reexportTwoLib, + reexportThreeLib; + Class SomeClass, + SomeOtherClass, + YetAnotherClass, + AUnicornClass, + ADuplicateClass; setUpAll(() { dartAsyncLib = utils.testPackageGraphSdk.libraries .firstWhere((l) => l.name == 'dart:async'); - anonLib = packageGraph.libraries .firstWhere((lib) => lib.name == 'anonymous_library'); @@ -736,10 +741,13 @@ void main() { .firstWhere((lib) => lib.name == 'reexport_one'); reexportTwoLib = packageGraph.libraries .firstWhere((lib) => lib.name == 'reexport_two'); + reexportThreeLib = packageGraph.libraries + .firstWhere((lib) => lib.name == 'reexport_three'); SomeClass = someLib.getClassByName('SomeClass'); SomeOtherClass = someLib.getClassByName('SomeOtherClass'); YetAnotherClass = someLib.getClassByName('YetAnotherClass'); AUnicornClass = someLib.getClassByName('AUnicornClass'); + ADuplicateClass = reexportThreeLib.getClassByName('ADuplicateClass'); isDeprecated = packageGraph.libraries .firstWhere((lib) => lib.name == 'is_deprecated'); @@ -871,6 +879,10 @@ void main() { expect(SomeOtherClass.canonicalLibrary, reexportOneLib); expect(SomeClass.canonicalLibrary, reexportTwoLib); }); + + test('with correct show/hide behavior', () { + expect(ADuplicateClass.definingLibrary.name, equals('shadowing_lib')); + }); }); group('Macros', () { @@ -2125,40 +2137,62 @@ void main() { .firstWhere((lib) => lib.name == 'reexport_one'); reexportTwoLib = packageGraph.libraries .firstWhere((lib) => lib.name == 'reexport_two'); - documentOnceReexportOne = reexportOneLib.extensions.firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); - documentOnceReexportTwo = reexportTwoLib.extensions.firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); + documentOnceReexportOne = reexportOneLib.extensions + .firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); + documentOnceReexportTwo = reexportTwoLib.extensions + .firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); ext = exLibrary.extensions.firstWhere((e) => e.name == 'AppleExtension'); - extensionReferencer = exLibrary.classes.firstWhere((c) => c.name == 'ExtensionReferencer'); + extensionReferencer = + exLibrary.classes.firstWhere((c) => c.name == 'ExtensionReferencer'); fancyList = exLibrary.extensions.firstWhere((e) => e.name == 'FancyList'); - doSomeStuff = exLibrary.classes.firstWhere((c) => c.name == 'ExtensionUser') - .allInstanceMethods.firstWhere((m) => m.name == 'doSomeStuff'); - doStuff = exLibrary.extensions.firstWhere((e) => e.name == 'SimpleStringExtension') - .instanceMethods.firstWhere((m) => m.name == 'doStuff'); + doSomeStuff = exLibrary.classes + .firstWhere((c) => c.name == 'ExtensionUser') + .allInstanceMethods + .firstWhere((m) => m.name == 'doSomeStuff'); + doStuff = exLibrary.extensions + .firstWhere((e) => e.name == 'SimpleStringExtension') + .instanceMethods + .firstWhere((m) => m.name == 'doStuff'); extensions = exLibrary.publicExtensions.toList(); }); test('basic canonicalization for extensions', () { expect(documentOnceReexportOne.isCanonical, isFalse); - expect(documentOnceReexportOne.href, equals(documentOnceReexportTwo.href)); + expect( + documentOnceReexportOne.href, equals(documentOnceReexportTwo.href)); expect(documentOnceReexportTwo.isCanonical, isTrue); }); // TODO(jcollins-g): implement feature and update tests test('documentation links do not crash in base cases', () { - packageGraph.packageWarningCounter.hasWarning(doStuff, PackageWarning.notImplemented, + + packageGraph.packageWarningCounter.hasWarning( + doStuff, + PackageWarning.notImplemented, 'Comment reference resolution inside extension methods is not yet implemented'); - packageGraph.packageWarningCounter.hasWarning(doSomeStuff, PackageWarning.notImplemented, + packageGraph.packageWarningCounter.hasWarning( + doSomeStuff, + PackageWarning.notImplemented, 'Comment reference resolution inside extension methods is not yet implemented'); expect(doStuff.documentationAsHtml, contains('another')); - expect(doSomeStuff.documentationAsHtml, contains('String.extensionNumber')); + expect(doSomeStuff.documentationAsHtml, + contains('String.extensionNumber')); }); - test('references from outside an extension refer correctly to the extension', () { - expect(extensionReferencer.documentationAsHtml, contains('_Shhh')); - expect(extensionReferencer.documentationAsHtml, contains('FancyList')); - expect(extensionReferencer.documentationAsHtml, contains('AnExtension.call')); - expect(extensionReferencer.documentationAsHtml, contains('DocumentThisExtensionOnce')); + test( + 'references from outside an extension refer correctly to the extension', + () { + expect(extensionReferencer.documentationAsHtml, + contains('_Shhh')); + expect(extensionReferencer.documentationAsHtml, + contains('FancyList')); + expect(extensionReferencer.documentationAsHtml, + contains('AnExtension.call')); + expect( + extensionReferencer.documentationAsHtml, + contains( + 'DocumentThisExtensionOnce')); }); test('has a fully qualified name', () { @@ -2190,10 +2224,8 @@ void main() { }); test('extended type has generics', () { - expect( - fancyList.extendedType.nameWithGenerics, - equals( - 'List<Z>')); + expect(fancyList.extendedType.nameWithGenerics, + equals('List<Z>')); }); test('get methods', () { @@ -2231,7 +2263,8 @@ void main() { setUpAll(() { animal = exLibrary.enums.firstWhere((e) => e.name == 'Animal'); - animalToString = animal.allInstanceMethods.firstWhere((m) => m.name == 'toString'); + animalToString = + animal.allInstanceMethods.firstWhere((m) => m.name == 'toString'); /// Trigger code reference resolution animal.documentationAsHtml; @@ -2876,11 +2909,11 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, }); test('Fields always have line and column information', () { - expect(implicitGetterExplicitSetter.characterLocation, isNotNull); - expect(explicitGetterImplicitSetter.characterLocation, isNotNull); - expect(explicitGetterSetter.characterLocation, isNotNull); - expect(constField.characterLocation, isNotNull); - expect(aProperty.characterLocation, isNotNull); + expect(implicitGetterExplicitSetter.characterLocation, isNotNull); + expect(explicitGetterImplicitSetter.characterLocation, isNotNull); + expect(explicitGetterSetter.characterLocation, isNotNull); + expect(constField.characterLocation, isNotNull); + expect(aProperty.characterLocation, isNotNull); }); test('covariant fields are recognized', () { @@ -3387,7 +3420,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, Constructor appleConstructorFromString; Constructor constructorTesterDefault, constructorTesterFromSomething; Constructor syntheticConstructor; - Class apple, constCat, constructorTester, referToADefaultConstructor, + Class apple, + constCat, + constructorTester, + referToADefaultConstructor, withSyntheticConstructor; setUpAll(() { apple = exLibrary.classes.firstWhere((c) => c.name == 'Apple'); @@ -3405,7 +3441,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, .firstWhere((c) => c.name == 'ConstructorTester.fromSomething'); referToADefaultConstructor = fakeLibrary.classes .firstWhere((c) => c.name == 'ReferToADefaultConstructor'); - withSyntheticConstructor = exLibrary.classes.firstWhere((c) => c.name == 'WithSyntheticConstructor'); + withSyntheticConstructor = exLibrary.classes + .firstWhere((c) => c.name == 'WithSyntheticConstructor'); syntheticConstructor = withSyntheticConstructor.defaultConstructor; }); diff --git a/testing/test_package/lib/reexport_three.dart b/testing/test_package/lib/reexport_three.dart new file mode 100644 index 0000000000..10e4b1a399 --- /dev/null +++ b/testing/test_package/lib/reexport_three.dart @@ -0,0 +1,6 @@ +library reexport_three; + +// Test show/hide handling. +export 'src/shadowing_lib.dart' show ADuplicateClass; +// ignore: directives_ordering +export 'src/shadow_lib.dart' hide ADuplicateClass; diff --git a/testing/test_package/lib/reexport_two.dart b/testing/test_package/lib/reexport_two.dart index e922e8aed6..d69e151bdb 100644 --- a/testing/test_package/lib/reexport_two.dart +++ b/testing/test_package/lib/reexport_two.dart @@ -5,6 +5,6 @@ /// {@category Unreal} library reexport_two; -export 'src/somelib.dart'; - +// Intentionally create some duplicates via reexporting. export 'src/mixins.dart' show MixedIn, AMixin; +export 'src/somelib.dart'; diff --git a/testing/test_package/lib/src/shadow_lib.dart b/testing/test_package/lib/src/shadow_lib.dart new file mode 100644 index 0000000000..8afe83d6aa --- /dev/null +++ b/testing/test_package/lib/src/shadow_lib.dart @@ -0,0 +1,7 @@ +library shadow_lib; + +class ADuplicateClass { + bool get aCompletelyDifferentGetter => true; +} + +class SomeOtherClass {} diff --git a/testing/test_package/lib/src/shadowing_lib.dart b/testing/test_package/lib/src/shadowing_lib.dart new file mode 100644 index 0000000000..4e728dc58f --- /dev/null +++ b/testing/test_package/lib/src/shadowing_lib.dart @@ -0,0 +1,7 @@ +library shadowing_lib; + +class ADuplicateClass { + bool get aGetter => true; +} + +class SomeMoreClassDeclaration {} \ No newline at end of file diff --git a/testing/test_package/lib/src/somelib.dart b/testing/test_package/lib/src/somelib.dart index 075745e6f5..697ec1f2b6 100644 --- a/testing/test_package/lib/src/somelib.dart +++ b/testing/test_package/lib/src/somelib.dart @@ -8,7 +8,6 @@ class YetAnotherClass {} class AUnicornClass {} - /// A private extension. extension _Unseen on Object { void doYouSeeMe() { } From fdb7c09c10800e1762bba7127b4900a90f09327a Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 30 Oct 2019 15:25:39 -0700 Subject: [PATCH 03/10] Applicable extensions --- lib/src/model.dart | 98 +++++++++++++++++++++++++++++++++++++++++++- pubspec.yaml | 2 +- test/model_test.dart | 7 +++- 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 36a3c0c84a..0ff0f330aa 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -794,6 +794,16 @@ class Class extends Container return _defaultConstructor; } + List _applicableExtensions; + Iterable get applicableExtensions { + if (_applicableExtensions == null) { + _applicableExtensions = packageGraph.extensions + .where((e) => e.couldApplyTo(this)) + .toList(growable: false); + } + return _applicableExtensions; + } + Iterable get allInstanceMethods => quiver.concat([instanceMethods, inheritedMethods]); @@ -1327,6 +1337,74 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } + /// Returns [true] if this extension in context applies to the given + /// [ModelElement]. + /// + /// It may or may not be the most specific for a given + /// method reference, and this does not guarantee any methods will actually + /// be applicable to, most specific for, and not shadowed for [e]. + bool appliesTo(ModelElement e) => (e.library == library) && couldApplyTo(e); + + /// Returns [true] if the extension could apply to [e] if both are imported, + /// possibly with show/hide. + bool couldApplyTo(ModelElement e) => extendedType.type.isSubtypeOf( + e.modelType.type); + + /// Assuming that [possibleExtensions] contain multiple applicable extensions + /// for the same extension method reference, return the set of extensions + /// most applicable. + static Iterable mostSpecificExtensions( + Iterable possibleExtensions) { + Set returningExtensions = {}; + // The extension at least tied for most specific that we have seen. + Extension mostSpecificSoFar; + for (Extension possible in possibleExtensions) { + if (mostSpecificSoFar == null) { + mostSpecificSoFar = possible; + continue; + } + if (possible._isMoreSpecificThan(mostSpecificSoFar)) { + mostSpecificSoFar = possible; + } + } + + // Collect all extensions tied with the most specific one. + for (Extension possible in possibleExtensions) { + if (!mostSpecificSoFar._isMoreSpecificThan(possible)) { + returningExtensions.add(possible); + } + } + return returningExtensions; + } + + /// Is this extension more specific than [e2]? + /// + /// If false, does not imply that this is less specific than [e2]. + /// + /// From: https://github.com/dart-lang/language/blob/master/accepted/2.6/static-extension-members/feature-specification.md#Specificity + bool _isMoreSpecificThan(Extension e2) { + Extension e1 = this; + DartType e1Type = e1.modelType.type; + DartType e2Type = e2.modelType.type; + DartType e1Resolved = e1Type.resolveToBound(null); + DartType e2Resolved = e2Type.resolveToBound(null); + // 1 + if ((e2.definingLibrary.isInSdk && !e1.definingLibrary.isInSdk) || ( + // 2 + e1.definingLibrary.isInSdk == e2.definingLibrary.isInSdk && ( + // 3 + e1Type.isSubtypeOf(e2Type) && ( + // 4 + !e2Type.isSubtypeOf(e1Type) || ( + // 5 + e1Resolved.isSubtypeOf(e2Resolved) && + !e2Resolved.isSubtypeOf(e1Resolved) + ))))) { + return false; + } + return true; + } + @override ModelElement get enclosingElement => library; @@ -2403,6 +2481,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer { List _allOriginalModelElementNames; + bool get isInSdk => _libraryElement.isInSdk; + final Package _package; @override @@ -5016,10 +5096,13 @@ class PackageGraph { package._libraries.sort((a, b) => compareNatural(a.name, b.name)); package._libraries.forEach((library) { library.allClasses.forEach(_addToImplementors); + _extensions.addAll(library.extensions); }); }); _implementors.values.forEach((l) => l.sort()); + _extensions.sort(byName); allImplementorsAdded = true; + allExtensionsAdded = true; // We should have found all special classes by now. specialClasses.assertSpecials(); @@ -5073,15 +5156,24 @@ class PackageGraph { SpecialClasses specialClasses; - /// It is safe to cache values derived from the _implementors table if this + /// It is safe to cache values derived from the [_implementors] table if this /// is true. bool allImplementorsAdded = false; + /// It is safe to cache values derived from the [_extensions] table if this + /// is true. + bool allExtensionsAdded = false; + Map> get implementors { assert(allImplementorsAdded); return _implementors; } + Iterable get extensions { + assert(allExtensionsAdded); + return _extensions; + } + Map> _findRefElementCache; Map> get findRefElementCache { @@ -5119,6 +5211,10 @@ class PackageGraph { /// Map of Class.href to a list of classes implementing that class final Map> _implementors = Map(); + /// A list of extensions that exist in the package graph. + // TODO(jcollins-g): Consider implementing a smarter structure for this. + final List _extensions = List(); + /// PackageMeta for the default package. final PackageMeta packageMeta; diff --git a/pubspec.yaml b/pubspec.yaml index 7220848213..55ddb94be5 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -5,7 +5,7 @@ author: Dart Team description: A documentation generator for Dart. homepage: https://github.com/dart-lang/dartdoc environment: - sdk: '>=2.1.0 <3.0.0' + sdk: '>=2.2.0 <3.0.0' dependencies: analyzer: ^0.38.3 diff --git a/test/model_test.dart b/test/model_test.dart index 42317ce34b..30995c413f 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2128,7 +2128,7 @@ void main() { Extension ext, fancyList; Extension documentOnceReexportOne, documentOnceReexportTwo; Library reexportOneLib, reexportTwoLib; - Class extensionReferencer; + Class apple, extensionReferencer; Method doSomeStuff, doStuff, s; List extensions; @@ -2142,6 +2142,7 @@ void main() { documentOnceReexportTwo = reexportTwoLib.extensions .firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); + apple = exLibrary.classes.firstWhere((e) => e.name == 'Apple'); ext = exLibrary.extensions.firstWhere((e) => e.name == 'AppleExtension'); extensionReferencer = exLibrary.classes.firstWhere((c) => c.name == 'ExtensionReferencer'); @@ -2164,6 +2165,10 @@ void main() { expect(documentOnceReexportTwo.isCanonical, isTrue); }); + test('classes know about applicable extensions', () { + expect(apple.applicableExtensions, contains(ext)); + }); + // TODO(jcollins-g): implement feature and update tests test('documentation links do not crash in base cases', () { From 217bd9d0ab3d7824aac96113725a8e9fa671023c Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 30 Oct 2019 15:37:06 -0700 Subject: [PATCH 04/10] Review comments --- lib/src/model.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 36a3c0c84a..b7bb0da02c 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -38,7 +38,6 @@ import 'package:analyzer/src/dart/element/member.dart' import 'package:analyzer/src/dart/sdk/sdk.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/java_io.dart'; -import 'package:analyzer/src/generated/resolver.dart' show NamespaceBuilder; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/source_io.dart'; @@ -2372,11 +2371,12 @@ class Library extends ModelElement with Categorization, TopLevelContainer { // Initialize the list of elements defined in this library and // exported via its export directives. - Set exportedAndLocalElements = NamespaceBuilder() - .createExportNamespaceForLibrary(element) + Set exportedAndLocalElements = _libraryElement + .exportNamespace .definedNames .values .toSet(); + // TODO(jcollins-g): Consider switch to [_libraryElement.topLevelElements]. exportedAndLocalElements .addAll(getDefinedElements(_libraryElement.definingCompilationUnit)); for (CompilationUnitElement cu in _libraryElement.parts) { From 744331eb0d88aa7f6031e20fde2341c7456c394b Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 30 Oct 2019 17:09:09 -0700 Subject: [PATCH 05/10] Add 'AvailableExtensions' to class pages --- lib/src/model.dart | 4 +++- lib/templates/class.html | 9 +++++++++ lib/templates/extension.html | 16 +++++++++------- test/model_test.dart | 11 ++++++++--- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index dbad55e5f0..4e6b1fb135 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -793,10 +793,12 @@ class Class extends Container return _defaultConstructor; } + bool get hasApplicableExtensions => applicableExtensions.isNotEmpty; + List _applicableExtensions; Iterable get applicableExtensions { if (_applicableExtensions == null) { - _applicableExtensions = packageGraph.extensions + _applicableExtensions = utils.filterNonDocumented(packageGraph.extensions) .where((e) => e.couldApplyTo(this)) .toList(growable: false); } diff --git a/lib/templates/class.html b/lib/templates/class.html index a9adc45ae2..93a198758a 100644 --- a/lib/templates/class.html +++ b/lib/templates/class.html @@ -57,6 +57,15 @@
{{parent.name}} {{parent.kind}}
{{/hasPublicImplementors}} + {{#hasApplicableExtensions}} +
Available Extensions
+
    + {{#applicableExtensions}} +
  • {{{linkedName}}}
    • + {{/applicableExtensions}} +
+ {{/hasApplicableExtensions}} + {{#hasAnnotations}}
Annotations
    diff --git a/lib/templates/extension.html b/lib/templates/extension.html index 9fdee1c40e..e722cc7622 100644 --- a/lib/templates/extension.html +++ b/lib/templates/extension.html @@ -13,16 +13,18 @@
    {{parent.name}} {{parent.kind}}
    {{#extension}} {{>documentation}} - -
    on
    -
    -
      +
      +
      +
      on
      +
      +
        {{#extendedType}}
      • {{{linkedName}}}
        • {{/extendedType}} -
      -
      - +
    +
    + + {{#hasPublicProperties}}
    diff --git a/test/model_test.dart b/test/model_test.dart index 30995c413f..996b4ca84e 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2128,7 +2128,7 @@ void main() { Extension ext, fancyList; Extension documentOnceReexportOne, documentOnceReexportTwo; Library reexportOneLib, reexportTwoLib; - Class apple, extensionReferencer; + Class apple, extensionReferencer, string; Method doSomeStuff, doStuff, s; List extensions; @@ -2141,7 +2141,10 @@ void main() { .firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); documentOnceReexportTwo = reexportTwoLib.extensions .firstWhere((e) => e.name == 'DocumentThisExtensionOnce'); - + string = packageGraph.allLibraries.values + .firstWhere((e) => e.name == 'dart:core') + .allClasses + .firstWhere((c) => c.name == 'String'); apple = exLibrary.classes.firstWhere((e) => e.name == 'Apple'); ext = exLibrary.extensions.firstWhere((e) => e.name == 'AppleExtension'); extensionReferencer = @@ -2166,7 +2169,9 @@ void main() { }); test('classes know about applicable extensions', () { - expect(apple.applicableExtensions, contains(ext)); + expect(apple.applicableExtensions, orderedEquals([ext])); + expect(string.applicableExtensions, isNot(contains(documentOnceReexportOne))); + expect(string.applicableExtensions, contains(documentOnceReexportTwo)); }); // TODO(jcollins-g): implement feature and update tests From c53b7224ed2802c0d5ad16b6c5fcf5c0fe940efb Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 30 Oct 2019 17:12:02 -0700 Subject: [PATCH 06/10] remove some unfinished bits, dartfmt --- lib/src/model.dart | 79 ++++------------------------------------------ 1 file changed, 7 insertions(+), 72 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 4e6b1fb135..4f29a68ed1 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -798,7 +798,8 @@ class Class extends Container List _applicableExtensions; Iterable get applicableExtensions { if (_applicableExtensions == null) { - _applicableExtensions = utils.filterNonDocumented(packageGraph.extensions) + _applicableExtensions = utils + .filterNonDocumented(packageGraph.extensions) .where((e) => e.couldApplyTo(this)) .toList(growable: false); } @@ -1338,73 +1339,10 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - /// Returns [true] if this extension in context applies to the given - /// [ModelElement]. - /// - /// It may or may not be the most specific for a given - /// method reference, and this does not guarantee any methods will actually - /// be applicable to, most specific for, and not shadowed for [e]. - bool appliesTo(ModelElement e) => (e.library == library) && couldApplyTo(e); - /// Returns [true] if the extension could apply to [e] if both are imported, /// possibly with show/hide. - bool couldApplyTo(ModelElement e) => extendedType.type.isSubtypeOf( - e.modelType.type); - - /// Assuming that [possibleExtensions] contain multiple applicable extensions - /// for the same extension method reference, return the set of extensions - /// most applicable. - static Iterable mostSpecificExtensions( - Iterable possibleExtensions) { - Set returningExtensions = {}; - // The extension at least tied for most specific that we have seen. - Extension mostSpecificSoFar; - for (Extension possible in possibleExtensions) { - if (mostSpecificSoFar == null) { - mostSpecificSoFar = possible; - continue; - } - if (possible._isMoreSpecificThan(mostSpecificSoFar)) { - mostSpecificSoFar = possible; - } - } - - // Collect all extensions tied with the most specific one. - for (Extension possible in possibleExtensions) { - if (!mostSpecificSoFar._isMoreSpecificThan(possible)) { - returningExtensions.add(possible); - } - } - return returningExtensions; - } - - /// Is this extension more specific than [e2]? - /// - /// If false, does not imply that this is less specific than [e2]. - /// - /// From: https://github.com/dart-lang/language/blob/master/accepted/2.6/static-extension-members/feature-specification.md#Specificity - bool _isMoreSpecificThan(Extension e2) { - Extension e1 = this; - DartType e1Type = e1.modelType.type; - DartType e2Type = e2.modelType.type; - DartType e1Resolved = e1Type.resolveToBound(null); - DartType e2Resolved = e2Type.resolveToBound(null); - // 1 - if ((e2.definingLibrary.isInSdk && !e1.definingLibrary.isInSdk) || ( - // 2 - e1.definingLibrary.isInSdk == e2.definingLibrary.isInSdk && ( - // 3 - e1Type.isSubtypeOf(e2Type) && ( - // 4 - !e2Type.isSubtypeOf(e1Type) || ( - // 5 - e1Resolved.isSubtypeOf(e2Resolved) && - !e2Resolved.isSubtypeOf(e1Resolved) - ))))) { - return false; - } - return true; - } + bool couldApplyTo(ModelElement e) => + extendedType.type.isSubtypeOf(e.modelType.type); @override ModelElement get enclosingElement => library; @@ -2451,11 +2389,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer { // Initialize the list of elements defined in this library and // exported via its export directives. - Set exportedAndLocalElements = _libraryElement - .exportNamespace - .definedNames - .values - .toSet(); + Set exportedAndLocalElements = + _libraryElement.exportNamespace.definedNames.values.toSet(); // TODO(jcollins-g): Consider switch to [_libraryElement.topLevelElements]. exportedAndLocalElements .addAll(getDefinedElements(_libraryElement.definingCompilationUnit)); @@ -5161,11 +5096,11 @@ class PackageGraph { /// It is safe to cache values derived from the [_implementors] table if this /// is true. bool allImplementorsAdded = false; + /// It is safe to cache values derived from the [_extensions] table if this /// is true. bool allExtensionsAdded = false; - Map> get implementors { assert(allImplementorsAdded); return _implementors; From 84eec60d637937732cf317afd14954168e88c46c Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 31 Oct 2019 12:30:27 -0700 Subject: [PATCH 07/10] Works --- lib/src/model.dart | 70 +++++++++++++++++++++++++++--- test/model_test.dart | 27 ++++++++++-- testing/test_package/lib/fake.dart | 33 ++++++++++++++ 3 files changed, 119 insertions(+), 11 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 4f29a68ed1..3cbbc44e80 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -22,6 +22,7 @@ import 'package:analyzer/dart/ast/ast.dart' InstanceCreationExpression; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/dart/element/type_system.dart'; import 'package:analyzer/dart/element/visitor.dart'; import 'package:analyzer/file_system/file_system.dart' as file_system; import 'package:analyzer/file_system/physical_file_system.dart'; @@ -35,12 +36,16 @@ import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/dart/element/inheritance_manager3.dart'; import 'package:analyzer/src/dart/element/member.dart' show ExecutableMember, Member, ParameterMember; +import 'package:analyzer/src/dart/element/type.dart' + show InterfaceTypeImpl; import 'package:analyzer/src/dart/sdk/sdk.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/java_io.dart'; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/source_io.dart'; +import 'package:analyzer/src/generated/type_system.dart' + show Dart2TypeSystem; import 'package:analyzer/src/source/package_map_resolver.dart'; import 'package:analyzer/src/source/sdk_ext.dart'; import 'package:args/args.dart'; @@ -798,6 +803,10 @@ class Class extends Container List _applicableExtensions; Iterable get applicableExtensions { if (_applicableExtensions == null) { + if (name.contains('BaseTest')) { + print('hello'); + print(packageGraph.extensions.firstWhere((e) => e.name == 'Uphill').couldApplyTo(this)); + } _applicableExtensions = utils .filterNonDocumented(packageGraph.extensions) .where((e) => e.couldApplyTo(this)) @@ -964,7 +973,8 @@ class Class extends Container hasAnnotations || hasPublicInterfaces || hasPublicSuperChainReversed || - hasPublicImplementors; + hasPublicImplementors || + hasApplicableExtensions; @override bool get hasPublicOperators => @@ -1339,10 +1349,54 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - /// Returns [true] if the extension could apply to [e] if both are imported, - /// possibly with show/hide. - bool couldApplyTo(ModelElement e) => - extendedType.type.isSubtypeOf(e.modelType.type); + /// Returns [true] if [this] extension could theoretically apply to [e] if + /// both are imported, possibly with show/hide. + bool couldApplyTo(Class c) { + if (name == 'Int8Pointer' && c.name == 'Pointer') + print ('hi'); + return _couldApplyTo( + extendedType.type, c.element, packageGraph.typeSystem); + } + + static bool _couldApplyTo(DartType thisType, ClassElement element, Dart2TypeSystem typeSystem) { + if (element.name == 'SuperMegaTron') { + print('hello'); + } + + /*if (thisType is TypeParameterType) { + return _couldApplyTo(thisType.element.bound, element, typeSystem); + }*/ + + InterfaceType classInstantiated = typeSystem.instantiateToBounds(element.thisType); + classInstantiated = typeSystem.instantiateType(element.thisType, + classInstantiated.typeArguments.map((a) { + if (a.isDynamic) { + return typeSystem.typeProvider.neverType; + } + return a; + }).toList()); + + + if (typeSystem.isSubtypeOf(classInstantiated, thisType)) { + return true; + } + + return classInstantiated.element == thisType.element; +/* + if (thisType is TypeParameterType) { + return (thisType.element.bound as InterfaceTypeImpl).asInstanceOf(element) != null; + } + if (!typeSystem.isAssignableTo(element.type, typeSystem.leastUpperBound(element.type, thisType))) return false; + if ((thisType as InterfaceTypeImpl).asInstanceOf(element) == null) + return false; + if (typeSystem.isSubtypeOf(element.type, thisType)) return true; + /*if (thisType is InterfaceTypeImpl) { + return thisType.asInstanceOf(element) != null; + } + */ + return (element.type as InterfaceTypeImpl).asInstanceOf(thisType.element as ClassElement) != null; + throw UnimplementedError('unrecognized DartType');*/ + } @override ModelElement get enclosingElement => library; @@ -4977,7 +5031,7 @@ class Operator extends Method { class PackageGraph { PackageGraph.UninitializedPackageGraph( - this.config, this.driver, this.sdk, this.hasEmbedderSdk) + this.config, this.driver, this.typeSystem, this.sdk, this.hasEmbedderSdk) : packageMeta = config.topLevelPackageMeta, session = driver.currentSession { _packageWarningCounter = PackageWarningCounter(this); @@ -5182,6 +5236,7 @@ class PackageGraph { /// TODO(brianwilkerson) Replace the driver with the session. final AnalysisDriver driver; final AnalysisSession session; + final TypeSystem typeSystem; final DartSdk sdk; Map _sdkLibrarySources; @@ -6844,7 +6899,8 @@ class PackageBuilder { } PackageGraph newGraph = PackageGraph.UninitializedPackageGraph( - config, driver, sdk, hasEmbedderSdkFiles); + config, driver, await driver.currentSession.typeSystem, sdk, + hasEmbedderSdkFiles); await getLibraries(newGraph); await newGraph.initializePackageGraph(); return newGraph; diff --git a/test/model_test.dart b/test/model_test.dart index 996b4ca84e..fdc39455dd 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2125,10 +2125,11 @@ void main() { }); group('Extension', () { - Extension ext, fancyList; + Extension arm, leg, ext, fancyList, uphill; Extension documentOnceReexportOne, documentOnceReexportTwo; Library reexportOneLib, reexportTwoLib; - Class apple, extensionReferencer, string; + Class apple, anotherExtended, baseTest, bigAnotherExtended, + extensionReferencer, megaTron, superMegaTron, string; Method doSomeStuff, doStuff, s; List extensions; @@ -2159,6 +2160,15 @@ void main() { .instanceMethods .firstWhere((m) => m.name == 'doStuff'); extensions = exLibrary.publicExtensions.toList(); + baseTest = fakeLibrary.classes.firstWhere((e) => e.name == 'BaseTest'); + bigAnotherExtended = fakeLibrary.classes.firstWhere((e) => e.name == 'BigAnotherExtended'); + anotherExtended = fakeLibrary.classes.firstWhere((e) => e.name == 'AnotherExtended'); + arm = fakeLibrary.extensions.firstWhere((e) => e.name == 'Arm'); + leg = fakeLibrary.extensions.firstWhere((e) => e.name == 'Leg'); + uphill = fakeLibrary.extensions.firstWhere((e) => e.name == 'Uphill'); + megaTron = fakeLibrary.classes.firstWhere((e) => e.name == 'Megatron'); + superMegaTron = fakeLibrary.classes + .firstWhere((e) => e.name == 'SuperMegaTron'); }); test('basic canonicalization for extensions', () { @@ -2168,10 +2178,19 @@ void main() { expect(documentOnceReexportTwo.isCanonical, isTrue); }); - test('classes know about applicable extensions', () { + test('classes know about applicableExtensions', () { expect(apple.applicableExtensions, orderedEquals([ext])); - expect(string.applicableExtensions, isNot(contains(documentOnceReexportOne))); + expect(string.applicableExtensions, + isNot(contains(documentOnceReexportOne))); expect(string.applicableExtensions, contains(documentOnceReexportTwo)); + expect(baseTest.applicableExtensions, isEmpty); + expect(anotherExtended.applicableExtensions, orderedEquals([uphill])); + expect(bigAnotherExtended.applicableExtensions, orderedEquals([uphill])); + }); + + test('type parameters and bounds work with applicableExtensions', () { + expect(superMegaTron.applicableExtensions, orderedEquals([leg])); + expect(megaTron.applicableExtensions, orderedEquals([arm, leg])); }); // TODO(jcollins-g): implement feature and update tests diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 5e828e7d06..e88600d7d3 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -990,6 +990,39 @@ class ReferringClass { } } +/// Discovery for extensions testing. +extension Arm on Megatron { + bool get hasLeftArm => true; +} + +extension Leg on Megatron { + bool get hasRightLeg => true; +} + +class Megatron {} + +class SuperMegaTron extends Megatron {} + + +extension Uphill on AnotherExtended { + bool get hasDirection => false; +} + +class SubclassBaseTest extends BaseTest {} + +class BaseTest {} + +class AnotherExtended extends BaseTest {} + +class BigAnotherExtended extends AnotherExtended {} + + + +void moof() { + SuperMegaTron().hasRightLeg; +} + + /// Test an edge case for cases where inherited ExecutableElements can come /// both from private classes and public interfaces. The test makes sure the /// class still takes precedence (#1561). From 7c1de778697615ede6d2d1924f22ca262213629b Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 31 Oct 2019 13:32:36 -0700 Subject: [PATCH 08/10] Cleaned up --- lib/src/model.dart | 68 +++++++++--------------------- lib/templates/class.html | 8 ++-- test/model_test.dart | 16 +++---- testing/test_package/lib/fake.dart | 15 +++---- tool/grind.dart | 1 + 5 files changed, 41 insertions(+), 67 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 3cbbc44e80..6d2265c999 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -798,21 +798,21 @@ class Class extends Container return _defaultConstructor; } - bool get hasApplicableExtensions => applicableExtensions.isNotEmpty; + bool get hasPotentiallyApplicableExtensions => potentiallyApplicableExtensions.isNotEmpty; - List _applicableExtensions; - Iterable get applicableExtensions { - if (_applicableExtensions == null) { + List _potentiallyApplicableExtensions; + Iterable get potentiallyApplicableExtensions { + if (_potentiallyApplicableExtensions == null) { if (name.contains('BaseTest')) { print('hello'); print(packageGraph.extensions.firstWhere((e) => e.name == 'Uphill').couldApplyTo(this)); } - _applicableExtensions = utils + _potentiallyApplicableExtensions = utils .filterNonDocumented(packageGraph.extensions) .where((e) => e.couldApplyTo(this)) .toList(growable: false); } - return _applicableExtensions; + return _potentiallyApplicableExtensions; } Iterable get allInstanceMethods => @@ -974,7 +974,7 @@ class Class extends Container hasPublicInterfaces || hasPublicSuperChainReversed || hasPublicImplementors || - hasApplicableExtensions; + hasPotentiallyApplicableExtensions; @override bool get hasPublicOperators => @@ -1349,53 +1349,26 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - /// Returns [true] if [this] extension could theoretically apply to [e] if - /// both are imported, possibly with show/hide. - bool couldApplyTo(Class c) { - if (name == 'Int8Pointer' && c.name == 'Pointer') - print ('hi'); - return _couldApplyTo( + /// Returns [true] if this extension could be applicable to any possible + /// instantiation of [c]. + bool couldApplyTo(Class c) => _couldApplyTo( extendedType.type, c.element, packageGraph.typeSystem); - } - - static bool _couldApplyTo(DartType thisType, ClassElement element, Dart2TypeSystem typeSystem) { - if (element.name == 'SuperMegaTron') { - print('hello'); - } - - /*if (thisType is TypeParameterType) { - return _couldApplyTo(thisType.element.bound, element, typeSystem); - }*/ - InterfaceType classInstantiated = typeSystem.instantiateToBounds(element.thisType); - classInstantiated = typeSystem.instantiateType(element.thisType, + static bool _couldApplyTo( + DartType extendedType, ClassElement element, Dart2TypeSystem typeSystem) { + InterfaceTypeImpl classInstantiated = typeSystem.instantiateToBounds( + element.thisType); + classInstantiated = element.instantiate(typeArguments: classInstantiated.typeArguments.map((a) { if (a.isDynamic) { return typeSystem.typeProvider.neverType; } return a; - }).toList()); + }).toList(), + nullabilitySuffix: classInstantiated.nullabilitySuffix); - - if (typeSystem.isSubtypeOf(classInstantiated, thisType)) { - return true; - } - - return classInstantiated.element == thisType.element; -/* - if (thisType is TypeParameterType) { - return (thisType.element.bound as InterfaceTypeImpl).asInstanceOf(element) != null; - } - if (!typeSystem.isAssignableTo(element.type, typeSystem.leastUpperBound(element.type, thisType))) return false; - if ((thisType as InterfaceTypeImpl).asInstanceOf(element) == null) - return false; - if (typeSystem.isSubtypeOf(element.type, thisType)) return true; - /*if (thisType is InterfaceTypeImpl) { - return thisType.asInstanceOf(element) != null; - } - */ - return (element.type as InterfaceTypeImpl).asInstanceOf(thisType.element as ClassElement) != null; - throw UnimplementedError('unrecognized DartType');*/ + return (classInstantiated.element == extendedType.element) || + typeSystem.isSubtypeOf(classInstantiated, extendedType); } @override @@ -5087,12 +5060,13 @@ class PackageGraph { package._libraries.sort((a, b) => compareNatural(a.name, b.name)); package._libraries.forEach((library) { library.allClasses.forEach(_addToImplementors); + // TODO(jcollins-g): Use a better data structure. _extensions.addAll(library.extensions); }); }); _implementors.values.forEach((l) => l.sort()); - _extensions.sort(byName); allImplementorsAdded = true; + _extensions.sort(byName); allExtensionsAdded = true; // We should have found all special classes by now. diff --git a/lib/templates/class.html b/lib/templates/class.html index 93a198758a..ae308d6eae 100644 --- a/lib/templates/class.html +++ b/lib/templates/class.html @@ -57,14 +57,14 @@
    {{parent.name}} {{parent.kind}}
{{/hasPublicImplementors}} - {{#hasApplicableExtensions}} + {{#hasPotentiallyApplicableExtensions}}
Available Extensions
    - {{#applicableExtensions}} + {{#potentiallyApplicableExtensions}}
  • {{{linkedName}}}
    • - {{/applicableExtensions}} + {{/potentiallyApplicableExtensions}}
- {{/hasApplicableExtensions}} + {{/hasPotentiallyApplicableExtensions}} {{#hasAnnotations}}
Annotations
diff --git a/test/model_test.dart b/test/model_test.dart index fdc39455dd..8e89ed516e 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2179,18 +2179,18 @@ void main() { }); test('classes know about applicableExtensions', () { - expect(apple.applicableExtensions, orderedEquals([ext])); - expect(string.applicableExtensions, + expect(apple.potentiallyApplicableExtensions, orderedEquals([ext])); + expect(string.potentiallyApplicableExtensions, isNot(contains(documentOnceReexportOne))); - expect(string.applicableExtensions, contains(documentOnceReexportTwo)); - expect(baseTest.applicableExtensions, isEmpty); - expect(anotherExtended.applicableExtensions, orderedEquals([uphill])); - expect(bigAnotherExtended.applicableExtensions, orderedEquals([uphill])); + expect(string.potentiallyApplicableExtensions, contains(documentOnceReexportTwo)); + expect(baseTest.potentiallyApplicableExtensions, isEmpty); + expect(anotherExtended.potentiallyApplicableExtensions, orderedEquals([uphill])); + expect(bigAnotherExtended.potentiallyApplicableExtensions, orderedEquals([uphill])); }); test('type parameters and bounds work with applicableExtensions', () { - expect(superMegaTron.applicableExtensions, orderedEquals([leg])); - expect(megaTron.applicableExtensions, orderedEquals([arm, leg])); + expect(superMegaTron.potentiallyApplicableExtensions, orderedEquals([leg])); + expect(megaTron.potentiallyApplicableExtensions, orderedEquals([arm, leg])); }); // TODO(jcollins-g): implement feature and update tests diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index e88600d7d3..332b86b225 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -990,7 +990,10 @@ class ReferringClass { } } -/// Discovery for extensions testing. +// +// Test classes for extension discovery. +// + extension Arm on Megatron { bool get hasLeftArm => true; } @@ -1003,7 +1006,6 @@ class Megatron {} class SuperMegaTron extends Megatron {} - extension Uphill on AnotherExtended { bool get hasDirection => false; } @@ -1016,12 +1018,9 @@ class AnotherExtended extends BaseTest {} class BigAnotherExtended extends AnotherExtended {} - - -void moof() { - SuperMegaTron().hasRightLeg; -} - +// +// +// /// Test an edge case for cases where inherited ExecutableElements can come /// both from private classes and public interfaces. The test makes sure the diff --git a/tool/grind.dart b/tool/grind.dart index 2f05c76566..eb229f5a89 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -486,6 +486,7 @@ Future> _buildTestPackageDocs( 'examples', '--include-source', '--json', + '--link-to-remote', '--pretty-index-json', ]..addAll(extraDartdocParameters), workingDirectory: testPackage.absolute.path); From 148e2472d421830f54cb5bd87434e082aaf51ce6 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 31 Oct 2019 13:34:12 -0700 Subject: [PATCH 09/10] dartfmt --- lib/src/model.dart | 32 ++++++++++++++++++-------------- test/model_test.dart | 36 ++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 6d2265c999..8841ff2700 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -36,16 +36,14 @@ import 'package:analyzer/src/dart/element/element.dart'; import 'package:analyzer/src/dart/element/inheritance_manager3.dart'; import 'package:analyzer/src/dart/element/member.dart' show ExecutableMember, Member, ParameterMember; -import 'package:analyzer/src/dart/element/type.dart' - show InterfaceTypeImpl; +import 'package:analyzer/src/dart/element/type.dart' show InterfaceTypeImpl; import 'package:analyzer/src/dart/sdk/sdk.dart'; import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/java_io.dart'; import 'package:analyzer/src/generated/sdk.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/generated/source_io.dart'; -import 'package:analyzer/src/generated/type_system.dart' - show Dart2TypeSystem; +import 'package:analyzer/src/generated/type_system.dart' show Dart2TypeSystem; import 'package:analyzer/src/source/package_map_resolver.dart'; import 'package:analyzer/src/source/sdk_ext.dart'; import 'package:args/args.dart'; @@ -798,14 +796,17 @@ class Class extends Container return _defaultConstructor; } - bool get hasPotentiallyApplicableExtensions => potentiallyApplicableExtensions.isNotEmpty; + bool get hasPotentiallyApplicableExtensions => + potentiallyApplicableExtensions.isNotEmpty; List _potentiallyApplicableExtensions; Iterable get potentiallyApplicableExtensions { if (_potentiallyApplicableExtensions == null) { if (name.contains('BaseTest')) { print('hello'); - print(packageGraph.extensions.firstWhere((e) => e.name == 'Uphill').couldApplyTo(this)); + print(packageGraph.extensions + .firstWhere((e) => e.name == 'Uphill') + .couldApplyTo(this)); } _potentiallyApplicableExtensions = utils .filterNonDocumented(packageGraph.extensions) @@ -1351,21 +1352,21 @@ class Extension extends Container /// Returns [true] if this extension could be applicable to any possible /// instantiation of [c]. - bool couldApplyTo(Class c) => _couldApplyTo( - extendedType.type, c.element, packageGraph.typeSystem); + bool couldApplyTo(Class c) => + _couldApplyTo(extendedType.type, c.element, packageGraph.typeSystem); static bool _couldApplyTo( DartType extendedType, ClassElement element, Dart2TypeSystem typeSystem) { - InterfaceTypeImpl classInstantiated = typeSystem.instantiateToBounds( - element.thisType); - classInstantiated = element.instantiate(typeArguments: - classInstantiated.typeArguments.map((a) { + InterfaceTypeImpl classInstantiated = + typeSystem.instantiateToBounds(element.thisType); + classInstantiated = element.instantiate( + typeArguments: classInstantiated.typeArguments.map((a) { if (a.isDynamic) { return typeSystem.typeProvider.neverType; } return a; }).toList(), - nullabilitySuffix: classInstantiated.nullabilitySuffix); + nullabilitySuffix: classInstantiated.nullabilitySuffix); return (classInstantiated.element == extendedType.element) || typeSystem.isSubtypeOf(classInstantiated, extendedType); @@ -6873,7 +6874,10 @@ class PackageBuilder { } PackageGraph newGraph = PackageGraph.UninitializedPackageGraph( - config, driver, await driver.currentSession.typeSystem, sdk, + config, + driver, + await driver.currentSession.typeSystem, + sdk, hasEmbedderSdkFiles); await getLibraries(newGraph); await newGraph.initializePackageGraph(); diff --git a/test/model_test.dart b/test/model_test.dart index 8e89ed516e..43a8876964 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -2128,8 +2128,14 @@ void main() { Extension arm, leg, ext, fancyList, uphill; Extension documentOnceReexportOne, documentOnceReexportTwo; Library reexportOneLib, reexportTwoLib; - Class apple, anotherExtended, baseTest, bigAnotherExtended, - extensionReferencer, megaTron, superMegaTron, string; + Class apple, + anotherExtended, + baseTest, + bigAnotherExtended, + extensionReferencer, + megaTron, + superMegaTron, + string; Method doSomeStuff, doStuff, s; List extensions; @@ -2161,14 +2167,16 @@ void main() { .firstWhere((m) => m.name == 'doStuff'); extensions = exLibrary.publicExtensions.toList(); baseTest = fakeLibrary.classes.firstWhere((e) => e.name == 'BaseTest'); - bigAnotherExtended = fakeLibrary.classes.firstWhere((e) => e.name == 'BigAnotherExtended'); - anotherExtended = fakeLibrary.classes.firstWhere((e) => e.name == 'AnotherExtended'); + bigAnotherExtended = + fakeLibrary.classes.firstWhere((e) => e.name == 'BigAnotherExtended'); + anotherExtended = + fakeLibrary.classes.firstWhere((e) => e.name == 'AnotherExtended'); arm = fakeLibrary.extensions.firstWhere((e) => e.name == 'Arm'); leg = fakeLibrary.extensions.firstWhere((e) => e.name == 'Leg'); uphill = fakeLibrary.extensions.firstWhere((e) => e.name == 'Uphill'); megaTron = fakeLibrary.classes.firstWhere((e) => e.name == 'Megatron'); - superMegaTron = fakeLibrary.classes - .firstWhere((e) => e.name == 'SuperMegaTron'); + superMegaTron = + fakeLibrary.classes.firstWhere((e) => e.name == 'SuperMegaTron'); }); test('basic canonicalization for extensions', () { @@ -2182,20 +2190,24 @@ void main() { expect(apple.potentiallyApplicableExtensions, orderedEquals([ext])); expect(string.potentiallyApplicableExtensions, isNot(contains(documentOnceReexportOne))); - expect(string.potentiallyApplicableExtensions, contains(documentOnceReexportTwo)); + expect(string.potentiallyApplicableExtensions, + contains(documentOnceReexportTwo)); expect(baseTest.potentiallyApplicableExtensions, isEmpty); - expect(anotherExtended.potentiallyApplicableExtensions, orderedEquals([uphill])); - expect(bigAnotherExtended.potentiallyApplicableExtensions, orderedEquals([uphill])); + expect(anotherExtended.potentiallyApplicableExtensions, + orderedEquals([uphill])); + expect(bigAnotherExtended.potentiallyApplicableExtensions, + orderedEquals([uphill])); }); test('type parameters and bounds work with applicableExtensions', () { - expect(superMegaTron.potentiallyApplicableExtensions, orderedEquals([leg])); - expect(megaTron.potentiallyApplicableExtensions, orderedEquals([arm, leg])); + expect( + superMegaTron.potentiallyApplicableExtensions, orderedEquals([leg])); + expect( + megaTron.potentiallyApplicableExtensions, orderedEquals([arm, leg])); }); // TODO(jcollins-g): implement feature and update tests test('documentation links do not crash in base cases', () { - packageGraph.packageWarningCounter.hasWarning( doStuff, PackageWarning.notImplemented, From 401466e88feff6362bf73b9e9862fa04d70923e5 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 31 Oct 2019 14:19:23 -0700 Subject: [PATCH 10/10] Review comments --- lib/src/model.dart | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 12653427f5..a02b983d9a 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -802,12 +802,6 @@ class Class extends Container List _potentiallyApplicableExtensions; Iterable get potentiallyApplicableExtensions { if (_potentiallyApplicableExtensions == null) { - if (name.contains('BaseTest')) { - print('hello'); - print(packageGraph.extensions - .firstWhere((e) => e.name == 'Uphill') - .couldApplyTo(this)); - } _potentiallyApplicableExtensions = utils .filterNonDocumented(packageGraph.extensions) .where((e) => e.couldApplyTo(this)) @@ -1350,8 +1344,8 @@ class Extension extends Container ElementType.from(_extension.extendedType, library, packageGraph); } - /// Returns [true] if this extension could be applicable to any possible - /// instantiation of [c]. + /// Returns [true] if there is an instantiation of [c] to which this extension + /// could be applied. bool couldApplyTo(Class c) => _couldApplyTo(extendedType.type, c.element, packageGraph.typeSystem);