From 54e2dada2066ce4b477e873c187830a62534713b Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 24 Sep 2024 10:23:07 +0300 Subject: [PATCH] gh-63143: Fix parsing mutually exclusive arguments in argparse (GH-124307) Arguments with the value identical to the default value (e.g. booleans, small integers, empty or 1-character strings) are no longer considered "not present". (cherry picked from commit 3094cd17b0e5ba69309c54964744c797a70aa11b) Co-authored-by: Serhiy Storchaka --- Lib/argparse.py | 5 +- Lib/test/test_argparse.py | 121 +++++++++++++++++- ...4-09-21-23-56-41.gh-issue-63143.YKu-LQ.rst | 3 + 3 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-09-21-23-56-41.gh-issue-63143.YKu-LQ.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index bf17af0faf75e4..6e35f2c26f8049 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1978,9 +1978,8 @@ def take_action(action, argument_strings, option_string=None): argument_values = self._get_values(action, argument_strings) # error if this argument is not allowed with other previously - # seen arguments, assuming that actions that use the default - # value don't really count as "present" - if argument_values is not action.default: + # seen arguments + if action.option_strings or argument_strings: seen_non_default_actions.add(action) for conflict_action in action_conflicts.get(action, []): if conflict_action in seen_non_default_actions: diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index b1f72a3abd46a1..f2ae8beb916681 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -2900,26 +2900,30 @@ def test_failures_when_not_required(self): parse_args = self.get_parser(required=False).parse_args error = ArgumentParserError for args_string in self.failures: - self.assertRaises(error, parse_args, args_string.split()) + with self.subTest(args=args_string): + self.assertRaises(error, parse_args, args_string.split()) def test_failures_when_required(self): parse_args = self.get_parser(required=True).parse_args error = ArgumentParserError for args_string in self.failures + ['']: - self.assertRaises(error, parse_args, args_string.split()) + with self.subTest(args=args_string): + self.assertRaises(error, parse_args, args_string.split()) def test_successes_when_not_required(self): parse_args = self.get_parser(required=False).parse_args successes = self.successes + self.successes_when_not_required for args_string, expected_ns in successes: - actual_ns = parse_args(args_string.split()) - self.assertEqual(actual_ns, expected_ns) + with self.subTest(args=args_string): + actual_ns = parse_args(args_string.split()) + self.assertEqual(actual_ns, expected_ns) def test_successes_when_required(self): parse_args = self.get_parser(required=True).parse_args for args_string, expected_ns in self.successes: - actual_ns = parse_args(args_string.split()) - self.assertEqual(actual_ns, expected_ns) + with self.subTest(args=args_string): + actual_ns = parse_args(args_string.split()) + self.assertEqual(actual_ns, expected_ns) def test_usage_when_not_required(self): format_usage = self.get_parser(required=False).format_usage @@ -3306,6 +3310,111 @@ def get_parser(self, required): test_successes_when_not_required = None test_successes_when_required = None + +class TestMutuallyExclusiveOptionalOptional(MEMixin, TestCase): + def get_parser(self, required=None): + parser = ErrorRaisingArgumentParser(prog='PROG') + group = parser.add_mutually_exclusive_group(required=required) + group.add_argument('--foo') + group.add_argument('--bar', nargs='?') + return parser + + failures = [ + '--foo X --bar Y', + '--foo X --bar', + ] + successes = [ + ('--foo X', NS(foo='X', bar=None)), + ('--bar X', NS(foo=None, bar='X')), + ('--bar', NS(foo=None, bar=None)), + ] + successes_when_not_required = [ + ('', NS(foo=None, bar=None)), + ] + usage_when_required = '''\ + usage: PROG [-h] (--foo FOO | --bar [BAR]) + ''' + usage_when_not_required = '''\ + usage: PROG [-h] [--foo FOO | --bar [BAR]] + ''' + help = '''\ + + options: + -h, --help show this help message and exit + --foo FOO + --bar [BAR] + ''' + + +class TestMutuallyExclusiveOptionalWithDefault(MEMixin, TestCase): + def get_parser(self, required=None): + parser = ErrorRaisingArgumentParser(prog='PROG') + group = parser.add_mutually_exclusive_group(required=required) + group.add_argument('--foo') + group.add_argument('--bar', type=bool, default=True) + return parser + + failures = [ + '--foo X --bar Y', + '--foo X --bar=', + ] + successes = [ + ('--foo X', NS(foo='X', bar=True)), + ('--bar X', NS(foo=None, bar=True)), + ('--bar=', NS(foo=None, bar=False)), + ] + successes_when_not_required = [ + ('', NS(foo=None, bar=True)), + ] + usage_when_required = '''\ + usage: PROG [-h] (--foo FOO | --bar BAR) + ''' + usage_when_not_required = '''\ + usage: PROG [-h] [--foo FOO | --bar BAR] + ''' + help = '''\ + + options: + -h, --help show this help message and exit + --foo FOO + --bar BAR + ''' + + +class TestMutuallyExclusivePositionalWithDefault(MEMixin, TestCase): + def get_parser(self, required=None): + parser = ErrorRaisingArgumentParser(prog='PROG') + group = parser.add_mutually_exclusive_group(required=required) + group.add_argument('--foo') + group.add_argument('bar', nargs='?', type=bool, default=True) + return parser + + failures = [ + '--foo X Y', + ] + successes = [ + ('--foo X', NS(foo='X', bar=True)), + ('X', NS(foo=None, bar=True)), + ] + successes_when_not_required = [ + ('', NS(foo=None, bar=True)), + ] + usage_when_required = '''\ + usage: PROG [-h] (--foo FOO | bar) + ''' + usage_when_not_required = '''\ + usage: PROG [-h] [--foo FOO | bar] + ''' + help = '''\ + + positional arguments: + bar + + options: + -h, --help show this help message and exit + --foo FOO + ''' + # ================================================= # Mutually exclusive group in parent parser tests # ================================================= diff --git a/Misc/NEWS.d/next/Library/2024-09-21-23-56-41.gh-issue-63143.YKu-LQ.rst b/Misc/NEWS.d/next/Library/2024-09-21-23-56-41.gh-issue-63143.YKu-LQ.rst new file mode 100644 index 00000000000000..cb031fd601a9bd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-09-21-23-56-41.gh-issue-63143.YKu-LQ.rst @@ -0,0 +1,3 @@ +Fix parsing mutually exclusive arguments in :mod:`argparse`. Arguments with +the value identical to the default value (e.g. booleans, small integers, +empty or 1-character strings) are no longer considered "not present".