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 all 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
61 changes: 49 additions & 12 deletions mypy/exprtotype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ def expr_to_unanalyzed_type(
else:
raise TypeTranslationError()
elif isinstance(expr, IndexExpr):
base = expr_to_unanalyzed_type(expr.base, options, allow_new_syntax, expr)
base = expr_to_unanalyzed_type(
expr.base, options, allow_new_syntax, expr, lookup_qualified=lookup_qualified
)
if isinstance(base, UnboundType):
if base.args:
raise TypeTranslationError()
Expand All @@ -124,9 +126,18 @@ def expr_to_unanalyzed_type(
# TODO: this is not the optimal solution as we are basically getting rid
# of the Annotation definition and only returning the type information,
# losing all the annotations.
return expr_to_unanalyzed_type(args[0], options, allow_new_syntax, expr)
return expr_to_unanalyzed_type(
args[0], options, allow_new_syntax, expr, lookup_qualified=lookup_qualified
)
base.args = tuple(
expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr, allow_unpack=True)
expr_to_unanalyzed_type(
Copy link
Collaborator

@A5rocks A5rocks Oct 22, 2025

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

@KarelKenens KarelKenens Oct 22, 2025

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

@KarelKenens KarelKenens Oct 22, 2025

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

@A5rocks A5rocks Oct 23, 2025

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 All @@ -141,8 +152,12 @@ def expr_to_unanalyzed_type(
):
return UnionType(
[
expr_to_unanalyzed_type(expr.left, options, allow_new_syntax),
expr_to_unanalyzed_type(expr.right, options, allow_new_syntax),
expr_to_unanalyzed_type(
expr.left, options, allow_new_syntax, lookup_qualified=lookup_qualified
),
expr_to_unanalyzed_type(
expr.right, options, allow_new_syntax, lookup_qualified=lookup_qualified
),
],
uses_pep604_syntax=True,
)
Expand Down Expand Up @@ -178,12 +193,16 @@ def expr_to_unanalyzed_type(
if typ is not default_type:
# Two types
raise TypeTranslationError()
typ = expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr)
typ = expr_to_unanalyzed_type(
arg, options, allow_new_syntax, expr, lookup_qualified=lookup_qualified
)
continue
else:
raise TypeTranslationError()
elif i == 0:
typ = expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr)
typ = expr_to_unanalyzed_type(
arg, options, allow_new_syntax, expr, lookup_qualified=lookup_qualified
)
elif i == 1:
name = _extract_argument_name(arg)
else:
Expand All @@ -192,7 +211,14 @@ def expr_to_unanalyzed_type(
elif isinstance(expr, ListExpr):
return TypeList(
[
expr_to_unanalyzed_type(t, options, allow_new_syntax, expr, allow_unpack=True)
expr_to_unanalyzed_type(
t,
options,
allow_new_syntax,
expr,
allow_unpack=True,
lookup_qualified=lookup_qualified,
)
for t in expr.items
],
line=expr.line,
Expand All @@ -203,7 +229,9 @@ def expr_to_unanalyzed_type(
elif isinstance(expr, BytesExpr):
return parse_type_string(expr.value, "builtins.bytes", expr.line, expr.column)
elif isinstance(expr, UnaryExpr):
typ = expr_to_unanalyzed_type(expr.expr, options, allow_new_syntax)
typ = expr_to_unanalyzed_type(
expr.expr, options, allow_new_syntax, lookup_qualified=lookup_qualified
)
if isinstance(typ, RawExpressionType):
if isinstance(typ.literal_value, int):
if expr.op == "-":
Expand All @@ -225,7 +253,10 @@ def expr_to_unanalyzed_type(
return EllipsisType(expr.line)
elif allow_unpack and isinstance(expr, StarExpr):
return UnpackType(
expr_to_unanalyzed_type(expr.expr, options, allow_new_syntax), from_star_syntax=True
expr_to_unanalyzed_type(
expr.expr, options, allow_new_syntax, lookup_qualified=lookup_qualified
),
from_star_syntax=True,
)
elif isinstance(expr, DictExpr):
if not expr.items:
Expand All @@ -236,12 +267,18 @@ def expr_to_unanalyzed_type(
if not isinstance(item_name, StrExpr):
if item_name is None:
extra_items_from.append(
expr_to_unanalyzed_type(value, options, allow_new_syntax, expr)
expr_to_unanalyzed_type(
value,
options,
allow_new_syntax,
expr,
lookup_qualified=lookup_qualified,
)
)
continue
raise TypeTranslationError()
items[item_name.value] = expr_to_unanalyzed_type(
value, options, allow_new_syntax, expr
value, options, allow_new_syntax, expr, lookup_qualified=lookup_qualified
)
result = TypedDictType(
items, set(), set(), Instance(MISSING_FALLBACK, ()), expr.line, expr.column
Expand Down
20 changes: 20 additions & 0 deletions test-data/unit/check-python312.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2168,3 +2168,23 @@ x: MyTuple[int, str]
reveal_type(x[0]) # N: Revealed type is "Any"
[builtins fixtures/tuple.pyi]
[typing fixtures/typing-full.pyi]

[case testAnnotatedWithCallableAsParameterTypeKeyword]
from typing_extensions import Annotated

def something() -> None: ...

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

[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]
20 changes: 20 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,23 @@ from typing_extensions import TypeAlias

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

[case testAnnotatedWithCallableAsParameterTypeAlias]
from typing_extensions import Annotated, TypeAlias

def something() -> None: ...

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

[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]