From 210549e524b613e47788dc23a3634e010d4c8faf Mon Sep 17 00:00:00 2001
From: Sam Rawlins <srawlins@google.com>
Date: Tue, 8 Sep 2020 11:52:18 -0700
Subject: [PATCH 1/3] Add a warning when an unknown directive is parsed in a
comment.
The warning helpfully mentions when the case of a name is wrong.
---
lib/src/model/comment_processable.dart | 54 ++++++++++++++++++++++++++
lib/src/model/package_graph.dart | 47 +++++++++++-----------
lib/src/warnings.dart | 5 +++
test/comment_processable_test.dart | 48 ++++++++++++++++-------
4 files changed, 117 insertions(+), 37 deletions(-)
diff --git a/lib/src/model/comment_processable.dart b/lib/src/model/comment_processable.dart
index c9a4cdaba7..c5b1e45599 100644
--- a/lib/src/model/comment_processable.dart
+++ b/lib/src/model/comment_processable.dart
@@ -55,9 +55,11 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
}
String processCommentDirectives(String docs) {
+ // The vast, vast majority of doc comments have no directives.
if (!docs.contains('{@')) {
return docs;
}
+ _checkForUnknownDirectives(docs);
docs = _injectExamples(docs);
docs = _injectYouTube(docs);
docs = _injectAnimations(docs);
@@ -78,6 +80,58 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
ModelElementRenderer get modelElementRenderer =>
packageGraph.rendererFactory.modelElementRenderer;
+ static const _allDirectiveNames = [
+ 'animation',
+ 'end-inject-html',
+ 'end-tool',
+ 'endtemplate',
+ 'example',
+ 'macro',
+ 'inject-html',
+ 'template',
+ 'tool',
+ 'youtube',
+
+ // Categorization directives, parsed elsewhere:
+ 'api',
+ 'canonicalFor',
+ 'category',
+ 'image',
+ 'samples',
+ 'subCategory',
+
+ // Common Dart annotations which may decorate named parameters:
+ 'deprecated',
+ 'required',
+ ];
+
+ static final _nameBreak = RegExp('[\\s}]');
+
+ // TODO(srawlins): Implement more checks; see
+ // https://github.com/dart-lang/dartdoc/issues/1814.
+ void _checkForUnknownDirectives(String docs) {
+ var index = 0;
+ while (true) {
+ var nameStartIndex = docs.indexOf('{@', index);
+ if (nameStartIndex == -1) return;
+ var nameEndIndex = docs.indexOf(_nameBreak, nameStartIndex + 2);
+ if (nameEndIndex == -1) return;
+ var name = docs.substring(nameStartIndex + 2, nameEndIndex);
+ if (!_allDirectiveNames.contains(name)) {
+ if (_allDirectiveNames.contains(name.toLowerCase())) {
+ warn(PackageWarning.unknownDirective,
+ message: "'$name' (use lowercase)");
+ } else {
+ warn(PackageWarning.unknownDirective, message: "'$name'");
+ }
+ }
+ // TODO(srawlins): Replace all `replaceAllMapped` usage within this file,
+ // running regex after regex over [docs], with simple calls here. This has
+ // interactivity / order-of-operations consequences, so take care.
+ index = nameEndIndex;
+ }
+ }
+
/// Replace {@tool ...}{@end-tool} in API comments with the
/// output of an external tool.
///
diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart
index 4169837181..f000aa4932 100644
--- a/lib/src/model/package_graph.dart
+++ b/lib/src/model/package_graph.dart
@@ -341,18 +341,18 @@ class PackageGraph {
// messages and warn for non-public canonicalization
// errors.
warningMessage =
- 'no canonical library found for ${warnableName}, not linking';
+ 'no canonical library found for $warnableName, not linking';
break;
case PackageWarning.ambiguousReexport:
// Fix these warnings by adding the original library exporting the
// symbol with --include, by using --auto-include-dependencies,
// or by using --exclude to hide one of the libraries involved
warningMessage =
- 'ambiguous reexport of ${warnableName}, canonicalization candidates: ${message}';
+ 'ambiguous reexport of $warnableName, canonicalization candidates: $message';
break;
case PackageWarning.noDefiningLibraryFound:
warningMessage =
- 'could not find the defining library for ${warnableName}; the '
+ 'could not find the defining library for $warnableName; the '
'library may be imported or exported with a non-standard URI';
break;
case PackageWarning.noLibraryLevelDocs:
@@ -360,76 +360,79 @@ class PackageGraph {
'${warnable.fullyQualifiedName} has no library level documentation comments';
break;
case PackageWarning.ambiguousDocReference:
- warningMessage = 'ambiguous doc reference ${message}';
+ warningMessage = 'ambiguous doc reference $message';
break;
case PackageWarning.ignoredCanonicalFor:
warningMessage =
- "library says it is {@canonicalFor ${message}} but ${message} can't be canonical there";
+ "library says it is {@canonicalFor $message} but $message can't be canonical there";
break;
case PackageWarning.packageOrderGivesMissingPackageName:
warningMessage =
- "--package-order gives invalid package name: '${message}'";
+ "--package-order gives invalid package name: '$message'";
break;
case PackageWarning.reexportedPrivateApiAcrossPackages:
warningMessage =
- 'private API of ${message} is reexported by libraries in other packages: ';
+ 'private API of $message is reexported by libraries in other packages: ';
break;
case PackageWarning.notImplemented:
warningMessage = message;
break;
case PackageWarning.unresolvedDocReference:
- warningMessage = 'unresolved doc reference [${message}]';
+ warningMessage = 'unresolved doc reference [$message]';
referredFromPrefix = 'in documentation inherited from';
break;
+ case PackageWarning.unknownDirective:
+ warningMessage = 'undefined directive [$message]';
+ break;
case PackageWarning.unknownMacro:
- warningMessage = 'undefined macro [${message}]';
+ warningMessage = 'undefined macro [$message]';
break;
case PackageWarning.unknownHtmlFragment:
- warningMessage = 'undefined HTML fragment identifier [${message}]';
+ warningMessage = 'undefined HTML fragment identifier [$message]';
break;
case PackageWarning.brokenLink:
- warningMessage = 'dartdoc generated a broken link to: ${message}';
+ warningMessage = 'dartdoc generated a broken link to: $message';
warnablePrefix = 'to element';
referredFromPrefix = 'linked to from';
break;
case PackageWarning.orphanedFile:
- warningMessage = 'dartdoc generated a file orphan: ${message}';
+ warningMessage = 'dartdoc generated a file orphan: $message';
break;
case PackageWarning.unknownFile:
warningMessage =
- 'dartdoc detected an unknown file in the doc tree: ${message}';
+ 'dartdoc detected an unknown file in the doc tree: $message';
break;
case PackageWarning.missingFromSearchIndex:
warningMessage =
- 'dartdoc generated a file not in the search index: ${message}';
+ 'dartdoc generated a file not in the search index: $message';
break;
case PackageWarning.typeAsHtml:
// The message for this warning can contain many punctuation and other symbols,
// so bracket with a triple quote for defense.
- warningMessage = 'generic type handled as HTML: """${message}"""';
+ warningMessage = 'generic type handled as HTML: """$message"""';
break;
case PackageWarning.invalidParameter:
- warningMessage = 'invalid parameter to dartdoc directive: ${message}';
+ warningMessage = 'invalid parameter to dartdoc directive: $message';
break;
case PackageWarning.toolError:
- warningMessage = 'tool execution failed: ${message}';
+ warningMessage = 'tool execution failed: $message';
break;
case PackageWarning.deprecated:
- warningMessage = 'deprecated dartdoc usage: ${message}';
+ warningMessage = 'deprecated dartdoc usage: $message';
break;
case PackageWarning.unresolvedExport:
- warningMessage = 'unresolved export uri: ${message}';
+ warningMessage = 'unresolved export uri: $message';
break;
case PackageWarning.duplicateFile:
- warningMessage = 'failed to write file at: ${message}';
+ warningMessage = 'failed to write file at: $message';
warnablePrefix = 'for symbol';
referredFromPrefix = 'conflicting with file already generated by';
break;
case PackageWarning.missingConstantConstructor:
- warningMessage = 'constant constructor missing: ${message}';
+ warningMessage = 'constant constructor missing: $message';
break;
case PackageWarning.missingExampleFile:
- warningMessage = 'example file not found: ${message}';
+ warningMessage = 'example file not found: $message';
break;
}
diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart
index ef564d6464..72f721b377 100644
--- a/lib/src/warnings.dart
+++ b/lib/src/warnings.dart
@@ -177,6 +177,10 @@ final Map<PackageWarning, PackageWarningDefinition> packageWarningDefinitions =
'A comment reference could not be found in parameters, enclosing class, enclosing library, or at the top level of any documented library with the package'),
PackageWarning.brokenLink: PackageWarningDefinition(PackageWarning.brokenLink,
'broken-link', 'Dartdoc generated a link to a non-existent file'),
+ PackageWarning.unknownDirective: PackageWarningDefinition(
+ PackageWarning.unknownDirective,
+ 'unknown-directive',
+ 'A comment contains an unknown directive'),
PackageWarning.unknownMacro: PackageWarningDefinition(
PackageWarning.unknownMacro,
'unknown-macro',
@@ -266,6 +270,7 @@ enum PackageWarning {
packageOrderGivesMissingPackageName,
reexportedPrivateApiAcrossPackages,
unresolvedDocReference,
+ unknownDirective,
unknownMacro,
unknownHtmlFragment,
brokenLink,
diff --git a/test/comment_processable_test.dart b/test/comment_processable_test.dart
index 1be4c2d7bd..2646ee5fc6 100644
--- a/test/comment_processable_test.dart
+++ b/test/comment_processable_test.dart
@@ -100,6 +100,32 @@ Text.
More text.'''));
});
+ test('warns when an unknown directive is parsed', () async {
+ processor.element = _FakeElement(source: _FakeSource(fullName: libFooPath));
+ await processor.processComment('''
+/// Text.
+///
+/// {@marco name}
+''');
+ verify(processor.packageGraph.warnOnElement(
+ processor, PackageWarning.unknownDirective,
+ message: "'marco'"))
+ .called(1);
+ });
+
+ test('warns when a directive with wrong case is parsed', () async {
+ processor.element = _FakeElement(source: _FakeSource(fullName: libFooPath));
+ await processor.processComment('''
+/// Text.
+///
+/// {@youTube url}
+''');
+ verify(processor.packageGraph.warnOnElement(
+ processor, PackageWarning.unknownDirective,
+ message: "'youTube' (use lowercase)"))
+ .called(1);
+ });
+
test('processes @animation', () async {
processor.element = _FakeElement(source: _FakeSource(fullName: libFooPath));
var doc = await processor.processComment('''
@@ -369,9 +395,7 @@ End text.'''));
});
test('processes @example with file', () async {
- var examplePath = resourceProvider.pathContext.canonicalize(
- resourceProvider.pathContext.join(projectRoot.path, 'abc.md'));
- resourceProvider.getFile(examplePath).writeAsStringSync('''
+ projectRoot.getChildAssumingFile('abc.md').writeAsStringSync('''
```
Code snippet
```
@@ -398,9 +422,9 @@ End text.'''));
});
test('processes @example with a region', () async {
- var examplePath = resourceProvider.pathContext.canonicalize(
- resourceProvider.pathContext.join(projectRoot.path, 'abc-r.md'));
- resourceProvider.getFile(examplePath).writeAsStringSync('Markdown text.');
+ projectRoot
+ .getChildAssumingFile('abc-r.md')
+ .writeAsStringSync('Markdown text.');
processor.element = _FakeElement(source: _FakeSource(fullName: libFooPath));
var doc = await processor.processComment('''
/// Text.
@@ -417,9 +441,7 @@ Markdown text.'''));
test('adds language to processed @example with an extension and no lang',
() async {
- var examplePath = resourceProvider.pathContext.canonicalize(
- resourceProvider.pathContext.join(projectRoot.path, 'abc.html.md'));
- resourceProvider.getFile(examplePath).writeAsStringSync('''
+ projectRoot.getChildAssumingFile('abc.html.md').writeAsStringSync('''
```
Code snippet
```
@@ -447,9 +469,7 @@ End text.'''));
test('adds language to processed @example with a lang and an extension',
() async {
- var examplePath = resourceProvider.pathContext.canonicalize(
- resourceProvider.pathContext.join(projectRoot.path, 'abc.html.md'));
- resourceProvider.getFile(examplePath).writeAsStringSync('''
+ projectRoot.getChildAssumingFile('abc.html.md').writeAsStringSync('''
```
Code snippet
```
@@ -473,9 +493,7 @@ Code snippet
test('adds language to processed @example with a lang and no extension',
() async {
- var examplePath = resourceProvider.pathContext.canonicalize(
- resourceProvider.pathContext.join(projectRoot.path, 'abc.md'));
- resourceProvider.getFile(examplePath).writeAsStringSync('''
+ projectRoot.getChildAssumingFile('abc.md').writeAsStringSync('''
```
Code snippet
```
From af6e111363184621a6c53f979e9cdac44dd38d24 Mon Sep 17 00:00:00 2001
From: Sam Rawlins <srawlins@google.com>
Date: Wed, 9 Sep 2020 21:52:55 -0700
Subject: [PATCH 2/3] little change
---
lib/src/model/package_graph.dart | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart
index f000aa4932..ee0115d48c 100644
--- a/lib/src/model/package_graph.dart
+++ b/lib/src/model/package_graph.dart
@@ -382,7 +382,7 @@ class PackageGraph {
referredFromPrefix = 'in documentation inherited from';
break;
case PackageWarning.unknownDirective:
- warningMessage = 'undefined directive [$message]';
+ warningMessage = 'undefined directive: $message';
break;
case PackageWarning.unknownMacro:
warningMessage = 'undefined macro [$message]';
From 35434c4ba8c782dabdc9af49f0588519939de1ae Mon Sep 17 00:00:00 2001
From: Sam Rawlins <srawlins@google.com>
Date: Thu, 10 Sep 2020 11:16:54 -0700
Subject: [PATCH 3/3] canonicalize
---
test/comment_processable_test.dart | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/test/comment_processable_test.dart b/test/comment_processable_test.dart
index 2646ee5fc6..f0082c8ed1 100644
--- a/test/comment_processable_test.dart
+++ b/test/comment_processable_test.dart
@@ -28,8 +28,8 @@ void main() {
setUp(() async {
resourceProvider = MemoryResourceProvider();
- projectRoot =
- resourceProvider.getFolder(resourceProvider.convertPath('/project'));
+ projectRoot = resourceProvider.getFolder(resourceProvider.pathContext
+ .canonicalize(resourceProvider.convertPath('/project')));
projectRoot.create();
resourceProvider
.getFile(