-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve non-imported symbols in comments [#1153] #1298
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,12 @@ const List<String> _oneLinerSkipTags = const ["code", "pre"]; | |
|
||
final List<md.InlineSyntax> _markdown_syntaxes = [new _InlineCodeSyntax()]; | ||
|
||
class MatchingLinkResult { | ||
final ModelElement element; | ||
final String label; | ||
MatchingLinkResult(this.element, this.label); | ||
} | ||
|
||
// TODO: this is in the wrong place | ||
NodeList<CommentReference> _getCommentRefs(ModelElement modelElement) { | ||
if (modelElement == null) return null; | ||
|
@@ -67,10 +73,10 @@ NodeList<CommentReference> _getCommentRefs(ModelElement modelElement) { | |
} | ||
|
||
/// Returns null if element is a parameter. | ||
ModelElement _getMatchingLinkElement( | ||
MatchingLinkResult _getMatchingLinkElement( | ||
String codeRef, ModelElement element, List<CommentReference> commentRefs, | ||
{bool isConstructor: false}) { | ||
if (commentRefs == null) return null; | ||
if (commentRefs == null) return new MatchingLinkResult(null, null); | ||
|
||
Element refElement; | ||
bool isEnum = false; | ||
|
@@ -87,7 +93,9 @@ ModelElement _getMatchingLinkElement( | |
} | ||
|
||
// Did not find an element in scope | ||
if (refElement == null) return null; | ||
if (refElement == null) { | ||
return _findRefElementInLibrary(codeRef, element, commentRefs); | ||
} | ||
|
||
if (refElement is PropertyAccessorElement) { | ||
// yay we found an accessor that wraps a const, but we really | ||
|
@@ -99,7 +107,7 @@ ModelElement _getMatchingLinkElement( | |
} | ||
} | ||
|
||
if (refElement is ParameterElement) return null; | ||
if (refElement is ParameterElement) return new MatchingLinkResult(null, null); | ||
|
||
// bug! this can fail to find the right library name if the element's name | ||
// we're looking for is the same as a name that comes in from an imported | ||
|
@@ -114,37 +122,79 @@ ModelElement _getMatchingLinkElement( | |
// Is there a way to pull this from a registry of known elements? | ||
// Seems like we're creating too many objects this way. | ||
if (isEnum) { | ||
return new EnumField(refElement, refLibrary); | ||
return new MatchingLinkResult(new EnumField(refElement, refLibrary), null); | ||
} | ||
return new ModelElement.from(refElement, refLibrary); | ||
return new MatchingLinkResult(new ModelElement.from(refElement, refLibrary), null); | ||
} | ||
return new MatchingLinkResult(null, null); | ||
} | ||
|
||
MatchingLinkResult _findRefElementInLibrary(String codeRef, ModelElement element, List<CommentReference> commentRefs) { | ||
final package = element.library.package; | ||
final Map<String, ModelElement> result = {}; | ||
package.libraries.forEach((library) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This lookup should probably live somewhere more general, like as a method on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good point! |
||
final List<ModelElement> modelElements = [] | ||
..addAll(library.allClasses) | ||
..addAll(library.constants) | ||
..addAll(library.enums) | ||
..addAll(library.functions) | ||
..addAll(library.properties) | ||
..addAll(library.typedefs); | ||
|
||
library.allClasses.forEach((c) { | ||
modelElements.addAll(c.allInstanceMethods); | ||
modelElements.addAll(c.allInstanceProperties); | ||
modelElements.addAll(c.allOperators); | ||
modelElements.addAll(c.staticMethods); | ||
modelElements.addAll(c.staticProperties); | ||
}); | ||
|
||
modelElements.forEach((modelElement) { | ||
if (codeRef == modelElement.name || codeRef == modelElement.fullyQualifiedName) { | ||
result[modelElement.fullyQualifiedName] = modelElement; | ||
} | ||
}); | ||
}); | ||
|
||
if (result.isEmpty) { | ||
return new MatchingLinkResult(null, null); | ||
} else if (result.length == 1) { | ||
return new MatchingLinkResult(result.values.first, result.values.first.name); | ||
} else { | ||
if (_emitWarning) { | ||
print("Abiguous reference '${codeRef}' in '${element.fullyQualifiedName}', " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sp: ambiguous Also, can you get the file and line number here in order to print them out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something like: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although, I see that we're not doing this (locating by file and line number) in the original code, so while this would be great, it's not necessary for this PR. |
||
"we found it in the following elements: ${result.keys.map((k) => "'${k}'").join(", ")}"); | ||
} | ||
return new MatchingLinkResult(null, null); | ||
} | ||
return null; | ||
} | ||
|
||
String _linkDocReference(String reference, ModelElement element, | ||
NodeList<CommentReference> commentRefs) { | ||
ModelElement linkedElement; | ||
// support for [new Constructor] and [new Class.namedCtr] | ||
var refs = reference.split(' '); | ||
MatchingLinkResult result; | ||
if (refs.length == 2 && refs.first == 'new') { | ||
linkedElement = _getMatchingLinkElement(refs[1], element, commentRefs, | ||
result = _getMatchingLinkElement(refs[1], element, commentRefs, | ||
isConstructor: true); | ||
} else { | ||
linkedElement = _getMatchingLinkElement(reference, element, commentRefs); | ||
result = _getMatchingLinkElement(reference, element, commentRefs); | ||
} | ||
final linkedElement = result.element; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add the type here (and below). |
||
final label = result.label ?? reference; | ||
if (linkedElement != null) { | ||
var classContent = ''; | ||
if (linkedElement.isDeprecated) { | ||
classContent = 'class="deprecated" '; | ||
} | ||
// this would be linkedElement.linkedName, but link bodies are slightly | ||
// different for doc references. sigh. | ||
return '<a ${classContent}href="${linkedElement.href}">$reference</a>'; | ||
return '<a ${classContent}href="${linkedElement.href}">$label</a>'; | ||
} else { | ||
if (_emitWarning) { | ||
print(" warning: unresolved doc reference '$reference' (in $element)"); | ||
} | ||
return '<code>${HTML_ESCAPE.convert(reference)}</code>'; | ||
return '<code>${HTML_ESCAPE.convert(label)}</code>'; | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add the type here