fix(shorebird_cli): always respect build-name and build-number args

packages/shorebird_cli/lib/src/commands/patch/patch_command.dart

@@ -47,18 +47,16 @@ class PatchCommand extends ShorebirdCommand {
         abbr: 'p',
         help: 'The platform(s) to to build this release for.',
         allowed: ReleaseType.values.map((e) => e.cliName).toList(),
-        // TODO(bryanoltman): uncomment this once https://github.com/dart-lang/args/pull/273 lands
-        // mandatory: true.
       )
       ..addOption(
-        'build-number',
-        help: '''
-An identifier used as an internal version number.
-Each build must have a unique identifier to differentiate it from previous builds.
-It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build.
-On Android it is used as "versionCode".
-On Xcode builds it is used as "CFBundleVersion".''',
-        defaultsTo: '1.0',
+        CommonArguments.buildNameArg.name,
+        help: CommonArguments.buildNameArg.description,
+        defaultsTo: CommonArguments.buildNameArg.defaultValue,
+      )
+      ..addOption(
+        CommonArguments.buildNumberArg.name,
+        help: CommonArguments.buildNumberArg.description,
+        defaultsTo: CommonArguments.buildNumberArg.defaultValue,
       )
       ..addOption(
         'target',

packages/shorebird_cli/lib/src/commands/patch/patcher.dart

@@ -6,6 +6,8 @@ import 'package:args/args.dart';
 import 'package:mason_logger/mason_logger.dart';
 import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
+import 'package:shorebird_cli/src/common_arguments.dart';
+import 'package:shorebird_cli/src/extensions/iterable.dart';
 import 'package:shorebird_cli/src/patch_diff_checker.dart';
 import 'package:shorebird_cli/src/platform/platform.dart';
 import 'package:shorebird_cli/src/release_type.dart';
@@ -139,8 +141,17 @@ https://docs.shorebird.dev/status#link-percentage-ios
       return [];
     }
 
