Skip to content

Raise ValueError for unresolvable annotations in get_types #1806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Qrytics wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Raise ValueError for unresolvable annotations in get_types #1806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd2/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1617,11 +1617,15 @@ 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 the return type hint or None if there is no return value type hint
:raises ValueError: if the `func_or_method` argument is not a valid object to pass to `inspect.get_annotations`
or if its annotations cannot be resolved
"""
try:
type_hints = inspect.get_annotations(func_or_method, eval_str=True) # Get dictionary of type hints
except TypeError as exc:

@tleonhardt tleonhardt Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This existing handler also swallows a TypeError raised while evaluating an annotation. For example, def b(x: "int[str]") reports "Argument passed to get_types should be a function or method", which is wrong.

Action: distinguish the two cases, e.g. check callable(func_or_method) up front for the invalid-argument error, then treat any TypeError from get_annotations as a resolution failure.

raise ValueError("Argument passed to get_types should be a function or method") from exc
except (NameError, SyntaxError) as exc:

@tleonhardt tleonhardt Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AttributeError still leaks: def a(x: "typing.NoSuch") raises AttributeError: module 'typing' has no attribute 'NoSuch'.

Action: add AttributeError to this tuple. _resolve_func_hints in cmd2/annotated.py catches (NameError, AttributeError, TypeError) for the same failure.

name = getattr(func_or_method, "__name__", func_or_method)
raise ValueError(f"Unable to resolve type hints for {name}: {exc}") from exc

@tleonhardt tleonhardt Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider raising TypeError here instead of ValueError. The only callers of get_types are the hook-registration validators in cmd2/cmd2.py, which raise TypeError for every other annotation problem, and _resolve_func_hints in cmd2/annotated.py already raises TypeError("Failed to resolve type hints for ...") for this exact failure.

Action: raise TypeError for unresolvable annotations and update the :raises docstring to match.

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
Expand Down
8 changes: 8 additions & 0 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1864,6 +1864,14 @@ def foo(x: int) -> str:
assert param_value is int


def test_get_types_unresolvable_forward_reference() -> None:

@tleonhardt tleonhardt Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the NameError path is tested.

Action: add cases (parametrizing this test would work well) for:

  • invalid syntax, e.g. "1 +"
  • a bad attribute, e.g. "typing.NoSuch"
  • a TypeError during evaluation, e.g. "int[str]"

Please also add one test that registers a hook with an unresolvable annotation (e.g. via register_postparsing_hook) and asserts the resulting error.

def foo(x: "NotDefinedAnywhere") -> None: # noqa: F821
print(x)

with pytest.raises(ValueError, match="Unable to resolve type hints"):
cu.get_types(foo)


def test_get_types_method() -> None:
class Foo:
def bar(self, x: bool) -> None:
Expand Down
Loading