From 1587eb315e8a5a27151033902a22d6c77b97d5aa Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sun, 8 Jun 2025 16:07:10 -0400 Subject: [PATCH 1/6] Change cmd2's runtime type annotation validation to be based on typing.get_type_hints Previously it was based on inspect.signature. The problem is that to Python 3.10, the inspect module doesn't have a safe way of evaluating type annotations that works equivalently both in the presence or absence of "from __future__ import annotations". Hence, any attempt at using that in an app would break cmd2. This change adds a get_types() helper function to the cmd2.utils module which uses typing.get_type_hints() to do the introspection in a safer way. --- cmd2/cmd2.py | 55 +++++++++++++++++++++++++++++---------------------- cmd2/utils.py | 51 ++++++++++++++++++++++++++--------------------- 2 files changed, 59 insertions(+), 47 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 1ad7964a..bc282e57 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -144,6 +144,7 @@ from .utils import ( Settable, get_defining_class, + get_types, strip_doc_annotations, suggest_similar, ) @@ -5544,10 +5545,10 @@ def _validate_callable_param_count(cls, func: Callable[..., Any], count: int) -> def _validate_prepostloop_callable(cls, func: Callable[[], None]) -> None: """Check parameter and return types for preloop and postloop hooks.""" cls._validate_callable_param_count(func, 0) - # make sure there is no return notation - signature = inspect.signature(func) - if signature.return_annotation is not None: - raise TypeError(f"{func.__name__} must declare return a return type of 'None'") + # make sure there is no return annotation or the return is specified as None + _, ret_ann = get_types(func) + if ret_ann is not None: + raise TypeError(f"{func.__name__} must have a return type of 'None', got: {ret_ann}") def register_preloop_hook(self, func: Callable[[], None]) -> None: """Register a function to be called at the beginning of the command loop.""" @@ -5563,11 +5564,13 @@ def register_postloop_hook(self, func: Callable[[], None]) -> None: def _validate_postparsing_callable(cls, func: Callable[[plugin.PostparsingData], plugin.PostparsingData]) -> None: """Check parameter and return types for postparsing hooks.""" cls._validate_callable_param_count(cast(Callable[..., Any], func), 1) - signature = inspect.signature(func) - _, param = next(iter(signature.parameters.items())) - if param.annotation != plugin.PostparsingData: + type_hints, ret_ann = get_types(func) + if not type_hints: + raise TypeError(f"{func.__name__} parameter is missing a type hint, expected: 'cmd2.plugin.PostparsingData'") + par_ann = next(iter(type_hints.values())) + if par_ann != plugin.PostparsingData: raise TypeError(f"{func.__name__} must have one parameter declared with type 'cmd2.plugin.PostparsingData'") - if signature.return_annotation != plugin.PostparsingData: + if ret_ann != plugin.PostparsingData: raise TypeError(f"{func.__name__} must declare return a return type of 'cmd2.plugin.PostparsingData'") def register_postparsing_hook(self, func: Callable[[plugin.PostparsingData], plugin.PostparsingData]) -> None: @@ -5582,21 +5585,21 @@ def _validate_prepostcmd_hook( cls, func: Callable[[CommandDataType], CommandDataType], data_type: type[CommandDataType] ) -> None: """Check parameter and return types for pre and post command hooks.""" - signature = inspect.signature(func) # validate that the callable has the right number of parameters cls._validate_callable_param_count(cast(Callable[..., Any], func), 1) + + type_hints, ret_ann = get_types(func) + if not type_hints: + raise TypeError(f"{func.__name__} parameter is missing a type hint, expected: {data_type}") + param_name, par_ann = next(iter(type_hints.items())) # validate the parameter has the right annotation - paramname = next(iter(signature.parameters.keys())) - param = signature.parameters[paramname] - if param.annotation != data_type: - raise TypeError(f'argument 1 of {func.__name__} has incompatible type {param.annotation}, expected {data_type}') + if par_ann != data_type: + raise TypeError(f'argument 1 of {func.__name__} has incompatible type {par_ann}, expected {data_type}') # validate the return value has the right annotation - if signature.return_annotation == signature.empty: + if ret_ann is None: raise TypeError(f'{func.__name__} does not have a declared return type, expected {data_type}') - if signature.return_annotation != data_type: - raise TypeError( - f'{func.__name__} has incompatible return type {signature.return_annotation}, expected {data_type}' - ) + if ret_ann != data_type: + raise TypeError(f'{func.__name__} has incompatible return type {ret_ann}, expected {data_type}') def register_precmd_hook(self, func: Callable[[plugin.PrecommandData], plugin.PrecommandData]) -> None: """Register a hook to be called before the command function.""" @@ -5614,12 +5617,16 @@ def _validate_cmdfinalization_callable( ) -> None: """Check parameter and return types for command finalization hooks.""" cls._validate_callable_param_count(func, 1) - signature = inspect.signature(func) - _, param = next(iter(signature.parameters.items())) - if param.annotation != plugin.CommandFinalizationData: - raise TypeError(f"{func.__name__} must have one parameter declared with type {plugin.CommandFinalizationData}") - if signature.return_annotation != plugin.CommandFinalizationData: - raise TypeError("{func.__name__} must declare return a return type of {plugin.CommandFinalizationData}") + type_hints, ret_ann = get_types(func) + if not type_hints: + raise TypeError(f"{func.__name__} parameter is missing a type hint, expected: {plugin.CommandFinalizationData}") + _, par_ann = next(iter(type_hints.items())) + if par_ann != plugin.CommandFinalizationData: + raise TypeError( + f"{func.__name__} must have one parameter declared with type {plugin.CommandFinalizationData}, got: {par_ann}" + ) + if ret_ann != plugin.CommandFinalizationData: + raise TypeError(f"{func.__name__} must declare return a return type of {plugin.CommandFinalizationData}") def register_cmdfinalization_hook( self, func: Callable[[plugin.CommandFinalizationData], plugin.CommandFinalizationData] diff --git a/cmd2/utils.py b/cmd2/utils.py index 84751723..6dd2927b 100644 --- a/cmd2/utils.py +++ b/cmd2/utils.py @@ -14,29 +14,13 @@ import threading import unicodedata from collections.abc import Callable, Iterable -from difflib import ( - SequenceMatcher, -) -from enum import ( - Enum, -) -from typing import ( - TYPE_CHECKING, - Any, - Optional, - TextIO, - TypeVar, - Union, - cast, -) - -from . import ( - constants, -) -from .argparse_custom import ( - ChoicesProviderFunc, - CompleterFunc, -) +from difflib import SequenceMatcher +from enum import Enum +from types import NoneType +from typing import TYPE_CHECKING, Any, Optional, TextIO, TypeVar, Union, cast, get_type_hints + +from . import constants +from .argparse_custom import ChoicesProviderFunc, CompleterFunc if TYPE_CHECKING: # pragma: no cover import cmd2 # noqa: F401 @@ -1261,3 +1245,24 @@ def suggest_similar( best_simil = simil proposed_command = each return proposed_command + + +def get_types(func_or_method: Callable[..., Any]) -> tuple[dict[str, Any], Any]: + """Use typing.get_type_hints() to extract type hints for parameters and return value. + + This exists because the inspect module doesn't have a safe way of doing this that works + both with and without importing annotations from __future__ until Python 3.10. + + TODO: Once cmd2 only supports Python 3.10+, change to use inspect.get_annotations(eval_str=True) + + :param func_or_method: Function or method to return the type hints for + :return tuple with first element being dictionary mapping param names to type hints + and second element being return type hint, unspecified, returns None + """ + type_hints = get_type_hints(func_or_method) # Get dictionary of type hints + ret_ann = type_hints.pop('return', None) # Pop off the return annotation if it exists + if inspect.ismethod(func_or_method): + type_hints.pop('self', None) # Pop off `self` hint for methods + if ret_ann is NoneType: + ret_ann = None # Simplify logic to just return None instead of NoneType + return type_hints, ret_ann From f8673cb58bb3a5d7a7f4a0a723e83b7619d0802a Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sun, 8 Jun 2025 16:13:34 -0400 Subject: [PATCH 2/6] Fix for Python 3.9 since types.NoneType doesn't exist until 3.10 --- cmd2/utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd2/utils.py b/cmd2/utils.py index 6dd2927b..37d34c57 100644 --- a/cmd2/utils.py +++ b/cmd2/utils.py @@ -16,7 +16,6 @@ from collections.abc import Callable, Iterable from difflib import SequenceMatcher from enum import Enum -from types import NoneType from typing import TYPE_CHECKING, Any, Optional, TextIO, TypeVar, Union, cast, get_type_hints from . import constants @@ -1263,6 +1262,6 @@ def get_types(func_or_method: Callable[..., Any]) -> tuple[dict[str, Any], Any]: ret_ann = type_hints.pop('return', None) # Pop off the return annotation if it exists if inspect.ismethod(func_or_method): type_hints.pop('self', None) # Pop off `self` hint for methods - if ret_ann is NoneType: + if ret_ann is type(None): ret_ann = None # Simplify logic to just return None instead of NoneType return type_hints, ret_ann From 4d0416361229a49c56704c0331053fa503cae517 Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sun, 8 Jun 2025 17:08:42 -0400 Subject: [PATCH 3/6] Added unit tests for new utils.get_types function --- cmd2/utils.py | 5 ++++- tests/test_utils.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/cmd2/utils.py b/cmd2/utils.py index 37d34c57..1c3506e6 100644 --- a/cmd2/utils.py +++ b/cmd2/utils.py @@ -1258,7 +1258,10 @@ def get_types(func_or_method: Callable[..., Any]) -> tuple[dict[str, Any], Any]: :return tuple with first element being dictionary mapping param names to type hints and second element being return type hint, unspecified, returns None """ - type_hints = get_type_hints(func_or_method) # Get dictionary of type hints + try: + type_hints = get_type_hints(func_or_method) # Get dictionary of type hints + except TypeError as exc: + raise ValueError("Argument passed to get_types should be a function or method") from exc ret_ann = type_hints.pop('return', None) # Pop off the return annotation if it exists if inspect.ismethod(func_or_method): type_hints.pop('self', None) # Pop off `self` hint for methods diff --git a/tests/test_utils.py b/tests/test_utils.py index efd5aa56..334b1300 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -892,3 +892,44 @@ def custom_similarity_function(s1, s2) -> float: suggested_command = cu.suggest_similar("test", ["test"], similarity_function_to_use=custom_similarity_function) assert suggested_command is None + + +def test_get_types_invalid_input() -> None: + x = 1 + with pytest.raises(ValueError, match="Argument passed to get_types should be a function or method"): + cu.get_types(x) + + +def test_get_types_empty() -> None: + def a(b): + print(b) + + param_ann, ret_ann = cu.get_types(a) + assert ret_ann is None + assert param_ann == {} + + +def test_get_types_non_empty() -> None: + def foo(x: int) -> str: + return f"{x * x}" + + param_ann, ret_ann = cu.get_types(foo) + assert ret_ann is str + param_name, param_value = next(iter(param_ann.items())) + assert param_name == 'x' + assert param_value is int + + +def test_get_types_method() -> None: + class Foo: + def bar(self, x: bool) -> None: + print(x) + + f = Foo() + + param_ann, ret_ann = cu.get_types(f.bar) + assert ret_ann is None + assert len(param_ann) == 1 + param_name, param_value = next(iter(param_ann.items())) + assert param_name == 'x' + assert param_value is bool From e8653daa0d29236b944c08080dfd55abdefa2777 Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sun, 8 Jun 2025 17:13:11 -0400 Subject: [PATCH 4/6] Add a note to the CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ad2e2d0..12465af4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.6.1 (TBD) + +- Enhancements + - Ruggedize runtime validation of type annotations when registering hook methods + ## 2.6.0 (May 31, 2025) - Breaking Change From 2c91df00691ce92c4488f0302fba884cb1d11dd6 Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sun, 8 Jun 2025 17:28:09 -0400 Subject: [PATCH 5/6] Added a new test file to cover the case representing the whole reason for this change I first verified that this test fails on the current master branch --- tests/test_future_annotations.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/test_future_annotations.py diff --git a/tests/test_future_annotations.py b/tests/test_future_annotations.py new file mode 100644 index 00000000..81e5953a --- /dev/null +++ b/tests/test_future_annotations.py @@ -0,0 +1,22 @@ +from __future__ import annotations + +import cmd2 + +from .conftest import normalize, run_cmd + + +def test_hooks_work_with_future_annotations() -> None: + class HookApp(cmd2.Cmd): + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + self.register_cmdfinalization_hook(self.hook) + + def hook(self: cmd2.Cmd, data: cmd2.plugin.CommandFinalizationData) -> cmd2.plugin.CommandFinalizationData: + if self.in_script(): + self.poutput("WE ARE IN SCRIPT") + return data + + hook_app = HookApp() + out, err = run_cmd(hook_app, '') + expected = normalize('') + assert out == expected From 896c7bb57dec7ed0035b9fbed2623245c7ad0409 Mon Sep 17 00:00:00 2001 From: Todd Leonhardt Date: Sun, 8 Jun 2025 17:35:24 -0400 Subject: [PATCH 6/6] Switched CHANGELOG type to Bug Fix --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12465af4..706feefb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## 2.6.1 (TBD) -- Enhancements - - Ruggedize runtime validation of type annotations when registering hook methods +- Bug Fixes + - Fix bug that prevented `cmd2` from working with `from __future__ import annotations` ## 2.6.0 (May 31, 2025)