Skip to content
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

Jump to bottom

Store comment reference data on ModelNode #3797

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Store comment reference data on ModelNode #3797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 37 additions & 7 deletions lib/src/model/model_node.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,22 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:meta/meta.dart';

/// Stripped down information derived from [AstNode] containing only information
/// needed to resurrect the source code of [element].
/// needed to resurrect the source code of [_element].
class ModelNode {
final Element _element;
final AnalysisContext _analysisContext;
final int _sourceEnd;
final int _sourceOffset;

/// Data about each comment reference found in the doc comment of this node.
final Map<String, CommentReferenceData>? commentReferenceData;

factory ModelNode(
AstNode? sourceNode, Element element, AnalysisContext analysisContext) {
AstNode? sourceNode,
Element element,
AnalysisContext analysisContext, {
required Map<String, CommentReferenceData>? commentReferenceData,
}) {
if (sourceNode == null) {
return ModelNode._(element, analysisContext,
sourceEnd: -1, sourceOffset: -1);
Expand All @@ -32,14 +39,23 @@ class ModelNode {
assert(sourceNode is FieldDeclaration ||
sourceNode is TopLevelVariableDeclaration);
}
return ModelNode._(element, analysisContext,
sourceEnd: sourceNode.end, sourceOffset: sourceNode.offset);
return ModelNode._(
element,
analysisContext,
sourceEnd: sourceNode.end,
sourceOffset: sourceNode.offset,
commentReferenceData: commentReferenceData,
);
}
}

ModelNode._(this._element, this._analysisContext,
{required int sourceEnd, required int sourceOffset})
: _sourceEnd = sourceEnd,
ModelNode._(
this._element,
this._analysisContext, {
required int sourceEnd,
required int sourceOffset,
this.commentReferenceData = const {},
}) : _sourceEnd = sourceEnd,
_sourceOffset = sourceOffset;

bool get _isSynthetic => _sourceEnd < 0 || _sourceOffset < 0;
Expand All @@ -63,6 +79,20 @@ class ModelNode {
}();
}

/// Comment reference data from the syntax tree.
///
/// Comment reference data is not available on the analyzer's Element model, so
/// we store it after resolving libraries in instances of this class.
class CommentReferenceData {
final Element element;
final String name;
final int offset;
final int length;

CommentReferenceData(this.element, String? name, this.offset, this.length)
: name = name ?? '';
}