-    // If the user already provided --build-name or --build-number, we don't
-    // want to override them.
+    // If the user provided --build-name or --build-number before the --, we
+    // don't want to override them.
+    if (argResults.options.containsAnyOf([
+      CommonArguments.buildNameArg.name,
+      CommonArguments.buildNumberArg.name,
+    ])) {
+      return [];
+    }
+
+    // If the user provided --build-name or --build-number after the --, we
+    // don't want to override them.
     if (argResults.rest.any(
       (a) => a.startsWith('--build-name') || a.startsWith('--build-number'),
     )) {

packages/shorebird_cli/lib/src/commands/release/release_command.dart

@@ -49,14 +49,14 @@ class ReleaseCommand extends ShorebirdCommand {
         help: 'The product flavor to use when building the app.',
       )
       ..addOption(
-        'build-number',
-        help: '''
-An identifier used as an internal version number.
-Each build must have a unique identifier to differentiate it from previous builds.
-It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build.
-On Android it is used as "versionCode".
-On Xcode builds it is used as "CFBundleVersion".''',
-        defaultsTo: '1.0',
+        CommonArguments.buildNameArg.name,
+        help: CommonArguments.buildNameArg.description,
+        defaultsTo: CommonArguments.buildNameArg.defaultValue,
+      )
+      ..addOption(
+        CommonArguments.buildNumberArg.name,
+        help: CommonArguments.buildNumberArg.description,
+        defaultsTo: CommonArguments.buildNumberArg.defaultValue,
       )
       ..addFlag(
         'codesign',

packages/shorebird_cli/lib/src/common_arguments.dart

@@ -8,18 +8,48 @@ class ArgumentDescriber {
   const ArgumentDescriber({
     required this.name,
     required this.description,
+    this.defaultValue,
   });
 
   /// Argument name as how the user writes it.
   final String name;
 
   /// Argument description that will be shown in the help of the command.
   final String description;
+
+  /// Default value for this argument. Only provide this if the default value
+  /// holds across all commands.
+  final String? defaultValue;
 }
 
 /// A class that houses the name of arguments that are shared between different
 /// commands and layers.
 class CommonArguments {
+  /// The Flutter --build-name argument.
+  static const buildNameArg = ArgumentDescriber(
+    name: 'build-name',
+    description: '''
+An identifier used as an internal version number.
+Each build must have a unique identifier to differentiate it from previous builds.
+It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build.
+On Android it is used as "versionCode".
+On Xcode builds it is used as "CFBundleVersion".''',
+  );
+
+  /// The Flutter --build-number argument.
+  static const buildNumberArg = ArgumentDescriber(
+    name: 'build-number',
+    description: '''
+A "x.y.z" string used as the version number shown to users.
+For each new version of your app, you will provide a version number to differentiate it
+from previous versions.
+On Android it is used as "versionName".
+On Xcode builds it is used as "CFBundleShortVersionString".
+On Windows it is used as the major, minor, and patch parts of the product and file
+versions.''',
+    defaultValue: '1.0',
+  );
+
   /// A multioption argument that defines constants for the built application.
   /// These are forwarded to Flutter.
   static const dartDefineArg = ArgumentDescriber(

packages/shorebird_cli/lib/src/extensions/arg_results.dart

@@ -105,6 +105,24 @@ extension ForwardedArgs on ArgResults {
   bool _isPositionalArgPlatform(String arg) =>
       ReleaseType.values.any((target) => target.cliName == arg);
 
+  /// All parsed args with the given name. Because of multioptions, there may
+  /// be multiple values for a single name, so we return a potentially empty
+  /// [Iterable<String>] instead of a [String?].
+  Iterable<String> _argsNamed(String name) {
+    if (!wasParsed(name)) {
+      return [];
+    }
+
+    final value = this[name];
+    if (value is List) {
+      return value.map((a) => '--$name=$a');
+    } else {
+      return ['--$name=$value'];
+    }
+  }
+
+  /// A list of arguments parsed by Shorebird commands that will be forwarded
+  /// to the underlying Flutter commands (that is, placed after `--`).
   List<String> get forwardedArgs {
     final List<String> forwarded;
     if (rest.isNotEmpty && _isPositionalArgPlatform(rest.first)) {
@@ -113,20 +131,14 @@ extension ForwardedArgs on ArgResults {
       forwarded = rest.toList();
     }
 
-    if (wasParsed(CommonArguments.dartDefineArg.name)) {
-      forwarded.addAll(
-        (this[CommonArguments.dartDefineArg.name] as List<String>).map(
-          (a) => '--${CommonArguments.dartDefineArg.name}=$a',
-        ),
-      );
-    }
-
-    if (wasParsed(CommonArguments.dartDefineFromFileArg.name)) {
-      forwarded.addAll(
-        (this[CommonArguments.dartDefineFromFileArg.name] as List<String>)
-            .map((a) => '--${CommonArguments.dartDefineFromFileArg.name}=$a'),
-      );
-    }
+    forwarded.addAll(
+      [
+        ..._argsNamed(CommonArguments.dartDefineArg.name),
+        ..._argsNamed(CommonArguments.dartDefineFromFileArg.name),
+        ..._argsNamed(CommonArguments.buildNameArg.name),
+        ..._argsNamed(CommonArguments.buildNumberArg.name),
+      ],
+    );
 
     return forwarded;
   }

packages/shorebird_cli/lib/src/extensions/iterable.dart

@@ -0,0 +1,5 @@
+/// Provides the [containsAnyOf] method to all [Iterable]s.
+extension ContainsAnyOf<T> on Iterable<T> {
+  /// Returns `true` if any of the elements in [elements] are in this iterable.
+  bool containsAnyOf(Iterable<T> elements) => elements.any(contains);
+}

packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart

@@ -136,6 +136,7 @@ void main() {
       shorebirdValidator = MockShorebirdValidator();
       shorebirdAndroidArtifacts = MockShorebirdAndroidArtifacts();
 
+      when(() => argResults.options).thenReturn([]);
       when(() => argResults.rest).thenReturn([]);
       when(() => argResults.wasParsed(any())).thenReturn(false);
 

packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart

@@ -129,7 +129,7 @@ void main() {
         shorebirdValidator = MockShorebirdValidator();
         xcodeBuild = MockXcodeBuild();
 
-        when(() => argResults['build-number']).thenReturn('1.0');
+        when(() => argResults.options).thenReturn([]);
         when(() => argResults.rest).thenReturn([]);
         when(() => argResults.wasParsed(any())).thenReturn(false);
 

packages/shorebird_cli/test/src/commands/patch/patcher_test.dart

@@ -4,6 +4,7 @@ import 'package:args/args.dart';
 import 'package:mocktail/mocktail.dart';
 import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
 import 'package:shorebird_cli/src/commands/commands.dart';
+import 'package:shorebird_cli/src/common_arguments.dart';
 import 'package:shorebird_cli/src/patch_diff_checker.dart';
 import 'package:shorebird_cli/src/platform/platform.dart';
 import 'package:shorebird_cli/src/release_type.dart';
@@ -45,6 +46,7 @@ void main() {
       late ArgResults argResults;
       setUp(() {
         argResults = MockArgResults();
+        when(() => argResults.options).thenReturn([]);
       });
 
       group('when releaseVersion is not specified', () {
@@ -108,6 +110,34 @@ void main() {
             );
           });
         });
+
+        group('when build-name and build-number were parsed as options', () {
+          setUp(() {
+            when(
+              () => argResults.wasParsed(CommonArguments.buildNameArg.name),
+            ).thenReturn(true);
+            when(
+              () => argResults.wasParsed(CommonArguments.buildNumberArg.name),
+            ).thenReturn(true);
+            when(() => argResults.options).thenReturn([
+              'release-version',
+              'build-name',
+              'build-number',
+              'platforms',
+            ]);
+          });
+
+          test('returns an empty list', () {
+            expect(
+              _TestPatcher(
+                argResults: argResults,
+                flavor: null,
+                target: null,
+              ).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'),
+              isEmpty,
+            );
+          });
+        });
       });
     });
   });

packages/shorebird_cli/test/src/extensions/arg_results_test.dart

@@ -98,6 +98,14 @@ void main() {
           CommonArguments.dartDefineFromFileArg.name,
           help: CommonArguments.dartDefineFromFileArg.description,
         )
+        ..addOption(
+          CommonArguments.buildNameArg.name,
+          help: CommonArguments.buildNameArg.description,
+        )
+        ..addOption(
+          CommonArguments.buildNumberArg.name,
+          help: CommonArguments.buildNumberArg.description,
+        )
         ..addMultiOption(
           'platforms',
           allowed: ReleaseType.values.map((e) => e.cliName),
@@ -178,5 +186,48 @@ void main() {
         );
       });
     });
