Skip to content

Centralize unparsing of comment references #2630

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 30, 2021
Merged

Centralize unparsing of comment references #2630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a9178ed
Flatten from head
jcollins-g Apr 27, 2021
26d01a9
Fix unparsing of comment references
jcollins-g Apr 28, 2021
32ca616
Merge branch 'master' into commentref-unparse
jcollins-g Apr 28, 2021
a46510c
simplify
jcollins-g Apr 28, 2021
773a0cb
Merge branch 'master' into commentref-unparse
jcollins-g Apr 30, 2021
2786435
Fix up comments
jcollins-g Apr 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions lib/src/comment_references/model_comment_reference.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
//
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/file_system/file_system.dart';
import 'package:dartdoc/src/model_utils.dart';

/// A stripped down analyzer AST [CommentReference] containing only that
/// information needed for Dartdoc. Drops link to the [CommentReference]
/// and [ResourceProvider] after construction.
class ModelCommentReference {
final String codeRef;
final Element staticElement;

ModelCommentReference(CommentReference ref, ResourceProvider resourceProvider)
: codeRef = _referenceText(ref, resourceProvider),
staticElement = ref.identifier.staticElement;

/// "Unparse" the code reference into the raw text associated with the
/// [CommentReference].
static String _referenceText(
CommentReference ref, ResourceProvider resourceProvider) {
var contents = getFileContentsFor(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this super expensive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortunately not. It is cached so we only pay a read penalty once per file.

(ref.root as CompilationUnit).declaredElement, resourceProvider);
return contents.substring(ref.offset, ref.end);
}
}
48 changes: 38 additions & 10 deletions lib/src/generator/templates.renderers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,12 +614,13 @@ class _Renderer_Canonicalization extends RendererBase<Canonicalization> {
getValue: (CT_ c) => c.commentRefs,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'List<ModelCommentReference>'),
renderIterable:
self.renderSimpleVariable(c, remainingNames,
'Map<String, ModelCommentReference>'),
isNullValue: (CT_ c) => c.commentRefs == null,
renderValue:
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
return c.commentRefs.map(
(e) => renderSimple(e, ast, r.template, parent: r));
return renderSimple(c.commentRefs, ast, r.template,
parent: r);
},
),
'isCanonical': Property(
Expand Down Expand Up @@ -902,6 +903,19 @@ class _Renderer_Category extends RendererBase<Category> {
(e) => _render_Class(e, ast, r.template, parent: r));
},
),
'commentRefs': Property(
getValue: (CT_ c) => c.commentRefs,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(c, remainingNames,
'Map<String, ModelCommentReference>'),
isNullValue: (CT_ c) => c.commentRefs == null,
renderValue:
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
return renderSimple(c.commentRefs, ast, r.template,
parent: r);
},
),
'config': Property(
getValue: (CT_ c) => c.config,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down Expand Up @@ -8912,12 +8926,13 @@ class _Renderer_ModelElement extends RendererBase<ModelElement> {
getValue: (CT_ c) => c.commentRefs,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'List<ModelCommentReference>'),
renderIterable:
self.renderSimpleVariable(c, remainingNames,
'Map<String, ModelCommentReference>'),
isNullValue: (CT_ c) => c.commentRefs == null,
renderValue:
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
return c.commentRefs.map(
(e) => renderSimple(e, ast, r.template, parent: r));
return renderSimple(c.commentRefs, ast, r.template,
parent: r);
},
),
'compilationUnitElement': Property(
Expand Down Expand Up @@ -10394,6 +10409,19 @@ class _Renderer_Package extends RendererBase<Package> {
(e) => _render_Category(e, ast, r.template, parent: r));
},
),
'commentRefs': Property(
getValue: (CT_ c) => c.commentRefs,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(c, remainingNames,
'Map<String, ModelCommentReference>'),
isNullValue: (CT_ c) => c.commentRefs == null,
renderValue:
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
return renderSimple(c.commentRefs, ast, r.template,
parent: r);
},
),
'config': Property(
getValue: (CT_ c) => c.config,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down
109 changes: 43 additions & 66 deletions lib/src/markdown_processor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,6 @@ final RegExp nonHTML =
/// parentheses.
final _trailingIgnorePattern = RegExp(r'(<.*>|\(.*\)|\?|!)$');

@Deprecated('Public variable intended to be private; will be removed as early '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo!

'as Dartdoc 1.0.0')
RegExp get trailingIgnoreStuff => _trailingIgnorePattern;

/// Things to ignore at the beginning of a doc reference.
///
/// This is intended to catch various keywords that a developer may include in
Expand All @@ -139,18 +135,10 @@ RegExp get trailingIgnoreStuff => _trailingIgnorePattern;
final _leadingIgnorePattern =
RegExp(r'^(const|final|var)[\s]+', multiLine: true);

@Deprecated('Public variable intended to be private; will be removed as early '
'as Dartdoc 1.0.0')
RegExp get leadingIgnoreStuff => _leadingIgnorePattern;

/// If found, this pattern may indicate a reference to a constructor.
final _constructorIndicationPattern =
RegExp(r'(^new[\s]+|\(\)$)', multiLine: true);

@Deprecated('Public variable intended to be private; will be removed as early '
'as Dartdoc 1.0.0')
RegExp get isConstructor => _constructorIndicationPattern;

/// A pattern indicating that text which looks like a doc reference is not
/// intended to be.
///
Expand Down Expand Up @@ -288,8 +276,8 @@ ModelElement _getPreferredClass(ModelElement modelElement) {
}

/// Implements _getMatchingLinkElement via [CommentReferable.referenceBy].
MatchingLinkResult _getMatchingLinkElementCommentReferable(String codeRef,
Warnable warnable, List<ModelCommentReference> commentRefs) {
MatchingLinkResult _getMatchingLinkElementCommentReferable(
String codeRef, Warnable warnable) {
if (!codeRef.contains(_constructorIndicationPattern) &&
codeRef.contains(notARealDocReference)) {
// Don't waste our time on things we won't ever find.
Expand All @@ -304,8 +292,8 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable(String codeRef,
}

/// Returns null if element is a parameter.
MatchingLinkResult _getMatchingLinkElementLegacy(String codeRef,
Warnable warnable, List<ModelCommentReference> commentRefs) {
MatchingLinkResult _getMatchingLinkElementLegacy(
String codeRef, Warnable warnable) {
if (!codeRef.contains(_constructorIndicationPattern) &&
codeRef.contains(notARealDocReference)) {
// Don't waste our time on things we won't ever find.
Expand All @@ -322,9 +310,9 @@ MatchingLinkResult _getMatchingLinkElementLegacy(String codeRef,
message:
'Comment reference resolution inside extension methods is not yet implemented');
} else {
refModelElement = _MarkdownCommentReference(
codeRef, warnable, commentRefs, preferredClass)
.computeReferredElement();
refModelElement =
_MarkdownCommentReference(codeRef, warnable, preferredClass)
.computeReferredElement();
}
}

Expand Down Expand Up @@ -368,27 +356,28 @@ MatchingLinkResult _getMatchingLinkElementLegacy(String codeRef,
return MatchingLinkResult(refModelElement);
}

/// Given a set of commentRefs, return the one whose name matches the codeRef.
Element _getRefElementFromCommentRefs(
List<ModelCommentReference> commentRefs, String codeRef) {
if (commentRefs != null) {
for (var ref in commentRefs) {
if (ref.name == codeRef) {
var isConstrElement = ref.staticElement is ConstructorElement;
// Constructors are now handled by library search.
if (!isConstrElement) {
var refElement = ref.staticElement;
if (refElement is PropertyAccessorElement) {
// yay we found an accessor that wraps a const, but we really
// want the top-level field itself
refElement = (refElement as PropertyAccessorElement).variable;
}
if (refElement is PrefixElement) {
// We found a prefix element, but what we really want is the library element.
refElement = (refElement as PrefixElement).enclosingElement;
}
return refElement;
/// Get the element referred by the [codeRef] in analyzer.
/// Deletes constructors and otherwise messes with the output for the rest
/// of the heuristics.
Element _getRefElementFromCommentRefs(Warnable element, String codeRef) {
if (element.commentRefs != null) {
var ref = element.commentRefs[codeRef];
// ref can be null here if the analyzer failed to recognize this as a
// comment reference (e.g. [Foo.operator []]).
if (ref != null) {
// Constructors are now handled by library search.
if (ref.staticElement is! ConstructorElement) {
var refElement = ref.staticElement;
if (refElement is PropertyAccessorElement) {
// yay we found an accessor that wraps a const, but we really
// want the top-level field itself
refElement = (refElement as PropertyAccessorElement).variable;
}
if (refElement is PrefixElement) {
// We found a prefix element, but what we really want is the library element.
refElement = (refElement as PrefixElement).enclosingElement;
}
return refElement;
}
}
}
Expand All @@ -403,9 +392,6 @@ class _MarkdownCommentReference {
/// The element containing the code reference.
final Warnable element;

/// A list of [ModelCommentReference]s for this element.
final List<ModelCommentReference> commentRefs;

/// Disambiguate inheritance with this class.
final Class preferredClass;

Expand All @@ -421,8 +407,7 @@ class _MarkdownCommentReference {
/// PackageGraph associated with this element.
PackageGraph packageGraph;

_MarkdownCommentReference(
this.codeRef, this.element, this.commentRefs, this.preferredClass) {
_MarkdownCommentReference(this.codeRef, this.element, this.preferredClass) {
assert(element != null);
assert(element.packageGraph.allLibrariesAdded);

Expand Down Expand Up @@ -589,7 +574,7 @@ class _MarkdownCommentReference {
_codeRefChompedParts ??= codeRefChomped.split('.');

void _reducePreferAnalyzerResolution() {
var refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
var refElement = _getRefElementFromCommentRefs(element, codeRef);
if (results.any((me) => me.element == refElement)) {
results.removeWhere((me) => me.element != refElement);
}
Expand Down Expand Up @@ -663,26 +648,23 @@ class _MarkdownCommentReference {
void _findWithoutLeadingIgnoreStuff() {
if (codeRef.contains(_leadingIgnorePattern)) {
var newCodeRef = codeRef.replaceFirst(_leadingIgnorePattern, '');
results.add(_MarkdownCommentReference(
newCodeRef, element, commentRefs, preferredClass)
results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass)
.computeReferredElement());
}
}

void _findWithoutTrailingIgnoreStuff() {
if (codeRef.contains(_trailingIgnorePattern)) {
var newCodeRef = codeRef.replaceFirst(_trailingIgnorePattern, '');
results.add(_MarkdownCommentReference(
newCodeRef, element, commentRefs, preferredClass)
results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass)
.computeReferredElement());
}
}

void _findWithoutOperatorPrefix() {
if (codeRef.startsWith(operatorPrefix)) {
var newCodeRef = codeRef.replaceFirst(operatorPrefix, '');
results.add(_MarkdownCommentReference(
newCodeRef, element, commentRefs, preferredClass)
results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass)
.computeReferredElement());
}
}
Expand Down Expand Up @@ -826,7 +808,7 @@ class _MarkdownCommentReference {
}

void _findAnalyzerReferences() {
var refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
var refElement = _getRefElementFromCommentRefs(element, codeRef);
if (refElement == null) return;

ModelElement refModelElement;
Expand Down Expand Up @@ -943,9 +925,8 @@ const _referenceLookupWarnings = {
PackageWarning.referenceLookupMissingWithNew,
};

md.Node _makeLinkNode(String codeRef, Warnable warnable,
List<ModelCommentReference> commentRefs) {
var result = _getMatchingLinkElement(warnable, codeRef, commentRefs);
md.Node _makeLinkNode(String codeRef, Warnable warnable) {
var result = _getMatchingLinkElement(warnable, codeRef);
var textContent = htmlEscape.convert(codeRef);
var linkedElement = result.modelElement;
if (linkedElement != null) {
Expand Down Expand Up @@ -974,18 +955,16 @@ md.Node _makeLinkNode(String codeRef, Warnable warnable,
return md.Element.text('code', textContent);
}

MatchingLinkResult _getMatchingLinkElement(Warnable warnable, String codeRef,
List<ModelCommentReference> commentRefs) {
MatchingLinkResult _getMatchingLinkElement(Warnable warnable, String codeRef) {
MatchingLinkResult result, resultOld, resultNew;
// Do a comparison between result types only if the warnings for them are
// enabled, because there's a significant performance penalty.
var doComparison = warnable.config.packageWarningOptions.warningModes.entries
.where((entry) => _referenceLookupWarnings.contains(entry.key))
.any((entry) => entry.value != PackageWarningMode.ignore);
if (doComparison) {
resultNew =
_getMatchingLinkElementCommentReferable(codeRef, warnable, commentRefs);
resultOld = _getMatchingLinkElementLegacy(codeRef, warnable, commentRefs);
resultNew = _getMatchingLinkElementCommentReferable(codeRef, warnable);
resultOld = _getMatchingLinkElementLegacy(codeRef, warnable);
if (resultNew.modelElement != null) {
markdownStats.resolvedNewLookupReferences++;
}
Expand All @@ -996,10 +975,9 @@ MatchingLinkResult _getMatchingLinkElement(Warnable warnable, String codeRef,
}
} else {
if (warnable.config.experimentalReferenceLookup) {
result = _getMatchingLinkElementCommentReferable(
codeRef, warnable, commentRefs);
result = _getMatchingLinkElementCommentReferable(codeRef, warnable);
} else {
result = _getMatchingLinkElementLegacy(codeRef, warnable, commentRefs);
result = _getMatchingLinkElementLegacy(codeRef, warnable);
}
}
if (doComparison) {
Expand Down Expand Up @@ -1084,13 +1062,12 @@ Iterable<int> findFreeHangingGenericsPositions(String string) sync* {
}

class MarkdownDocument extends md.Document {
factory MarkdownDocument.withElementLinkResolver(
Canonicalization element, List<ModelCommentReference> commentRefs) {
factory MarkdownDocument.withElementLinkResolver(Canonicalization element) {
md.Node /*?*/ linkResolver(String name, [String /*?*/ _]) {
if (name.isEmpty) {
return null;
}
return _makeLinkNode(name, element, commentRefs);
return _makeLinkNode(name, element);
}

return MarkdownDocument(
Expand Down
2 changes: 1 addition & 1 deletion lib/src/model/accessor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/dart/element/member.dart' show ExecutableMember;
import 'package:dartdoc/src/model/comment_referable.dart';
import 'package:dartdoc/src/element_type.dart';
import 'package:dartdoc/src/model/comment_referable.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/render/source_code_renderer.dart';
import 'package:dartdoc/src/utils.dart';
Expand Down
8 changes: 7 additions & 1 deletion lib/src/model/canonicalization.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:dartdoc/src/comment_references/model_comment_reference.dart';
import 'package:dartdoc/src/model/model.dart';

/// Classes extending this class have canonicalization support in Dartdoc.
Expand All @@ -10,7 +11,12 @@ abstract class Canonicalization implements Locatable, Documentable {

Library get canonicalLibrary;

List<ModelCommentReference> get commentRefs => null;
/// A map of [ModelCommentReference.codeRef] to [ModelCommentReference].
/// This map deduplicates comment references as all identical reference
/// strings inside a single documentation comment will point to the same
/// place, so it should not be used to count exactly how many references
/// there are.
Map<String, ModelCommentReference> get commentRefs;

/// Pieces of the location, split to remove 'package:' and slashes.
Set<String> get locationPieces;
Expand Down
Loading