Skip to content

Commit

Permalink
Merge 0d925cd into 99f45ab
Browse files Browse the repository at this point in the history
  • Loading branch information
srawlins committed Jun 17, 2021
2 parents 99f45ab + 0d925cd commit 0e3b727
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 96 deletions.
44 changes: 23 additions & 21 deletions lib/src/model/documentation_comment.dart
Expand Up @@ -4,6 +4,7 @@ import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/render/model_element_renderer.dart';
import 'package:dartdoc/src/utils.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as path show Context;

final _templatePattern = RegExp(
Expand Down Expand Up @@ -232,30 +233,31 @@ mixin DocumentationComment
// Count the number of invocations of tools in this dartdoc block,
// so that tools can differentiate different blocks from each other.
invocationIndex++;
return await config.tools.runner.run(
args,
(String message) async =>
warn(PackageWarning.toolError, message: message),
content: basicMatch[2],
environment: {
'SOURCE_LINE': characterLocation?.lineNumber.toString(),
'SOURCE_COLUMN': characterLocation?.columnNumber.toString(),
'SOURCE_PATH':
(sourceFileName == null || package?.packagePath == null)
? null
: pathContext.relative(sourceFileName,
from: package.packagePath),
'PACKAGE_PATH': package?.packagePath,
'PACKAGE_NAME': package?.name,
'LIBRARY_NAME': library?.fullyQualifiedName,
'ELEMENT_NAME': fullyQualifiedNameWithoutLibrary,
'INVOCATION_INDEX': invocationIndex.toString(),
'PACKAGE_INVOCATION_INDEX':
(package.toolInvocationIndex++).toString(),
}..removeWhere((key, value) => value == null));
return await config.tools.runner.run(args, content: basicMatch[2],
toolErrorCallback: (String message) async {
print('toolerrocallback for $args');
warn(PackageWarning.toolError, message: message);
}, environment: _toolsEnvironment(invocationIndex: invocationIndex));
});
}

/// The environment variables to use when running a tool.
Map<String, String> _toolsEnvironment({@required int invocationIndex}) {
return {
'SOURCE_LINE': characterLocation?.lineNumber.toString(),
'SOURCE_COLUMN': characterLocation?.columnNumber.toString(),
if (sourceFileName != null && package?.packagePath != null)
'SOURCE_PATH':
pathContext.relative(sourceFileName, from: package.packagePath),
'PACKAGE_PATH': package?.packagePath,
'PACKAGE_NAME': package?.name,
'LIBRARY_NAME': library?.fullyQualifiedName,
'ELEMENT_NAME': fullyQualifiedNameWithoutLibrary,
'INVOCATION_INDEX': invocationIndex.toString(),
'PACKAGE_INVOCATION_INDEX': (package.toolInvocationIndex++).toString(),
}..removeWhere((key, value) => value == null);
}

/// Replace &#123;@example ...&#125; in API comments with the content of named file.
///
/// Syntax:
Expand Down
156 changes: 90 additions & 66 deletions lib/src/tool_runner.dart
Expand Up @@ -8,6 +8,7 @@ import 'dart:io' show Process, ProcessException;

import 'package:analyzer/file_system/file_system.dart';
import 'package:dartdoc/src/io_utils.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;
import 'dartdoc_options.dart';