+
+    group('when build-name and build-number are provided', () {
+      test('forwards build-name and build-number', () {
+        final args = [
+          '--verbose',
+          '--',
+          '--build-name=1.2.3',
+          '--build-number=4',
+        ];
+        final result = parser.parse(args);
+        expect(result.forwardedArgs, hasLength(2));
+        expect(
+          result.forwardedArgs,
+          containsAll(
+            [
+              '--build-name=1.2.3',
+              '--build-number=4',
+            ],
+          ),
+        );
+      });
+    });
+
+    group('when build-name and build-number are before the --', () {
+      test('forwards build-name and build-number', () {
+        final args = [
+          '--verbose',
+          '--build-name=1.2.3',
+          '--build-number=4',
+        ];
+        final result = parser.parse(args);
+        expect(result.forwardedArgs, hasLength(2));
+        expect(
+          result.forwardedArgs,
+          containsAll(
+            [
+              '--build-name=1.2.3',
+              '--build-number=4',
+            ],
+          ),
+        );
+      });
+    });
   });
 }

packages/shorebird_cli/test/src/extensions/iterable_test.dart

@@ -0,0 +1,18 @@
+import 'package:shorebird_cli/src/extensions/iterable.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('containsAnyOf', () {
+    test('returns true when any element is in the iterable', () {
+      final iterable = [1, 2, 3];
+      final elements = [3, 4, 5];
+      expect(iterable.containsAnyOf(elements), isTrue);
+    });
+
+    test('returns false when no element is in the iterable', () {
+      final iterable = [1, 2, 3];
+      final elements = [4, 5, 6];
+      expect(iterable.containsAnyOf(elements), isFalse);
+    });
+  });
+}