From 63e78b41cdcb6e5469998fe4fba97260b3a3e95c Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Thu, 20 Mar 2025 22:19:56 +0000 Subject: [PATCH] [red-knot] Ban most `Type::Instance` types in type expressions (#16872) ## Summary Catch some Instances, but raise type error for the rest of them Fixes #16851 ## Test Plan Extend invalid.md in annotations --------- Co-authored-by: Alex Waygood --- .../resources/mdtest/annotations/invalid.md | 46 +++---- .../resources/mdtest/annotations/new_types.md | 20 ++++ .../annotations/unsupported_special_forms.md | 6 +- .../resources/mdtest/generics/scoping.md | 2 +- crates/red_knot_python_semantic/src/types.rs | 26 +++- .../src/types/class.rs | 113 +++++++++++------- 6 files changed, 144 insertions(+), 69 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/annotations/new_types.md diff --git a/crates/red_knot_python_semantic/resources/mdtest/annotations/invalid.md b/crates/red_knot_python_semantic/resources/mdtest/annotations/invalid.md index 5daa0b6249..a5154b450a 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/annotations/invalid.md +++ b/crates/red_knot_python_semantic/resources/mdtest/annotations/invalid.md @@ -9,6 +9,8 @@ import typing from knot_extensions import AlwaysTruthy, AlwaysFalsy from typing_extensions import Literal, Never +class A: ... + def _( a: type[int], b: AlwaysTruthy, @@ -18,30 +20,34 @@ def _( f: Literal[b"foo"], g: tuple[int, str], h: Never, + i: int, + j: A, ): def foo(): ... def invalid( - i: a, # error: [invalid-type-form] "Variable of type `type[int]` is not allowed in a type expression" - j: b, # error: [invalid-type-form] - k: c, # error: [invalid-type-form] - l: d, # error: [invalid-type-form] - m: e, # error: [invalid-type-form] - n: f, # error: [invalid-type-form] - o: g, # error: [invalid-type-form] - p: h, # error: [invalid-type-form] - q: typing, # error: [invalid-type-form] - r: foo, # error: [invalid-type-form] + a_: a, # error: [invalid-type-form] "Variable of type `type[int]` is not allowed in a type expression" + b_: b, # error: [invalid-type-form] + c_: c, # error: [invalid-type-form] + d_: d, # error: [invalid-type-form] + e_: e, # error: [invalid-type-form] + f_: f, # error: [invalid-type-form] + g_: g, # error: [invalid-type-form] + h_: h, # error: [invalid-type-form] + i_: typing, # error: [invalid-type-form] + j_: foo, # error: [invalid-type-form] + k_: i, # error: [invalid-type-form] "Variable of type `int` is not allowed in a type expression" + l_: j, # error: [invalid-type-form] "Variable of type `A` is not allowed in a type expression" ): - reveal_type(i) # revealed: Unknown - reveal_type(j) # revealed: Unknown - reveal_type(k) # revealed: Unknown - reveal_type(l) # revealed: Unknown - reveal_type(m) # revealed: Unknown - reveal_type(n) # revealed: Unknown - reveal_type(o) # revealed: Unknown - reveal_type(p) # revealed: Unknown - reveal_type(q) # revealed: Unknown - reveal_type(r) # revealed: Unknown + reveal_type(a_) # revealed: Unknown + reveal_type(b_) # revealed: Unknown + reveal_type(c_) # revealed: Unknown + reveal_type(d_) # revealed: Unknown + reveal_type(e_) # revealed: Unknown + reveal_type(f_) # revealed: Unknown + reveal_type(g_) # revealed: Unknown + reveal_type(h_) # revealed: Unknown + reveal_type(i_) # revealed: Unknown + reveal_type(j_) # revealed: Unknown ``` ## Invalid AST nodes diff --git a/crates/red_knot_python_semantic/resources/mdtest/annotations/new_types.md b/crates/red_knot_python_semantic/resources/mdtest/annotations/new_types.md new file mode 100644 index 0000000000..c2f2fc2820 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/annotations/new_types.md @@ -0,0 +1,20 @@ +# NewType + +Currently, red-knot doesn't support `typing.NewType` in type annotations. + +## Valid forms + +```py +from typing_extensions import NewType +from types import GenericAlias + +A = NewType("A", int) +B = GenericAlias(A, ()) + +def _( + a: A, + b: B, +): + reveal_type(a) # revealed: @Todo(Support for `typing.NewType` instances in type expressions) + reveal_type(b) # revealed: @Todo(Support for `typing.GenericAlias` instances in type expressions) +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_special_forms.md b/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_special_forms.md index a798bb100f..f81c230b0b 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_special_forms.md +++ b/crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_special_forms.md @@ -43,18 +43,22 @@ class Foo: One thing that is supported is error messages for using special forms in type expressions. ```py -from typing_extensions import Unpack, TypeGuard, TypeIs, Concatenate +from typing_extensions import Unpack, TypeGuard, TypeIs, Concatenate, ParamSpec def _( a: Unpack, # error: [invalid-type-form] "`typing.Unpack` requires exactly one argument when used in a type expression" b: TypeGuard, # error: [invalid-type-form] "`typing.TypeGuard` requires exactly one argument when used in a type expression" c: TypeIs, # error: [invalid-type-form] "`typing.TypeIs` requires exactly one argument when used in a type expression" d: Concatenate, # error: [invalid-type-form] "`typing.Concatenate` requires at least two arguments when used in a type expression" + e: ParamSpec, ) -> None: reveal_type(a) # revealed: Unknown reveal_type(b) # revealed: Unknown reveal_type(c) # revealed: Unknown reveal_type(d) # revealed: Unknown + + def foo(a_: e) -> None: + reveal_type(a_) # revealed: @Todo(Support for `typing.ParamSpec` instances in type expressions) ``` ## Inheritance diff --git a/crates/red_knot_python_semantic/resources/mdtest/generics/scoping.md b/crates/red_knot_python_semantic/resources/mdtest/generics/scoping.md index 14342a838f..0d21a82294 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/generics/scoping.md +++ b/crates/red_knot_python_semantic/resources/mdtest/generics/scoping.md @@ -113,7 +113,7 @@ class Legacy(Generic[T]): legacy: Legacy[int] = Legacy() # TODO: revealed: str -reveal_type(legacy.m(1, "string")) # revealed: @Todo(Invalid or unsupported `Instance` in `Type::to_type_expression`) +reveal_type(legacy.m(1, "string")) # revealed: @Todo(Support for `typing.TypeVar` instances in type expressions) ``` With PEP 695 syntax, it is clearer that the method uses a separate typevar: diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 23d3212b59..1deeb0e08e 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -3318,9 +3318,29 @@ impl<'db> Type<'db> { Type::Dynamic(_) => Ok(*self), - Type::Instance(_) => Ok(todo_type!( - "Invalid or unsupported `Instance` in `Type::to_type_expression`" - )), + Type::Instance(InstanceType { class }) => match class.known(db) { + Some(KnownClass::TypeVar) => Ok(todo_type!( + "Support for `typing.TypeVar` instances in type expressions" + )), + Some(KnownClass::ParamSpec) => Ok(todo_type!( + "Support for `typing.ParamSpec` instances in type expressions" + )), + Some(KnownClass::TypeVarTuple) => Ok(todo_type!( + "Support for `typing.TypeVarTuple` instances in type expressions" + )), + Some(KnownClass::NewType) => Ok(todo_type!( + "Support for `typing.NewType` instances in type expressions" + )), + Some(KnownClass::GenericAlias) => Ok(todo_type!( + "Support for `typing.GenericAlias` instances in type expressions" + )), + _ => Err(InvalidTypeExpressionError { + invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType( + *self + )], + fallback_type: Type::unknown(), + }), + }, Type::Intersection(_) => Ok(todo_type!("Type::Intersection.in_type_expression")), } diff --git a/crates/red_knot_python_semantic/src/types/class.rs b/crates/red_knot_python_semantic/src/types/class.rs index 5ae487e725..6db2590c01 100644 --- a/crates/red_knot_python_semantic/src/types/class.rs +++ b/crates/red_knot_python_semantic/src/types/class.rs @@ -829,8 +829,11 @@ pub enum KnownClass { StdlibAlias, SpecialForm, TypeVar, + ParamSpec, + TypeVarTuple, TypeAliasType, NoDefaultType, + NewType, // TODO: This can probably be removed when we have support for protocols SupportsIndex, // Collections @@ -873,6 +876,8 @@ impl<'db> KnownClass { | Self::VersionInfo | Self::TypeAliasType | Self::TypeVar + | Self::ParamSpec + | Self::TypeVarTuple | Self::WrapperDescriptorType | Self::MethodWrapperType => Truthiness::AlwaysTrue, @@ -886,6 +891,7 @@ impl<'db> KnownClass { | Self::Str | Self::List | Self::GenericAlias + | Self::NewType | Self::StdlibAlias | Self::SupportsIndex | Self::Set @@ -939,8 +945,11 @@ impl<'db> KnownClass { Self::NoneType => "NoneType", Self::SpecialForm => "_SpecialForm", Self::TypeVar => "TypeVar", + Self::ParamSpec => "ParamSpec", + Self::TypeVarTuple => "TypeVarTuple", Self::TypeAliasType => "TypeAliasType", Self::NoDefaultType => "_NoDefaultType", + Self::NewType => "NewType", Self::SupportsIndex => "SupportsIndex", Self::ChainMap => "ChainMap", Self::Counter => "Counter", @@ -1109,7 +1118,9 @@ impl<'db> KnownClass { Self::SpecialForm | Self::TypeVar | Self::StdlibAlias | Self::SupportsIndex => { KnownModule::Typing } - Self::TypeAliasType => KnownModule::TypingExtensions, + Self::TypeAliasType | Self::TypeVarTuple | Self::ParamSpec | Self::NewType => { + KnownModule::TypingExtensions + } Self::NoDefaultType => { let python_version = Program::get(db).python_version(db); @@ -1142,46 +1153,49 @@ impl<'db> KnownClass { /// Return true if all instances of this `KnownClass` compare equal. pub(super) const fn is_single_valued(self) -> bool { match self { - KnownClass::NoneType - | KnownClass::NoDefaultType - | KnownClass::VersionInfo - | KnownClass::EllipsisType - | KnownClass::TypeAliasType => true, + Self::NoneType + | Self::NoDefaultType + | Self::VersionInfo + | Self::EllipsisType + | Self::TypeAliasType => true, - KnownClass::Bool - | KnownClass::Object - | KnownClass::Bytes - | KnownClass::Type - | KnownClass::Int - | KnownClass::Float - | KnownClass::Complex - | KnownClass::Str - | KnownClass::List - | KnownClass::Tuple - | KnownClass::Set - | KnownClass::FrozenSet - | KnownClass::Dict - | KnownClass::Slice - | KnownClass::Range - | KnownClass::Property - | KnownClass::BaseException - | KnownClass::BaseExceptionGroup - | KnownClass::Classmethod - | KnownClass::GenericAlias - | KnownClass::ModuleType - | KnownClass::FunctionType - | KnownClass::MethodType - | KnownClass::MethodWrapperType - | KnownClass::WrapperDescriptorType - | KnownClass::SpecialForm - | KnownClass::ChainMap - | KnownClass::Counter - | KnownClass::DefaultDict - | KnownClass::Deque - | KnownClass::OrderedDict - | KnownClass::SupportsIndex - | KnownClass::StdlibAlias - | KnownClass::TypeVar => false, + Self::Bool + | Self::Object + | Self::Bytes + | Self::Type + | Self::Int + | Self::Float + | Self::Complex + | Self::Str + | Self::List + | Self::Tuple + | Self::Set + | Self::FrozenSet + | Self::Dict + | Self::Slice + | Self::Range + | Self::Property + | Self::BaseException + | Self::BaseExceptionGroup + | Self::Classmethod + | Self::GenericAlias + | Self::ModuleType + | Self::FunctionType + | Self::MethodType + | Self::MethodWrapperType + | Self::WrapperDescriptorType + | Self::SpecialForm + | Self::ChainMap + | Self::Counter + | Self::DefaultDict + | Self::Deque + | Self::OrderedDict + | Self::SupportsIndex + | Self::StdlibAlias + | Self::TypeVar + | Self::ParamSpec + | Self::TypeVarTuple + | Self::NewType => false, } } @@ -1230,7 +1244,10 @@ impl<'db> KnownClass { | Self::BaseException | Self::BaseExceptionGroup | Self::Classmethod - | Self::TypeVar => false, + | Self::TypeVar + | Self::ParamSpec + | Self::TypeVarTuple + | Self::NewType => false, } } @@ -1268,8 +1285,11 @@ impl<'db> KnownClass { "MethodType" => Self::MethodType, "MethodWrapperType" => Self::MethodWrapperType, "WrapperDescriptorType" => Self::WrapperDescriptorType, + "NewType" => Self::NewType, "TypeAliasType" => Self::TypeAliasType, "TypeVar" => Self::TypeVar, + "ParamSpec" => Self::ParamSpec, + "TypeVarTuple" => Self::TypeVarTuple, "ChainMap" => Self::ChainMap, "Counter" => Self::Counter, "defaultdict" => Self::DefaultDict, @@ -1331,9 +1351,14 @@ impl<'db> KnownClass { | Self::MethodWrapperType | Self::WrapperDescriptorType => module == self.canonical_module(db), Self::NoneType => matches!(module, KnownModule::Typeshed | KnownModule::Types), - Self::SpecialForm | Self::TypeVar | Self::TypeAliasType | Self::NoDefaultType | Self::SupportsIndex => { - matches!(module, KnownModule::Typing | KnownModule::TypingExtensions) - } + Self::SpecialForm + | Self::TypeVar + | Self::TypeAliasType + | Self::NoDefaultType + | Self::SupportsIndex + | Self::ParamSpec + | Self::TypeVarTuple + | Self::NewType => matches!(module, KnownModule::Typing | KnownModule::TypingExtensions), } } }