8000 gh-126068: Fix exceptions in the argparse module (GH-126069) · python/cpython@cc9a183 · GitHub
[go: up one dir, main page]
Skip to content

Commit cc9a183

Browse files
serhiy-storchakaserhiy-storchaka
authored
gh-126068: Fix exceptions in the argparse module (GH-126069)
* 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. * TypeError is now raised instead of ValueError for some logical errors.
1 parent 1f16df4 commit cc9a183

File tree

4 files changed

+72
-58
lines changed

4 files changed

+72
-58
lines changed

‎Lib/argparse.py

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ def format_usage(self):
846846
return self.option_strings[0]
847847

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

851851

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

11741174
if name in self._name_parser_map:
1175-
raise ArgumentError(self, _('conflicting subparser: %s') % name)
1175+
raise ValueError(f'conflicting subparser: {name}')
11761176
for alias in aliases:
11771177
if alias in self._name_parser_map:
1178-
raise ArgumentError(
1179-
self, _('conflicting subparser alias: %s') % alias)
1178+
raise ValueError(f'conflicting subparser alias: {alias}')
11801179

11811180
# create a pseudo-action to hold the choice help
11821181
if 'help' in kwargs:
@@ -1430,8 +1429,8 @@ def add_argument(self, *args, **kwargs):
14301429
chars = self.prefix_chars
14311430
if not args or len(args) == 1 and args[0][0] not in chars:
14321431
if args and 'dest' in kwargs:
1433-
raise ValueError('dest supplied twice for positional argument,'
1434-
' did you mean metavar?')
1432+
raise TypeError('dest supplied twice for positional argument,'
1433+
' did you mean metavar?')
14351434
kwargs = self._get_positional_kwargs(*args, **kwargs)
14361435

14371436
# otherwise, we're adding an optional argument
@@ -1450,7 +1449,7 @@ def add_argument(self, *args, **kwargs):
14501449
action_name = kwargs.get('action')
14511450
action_class = self._pop_action_class(kwargs)
14521451
if not callable(action_class):
1453-
raise ValueError('unknown action "%s"' % (action_class,))
1452+
raise ValueError('unknown action {action_class!r}')
14541453
action = action_class(**kwargs)
14551454

14561455
# raise an error if action for positional argument does not
@@ -1461,11 +1460,11 @@ def add_argument(self, *args, **kwargs):
14611460
# raise an error if the action type is not callable
14621461
type_func = self._registry_get('type', action.type, action.type)
14631462
if not callable(type_func):
1464-
raise ValueError('%r is not callable' % (type_func,))
1463+
raise TypeError(f'{type_func!r} is not callable')
14651464

14661465
if type_func is FileType:
1467-
raise ValueError('%r is a FileType class object, instance of it'
1468-
' must be passed' % (type_func,))
1466+
raise TypeError(f'{type_func!r} is a FileType class object, '
1467+
f'instance of it must be passed')
14691468

14701469
# raise an error if the metavar does not match the type
14711470
if hasattr(self, "_get_formatter"):
@@ -1518,8 +1517,8 @@ def _add_container_actions(self, container):
15181517
if group.title in title_group_map:
15191518
# This branch could happen if a derived class added
15201519
# groups with duplicated titles in __init__
1521-
msg = _('cannot merge actions - two groups are named %r')
1522-
raise ValueError(msg % (group.title))
1520+
msg = f'cannot merge actions - two groups are named {group.title!r}'
1521+
raise ValueError(msg)
15231522
title_group_map[group.title] = group
15241523

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

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

15901587
# strings starting with two prefix characters are long options
15911588
option_strings.append(option_string)
@@ -1601,8 +1598,8 @@ def _get_optional_kwargs(self, *args, **kwargs):
16011598
dest_option_string = option_strings[0]
16021599
dest = dest_option_string.lstrip(self.prefix_chars)
16031600
if not dest:
1604-
msg = _('dest= is required for options like %r')
1605-
raise ValueError(msg % option_string)
1601+
msg = f'dest= is required for options like {option_string!r}'
1602+
raise TypeError(msg)
16061603
dest = dest.replace('-', '_')
16071604

16081605
# return the updated keyword arguments
@@ -1618,8 +1615,8 @@ def _get_handler(self):
16181615
try:
16191616
return getattr(self, handler_func_name)
16201617
except AttributeError:
1621-
msg = _('invalid conflict_resolution value: %r')
1622-
raise ValueError(msg % self.conflict_handler)
1618+
msg = f'invalid conflict_resolution value: {self.conflict_handler!r}'
1619+
raise ValueError(msg)
16231620

