Skip to content

Commit f61f3ba

Browse files
authored
Prepare to switch on new lookup code (#2701)
* Beginnings of grandparent override scoping * intermediate * implement basic overrides, constructors still broken * intermediate * this seems to work much better * dartfmt * This now seems to work with tests * rebuild && dartfmt * fix up tests a bit * add comment * fix package / library cases * move test to correct group * remove assert * switch it on * bare prefix changes * starting to work on test problems * Fix some errors in the tests * correct tests * switch it off... one more PR ;-) * Patch up extension method test, almost forgot why we bothered with this * class counter increment
1 parent e8be4d8 commit f61f3ba

File tree

6 files changed

+77
-39
lines changed

6 files changed

+77
-39
lines changed

lib/src/dartdoc_options.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,8 +1289,8 @@ class DartdocOptionContext extends DartdocOptionContextBase
12891289
List<String> get excludePackages =>
12901290
optionSet['excludePackages'].valueAt(context);
12911291

1292-
bool get experimentalReferenceLookup =>
1293-
optionSet['experimentalReferenceLookup'].valueAt(context);
1292+
bool get enhancedReferenceLookup =>
1293+
optionSet['enhancedReferenceLookup'].valueAt(context);
12941294

12951295
String get flutterRoot => optionSet['flutterRoot'].valueAt(context);
12961296

