diff --git a/Lib/argparse.py b/Lib/argparse.py index 543d9944f9e..dfc98695f64 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1528,6 +1528,8 @@ class _ActionsContainer(object): title_group_map = {} for group in self._action_groups: 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)) title_group_map[group.title] = group @@ -1811,13 +1813,11 @@ class ArgumentParser(_AttributeHolder, _ActionsContainer): # add parent arguments and defaults for parent in parents: + if not isinstance(parent, ArgumentParser): + raise TypeError('parents must be a list of ArgumentParser') self._add_container_actions(parent) - try: - defaults = parent._defaults - except AttributeError: - pass - else: - self._defaults.update(defaults) + defaults = parent._defaults + self._defaults.update(defaults) # ======================= # Pretty __repr__ methods diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 3a62a16cee3..7c1f5d36999 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -15,7 +15,7 @@ import unittest import argparse import warnings -from test.support import os_helper +from test.support import os_helper, captured_stderr from unittest import mock @@ -1382,6 +1382,19 @@ class TestPositionalsActionAppend(ParserTestCase): ('a b c', NS(spam=['a', ['b', 'c']])), ] + +class TestPositionalsActionExtend(ParserTestCase): + """Test the 'extend' action""" + + argument_signatures = [ + Sig('spam', action='extend'), + Sig('spam', action='extend', nargs=2), + ] + failures = ['', '--foo', 'a', 'a b', 'a b c d'] + successes = [ + ('a b c', NS(spam=['a', 'b', 'c'])), + ] + # ======================================== # Combined optionals and positionals tests # ======================================== @@ -1419,6 +1432,32 @@ class TestOptionalsAlmostNumericAndPositionals(ParserTestCase): ] +class TestOptionalsAndPositionalsAppend(ParserTestCase): + argument_signatures = [ + Sig('foo', nargs='*', action='append'), + Sig('--bar'), + ] + failures = ['-foo'] + successes = [ + ('a b', NS(foo=[['a', 'b']], bar=None)), + ('--bar a b', NS(foo=[['b']], bar='a')), + ('a b --bar c', NS(foo=[['a', 'b']], bar='c')), + ] + + +class TestOptionalsAndPositionalsExtend(ParserTestCase): + argument_signatures = [ + Sig('foo', nargs='*', action='extend'), + Sig('--bar'), + ] + failures = ['-foo'] + successes = [ + ('a b', NS(foo=['a', 'b'], bar=None)), + ('--bar a b', NS(foo=['b'], bar='a')), + ('a b --bar c', NS(foo=['a', 'b'], bar='c')), + ] + + class TestEmptyAndSpaceContainingArguments(ParserTestCase): argument_signatures = [ @@ -1899,6 +1938,10 @@ class TestFileTypeOpenArgs(TestCase): type('foo') m.assert_called_with('foo', *args) + def test_invalid_file_type(self): + with self.assertRaises(ValueError): + argparse.FileType('b')('-test') + class TestFileTypeMissingInitialization(TestCase): """ @@ -2092,6 +2135,27 @@ class TestActionExtend(ParserTestCase): ('--foo f1 --foo f2 f3 f4', NS(foo=['f1', 'f2', 'f3', 'f4'])), ] + +class TestInvalidAction(TestCase): + """Test invalid user defined Action""" + + class ActionWithoutCall(argparse.Action): + pass + + def test_invalid_type(self): + parser = argparse.ArgumentParser() + + parser.add_argument('--foo', action=self.ActionWithoutCall) + self.assertRaises(NotImplementedError, parser.parse_args, ['--foo', 'bar']) + + def test_modified_invalid_action(self): + parser = ErrorRaisingArgumentParser() + action = parser.add_argument('--foo') + # Someone got crazy and did this + action.type = 1 + self.assertRaises(ArgumentParserError, parser.parse_args, ['--foo', 'bar']) + + # ================ # Subparsers tests # ================ @@ -2727,6 +2791,9 @@ class TestParentParsers(TestCase): -x X '''.format(progname, ' ' if progname else '' ))) + def test_wrong_type_parents(self): + self.assertRaises(TypeError, ErrorRaisingArgumentParser, parents=[1]) + # ============================== # Mutually exclusive group tests # ============================== @@ -4743,6 +4810,9 @@ class TestInvalidArgumentConstructors(TestCase): self.assertValueError('--') self.assertValueError('---') + def test_invalid_prefix(self): + self.assertValueError('--foo', '+foo') + def test_invalid_type(self): self.assertValueError('--foo', type='int') self.assertValueError('--foo', type=(int, float)) @@ -4807,6 +4877,9 @@ class TestInvalidArgumentConstructors(TestCase): self.assertTypeError('command', action='parsers', parser_class=argparse.ArgumentParser) + def test_version_missing_params(self): + self.assertTypeError('command', action='version') + def test_required_positional(self): self.assertTypeError('foo', required=True) @@ -5400,6 +5473,17 @@ class TestIntermixedArgs(TestCase): self.assertRaises(TypeError, parser.parse_intermixed_args, []) self.assertEqual(group.required, True) + def test_invalid_args(self): + parser = ErrorRaisingArgumentParser(prog='PROG') + self.assertRaises(ArgumentParserError, parser.parse_intermixed_args, ['a']) + + parser = ErrorRaisingArgumentParser(prog='PROG') + parser.add_argument('--foo', nargs="*") + parser.add_argument('foo') + with captured_stderr() as stderr: + parser.parse_intermixed_args(['hello', '--foo']) + self.assertIn("UserWarning", stderr.getvalue()) + class TestIntermixedMessageContentError(TestCase): # case where Intermixed gives different error message # error is raised by 1st parsing step diff --git a/Misc/NEWS.d/next/Library/2023-04-15-23-26-16.gh-issue-103558.w9OzK4.rst b/Misc/NEWS.d/next/Library/2023-04-15-23-26-16.gh-issue-103558.w9OzK4.rst new file mode 100644 index 00000000000..e62af647fcc --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-15-23-26-16.gh-issue-103558.w9OzK4.rst @@ -0,0 +1 @@ +Fixed ``parent`` argument validation mechanism of :mod:`argparse`. Improved test coverage.