[red-knot] PEP 695 type aliases #14357

sharkdp · 2024-11-15T10:55:18Z

Summary

Add support for (non-generic) type aliases. The main motivation behind this was to get rid of panics involving expressions in (generic) type aliases. But it turned out the best way to fix it was to implement (partial) support for type aliases.

type IntOrStr = int | str

reveal_type(IntOrStr)  # revealed: typing.TypeAliasType
reveal_type(IntOrStr.__name__)  # revealed: Literal["IntOrStr"]
reveal_type(IntOrStr.__value__)  # revealed: int | str

x: IntOrStr = 1

reveal_type(x)  # revealed: Literal[1]

def f() -> None:
    reveal_type(x)  # revealed: int | str

Test Plan

Updated corpus test allow list to reflect that we don't panic anymore.
Added Markdown-based test for type aliases (type_alias.md)

carljm

Happy to do a VC call next week if anything remains unclear about how this is all supposed to work!

crates/red_knot_python_semantic/src/semantic_index/builder.rs

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

crates/red_knot_python_semantic/src/types/infer.rs

github-actions · 2024-11-20T15:12:33Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

carljm

The updated type inference structure looks great!

crates/red_knot_python_semantic/src/semantic_index/builder.rs

carljm · 2024-11-21T01:11:57Z

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

