From fca21f99ca7d9d4c35000e2d3ad3a8ef14dd83aa Mon Sep 17 00:00:00 2001 From: jcollins-g Date: Thu, 4 May 2017 11:22:58 -0700 Subject: [PATCH] use analyzer to help resolve commentRefs (#1398) * use analyzer to help resolve commentRefs * change comment on _getRefElementFromCommentRefs --- CHANGELOG.md | 1 + lib/src/markdown_processor.dart | 51 ++++++++++++++++++++++----------- lib/src/model.dart | 5 ---- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6f305916..4eca2f3ef2 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 eaa2589bbc..6b63ee1116 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -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. @@ -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 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 @@ Map> _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 commentRefs) { assert(element.package.allLibrariesAdded); String codeRefChomped = codeRef.replaceFirst(isConstructor, ''); @@ -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. @@ -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) { diff --git a/lib/src/model.dart b/lib/src/model.dart index c461f015ba..3dc7db0260 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -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(