From 869aa0393d2d4a80357dc1b0d3d3fb03ee3f281d Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 15 Mar 2021 16:44:22 -0700 Subject: [PATCH 1/4] Add option for setting --no-sound-null-safety on tool compilation. --- lib/src/dartdoc_options.dart | 14 ++++++++++---- lib/src/model/documentation_comment.dart | 14 +++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index e775bcc18a..07f6977713 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -147,13 +147,13 @@ class ToolDefinition { List command, List setupCommand, String description, - ResourceProvider resourceProvider) { + ResourceProvider resourceProvider, {bool soundNullSafety = true}) { assert(command != null); assert(command.isNotEmpty); assert(description != null); if (isDartExecutable(command[0])) { return DartToolDefinition( - command, setupCommand, description, resourceProvider); + command, setupCommand, description, resourceProvider, soundNullSafety: soundNullSafety); } else { return ToolDefinition(command, setupCommand, description); } @@ -274,6 +274,9 @@ class SnapshotCache { class DartToolDefinition extends ToolDefinition { final ResourceProvider _resourceProvider; + /// True if the command can successfully be compiled with sound null safety. + final bool soundNullSafety; + /// Takes a list of args to modify, and returns the name of the executable /// to run. If no snapshot file existed, then create one and modify the args /// so that if they are executed with dart, will result in the snapshot being @@ -293,6 +296,7 @@ class DartToolDefinition extends ToolDefinition { // https://dart-review.googlesource.com/c/sdk/+/181421 is safely // in the rearview mirror in dev/Flutter. '--ignore-unrecognized-flags', + if (!soundNullSafety) '--no-sound-null-safety', '--verbosity=error', '--snapshot=${_resourceProvider.pathContext.absolute(snapshotFile.path)}', '--snapshot_kind=app-jit' @@ -307,7 +311,7 @@ class DartToolDefinition extends ToolDefinition { } DartToolDefinition(List command, List setupCommand, - String description, this._resourceProvider) + String description, this._resourceProvider, {this.soundNullSafety = true}) : super(command, setupCommand, description); } @@ -335,11 +339,13 @@ class ToolConfiguration { for (var entry in yamlMap.entries) { var name = entry.key.toString(); var toolMap = entry.value; + var soundNullSafety = true; String description; List command; List setupCommand; if (toolMap is Map) { description = toolMap['description']?.toString(); + soundNullSafety = toolMap['sound-null-safety'] ?? true; List findCommand([String prefix = '']) { List command; // If the command key is given, then it applies to all platforms. @@ -421,7 +427,7 @@ class ToolConfiguration { setupCommand; } newToolDefinitions[name] = ToolDefinition.fromCommand( - [executable] + command, setupCommand, description, resourceProvider); + [executable] + command, setupCommand, description, resourceProvider, soundNullSafety: soundNullSafety); } return ToolConfiguration._(newToolDefinitions, resourceProvider); } diff --git a/lib/src/model/documentation_comment.dart b/lib/src/model/documentation_comment.dart index 49d459bbc0..cad6b3b81f 100644 --- a/lib/src/model/documentation_comment.dart +++ b/lib/src/model/documentation_comment.dart @@ -191,11 +191,15 @@ mixin DocumentationComment /// ```yaml /// dartdoc: /// tools: - /// # Prefixes the given input with "## " - /// # Path is relative to project root. - /// prefix: "bin/prefix.dart" - /// # Prints the date - /// date: "/bin/date" + /// prefix: + /// # Path is relative to project root. + /// command: ["bin/prefix.dart"] + /// description: "Prefixes the given input with '##'" + /// # If false, adds --no-sound-null-safety when compiling. + /// sound-null-safety: false + /// date: + /// command: ["/bin/date"] + /// description: "Prints the date" /// ``` /// /// In code: From dca940338eaa6de9201002bc6315efa8dfde766c Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 15 Mar 2021 17:19:22 -0700 Subject: [PATCH 2/4] Switch to general compile args. --- lib/src/dartdoc_options.dart | 54 +++++++++++++++++++----- lib/src/model/documentation_comment.dart | 5 +-- test/tool_runner_test.dart | 7 +++ 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index 07f6977713..4f2e54eb04 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -147,14 +147,22 @@ class ToolDefinition { List command, List setupCommand, String description, - ResourceProvider resourceProvider, {bool soundNullSafety = true}) { + ResourceProvider resourceProvider, + {List compileArgs}) { assert(command != null); assert(command.isNotEmpty); assert(description != null); if (isDartExecutable(command[0])) { return DartToolDefinition( - command, setupCommand, description, resourceProvider, soundNullSafety: soundNullSafety); + command, setupCommand, description, resourceProvider, + compileArgs: compileArgs ?? const []); } else { + if (compileArgs != null && compileArgs.isNotEmpty) { + throw DartdocOptionError( + 'Compile arguments may only be specified for Dart tools, but ' + 'compile-args of $compileArgs were specified for ' + '$command.'); + } return ToolDefinition(command, setupCommand, description); } } @@ -274,8 +282,8 @@ class SnapshotCache { class DartToolDefinition extends ToolDefinition { final ResourceProvider _resourceProvider; - /// True if the command can successfully be compiled with sound null safety. - final bool soundNullSafety; + /// A list of arguments to add to the snapshot compilation arguments. + final List compileArgs; /// Takes a list of args to modify, and returns the name of the executable /// to run. If no snapshot file existed, then create one and modify the args @@ -296,10 +304,10 @@ class DartToolDefinition extends ToolDefinition { // https://dart-review.googlesource.com/c/sdk/+/181421 is safely // in the rearview mirror in dev/Flutter. '--ignore-unrecognized-flags', - if (!soundNullSafety) '--no-sound-null-safety', '--verbosity=error', '--snapshot=${_resourceProvider.pathContext.absolute(snapshotFile.path)}', - '--snapshot_kind=app-jit' + '--snapshot_kind=app-jit', + ...compileArgs, ]); } else { await snapshot.snapshotValid(); @@ -311,8 +319,10 @@ class DartToolDefinition extends ToolDefinition { } DartToolDefinition(List command, List setupCommand, - String description, this._resourceProvider, {this.soundNullSafety = true}) - : super(command, setupCommand, description); + String description, this._resourceProvider, + {this.compileArgs = const []}) + : assert(compileArgs != null), + super(command, setupCommand, description); } /// A configuration class that can interpret [ToolDefinition]s from a YAML map. @@ -339,13 +349,12 @@ class ToolConfiguration { for (var entry in yamlMap.entries) { var name = entry.key.toString(); var toolMap = entry.value; - var soundNullSafety = true; + List compileArgs; String description; List command; List setupCommand; if (toolMap is Map) { description = toolMap['description']?.toString(); - soundNullSafety = toolMap['sound-null-safety'] ?? true; List findCommand([String prefix = '']) { List command; // If the command key is given, then it applies to all platforms. @@ -382,6 +391,28 @@ class ToolConfiguration { command = findCommand(); setupCommand = findCommand('setup_'); + + List findArgs() { + const tagName = 'compile-args'; + List args; + if (toolMap.containsKey(tagName)) { + var compileArgs = toolMap[tagName]; + if (compileArgs is String) { + args = [toolMap[tagName].toString()]; + } else if (compileArgs is YamlList) { + args = + compileArgs.map((node) => node.toString()).toList(); + } else { + throw DartdocOptionError( + 'Tool compile arguments must be a list of strings. The tool ' + '$name has a $tagName entry that is a ' + '${compileArgs.runtimeType}'); + } + } + return args; + } + + compileArgs = findArgs(); } else { throw DartdocOptionError( 'Tools must be defined as a map of tool names to definitions. Tool ' @@ -427,7 +458,8 @@ class ToolConfiguration { setupCommand; } newToolDefinitions[name] = ToolDefinition.fromCommand( - [executable] + command, setupCommand, description, resourceProvider, soundNullSafety: soundNullSafety); + [executable] + command, setupCommand, description, resourceProvider, + compileArgs: compileArgs ?? const []); } return ToolConfiguration._(newToolDefinitions, resourceProvider); } diff --git a/lib/src/model/documentation_comment.dart b/lib/src/model/documentation_comment.dart index cad6b3b81f..aa72251d11 100644 --- a/lib/src/model/documentation_comment.dart +++ b/lib/src/model/documentation_comment.dart @@ -194,9 +194,8 @@ mixin DocumentationComment /// prefix: /// # Path is relative to project root. /// command: ["bin/prefix.dart"] - /// description: "Prefixes the given input with '##'" - /// # If false, adds --no-sound-null-safety when compiling. - /// sound-null-safety: false + /// description: "Prefixes the given input with '##'." + /// compile-args: ["--no-sound-null-safety"] /// date: /// command: ["/bin/date"] /// description: "Prints the date" diff --git a/test/tool_runner_test.dart b/test/tool_runner_test.dart index f416287af5..38c114b5d1 100644 --- a/test/tool_runner_test.dart +++ b/test/tool_runner_test.dart @@ -59,6 +59,7 @@ void main() { drill: command: ["bin/drill.dart"] description: "Puts holes in things." + compile-args: ["--no-sound-null-safety"] snapshot_drill: command: ["${snapshotFile.replaceAll(r'\', r'\\')}"] description: "Puts holes in things, but faster." @@ -104,6 +105,10 @@ echo: }); // This test must come first, to verify that the first run creates // a snapshot. + test('Tool definition includes compile arguments.', () async { + DartToolDefinition definition = runner.toolConfiguration.tools['drill']; + expect(definition.compileArgs, equals(['--no-sound-null-safety'])); + }); test('can invoke a Dart tool, and second run is a snapshot.', () async { var result = await runner.run( ['drill', r'--file=$INPUT'], @@ -114,6 +119,8 @@ echo: expect(result, contains('--file=')); expect(result, contains('## `TEST INPUT`')); expect(result, contains('Script location is in dartdoc tree.')); + // This shouldn't be in the args passed to the tool. + expect(result, isNot(contains('--no-sound-null-safety'))); expect(setupFile.existsSync(), isFalse); result = await runner.run( ['drill', r'--file=$INPUT'], From be4599af872751d016b5e9861db56269b5ab684d Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 17 Mar 2021 11:39:12 -0700 Subject: [PATCH 3/4] Added to README.md, changed name to compile_args from compile-args --- README.md | 4 ++++ lib/src/dartdoc_options.dart | 13 +++++++------ lib/src/model/documentation_comment.dart | 2 +- test/tool_runner_test.dart | 4 ++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 5009dd2bee..563d8155d0 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,7 @@ dartdoc: command: ["bin/drill.dart"] setup_command: ["bin/setup.dart"] description: "Puts holes in things." + compile_args: ["-no-sound-null-safety"] echo: macos: ['/bin/sh', '-c', 'echo'] setup_macos: ['/bin/sh', '-c', 'setup.sh'] @@ -328,6 +329,9 @@ The `description` is just a short description of the tool for use as help text. Only tools which are configured in the `dartdoc_options.yaml` file are able to be invoked. +The `compile_args` tag is used to pass options to the dart compiler when the +first run of the tool is being snapshotted. + To use the tools in comment documentation, use the `{@tool [ ...] [$INPUT]}` directive to invoke the tool: diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index 4f2e54eb04..d7aff47b38 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -39,6 +39,8 @@ const int _kIntVal = 0; const double _kDoubleVal = 0.0; const bool _kBoolVal = true; +const String _kCompileArgsTagName = 'compile_args'; + int get _usageLineLength => stdout.hasTerminal ? stdout.terminalColumns : null; typedef ConvertYamlToType = T Function(YamlMap, String, ResourceProvider); @@ -160,7 +162,7 @@ class ToolDefinition { if (compileArgs != null && compileArgs.isNotEmpty) { throw DartdocOptionError( 'Compile arguments may only be specified for Dart tools, but ' - 'compile-args of $compileArgs were specified for ' + '$_kCompileArgsTagName of $compileArgs were specified for ' '$command.'); } return ToolDefinition(command, setupCommand, description); @@ -393,19 +395,18 @@ class ToolConfiguration { setupCommand = findCommand('setup_'); List findArgs() { - const tagName = 'compile-args'; List args; - if (toolMap.containsKey(tagName)) { - var compileArgs = toolMap[tagName]; + if (toolMap.containsKey(_kCompileArgsTagName)) { + var compileArgs = toolMap[_kCompileArgsTagName]; if (compileArgs is String) { - args = [toolMap[tagName].toString()]; + args = [toolMap[_kCompileArgsTagName].toString()]; } else if (compileArgs is YamlList) { args = compileArgs.map((node) => node.toString()).toList(); } else { throw DartdocOptionError( 'Tool compile arguments must be a list of strings. The tool ' - '$name has a $tagName entry that is a ' + '$name has a $_kCompileArgsTagName entry that is a ' '${compileArgs.runtimeType}'); } } diff --git a/lib/src/model/documentation_comment.dart b/lib/src/model/documentation_comment.dart index aa72251d11..a8d63748f6 100644 --- a/lib/src/model/documentation_comment.dart +++ b/lib/src/model/documentation_comment.dart @@ -195,7 +195,7 @@ mixin DocumentationComment /// # Path is relative to project root. /// command: ["bin/prefix.dart"] /// description: "Prefixes the given input with '##'." - /// compile-args: ["--no-sound-null-safety"] + /// compile_args: ["--no-sound-null-safety"] /// date: /// command: ["/bin/date"] /// description: "Prints the date" diff --git a/test/tool_runner_test.dart b/test/tool_runner_test.dart index 38c114b5d1..e7b0d9681e 100644 --- a/test/tool_runner_test.dart +++ b/test/tool_runner_test.dart @@ -59,7 +59,7 @@ void main() { drill: command: ["bin/drill.dart"] description: "Puts holes in things." - compile-args: ["--no-sound-null-safety"] + compile_args: ["--no-sound-null-safety"] snapshot_drill: command: ["${snapshotFile.replaceAll(r'\', r'\\')}"] description: "Puts holes in things, but faster." @@ -119,7 +119,7 @@ echo: expect(result, contains('--file=')); expect(result, contains('## `TEST INPUT`')); expect(result, contains('Script location is in dartdoc tree.')); - // This shouldn't be in the args passed to the tool. + // Compile args shouldn't be in the args passed to the tool. expect(result, isNot(contains('--no-sound-null-safety'))); expect(setupFile.existsSync(), isFalse); result = await runner.run( From b4bb6cb19c959853d170100c70f30879dfc833b1 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 17 Mar 2021 11:48:16 -0700 Subject: [PATCH 4/4] Fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 563d8155d0..01fb7d69d5 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,7 @@ dartdoc: command: ["bin/drill.dart"] setup_command: ["bin/setup.dart"] description: "Puts holes in things." - compile_args: ["-no-sound-null-safety"] + compile_args: ["--no-sound-null-safety"] echo: macos: ['/bin/sh', '-c', 'echo'] setup_macos: ['/bin/sh', '-c', 'setup.sh']