Fix in dartdev to enable the query of help output for pub subcommands

Bug: dart-lang#42965
Change-Id: Id2446b88fb983607f841c2fda81434087dc71d62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/159341
Commit-Queue: Jaime Wren <jwren@google.com>
Reviewed-by: Devon Carew <devoncarew@google.com>

pkg/dartdev/lib/dartdev.dart

@@ -23,6 +23,7 @@ import 'src/commands/run.dart';
 import 'src/commands/test.dart';
 import 'src/core.dart';
 import 'src/experiments.dart';
+import 'src/utils.dart';
 import 'src/vm_interop_handler.dart';
 
 /// This is typically called from bin/, but given the length of the method and
@@ -119,6 +120,14 @@ Future<void> runDartdev(List<String> args, SendPort port) async {
       analytics.sendEvent(eventCategory, commandName);
     }
 
+    // If ... help pub ... is in the args list, remove 'help', and add '--help'
+    // to the end of the list.  This will make it possible to use the help
+    // command to access subcommands of pub such as `dart help pub publish`, see
+    // https://github.com/dart-lang/sdk/issues/42965
+    if (PubUtils.shouldModifyArgs(args, runner.commands.keys.toList())) {
+      args = PubUtils.modifyArgs(args);
+    }
+
     // Finally, call the runner to execute the command, see DartdevRunner.
     result = await runner.run(args);
   } catch (e, st) {
@@ -218,6 +227,25 @@ class DartdevRunner<int> extends CommandRunner {
   String get invocation =>
       'dart [<vm-flags>] <command|dart-file> [<arguments>]';
 
+  void addExperimentalFlags(ArgParser argParser, bool verbose) {
+    List<ExperimentalFeature> features = experimentalFeatures;
+
+    Map<String, String> 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.',
+    );
+  }
+
   @override
   Future<int> runCommand(ArgResults topLevelResults) async {
     assert(!topLevelResults.arguments.contains('--disable-dartdev-analytics'));
@@ -255,23 +283,4 @@ class DartdevRunner<int> extends CommandRunner {
 
     return await super.runCommand(topLevelResults);
   }
-
-  void addExperimentalFlags(ArgParser argParser, bool verbose) {
-    List<ExperimentalFeature> features = experimentalFeatures;
-
-    Map<String, String> 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',
-      allowedHelp: verbose ? allowedHelp : null,
-      help: 'Enable one or more experimental features '
-          '(see dart.dev/go/experiments).',
-    );
-  }
 }

pkg/dartdev/lib/src/utils.dart

@@ -3,8 +3,16 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:io';
+
 import 'package:path/path.dart' as p;
 
+/// Given a data structure which is a Map of String to dynamic values, return
+/// the same structure (`Map<String, dynamic>`) with the correct runtime types.
+Map<String, dynamic> castStringKeyedMap(dynamic untyped) {
+  final Map<dynamic, dynamic> map = untyped as Map<dynamic, dynamic>;
+  return map?.cast<String, dynamic>();
+}
+
 /// Emit the given word with the correct pluralization.
 String pluralize(String word, int count) => count == 1 ? word : '${word}s';
 
@@ -25,11 +33,28 @@ String trimEnd(String s, String suffix) {
   return s;
 }
 
