From 4921ac8a7daf304a9ee54de1e45bcbd546ae757d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 28 Jun 2024 17:21:59 +0300 Subject: [PATCH 1/2] [3.13] gh-121018: Fix more cases of exiting in argparse when exit_on_error=False (GH-121056) * parse_intermixed_args() now raises ArgumentError instead of calling error() if exit_on_error is false. * Internal code now always raises ArgumentError instead of calling error(). It is then caught at the higher level and error() is called if exit_on_error is true. (cherry picked from commit 81a654a3425eaa05a51342509089533c1f623f1b) Co-authored-by: Serhiy Storchaka --- Lib/argparse.py | 22 +++++---- Lib/test/test_argparse.py | 48 +++++++++++++++++-- ...-06-26-03-04-24.gh-issue-121018.clVSc4.rst | 2 + 3 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index 64893b53efbfee..7e5e3129dbd47b 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1819,7 +1819,7 @@ def _get_kwargs(self): # ================================== def add_subparsers(self, **kwargs): if self._subparsers is not None: - self.error(_('cannot have multiple subparser arguments')) + raise ArgumentError(None, _('cannot have multiple subparser arguments')) # add the parser class to the arguments if it's not present kwargs.setdefault('parser_class', type(self)) @@ -1874,7 +1874,8 @@ def parse_args(self, args=None, namespace=None): msg = _('unrecognized arguments: %s') % ' '.join(argv) if self.exit_on_error: self.error(msg) - raise ArgumentError(None, msg) + else: + raise ArgumentError(None, msg) return args def parse_known_args(self, args=None, namespace=None): @@ -2163,7 +2164,7 @@ def consume_positionals(start_index): self._get_value(action, action.default)) if required_actions: - self.error(_('the following arguments are required: %s') % + raise ArgumentError(None, _('the following arguments are required: %s') % ', '.join(required_actions)) # make sure all required groups had one option present @@ -2179,7 +2180,7 @@ def consume_positionals(start_index): for action in group._group_actions if action.help is not SUPPRESS] msg = _('one of the arguments %s is required') - self.error(msg % ' '.join(names)) + raise ArgumentError(None, msg % ' '.join(names)) # return the updated namespace and the extra arguments return namespace, extras @@ -2206,7 +2207,7 @@ def _read_args_from_files(self, arg_strings): arg_strings = self._read_args_from_files(arg_strings) new_arg_strings.extend(arg_strings) except OSError as err: - self.error(str(err)) + raise ArgumentError(None, str(err)) # return the modified argument list return new_arg_strings @@ -2286,7 +2287,7 @@ def _parse_optional(self, arg_string): for action, option_string, sep, explicit_arg in option_tuples]) args = {'option': arg_string, 'matches': options} msg = _('ambiguous option: %(option)s could match %(matches)s') - self.error(msg % args) + raise ArgumentError(None, msg % args) # if exactly one action matched, this segmentation is good, # so return the parsed action @@ -2346,7 +2347,7 @@ def _get_option_tuples(self, option_string): # shouldn't ever get here else: - self.error(_('unexpected option string: %s') % option_string) + raise ArgumentError(None, _('unexpected option string: %s') % option_string) # return the collected option tuples return result @@ -2403,8 +2404,11 @@ def _get_nargs_pattern(self, action): def parse_intermixed_args(self, args=None, namespace=None): args, argv = self.parse_known_intermixed_args(args, namespace) if argv: - msg = _('unrecognized arguments: %s') - self.error(msg % ' '.join(argv)) + msg = _('unrecognized arguments: %s') % ' '.join(argv) + if self.exit_on_error: + self.error(msg) + else: + raise ArgumentError(None, msg) return args def parse_known_intermixed_args(self, args=None, namespace=None): diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 374852949e3e25..d461911307703c 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2190,7 +2190,9 @@ def _get_parser(self, subparser_help=False, prefix_chars=None, else: subparsers_kwargs['help'] = 'command help' subparsers = parser.add_subparsers(**subparsers_kwargs) - self.assertArgumentParserError(parser.add_subparsers) + self.assertRaisesRegex(argparse.ArgumentError, + 'cannot have multiple subparser arguments', + parser.add_subparsers) # add first sub-parser parser1_kwargs = dict(description='1 description') @@ -6085,7 +6087,8 @@ def test_help_with_metavar(self): class TestExitOnError(TestCase): def setUp(self): - self.parser = argparse.ArgumentParser(exit_on_error=False) + self.parser = argparse.ArgumentParser(exit_on_error=False, + fromfile_prefix_chars='@') self.parser.add_argument('--integers', metavar='N', type=int) def test_exit_on_error_with_good_args(self): @@ -6096,9 +6099,44 @@ def test_exit_on_error_with_bad_args(self): with self.assertRaises(argparse.ArgumentError): self.parser.parse_args('--integers a'.split()) - def test_exit_on_error_with_unrecognized_args(self): - with self.assertRaises(argparse.ArgumentError): - self.parser.parse_args('--foo bar'.split()) + def test_unrecognized_args(self): + self.assertRaisesRegex(argparse.ArgumentError, + 'unrecognized arguments: --foo bar', + self.parser.parse_args, '--foo bar'.split()) + + def test_unrecognized_intermixed_args(self): + self.assertRaisesRegex(argparse.ArgumentError, + 'unrecognized arguments: --foo bar', + self.parser.parse_intermixed_args, '--foo bar'.split()) + + def test_required_args(self): + self.parser.add_argument('bar') + self.parser.add_argument('baz') + self.assertRaisesRegex(argparse.ArgumentError, + 'the following arguments are required: bar, baz', + self.parser.parse_args, []) + + def test_required_mutually_exclusive_args(self): + group = self.parser.add_mutually_exclusive_group(required=True) + group.add_argument('--bar') + group.add_argument('--baz') + self.assertRaisesRegex(argparse.ArgumentError, + 'one of the arguments --bar --baz is required', + self.parser.parse_args, []) + + def test_ambiguous_option(self): + self.parser.add_argument('--foobaz') + self.parser.add_argument('--fooble', action='store_true') + self.assertRaisesRegex(argparse.ArgumentError, + "ambiguous option: --foob could match --foobaz, --fooble", + self.parser.parse_args, ['--foob']) + + def test_os_error(self): + self.parser.add_argument('file') + self.assertRaisesRegex(argparse.ArgumentError, + "No such file or directory: 'no-such-file'", + self.parser.parse_args, ['@no-such-file']) + def tearDownModule(): # Remove global references to avoid looking like we have refleaks. diff --git a/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst b/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst new file mode 100644 index 00000000000000..aa048198fb2327 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst @@ -0,0 +1,2 @@ +Fixed other issues where :class:`argparse.ArgumentParser` did not honor +``exit_on_error=False``. \ No newline at end of file From 89a39b61383482d6ce2198771907f879251e6287 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 28 Jun 2024 17:35:42 +0300 Subject: [PATCH 2/2] Update 2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst --- .../next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst b/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst index aa048198fb2327..98a1044f887a7a 100644 --- a/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst +++ b/Misc/NEWS.d/next/Library/2024-06-26-03-04-24.gh-issue-121018.clVSc4.rst @@ -1,2 +1,2 @@ Fixed other issues where :class:`argparse.ArgumentParser` did not honor -``exit_on_error=False``. \ No newline at end of file +``exit_on_error=False``.