diff --git a/Lib/argparse.py b/Lib/argparse.py index d91707d9eec546..d83b167036a0d5 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -2199,6 +2199,17 @@ def take_action(action, argument_strings, option_string=None): if argument_values is not SUPPRESS: action(self, namespace, argument_values, option_string) + # returns the minimum number of arg strings an action requires; + # used by consume_optional() when computing positional reservations + def min_for_action(action): + nargs = action.nargs + if nargs is None or nargs in (ONE_OR_MORE, PARSER): + return 1 + elif isinstance(nargs, int): + return nargs + # OPTIONAL, ZERO_OR_MORE, REMAINDER contribute 0 + return 0 + # function to convert arg_strings into an optional action def consume_optional(start_index): @@ -2282,6 +2293,88 @@ def consume_optional(start_index): else: start = start_index + 1 selected_patterns = arg_strings_pattern[start:] + + # Reserve the minimum number of args required by the + # remaining positionals; replace excess 'A' tokens with 'O' + # so that match_argument cannot consume them. + # + # Note that the current option can only consume 'A' tokens + # up to the next 'O' (another option flag) in the pattern + # as the regex itself stops there. Only count tokens in + # that segment -- 'A' tokens in later segments belong to + # other options' territories and are handled when those + # options are processed. + first_o = selected_patterns.find('O') + if first_o == -1: + segment, rest = selected_patterns, '' + else: + segment = selected_patterns[:first_o] + rest = selected_patterns[first_o:] + segment_arg_count = segment.count('A') + + # Compute the minimum number of arg strings still required + # by the remaining positionals to avoid greedy consumption + min_pos = sum(min_for_action(pos) for pos in positionals) + if intermixed: + # In intermixed mode consume_positionals is never + # called during the main loop, so `positionals` never + # shrinks. Positional-typed tokens already collected in + # extras will satisfy the positional requirement in the + # second pass, so don't count them as still-unsatisfied + extras_arg_count = sum( + 1 for c in extras_pattern if c == 'A' + ) + min_pos = max(0, min_pos - extras_arg_count) + + if min_pos > 0 and segment_arg_count > 0: + # Compute the maximum positional args that future + # option segments can absorb, accounting for each + # future option's own minimum nargs requirement. + pattern_end = len(arg_strings_pattern) + later_option_string_indices = sorted( + i for i in option_string_indices + if i > start_index + ) + later_arg_capacity = 0 + for k, later_option_string_index in enumerate( + later_option_string_indices + ): + seg_start = later_option_string_index + 1 + seg_stop = ( + later_option_string_indices[k + 1] + if k + 1 < len(later_option_string_indices) + else pattern_end + ) + seg_arg_count = arg_strings_pattern[seg_start:seg_stop].count('A') + later_option_tuples = option_string_indices[later_option_string_index] + later_action = later_option_tuples[0][0] + later_explicit_arg = later_option_tuples[0][3] + if later_explicit_arg is not None or later_action is None: + # explicit arg (--opt=val) or unknown action: + # consumes no pattern 'A' tokens + later_min_arg_count = 0 + else: + later_min_arg_count = min_for_action(later_action) + later_arg_capacity += max( + 0, seg_arg_count - later_min_arg_count + ) + + reserved_arg_count = max(0, min_pos - later_arg_capacity) + max_arg_count = max( + min_for_action(action), + segment_arg_count - reserved_arg_count, + ) + if max_arg_count < segment_arg_count: + count = 0 + limited = [] + for c in segment: + if c == 'A': + count += 1 + if count > max_arg_count: + c = 'O' + limited.append(c) + selected_patterns = ''.join(limited) + rest + arg_count = match_argument(action, selected_patterns) stop = start + arg_count args = arg_strings[start:stop] diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index e0c32976fd6f0d..e94118117c548f 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -664,6 +664,17 @@ class TestOptionalsNargs3(ParserTestCase): ('-x a b c', NS(x=['a', 'b', 'c'])), ] + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + # Fixed integer nargs is never greedy: --foo takes exactly N args and + # leaves the remainder for positionals. + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs=2) + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'a', 'b', 'c']) + self.assertEqual(args.foo, ['a', 'b']) + self.assertEqual(args.bar, 'c') + class TestOptionalsNargsOptional(ParserTestCase): """Tests specifying an Optional arg for an Optional""" @@ -687,6 +698,48 @@ class TestOptionalsNargsOptional(ParserTestCase): ('-z 2', NS(w=None, x=None, y='spam', z=2)), ] + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'abc']) + self.assertIsNone(args.foo) + self.assertEqual(args.bar, 'abc') + + def test_does_not_steal_two_required_positionals(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + parser.add_argument('bax') + args = parser.parse_args(['--foo', 'a', 'b']) + self.assertIsNone(args.foo) + self.assertEqual(args.bar, 'a') + self.assertEqual(args.bax, 'b') + + def test_does_not_steal_two_required_positional_interspersed(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('--bar', nargs='?') + parser.add_argument('baz') + parser.add_argument('bax') + args = parser.parse_args(['--foo', 'a', '--bar', 'b']) + self.assertEqual(args.baz, 'a') + self.assertEqual(args.bax, 'b') + + def test_greedy_when_positional_also_optional(self): + # https://github.com/python/cpython/issues/53584 + # When the following positional is also optional, --foo taking the arg + # is acceptable (both are optional, no unambiguous correct answer). + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar', nargs='?') + args = parser.parse_args(['--foo', 'abc']) + self.assertEqual(args.foo, 'abc') + self.assertIsNone(args.bar) + class TestOptionalsNargsZeroOrMore(ParserTestCase): """Tests specifying args for an Optional that accepts zero or more""" @@ -706,6 +759,15 @@ class TestOptionalsNargsZeroOrMore(ParserTestCase): ('-y a b', NS(x=None, y=['a', 'b'])), ] + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='*') + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'abc']) + self.assertEqual(args.foo, []) + self.assertEqual(args.bar, 'abc') + class TestOptionalsNargsOneOrMore(ParserTestCase): """Tests specifying args for an Optional that accepts one or more""" @@ -723,6 +785,27 @@ class TestOptionalsNargsOneOrMore(ParserTestCase): ('-y a b', NS(x=None, y=['a', 'b'])), ] + def test_does_not_steal_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='+') + parser.add_argument('bar') + args = parser.parse_args(['--foo', 'a', 'b']) + self.assertEqual(args.foo, ['a']) + self.assertEqual(args.bar, 'b') + + def test_does_not_steal_interspersed_required_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='+') + parser.add_argument('--bar', nargs='+') + parser.add_argument('baz') + parser.add_argument('bax') + args = parser.parse_args(['--foo', 'a', 'c', '--bar', 'b', 'd']) + self.assertEqual(args.foo, ['a']) + self.assertEqual(args.bar, ['b']) + self.assertEqual(args.baz, 'c') + self.assertEqual(args.bax, 'd') class TestOptionalsChoices(ParserTestCase): """Tests specifying the choices for an Optional""" @@ -6760,6 +6843,24 @@ def test_invalid_args(self): parser = ErrorRaisingArgumentParser(prog='PROG') self.assertRaises(ArgumentParserError, parser.parse_intermixed_args, ['a']) + def test_variable_nargs_option_before_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + args = parser.parse_intermixed_args(['--foo', 'abc']) + self.assertIsNone(args.foo) + self.assertEqual(args.bar, 'abc') + + def test_variable_nargs_option_after_positional(self): + # https://github.com/python/cpython/issues/53584 + parser = ErrorRaisingArgumentParser() + parser.add_argument('--foo', nargs='?') + parser.add_argument('bar') + args = parser.parse_intermixed_args(['abc', '--foo', 'xyz']) + self.assertEqual(args.foo, 'xyz') + self.assertEqual(args.bar, 'abc') + class TestIntermixedMessageContentError(TestCase): # case where Intermixed gives different error message diff --git a/Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst b/Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst new file mode 100644 index 00000000000000..b322b6abbd7428 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-03-27-12-54-44.gh-issue-53584.4mA7Wp.rst @@ -0,0 +1,3 @@ +Fix :mod:`argparse` options with variable :ref:`nargs ` (``'?'``, +``'*'``, ``'+'``) greedily consuming argument strings that should be +reserved for required positional arguments.