16241621
def _check_conflict(self, action):
16251622

@@ -1727,7 +1724,7 @@ def __init__(self, container, required=False):
17271724

17281725
def _add_action(self, action):
17291726
if action.required:
1730-
msg = _('mutually exclusive arguments must be optional')
1727+
msg = 'mutually exclusive arguments must be optional'
17311728
raise ValueError(msg)
17321729
action = self._container._add_action(action)
17331730
self._group_actions.append(action)
@@ -1871,7 +1868,7 @@ def _get_kwargs(self):
18711868
# ==================================
18721869
def add_subparsers(self, **kwargs):
18731870
if self._subparsers is not None:
1874-
raise ArgumentError(None, _('cannot have multiple subparser arguments'))
1871+
raise ValueError('cannot have multiple subparser arguments')
18751872

18761873
# add the parser class to the arguments if it's not present
18771874
kwargs.setdefault('parser_class', type(self))
@@ -2565,8 +2562,7 @@ def _get_values(self, action, arg_strings):
25652562
def _get_value(self, action, arg_string):
25662563
type_func = self._registry_get('type', action.type, action.type)
25672564
if not callable(type_func):
2568-
msg = _('%r is not callable')
2569-
raise ArgumentError(action, msg % type_func)
2565+
raise TypeError(f'{type_func!r} is not callable')
25702566

25712567
# convert the value to the appropriate type
25722568
try:

‎Lib/test/test_argparse.py

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,7 +2055,7 @@ class TestFileTypeMissingInitialization(TestCase):
20552055

20562056
def test(self):
20572057
parser = argparse.ArgumentParser()
2058-
with self.assertRaises(ValueError) as cm:
2058+
with self.assertRaises(TypeError) as cm:
20592059
parser.add_argument('-x', type=argparse.FileType)
20602060

