gh-85935: Check for nargs=0 for positional arguments in argparse (GH-124839)

Raise ValueError in add_argument() if either explicit nargs=0 or action
that does not consume arguments (like 'store_const' or 'store_true') is
specified for positional argument.

diff --git a/Lib/argparse.py b/Lib/argparse.py
index cbecb3b753c2b9cfd217d0107c5253016291e4eb..550415dc93478b401e22dc8a0591d4363a3e73f4 100644 (file)
--- a/Lib/argparse.py
+++ b/Lib/argparse.py
@@ -1441,11 +1441,17 @@ class _ActionsContainer(object):
                 kwargs['default'] = self.argument_default
 
         # create the action object, and add it to the parser
+        action_name = kwargs.get('action')
         action_class = self._pop_action_class(kwargs)
         if not callable(action_class):
             raise ValueError('unknown action "%s"' % (action_class,))
         action = action_class(**kwargs)
 
+        # raise an error if action for positional argument does not
+        # consume arguments
+        if not action.option_strings and action.nargs == 0:
+            raise ValueError(f'action {action_name!r} is not valid for positional arguments')
+
         # 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):
@@ -1554,7 +1560,9 @@ class _ActionsContainer(object):
         # mark positional arguments as required if at least one is
         # always required
         nargs = kwargs.get('nargs')
-        if nargs not in [OPTIONAL, ZERO_OR_MORE, REMAINDER, SUPPRESS, 0]:
+        if nargs == 0:
+            raise ValueError('nargs for positionals must be != 0')
+        if nargs not in [OPTIONAL, ZERO_OR_MORE, REMAINDER, SUPPRESS]:
             kwargs['required'] = True
 
         # return the keyword arguments with no option strings

diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py
index 1fc97de78f7f895add34a29340796cdbfe36d619..f52a4b6bdd8acacf017856170d4b2fecf8e6091a 100644 (file)
--- a/Lib/test/test_argparse.py
+++ b/Lib/test/test_argparse.py
@@ -5424,8 +5424,11 @@ class TestInvalidArgumentConstructors(TestCase):
                     with self.subTest(attrs=attrs):
                         self.assertTypeError('-x', action=action, **attrs)
                         self.assertTypeError('x', action=action, **attrs)
+                self.assertValueError('x', action=action,
+                    errmsg=f"action '{action}' is not valid for positional arguments")
                 self.assertTypeError('-x', action=action, nargs=0)
-                self.assertTypeError('x', action=action, nargs=0)
+                self.assertValueError('x', action=action, nargs=0,
+                    errmsg='nargs for positionals must be != 0')
 
     def test_no_argument_no_const_actions(self):
         # options with zero arguments
@@ -5445,7 +5448,7 @@ class TestInvalidArgumentConstructors(TestCase):
                 self.assertValueError('-x', nargs=0, action=action,
                     errmsg=f'nargs for {action_name} actions must be != 0')
                 self.assertValueError('spam', nargs=0, action=action,
-                    errmsg=f'nargs for {action_name} actions must be != 0')
+                    errmsg='nargs for positionals must be != 0')
 
                 # const is disallowed with non-optional arguments
                 for nargs in [1, '*', '+']:

diff --git a/Misc/NEWS.d/next/Library/2024-10-01-13-11-53.gh-issue-85935.CTwJUy.rst b/Misc/NEWS.d/next/Library/2024-10-01-13-11-53.gh-issue-85935.CTwJUy.rst
new file mode 100644 (file)
index 0000000..553f206
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-10-01-13-11-53.gh-issue-85935.CTwJUy.rst
@@ -0,0 +1,4 @@
+:meth:`argparse.ArgumentParser.add_argument` now raises an exception if
+an :ref:`action` that does not consume arguments (like 'store_const' or
+'store_true') or explicit ``nargs=0`` are specified for positional
+arguments.