From bafa71a8aee74842574ebed6b3bef534ca95e6e5 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 30 Sep 2021 16:35:59 +0000 Subject: [PATCH] Add the ability to specify additional imports for doc samples. Change-Id: I1c9592d4ce66d08aafacd5ae9722defa114c9953 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214867 Reviewed-by: Konstantin Shcheglov Commit-Queue: Devon Carew --- sdk/lib/_http/http.dart | 2 +- sdk/lib/async/async.dart | 6 +-- sdk/lib/async/future.dart | 2 +- sdk/lib/async/stream.dart | 2 +- sdk/lib/convert/convert.dart | 2 +- sdk/lib/io/io.dart | 4 +- tools/verify_docs/README.md | 35 ++++++++++++++- tools/verify_docs/bin/verify_docs.dart | 61 ++++++++++++++++++-------- 8 files changed, 84 insertions(+), 30 deletions(-) diff --git a/sdk/lib/_http/http.dart b/sdk/lib/_http/http.dart index d3034128787..b7b1f5083c5 100644 --- a/sdk/lib/_http/http.dart +++ b/sdk/lib/_http/http.dart @@ -2063,7 +2063,7 @@ abstract class HttpClientRequest implements IOSink { /// Using the [IOSink] methods (e.g., [write] and [add]) has no effect after /// the request has been aborted /// - /// ```dart + /// ```dart import:async /// HttpClientRequst request = ... /// request.write(); /// Timer(Duration(seconds: 1), () { diff --git a/sdk/lib/async/async.dart b/sdk/lib/async/async.dart index 33df08e1563..00b7fc6c6aa 100644 --- a/sdk/lib/async/async.dart +++ b/sdk/lib/async/async.dart @@ -32,7 +32,7 @@ /// Many methods in the Dart libraries return `Future`s when /// performing tasks. For example, when binding an `HttpServer` /// to a host and port, the `bind()` method returns a Future. -/// ```dart +/// ```dart import:io /// HttpServer.bind('127.0.0.1', 4444) /// .then((server) => print('${server.isBroadcast}')) /// .catchError(print); @@ -60,7 +60,7 @@ /// the stream has finished. /// Further functionality is provided on [Stream], implemented by calling /// [Stream.listen] to get the actual data. -/// ```dart +/// ```dart import:io import:convert /// Stream> stream = File('quotes.txt').openRead(); /// stream.transform(utf8.decoder).forEach(print); /// ``` @@ -72,7 +72,7 @@ /// /// Another common use of streams is for user-generated events /// in a web app: The following code listens for mouse clicks on a button. -/// ```dart +/// ```dart import:html /// querySelector('#myButton').onClick.forEach((_) => print('Click.')); /// ``` /// ## Other resources diff --git a/sdk/lib/async/future.dart b/sdk/lib/async/future.dart index f663b085e27..c5875368ae7 100644 --- a/sdk/lib/async/future.dart +++ b/sdk/lib/async/future.dart @@ -738,7 +738,7 @@ abstract class Future { /// /// This method is equivalent to: /// ```dart - /// Future whenComplete(action()) { + /// Future whenComplete(action() { /// return this.then((v) { /// var f2 = action(); /// if (f2 is Future) return f2.then((_) => v); diff --git a/sdk/lib/async/stream.dart b/sdk/lib/async/stream.dart index 208351026d8..3f12cb3e3f5 100644 --- a/sdk/lib/async/stream.dart +++ b/sdk/lib/async/stream.dart @@ -2059,7 +2059,7 @@ abstract class StreamTransformer { /// [StreamTransformer.bind] API and can be used when the transformation is /// available as a stream-to-stream function. /// - /// ```dart + /// ```dart import:convert /// final splitDecoded = StreamTransformer, String>.fromBind( /// (stream) => stream.transform(utf8.decoder).transform(LineSplitter())); /// ``` diff --git a/sdk/lib/convert/convert.dart b/sdk/lib/convert/convert.dart index 081f00613d2..b1a1cea1d29 100644 --- a/sdk/lib/convert/convert.dart +++ b/sdk/lib/convert/convert.dart @@ -34,7 +34,7 @@ /// as it's read from a file, /// The second is an instance of [LineSplitter], /// which splits the data on newline boundaries. -/// ```dart +/// ```dart import:io /// var lineNumber = 1; /// var stream = File('quotes.txt').openRead(); /// diff --git a/sdk/lib/io/io.dart b/sdk/lib/io/io.dart index 84ad5f922e6..4cad26d850d 100644 --- a/sdk/lib/io/io.dart +++ b/sdk/lib/io/io.dart @@ -111,7 +111,7 @@ /// and listens for the data on the returned web socket. /// For example, here's a mini server that listens for 'ws' data /// on a WebSocket: -/// ```dart +/// ```dart import:async /// runZoned(() async { /// var server = await HttpServer.bind('127.0.0.1', 4040); /// server.listen((HttpRequest req) async { @@ -140,7 +140,7 @@ /// Use [ServerSocket] on the server side and [Socket] on the client. /// The server creates a listening socket using the `bind()` method and /// then listens for incoming connections on the socket. For example: -/// ```dart +/// ```dart import:convert /// ServerSocket.bind('127.0.0.1', 4041) /// .then((serverSocket) { /// serverSocket.listen((socket) { diff --git a/tools/verify_docs/README.md b/tools/verify_docs/README.md index 13f72bd2e79..cc6401793ae 100644 --- a/tools/verify_docs/README.md +++ b/tools/verify_docs/README.md @@ -20,5 +20,36 @@ The tool should be run from the root of the sdk repository. ## Authoring code samples -TODO(devoncarew): Document the conventions for code samples in the dart: libraries -and the tools available to configure them. +### What gets analyzed + +This tool will walk all dartdoc api docs looking for code samples in doc comments. +It will analyze any code sample in a `dart` code fence. For example: + +> ```dart +> print('hello world!'); +> ``` + +By default, an import for that library is added to the sample being analyzed (i.e., +`import 'dart:async";`). Additionally, the code sample is automatically embedded in +the body of a simple main() method. + +### Excluding code samples from analysis + +In order to exclude a code sample from analysis, change it to a plain code fence style: + +> ``` +> print("I'm not analyzed :("); +> ``` + +### Specifying additional imports + +In order to reference code from other Dart core libraries, you can either explicitly add +the import to the code sample - in-line in the sample - or use a directive on the same +line as the code fence. The directive style looks like: + +> ```dart import:async +> print('hello world ${Timer()}'); +> ``` + +Multiple imports can be specified like this if desired (i.e., "```dart import:async import:convert"). + diff --git a/tools/verify_docs/bin/verify_docs.dart b/tools/verify_docs/bin/verify_docs.dart index ebd4822d4c5..8c4fc67f0ac 100644 --- a/tools/verify_docs/bin/verify_docs.dart +++ b/tools/verify_docs/bin/verify_docs.dart @@ -28,8 +28,10 @@ void main(List args) async { print('Validating the dartdoc code samples from the dart: libraries.'); print(''); print('To run this tool, run `dart tools/verify_docs/bin/verify_docs.dart`.'); + print(''); print('For documentation about how to author dart: code samples,' ' see tools/verify_docs/README.md'); + print(''); final coreLibraries = args.isEmpty ? libDir.listSync().whereType().toList() @@ -90,7 +92,6 @@ Future verifyFile(String coreLibName, File file, Directory parent) async { return error.errorCode.type == ErrorType.SYNTACTIC_ERROR; }).toList(); if (syntacticErrors.isNotEmpty) { - // todo: have a better failure mode throw Exception(syntacticErrors); } @@ -108,21 +109,22 @@ Future verifyFile(String coreLibName, File file, Directory parent) async { return visitor.errors.isEmpty; } -/// todo: doc +/// Visit a compilation unit and collect the list of code samples found in +/// dartdoc comments. class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { final String coreLibName; final String filePath; final LineInfo lineInfo; + final List samples = []; + final StringBuffer errors = StringBuffer(); + ValidateCommentCodeSamplesVisitor( this.coreLibName, this.filePath, this.lineInfo, ); - final List samples = []; - final StringBuffer errors = StringBuffer(); - Future process(ParseStringResult parseResult) async { // collect code samples visitCompilationUnit(parseResult.unit); @@ -135,7 +137,6 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { @override void visitAnnotatedNode(AnnotatedNode node) { - // todo: ignore (or fail?) doc comments on non-public symbols _handleDocumentableNode(node); super.visitAnnotatedNode(node); } @@ -158,6 +159,11 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { var offset = text.indexOf(sampleStart); while (offset != -1) { + // Collect template directives, like "```dart import:async". + final codeFenceSuffix = text.substring( + offset + sampleStart.length, text.indexOf('\n', offset)); + final directives = Set.unmodifiable(codeFenceSuffix.trim().split(' ')); + offset = text.indexOf('\n', offset) + 1; final end = text.indexOf(sampleEnd, offset); @@ -166,13 +172,12 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { List lines = snippet.split('\n'); - // TODO(devoncarew): Also look for template directives. - samples.add( CodeSample( - coreLibName, lines.map((e) => ' ${cleanDocLine(e)}').join('\n'), - commentLineStart + + coreLibName: coreLibName, + directives: directives, + lineStartOffset: commentLineStart + text.substring(0, offset - 1).split('\n').length - 1, ), @@ -183,9 +188,6 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { } Future validateCodeSample(CodeSample sample) async { - // TODO(devoncarew): Support ? - // TODO(devoncarew): Support ? - final resourceProvider = OverlayResourceProvider(PhysicalResourceProvider.INSTANCE); @@ -212,6 +214,12 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { text = "main() async {\n${text.trimRight()}\n}\n"; } + for (final directive + in sample.directives.where((str) => str.startsWith('import:'))) { + final libName = directive.substring('import:'.length); + text = "import 'dart:$libName';\n$text"; + } + if (sample.coreLibName != 'internal') { text = "import 'dart:${sample.coreLibName}';\n$text"; } @@ -227,7 +235,7 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { modificationStamp: 0, ); - // TODO(devoncarew): refactor to use AnalysisContextCollection to avoid + // TODO(devoncarew): Refactor to use AnalysisContextCollection to avoid // re-creating analysis contexts. final result = await resolveFile2( path: sampleFilePath, @@ -239,8 +247,9 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { if (result is ResolvedUnitResult) { // Filter out unused imports, since we speculatively add imports to some // samples. - var errors = - result.errors.where((e) => e.errorCode != HintCode.UNUSED_IMPORT); + var errors = result.errors.where( + (e) => e.errorCode != HintCode.UNUSED_IMPORT, + ); // Also, don't worry about 'unused_local_variable' and related; this may // be intentional in samples. @@ -250,8 +259,16 @@ class ValidateCommentCodeSamplesVisitor extends GeneralizingAstVisitor { e.errorCode != HintCode.UNUSED_ELEMENT, ); + // Remove warnings about deprecated member use from the same library. + errors = errors.where( + (e) => + e.errorCode != HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE && + e.errorCode != + HintCode.DEPRECATED_MEMBER_USE_FROM_SAME_PACKAGE_WITH_MESSAGE, + ); + if (errors.isNotEmpty) { - print('$filePath:${sample.lineStart}: ${errors.length} errors'); + print('$filePath:${sample.lineStartOffset}: ${errors.length} errors'); for (final error in errors) { final location = result.lineInfo.getLocation(error.offset); @@ -290,10 +307,16 @@ String cleanDocLine(String line) { class CodeSample { final String coreLibName; + final Set directives; final String text; - final int lineStart; + final int lineStartOffset; - CodeSample(this.coreLibName, this.text, this.lineStart); + CodeSample( + this.text, { + required this.coreLibName, + this.directives = const {}, + required this.lineStartOffset, + }); } String _severity(Severity severity) {