analyzer: only parse a link reference def at beginning of a line
Fixes https://github.com/dart-lang/linter/issues/1878
The analyzer does not have a full Markdown parser (CommonMark or
otherwise), but it does a small job to try and distinguish inline
links, reference links, and link reference definitions, from code
references. The issue here is that link reference definitions can only
occur at the beginning of a line, which analyzer was not accounting
for. So text like `/// Text, [a]: text` should not be parsed as a
link reference definition, but as a code reference instead.
Coupled with the analyzer's parser changes, I make similar changes to
the comment_references lint rule.
Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: I55a6dad9587bc5556592774babb9766ea040d7b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375081
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
diff --git a/pkg/analyzer/lib/src/fasta/doc_comment_builder.dart b/pkg/analyzer/lib/src/fasta/doc_comment_builder.dart
index 13c77c5..ac320ca 100644
--- a/pkg/analyzer/lib/src/fasta/doc_comment_builder.dart
+++ b/pkg/analyzer/lib/src/fasta/doc_comment_builder.dart
@@ -48,25 +48,35 @@
bool _isEqualSign(int character) => character == 0x3D /* '=' */;
-/// Given that we have just found bracketed text within the given [comment],
-/// looks to see whether that text is (a) followed by a parenthesized link
-/// address, (b) followed by a colon, or (c) followed by optional whitespace
-/// and another square bracket.
+/// Returns whether bracketed text, ending at [rightIndex], appears to be a
+/// Markdown link
///
-/// [rightIndex] is the index of the right bracket. Return `true` if the
-/// bracketed text is followed by a link address.
+/// Given that we have just found bracketed text (surrounded with '[' and ']')
+/// within the given [comment], looks to see whether that text is (a) followed
+/// by a parenthesized link address (maybe an [inline link][]), (b) followed by
+/// a colon (maybe a [link reference definition][]), or (c) followed by optional
+/// whitespace and another left square bracket (maybe a [reference link][]).
+///
+/// [rightIndex] is the index of the right bracket.
///
/// This method uses the syntax described by the
/// <a href="http://daringfireball.net/projects/markdown/syntax">markdown</a>
/// project.
-bool _isLinkText(String comment, int rightIndex) {
+/// [inline link]: https://spec.commonmark.org/0.31.2/#inline-link
+/// [link reference definition]: https://spec.commonmark.org/0.31.2/#link-reference-definitions
+/// [reference link]: https://spec.commonmark.org/0.31.2/#reference-link
+bool _isLinkText(String comment, int rightIndex,
+ {required bool canBeLinkReference}) {
var length = comment.length;
var index = rightIndex + 1;
if (index >= length) {
return false;
}
var ch = comment.codeUnitAt(index);
- if (ch == 0x28 || ch == 0x3A) {
+ if (ch == 0x28 /* `(` */) {
+ return true;
+ }
+ if (canBeLinkReference && ch == 0x3A /* `:` */) {
return true;
}
while (isWhitespace(ch)) {
@@ -76,10 +86,10 @@
}
ch = comment.codeUnitAt(index);
}
- return ch == 0x5B;
+ return ch == 0x5B /* `[` */;
}
-bool _isRightCurlyBrace(int character) => character == 0x7D /* '}' */;
+bool _isRightCurlyBrace(int character) => character == 0x7D /* `}` */;
/// Reads past any opening whitespace in [content], returning the index after
/// the last whitespace character.
@@ -95,8 +105,8 @@
return index;
}
-/// A class which temporarily stores data for a [CommentType.DOCUMENTATION]-type
-/// [Comment], which is ultimately built with [build].
+/// A class which temporarily stores data for a documentation [Comment], which
+/// is ultimately built with [build].
final class DocCommentBuilder {
final Parser _parser;
final ErrorReporter? _errorReporter;
@@ -268,7 +278,7 @@
} else if (_parseNodoc(index: whitespaceEndIndex, content: content)) {
isPreviousLineEmpty = false;
} else {
- _parseDocCommentLine(offset, content);
+ _parseReferences(offset, content);
isPreviousLineEmpty = content.isEmpty;
}
lineInfo = _characterSequence.next();
@@ -299,56 +309,6 @@
_docDirectives.addAll(blockDocDirectiveBuilder.innerDocDirectives);
}
- /// Parses the comment references in [content] which starts at [offset].
- void _parseDocCommentLine(int offset, String content) {
- var index = 0;
- var end = content.length;
- while (index < end) {
- var ch = content.codeUnitAt(index);
- if (ch == 0x5B /* `[` */) {
- ++index;
- if (index < end && content.codeUnitAt(index) == 0x3A /* `:` */) {
- // Skip old-style code block.
- index = content.indexOf(':]', index + 1) + 1;
- if (index == 0 || index > end) {
- break;
- }
- } else {
- var referenceStart = index;
- index = content.indexOf(']', index);
- var isSynthetic = false;
- if (index == -1 || index >= end) {
- // Recovery: terminating ']' is not typed yet.
- index = _findCommentReferenceEnd(content, referenceStart, end);
- isSynthetic = true;
- }
- if (ch != 0x27 /* `'` */ && ch != 0x22 /* `"` */) {
- if (_isLinkText(content, index)) {
- // TODO(brianwilkerson): Handle the case where there's a library
- // URI in the link text.
- } else {
- var reference = _parseOneCommentReference(
- content.substring(referenceStart, index),
- offset + referenceStart,
- isSynthetic: isSynthetic,
- );
- if (reference != null) {
- _references.add(reference);
- }
- }
- }
- }
- } else if (ch == 0x60 /* '`' */) {
- // Skip inline code block if there is both starting '`' and ending '`'.
- var endCodeBlock = content.indexOf('`', index + 1);
- if (endCodeBlock != -1 && endCodeBlock < end) {
- index = endCodeBlock;
- }
- }
- ++index;
- }
- }
-
bool _parseDocDirectiveTag({required int index, required String content}) {
const openingLength = '{@'.length;
if (!content.startsWith('{@', index)) return false;
@@ -791,6 +751,61 @@
}
}
+ /// Parses the comment references in [content] which starts at [offset].
+ void _parseReferences(int offset, String content) {
+ var index = 0;
+ var end = content.length;
+ var seenOnlyWhitespace = true;
+ while (index < end) {
+ var ch = content.codeUnitAt(index);
+ if (ch == 0x5B /* `[` */) {
+ //var canBeLinkReference = !seenNonWhitespace;
+ ++index;
+ if (index < end && content.codeUnitAt(index) == 0x3A /* `:` */) {
+ // Skip old-style code block, e.g. `/// Text [:int:]`.
+ index = content.indexOf(':]', index + 1) + 1;
+ if (index == 0 || index > end) {
+ break;
+ }
+ } else {
+ var referenceStart = index;
+ index = content.indexOf(']', index);
+ var isSynthetic = false;
+ if (index == -1 || index >= end) {
+ // Recovery: terminating ']' is not typed yet.
+ index = _findCommentReferenceEnd(content, referenceStart, end);
+ isSynthetic = true;
+ }
+ if (_isLinkText(content, index,
+ canBeLinkReference: seenOnlyWhitespace)) {
+ // TODO(brianwilkerson): Handle the case where there's a library
+ // URI in the link text.
+ } else {
+ var reference = _parseOneCommentReference(
+ content.substring(referenceStart, index),
+ offset + referenceStart,
+ isSynthetic: isSynthetic,
+ );
+ if (reference != null) {
+ _references.add(reference);
+ }
+ }
+ }
+ seenOnlyWhitespace = false;
+ } else if (ch == 0x60 /* '`' */) {
+ // Skip inline code block if there is both starting '`' and ending '`'.
+ var endCodeBlock = content.indexOf('`', index + 1);
+ if (endCodeBlock != -1 && endCodeBlock < end) {
+ index = endCodeBlock;
+ }
+ seenOnlyWhitespace = false;
+ } else if (!isWhitespace(ch)) {
+ seenOnlyWhitespace = false;
+ }
+ ++index;
+ }
+ }
+
void _pushBlockDocDirectiveAndInnerDirectives(
_BlockDocDirectiveBuilder builder) {
_pushDocDirective(builder.build());
diff --git a/pkg/analyzer/test/src/dart/parser/doc_comment_test.dart b/pkg/analyzer/test/src/dart/parser/doc_comment_test.dart
index eb515c0..1ed3b8c 100644
--- a/pkg/analyzer/test/src/dart/parser/doc_comment_test.dart
+++ b/pkg/analyzer/test/src/dart/parser/doc_comment_test.dart
@@ -210,6 +210,25 @@
''');
}
+ test_commentReference_followedByColon() {
+ var parseResult = parseStringWithErrors(r'''
+/// Regarding [a]: it's an A.
+class A {}
+''');
+ parseResult.assertNoErrors();
+
+ var node = parseResult.findNode.comment('[a]');
+ assertParsedNodeText(node, r'''
+Comment
+ references
+ CommentReference
+ expression: SimpleIdentifier
+ token: a
+ tokens
+ /// Regarding [a]: it's an A.
+''');
+ }
+
test_commentReference_multiple() {
var parseResult = parseStringWithErrors(r'''
/// [a] and [b].
diff --git a/pkg/linter/lib/src/rules/comment_references.dart b/pkg/linter/lib/src/rules/comment_references.dart
index 82dd859..8f7a3cf 100644
--- a/pkg/linter/lib/src/rules/comment_references.dart
+++ b/pkg/linter/lib/src/rules/comment_references.dart
@@ -90,6 +90,8 @@
}
class _Visitor extends SimpleAstVisitor<void> {
+ static final _commentStartPattern = RegExp(r'^///+\s*$');
+
final LintRule rule;
/// Recognized Markdown link references (see
@@ -104,7 +106,7 @@
linkReferences.clear();
// Check for keywords that are not treated as references by the parser
- // but should be flagged by the linter.
+ // but should be reported.
// Note that no special care is taken to handle embedded code blocks.
// TODO(srawlins): Skip over code blocks, made available via
// `Comment.codeBlocks`.
@@ -112,21 +114,28 @@
if (token.isSynthetic) continue;
var comment = token.lexeme;
- var leftIndex = comment.indexOf('[');
- while (leftIndex >= 0) {
- var rightIndex = comment.indexOf(']', leftIndex);
- if (rightIndex >= 0) {
- var reference = comment.substring(leftIndex + 1, rightIndex);
- if (_isParserSpecialCase(reference)) {
- var nameOffset = token.offset + leftIndex + 1;
- rule.reportLintForOffset(nameOffset, reference.length);
- }
- if (rightIndex + 1 < comment.length &&
- comment[rightIndex + 1] == ':') {
- linkReferences.add(reference);
- }
+ var referenceIndices = comment.referenceIndices(0);
+ if (referenceIndices == null) continue;
+ var (leftIndex, rightIndex) = referenceIndices;
+ var prefix = comment.substring(0, leftIndex);
+ if (_commentStartPattern.hasMatch(prefix)) {
+ // Check for a Markdown [link reference
+ // definition](https://spec.commonmark.org/0.31.2/#link-reference-definitions).
+ var reference = comment.substring(leftIndex + 1, rightIndex);
+ if (rightIndex + 1 < comment.length && comment[rightIndex + 1] == ':') {
+ linkReferences.add(reference);
}
- leftIndex = rightIndex < 0 ? -1 : comment.indexOf('[', rightIndex);
+ }
+
+ while (referenceIndices != null) {
+ (leftIndex, rightIndex) = referenceIndices;
+ var reference = comment.substring(leftIndex + 1, rightIndex);
+ if (_isParserSpecialCase(reference)) {
+ var nameOffset = token.offset + leftIndex + 1;
+ rule.reportLintForOffset(nameOffset, reference.length);
+ }
+
+ referenceIndices = comment.referenceIndices(rightIndex);
}
}
}
@@ -159,3 +168,16 @@
reference == 'true' ||
reference == 'false';
}
+
+extension on String {
+ /// Returns the first indices of a left and right bracket, if a left bracket
+ /// is found before a right bracket in this [String], starting at [start], and
+ /// `null` otherwise.
+ (int, int)? referenceIndices(int start) {
+ var leftIndex = indexOf('[', start);
+ if (leftIndex < 0) return null;
+ var rightIndex = indexOf(']', leftIndex);
+ if (rightIndex < 0) return null;
+ return (leftIndex, rightIndex);
+ }
+}
diff --git a/pkg/linter/test/rules/comment_references_test.dart b/pkg/linter/test/rules/comment_references_test.dart
index b7625b0..12ffddd 100644
--- a/pkg/linter/test/rules/comment_references_test.dart
+++ b/pkg/linter/test/rules/comment_references_test.dart
@@ -263,6 +263,15 @@
]);
}
+ test_unknownElement_followedByColon() async {
+ await assertDiagnostics(r'''
+/// Parameter [y]: z.
+void f(int x) {}
+''', [
+ lint(15, 1),
+ ]);
+ }
+
test_unknownElement_twiceDottedName() async {
await assertDiagnostics(r'''
/// Parameter [x.y.z].
diff --git a/pkg/test_runner/lib/src/multitest.dart b/pkg/test_runner/lib/src/multitest.dart
index 7b21b39..cae39d3 100644
--- a/pkg/test_runner/lib/src/multitest.dart
+++ b/pkg/test_runner/lib/src/multitest.dart
@@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
/// Multitests are Dart test scripts containing lines of the form
-/// " [some dart code] //# [key]: [error type]"
+/// ` [some dart code] //# [key]: [error type]`
///
/// For each key in the file, a new test file is made containing all the normal
/// lines of the file, and all of the multitest lines containing that key, in