20612061
self.assertEqual(
@@ -2377,11 +2377,24 @@ def test_invalid_type(self):
23772377
self.assertRaises(NotImplementedError, parser.parse_args, ['--foo', 'bar'])
23782378

23792379
def test_modified_invalid_action(self):
2380-
parser = ErrorRaisingArgumentParser()
2380+
parser = argparse.ArgumentParser(exit_on_error=False)
23812381
action = parser.add_argument('--foo')
23822382
# Someone got crazy and did this
23832383
action.type = 1
2384-
self.assertRaises(ArgumentParserError, parser.parse_args, ['--foo', 'bar'])
2384+
self.assertRaisesRegex(TypeError, '1 is not callable',
2385+
parser.parse_args, ['--foo', 'bar'])
2386+
action.type = ()
2387+
self.assertRaisesRegex(TypeError, r'\(\) is not callable',
2388+
parser.parse_args, ['--foo', 'bar'])
2389+
# It is impossible to distinguish a TypeError raised due to a mismatch
2390+
# of the required function arguments from a TypeError raised for an incorrect
2391+
# argument value, and using the heavy inspection machinery is not worthwhile
2392+
# as it does not reliably work in all cases.
2393+
# Therefore, a generic ArgumentError is raised to handle this logical error.
2394+
action.type = pow
2395+
self.assertRaisesRegex(argparse.ArgumentError,
2396+
"argument --foo: invalid pow value: 'bar'",
2397+
parser.parse_args, ['--foo', 'bar'])
23852398

23862399

23872400
# ================
@@ -2418,7 +2431,7 @@ def _get_parser(self, subparser_help=False, prefix_chars=None,
24182431
else:
24192432
subparsers_kwargs['help'] = 'command help'
24202433
subparsers = parser.add_subparsers(**subparsers_kwargs)
2421-
self.assertRaisesRegex(argparse.ArgumentError,
2434+
self.assertRaisesRegex(ValueError,
24222435
'cannot have multiple subparser arguments',
24232436
parser.add_subparsers)
24242437

@@ -5534,20 +5547,27 @@ def test_missing_destination(self):
55345547
self.assertTypeError(action=action)
55355548

55365549
def test_invalid_option_strings(self):
5537-
self.assertValueError('--')
5538-
self.assertValueError('---')
5550+
self.assertTypeError('-', errmsg='dest= is required')
5551+
self.assertTypeError('--', errmsg='dest= is required')
5552+
self.assertTypeError('---', errmsg='dest= is required')
55395553

55405554
def test_invalid_prefix(self):
5541-
self.assertValueError('--foo', '+foo')
5555+
self.assertValueError('--foo', '+foo',
5556+
errmsg='must start with a character')
55425557

55435558
def test_invalid_type(self):
5544-
self.assertValueError('--foo', type='int')
5545-
self.assertValueError('--foo', type=(int, float))
5559+
self.assertTypeError('--foo', type='int',
5560+
errmsg="'int' is not callable")
5561+
self.assertTypeError('--foo', type=(int, float),
5562+
errmsg='is not callable')
55465563

55475564
def test_invalid_action(self):
5548-
self.assertValueError('-x', action='foo')
5549-
self.assertValueError('foo', action='baz')
5550-
self.assertValueError('--foo', action=('store', 'append'))
5565+
self.assertValueError('-x', action='foo',
5566+
errmsg='unknown action')
5567+
self.assertValueError('foo', action='baz',
5568+
errmsg='unknown action')
5569+
self.assertValueError('--foo', action=('store', 'append'),
5570+
errmsg='unknown action')
55515571
self.assertValueError('--foo', action="store-true",
55525572
errmsg='unknown action')
55535573

@@ -5562,7 +5582,7 @@ def test_invalid_help(self):
55625582
def test_multiple_dest(self):
55635583
parser = argparse.ArgumentParser()
55645584
parser.add_argument(dest='foo')
5565-
with self.assertRaises(ValueError) as cm:
5585+
with self.assertRaises(TypeError) as cm:
55665586
parser.add_argument('bar', dest='baz')
55675587
self.assertIn('dest supplied twice for positional argument,'
55685588
' did you mean metavar?',
@@ -5733,14 +5753,18 @@ def test_subparser_conflict(self):
57335753
parser = argparse.ArgumentParser()
57345754
sp = parser.add_subparsers()
57355755
sp.add_parser('fullname', aliases=['alias'])
5736-
self.assertRaises(argparse.ArgumentError,
5737-
sp.add_parser, 'fullname')
5738-
self.assertRaises(argparse.ArgumentError,
5739-
sp.add_parser, 'alias')
5740-
self.assertRaises(argparse.ArgumentError,
5741-
sp.add_parser, 'other', aliases=['fullname'])
5742-
self.assertRaises(argparse.ArgumentError,
5743-
sp.add_parser, 'other', aliases=['alias'])
5756+
self.assertRaisesRegex(ValueError,
5757+
'conflicting subparser: fullname',
5758+
sp.add_parser, 'fullname')
5759+
self.assertRaisesRegex(ValueError,
5760+
'conflicting subparser: alias',
5761+
sp.add_parser, 'alias')
5762+
self.assertRaisesRegex(ValueError,
5763+
'conflicting subparser alias: fullname',
5764+
sp.add_parser, 'other', aliases=['fullname'])
5765+
self.assertRaisesRegex(ValueError,
5766+
'conflicting subparser alias: alias',
5767+
sp.add_parser, 'other', aliases=['alias'])
57445768

57455769

57465770
# =============================

‎Lib/test/translationdata/argparse/msgids.txt

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,19 @@
22
%(heading)s:
33
%(prog)s: error: %(message)s\n
44
%(prog)s: warning: %(message)s\n
5-
%r is not callable
6-
'required' is an invalid argument for positionals
7-
.__call__() not defined
85
ambiguous option: %(option)s could match %(matches)s
96
argument "-" with mode %r
107
argument %(argument_name)s: %(message)s
118
argument '%(argument_name)s' is deprecated
129
can't open '%(filename)s': %(error)s
13-
cannot have multiple subparser arguments
14-
cannot merge actions - two groups are named %r
1510
command '%(parser_name)s' is deprecated
16-
conflicting subparser alias: %s
17-
conflicting subparser: %s
18-
dest= is required for options like %r
1911
expected at least one argument
2012
expected at most one argument
2113
expected one argument
2214
ignored explicit argument %r
2315
invalid %(type)s value: %(value)r
2416
invalid choice: %(value)r (choose from %(choices)s)
2517
invalid choice: %(value)r, maybe you meant %(closest)r? (choose from %(choices)s)
26-
invalid conflict_resolution value: %r
27-
invalid option string %(option)r: must start with a character %(prefix_chars)r
28-
mutually exclusive arguments must be optional
2918
not allowed with argument %s
3019
one of the arguments %s is required
3120
option '%(option)s' is deprecated

‎Misc/NEWS.d/next/Library/2024-10-28-11-33-59.gh-issue-126068.Pdznm_.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix exceptions in the :mod:`argparse` module so that only error messages for
2+
ArgumentError and ArgumentTypeError are now translated.
3+
ArgumentError is now only used for command line errors, not for logical
4+
errors in the program. TypeError is now raised instead of ValueError for
5+
some logical errors.

0 commit comments

Comments
 (0)
0