From 7befeda451b12e9ed78f4deb436a995fd7e1a261 Mon Sep 17 00:00:00 2001
From: Stuart Morgan <stuartmorgan@google.com>
Date: Tue, 10 May 2022 16:21:23 -0400
Subject: [PATCH] [tools] Fix `publish` flag calculation

Makes `getStringListArg` safer, by preventing accidental modification of
the arg parser results themselves. Also changes the way `publish` works
to determine the flags to pass once, rather than once per package, since
they don't change on each run.

Either fixes the accumulation of `--force` flags when publishing
multiple packages. The former is a general safety fix for the internal
utility API, and the latter is a minor cleanup that is no longer
necessary, but avoids pointless duplicate work.
---
 script/tool/CHANGELOG.md                      |  1 +
 .../tool/lib/src/common/plugin_command.dart   |  4 ++-
 .../tool/lib/src/publish_plugin_command.dart  | 17 ++++++-----
 .../test/publish_plugin_command_test.dart     | 29 +++++++++++++++++++
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index 9ed2a9278653..a8a8268c047f 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## NEXT
 
 - Fixes changelog validation when reverting to a `NEXT` state.
+- Fixes multiplication of `--force` flag when publishing multiple packages.
 
 ## 0.8.5
 
diff --git a/script/tool/lib/src/common/plugin_command.dart b/script/tool/lib/src/common/plugin_command.dart
index 0ec890368fb5..be9fb23e57a5 100644
--- a/script/tool/lib/src/common/plugin_command.dart
+++ b/script/tool/lib/src/common/plugin_command.dart
@@ -192,7 +192,9 @@ abstract class PluginCommand extends Command<void> {
 
   /// Convenience accessor for List<String> arguments.
   List<String> getStringListArg(String key) {
-    return (argResults![key] as List<String>?) ?? <String>[];
+    // Clone the list so that if a caller modifies the result it won't change
+    // the actual arguments list for future queries.
+    return List<String>.from(argResults![key] as List<String>? ?? <String>[]);
   }
 
   /// If true, commands should log timing information that might be useful in
diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart
index 05f0afd0c06f..7aa70bd4fd1c 100644
--- a/script/tool/lib/src/publish_plugin_command.dart
+++ b/script/tool/lib/src/publish_plugin_command.dart
@@ -121,6 +121,8 @@ class PublishPluginCommand extends PackageLoopingCommand {
   List<String> _existingGitTags = <String>[];
   // The remote to push tags to.
   late _RemoteInfo _remote;
+  // Flags to pass to `pub publish`.
+  late List<String> _publishFlags;
 
   @override
   String get successSummaryMessage => 'published';
@@ -149,6 +151,11 @@ class PublishPluginCommand extends PackageLoopingCommand {
     _existingGitTags = (existingTagsResult.stdout as String).split('\n')
       ..removeWhere((String element) => element.isEmpty);
 
+    _publishFlags = <String>[
+      ...getStringListArg(_pubFlagsOption),
+      if (getBoolArg(_skipConfirmationFlag)) '--force',
+    ];
+
     if (getBoolArg(_dryRunFlag)) {
       print('=============== DRY RUN ===============');
     }
@@ -333,22 +340,18 @@ Safe to ignore if the package is deleted in this commit.
 
   Future<bool> _publish(RepositoryPackage package) async {
     print('Publishing...');
-    final List<String> publishFlags = getStringListArg(_pubFlagsOption);
-    print('Running `pub publish ${publishFlags.join(' ')}` in '
+    print('Running `pub publish ${_publishFlags.join(' ')}` in '
         '${package.directory.absolute.path}...\n');
     if (getBoolArg(_dryRunFlag)) {
       return true;
     }
 
-    if (getBoolArg(_skipConfirmationFlag)) {
-      publishFlags.add('--force');
-    }
-    if (publishFlags.contains('--force')) {
+    if (_publishFlags.contains('--force')) {
       _ensureValidPubCredential();
     }
 
     final io.Process publish = await processRunner.start(
-        flutterCommand, <String>['pub', 'publish'] + publishFlags,
+        flutterCommand, <String>['pub', 'publish', ..._publishFlags],
         workingDirectory: package.directory);
     publish.stdout.transform(utf8.decoder).listen((String data) => print(data));
     publish.stderr.transform(utf8.decoder).listen((String data) => print(data));
diff --git a/script/tool/test/publish_plugin_command_test.dart b/script/tool/test/publish_plugin_command_test.dart
index 857828ab9306..46ce93583e51 100644
--- a/script/tool/test/publish_plugin_command_test.dart
+++ b/script/tool/test/publish_plugin_command_test.dart
@@ -224,6 +224,35 @@ void main() {
               plugin.path)));
     });
 
+    test('--force is only added once, regardless of plugin count', () async {
+      _createMockCredentialFile();
+      final RepositoryPackage plugin1 =
+          createFakePlugin('plugin_a', packagesDir, examples: <String>[]);
+      final RepositoryPackage plugin2 =
+          createFakePlugin('plugin_b', packagesDir, examples: <String>[]);
+
+      await runCapturingPrint(commandRunner, <String>[
+        'publish-plugin',
+        '--packages=plugin_a,plugin_b',
+        '--skip-confirmation',
+        '--pub-publish-flags',
+        '--server=bar'
+      ]);
+
+      expect(
+          processRunner.recordedCalls,
+          containsAllInOrder(<ProcessCall>[
+            ProcessCall(
+                flutterCommand,
+                const <String>['pub', 'publish', '--server=bar', '--force'],
+                plugin1.path),
+            ProcessCall(
+                flutterCommand,
+                const <String>['pub', 'publish', '--server=bar', '--force'],
+                plugin2.path),
+          ]));
+    });
+
     test('throws if pub publish fails', () async {
       createFakePlugin('foo', packagesDir, examples: <String>[]);