Skip to content

Commit fca21f9

Browse files
authored
use analyzer to help resolve commentRefs (#1398)
* use analyzer to help resolve commentRefs * change comment on _getRefElementFromCommentRefs
1 parent 94bf818 commit fca21f9

File tree

3 files changed

+35
-22
lines changed

3 files changed

+35
-22
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
## 0.11.0
22

3+
* Fix resolution of ambiguous classes where the analyzer can help us. #1397
34
* Many cleanups to dartdoc stdout/stderr, error messages, and warnings:
45
* Display fatal errors with 'fatal error' string to distinguish them from ordinary errors
56
* Upgrades to new Package.warn system.

lib/src/markdown_processor.dart

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -236,24 +236,14 @@ MatchingLinkResult _getMatchingLinkElement(
236236

237237
// Try expensive not-scoped lookup.
238238
if (refElement == null) {
239-
refElement = _findRefElementInLibrary(codeRef, element);
239+
refElement = _findRefElementInLibrary(codeRef, element, commentRefs);
240240
}
241241

242242
// This is faster but does not take canonicalization into account; try
243243
// only as a last resort. TODO(jcollins-g): make analyzer comment references
244244
// dartdoc-canonicalization-aware?
245245
if (refElement == null) {
246-
for (CommentReference ref in commentRefs) {
247-
if (ref.identifier.name == codeRef) {
248-
bool isConstrElement =
249-
ref.identifier.staticElement is ConstructorElement;
250-
// Constructors are now handled by library search.
251-
if (!isConstrElement) {
252-
refElement = ref.identifier.staticElement;
253-
break;
254-
}
255-
}
256-
}
246+
refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
257247
}
258248

259249
// Did not find it anywhere.
@@ -302,6 +292,21 @@ MatchingLinkResult _getMatchingLinkElement(
302292
return new MatchingLinkResult(refModelElement, null);
303293
}
304294

295+
/// Given a set of commentRefs, return the one whose name matches the codeRef.
296+
Element _getRefElementFromCommentRefs(List<CommentReference> commentRefs, String codeRef) {
297+
for (CommentReference ref in commentRefs) {
298+
if (ref.identifier.name == codeRef) {
299+
bool isConstrElement =
300+
ref.identifier.staticElement is ConstructorElement;
301+
// Constructors are now handled by library search.
302+
if (!isConstrElement) {
303+
return ref.identifier.staticElement;
304+
}
305+
}
306+
}
307+
return null;
308+
}
309+
305310
/// Returns true if this is a constructor we should consider in
306311
/// _findRefElementInLibrary, or if this isn't a constructor.
307312
bool _ConsiderIfConstructor(String codeRef, ModelElement modelElement) {
@@ -333,7 +338,7 @@ Map<String, Set<ModelElement>> _findRefElementCache;
333338
// TODO(jcollins-g): Subcomponents of this function shouldn't be adding nulls to results, strip the
334339
// removes out that are gratuitous and debug the individual pieces.
335340
// TODO(jcollins-g): A complex package winds up spending a lot of cycles in here. Optimize.
336-
Element _findRefElementInLibrary(String codeRef, ModelElement element) {
341+
Element _findRefElementInLibrary(String codeRef, ModelElement element, List<CommentReference> commentRefs) {
337342
assert(element.package.allLibrariesAdded);
338343

339344
String codeRefChomped = codeRef.replaceFirst(isConstructor, '');
@@ -345,21 +350,21 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element) {
345350
// This might be an operator. Strip the operator prefix and try again.
346351
if (results.isEmpty && codeRef.startsWith('operator')) {
347352
String newCodeRef = codeRef.replaceFirst('operator', '');
348-
return _findRefElementInLibrary(newCodeRef, element);
353+
return _findRefElementInLibrary(newCodeRef, element, commentRefs);
349354
}
350355

351356
results.remove(null);
352357
// Oh, and someone might have some type parameters or other garbage.
353358
if (results.isEmpty && codeRef.contains(trailingIgnoreStuff)) {
354359
String newCodeRef = codeRef.replaceFirst(trailingIgnoreStuff, '');
355-
return _findRefElementInLibrary(newCodeRef, element);
360+
return _findRefElementInLibrary(newCodeRef, element, commentRefs);
356361
}
357362

358363
results.remove(null);
359364
// Oh, and someone might have thrown on a 'const' or 'final' in front.
360365
if (results.isEmpty && codeRef.contains(leadingIgnoreStuff)) {
361366
String newCodeRef = codeRef.replaceFirst(leadingIgnoreStuff, '');
362-
return _findRefElementInLibrary(newCodeRef, element);
367+
return _findRefElementInLibrary(newCodeRef, element, commentRefs);
363368
}
364369

365370
// Maybe this ModelElement has parameters, and this is one of them.
@@ -515,8 +520,20 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element) {
515520
results.removeWhere((r) => !r.fullyQualifiedName.startsWith(startName));
516521
}
517522
}
518-
// TODO(jcollins-g): further disambiguations based on package information?
519523

524+
// TODO(jcollins-g): As a last resort, try disambiguation with commentRefs.
525+
// Maybe one of these is the same as what's resolvable with
526+
// the analyzer, and if so, drop the others. We can't
527+
// do this in reverse order because commentRefs don't know
528+
// about dartdoc canonicalization.
529+
if (results.length > 1) {
530+
Element refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
531+
if (results.any((me) => me.element == refElement)) {
532+
results.removeWhere((me) => me.element != refElement);
533+
}
534+
}
535+
536+
// TODO(jcollins-g): further disambiguations based on package information?
520537
if (results.isEmpty) {
521538
result = null;
522539
} else if (results.length == 1) {

lib/src/model.dart

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3027,11 +3027,6 @@ class Package implements Nameable, Documentable, Locatable {
30273027
warningMessage = 'generic type handled as HTML: """${message}"""';
30283028
break;
30293029
}
3030-
// warningMessage should not contain "file:" or "dart:" -- confuses IntelliJ.
3031-
['file:', 'dart:'].forEach((s) {
3032-
// message can contain user text; nothing we can do about that.
3033-
assert(!warningMessage.contains(s) || message.contains(s));
3034-
});
30353030
String fullMessage =
30363031
"${warningMessage} ${modelElement != null ? modelElement.elementLocation : ''}";
30373032
packageWarningCounter.addWarning(

0 commit comments

Comments
 (0)