8000 gh-126068: Fix exceptions in the argparse module by serhiy-storchaka · Pull Request #126069 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-126068: Fix exceptions in the argparse module #126069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 30, 2024
Merged

gh-126068: Fix exceptions in the argparse module #126069

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42b91d3
gh-126068: Fix exceptions in the argparse module
serhiy-storchaka Oct 28, 2024
c3a688b
Update Lib/argparse.py
serhiy-storchaka Oct 29, 2024
41d67ce
Apply suggestions from code review
serhiy-storchaka Oct 29, 2024
27964f7
Revert NEWS changes.
serhiy-storchaka Oct 30, 2024
fccf382
Change some ValueError to TypeError.
serhiy-storchaka Oct 30, 2024
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
35 changes: 15 additions & 20 deletions Lib/argparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ def format_usage(self):
return self.option_strings[0]

def __call__(self, parser, namespace, values, option_string=None):
raise NotImplementedError(_('.__call__() not defined'))
raise NotImplementedError('.__call__() not defined')


class BooleanOptionalAction(Action):
Expand Down Expand Up @@ -1172,11 +1172,10 @@ def add_parser(self, name, *, deprecated=False, **kwargs):
aliases = kwargs.pop('aliases', ())

if name in self._name_parser_map:
raise ArgumentError(self, _('conflicting subparser: %s') % name)
raise ValueError(f'conflicting subparser: {name}')
for alias in aliases:
if alias in self._name_parser_map:
raise ArgumentError(
self, _('conflicting subparser alias: %s') % alias)
raise ValueError(f'conflicting subparser alias: {alias}')

# create a pseudo-action to hold the choice help
if 'help' in kwargs:
Expand Down Expand Up @@ -1518,8 +1517,8 @@ def _add_container_actions(self, container):
if group.title in title_group_map:
# This branch could happen if a derived class added
# groups with duplicated titles in __init__
msg = _('cannot merge actions - two groups are named %r')
raise ValueError(msg % (group.title))
msg = f'cannot merge actions - two groups are named {group.title!r}'
raise ValueError(msg)
title_group_map[group.title] = group

# map each action to its group
Expand Down Expand Up @@ -1560,7 +1559,7 @@ def _add_container_actions(self, container):
def _get_positional_kwargs(self, dest, **kwargs):
# make sure required is not specified
if 'required' in kwargs:
msg = _("'required' is an invalid argument for positionals")
msg = "'required' is an invalid argument for positionals"
raise TypeError(msg)

# mark positional arguments as required if at least one is
Expand All @@ -1581,11 +1580,8 @@ def _get_optional_kwargs(self, *args, **kwargs):
for option_string in args:
# error on strings that don't start with an appropriate prefix
if not option_string[0] in self.prefix_chars:
args = {'option': option_string,
'prefix_chars': self.prefix_chars}
msg = _('invalid option string %(option)r: '
'must start with a character %(prefix_chars)r')
raise ValueError(msg % args)
raise ValueError(f'invalid option string {option_string!r}: '
f'must start with a character {self.prefix_chars!r}')

# strings starting with two prefix characters are long options
option_strings.append(option_string)
Expand All @@ -1601,8 +1597,8 @@ def _get_optional_kwargs(self, *args, **kwargs):
dest_option_string = option_strings[0]
dest = dest_option_string.lstrip(self.prefix_chars)
if not dest:
msg = _('dest= is required for options like %r')
raise ValueError(msg % option_string)
msg = f'dest= is required for options like {option_string!r}'
raise ValueError(msg)
dest = dest.replace('-', '_')

# return the updated keyword arguments
Expand All @@ -1618,8 +1614,8 @@ def _get_handler(self):
try:
return getattr(self, handler_func_name)
except AttributeError:
msg = _('invalid conflict_resolution value: %r')
raise ValueError(msg % self.conflict_handler)
msg = f'invalid conflict_resolution value: {self.conflict_handler!r}'
raise ValueError(msg)

def _check_conflict(self, action):

Expand Down Expand Up @@ -1727,7 +1723,7 @@ def __init__(self, container, required=False):

def _add_action(self, action):
if action.required:
msg = _('mutually exclusive arguments must be optional')
msg = 'mutually exclusive arguments must be optional'
raise ValueError(msg)
action = self._container._add_action(action)
self._group_actions.append(action)
Expand Down Expand Up @@ -1871,7 +1867,7 @@ def _get_kwargs(self):
# ==================================
def add_subparsers(self, **kwargs):
if self._subparsers is not None:
raise ArgumentError(None, _('cannot have multiple subparser arguments'))
raise ValueError('cannot have multiple subparser arguments')

