Skip to content

Add a list of potentially applicable extensions to every class page #2053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Oct 31, 2019
Merged
79 changes: 70 additions & 9 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -35,12 +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/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';
Expand Down Expand Up @@ -793,6 +796,20 @@ class Class extends Container
return _defaultConstructor;
}

bool get hasPotentiallyApplicableExtensions =>
potentiallyApplicableExtensions.isNotEmpty;

List<Extension> _potentiallyApplicableExtensions;
Iterable<Extension> get potentiallyApplicableExtensions {
if (_potentiallyApplicableExtensions == null) {
_potentiallyApplicableExtensions = utils
.filterNonDocumented(packageGraph.extensions)
.where((e) => e.couldApplyTo(this))
.toList(growable: false);
}
return _potentiallyApplicableExtensions;
}

Iterable<Method> get allInstanceMethods =>
quiver.concat([instanceMethods, inheritedMethods]);

Expand Down Expand Up @@ -951,7 +968,8 @@ class Class extends Container
hasAnnotations ||
hasPublicInterfaces ||
hasPublicSuperChainReversed ||
hasPublicImplementors;
hasPublicImplementors ||
hasPotentiallyApplicableExtensions;

@override
bool get hasPublicOperators =>
Expand Down Expand Up @@ -1326,6 +1344,28 @@ class Extension extends Container
ElementType.from(_extension.extendedType, library, packageGraph);
}

/// 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);

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(),
nullabilitySuffix: classInstantiated.nullabilitySuffix);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's be explicit, use NullabilitySuffix.none.

Copy link
Contributor Author

@jcollins-g jcollins-g Oct 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to preserve whatever I was given rather than explicitly specify something potentially wrong.


return (classInstantiated.element == extendedType.element) ||
typeSystem.isSubtypeOf(classInstantiated, extendedType);
}

@override
ModelElement get enclosingElement => library;

Expand Down Expand Up @@ -2371,11 +2411,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer {

// Initialize the list of elements defined in this library and
// exported via its export directives.
Set<Element> exportedAndLocalElements = _libraryElement
.exportNamespace
.definedNames
.values
.toSet();
Set<Element> exportedAndLocalElements =
_libraryElement.exportNamespace.definedNames.values.toSet();
// TODO(jcollins-g): Consider switch to [_libraryElement.topLevelElements].
exportedAndLocalElements
.addAll(getDefinedElements(_libraryElement.definingCompilationUnit));
Expand Down Expand Up @@ -2403,6 +2440,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer {

List<String> _allOriginalModelElementNames;

bool get isInSdk => _libraryElement.isInSdk;

final Package _package;

@override
Expand Down Expand Up @@ -4967,7 +5006,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);
Expand Down Expand Up @@ -5023,10 +5062,14 @@ 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());
allImplementorsAdded = true;
_extensions.sort(byName);
allExtensionsAdded = true;

// We should have found all special classes by now.
specialClasses.assertSpecials();
Expand Down Expand Up @@ -5080,15 +5123,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<String, List<Class>> get implementors {
assert(allImplementorsAdded);
return _implementors;
}

Iterable<Extension> get extensions {
assert(allExtensionsAdded);
return _extensions;
}

Map<String, Set<ModelElement>> _findRefElementCache;