-/// Given a data structure which is a Map of String to dynamic values, return
-/// the same structure (`Map<String, dynamic>`) with the correct runtime types.
-Map<String, dynamic> castStringKeyedMap(dynamic untyped) {
-  final Map<dynamic, dynamic> map = untyped as Map<dynamic, dynamic>;
-  return map?.cast<String, dynamic>();
+/// Static util methods used in dartdev to potentially modify the order of the
+/// arguments passed into dartdev.
+class PubUtils {
+  /// If ... help pub ..., and no other verb (such as 'analyze') appears before
+  /// the ... help pub ... in the argument list, then return true.
+  static bool shouldModifyArgs(List<String> args, List<String> allCmds) =>
+      args != null &&
+      allCmds != null &&
+      args.isNotEmpty &&
+      allCmds.isNotEmpty &&
+      args.firstWhere((arg) => allCmds.contains(arg)) == 'help' &&
+      args.contains('help') &&
+      args.contains('pub') &&
+      args.indexOf('help') + 1 == args.indexOf('pub');
+
+  /// If [doModifyArgs] returns true, then this method returns a modified copy
+  /// of the argument list, 'help' is removed from the interior of the list, and
+  /// '--help' is added to the end of the list of arguments. This method returns
+  /// a modified copy of the list, the list itself is not modified.
+  static List<String> modifyArgs(List<String> args) => List.from(args)
+    ..remove('help')
+    ..add('--help');
 }
 
 extension FileSystemEntityExtension on FileSystemEntity {

pkg/dartdev/test/commands/pub_test.dart

@@ -2,6 +2,8 @@
 // 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 'dart:io';
+
 import 'package:test/test.dart';
 
 import '../utils.dart';
@@ -15,23 +17,53 @@ void pub() {
 
   tearDown(() => p?.dispose());
 
-  test('implicit --help', () {
-    p = project();
-    var result = p.runSync('pub', []);
+  void _assertPubHelpInvoked(ProcessResult result) {
+    expect(result, isNotNull);
     expect(result.exitCode, 0);
     expect(result.stdout, contains('Pub is a package manager for Dart'));
     expect(result.stdout, contains('Available commands:'));
     expect(result.stderr, isEmpty);
+  }
+
+  test('implicit --help', () {
+    _assertPubHelpInvoked(project().runSync('pub', []));
   });
 
   test('--help', () {
+    _assertPubHelpInvoked(project().runSync('pub', ['--help']));
+  });
+
+  test('-h', () {
+    _assertPubHelpInvoked(project().runSync('pub', ['-h']));
+  });
+
+  test('help cache', () {
     p = project();
-    var result = p.runSync('pub', ['--help']);
+    var result = p.runSync('pub', ['help', 'cache']);
+    var result2 = p.runSync('pub', ['cache', '--help']);
 
     expect(result.exitCode, 0);
-    expect(result.stdout, contains('Pub is a package manager for Dart'));
-    expect(result.stdout, contains('Available commands:'));
+
+    expect(result.stdout, contains('Work with the system cache.'));
+    expect(result.stdout, result2.stdout);
+
+    expect(result.stderr, isEmpty);
+    expect(result.stderr, result2.stderr);
+  });
+
+  test('help publish', () {
+    p = project();
+    var result = p.runSync('pub', ['help', 'publish']);
+    var result2 = p.runSync('pub', ['publish', '--help']);
+
+    expect(result.exitCode, 0);
+
+    expect(result.stdout,
+        contains('Publish the current package to pub.dartlang.org.'));
+    expect(result.stdout, result2.stdout);
+
     expect(result.stderr, isEmpty);
+    expect(result.stderr, result2.stderr);
   });
 
   test('--enable-experiment pub run', () {

pkg/dartdev/test/utils_test.dart

@@ -100,6 +100,37 @@ void main() {
       expect(File('bar.bart').name, 'bar.bart');
     });
   });
+
+  group('PubUtils', () {
+    test('doModifyArgs', () {
+      const allCmds = ['analyze', 'help', 'pub', 'migrate'];
+      expect(PubUtils.shouldModifyArgs(null, null), isFalse);
+      expect(PubUtils.shouldModifyArgs([], null), isFalse);
+      expect(PubUtils.shouldModifyArgs(null, []), isFalse);
+      expect(PubUtils.shouldModifyArgs([], []), isFalse);
+      expect(PubUtils.shouldModifyArgs(['help'], allCmds), isFalse);
+      expect(PubUtils.shouldModifyArgs(['pub'], allCmds), isFalse);
+      expect(PubUtils.shouldModifyArgs(['analyze', 'help', 'pub'], allCmds),
+          isFalse);
+
+      expect(PubUtils.shouldModifyArgs(['--some-flag', 'help', 'pub'], allCmds),
+          isTrue);
+      expect(PubUtils.shouldModifyArgs(['help', 'pub'], allCmds), isTrue);
+      expect(PubUtils.shouldModifyArgs(['help', 'pub', 'publish'], allCmds),
+          isTrue);
+      expect(PubUtils.shouldModifyArgs(['help', 'pub', 'analyze'], allCmds),
+          isTrue);
+    });
+
+    test('modifyArgs', () {
+      expect(PubUtils.modifyArgs(['--some-flag', 'help', 'pub']),
+          orderedEquals(['--some-flag', 'pub', '--help']));
+      expect(PubUtils.modifyArgs(['help', 'pub']),
+          orderedEquals(['pub', '--help']));
+      expect(PubUtils.modifyArgs(['help', 'pub', 'publish']),
+          orderedEquals(['pub', 'publish', '--help']));
+    });
+  });
 }
 
 const String _packageData = '''{