Skip to content

Fix annotated with function as type keyword list parameter #20094

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
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix annotated with function as type keyword list parameter #20094

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c7f95a4
fix: add lookup_qualified
KarelKenens Oct 20, 2025
b47d21e
test: type keyword would fail before fix
KarelKenens Oct 20, 2025
e3a9a3d
Merge branch 'master' into fix-annotated-with-function-as-type-keywor…
KarelKenens Oct 20, 2025
777e4c3
test: restructure to run with correct python version
KarelKenens Oct 20, 2025
8136e70
Merge branch 'master' into fix-annotated-with-function-as-type-keywor…
KarelKenens Oct 22, 2025
0bd6bb5
fix: pass through argument to all recursive calls
KarelKenens Oct 22, 2025
9ad2101
test: deeper nested annotated
KarelKenens Oct 22, 2025
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
9 changes: 8 additions & 1 deletion mypy/exprtotype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,14 @@ def expr_to_unanalyzed_type(
# losing all the annotations.
return expr_to_unanalyzed_type(args[0], options, allow_new_syntax, expr)
base.args = tuple(
expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr, allow_unpack=True)
expr_to_unanalyzed_type(
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's other recursion happening here, do you think it's a good idea to preemptively thread through lookup_qualified?

Copy link
Author

Choose a reason for hiding this comment

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

Not going to pretend I have a good understanding of the consequences. But I can trigger what is basically the same behaviour by nesting annotations. And to me it seems that the cause is indeed that lookup_qualified is not being passed on in the recursions.
This test passes.

[case testAnnotatedWithCallableAsParameterTypeAliasDeeper]
from typing_extensions import Annotated, TypeAlias

def something() -> None: ...

A: TypeAlias = list[Annotated[Annotated[str, something()], something()]]
a: A
reveal_type(a)  # N: Revealed type is "builtins.list[builtins.str]"
[builtins fixtures/tuple.pyi]

This one does not (even after changes currently in this PR).

[case testAnnotatedWithCallableAsParameterTypeKeywordDeeper]
from typing_extensions import Annotated

def something() -> None: ...

type A = list[Annotated[Annotated[str, something()], something()]]
a: A
reveal_type(a)  # N: Revealed type is "builtins.list[builtins.str]"
[builtins fixtures/tuple.pyi]

Can make the second one pass by pushing lookup_qualified through to another recursive call.

Copy link
Collaborator

@A5rocks A5rocks Oct 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe check blame to quickly look at the PR that introduced things (maybe it addresses this), but it sounds like it would be good to pass lookup_qualified through.

I suspect:

  1. the recursive calls were added before the lookup_qualified
  2. the lookup_qualified PR was part of a larger PR so this got missed

Unfortunately I'm on mobile so blame UI kinda sucks :(

Copy link
Author

Choose a reason for hiding this comment

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

PR that added lookup_qualified doesn't address passing through the argument.
But as you said, the recursion was there already. An oversight sound plausible to me.
@A5rocks , should I add the passing through to this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, please do. @JukkaL

arg,
options,
allow_new_syntax,
expr,
allow_unpack=True,
lookup_qualified=lookup_qualified,
)
for arg in args
)
if not base.args:
Expand Down
22 changes: 22 additions & 0 deletions test-data/unit/check-type-aliases.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1318,3 +1318,25 @@ from typing_extensions import TypeAlias

Foo: TypeAlias = ClassVar[int] # E: ClassVar[...] can't be used inside a type alias
[builtins fixtures/tuple.pyi]

[case testAnnotatedWithCallableAsParameterTypeKeyword]
# flags: --python-version 3.12
from typing_extensions import Annotated

def something() -> None: ...

type A = list[Annotated[str, something()]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can move this test to test-data/unit/check-python312.test: --python-version does not override syntax errors (so won't help here), and that file only runs on 3.12+

a: A
reveal_type(a) # N: Revealed type is "builtins.list[builtins.str]"
[builtins fixtures/tuple.pyi]

[case testAnnotatedWithCallableAsParameterTypeAlias]
# flags: --python-version 3.12
from typing_extensions import Annotated, TypeAlias

def something() -> None: ...

B: TypeAlias = list[Annotated[str, something()]]
b: B
reveal_type(b) # N: Revealed type is "builtins.list[builtins.str]"
[builtins fixtures/tuple.pyi]
Loading