@visibleForTesting
extension SourceStringExtensions on String {
String substringFromLineStart(int offset, int endOffset) {
Expand Down
87 changes: 83 additions & 4 deletions lib/src/model/package_graph.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
import 'dart:collection';

import 'package:analyzer/dart/analysis/analysis_context.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/source/source.dart';
// ignore: implementation_imports
import 'package:analyzer/src/dart/ast/ast.dart';
// ignore: implementation_imports
import 'package:analyzer/src/dart/element/inheritance_manager3.dart'
show InheritanceManager3;
// ignore: implementation_imports
Expand Down Expand Up @@ -234,6 +235,20 @@ class PackageGraph with CommentReferable, Nameable {
// me how, because the data is on AST nodes, not the element model.
void gatherModelNodes(DartDocResolvedLibrary resolvedLibrary) {
for (var unit in resolvedLibrary.units) {
for (var directive in unit.directives.whereType<LibraryDirective>()) {
// There should be only one library directive. If there are more, there
// is no harm in grabbing ModelNode for each.
var commentReferenceData = directive.documentationComment?.data;
_modelNodes.putIfAbsent(
resolvedLibrary.element,
() => ModelNode(
directive,
resolvedLibrary.element,
analysisContext,
commentReferenceData: commentReferenceData,
));
}

for (var declaration in unit.declarations) {
_populateModelNodeFor(declaration);
switch (declaration) {
Expand All @@ -243,6 +258,9 @@ class PackageGraph with CommentReferable, Nameable {
}
case EnumDeclaration():
if (declaration.declaredElement?.isPublic ?? false) {
for (var constant in declaration.constants) {
_populateModelNodeFor(constant);
}
for (var member in declaration.members) {
_populateModelNodeFor(member);
}
Expand All @@ -269,12 +287,21 @@ class PackageGraph with CommentReferable, Nameable {
}

void _populateModelNodeFor(Declaration declaration) {
var commentReferenceData = declaration.documentationComment?.data;

if (declaration is FieldDeclaration) {
var fields = declaration.fields.variables;
for (var field in fields) {
var element = field.declaredElement!;
_modelNodes.putIfAbsent(
element, () => ModelNode(field, element, analysisContext));
element,
() => ModelNode(
field,
element,
analysisContext,
commentReferenceData: commentReferenceData,
),
);
}
return;
}
Expand All @@ -283,13 +310,27 @@ class PackageGraph with CommentReferable, Nameable {
for (var field in fields) {
var element = field.declaredElement!;
_modelNodes.putIfAbsent(
element, () => ModelNode(field, element, analysisContext));
element,
() => ModelNode(
field,
element,
analysisContext,
commentReferenceData: commentReferenceData,
),
);
}
return;
}
var element = declaration.declaredElement!;
_modelNodes.putIfAbsent(
element, () => ModelNode(declaration, element, analysisContext));
element,
() => ModelNode(
declaration,
element,
analysisContext,
commentReferenceData: commentReferenceData,
),
);
}

ModelNode? getModelNodeFor(Element element) => _modelNodes[element];
Expand Down Expand Up @@ -1029,3 +1070,41 @@ class InheritableElementsKey {

InheritableElementsKey(this.element, this.library);
}

extension on Comment {
/// A mapping of all comment references to their various data.
Map<String, CommentReferenceData> get data {
if (references.isEmpty) return const {};

var data = <String, CommentReferenceData>{};
for (var reference in references) {
var commentReferable = reference.expression;
String name;
Element? staticElement;
if (commentReferable case PropertyAccessImpl(:var propertyName)) {
var target = commentReferable.target;
if (target is! PrefixedIdentifierImpl) continue;
name = '${target.name}.${propertyName.name}';
staticElement = propertyName.staticElement;
} else if (commentReferable case PrefixedIdentifier(:var identifier)) {
name = commentReferable.name;
staticElement = identifier.staticElement;
} else if (commentReferable case SimpleIdentifier()) {
name = commentReferable.name;
staticElement = commentReferable.staticElement;
} else {
continue;
}

if (staticElement != null && !data.containsKey(name)) {
data[name] = CommentReferenceData(
Copy link
Member

Choose a reason for hiding this comment

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

I haven't thought about this deeply like I'm sure you have, but at first glance this appears to be prone to having information overwritten if it's possible to have two comment references with the same name that resolve to different elements.

For example, could a reference to a parameter in one method and a parameter with the same name in a different method cause the map to tell us that both references refer to the second method's parameter? Or a similar question about references to type parameters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! Since this get data is made available on a single Comment, each reference in square brackets with the same name must refer to the same thing. Like in

/// Text [foo] and [bar] and [foo].

There is never a change to the scope within a doc comment, so [foo] and [foo] always refer to the same element.

And then the Map of CommentReferenceData is not merged with any other maps from any other comments. It is just stored in a ModelNode for the documented element.

staticElement,
name,
commentReferable.offset,
commentReferable.length,
);
}
}
return data;
}
}
7 changes: 7 additions & 0 deletions test/dartdoc_test_base.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,11 @@ $libraryContent
);
return await Dartdoc.fromContext(context, packageBuilder);
}

/// The real offset in a library generated with [bootPackageWithLibrary].
///
/// When a library is written via [bootPackageWithLibrary], the test author
/// provides `libraryContent`, which is a snippet of Dart library text.
int realOffsetFor(int offsetInContent) =>
'\n\nlibrary $libraryName\n\n'.length + offsetInContent;
}
Loading