Expand All @@ -19,9 +20,6 @@ typedef FakeResultCallback = String Function(String tool,
/// limiting both parallelization and the number of open temporary files.
final MultiFutureTracker<void> _toolTracker = MultiFutureTracker(4);

/// Can be called when the ToolRunner is no longer needed.
///
/// This will remove any temporary files created by the tool runner.
class ToolTempFileTracker {
final ResourceProvider resourceProvider;
final Folder temporaryDirectory;
Expand All @@ -40,7 +38,7 @@ class ToolTempFileTracker {

int _temporaryFileCount = 0;

Future<File> createTemporaryFile() async {
File createTemporaryFile() {
_temporaryFileCount++;
// TODO(srawlins): Assume [temporaryDirectory]'s path is always absolute.
var tempFile = resourceProvider.getFile(resourceProvider.pathContext.join(
Expand Down Expand Up @@ -79,22 +77,24 @@ class ToolRunner {
String commandPath;

if (isDartSetup) {
commandPath = toolConfiguration.resourceProvider.resolvedExecutable;
commandPath = resourceProvider.resolvedExecutable;
} else {
commandPath = args.removeAt(0);
}
await _runProcess(
name, '', commandPath, args, environment, toolErrorCallback);
// We do not use the stdout of the setup process.
await _runProcess(name, '', commandPath, args, environment,
toolErrorCallback: toolErrorCallback);
tool.setupComplete = true;
}

Future<String> _runProcess(
String name,
String content,
String commandPath,
List<String> args,
Map<String, String> environment,
ToolErrorCallback toolErrorCallback) async {
/// Runs the tool with [Process.run], awaiting the exit code, and returning
/// the stdout.
///
/// If the process's exit code is not 0, or if a [ProcessException] is thrown,
/// calls [toolErrorCallback] with a detailed error message, and returns `''`.
Future<String> _runProcess(String name, String content, String commandPath,
List<String> args, Map<String, String> environment,
{@required ToolErrorCallback toolErrorCallback}) async {
String commandString() => ([commandPath] + args).join(' ');
try {
var result =
Expand All @@ -119,58 +119,52 @@ class ToolRunner {
}
}

/// Run a tool. The name of the tool is the first argument in the [args].
/// The content to be sent to to the tool is given in the optional [content],
/// and the stdout of the tool is returned.
/// Run a tool.
///
/// The [args] must not be null, and it must have at least one member (the name
/// of the tool).
Future<String> run(List<String> args, ToolErrorCallback toolErrorCallback,
{String content, Map<String, String> environment}) async {
/// The name of the tool is the first argument in the [args]. The content to
/// be sent to to the tool is given in the optional [content]. The stdout of
/// the tool is returned.
Future<String> run(List<String> args,
{@required String content,
@required ToolErrorCallback toolErrorCallback,
Map<String, String> environment}) async {
assert(args != null);
assert(args.isNotEmpty);
Future<String> runner;
// Prevent too many tools from running simultaneously.
await _toolTracker.addFutureFromClosure(() {
runner = _run(args, toolErrorCallback,
content: content, environment: environment);
runner = _run(args,
toolErrorCallback: toolErrorCallback,
content: content,
environment: environment);
return runner;
});
return runner;
}

Future<String> _run(List<String> args, ToolErrorCallback toolErrorCallback,
{String content, Map<String, String> environment}) async {
Future<String> _run(List<String> args,
{@required ToolErrorCallback toolErrorCallback,
String content,
Map<String, String> environment}) async {
assert(args != null);
assert(args.isNotEmpty);
content ??= '';
environment ??= <String, String>{};
var tool = args.removeAt(0);
if (!toolConfiguration.tools.containsKey(tool)) {
var toolName = args.removeAt(0);
if (!toolConfiguration.tools.containsKey(toolName)) {
toolErrorCallback(
'Unable to find definition for tool "$tool" in tool map. '
'Unable to find definition for tool "$toolName" in tool map. '
'Did you add it to dartdoc_options.yaml?');
return '';
}
var toolDefinition = toolConfiguration.tools[tool];
var toolDefinition = toolConfiguration.tools[toolName];
var toolArgs = toolDefinition.command;
// Ideally, we would just be able to send the input text into stdin, but
// there's no way to do that synchronously, and converting dartdoc to an
// async model of execution is a huge amount of work. Using dart:cli's
// waitFor feels like a hack (and requires a similar amount of work anyhow
// to fix order of execution issues). So, instead, we have the tool take a
// filename as part of its arguments, and write the input to a temporary
// file before running the tool synchronously.

// Write the content to a temp file.
var tmpFile = await ToolTempFileTracker.createInstance(
toolConfiguration.resourceProvider)
.createTemporaryFile();
tmpFile.writeAsStringSync(content);

// Substitute the temp filename for the "$INPUT" token, and all of the other
// environment variables. Variables are allowed to either be in $(VAR) form,
// or $VAR form.
var envWithInput = {
'INPUT': pathContext.absolute(tmpFile.path),
'INPUT': _tmpFileWithContent(content),
'TOOL_COMMAND': toolDefinition.command[0],
...environment,
};
Expand All @@ -182,30 +176,22 @@ class ToolRunner {
// find out where their script was coming from as an absolute path on the
// filesystem.
envWithInput['DART_SNAPSHOT_CACHE'] = pathContext.absolute(
SnapshotCache.createInstance(toolConfiguration.resourceProvider)
.snapshotCache
.path);
SnapshotCache.createInstance(resourceProvider).snapshotCache.path);
if (toolDefinition.setupCommand != null) {
envWithInput['DART_SETUP_COMMAND'] = toolDefinition.setupCommand[0];
}
}
var substitutions = envWithInput.map<RegExp, String>((key, value) {
var escapedKey = RegExp.escape(key);
return MapEntry(RegExp('\\\$(\\($escapedKey\\)|$escapedKey\\b)'), value);
});
var argsWithInput = <String>[];
for (var arg in args) {
var newArg = arg;
substitutions
.forEach((regex, value) => newArg = newArg.replaceAll(regex, value));
argsWithInput.add(newArg);
}

var argsWithInput = [
...toolArgs,
..._substituteInArgs(args, envWithInput),
];

if (toolDefinition.setupCommand != null && !toolDefinition.setupComplete) {
await _runSetup(tool, toolDefinition, envWithInput, toolErrorCallback);
await _runSetup(
toolName, toolDefinition, envWithInput, toolErrorCallback);
}

argsWithInput = toolArgs + argsWithInput;
String commandPath;
void Function() callCompleter;
if (toolDefinition is DartToolDefinition) {
Expand All @@ -216,16 +202,54 @@ class ToolRunner {
} else {
commandPath = argsWithInput.removeAt(0);
}
var stdout = _runProcess(
toolName, content, commandPath, argsWithInput, envWithInput,
toolErrorCallback: toolErrorCallback);

if (callCompleter != null) {
return _runProcess(tool, content, commandPath, argsWithInput,
envWithInput, toolErrorCallback)
.whenComplete(callCompleter);
if (callCompleter == null) {
return stdout;
} else {
return _runProcess(tool, content, commandPath, argsWithInput,
envWithInput, toolErrorCallback);
return stdout.whenComplete(callCompleter);
}
}

/// Returns the path to the temp file after [content] is written to it.
String _tmpFileWithContent(String content) {
// Ideally, we would just be able to send the input text into stdin, but
// there's no way to do that synchronously, and converting dartdoc to an
// async model of execution is a huge amount of work. Using dart:cli's
// waitFor feels like a hack (and requires a similar amount of work anyhow
// to fix order of execution issues). So, instead, we have the tool take a
// filename as part of its arguments, and write the input to a temporary
// file before running the tool synchronously.

// Write the content to a temp file.
var tmpFile = ToolTempFileTracker.createInstance(resourceProvider)
.createTemporaryFile();
tmpFile.writeAsStringSync(content);
return pathContext.absolute(tmpFile.path);
}

// TODO(srawlins): Unit tests.
List<String> _substituteInArgs(
List<String> args, Map<String, String> envWithInput) {
var substitutions = envWithInput.map<RegExp, String>((key, value) {
var escapedKey = RegExp.escape(key);
return MapEntry(RegExp('\\\$(\\($escapedKey\\)|$escapedKey\\b)'), value);
});

var argsWithInput = <String>[];
for (var arg in args) {
var newArg = arg;
substitutions
.forEach((regex, value) => newArg = newArg.replaceAll(regex, value));
argsWithInput.add(newArg);
}

return argsWithInput;
}

p.Context get pathContext => toolConfiguration.resourceProvider.pathContext;
ResourceProvider get resourceProvider => toolConfiguration.resourceProvider;

p.Context get pathContext => resourceProvider.pathContext;
}
18 changes: 9 additions & 9 deletions test/tool_runner_test.dart
Expand Up @@ -112,8 +112,8 @@ echo:
test('can invoke a Dart tool, and second run is a snapshot.', () async {
var result = await runner.run(
['drill', r'--file=$INPUT'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isEmpty);
expect(result, contains('--file=<INPUT_FILE>'));
Expand All @@ -124,8 +124,8 @@ echo:
expect(setupFile.existsSync(), isFalse);
result = await runner.run(
['drill', r'--file=$INPUT'],
errorCallback,
content: 'TEST INPUT 2',
toolErrorCallback: errorCallback,
);
expect(errors, isEmpty);
expect(result, contains('--file=<INPUT_FILE>'));
Expand All @@ -136,8 +136,8 @@ echo:
test('can invoke a Dart tool', () async {
var result = await runner.run(
['drill', r'--file=$INPUT'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isEmpty);
expect(result, contains('Script location is in snapshot cache.'));
Expand All @@ -148,17 +148,17 @@ echo:
test('can invoke a non-Dart tool', () async {
var result = await runner.run(
['non_dart', '--version'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isEmpty);
expect(result, isEmpty); // Output is on stderr.
});
test('can invoke a pre-snapshotted tool', () async {
var result = await runner.run(
['snapshot_drill', r'--file=$INPUT'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isEmpty);
expect(result, contains('--file=<INPUT_FILE>'));
Expand All @@ -167,8 +167,8 @@ echo:
test('can invoke a tool with a setup action', () async {
var result = await runner.run(
['setup_drill', r'--file=$INPUT'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isEmpty);
expect(result, contains('--file=<INPUT_FILE>'));
Expand All @@ -178,8 +178,8 @@ echo:
test('fails if tool not in tool map', () async {
var result = await runner.run(
['hammer', r'--file=$INPUT'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isNotEmpty);
expect(
Expand All @@ -189,8 +189,8 @@ echo:
test('fails if tool returns non-zero status', () async {
var result = await runner.run(
['drill', r'--file=/a/missing/file'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isNotEmpty);
expect(errors[0], contains('Tool "drill" returned non-zero exit code'));
Expand All @@ -199,8 +199,8 @@ echo:
test("fails if tool in tool map doesn't exist", () async {
var result = await runner.run(
['missing'],
errorCallback,
content: 'TEST INPUT',
toolErrorCallback: errorCallback,
);
expect(errors, isNotEmpty);
expect(errors[0],
Expand Down

0 comments on commit 0e3b727

Please sign in to comment.