@@ -120,6 +120,7 @@ impl<'db> ScopeId<'db> {
            NodeWithScopeKind::ClassTypeParameters(_)
                | NodeWithScopeKind::FunctionTypeParameters(_)
                | NodeWithScopeKind::Function(_)
+                | NodeWithScopeKind::TypeAlias(_)


On closer look here I realize that NodeWithScopeKind::TypeAliasTypeParameters also belongs here, just like ClassTypeParameters and FunctionTypeParameters.

Though it seems like we could also simplify this to match on self.node(db).kind() instead, and then including ScopeKind::Annotation would cover all three of the type-param scopes.

Either option seems fine to me.

Though it seems like we could also simplify this to match on self.node(db).kind() instead, and then including ScopeKind::Annotation would cover all three of the type-param scopes.

If I understand correctly, you would want to change it to:

matches!( self.node(db).scope_kind(), ScopeKind::Annotation | ScopeKind::Function | ScopeKind::TypeAlias | ScopeKind::Comprehension )

This would match everything that was matched previously, but it would additionally iunclude NodeWithScopeKind::Lambda(_). I wasn't sure if that's correct, so I just added NodeWithScopeKind::TypeAliasTypeParameters for now.

Yes, a lambda scope is a function scope, so that would actually be a bugfix.

(For reference, https://github.com/python/cpython/blob/main/Python/symtable.c#L563 is the upstream equivalent of our is_function_like, and it operates on a CPython enum which is precisely parallel to our ScopeKind enum. And we can see at https://github.com/python/cpython/blob/main/Python/symtable.c#L2379 that Lambda nodes do get a FunctionBlock scope.)

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

crates/red_knot_python_semantic/src/types.rs

crates/red_knot_python_semantic/src/types/infer.rs

crates/red_knot_python_semantic/src/types/mro.rs

carljm · 2024-11-21T01:51:58Z

crates/red_knot_workspace/tests/check.rs

+    // fails with salsa cycle panic:
+    ("crates/ruff_python_parser/resources/inline/err/type_alias_invalid_value_expr.py", true, true),


This is interesting. I suspect it might go away if we stop trying to infer the type of .value() on a type alias anytime we encounter it. If not I'll want to take a closer look.

It happens for self-referential type aliases like type NestedInt = int | list[NestedInt]. We form a genuine cycle when inferring the annotation type on the RHS and look up the name NestedInt.

crates/red_knot_python_semantic/resources/mdtest/type_alias.md

crates/red_knot_python_semantic/src/types.rs

carljm · 2024-11-21T22:15:23Z

crates/red_knot_python_semantic/resources/mdtest/type_alias.md

+
+reveal_type(IntOrStr)  # revealed: typing.TypeAliasType
+reveal_type(IntOrStr.__name__)  # revealed: Literal["IntOrStr"]
+reveal_type(IntOrStr.__value__)  # revealed: int | str


Heh, I thought yesterday about recommending a test of the __value__ attribute, and then I didn't because it's actually pretty tricky. But I should have gone ahead and mentioned why it's tricky, to save you some time.

Internally the value type we care about for a type alias is the type of the RHS expression as a type expression. And that type here is the type int | str. But the type of the actual __value__ attribute at runtime is the type of the RHS as a runtime value expression. Which in this case would be an instance of types.UnionType, the result at runtime of evaluating the expression int | str. That's a very different type from the type int | str.

If we want to do this precisely, it's possible, but I think it would require inferring the type of the RHS twice, once as a value expression and once as a type expression. And that gets into some difficulties with our expectation that a given expression has only one inferred type.

So the question is, how valuable is it to have precise typing of the __value__ attribute of a type alias? Tbh I'm not sure; it's possible that with the increasing use of runtime-typing libraries, like Pydantic, we may get user requests for this precision. But existing type checkers don't bother; they just go with the typeshed definition of TypeAliasType, which says the type of __value__ is Any: https://github.com/python/typeshed/blob/main/stdlib/typing.pyi#L1045

For TypeVars, I did try to do precise typing of __bounds__ and __constraints__, mostly so I could write tests like this one, because unlike with a type alias, I didn't even have any way to write a test like the one you wrote down on line 17 here, which correctly asserts for the type int | str. But the way I did it (using to_meta_type to try to convert "type as type expression" to "type as value expression") is hacky and wrong, as I've realized more clearly since. It works only in simple cases (if A is a class, the meta type of A is Literal[A], and the latter is also the runtime type of the expression A), but it doesn't even work for a union case like this. The meta type of A | B is Literal[A] | Literal[B], but that's not the type of the runtime value expression A | B; the latter is an instance of types.UnionType.

All that is to say: I think for this PR we should not implement any special handling for the __value__ attribute of a type alias type, and just let it fallback to typeshed. (And, separately, we should probably remove the special handling I added for __bounds__ and __constraints__ on a TypeVar. I'll push a PR for this.)

carljm · 2024-11-21T22:36:46Z

crates/red_knot_python_semantic/resources/mdtest/type_alias.md

+type IntOrStr = int | str
+type IntOrStrOrBytes = IntOrStr | bytes
+
+reveal_type(IntOrStrOrBytes.__value__)  # revealed: int | str | bytes


We should use the more verbose "annotate a name as IntOrStrOrBytes and check its revealed type from a different scope" approach here, for the reasons discussed above; the actual type of IntOrStrOrBytes.__value__ here would be types.UnionType.

carljm · 2024-11-21T22:37:34Z

crates/red_knot_python_semantic/resources/mdtest/type_alias.md

+
+MyIntOrStr = IntOrStr
+
+reveal_type(MyIntOrStr.__value__)  # revealed: int | str


And similarly this assertion is not right.

carljm · 2024-11-21T22:57:19Z

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

@@ -120,6 +120,7 @@ impl<'db> ScopeId<'db> {
            NodeWithScopeKind::ClassTypeParameters(_)
                | NodeWithScopeKind::FunctionTypeParameters(_)
                | NodeWithScopeKind::Function(_)
+                | NodeWithScopeKind::TypeAlias(_)


Yes, a lambda scope is a function scope, so that would actually be a bugfix.

(For reference, https://github.com/python/cpython/blob/main/Python/symtable.c#L563 is the upstream equivalent of our is_function_like, and it operates on a CPython enum which is precisely parallel to our ScopeKind enum. And we can see at https://github.com/python/cpython/blob/main/Python/symtable.c#L2379 that Lambda nodes do get a FunctionBlock scope.)

carljm · 2024-11-21T22:59:53Z

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

            Self::Function(_) => ScopeKind::Function,
+            Self::TypeAlias(_) => ScopeKind::TypeAlias,
            Self::Lambda(_) => ScopeKind::Function,


very minor code organization nit: maybe let's not split the two Function scopes (function and lambda) apart here? We could even group them into a single | pattern, like with the annotation and comprehension scopes below

carljm · 2024-11-21T23:03:11Z

crates/red_knot_python_semantic/src/types.rs

@@ -1954,6 +1968,11 @@ impl<'db> KnownInstanceType<'db> {
                .default_ty(db)
                .map(|ty| ty.to_meta_type(db))
                .unwrap_or_else(|| KnownClass::NoDefaultType.to_instance(db)),
+            (Self::TypeAliasType(alias), "__name__") => Type::string_literal(db, alias.name(db)),
+            (Self::TypeAliasType(alias), "__value__") => alias.value_ty(db),


As discussed above, I think we should remove this.

carljm · 2024-11-21T23:04:45Z

crates/red_knot_python_semantic/src/types.rs

+            (Self::TypeAliasType(_), "__type_params__") => {
+                todo_type!("TypeAliasType __type_params__")
+            }


I'm also not sure we need a match arm and a Todo here, when it would be correct (and match the behavior of other type checkers) to just allow it to fall back to the typeshed definition of the attribute. We can potentially add more precise per-instance typing for it in the future if there are compelling use cases, but we don't need to consider it a todo.

sharkdp added the red-knot Multi-file analysis & type inference label Nov 15, 2024

sharkdp force-pushed the david/type-aliases branch from ceb5916 to f56e867 Compare November 15, 2024 16:12

carljm reviewed Nov 16, 2024

View reviewed changes

sharkdp force-pushed the david/type-aliases branch 4 times, most recently from 9f827d6 to b9d931c Compare November 20, 2024 15:05

sharkdp force-pushed the david/type-aliases branch 5 times, most recently from e8c92e3 to 06e615d Compare November 20, 2024 21:23

sharkdp marked this pull request as ready for review November 20, 2024 21:24

sharkdp requested review from MichaReiser and AlexWaygood as code owners November 20, 2024 21:24

carljm reviewed Nov 21, 2024

View reviewed changes

crates/red_knot_python_semantic/resources/mdtest/type_alias.md Outdated Show resolved Hide resolved

crates/red_knot_python_semantic/src/types.rs Outdated Show resolved Hide resolved

crates/red_knot_python_semantic/src/types.rs Outdated Show resolved Hide resolved

dhruvmanila reviewed Nov 21, 2024

View reviewed changes

crates/red_knot_python_semantic/src/types.rs Outdated Show resolved Hide resolved

MichaReiser approved these changes Nov 21, 2024

View reviewed changes

crates/red_knot_python_semantic/src/types.rs Outdated Show resolved Hide resolved

sharkdp added 9 commits November 21, 2024 11:50

[red-knot] Preliminary support for type aliases

600e160

Clarify type alias comment

45405e8

Remove resolved TODO

66772b0

Rename value => value_ty

3777e57

Make TypeAliasType::value_ty a salsa query

a22b998

Add TypeAliasTypeParameters in 'is_function_like'

6c2e93f

Use custom NodeWithScopeKey for TypeAlias, add assertion

30d7435

Type aliases as known instances

6a7b33e

Fix new panic

2a7070d

sharkdp force-pushed the david/type-aliases branch from 06e615d to 2a7070d Compare November 21, 2024 11:00

sharkdp added 5 commits November 21, 2024 12:58

Add test to ensure distinct/disjoint property

37c0828

Get rid of .expect(…)

e02c9f0

Add more tests

c08cb76

Clean up list of known failures

4440040

Tests related to aliasing aliases

aec923f

sharkdp changed the title ~~[red-knot] Preliminary support for type aliases~~ [red-knot] PEP 695 type aliases Nov 21, 2024

carljm approved these changes Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[red-knot] PEP 695 type aliases #14357

[red-knot] PEP 695 type aliases #14357

sharkdp commented Nov 15, 2024 •

edited

Loading

carljm left a comment

github-actions bot commented Nov 20, 2024 •

edited

Loading

carljm left a comment

carljm Nov 21, 2024

sharkdp Nov 21, 2024

carljm Nov 21, 2024

carljm Nov 21, 2024

sharkdp Nov 21, 2024 •

edited

Loading

carljm Nov 21, 2024 •

edited

Loading

carljm Nov 21, 2024

carljm Nov 21, 2024

carljm Nov 21, 2024

carljm Nov 21, 2024

carljm Nov 21, 2024

carljm Nov 21, 2024

		// fails with salsa cycle panic:
		("crates/ruff_python_parser/resources/inline/err/type_alias_invalid_value_expr.py", true, true),


		MyIntOrStr = IntOrStr

		reveal_type(MyIntOrStr.__value__) # revealed: int \| str

[red-knot] PEP 695 type aliases #14357

Are you sure you want to change the base?

[red-knot] PEP 695 type aliases #14357

Conversation

sharkdp commented Nov 15, 2024 • edited Loading

Summary

Test Plan

carljm left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 20, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

carljm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

carljm Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp commented Nov 15, 2024 •

edited

Loading

github-actions bot commented Nov 20, 2024 •

edited

Loading

`ruff-ecosystem` results

sharkdp Nov 21, 2024 •

edited

Loading

carljm Nov 21, 2024 •

edited

Loading