From 0e823a3b42e27959c479e24c39d0c2868f459b39 Mon Sep 17 00:00:00 2001 From: Sebastian Rittau Date: Tue, 25 Jun 2024 18:01:08 +0200 Subject: [PATCH 1/7] argparse: Forbid invalid `type` and `choices` combinations --- stdlib/@tests/test_cases/check_argparse.py | 18 +++++ stdlib/argparse.pyi | 78 +++++++++++++++++++++- 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 stdlib/@tests/test_cases/check_argparse.py diff --git a/stdlib/@tests/test_cases/check_argparse.py b/stdlib/@tests/test_cases/check_argparse.py new file mode 100644 index 000000000000..186b10b2d198 --- /dev/null +++ b/stdlib/@tests/test_cases/check_argparse.py @@ -0,0 +1,18 @@ +from argparse import _ActionsContainer, FileType + +container = _ActionsContainer("", "", "", "") + +container.add_argument("--foo") +container.add_argument("--foo", choices=[""]) +container.add_argument("--foo", choices=[1, 2, 3]) # type: ignore + +container.add_argument("--foo", type=int) +container.add_argument("--foo", type=int, choices=[1, 2, 3]) +container.add_argument("--foo", type=int, choices=[""]) # type: ignore + +container.add_argument("--foo", type="") +container.add_argument("--foo", type="", choices=[1, 2, 3]) +container.add_argument("--foo", type="", choices=[""]) + +container.add_argument("--foo", type=FileType()) +container.add_argument("--foo", type=FileType(), choices=[""]) # type: ignore diff --git a/stdlib/argparse.pyi b/stdlib/argparse.pyi index 016920923af2..a2bef0a58fe6 100644 --- a/stdlib/argparse.pyi +++ b/stdlib/argparse.pyi @@ -2,7 +2,7 @@ import sys from _typeshed import sentinel from collections.abc import Callable, Generator, Iterable, Sequence from re import Pattern -from typing import IO, Any, Generic, Literal, NewType, NoReturn, Protocol, TypeVar, overload +from typing import IO, Any, Generic, Literal, Never, NewType, NoReturn, Protocol, TypeVar, overload from typing_extensions import Self, TypeAlias, deprecated __all__ = [ @@ -83,6 +83,8 @@ class _ActionsContainer: def _registry_get(self, registry_name: str, value: Any, default: Any = None) -> Any: ... def set_defaults(self, **kwargs: Any) -> None: ... def get_default(self, dest: str) -> Any: ... + # If `choices` is not supplied (or `None`), `type` is unrestrained. + @overload def add_argument( self, *name_or_flags: str, @@ -91,7 +93,79 @@ class _ActionsContainer: const: Any = ..., default: Any = ..., type: _ActionType = ..., - choices: Iterable[_T] | None = ..., + choices: None = None, + required: bool = ..., + help: str | None = ..., + metavar: str | tuple[str, ...] | None = ..., + dest: str | None = ..., + version: str = ..., + **kwargs: Any, + ) -> Action: ... + # If `type` is not supplied, `choices` must be strings. + @overload + def add_argument( + self, + *name_or_flags: str, + action: _ActionStr | type[Action] = ..., + nargs: int | _NArgsStr | _SUPPRESS_T | None = None, + const: Any = ..., + default: Any = ..., + type: Never = ..., + choices: Iterable[str], + required: bool = ..., + help: str | None = ..., + metavar: str | tuple[str, ...] | None = ..., + dest: str | None = ..., + version: str = ..., + **kwargs: Any, + ) -> Action: ... + # If `type` is `FileType`, supplying `choices` makes no sense. + @overload + def add_argument( + self, + *name_or_flags: str, + action: _ActionStr | type[Action] = ..., + nargs: int | _NArgsStr | _SUPPRESS_T | None = None, + const: Any = ..., + default: Any = ..., + type: FileType, + choices: Never = ..., + required: bool = ..., + help: str | None = ..., + metavar: str | tuple[str, ...] | None = ..., + dest: str | None = ..., + version: str = ..., + **kwargs: Any, + ) -> Action: ... + # If `type` is a string, the possible `choices` depend on `type`. + @overload + def add_argument( + self, + *name_or_flags: str, + action: _ActionStr | type[Action] = ..., + nargs: int | _NArgsStr | _SUPPRESS_T | None = None, + const: Any = ..., + default: Any = ..., + type: str, + choices: Iterable[Any], + required: bool = ..., + help: str | None = ..., + metavar: str | tuple[str, ...] | None = ..., + dest: str | None = ..., + version: str = ..., + **kwargs: Any, + ) -> Action: ... + # If `type` is a callable, its return type is supplied to `choices`. + @overload + def add_argument( + self, + *name_or_flags: str, + action: _ActionStr | type[Action] = ..., + nargs: int | _NArgsStr | _SUPPRESS_T | None = None, + const: Any = ..., + default: Any = ..., + type: Callable[[str], _T], + choices: Iterable[_T], required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., From f9c244972db50674b0e89065ba84576d4bb08c6b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 25 Jun 2024 16:02:58 +0000 Subject: [PATCH 2/7] [pre-commit.ci] auto fixes from pre-commit.com hooks --- stdlib/@tests/test_cases/check_argparse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/@tests/test_cases/check_argparse.py b/stdlib/@tests/test_cases/check_argparse.py index 186b10b2d198..6aa2aba2bb82 100644 --- a/stdlib/@tests/test_cases/check_argparse.py +++ b/stdlib/@tests/test_cases/check_argparse.py @@ -1,4 +1,4 @@ -from argparse import _ActionsContainer, FileType +from argparse import FileType, _ActionsContainer container = _ActionsContainer("", "", "", "") From a987bdf2d268ee3037b8f745c25b49befdd658c1 Mon Sep 17 00:00:00 2001 From: Sebastian Rittau Date: Tue, 25 Jun 2024 18:08:38 +0200 Subject: [PATCH 3/7] Use `ArgumentParser` in tests --- stdlib/@tests/test_cases/check_argparse.py | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/stdlib/@tests/test_cases/check_argparse.py b/stdlib/@tests/test_cases/check_argparse.py index 6aa2aba2bb82..9b31abaf1e8b 100644 --- a/stdlib/@tests/test_cases/check_argparse.py +++ b/stdlib/@tests/test_cases/check_argparse.py @@ -1,18 +1,18 @@ -from argparse import FileType, _ActionsContainer +from argparse import ArgumentParser, FileType -container = _ActionsContainer("", "", "", "") +parser = ArgumentParser() -container.add_argument("--foo") -container.add_argument("--foo", choices=[""]) -container.add_argument("--foo", choices=[1, 2, 3]) # type: ignore +parser.add_argument("--foo") +parser.add_argument("--foo", choices=[""]) +parser.add_argument("--foo", choices=[1, 2, 3]) # type: ignore -container.add_argument("--foo", type=int) -container.add_argument("--foo", type=int, choices=[1, 2, 3]) -container.add_argument("--foo", type=int, choices=[""]) # type: ignore +parser.add_argument("--foo", type=int) +parser.add_argument("--foo", type=int, choices=[1, 2, 3]) +parser.add_argument("--foo", type=int, choices=[""]) # type: ignore -container.add_argument("--foo", type="") -container.add_argument("--foo", type="", choices=[1, 2, 3]) -container.add_argument("--foo", type="", choices=[""]) +parser.add_argument("--foo", type="") +parser.add_argument("--foo", type="", choices=[1, 2, 3]) +parser.add_argument("--foo", type="", choices=[""]) -container.add_argument("--foo", type=FileType()) -container.add_argument("--foo", type=FileType(), choices=[""]) # type: ignore +parser.add_argument("--foo", type=FileType()) +parser.add_argument("--foo", type=FileType(), choices=[""]) # type: ignore From 600e1641032d0c843d6ee8e74c024c23678876d8 Mon Sep 17 00:00:00 2001 From: Sebastian Rittau Date: Tue, 25 Jun 2024 18:16:52 +0200 Subject: [PATCH 4/7] Fix Never import Co-authored-by: Alex Waygood --- stdlib/argparse.pyi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/argparse.pyi b/stdlib/argparse.pyi index a2bef0a58fe6..32009a4bc8fa 100644 --- a/stdlib/argparse.pyi +++ b/stdlib/argparse.pyi @@ -2,8 +2,8 @@ import sys from _typeshed import sentinel from collections.abc import Callable, Generator, Iterable, Sequence from re import Pattern -from typing import IO, Any, Generic, Literal, Never, NewType, NoReturn, Protocol, TypeVar, overload -from typing_extensions import Self, TypeAlias, deprecated +from typing import IO, Any, Generic, Literal, NewType, NoReturn, Protocol, TypeVar, overload +from typing_extensions import Never, Self, TypeAlias, deprecated __all__ = [ "ArgumentParser", From 505dff28c2b7fb5f7181777907dfaf8cbcef80db Mon Sep 17 00:00:00 2001 From: Sebastian Rittau Date: Tue, 25 Jun 2024 18:56:09 +0200 Subject: [PATCH 5/7] Remove redundant overload --- stdlib/argparse.pyi | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/stdlib/argparse.pyi b/stdlib/argparse.pyi index 32009a4bc8fa..db506c0da70d 100644 --- a/stdlib/argparse.pyi +++ b/stdlib/argparse.pyi @@ -83,24 +83,6 @@ class _ActionsContainer: def _registry_get(self, registry_name: str, value: Any, default: Any = None) -> Any: ... def set_defaults(self, **kwargs: Any) -> None: ... def get_default(self, dest: str) -> Any: ... - # If `choices` is not supplied (or `None`), `type` is unrestrained. - @overload - def add_argument( - self, - *name_or_flags: str, - action: _ActionStr | type[Action] = ..., - nargs: int | _NArgsStr | _SUPPRESS_T | None = None, - const: Any = ..., - default: Any = ..., - type: _ActionType = ..., - choices: None = None, - required: bool = ..., - help: str | None = ..., - metavar: str | tuple[str, ...] | None = ..., - dest: str | None = ..., - version: str = ..., - **kwargs: Any, - ) -> Action: ... # If `type` is not supplied, `choices` must be strings. @overload def add_argument( @@ -111,7 +93,7 @@ class _ActionsContainer: const: Any = ..., default: Any = ..., type: Never = ..., - choices: Iterable[str], + choices: Iterable[str] = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -137,7 +119,7 @@ class _ActionsContainer: version: str = ..., **kwargs: Any, ) -> Action: ... - # If `type` is a string, the possible `choices` depend on `type`. + # If `type` is a callable, its return type is supplied to `choices`. @overload def add_argument( self, @@ -146,8 +128,8 @@ class _ActionsContainer: nargs: int | _NArgsStr | _SUPPRESS_T | None = None, const: Any = ..., default: Any = ..., - type: str, - choices: Iterable[Any], + type: Callable[[str], _T], + choices: Iterable[_T] = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -155,7 +137,7 @@ class _ActionsContainer: version: str = ..., **kwargs: Any, ) -> Action: ... - # If `type` is a callable, its return type is supplied to `choices`. + # If `type` is a string, the possible `choices` depend on `type`. @overload def add_argument( self, @@ -164,8 +146,8 @@ class _ActionsContainer: nargs: int | _NArgsStr | _SUPPRESS_T | None = None, const: Any = ..., default: Any = ..., - type: Callable[[str], _T], - choices: Iterable[_T], + type: str, + choices: Iterable[Any] = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., From 7bf8d181f15d948fd9f8f58aeaec7590e9347679 Mon Sep 17 00:00:00 2001 From: Sebastian Rittau Date: Tue, 25 Jun 2024 19:19:16 +0200 Subject: [PATCH 6/7] Shuffle overloads around --- stdlib/argparse.pyi | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/stdlib/argparse.pyi b/stdlib/argparse.pyi index db506c0da70d..bafd8b7aab2c 100644 --- a/stdlib/argparse.pyi +++ b/stdlib/argparse.pyi @@ -83,7 +83,10 @@ class _ActionsContainer: def _registry_get(self, registry_name: str, value: Any, default: Any = None) -> Any: ... def set_defaults(self, **kwargs: Any) -> None: ... def get_default(self, dest: str) -> Any: ... - # If `type` is not supplied, `choices` must be strings. + # If `type` is `FileType`, supplying `choices` makes no sense. + # (This overload is provided for documentation's sake, as the next overload + # will match if `type` is `FileType` and `choices` is provided, although + # the possible choices are `IO[Any]`.) @overload def add_argument( self, @@ -92,8 +95,8 @@ class _ActionsContainer: nargs: int | _NArgsStr | _SUPPRESS_T | None = None, const: Any = ..., default: Any = ..., - type: Never = ..., - choices: Iterable[str] = ..., + type: FileType, + choices: Never = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -101,7 +104,7 @@ class _ActionsContainer: version: str = ..., **kwargs: Any, ) -> Action: ... - # If `type` is `FileType`, supplying `choices` makes no sense. + # If `type` is a callable, its return type is supplied to `choices`. @overload def add_argument( self, @@ -110,8 +113,8 @@ class _ActionsContainer: nargs: int | _NArgsStr | _SUPPRESS_T | None = None, const: Any = ..., default: Any = ..., - type: FileType, - choices: Never = ..., + type: Callable[[str], _T], + choices: Iterable[_T] = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -119,7 +122,7 @@ class _ActionsContainer: version: str = ..., **kwargs: Any, ) -> Action: ... - # If `type` is a callable, its return type is supplied to `choices`. + # If `type` is a string, the possible `choices` depend on `type`. @overload def add_argument( self, @@ -128,8 +131,8 @@ class _ActionsContainer: nargs: int | _NArgsStr | _SUPPRESS_T | None = None, const: Any = ..., default: Any = ..., - type: Callable[[str], _T], - choices: Iterable[_T] = ..., + type: str, + choices: Iterable[Any] = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -137,7 +140,7 @@ class _ActionsContainer: version: str = ..., **kwargs: Any, ) -> Action: ... - # If `type` is a string, the possible `choices` depend on `type`. + # If `type` is not supplied, `choices` must be strings. @overload def add_argument( self, @@ -146,8 +149,8 @@ class _ActionsContainer: nargs: int | _NArgsStr | _SUPPRESS_T | None = None, const: Any = ..., default: Any = ..., - type: str, - choices: Iterable[Any] = ..., + type: Never = ..., + choices: Iterable[str] = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., From 96fe1668ee5a74c223e608145b111586378350fc Mon Sep 17 00:00:00 2001 From: Sebastian Rittau Date: Tue, 25 Jun 2024 19:21:43 +0200 Subject: [PATCH 7/7] Allow `None` choices --- stdlib/argparse.pyi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stdlib/argparse.pyi b/stdlib/argparse.pyi index bafd8b7aab2c..70afcf71b6d1 100644 --- a/stdlib/argparse.pyi +++ b/stdlib/argparse.pyi @@ -96,7 +96,7 @@ class _ActionsContainer: const: Any = ..., default: Any = ..., type: FileType, - choices: Never = ..., + choices: None = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -114,7 +114,7 @@ class _ActionsContainer: const: Any = ..., default: Any = ..., type: Callable[[str], _T], - choices: Iterable[_T] = ..., + choices: Iterable[_T] | None = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -132,7 +132,7 @@ class _ActionsContainer: const: Any = ..., default: Any = ..., type: str, - choices: Iterable[Any] = ..., + choices: Iterable[Any] | None = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ..., @@ -150,7 +150,7 @@ class _ActionsContainer: const: Any = ..., default: Any = ..., type: Never = ..., - choices: Iterable[str] = ..., + choices: Iterable[str] | None = ..., required: bool = ..., help: str | None = ..., metavar: str | tuple[str, ...] | None = ...,