From fc1999de15c4d46f8e4aebe3e707c00f6c8f079b Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 30 Jun 2020 23:22:42 +0000 Subject: [PATCH] [dartdev] Implement --enable-experiments for `dart run` and `dart pub run`. Related to https://github.com/dart-lang/sdk/issues/42339. Change-Id: I3fdeee33dcad0ca031f483e2e3692be300392958 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/152960 Reviewed-by: Jaime Wren Commit-Queue: Devon Carew --- pkg/analyzer/lib/source/line_info.dart | 3 +- .../lib/src/dart/analysis/experiments.dart | 2 +- .../lib/src/dart/analysis/experiments.g.dart | 4 +- pkg/dartdev/lib/dartdev.dart | 41 +++++++++++++++++-- pkg/dartdev/lib/src/commands/analyze.dart | 2 + pkg/dartdev/lib/src/commands/pub.dart | 22 +++++++++- pkg/dartdev/lib/src/commands/run.dart | 29 +++++++++---- pkg/dartdev/lib/src/experiments.dart | 25 +++++++++++ pkg/dartdev/test/commands/flag_test.dart | 12 ++++++ pkg/dartdev/test/commands/pub_test.dart | 38 +++++++++++++++++ pkg/dartdev/test/commands/run_test.dart | 18 +++++++- pkg/dartdev/test/experiments_test.dart | 18 ++++++++ tools/experimental_features.yaml | 6 +-- 13 files changed, 197 insertions(+), 23 deletions(-) create mode 100644 pkg/dartdev/lib/src/experiments.dart create mode 100644 pkg/dartdev/test/experiments_test.dart diff --git a/pkg/analyzer/lib/source/line_info.dart b/pkg/analyzer/lib/source/line_info.dart index c4126fe9bbd..dbd0e2fa1b3 100644 --- a/pkg/analyzer/lib/source/line_info.dart +++ b/pkg/analyzer/lib/source/line_info.dart @@ -47,8 +47,7 @@ class LineInfo { /// Return the location information for the character at the given [offset]. /// /// A future version of this API will return a [CharacterLocation] rather than - /// // ignore: deprecated_member_use_from_same_package - /// a [LineInfo_Location]. + /// a [LineInfo_Location]. // ignore: deprecated_member_use_from_same_package // ignore: deprecated_member_use_from_same_package LineInfo_Location getLocation(int offset) { var min = 0; diff --git a/pkg/analyzer/lib/src/dart/analysis/experiments.dart b/pkg/analyzer/lib/src/dart/analysis/experiments.dart index 0e3771b4931..55d1a348b90 100644 --- a/pkg/analyzer/lib/src/dart/analysis/experiments.dart +++ b/pkg/analyzer/lib/src/dart/analysis/experiments.dart @@ -33,7 +33,7 @@ class ExperimentStatus with _CurrentState implements FeatureSet { static final Version currentVersion = Version.parse(_currentVersion); /// A map containing information about all known experimental flags. - static const knownFeatures = _knownFeatures; + static const Map knownFeatures = _knownFeatures; final List _enableFlags; diff --git a/pkg/analyzer/lib/src/dart/analysis/experiments.g.dart b/pkg/analyzer/lib/src/dart/analysis/experiments.g.dart index 0fb343cfcf9..98663d93fc0 100644 --- a/pkg/analyzer/lib/src/dart/analysis/experiments.g.dart +++ b/pkg/analyzer/lib/src/dart/analysis/experiments.g.dart @@ -125,7 +125,7 @@ class ExperimentalFeatures { enableString: EnableString.nonfunction_type_aliases, isEnabledByDefault: IsEnabledByDefault.nonfunction_type_aliases, isExpired: IsExpired.nonfunction_type_aliases, - documentation: 'Type aliases define a , not just a .', + documentation: 'Type aliases define a , not just a ', firstSupportedVersion: null, ); @@ -161,7 +161,7 @@ class ExperimentalFeatures { enableString: EnableString.variance, isEnabledByDefault: IsEnabledByDefault.variance, isExpired: IsExpired.variance, - documentation: 'Sound variance.', + documentation: 'Sound variance', firstSupportedVersion: null, ); diff --git a/pkg/dartdev/lib/dartdev.dart b/pkg/dartdev/lib/dartdev.dart index c1cc2244b2f..7323422b2f8 100644 --- a/pkg/dartdev/lib/dartdev.dart +++ b/pkg/dartdev/lib/dartdev.dart @@ -4,6 +4,7 @@ import 'dart:io' as io; +import 'package:analyzer/src/dart/analysis/experiments.dart'; import 'package:args/args.dart'; import 'package:args/command_runner.dart'; import 'package:cli_util/cli_logging.dart'; @@ -19,13 +20,14 @@ import 'src/commands/pub.dart'; import 'src/commands/run.dart'; import 'src/commands/test.dart'; import 'src/core.dart'; +import 'src/experiments.dart'; /// This is typically called from bin/, but given the length of the method and /// analytics logic, it has been moved here. Also note that this method calls /// [io.exit(code)] directly. Future runDartdev(List args) async { final stopwatch = Stopwatch(); - dynamic result; + int result; // The Analytics instance used to report information back to Google Analytics, // see lib/src/analytics.dart. @@ -104,7 +106,7 @@ Future runDartdev(List args) async { stopwatch.stop(); // Set the exitCode, if it wasn't set in the catch block above. - exitCode ??= result is int ? result : 0; + exitCode ??= result ?? 0; // Send analytics before exiting if (analytics.enabled) { @@ -151,8 +153,7 @@ class DartdevRunner extends CommandRunner { argParser.addFlag('disable-analytics', negatable: false, help: 'Disable anonymous analytics.'); - // TODO(jwren): hook up. - argParser.addMultiOption('enable-experiment', hide: true); + addExperimentalFlags(argParser, verbose); // A hidden flag to disable analytics on this run, this constructor can be // called with this flag, but should be removed before run() is called as @@ -179,6 +180,7 @@ class DartdevRunner extends CommandRunner { @override Future runCommand(ArgResults topLevelResults) async { assert(!topLevelResults.arguments.contains('--disable-dartdev-analytics')); + if (topLevelResults.command == null && topLevelResults.arguments.isNotEmpty) { final firstArg = topLevelResults.arguments.first; @@ -195,6 +197,37 @@ class DartdevRunner extends CommandRunner { final Ansi ansi = Ansi(Ansi.terminalSupportsAnsi); log = isVerbose ? Logger.verbose(ansi: ansi) : Logger.standard(ansi: ansi); + if (wereExperimentsSpecified(topLevelResults)) { + List experimentIds = specifiedExperiments(topLevelResults); + for (ExperimentalFeature feature in experimentalFeatures) { + // We allow default true flags, but complain when they are passed in. + if (feature.isEnabledByDefault && + experimentIds.contains(feature.enableString)) { + print("'${feature.enableString}' is now enabled by default; this " + 'flag is no longer required.'); + } + } + } + return await super.runCommand(topLevelResults); } + + void addExperimentalFlags(ArgParser argParser, bool verbose) { + List features = experimentalFeatures; + + Map allowedHelp = {}; + for (ExperimentalFeature feature in features) { + String suffix = + feature.isEnabledByDefault ? ' (no-op - enabled by default)' : ''; + allowedHelp[feature.enableString] = '${feature.documentation}$suffix'; + } + + argParser.addMultiOption( + experimentFlagName, + valueHelp: 'experiment', + allowed: features.map((feature) => feature.enableString), + allowedHelp: verbose ? allowedHelp : null, + help: 'Enable one or more experimental features.', + ); + } } diff --git a/pkg/dartdev/lib/src/commands/analyze.dart b/pkg/dartdev/lib/src/commands/analyze.dart index ccf9a570d4a..a98443cb4c7 100644 --- a/pkg/dartdev/lib/src/commands/analyze.dart +++ b/pkg/dartdev/lib/src/commands/analyze.dart @@ -12,6 +12,8 @@ import '../sdk.dart'; import '../utils.dart'; import 'analyze_impl.dart'; +// TODO: Support enable-experiment for 'dart analyze'. + class AnalyzeCommand extends DartdevCommand { AnalyzeCommand({bool verbose = false}) : super('analyze', "Analyze the project's Dart code.") { diff --git a/pkg/dartdev/lib/src/commands/pub.dart b/pkg/dartdev/lib/src/commands/pub.dart index 0019696f216..36aa28a9cc2 100644 --- a/pkg/dartdev/lib/src/commands/pub.dart +++ b/pkg/dartdev/lib/src/commands/pub.dart @@ -8,6 +8,7 @@ import 'dart:io'; import 'package:args/args.dart'; import '../core.dart'; +import '../experiments.dart'; import '../sdk.dart'; class PubCommand extends DartdevCommand { @@ -42,7 +43,26 @@ class PubCommand extends DartdevCommand { @override FutureOr run() async { final command = sdk.pub; - final args = argResults.arguments; + var args = argResults.arguments; + + // Pass any --enable-experiment options along. + if (args.isNotEmpty && wereExperimentsSpecified(globalResults)) { + List experimentIds = specifiedExperiments(globalResults); + + if (args.first == 'run') { + args = [ + ...args.sublist(0, 1), + '--$experimentFlagName=${experimentIds.join(',')}', + ...args.sublist(1), + ]; + } else if (args.length > 1 && args[0] == 'global' && args[0] == 'run') { + args = [ + ...args.sublist(0, 2), + '--$experimentFlagName=${experimentIds.join(',')}', + ...args.sublist(2), + ]; + } + } log.trace('$command ${args.join(' ')}'); diff --git a/pkg/dartdev/lib/src/commands/run.dart b/pkg/dartdev/lib/src/commands/run.dart index 7496a2a1f41..5d9bcfa8d6a 100644 --- a/pkg/dartdev/lib/src/commands/run.dart +++ b/pkg/dartdev/lib/src/commands/run.dart @@ -11,6 +11,7 @@ import 'package:dds/dds.dart'; import 'package:path/path.dart'; import '../core.dart'; +import '../experiments.dart'; import '../sdk.dart'; import '../utils.dart'; @@ -55,7 +56,7 @@ Run a Dart file.'''); @override FutureOr run() async { // The command line arguments after 'run' - final args = argResults.arguments.toList(); + var args = argResults.arguments.toList(); var argsContainFileOrHelp = false; for (var arg in args) { @@ -70,6 +71,7 @@ Run a Dart file.'''); } final cwd = Directory.current; + if (!argsContainFileOrHelp && cwd.existsSync()) { var foundImplicitFileToRun = false; var cwdName = cwd.name; @@ -90,6 +92,7 @@ Run a Dart file.'''); break; } } + if (!foundImplicitFileToRun) { log.stderr( 'Could not find the implicit file to run: ' @@ -98,6 +101,16 @@ Run a Dart file.'''); } } + // Pass any --enable-experiment options along. + // todo: test + if (args.isNotEmpty && wereExperimentsSpecified(globalResults)) { + List experimentIds = specifiedExperiments(globalResults); + args = [ + '--$experimentFlagName=${experimentIds.join(',')}', + ...args, + ]; + } + // If the user wants to start a debugging session we need to do some extra // work and spawn a Dart Development Service (DDS) instance. DDS is a VM // service intermediary which implements the VM service protocol and @@ -107,14 +120,14 @@ Run a Dart file.'''); element.startsWith('--observe') || element.startsWith('--enable-vm-service'))) { return await _DebuggingSession(this, args).start(); + } else { + // Starting in ProcessStartMode.inheritStdio mode means the child process + // can detect support for ansi chars. + final process = await Process.start( + sdk.dart, ['--disable-dart-dev', ...args], + mode: ProcessStartMode.inheritStdio); + return process.exitCode; } - - // Starting in ProcessStartMode.inheritStdio mode means the child process - // can detect support for ansi chars. - final process = await Process.start( - sdk.dart, ['--disable-dart-dev', ...args], - mode: ProcessStartMode.inheritStdio); - return process.exitCode; } } diff --git a/pkg/dartdev/lib/src/experiments.dart b/pkg/dartdev/lib/src/experiments.dart new file mode 100644 index 00000000000..052c827a79b --- /dev/null +++ b/pkg/dartdev/lib/src/experiments.dart @@ -0,0 +1,25 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/src/dart/analysis/experiments.dart'; +import 'package:args/args.dart'; + +const experimentFlagName = 'enable-experiment'; + +/// Return a list of all the non-expired Dart experiments. +List get experimentalFeatures { + List features = ExperimentStatus.knownFeatures.values + .where((feature) => !feature.isExpired) + .toList(); + features.sort((a, b) => a.enableString.compareTo(b.enableString)); + return features; +} + +/// Return whether any Dart experiments were specified by the user. +bool wereExperimentsSpecified(ArgResults argResults) => + argResults.wasParsed(experimentFlagName); + +/// Return the list of Dart experiment flags specified by the user. +List specifiedExperiments(ArgResults argResults) => + argResults[experimentFlagName]; diff --git a/pkg/dartdev/test/commands/flag_test.dart b/pkg/dartdev/test/commands/flag_test.dart index 978c6aed9be..748240f5e25 100644 --- a/pkg/dartdev/test/commands/flag_test.dart +++ b/pkg/dartdev/test/commands/flag_test.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'package:args/args.dart'; import 'package:args/command_runner.dart'; import 'package:dartdev/dartdev.dart'; import 'package:test/test.dart'; @@ -37,6 +38,17 @@ void command() { } }); }); + + test('enable experiments flag is supported', () { + final args = [ + '--disable-dartdev-analytics', + '--enable-experiment=non-nullable' + ]; + final runner = DartdevRunner(args); + ArgResults results = runner.parse(args); + expect(results['enable-experiment'], isNotEmpty); + expect(results['enable-experiment'].first, 'non-nullable'); + }); } void help() { diff --git a/pkg/dartdev/test/commands/pub_test.dart b/pkg/dartdev/test/commands/pub_test.dart index b8a98eeb4fe..ed9f801f272 100644 --- a/pkg/dartdev/test/commands/pub_test.dart +++ b/pkg/dartdev/test/commands/pub_test.dart @@ -34,6 +34,44 @@ void pub() { expect(result.stderr, isEmpty); }); + test('--enable-experiment pub run', () { + p = project(); + p.file('bin/main.dart', + "void main() { int a; a = null; print('a is \$a.'); }"); + + // run 'pub get' + p.runSync('pub', ['get']); + + var result = p.runSync( + '--enable-experiment=non-nullable', ['pub', 'run', 'main.dart']); + + expect(result.exitCode, 254); + expect(result.stdout, isEmpty); + expect( + result.stderr, + contains("A value of type 'Null' can't be assigned to a variable of " + "type 'int'")); + }); + + test('pub run --enable-experiment', () { + p = project(); + p.file('bin/main.dart', + "void main() { int a; a = null; print('a is \$a.'); }"); + + // run 'pub get' + p.runSync('pub', ['get']); + + var result = p.runSync( + 'pub', ['run', '--enable-experiment=non-nullable', 'main.dart']); + + expect(result.exitCode, 254); + expect(result.stdout, isEmpty); + expect( + result.stderr, + contains("A value of type 'Null' can't be assigned to a variable of " + "type 'int'")); + }); + test('failure', () { p = project(mainSrc: 'int get foo => 1;\n'); var result = p.runSync('pub', ['deps']); diff --git a/pkg/dartdev/test/commands/run_test.dart b/pkg/dartdev/test/commands/run_test.dart index e6b8931084f..ac2527a1259 100644 --- a/pkg/dartdev/test/commands/run_test.dart +++ b/pkg/dartdev/test/commands/run_test.dart @@ -36,6 +36,20 @@ void run() { expect(result.exitCode, 0); }); + test('--enable-experiment', () { + p = project(); + p.file('main.dart', "void main() { int a; a = null; print('a is \$a.'); }"); + var result = + p.runSync('--enable-experiment=non-nullable', ['run', 'main.dart']); + + expect(result.exitCode, 254); + expect(result.stdout, isEmpty); + expect( + result.stderr, + contains("A value of type 'Null' can't be assigned to a variable of " + "type 'int'")); + }); + test('no such file', () { p = project(mainSrc: "void main() { print('Hello World'); }"); ProcessResult result = @@ -47,7 +61,7 @@ void run() { test('implicit packageName.dart', () { // TODO(jwren) circle back to reimplement this test if possible, the file - // name (package name) will be the name of the temporary directory on disk + // name (package name) will be the name of the temporary directory on disk p = project(mainSrc: "void main() { print('Hello World'); }"); p.file('bin/main.dart', "void main() { print('Hello main.dart'); }"); ProcessResult result = p.runSync('run', []); @@ -57,7 +71,7 @@ void run() { expect(result.exitCode, 0); }, skip: true); - //Could not find the implicit file to run: bin + // Could not find the implicit file to run: bin test('missing implicit packageName.dart', () { p = project(mainSrc: "void main() { print('Hello World'); }"); p.file('bin/foo.dart', "void main() { print('Hello main.dart'); }"); diff --git a/pkg/dartdev/test/experiments_test.dart b/pkg/dartdev/test/experiments_test.dart new file mode 100644 index 00000000000..f9bec0e8926 --- /dev/null +++ b/pkg/dartdev/test/experiments_test.dart @@ -0,0 +1,18 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:dartdev/src/experiments.dart'; +import 'package:test/test.dart'; + +void main() { + group('experiments', () { + test('experimentalFeatures', () { + expect(experimentalFeatures, isNotEmpty); + expect( + experimentalFeatures.map((experiment) => experiment.enableString), + contains('non-nullable'), + ); + }); + }); +} diff --git a/tools/experimental_features.yaml b/tools/experimental_features.yaml index 1a60f75f919..a680662ea0f 100644 --- a/tools/experimental_features.yaml +++ b/tools/experimental_features.yaml @@ -95,13 +95,13 @@ features: help: "Triple-shift operator" variance: - help: "Sound variance." + help: "Sound variance" nonfunction-type-aliases: - help: "Type aliases define a , not just a ." + help: "Type aliases define a , not just a " alternative-invalidation-strategy: - help: "Alternative invalidation strategy for incremental compilation." + help: "Alternative invalidation strategy for incremental compilation" category: "CFE" #