From a6da57dd4d164fdab904ff00d3255b508af204e2 Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Mon, 17 Oct 2022 21:22:45 +0200 Subject: [PATCH 1/9] Disallow 'store_true' action for positional arguments --- Lib/argparse.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Lib/argparse.py b/Lib/argparse.py index d2dcfdf5682f7c..3b37b60e9add04 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1540,6 +1540,11 @@ def _get_positional_kwargs(self, dest, **kwargs): msg = _("'required' is an invalid argument for positionals") raise TypeError(msg) + # make sure 'store_true' action is not specified + if 'action' in kwargs and kwargs.get('action') == 'store_true': + msg = _("'store_true' is an invalid action for positionals") + raise TypeError(msg) + # mark positional arguments as required if at least one is # always required if kwargs.get('nargs') not in [OPTIONAL, ZERO_OR_MORE]: From d8be72492d79a41008acb34d8ee994174d07c8bb Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Mon, 17 Oct 2022 21:27:13 +0200 Subject: [PATCH 2/9] Disallow 'store_false' action for positional arguments --- Lib/argparse.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Lib/argparse.py b/Lib/argparse.py index 3b37b60e9add04..818959c5467bbd 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1545,6 +1545,11 @@ def _get_positional_kwargs(self, dest, **kwargs): msg = _("'store_true' is an invalid action for positionals") raise TypeError(msg) + # make sure 'store_false' action is not specified + if 'action' in kwargs and kwargs.get('action') == 'store_false': + msg = _("'store_false' is an invalid action for positionals") + raise TypeError(msg) + # mark positional arguments as required if at least one is # always required if kwargs.get('nargs') not in [OPTIONAL, ZERO_OR_MORE]: From 3772f0045bb2e6b15f09330e5f625b676c0aca28 Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Mon, 17 Oct 2022 21:28:27 +0200 Subject: [PATCH 3/9] Disallow 'store_const' action for positional arguments --- Lib/argparse.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Lib/argparse.py b/Lib/argparse.py index 818959c5467bbd..0f898220940bb4 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1550,6 +1550,11 @@ def _get_positional_kwargs(self, dest, **kwargs): msg = _("'store_false' is an invalid action for positionals") raise TypeError(msg) + # make sure 'store_const' action is not specified + if 'action' in kwargs and kwargs.get('action') == 'store_const': + msg = _("'store_const' is an invalid action for positionals") + raise TypeError(msg) + # mark positional arguments as required if at least one is # always required if kwargs.get('nargs') not in [OPTIONAL, ZERO_OR_MORE]: From b43e81cc2a473250b731ed21c272e337a80c58fc Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Mon, 17 Oct 2022 21:37:08 +0200 Subject: [PATCH 4/9] Refactor code --- Lib/argparse.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index 0f898220940bb4..97411de2d37f9d 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1540,20 +1540,18 @@ def _get_positional_kwargs(self, dest, **kwargs): msg = _("'required' is an invalid argument for positionals") raise TypeError(msg) - # make sure 'store_true' action is not specified - if 'action' in kwargs and kwargs.get('action') == 'store_true': - msg = _("'store_true' is an invalid action for positionals") - raise TypeError(msg) - - # make sure 'store_false' action is not specified - if 'action' in kwargs and kwargs.get('action') == 'store_false': - msg = _("'store_false' is an invalid action for positionals") - raise TypeError(msg) - - # make sure 'store_const' action is not specified - if 'action' in kwargs and kwargs.get('action') == 'store_const': - msg = _("'store_const' is an invalid action for positionals") - raise TypeError(msg) + # make sure 'store_true', 'store_false', or 'store_const' action + # is not specified + if 'action' in kwargs: + if kwargs.get('action') == 'store_true': + msg = _("'store_true' is an invalid action for positionals") + raise TypeError(msg) + elif kwargs.get('action') == 'store_false': + msg = _("'store_false' is an invalid action for positionals") + raise TypeError(msg) + elif kwargs.get('action') == 'store_const': + msg = _("'store_const' is an invalid action for positionals") + raise TypeError(msg) # mark positional arguments as required if at least one is # always required From 6644aab803bb8e222ee22c0e41a3498fe4f9a134 Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Mon, 17 Oct 2022 22:03:25 +0200 Subject: [PATCH 5/9] Add tests --- Lib/test/test_argparse.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 2b7f008d38564b..310d173689a53f 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -4714,6 +4714,15 @@ def test_parsers_action_missing_params(self): def test_required_positional(self): self.assertTypeError('foo', required=True) + def test_positionals_with_store_true_action(self): + self.assertTypeError('foo', action='store_true') + + def test_positionals_with_store_false_action(self): + self.assertTypeError('foo', action='store_false') + + def test_positionals_with_store_const_action(self): + self.assertTypeError('foo', action='store_const', const='bar') + def test_user_defined_action(self): class Success(Exception): From a0e7b906df239863249a73d891461c5dc12f5a32 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 17 Oct 2022 20:20:19 +0000 Subject: [PATCH 6/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst diff --git a/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst b/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst new file mode 100644 index 00000000000000..c78cae119d8bd4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst @@ -0,0 +1 @@ +Disallowed 'store_true', 'store_false', and 'store_const' actions for positional arguments in argparse module. From f07104cf8def397841c31a516e04e9fdf7f5aea7 Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Tue, 18 Oct 2022 10:24:12 +0200 Subject: [PATCH 7/9] Refactor code --- Lib/argparse.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index 97411de2d37f9d..47b98996d8497e 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1543,14 +1543,9 @@ def _get_positional_kwargs(self, dest, **kwargs): # make sure 'store_true', 'store_false', or 'store_const' action # is not specified if 'action' in kwargs: - if kwargs.get('action') == 'store_true': - msg = _("'store_true' is an invalid action for positionals") - raise TypeError(msg) - elif kwargs.get('action') == 'store_false': - msg = _("'store_false' is an invalid action for positionals") - raise TypeError(msg) - elif kwargs.get('action') == 'store_const': - msg = _("'store_const' is an invalid action for positionals") + action = kwargs.get('action') + if action in ('store_true', 'store_false', 'store_const'): + msg = _("{action} is an invalid action for positionals", {'action': action}) raise TypeError(msg) # mark positional arguments as required if at least one is From b26fdb490ca2e593ac0de7e441f49f08627df786 Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Tue, 18 Oct 2022 13:26:57 +0200 Subject: [PATCH 8/9] Update Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst Co-authored-by: Nikita Sobolev --- .../next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst b/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst index c78cae119d8bd4..de326049a6dfd6 100644 --- a/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst +++ b/Misc/NEWS.d/next/Library/2022-10-17-20-20-18.gh-issue-97848.RnkhKZ.rst @@ -1 +1 @@ -Disallowed 'store_true', 'store_false', and 'store_const' actions for positional arguments in argparse module. +Disallow ``store_true``, ``store_false``, and ``store_const`` actions for positional arguments in :mod:`argparse` module. From a4597b173309cbbbb25cdb09c977741be9b99ef3 Mon Sep 17 00:00:00 2001 From: Krzysiek Karbowiak Date: Fri, 21 Oct 2022 11:12:42 +0200 Subject: [PATCH 9/9] Avoid double search --- Lib/argparse.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index 47b98996d8497e..4bc275217eef82 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1542,11 +1542,10 @@ def _get_positional_kwargs(self, dest, **kwargs): # make sure 'store_true', 'store_false', or 'store_const' action # is not specified - if 'action' in kwargs: - action = kwargs.get('action') - if action in ('store_true', 'store_false', 'store_const'): - msg = _("{action} is an invalid action for positionals", {'action': action}) - raise TypeError(msg) + action = kwargs.get('action') + if action in ('store_true', 'store_false', 'store_const'): + msg = _("{action} is an invalid action for positionals", {'action': action}) + raise TypeError(msg) # mark positional arguments as required if at least one is # always required