Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Expand Down Expand Up @@ -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]
Expand Down
101 changes: 101 additions & 0 deletions Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand All @@ -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"""
Expand All @@ -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"""
Expand All @@ -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"""
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :mod:`argparse` options with variable :ref:`nargs <nargs>` (``'?'``,
``'*'``, ``'+'``) greedily consuming argument strings that should be
reserved for required positional arguments.
Loading