@@ -1426,10 +1426,13 @@ Future<List<DartdocOption<Object>>> createDartdocOptions(
14261426
DartdocOptionArgOnly<List<String>>('excludePackages', [], resourceProvider,
14271427
help: 'Package names to ignore.', splitCommas: true),
14281428
DartdocOptionArgFile<bool>(
1429-
'experimentalReferenceLookup', false, resourceProvider,
1429+
'enhancedReferenceLookup', false, resourceProvider,
14301430
hide: true,
14311431
help:
1432-
'Use an experimental code path to resolve comment reference lookups',
1432+
'Use the new comment reference lookup system instead of the legacy '
1433+
'version. Please file a bug referencing this flag if you have to '
1434+
'change it to false -- this flag and associated behavior will '
1435+
'disappear in a future version.',
14331436
negatable: true),
14341437
// This could be a ArgOnly, but trying to not provide too many ways
14351438
// to set the flutter root.

lib/src/generator/templates.runtime_renderers.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14694,7 +14694,7 @@ const _invisibleGetters = {
1469414694
'examplePathPrefix',
1469514695
'exclude',
1469614696
'excludePackages',
14697-
'experimentalReferenceLookup',
14697+
'enhancedReferenceLookup',
1469814698
'flutterRoot',
1469914699
'hideSdkText',
1470014700
'include',

lib/src/markdown_processor.dart

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,10 @@ MatchingLinkResult _getMatchingLinkElementLegacy(
380380
// Try expensive not-scoped lookup.
381381
if (refModelElement == null && warnable is ModelElement) {
382382
Container preferredClass = _getPreferredClass(warnable);
383-
if (preferredClass is Extension) {
383+
// We might still get here in comparison mode, don't complain if that's
384+
// the case.
385+
if (preferredClass is Extension &&
386+
warnable.config.enhancedReferenceLookup == false) {
384387
warnable.warn(PackageWarning.notImplemented,
385388
message:
386389
'Comment reference resolution inside extension methods is not yet implemented');
@@ -1032,8 +1035,8 @@ md.Node _makeLinkNode(String codeRef, Warnable warnable) {
10321035

10331036
@visibleForTesting
10341037
MatchingLinkResult getMatchingLinkElement(Warnable warnable, String codeRef,
1035-
{bool experimentalReferenceLookup}) {
1036-
experimentalReferenceLookup ??= warnable.config.experimentalReferenceLookup;
1038+
{bool enhancedReferenceLookup}) {
1039+
enhancedReferenceLookup ??= warnable.config.enhancedReferenceLookup;
10371040
MatchingLinkResult result, resultOld, resultNew;
10381041
// Do a comparison between result types only if the warnings for them are
10391042
// enabled, because there's a significant performance penalty.
@@ -1046,12 +1049,12 @@ MatchingLinkResult getMatchingLinkElement(Warnable warnable, String codeRef,
10461049
if (resultNew.commentReferable != null) {
10471050
markdownStats.resolvedNewLookupReferences++;
10481051
}
1049-
result = experimentalReferenceLookup ? resultNew : resultOld;
1052+
result = enhancedReferenceLookup ? resultNew : resultOld;
10501053
if (resultOld.commentReferable != null) {
10511054
markdownStats.resolvedOldLookupReferences++;
10521055
}
10531056
} else {
1054-
if (experimentalReferenceLookup) {
1057+
if (enhancedReferenceLookup) {
10551058
result = _getMatchingLinkElementCommentReferable(codeRef, warnable);
10561059
} else {
10571060
result = _getMatchingLinkElementLegacy(codeRef, warnable);

lib/src/model/prefix.dart

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:analyzer/dart/element/element.dart';
66
import 'package:analyzer/dart/element/scope.dart';
7+
import 'package:analyzer/src/dart/element/element.dart';
78
import 'package:dartdoc/src/model/comment_referable.dart';
89
import 'package:dartdoc/src/model/library.dart';
910

@@ -12,21 +13,34 @@ import '../../dartdoc.dart';
1213
/// Represents a [PrefixElement] for dartdoc.
1314
///
1415
/// Like [Parameter], it doesn't have doc pages, but participates in lookups.
16+
/// Forwards to its referenced library if referred to directly.
1517
class Prefix extends ModelElement implements EnclosedElement {
1618
/// [library] is the library the prefix is defined in, not the [Library]
1719
/// referred to by the [PrefixElement].
1820
Prefix(PrefixElement element, Library library, PackageGraph packageGraph)
1921
: super(element, library, packageGraph);
2022

2123
@override
22-
// TODO(jcollins-g): consider allowing bare prefixes to link to a library doc?
2324
bool get isCanonical => false;
2425

26+
Library _associatedLibrary;
27+
// TODO(jcollins-g): consider connecting PrefixElement to the imported library
28+
// in analyzer?
29+
Library get associatedLibrary =>
30+
_associatedLibrary ??= ModelElement.fromElement(
31+
library.element.imports
32+
.firstWhere((i) => i.prefix == element)
33+
.importedLibrary,
34+
packageGraph);
35+
36+
@override
37+
Library get canonicalModelElement => associatedLibrary.canonicalLibrary;
38+
2539
@override
2640
Scope get scope => element.scope;
2741

2842
@override
29-
PrefixElement get element => super.element;
43+
PrefixElementImpl get element => super.element;
3044

3145
@override
3246
ModelElement get enclosingElement => library;
@@ -36,7 +50,8 @@ class Prefix extends ModelElement implements EnclosedElement {
3650
throw UnimplementedError('prefixes have no generated files in dartdoc');
3751

3852
@override
39-
String get href => null;
53+
String get href =>
54+
canonicalModelElement == null ? null : canonicalModelElement.href;
4055

4156
@override
4257
String get kind => 'prefix';

test/end2end/model_test.dart

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,7 +1897,7 @@ void main() {
18971897
});
18981898

18991899
test('correctly finds all the classes', () {
1900-
expect(classes, hasLength(38));
1900+
expect(classes, hasLength(39));
19011901
});
19021902

19031903
test('abstract', () {
@@ -2140,10 +2140,10 @@ void main() {
21402140

21412141
MatchingLinkResult originalLookup(Warnable element, String codeRef) =>
21422142
definingLinkResult(getMatchingLinkElement(element, codeRef,
2143-
experimentalReferenceLookup: false));
2143+
enhancedReferenceLookup: false));
21442144
MatchingLinkResult newLookup(Warnable element, String codeRef) =>
21452145
definingLinkResult(getMatchingLinkElement(element, codeRef,
2146-
experimentalReferenceLookup: true));
2146+
enhancedReferenceLookup: true));
21472147

21482148
MatchingLinkResult bothLookup(Warnable element, String codeRef) {
21492149
var originalLookupResult = originalLookup(element, codeRef);
@@ -2254,8 +2254,11 @@ void main() {
22542254

22552255
group('Ordinary namespace cases', () {
22562256
Package DartPackage;
2257-
Library Dart;
2258-
ModelFunction doesStuff, function1, topLevelFunction;
2257+
Library Dart, mylibpub;
2258+
ModelFunction doesStuff,
2259+
function1,
2260+
topLevelFunction,
2261+
aFunctionUsingRenamedLib;
22592262
TopLevelVariable incorrectDocReference,
22602263
incorrectDocReferenceFromEx,
22612264
nameWithTwoUnderscores,
@@ -2281,6 +2284,10 @@ void main() {
22812284
aConstructorShadowedField;
22822285

22832286
setUpAll(() async {
2287+
mylibpub = packageGraph.allLibraries.values
2288+
.firstWhere((l) => l.name == 'mylibpub');
2289+
aFunctionUsingRenamedLib = fakeLibrary.functions
2290+
.firstWhere((f) => f.name == 'aFunctionUsingRenamedLib');
22842291
Dart = packageGraph.allLibraries.values
22852292
.firstWhere((l) => l.name == 'Dart');
22862293
DartPackage = packageGraph.packages.firstWhere((p) => p.name == 'Dart');
@@ -2356,6 +2363,16 @@ void main() {
23562363
.firstWhere((f) => f.name == 'aConstructorShadowed');
23572364
});
23582365

2366+
test('Referring to a renamed library directly works', () {
2367+
// The new code forwards from the prefix, so doesn't quite work the
2368+
// same as the old for an equality comparison via [MatchingLinkResult].
2369+
expect(
2370+
(bothLookup(aFunctionUsingRenamedLib, 'renamedLib').commentReferable
2371+
as ModelElement)
2372+
.canonicalModelElement,
2373+
equals(mylibpub));
2374+
});
2375+
23592376
test('Referring to libraries and packages with the same name is fine',
23602377
() {
23612378
expect(bothLookup(Apple, 'Dart'), equals(MatchingLinkResult(Dart)));
@@ -2660,20 +2677,20 @@ void main() {
26602677
megaTron.potentiallyApplicableExtensions, orderedEquals([arm, leg]));
26612678
});
26622679

2663-
// TODO(jcollins-g): implement feature and update tests
26642680
test('documentation links do not crash in base cases', () {
2665-
packageGraph.packageWarningCounter.hasWarning(
2666-
doStuff,
2667-
PackageWarning.notImplemented,
2668-
'Comment reference resolution inside extension methods is not yet implemented');
2669-
packageGraph.packageWarningCounter.hasWarning(
2670-
doSomeStuff,
2671-
PackageWarning.notImplemented,
2672-
'Comment reference resolution inside extension methods is not yet implemented');
2681+
// Parameters are OK.
26732682
expect(doStuff.documentationAsHtml, contains('<code>another</code>'));
2674-
expect(doSomeStuff.documentationAsHtml,
2675-
contains('<code>String.extensionNumber</code>'));
2676-
});
2683+
expect(
2684+
doStuff.documentationAsHtml,
2685+
contains(
2686+
'<a href="%%__HTMLBASE_dartdoc_internal__%%ex/AnExtendableThing/aMember.html">aMember</a>'));
2687+
expect(
2688+
doStuff.documentationAsHtml,
2689+
contains(
2690+
'<a href="%%__HTMLBASE_dartdoc_internal__%%ex/AnExtendableThing-class.html">AnExtendableThing</a>'));
2691+
// TODO(jcollins-g): consider linking via applied extensions?
2692+
expect(doSomeStuff.documentationAsHtml, contains('<code>aMember</code>'));
2693+
}, skip: 'unskip when enhanced lookups are on by default');
26772694

26782695
test(
26792696
'references from outside an extension refer correctly to the extension',

testing/test_package/lib/example.dart

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -610,22 +610,22 @@ extension AnExtension<Q> on WithGeneric<Q> {
610610
int call(String s) => 0;
611611
}
612612

613-
extension SimpleStringExtension on String {
613+
class AnExtendableThing {
614+
int aMember;
615+
}
616+
617+
extension SimpleStringExtension on AnExtendableThing {
614618
/// Print this and [another].
615-
/// Refer to [indexOf], from [String].
619+
/// Refer to [aMember], from [AnExtendableThing].
616620
/// Also refer to [extensionNumber].
617-
void doStuff(String another) {
618-
print(this + another);
619-
}
621+
void doStuff(String another) {}
620622

621623
int get extensionNumber => 3;
622624
}
623625

624626
class ExtensionUser {
625-
/// Refer to [String.extensionNumber], which we use here.
626-
void doSomeStuff(String things) {
627-
print(things.extensionNumber + 1);
628-
}
627+
/// Refer to [aMember], which we use here.
628+
void doSomeStuff(String things) {}
629629
}
630630

631631
/// Extension on List

0 commit comments

Comments
 (0)