Skip to content

Commit

Permalink
Start using docImport resolution when resolving references (#3805)
Browse files Browse the repository at this point in the history
  • Loading branch information
srawlins authored Jul 1, 2024
1 parent 9d756dc commit e7f3694
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 46 deletions.
45 changes: 29 additions & 16 deletions lib/src/model/comment_referable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'dart:core';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/scope.dart';
import 'package:collection/collection.dart';
import 'package:dartdoc/src/model/container.dart';
import 'package:dartdoc/src/model/library.dart';
import 'package:dartdoc/src/model/model_element.dart';
import 'package:dartdoc/src/model/nameable.dart';
Expand Down Expand Up @@ -53,12 +52,13 @@ mixin CommentReferable implements Nameable {
return tryParents ? null : this;
}
for (var referenceLookup in _childLookups(reference)) {
if (scope != null) {
var result = _lookupViaScope(referenceLookup, filter: filter);
if (result != null) {
return result;
}
// First attempt: Ask analyzer's `Scope.lookup` API.
var result = _lookupViaScope(referenceLookup, filter: filter);
if (result != null) {
return result;
}

// Second attempt: Look through `referenceChildren`.
final referenceChildren = this.referenceChildren;
final childrenResult = referenceChildren[referenceLookup.lookup];
if (childrenResult != null) {
Expand Down Expand Up @@ -97,27 +97,40 @@ mixin CommentReferable implements Nameable {
_ReferenceChildrenLookup referenceLookup, {
required bool Function(CommentReferable?) filter,
}) {
final resultElement = scope!.lookupPreferGetter(referenceLookup.lookup);
if (resultElement == null) return null;
Element? resultElement;
final scope = this.scope;
if (scope != null) {
resultElement = scope.lookupPreferGetter(referenceLookup.lookup);
if (resultElement == null) return null;
}

if (this case ModelElement(:var modelNode?) when resultElement == null) {
var references = modelNode.commentData?.references;
if (references != null) {
resultElement = references[referenceLookup.lookup]?.element;
}
}

if (resultElement == null) {
return null;
}

ModelElement result;
if (resultElement is PropertyAccessorElement) {
final variable = resultElement.variable2!;
if (variable.isSynthetic) {
// First, cache the synthetic variable, so that the
// PropertyAccessorElement getter and/or setter are set (see
// `Field.new` regarding `enclosingCombo`).
packageGraph.getModelForElement(variable);
// Then, use the result for the PropertyAccessorElement.
result = packageGraph.getModelForElement(resultElement);
} else {
result = packageGraph.getModelForElement(variable);
}
} else {
result = packageGraph.getModelForElement(resultElement);
}
if (result.enclosingElement is Container) {
assert(
false,
'[Container] member detected, support not implemented for analyzer '
'scope inside containers',
);
return null;
}
return _recurseChildrenAndFilter(referenceLookup, result, filter: filter);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/extension.dart
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Extension extends Container {
setter = ContainerAccessor(fieldSetter, library, packageGraph);
}
return getModelForPropertyInducingElement(field, library,
getter: getter, setter: setter) as Field;
getter: getter, setter: setter, enclosingContainer: this) as Field;
}).toList(growable: false);

@override
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/inheriting_container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ abstract class InheritingContainer extends Container {
(setter == null || setter.isInherited)) {
// Field is 100% inherited.
return getModelForPropertyInducingElement(field, library,
enclosingContainer: this, getter: getter, setter: setter) as Field;
getter: getter, setter: setter, enclosingContainer: this) as Field;
} else {
// Field is <100% inherited (could be half-inherited).
// TODO(jcollins-g): Navigation is probably still confusing for
Expand Down
19 changes: 9 additions & 10 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,15 @@ abstract class ModelElement
}

ModelElement newModelElement;
if (e is FieldElement) {
if (enclosingContainer == null) {
if (e is TopLevelVariableElement) {
assert(getter != null || setter != null);
newModelElement =
TopLevelVariable(e, library, packageGraph, getter, setter);
} else if (e is FieldElement) {
if (enclosingContainer is Extension) {
newModelElement = Field(e, library, packageGraph,
getter as ContainerAccessor?, setter as ContainerAccessor?);
} else if (enclosingContainer == null) {
if (e.isEnumConstant) {
var constantValue = e.computeConstantValue();
if (constantValue == null) {
Expand All @@ -157,7 +164,6 @@ abstract class ModelElement
getter as ContainerAccessor?, setter as ContainerAccessor?);
}
} else {
// EnumFields can't be inherited, so this case is simpler.
newModelElement = Field.inherited(
e,
enclosingContainer,
Expand All @@ -167,16 +173,11 @@ abstract class ModelElement
setter as ContainerAccessor?,
);
}
} else if (e is TopLevelVariableElement) {
assert(getter != null || setter != null);
newModelElement =
TopLevelVariable(e, library, packageGraph, getter, setter);
} else {
throw UnimplementedError(
'Unrecognized property inducing element: $e (${e.runtimeType})');
}

if (enclosingContainer != null) assert(newModelElement is Inheritable);
_cacheNewModelElement(e, newModelElement, library,
enclosingContainer: enclosingContainer);

Expand All @@ -193,8 +194,6 @@ abstract class ModelElement
// clean that up.
// TODO(jcollins-g): Enforce construction restraint.
// TODO(jcollins-g): Allow e to be null and drop extraneous null checks.
// TODO(jcollins-g): Auto-vivify element's defining library for library
// parameter when given a null.
factory ModelElement.for_(
Element e, Library library, PackageGraph packageGraph,
{Container? enclosingContainer}) {
Expand Down
13 changes: 1 addition & 12 deletions test/end2end/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2317,8 +2317,7 @@ void main() async {
late final Class TypeParameterThings,
TypeParameterThingsExtended,
TypeParameterThingsExtendedQ;
late final Extension UnboundTypeTargetExtension;
late final Field aName, aThing, doesNotCrash;
late final Field aName, aThing;
late final TypeParameter ATypeParam,
BTypeParam,
CTypeParam,
Expand All @@ -2329,11 +2328,6 @@ void main() async {
late final ModelFunction aTopLevelTypeParameterFunction;

setUpAll(() {
UnboundTypeTargetExtension =
fakeLibrary.extensions.named('UnboundTypeTargetExtension');
doesNotCrash =
UnboundTypeTargetExtension.instanceFields.named('doesNotCrash');

aTopLevelTypeParameterFunction =
fakeLibrary.functions.named('aTopLevelTypeParameterFunction');
// TODO(jcollins-g): dart-lang/dartdoc#2704, HTML and type parameters
Expand Down Expand Up @@ -2368,11 +2362,6 @@ void main() async {
QTypeParam = aMethodExtendedQ.typeParameters.named('QTypeParam');
});

test('on extension targeting an unbound type', () {
expect(referenceLookup(UnboundTypeTargetExtension, 'doesNotCrash'),
equals(MatchingLinkResult(doesNotCrash)));
});

test('on inherited documentation', () {
expect(referenceLookup(aMethodExtended, 'ATypeParam'),
equals(MatchingLinkResult(ATypeParam)));
Expand Down
32 changes: 32 additions & 0 deletions test/extensions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,38 @@ var f() {}
);
}

void test_referenceToExtensionGetter() async {
var library = await bootPackageWithLibrary('''
extension Ex on int {
bool get b => true;
}
/// Text [Ex.b].
var f() {}
''');

expect(
library.functions.named('f').documentationAsHtml,
contains('<a href="$linkPrefix/Ex/b.html">Ex.b</a>'),
);
}

void test_referenceToExtensionSetter() async {
var library = await bootPackageWithLibrary('''
extension Ex on int {
set b(int value) {}
}
/// Text [Ex.b].
var f() {}
''');

expect(
library.functions.named('f').documentationAsHtml,
contains('<a href="$linkPrefix/Ex/b.html">Ex.b</a>'),
);
}

// TODO(srawlins): Test everything else about extensions.
}

Expand Down
6 changes: 0 additions & 6 deletions testing/test_package/lib/fake.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1076,12 +1076,6 @@ extension Leg on Megatron<String> {
bool get hasRightLeg => true;
}

/// Refer to [doesNotCrash] here.
extension UnboundTypeTargetExtension<T> on T {
/// We hope so!
bool get doesNotCrash => true;
}

class Megatron<T> {}

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

0 comments on commit e7f3694

Please sign in to comment.