# add the parser class to the arguments if it's not present
kwargs.setdefault('parser_class', type(self))
Expand Down Expand Up @@ -2565,8 +2561,7 @@ def _get_values(self, action, arg_strings):
def _get_value(self, action, arg_string):
type_func = self._registry_get('type', action.type, action.type)
if not callable(type_func):
msg = _('%r is not callable')
raise ArgumentError(action, msg % type_func)
raise RuntimeError('%r is not callable' % (type_func,))

# convert the value to the appropriate type
try:
Expand Down
39 changes: 28 additions & 11 deletions Lib/test/test_argparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2377,11 +2377,24 @@ def test_invalid_type(self):
self.assertRaises(NotImplementedError, parser.parse_args, ['--foo', 'bar'])

def test_modified_invalid_action(self):
parser = ErrorRaisingArgumentParser()
parser = argparse.ArgumentParser(exit_on_error=False)
action = parser.add_argument('--foo')
# Someone got crazy and did this
action.type = 1
self.assertRaises(ArgumentParserError, parser.parse_args, ['--foo', 'bar'])
self.assertRaisesRegex(RuntimeError, '1 is not callable',
parser.parse_args, ['--foo', 'bar'])
action.type = ()
self.assertRaisesRegex(RuntimeError, r'\(\) is not callable',
parser.parse_args, ['--foo', 'bar'])
# It is impossible to distinguish a TypeError raised due to mismatch
# of the required function arguments from a TypeError raised for wrong
# argument value, and it is not worth to use heavy inspect machinery
# which does not work in all cases anyway.
# So we get a dumb ArgumentError for such logical error.
action.type = pow
self.assertRaisesRegex(argparse.ArgumentError,
"argument --foo: invalid pow value: 'bar'",
parser.parse_args, ['--foo', 'bar'])


# ================
Expand Down Expand Up @@ -2418,7 +2431,7 @@ def _get_parser(self, subparser_help=False, prefix_chars=None,
else:
subparsers_kwargs['help'] = 'command help'
subparsers = parser.add_subparsers(**subparsers_kwargs)
self.assertRaisesRegex(argparse.ArgumentError,
self.assertRaisesRegex(ValueError,
'cannot have multiple subparser arguments',
parser.add_subparsers)

Expand Down Expand Up @@ -5733,14 +5746,18 @@ def test_subparser_conflict(self):
parser = argparse.ArgumentParser()
sp = parser.add_subparsers()
sp.add_parser('fullname', aliases=['alias'])
self.assertRaises(argparse.ArgumentError,
sp.add_parser, 'fullname')
self.assertRaises(argparse.ArgumentError,
sp.add_parser, 'alias')
self.assertRaises(argparse.ArgumentError,
sp.add_parser, 'other', aliases=['fullname'])
self.assertRaises(argparse.ArgumentError,
sp.add_parser, 'other', aliases=['alias'])
self.assertRaisesRegex(ValueError,
'conflicting subparser: fullname',
sp.add_parser, 'fullname')
self.assertRaisesRegex(ValueError,
'conflicting subparser: alias',
sp.add_parser, 'alias')
self.assertRaisesRegex(ValueError,
'conflicting subparser alias: fullname',
sp.add_parser, 'other', aliases=['fullname'])
self.assertRaisesRegex(ValueError,
'conflicting subparser alias: alias',
sp.add_parser, 'other', aliases=['alias'])


# =============================
Expand Down
11 changes: 0 additions & 11 deletions Lib/test/translationdata/argparse/msgids.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,19 @@
%(heading)s:
%(prog)s: error: %(message)s\n
%(prog)s: warning: %(message)s\n
%r is not callable
'required' is an invalid argument for positionals
.__call__() not defined
ambiguous option: %(option)s could match %(matches)s
argument "-" with mode %r
argument %(argument_name)s: %(message)s
argument '%(argument_name)s' is deprecated
can't open '%(filename)s': %(error)s
cannot have multiple subparser arguments
cannot merge actions - two groups are named %r
command '%(parser_name)s' is deprecated
conflicting subparser alias: %s
conflicting subparser: %s
dest= is required for options like %r
expected at least one argument
expected at most one argument
expected one argument
ignored explicit argument %r
invalid %(type)s value: %(value)r
invalid choice: %(value)r (choose from %(choices)s)
invalid choice: %(value)r, maybe you meant %(closest)r? (choose from %(choices)s)
invalid conflict_resolution value: %r
invalid option string %(option)r: must start with a character %(prefix_chars)r
mutually exclusive arguments must be optional
not allowed with argument %s
one of the arguments %s is required
option '%(option)s' is deprecated
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2024-10-28-11-33-59.gh-issue-126068.Pdznm_.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix exceptions in the :mod:`argparse` module. Only error messages for
ArgumentError and ArgumentTypeError are now translated. ArgumentError is now
only used for command line errors, not for logical errors in the program.
Loading
0