diff --git a/lib/src/model/accessor.dart b/lib/src/model/accessor.dart index 30d77d2b56..0a251507f5 100644 --- a/lib/src/model/accessor.dart +++ b/lib/src/model/accessor.dart @@ -72,7 +72,7 @@ class Accessor extends ModelElement { } // TODO(srawlins): This doesn't seem right... the super value has delimiters // (like `///`), but this one doesn't? - return stripComments(super.documentationComment); + return stripCommentDelimiters(super.documentationComment); }(); /// If this is a getter, assume we want synthetic documentation. diff --git a/lib/src/model/documentation_comment.dart b/lib/src/model/documentation_comment.dart index 1365373f26..7f0047244a 100644 --- a/lib/src/model/documentation_comment.dart +++ b/lib/src/model/documentation_comment.dart @@ -96,7 +96,10 @@ mixin DocumentationComment /// `{@}`-style directives (except tool directives), returning the processed /// result. String _processCommentWithoutTools(String documentationComment) { - var docs = stripComments(documentationComment); + // We must first strip the comment of directives like `@docImport`, since + // the offsets are for the source text. + var docs = _stripDocImports(documentationComment); + docs = stripCommentDelimiters(docs); if (docs.contains('{@')) { docs = _injectYouTube(docs); docs = _injectAnimations(docs); @@ -111,9 +114,13 @@ mixin DocumentationComment /// Process [documentationComment], performing various actions based on /// `{@}`-style directives, returning the processed result. @visibleForTesting - Future processComment(String documentationComment) async { - var docs = stripComments(documentationComment); - // Must evaluate tools first, in case they insert any other directives. + Future processComment() async { + // We must first strip the comment of directives like `@docImport`, since + // the offsets are for the source text. + var docs = _stripDocImports(documentationComment); + docs = stripCommentDelimiters(docs); + // Then we evaluate tools, in case they insert any other directives that + // would need to be processed by `processCommentDirectives`. docs = await _evaluateTools(docs); docs = processCommentDirectives(docs); _analyzeCodeBlocks(docs); @@ -557,6 +564,27 @@ mixin DocumentationComment }); } + String _stripDocImports(String content) { + if (modelNode?.commentData case var commentData?) { + var commentOffset = commentData.offset; + var buffer = StringBuffer(); + if (commentData.docImports.isEmpty) return content; + var firstDocImport = commentData.docImports.first; + buffer.write(content.substring(0, firstDocImport.offset - commentOffset)); + var offset = firstDocImport.end - commentOffset; + for (var docImport in commentData.docImports.skip(1)) { + buffer + .write(content.substring(offset, docImport.offset - commentOffset)); + offset = docImport.end - commentOffset; + } + // Write from the end of the last doc-import to the end of the comment. + buffer.write(content.substring(offset)); + return buffer.toString(); + } else { + return content; + } + } + /// Parse and remove {@inject-html ...} in API comments and store /// them in the index on the package, replacing them with a SHA1 hash of the /// contents, where the HTML will be re-injected after Markdown processing of @@ -793,7 +821,7 @@ mixin DocumentationComment assert(_rawDocs == null, 'reentrant calls to _buildDocumentation* not allowed'); // Do not use the sync method if we need to evaluate tools or templates. - var rawDocs = await processComment(documentationComment); + var rawDocs = await processComment(); return _rawDocs = buildDocumentationAddition(rawDocs); } diff --git a/lib/src/model/model_node.dart b/lib/src/model/model_node.dart index 50d65abd12..b21db3f50a 100644 --- a/lib/src/model/model_node.dart +++ b/lib/src/model/model_node.dart @@ -18,14 +18,14 @@ class ModelNode { final int _sourceEnd; final int _sourceOffset; - /// Data about each comment reference found in the doc comment of this node. - final Map? commentReferenceData; + /// Data about the doc comment of this node. + final CommentData? commentData; factory ModelNode( AstNode? sourceNode, Element element, AnalysisContext analysisContext, { - required Map? commentReferenceData, + CommentData? commentData, }) { if (sourceNode == null) { return ModelNode._(element, analysisContext, @@ -44,7 +44,7 @@ class ModelNode { analysisContext, sourceEnd: sourceNode.end, sourceOffset: sourceNode.offset, - commentReferenceData: commentReferenceData, + commentData: commentData, ); } } @@ -54,7 +54,7 @@ class ModelNode { this._analysisContext, { required int sourceEnd, required int sourceOffset, - this.commentReferenceData = const {}, + this.commentData, }) : _sourceEnd = sourceEnd, _sourceOffset = sourceOffset; @@ -79,10 +79,41 @@ class ModelNode { }(); } +/// Comment data from the syntax tree. +/// +/// Various comment data is not available on the analyzer's Element model, so we +/// store it in instances of this class after resolving libraries. +class CommentData { + /// The offset of this comment in the source text. + final int offset; + final List docImports; + final Map references; + + CommentData({ + required this.offset, + required this.docImports, + required this.references, + }); +} + +/// doc-import data from the syntax tree. +/// +/// Comment doc-import data is not available on the analyzer's Element model, so +/// we store it in instances of this class after resolving libraries. +class CommentDocImportData { + /// The offset of the doc import in the source text. + final int offset; + + /// The offset of the end of the doc import in the source text. + final int end; + + CommentDocImportData({required this.offset, required this.end}); +} + /// 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. +/// we store it in instances of this class after resolving libraries. class CommentReferenceData { final Element element; final String name; diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index b0d06033c7..8dead13969 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -238,14 +238,14 @@ class PackageGraph with CommentReferable, Nameable { for (var directive in unit.directives.whereType()) { // 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; + var commentData = directive.documentationComment?.data; _modelNodes.putIfAbsent( resolvedLibrary.element, () => ModelNode( directive, resolvedLibrary.element, analysisContext, - commentReferenceData: commentReferenceData, + commentData: commentData, )); } @@ -287,50 +287,39 @@ class PackageGraph with CommentReferable, Nameable { } void _populateModelNodeFor(Declaration declaration) { - var commentReferenceData = declaration.documentationComment?.data; + var commentData = declaration.documentationComment?.data; + + void addModelNode(Declaration declaration) { + var element = declaration.declaredElement; + if (element == null) { + throw StateError("Expected '$declaration' to declare an element"); + } + _modelNodes.putIfAbsent( + element, + () => ModelNode( + declaration, + element, + analysisContext, + commentData: commentData, + ), + ); + } 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, - commentReferenceData: commentReferenceData, - ), - ); + addModelNode(field); } return; } if (declaration is TopLevelVariableDeclaration) { var fields = declaration.variables.variables; for (var field in fields) { - var element = field.declaredElement!; - _modelNodes.putIfAbsent( - element, - () => ModelNode( - field, - element, - analysisContext, - commentReferenceData: commentReferenceData, - ), - ); + addModelNode(field); } return; } - var element = declaration.declaredElement!; - _modelNodes.putIfAbsent( - element, - () => ModelNode( - declaration, - element, - analysisContext, - commentReferenceData: commentReferenceData, - ), - ); + addModelNode(declaration); } ModelNode? getModelNodeFor(Element element) => _modelNodes[element]; @@ -1073,10 +1062,16 @@ class InheritableElementsKey { extension on Comment { /// A mapping of all comment references to their various data. - Map get data { - if (references.isEmpty) return const {}; + CommentData get data { + var docImportsData = []; + for (var docImport in docImports) { + docImportsData.add( + CommentDocImportData( + offset: docImport.offset, end: docImport.import.end), + ); + } - var data = {}; + var referencesData = {}; for (var reference in references) { var commentReferable = reference.expression; String name; @@ -1096,8 +1091,8 @@ extension on Comment { continue; } - if (staticElement != null && !data.containsKey(name)) { - data[name] = CommentReferenceData( + if (staticElement != null && !referencesData.containsKey(name)) { + referencesData[name] = CommentReferenceData( staticElement, name, commentReferable.offset, @@ -1105,6 +1100,7 @@ extension on Comment { ); } } - return data; + return CommentData( + offset: offset, docImports: docImportsData, references: referencesData); } } diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 0630dff3b1..66babb42a0 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -30,7 +30,8 @@ Iterable stripCommonWhitespace(String str) sync* { } } -String stripComments(String str) { +/// Strips [str] of all comment delimiters. +String stripCommentDelimiters(String str) { if (str.isEmpty) return ''; final buf = StringBuffer(); diff --git a/test/documentation_comment_test.dart b/test/documentation_comment_test.dart index cdad6b2be0..e9701a1a78 100644 --- a/test/documentation_comment_test.dart +++ b/test/documentation_comment_test.dart @@ -28,14 +28,14 @@ class DocumentationCommentTest extends DartdocTestBase { late ModelElement libraryModel; void expectNoWarnings() { - expect(packageGraph.packageWarningCounter.hasWarnings, isFalse); expect(packageGraph.packageWarningCounter.countedWarnings, isEmpty); + expect(packageGraph.packageWarningCounter.hasWarnings, isFalse); } - @override - Future setUp() async { - await super.setUp(); - + Future writePackageWithCommentedLibraries( + List<(String, String)> filesAndComments, { + List additionalArguments = const [], + }) async { projectRoot = utils.writePackage( 'my_package', resourceProvider, packageConfigProvider); projectRoot @@ -46,28 +46,36 @@ class DocumentationCommentTest extends DartdocTestBase { - missing-code-block-language '''); - projectRoot - .getChildAssumingFolder('lib') - .getChildAssumingFile('a.dart') - .writeAsStringSync(''' -/// Documentation comment. -int x = 1; -'''); + for (var (fileName, comment) in filesAndComments) { + projectRoot + .getChildAssumingFolder('lib') + .getChildAssumingFile(fileName) + .writeAsStringSync('$comment\n' + 'library;'); + } var optionSet = DartdocOptionRoot.fromOptionGenerators( 'dartdoc', [createDartdocOptions], packageMetaProvider); optionSet.parseArguments([]); packageGraph = await utils.bootBasicPackage( projectRoot.path, packageMetaProvider, packageConfigProvider, - additionalArguments: []); + additionalArguments: additionalArguments); libraryModel = packageGraph.defaultPackage.libraries.first; } + Future writePackageWithCommentedLibrary( + String comment, { + List additionalArguments = const [], + }) => + writePackageWithCommentedLibraries([('a.dart', comment)], + additionalArguments: additionalArguments); + void test_removesTripleSlashes() async { - var doc = await libraryModel.processComment(''' + await writePackageWithCommentedLibrary(''' /// Text. /// More text. '''); + var doc = libraryModel.documentation; expect(doc, equals(''' Text. @@ -75,10 +83,11 @@ More text.''')); } void test_removesSpaceAfterTripleSlashes() async { - var doc = await libraryModel.processComment(''' + await writePackageWithCommentedLibrary(''' /// Text. /// More text. '''); + var doc = libraryModel.documentation; // TODO(srawlins): Actually, the three spaces before 'More' is perhaps not // the best fit. Should it only be two, to match the indent from the first @@ -89,11 +98,12 @@ Text. } void test_leavesBlankLines() async { - var doc = await libraryModel.processComment(''' + await writePackageWithCommentedLibrary(''' /// Text. /// /// More text. '''); + var doc = libraryModel.documentation; expect(doc, equals(''' Text. @@ -101,14 +111,15 @@ Text. More text.''')); } - void test_processesAanimationDirective() async { - var doc = await libraryModel.processComment(''' + void test_processesAnimationDirective() async { + await writePackageWithCommentedLibrary(''' /// Text. /// /// {@animation 100 200 http://host/path/to/video.mp4 id=barHerderAnimation} /// /// End text. '''); + var doc = libraryModel.documentation; expectNoWarnings(); expect(doc, equals(''' @@ -153,87 +164,175 @@ End text.''')); } void test_rendersUnnamedAnimation() async { - var doc = await libraryModel.processComment(''' + await writePackageWithCommentedLibrary(''' /// First line. /// /// {@animation 100 200 http://host/path/to/video.mp4} '''); + var doc = libraryModel.documentation; expectNoWarnings(); expect(doc, contains('