Skip to content

Commit

Permalink
pythongh-58573: Fix conflicts between abbreviated long options in the…
Browse files Browse the repository at this point in the history
… parent parser and subparsers in argparse

Check for ambiguous options if the option is consumed, not when it is
parsed.
  • Loading branch information
serhiy-storchaka committed Sep 26, 2024
1 parent 08a467b commit 333c23e
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 34 deletions.
62 changes: 28 additions & 34 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -1927,11 +1927,11 @@ def _parse_known_args(self, arg_strings, namespace):
# otherwise, add the arg to the arg strings
# and note the index if it was an option
else:
option_tuple = self._parse_optional(arg_string)
if option_tuple is None:
option_tuples = self._parse_optional(arg_string)
if option_tuples is None:
pattern = 'A'
else:
option_string_indices[i] = option_tuple
option_string_indices[i] = option_tuples
pattern = 'O'
arg_string_pattern_parts.append(pattern)

Expand Down Expand Up @@ -1966,8 +1966,16 @@ def take_action(action, argument_strings, option_string=None):
def consume_optional(start_index):

# get the optional identified at this index
option_tuple = option_string_indices[start_index]
action, option_string, sep, explicit_arg = option_tuple
option_tuples = option_string_indices[start_index]
# if multiple actions match, the option string was ambiguous
if len(option_tuples) > 1:
options = ', '.join([option_string
for action, option_string, sep, explicit_arg in option_tuples])
args = {'option': arg_string, 'matches': options}
msg = _('ambiguous option: %(option)s could match %(matches)s')
raise ArgumentError(None, msg % args)

action, option_string, sep, explicit_arg = option_tuples[0]

# identify additional optionals in the same arg string
# (e.g. -xyz is the same as -x -y -z if no args are required)
Expand Down Expand Up @@ -2253,7 +2261,7 @@ def _parse_optional(self, arg_string):
# if the option string is present in the parser, return the action
if arg_string in self._option_string_actions:
action = self._option_string_actions[arg_string]
return action, arg_string, None, None
return [(action, arg_string, None, None)]

# if it's just a single character, it was meant to be positional
if len(arg_string) == 1:
Expand All @@ -2263,25 +2271,14 @@ def _parse_optional(self, arg_string):
option_string, sep, explicit_arg = arg_string.partition('=')
if sep and option_string in self._option_string_actions:
action = self._option_string_actions[option_string]
return action, option_string, sep, explicit_arg
return [(action, option_string, sep, explicit_arg)]

# search through all possible prefixes of the option string
# and all actions in the parser for possible interpretations
option_tuples = self._get_option_tuples(arg_string)

# if multiple actions match, the option string was ambiguous
if len(option_tuples) > 1:
options = ', '.join([option_string
for action, option_string, sep, explicit_arg in option_tuples])
args = {'option': arg_string, 'matches': options}
msg = _('ambiguous option: %(option)s could match %(matches)s')
raise ArgumentError(None, msg % args)

# if exactly one action matched, this segmentation is good,
# so return the parsed action
elif len(option_tuples) == 1:
option_tuple, = option_tuples
return option_tuple
if option_tuples:
return option_tuples

# if it was not found as an option, but it looks like a negative
# number, it was meant to be positional
Expand All @@ -2296,7 +2293,7 @@ def _parse_optional(self, arg_string):

# it was meant to be an optional but there is no such option
# in this parser (though it might be a valid option in a subparser)
return None, arg_string, None, None
return [(None, arg_string, None, None)]

def _get_option_tuples(self, option_string):
result = []
Expand Down Expand Up @@ -2344,43 +2341,40 @@ def _get_nargs_pattern(self, action):
# in all examples below, we have to allow for '--' args
# which are represented as '-' in the pattern
nargs = action.nargs
# if this is an optional action, -- is not allowed
option = action.option_strings

# the default (None) is assumed to be a single argument
if nargs is None:
nargs_pattern = '(-*A-*)'
nargs_pattern = '([A])' if option else '(-*A-*)'

# allow zero or one arguments
elif nargs == OPTIONAL:
nargs_pattern = '(-*A?-*)'
nargs_pattern = '(A?)' if option else '(-*A?-*)'

# allow zero or more arguments
elif nargs == ZERO_OR_MORE:
nargs_pattern = '(-*[A-]*)'
nargs_pattern = '(A*)' if option else '(-*[A-]*)'

# allow one or more arguments
elif nargs == ONE_OR_MORE:
nargs_pattern = '(-*A[A-]*)'
nargs_pattern = '(A+)' if option else '(-*A[A-]*)'

# allow any number of options or arguments
elif nargs == REMAINDER:
nargs_pattern = '([-AO]*)'
nargs_pattern = '([AO]*)' if option else '(.*)'

# allow one argument followed by any number of options or arguments
elif nargs == PARSER:
nargs_pattern = '(-*A[-AO]*)'
nargs_pattern = '(A[AO]*)' if option else '(-*A[-AO]*)'

# suppress action, like nargs=0
elif nargs == SUPPRESS:
nargs_pattern = '(-*-*)'
nargs_pattern = '()' if option else '(-*)'

# all others should be integers
else:
nargs_pattern = '(-*%s-*)' % '-*'.join('A' * nargs)

# if this is an optional action, -- is not allowed
if action.option_strings:
nargs_pattern = nargs_pattern.replace('-*', '')
nargs_pattern = nargs_pattern.replace('-', '')
nargs_pattern = '([AO]{%d})' % nargs if option else '((?:-*A){%d}-*)' % nargs

# return the pattern
return nargs_pattern
Expand Down
22 changes: 22 additions & 0 deletions Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -2312,6 +2312,28 @@ def test_parse_known_args(self):
(NS(foo=False, bar=0.5, w=7, x='b'), ['-W', '-X', 'Y', 'Z']),
)

def test_abbreviation(self):
parser = ErrorRaisingArgumentParser()
parser.add_argument('--foodle')
parser.add_argument('--foonly')
subparsers = parser.add_subparsers()
parser1 = subparsers.add_parser('bar')
parser1.add_argument('--fo')
parser1.add_argument('--foonew')

self.assertEqual(parser.parse_args(['--food', 'baz', 'bar']),
NS(foodle='baz', foonly=None, fo=None, foonew=None))
self.assertEqual(parser.parse_args(['--foon', 'baz', 'bar']),
NS(foodle=None, foonly='baz', fo=None, foonew=None))
self.assertArgumentParserError(parser.parse_args, ['--fo', 'baz', 'bar'])
self.assertEqual(parser.parse_args(['bar', '--fo', 'baz']),
NS(foodle=None, foonly=None, fo='baz', foonew=None))
self.assertEqual(parser.parse_args(['bar', '--foo', 'baz']),
NS(foodle=None, foonly=None, fo=None, foonew='baz'))
self.assertEqual(parser.parse_args(['bar', '--foon', 'baz']),
NS(foodle=None, foonly=None, fo=None, foonew='baz'))
self.assertArgumentParserError(parser.parse_args, ['bar', '--food', 'baz'])

def test_parse_known_args_with_single_dash_option(self):
parser = ErrorRaisingArgumentParser()
parser.add_argument('-k', '--known', action='count', default=0)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix conflicts between abbreviated long options in the parent parser and
subparsers in :mod:`argparse`.

0 comments on commit 333c23e

Please sign in to comment.