Map<String, Set<ModelElement>> get findRefElementCache {
Expand Down Expand Up @@ -5126,6 +5178,10 @@ class PackageGraph {
/// Map of Class.href to a list of classes implementing that class
final Map<String, List<Class>> _implementors = Map();

/// A list of extensions that exist in the package graph.
// TODO(jcollins-g): Consider implementing a smarter structure for this.
final List<Extension> _extensions = List();

/// PackageMeta for the default package.
final PackageMeta packageMeta;

Expand Down Expand Up @@ -5156,6 +5212,7 @@ class PackageGraph {
/// TODO(brianwilkerson) Replace the driver with the session.
final AnalysisDriver driver;
final AnalysisSession session;
final TypeSystem typeSystem;
final DartSdk sdk;

Map<Source, SdkLibrary> _sdkLibrarySources;
Expand Down Expand Up @@ -6818,7 +6875,11 @@ 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;
Expand Down
9 changes: 9 additions & 0 deletions lib/templates/class.html
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ <h5>{{parent.name}} {{parent.kind}}</h5>
</ul></dd>
{{/hasPublicImplementors}}

{{#hasPotentiallyApplicableExtensions}}
<dt>Available Extensions</dt>
<dd><ul class="comma-separated clazz-relationships">
{{#potentiallyApplicableExtensions}}
<li>{{{linkedName}}}</li>
{{/potentiallyApplicableExtensions}}
</ul></dd>
{{/hasPotentiallyApplicableExtensions}}

{{#hasAnnotations}}
<dt>Annotations</dt>
<dd><ul class="annotation-list clazz-relationships">
Expand Down
16 changes: 9 additions & 7 deletions lib/templates/extension.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,18 @@ <h5>{{parent.name}} {{parent.kind}}</h5>

{{#extension}}
{{>documentation}}

<dt>on</dt>
<dd>
<ul class="comma-separated clazz-relationships">
<section>
<dl class="dl-horizontal">
<dt>on</dt>
<dd>
<ul class="comma-separated clazz-relationships">
{{#extendedType}}
<li>{{{linkedName}}}</li>
{{/extendedType}}
</ul>
</dd>

</ul>
</dd>
</dl>
</section>

{{#hasPublicProperties}}
<section class="summary offset-anchor" id="instance-properties">
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ author: Dart Team <[email protected]>
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.39.0
Expand Down
49 changes: 45 additions & 4 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2125,10 +2125,17 @@ void main() {
});

group('Extension', () {
Extension ext, fancyList;
Extension arm, leg, ext, fancyList, uphill;
Extension documentOnceReexportOne, documentOnceReexportTwo;
Library reexportOneLib, reexportTwoLib;
Class extensionReferencer;
Class apple,
anotherExtended,
baseTest,
bigAnotherExtended,
extensionReferencer,
megaTron,
superMegaTron,
string;
Method doSomeStuff, doStuff, s;
List<Extension> extensions;

Expand All @@ -2141,7 +2148,11 @@ 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 =
exLibrary.classes.firstWhere((c) => c.name == 'ExtensionReferencer');
Expand All @@ -2155,6 +2166,17 @@ 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', () {
Expand All @@ -2164,9 +2186,28 @@ void main() {
expect(documentOnceReexportTwo.isCanonical, isTrue);
});

test('classes know about applicableExtensions', () {
expect(apple.potentiallyApplicableExtensions, orderedEquals([ext]));
expect(string.potentiallyApplicableExtensions,
isNot(contains(documentOnceReexportOne)));
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.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,
Expand Down
32 changes: 32 additions & 0 deletions testing/test_package/lib/fake.dart
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,38 @@ class ReferringClass {
}
}

//
// Test classes for extension discovery.
//

extension Arm on Megatron<int> {
bool get hasLeftArm => true;
}

extension Leg on Megatron<String> {
bool get hasRightLeg => true;
}

class Megatron<T> {}

class SuperMegaTron<T extends String> extends Megatron<String> {}

extension Uphill on AnotherExtended<SubclassBaseTest> {
bool get hasDirection => false;
}

class SubclassBaseTest extends BaseTest {}

class BaseTest {}

class AnotherExtended<T extends BaseTest> extends BaseTest {}

class BigAnotherExtended extends AnotherExtended<SubclassBaseTest> {}

//
//
//

/// 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).
Expand Down
1 change: 1 addition & 0 deletions tool/grind.dart
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ Future<List<Map>> _buildTestPackageDocs(
'examples',
'--include-source',
'--json',
'--link-to-remote',
'--pretty-index-json',
]..addAll(extraDartdocParameters),
workingDirectory: testPackage.absolute.path);
Expand Down