Skip to content

Commit

Permalink
use analyzer to help resolve commentRefs (#1398)
Browse files Browse the repository at this point in the history
* use analyzer to help resolve commentRefs

* change comment on _getRefElementFromCommentRefs
  • Loading branch information
jcollins-g authored May 4, 2017
1 parent 94bf818 commit fca21f9
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
51 changes: 34 additions & 17 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,24 +236,14 @@ MatchingLinkResult _getMatchingLinkElement(

// 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.
Expand Down Expand Up @@ -302,6 +292,21 @@ MatchingLinkResult _getMatchingLinkElement(
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) {
Expand Down Expand Up @@ -333,7 +338,7 @@ Map<String, Set<ModelElement>> _findRefElementCache;
// 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, '');
Expand All @@ -345,21 +350,21 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element) {
// 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.
Expand Down Expand Up @@ -515,8 +520,20 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element) {
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) {
Expand Down
5 changes: 0 additions & 5 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3027,11 +3027,6 @@ class Package implements Nameable, Documentable, Locatable {
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(
Expand Down

0 comments on commit fca21f9

Please sign in to comment.