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.

Author: serhiy-storchaka

Lib/argparse.py

@@ -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):
@@ -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:
@@ -1430,8 +1429,8 @@ def add_argument(self, *args, **kwargs):
         chars = self.prefix_chars
         if not args or len(args) == 1 and args[0][0] not in chars:
             if args and 'dest' in kwargs:
-                raise ValueError('dest supplied twice for positional argument,'
-                                 ' did you mean metavar?')
+                raise TypeError('dest supplied twice for positional argument,'
+                                ' did you mean metavar?')
             kwargs = self._get_positional_kwargs(*args, **kwargs)
 
         # otherwise, we're adding an optional argument
@@ -1450,7 +1449,7 @@ def add_argument(self, *args, **kwargs):
         action_name = kwargs.get('action')
         action_class = self._pop_action_class(kwargs)
         if not callable(action_class):
-            raise ValueError('unknown action "%s"' % (action_class,))
+            raise ValueError('unknown action {action_class!r}')
         action = action_class(**kwargs)
 
         # raise an error if action for positional argument does not
@@ -1461,11 +1460,11 @@ def add_argument(self, *args, **kwargs):
         # raise an error if the action type is not callable
         type_func = self._registry_get('type', action.type, action.type)
         if not callable(type_func):
-            raise ValueError('%r is not callable' % (type_func,))
+            raise TypeError(f'{type_func!r} is not callable')
 
         if type_func is FileType:
-            raise ValueError('%r is a FileType class object, instance of it'
-                             ' must be passed' % (type_func,))
+            raise TypeError(f'{type_func!r} is a FileType class object, '
+                            f'instance of it must be passed')
 
         # raise an error if the metavar does not match the type
         if hasattr(self, "_get_formatter"):
@@ -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
@@ -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
@@ -1581,11 +1580,9 @@ 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)
@@ -1601,8 +1598,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 TypeError(msg)
             dest = dest.replace('-', '_')
 
         # return the updated keyword arguments
@@ -1618,8 +1615,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):
 
@@ -1727,7 +1724,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)
@@ -1871,7 +1868,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))
@@ -2565,8 +2562,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 TypeError(f'{type_func!r} is not callable')
 
         # convert the value to the appropriate type
         try:

Lib/test/test_argparse.py

@@ -2055,7 +2055,7 @@ class TestFileTypeMissingInitialization(TestCase):
 
     def test(self):
         parser = argparse.ArgumentParser()
-        with self.assertRaises(ValueError) as cm:
+        with self.assertRaises(TypeError) as cm:
             parser.add_argument('-x', type=argparse.FileType)
 
         self.assertEqual(
@@ -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(TypeError, '1 is not callable',
+                               parser.parse_args, ['--foo', 'bar'])
+        action.type = ()
+        self.assertRaisesRegex(TypeError, r'\(\) is not callable',
+                               parser.parse_args, ['--foo', 'bar'])
+        # It is impossible to distinguish a TypeError raised due to a mismatch
+        # of the required function arguments from a TypeError raised for an incorrect
+        # argument value, and using the heavy inspection machinery is not worthwhile
+        # as it does not reliably work in all cases.
+        # Therefore, a generic ArgumentError is raised to handle this logical error.
+        action.type = pow
+        self.assertRaisesRegex(argparse.ArgumentError,
+                               "argument --foo: invalid pow value: 'bar'",
+                               parser.parse_args, ['--foo', 'bar'])
 
 
 # ================
@@ -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)
 
@@ -5534,20 +5547,27 @@ def test_missing_destination(self):
                 self.assertTypeError(action=action)
 
     def test_invalid_option_strings(self):
-        self.assertValueError('--')
-        self.assertValueError('---')
+        self.assertTypeError('-', errmsg='dest= is required')
+        self.assertTypeError('--', errmsg='dest= is required')
+        self.assertTypeError('---', errmsg='dest= is required')
 
     def test_invalid_prefix(self):
-        self.assertValueError('--foo', '+foo')
+        self.assertValueError('--foo', '+foo',
+                              errmsg='must start with a character')
 
     def test_invalid_type(self):
-        self.assertValueError('--foo', type='int')
-        self.assertValueError('--foo', type=(int, float))
+        self.assertTypeError('--foo', type='int',
+                             errmsg="'int' is not callable")
+        self.assertTypeError('--foo', type=(int, float),
+                             errmsg='is not callable')
 
     def test_invalid_action(self):
-        self.assertValueError('-x', action='foo')
-        self.assertValueError('foo', action='baz')
-        self.assertValueError('--foo', action=('store', 'append'))
+        self.assertValueError('-x', action='foo',
+                              errmsg='unknown action')
+        self.assertValueError('foo', action='baz',
+                              errmsg='unknown action')
+        self.assertValueError('--foo', action=('store', 'append'),
+                              errmsg='unknown action')
         self.assertValueError('--foo', action="store-true",
                               errmsg='unknown action')
 
@@ -5562,7 +5582,7 @@ def test_invalid_help(self):
     def test_multiple_dest(self):
         parser = argparse.ArgumentParser()
         parser.add_argument(dest='foo')
-        with self.assertRaises(ValueError) as cm:
+        with self.assertRaises(TypeError) as cm:
             parser.add_argument('bar', dest='baz')
         self.assertIn('dest supplied twice for positional argument,'
                       ' did you mean metavar?',
@@ -5733,14 +5753,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'])
 
 
 # =============================

Lib/test/translationdata/argparse/msgids.txt

@@ -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

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

@@ -0,0 +1,5 @@
+Fix exceptions in the :mod:`argparse` module so that 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.