use analyzer to help resolve commentRefs (#1398)
* use analyzer to help resolve commentRefs
* change comment on _getRefElementFromCommentRefs
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e6f305..4eca2f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
## 0.11.0
+* Fix resolution of ambiguous classes where the analyzer can help us. #1397
* Many cleanups to dartdoc stdout/stderr, error messages, and warnings:
* Display fatal errors with 'fatal error' string to distinguish them from ordinary errors
* Upgrades to new Package.warn system.
diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart
index eaa2589..6b63ee1 100644
--- a/lib/src/markdown_processor.dart
+++ b/lib/src/markdown_processor.dart
@@ -236,24 +236,14 @@
// Try expensive not-scoped lookup.
if (refElement == null) {
- refElement = _findRefElementInLibrary(codeRef, element);
+ refElement = _findRefElementInLibrary(codeRef, element, commentRefs);
}
// This is faster but does not take canonicalization into account; try
// only as a last resort. TODO(jcollins-g): make analyzer comment references
// dartdoc-canonicalization-aware?
if (refElement == null) {
- for (CommentReference ref in commentRefs) {
- if (ref.identifier.name == codeRef) {
- bool isConstrElement =
- ref.identifier.staticElement is ConstructorElement;
- // Constructors are now handled by library search.
- if (!isConstrElement) {
- refElement = ref.identifier.staticElement;
- break;
- }
- }
- }
+ refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
}
// Did not find it anywhere.
@@ -302,6 +292,21 @@
return new MatchingLinkResult(refModelElement, null);
}
+/// Given a set of commentRefs, return the one whose name matches the codeRef.
+Element _getRefElementFromCommentRefs(List<CommentReference> commentRefs, String codeRef) {
+ for (CommentReference ref in commentRefs) {
+ if (ref.identifier.name == codeRef) {
+ bool isConstrElement =
+ ref.identifier.staticElement is ConstructorElement;
+ // Constructors are now handled by library search.
+ if (!isConstrElement) {
+ return ref.identifier.staticElement;
+ }
+ }
+ }
+ return null;
+}
+
/// Returns true if this is a constructor we should consider in
/// _findRefElementInLibrary, or if this isn't a constructor.
bool _ConsiderIfConstructor(String codeRef, ModelElement modelElement) {
@@ -333,7 +338,7 @@
// TODO(jcollins-g): Subcomponents of this function shouldn't be adding nulls to results, strip the
// removes out that are gratuitous and debug the individual pieces.
// TODO(jcollins-g): A complex package winds up spending a lot of cycles in here. Optimize.
-Element _findRefElementInLibrary(String codeRef, ModelElement element) {
+Element _findRefElementInLibrary(String codeRef, ModelElement element, List<CommentReference> commentRefs) {
assert(element.package.allLibrariesAdded);
String codeRefChomped = codeRef.replaceFirst(isConstructor, '');
@@ -345,21 +350,21 @@
// This might be an operator. Strip the operator prefix and try again.
if (results.isEmpty && codeRef.startsWith('operator')) {
String newCodeRef = codeRef.replaceFirst('operator', '');
- return _findRefElementInLibrary(newCodeRef, element);
+ return _findRefElementInLibrary(newCodeRef, element, commentRefs);
}
results.remove(null);
// Oh, and someone might have some type parameters or other garbage.
if (results.isEmpty && codeRef.contains(trailingIgnoreStuff)) {
String newCodeRef = codeRef.replaceFirst(trailingIgnoreStuff, '');
- return _findRefElementInLibrary(newCodeRef, element);
+ return _findRefElementInLibrary(newCodeRef, element, commentRefs);
}
results.remove(null);
// Oh, and someone might have thrown on a 'const' or 'final' in front.
if (results.isEmpty && codeRef.contains(leadingIgnoreStuff)) {
String newCodeRef = codeRef.replaceFirst(leadingIgnoreStuff, '');
- return _findRefElementInLibrary(newCodeRef, element);
+ return _findRefElementInLibrary(newCodeRef, element, commentRefs);
}
// Maybe this ModelElement has parameters, and this is one of them.
@@ -515,8 +520,20 @@
results.removeWhere((r) => !r.fullyQualifiedName.startsWith(startName));
}
}
- // TODO(jcollins-g): further disambiguations based on package information?
+ // TODO(jcollins-g): As a last resort, try disambiguation with commentRefs.
+ // Maybe one of these is the same as what's resolvable with
+ // the analyzer, and if so, drop the others. We can't
+ // do this in reverse order because commentRefs don't know
+ // about dartdoc canonicalization.
+ if (results.length > 1) {
+ Element refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
+ if (results.any((me) => me.element == refElement)) {
+ results.removeWhere((me) => me.element != refElement);
+ }
+ }
+
+ // TODO(jcollins-g): further disambiguations based on package information?
if (results.isEmpty) {
result = null;
} else if (results.length == 1) {
diff --git a/lib/src/model.dart b/lib/src/model.dart
index c461f01..3dc7db0 100644
--- a/lib/src/model.dart
+++ b/lib/src/model.dart
@@ -3027,11 +3027,6 @@
warningMessage = 'generic type handled as HTML: """${message}"""';
break;
}
- // warningMessage should not contain "file:" or "dart:" -- confuses IntelliJ.
- ['file:', 'dart:'].forEach((s) {
- // message can contain user text; nothing we can do about that.
- assert(!warningMessage.contains(s) || message.contains(s));
- });
String fullMessage =
"${warningMessage} ${modelElement != null ? modelElement.elementLocation : ''}";
packageWarningCounter.addWarning(
