Skip to content

Rename --checked to --check-asserts #1964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions lib/src/command/global_run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,28 @@ import '../utils.dart';

/// Handles the `global run` pub command.
class GlobalRunCommand extends PubCommand {
String get name => "run";
String get name => 'run';
String get description =>
"Run an executable from a globally activated package.\n"
'Run an executable from a globally activated package.\n'
"NOTE: We are currently optimizing this command's startup time.";
String get invocation => "pub global run <package>:<executable> [args...]";
String get invocation => 'pub global run <package>:<executable> [args...]';
bool get allowTrailingOptions => false;

GlobalRunCommand() {
argParser.addFlag("checked",
abbr: "c", help: "Enable runtime type checks and assertions.");
argParser.addOption("mode", help: "Deprecated option", hide: true);
argParser.addFlag('enable-asserts', help: 'Enable assert statements.');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following seems clearer / more accurate as help text: Enable runtime checks of assert statements. WDYT? /cc @kwalrath @kevmoo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this directly from dart --help. If we want to change this I think we should do it in both places.

argParser.addFlag('checked', abbr: 'c', hide: true);
argParser.addOption('mode', help: 'Deprecated option', hide: true);
}

Future run() async {
if (argResults.rest.isEmpty) {
usageException("Must specify an executable to run.");
usageException('Must specify an executable to run.');
}

var package;
var executable = argResults.rest[0];
if (executable.contains(":")) {
var parts = split1(executable, ":");
if (executable.contains(':')) {
var parts = split1(executable, ':');
package = parts[0];
executable = parts[1];
} else {
Expand All @@ -48,12 +48,12 @@ class GlobalRunCommand extends PubCommand {
'package.');
}

if (argResults.wasParsed("mode")) {
log.warning("The --mode flag is deprecated and has no effect.");
if (argResults.wasParsed('mode')) {
log.warning('The --mode flag is deprecated and has no effect.');
}

var exitCode = await globals.runExecutable(package, executable, args,
checked: argResults["checked"]);
checked: argResults['enable-asserts'] || argResults['checked']);
await flushThenExit(exitCode);
}
}
30 changes: 15 additions & 15 deletions lib/src/command/run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,20 @@ import '../utils.dart';

/// Handles the `run` pub command.
class RunCommand extends PubCommand {
String get name => "run";
String get description => "Run an executable from a package.";
String get invocation => "pub run <executable> [args...]";
String get name => 'run';
String get description => 'Run an executable from a package.';
String get invocation => 'pub run <executable> [args...]';
bool get allowTrailingOptions => false;

RunCommand() {
argParser.addFlag("checked",
abbr: "c", help: "Enable runtime type checks and assertions.");
argParser.addOption("mode", help: "Deprecated option", hide: true);
argParser.addFlag('enable-asserts', help: 'Enable assert statements.');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

argParser.addFlag('checked', abbr: 'c', hide: true);
argParser.addOption('mode', help: 'Deprecated option', hide: true);
}

Future run() async {
if (argResults.rest.isEmpty) {
usageException("Must specify an executable to run.");
usageException('Must specify an executable to run.');
}

var package = entrypoint.root.name;
Expand All @@ -36,31 +36,31 @@ class RunCommand extends PubCommand {

// A command like "foo:bar" runs the "bar" script from the "foo" package.
// If there is no colon prefix, default to the root package.
if (executable.contains(":")) {
var components = split1(executable, ":");
if (executable.contains(':')) {
var components = split1(executable, ':');
package = components[0];
executable = components[1];

if (p.split(executable).length > 1) {
usageException(
"Cannot run an executable in a subdirectory of a dependency.");
'Cannot run an executable in a subdirectory of a dependency.');
}
} else if (onlyIdentifierRegExp.hasMatch(executable)) {
// "pub run foo" means the same thing as "pub run foo:foo" as long as
// "foo" is a valid Dart identifier (and thus package name).
package = executable;
}

if (argResults.wasParsed("mode")) {
log.warning("The --mode flag is deprecated and has no effect.");
if (argResults.wasParsed('mode')) {
log.warning('The --mode flag is deprecated and has no effect.');
}

// The user may pass in an executable without an extension, but the file
// to actually execute will always have one.
if (p.extension(executable) != ".dart") executable += ".dart";
if (p.extension(executable) != '.dart') executable += '.dart';

var snapshotPath = p.join(
entrypoint.cachePath, "bin", package, "$executable.snapshot.dart2");
entrypoint.cachePath, 'bin', package, '$executable.snapshot.dart2');

// Don't ever compile snapshots for mutable packages, since their code may
// change later on.
Expand All @@ -69,7 +69,7 @@ class RunCommand extends PubCommand {
!entrypoint.packageGraph.isPackageMutable(package));

var exitCode = await runExecutable(entrypoint, package, executable, args,
checked: argResults['checked'],
checked: argResults['enable-asserts'] || argResults['checked'],
snapshotPath: useSnapshot ? snapshotPath : null,
recompile: entrypoint.precompileExecutables);
await flushThenExit(exitCode);
Expand Down
10 changes: 5 additions & 5 deletions lib/src/executable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import 'utils.dart';
///
/// Arguments from [args] will be passed to the spawned Dart application.
///
/// If [checked] is true, the program is run in checked mode.
/// If [checked] is true, the program is run with assertions enabled.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere: how about "with assertion checking enabled"?

///
/// If [packagesFile] is passed, it's used as the package config file path for
/// the executable. Otherwise, `entrypoint.packagesFile` is used.
Expand Down Expand Up @@ -117,7 +117,7 @@ Future<String> _executablePath(
return p.absolute(fullPath);
}

/// Like [runSnapshot], but runs [recompile] if [path] doesn't exist yet.
/// Like [_runSnapshot], but runs [recompile] if [path] doesn't exist yet.
///
/// Returns `null` if [path] doesn't exist and isn't generated by [recompile].
Future<int> _runOrCompileSnapshot(String path, Iterable<String> args,
Expand All @@ -130,7 +130,7 @@ Future<int> _runOrCompileSnapshot(String path, Iterable<String> args,
if (!fileExists(path)) return null;
}

return await runSnapshot(path, args,
return await _runSnapshot(path, args,
recompile: recompile, packagesFile: packagesFile, checked: checked);
}

Expand All @@ -141,12 +141,12 @@ Future<int> _runOrCompileSnapshot(String path, Iterable<String> args,
/// expected to regenerate a snapshot at [path], after which the snapshot will
/// be re-run.
///
/// If [checked] is set, runs the snapshot in checked mode.
/// If [checked] is set, runs the snapshot with assertions enabled.
///
/// Returns the snapshot's exit code.
///
/// This doesn't do any validation of the snapshot's SDK version.
Future<int> runSnapshot(String path, Iterable<String> args,
Future<int> _runSnapshot(String path, Iterable<String> args,
{Future<void> recompile(),
String packagesFile,
bool checked = false}) async {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/global_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ class GlobalPackages {
/// recompiled if the SDK has been upgraded since it was first compiled and
/// then run. Otherwise, it will be run from source.
///
/// If [checked] is true, the program is run in checked mode.
/// If [checked] is true, the program is run with assertions enabled.
///
/// Returns the exit code from the executable.
Future<int> runExecutable(
Expand Down
4 changes: 2 additions & 2 deletions test/global/run/errors_if_outside_bin_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ main() {
Cannot run an executable in a subdirectory of a global package.

Usage: pub global run <package>:<executable> [args...]
-h, --help Print this usage information.
-c, --[no-]checked Enable runtime type checks and assertions.
-h, --help Print this usage information.
--[no-]enable-asserts Enable assert statements.

Run "pub help" to see global options.
""", exitCode: exit_codes.USAGE);
Expand Down
8 changes: 4 additions & 4 deletions test/global/run/missing_executable_arg_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import '../../test_pub.dart';

main() {
test('fails if no executable was given', () {
return runPub(args: ["global", "run"], error: """
return runPub(args: ['global', 'run'], error: '''
Must specify an executable to run.

Usage: pub global run <package>:<executable> [args...]
-h, --help Print this usage information.
-c, --[no-]checked Enable runtime type checks and assertions.
-h, --help Print this usage information.
--[no-]enable-asserts Enable assert statements.

Run "pub help" to see global options.
""", exitCode: exit_codes.USAGE);
''', exitCode: exit_codes.USAGE);
});
}
13 changes: 7 additions & 6 deletions test/global/run/runs_script_in_checked_mode_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,18 @@ import '../../descriptor.dart' as d;
import '../../test_pub.dart';

main() {
test('runs a script in checked mode', () async {
test('runs a script with assertions enabled', () async {
await servePackages((builder) {
builder.serve("foo", "1.0.0", contents: [
d.dir("bin", [d.file("script.dart", "main() { assert(false); }")])
builder.serve('foo', '1.0.0', contents: [
d.dir('bin', [d.file('script.dart', 'main() { assert(false); }')])
]);
});

await runPub(args: ["global", "activate", "foo"]);
await runPub(args: ['global', 'activate', 'foo']);

var pub = await pubRun(global: true, args: ["--checked", "foo:script"]);
expect(pub.stderr, emitsThrough(contains("Failed assertion")));
var pub =
await pubRun(global: true, args: ['--enable-asserts', 'foo:script']);
expect(pub.stderr, emitsThrough(contains('Failed assertion')));
await pub.shouldExit(255);
});
}
8 changes: 4 additions & 4 deletions test/run/errors_if_no_executable_is_given_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ main() {
test('Errors if the executable does not exist.', () async {
await d.dir(appPath, [d.appPubspec()]).create();

await runPub(args: ["run"], error: """
await runPub(args: ['run'], error: '''
Must specify an executable to run.

Usage: pub run <executable> [args...]
-h, --help Print this usage information.
-c, --[no-]checked Enable runtime type checks and assertions.
-h, --help Print this usage information.
--[no-]enable-asserts Enable assert statements.

Run "pub help" to see global options.
""", exitCode: exit_codes.USAGE);
''', exitCode: exit_codes.USAGE);
});
}
12 changes: 6 additions & 6 deletions test/run/errors_if_path_in_dependency_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@ main() {
test(
'Errors if the executable is in a subdirectory in a '
'dependency.', () async {
await d.dir("foo", [d.libPubspec("foo", "1.0.0")]).create();
await d.dir('foo', [d.libPubspec('foo', '1.0.0')]).create();

await d.dir(appPath, [
d.appPubspec({
"foo": {"path": "../foo"}
'foo': {'path': '../foo'}
})
]).create();

await runPub(args: ["run", "foo:sub/dir"], error: """
await runPub(args: ['run', 'foo:sub/dir'], error: '''
Cannot run an executable in a subdirectory of a dependency.

Usage: pub run <executable> [args...]
-h, --help Print this usage information.
-c, --[no-]checked Enable runtime type checks and assertions.
-h, --help Print this usage information.
--[no-]enable-asserts Enable assert statements.

Run "pub help" to see global options.
""", exitCode: exit_codes.USAGE);
''', exitCode: exit_codes.USAGE);
});
}
8 changes: 4 additions & 4 deletions test/run/runs_the_script_in_checked_mode_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import '../descriptor.dart' as d;
import '../test_pub.dart';

main() {
test('runs the script in checked mode with "--checked"', () async {
test('runs the script with assertions enabled', () async {
await d.dir(appPath, [
d.appPubspec(),
d.dir("bin", [d.file("script.dart", "main() { assert(false); }")])
d.dir('bin', [d.file('script.dart', 'main() { assert(false); }')])
]).create();

await pubGet();
await runPub(
args: ["run", "--checked", "bin/script"],
error: contains("Failed assertion"),
args: ['run', '--enable-asserts', 'bin/script'],
error: contains('Failed assertion'),
exitCode: 255);
});
}
10 changes: 5 additions & 5 deletions test/run/runs_the_script_in_unchecked_mode_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ import 'package:test/test.dart';
import '../descriptor.dart' as d;
import '../test_pub.dart';

const SCRIPT = """
const SCRIPT = '''
main() {
assert(false);
print("no checks");
}
""";
''';

main() {
test('runs the script in unchecked mode by default', () async {
test('runs the script without assertions by default', () async {
await d.dir(appPath, [
d.appPubspec(),
d.dir("bin", [d.file("script.dart", SCRIPT)])
d.dir('bin', [d.file('script.dart', SCRIPT)])
]).create();

await pubGet();
await runPub(args: ["run", "bin/script"], output: contains("no checks"));
await runPub(args: ['run', 'bin/script'], output: contains('no checks'));
});
}