Show more precise messages in invalid type expressions (#16850)

## Summary

Some error messages were not very specific; this PR improves them

## Test Plan

New mdtests added; existing mdtests tweaked

Author: MatthewMckee4

crates/red_knot_python_semantic/resources/mdtest/annotations/annotated.md

@@ -29,7 +29,7 @@ It is invalid to parameterize `Annotated` with less than two arguments.
 ```py
 from typing_extensions import Annotated
 
-# error: [invalid-type-form] "`Annotated` requires at least two arguments when used in an annotation or type expression"
+# error: [invalid-type-form] "`typing.Annotated` requires at least two arguments when used in a type expression"
 def _(x: Annotated):
     reveal_type(x)  # revealed: Unknown
 
@@ -39,11 +39,11 @@ def _(flag: bool):
     else:
         X = bool
 
-    # error: [invalid-type-form] "`Annotated` requires at least two arguments when used in an annotation or type expression"
+    # error: [invalid-type-form] "`typing.Annotated` requires at least two arguments when used in a type expression"
     def f(y: X):
         reveal_type(y)  # revealed: Unknown | bool
 
-# error: [invalid-type-form] "`Annotated` requires at least two arguments when used in an annotation or type expression"
+# error: [invalid-type-form] "`typing.Annotated` requires at least two arguments when used in a type expression"
 def _(x: Annotated | bool):
     reveal_type(x)  # revealed: Unknown | bool
 

crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_special_forms.md

@@ -18,7 +18,7 @@ def f(*args: Unpack[Ts]) -> tuple[Unpack[Ts]]:
     # TODO: should understand the annotation
     reveal_type(args)  # revealed: tuple
 
-    reveal_type(Alias)  # revealed: @Todo(Invalid or unsupported `KnownInstanceType` in `Type::to_type_expression`)
+    reveal_type(Alias)  # revealed: @Todo(Support for `typing.TypeAlias`)
 
 def g() -> TypeGuard[int]: ...
 def h() -> TypeIs[int]: ...
@@ -35,7 +35,26 @@ def i(callback: Callable[Concatenate[int, P], R_co], *args: P.args, **kwargs: P.
 
 class Foo:
     def method(self, x: Self):
-        reveal_type(x)  # revealed: @Todo(Invalid or unsupported `KnownInstanceType` in `Type::to_type_expression`)
+        reveal_type(x)  # revealed: @Todo(Support for `typing.Self`)
+```
+
+## Type expressions
+
+One thing that is supported is error messages for using special forms in type expressions.
+
+```py
+from typing_extensions import Unpack, TypeGuard, TypeIs, Concatenate
+
+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"
+) -> None:
+    reveal_type(a)  # revealed: Unknown
+    reveal_type(b)  # revealed: Unknown
+    reveal_type(c)  # revealed: Unknown
+    reveal_type(d)  # revealed: Unknown
 ```
 
 ## Inheritance

crates/red_knot_python_semantic/resources/mdtest/annotations/unsupported_type_qualifiers.md

@@ -1,6 +1,6 @@
 # Unsupported type qualifiers
 
-## Not yet supported
+## Not yet fully supported
 
 Several type qualifiers are unsupported by red-knot currently. However, we also don't emit
 false-positive errors if you use one in an annotation:
@@ -19,6 +19,33 @@ class Bar(TypedDict):
     z: ReadOnly[bytes]
 ```
 
+## Type expressions
+
+One thing that is supported is error messages for using type qualifiers in type expressions.
+
+```py
+from typing_extensions import Final, ClassVar, Required, NotRequired, ReadOnly
+
+def _(
+    a: (
+        Final  # error: [invalid-type-form] "Type qualifier `typing.Final` is not allowed in type expressions (only in annotation expressions)"
+        | int
+    ),
+    b: (
+        ClassVar  # error: [invalid-type-form] "Type qualifier `typing.ClassVar` is not allowed in type expressions (only in annotation expressions)"
+        | int
+    ),
+    c: Required,  # error: [invalid-type-form] "Type qualifier `typing.Required` is not allowed in type expressions (only in annotation expressions, and only with exactly one argument)"
+    d: NotRequired,  # error: [invalid-type-form] "Type qualifier `typing.NotRequired` is not allowed in type expressions (only in annotation expressions, and only with exactly one argument)"
+    e: ReadOnly,  # error: [invalid-type-form] "Type qualifier `typing.ReadOnly` is not allowed in type expressions (only in annotation expressions, and only with exactly one argument)"
+) -> None:
+    reveal_type(a)  # revealed: Unknown | int
+    reveal_type(b)  # revealed: Unknown | int
+    reveal_type(c)  # revealed: Unknown
+    reveal_type(d)  # revealed: Unknown
+    reveal_type(e)  # revealed: Unknown
+```
+
 ## Inheritance
 
 You can't inherit from a type qualifier.

crates/red_knot_python_semantic/src/types.rs

@@ -3182,6 +3182,7 @@ impl<'db> Type<'db> {
                 };
                 Ok(ty)
             }
+
             Type::SubclassOf(_)
             | Type::BooleanLiteral(_)
             | Type::BytesLiteral(_)
@@ -3200,30 +3201,95 @@ impl<'db> Type<'db> {
                 fallback_type: Type::unknown(),
             }),
 
-            // We treat `typing.Type` exactly the same as `builtins.type`:
-            Type::KnownInstance(KnownInstanceType::Type) => Ok(KnownClass::Type.to_instance(db)),
-            Type::KnownInstance(KnownInstanceType::Tuple) => Ok(KnownClass::Tuple.to_instance(db)),
+            Type::KnownInstance(known_instance) => match known_instance {
+                KnownInstanceType::TypeAliasType(alias) => Ok(alias.value_type(db)),
+                KnownInstanceType::Never | KnownInstanceType::NoReturn => Ok(Type::Never),
+                KnownInstanceType::LiteralString => Ok(Type::LiteralString),
+                KnownInstanceType::Any => Ok(Type::any()),
+                KnownInstanceType::Unknown => Ok(Type::unknown()),
+                KnownInstanceType::AlwaysTruthy => Ok(Type::AlwaysTruthy),
+                KnownInstanceType::AlwaysFalsy => Ok(Type::AlwaysFalsy),
+
+                // We treat `typing.Type` exactly the same as `builtins.type`:
+                KnownInstanceType::Type => Ok(KnownClass::Type.to_instance(db)),
+                KnownInstanceType::Tuple => Ok(KnownClass::Tuple.to_instance(db)),
+
+                // Legacy `typing` aliases
+                KnownInstanceType::List => Ok(KnownClass::List.to_instance(db)),
+                KnownInstanceType::Dict => Ok(KnownClass::Dict.to_instance(db)),
+                KnownInstanceType::Set => Ok(KnownClass::Set.to_instance(db)),
+                KnownInstanceType::FrozenSet => Ok(KnownClass::FrozenSet.to_instance(db)),
+                KnownInstanceType::ChainMap => Ok(KnownClass::ChainMap.to_instance(db)),
+                KnownInstanceType::Counter => Ok(KnownClass::Counter.to_instance(db)),
+                KnownInstanceType::DefaultDict => Ok(KnownClass::DefaultDict.to_instance(db)),
+                KnownInstanceType::Deque => Ok(KnownClass::Deque.to_instance(db)),
+                KnownInstanceType::OrderedDict => Ok(KnownClass::OrderedDict.to_instance(db)),
+
+                // TODO map this to a new `Type::TypeVar` variant
+                KnownInstanceType::TypeVar(_) => Ok(*self),
 
-            // Legacy `typing` aliases
-            Type::KnownInstance(KnownInstanceType::List) => Ok(KnownClass::List.to_instance(db)),
-            Type::KnownInstance(KnownInstanceType::Dict) => Ok(KnownClass::Dict.to_instance(db)),
-            Type::KnownInstance(KnownInstanceType::Set) => Ok(KnownClass::Set.to_instance(db)),
-            Type::KnownInstance(KnownInstanceType::FrozenSet) => {
-                Ok(KnownClass::FrozenSet.to_instance(db))
-            }
-            Type::KnownInstance(KnownInstanceType::ChainMap) => {
-                Ok(KnownClass::ChainMap.to_instance(db))
-            }
-            Type::KnownInstance(KnownInstanceType::Counter) => {
-                Ok(KnownClass::Counter.to_instance(db))
-            }
-            Type::KnownInstance(KnownInstanceType::DefaultDict) => {
-                Ok(KnownClass::DefaultDict.to_instance(db))
-            }
-            Type::KnownInstance(KnownInstanceType::Deque) => Ok(KnownClass::Deque.to_instance(db)),
-            Type::KnownInstance(KnownInstanceType::OrderedDict) => {
-                Ok(KnownClass::OrderedDict.to_instance(db))
-            }
+                // TODO: Use an opt-in rule for a bare `Callable`
+                KnownInstanceType::Callable => Ok(Type::Callable(CallableType::General(
+                    GeneralCallableType::unknown(db),
+                ))),
+
+                KnownInstanceType::TypingSelf => Ok(todo_type!("Support for `typing.Self`")),
+                KnownInstanceType::TypeAlias => Ok(todo_type!("Support for `typing.TypeAlias`")),
+
+                KnownInstanceType::Protocol => Err(InvalidTypeExpressionError {
+                    invalid_expressions: smallvec::smallvec![InvalidTypeExpression::Protocol],
+                    fallback_type: Type::unknown(),
+                }),
+
+                KnownInstanceType::Literal
+                | KnownInstanceType::Union
+                | KnownInstanceType::Intersection => Err(InvalidTypeExpressionError {
+                    invalid_expressions: smallvec::smallvec![
+                        InvalidTypeExpression::RequiresArguments(*self)
+                    ],
+                    fallback_type: Type::unknown(),
+                }),
+
+                KnownInstanceType::Optional
+                | KnownInstanceType::Not
+                | KnownInstanceType::TypeOf
+                | KnownInstanceType::TypeIs
+                | KnownInstanceType::TypeGuard
+                | KnownInstanceType::Unpack
+                | KnownInstanceType::CallableTypeFromFunction => Err(InvalidTypeExpressionError {
+                    invalid_expressions: smallvec::smallvec![
+                        InvalidTypeExpression::RequiresOneArgument(*self)
+                    ],
+                    fallback_type: Type::unknown(),
+                }),
+
+                KnownInstanceType::Annotated | KnownInstanceType::Concatenate => {
+                    Err(InvalidTypeExpressionError {
+                        invalid_expressions: smallvec::smallvec![
+                            InvalidTypeExpression::RequiresTwoArguments(*self)
+                        ],
+                        fallback_type: Type::unknown(),
+                    })
+                }
+
+                KnownInstanceType::ClassVar | KnownInstanceType::Final => {
+                    Err(InvalidTypeExpressionError {
+                        invalid_expressions: smallvec::smallvec![
+                            InvalidTypeExpression::TypeQualifier(*known_instance)
+                        ],
+                        fallback_type: Type::unknown(),
+                    })
+                }
+
+                KnownInstanceType::ReadOnly
+                | KnownInstanceType::NotRequired
+                | KnownInstanceType::Required => Err(InvalidTypeExpressionError {
+                    invalid_expressions: smallvec::smallvec![
+                        InvalidTypeExpression::TypeQualifierRequiresOneArgument(*known_instance)
+                    ],
+                    fallback_type: Type::unknown(),
+                }),
+            },
 
             Type::Union(union) => {
                 let mut builder = UnionBuilder::new(db);
@@ -3249,85 +3315,13 @@ impl<'db> Type<'db> {
                     })
                 }
             }
+
             Type::Dynamic(_) => Ok(*self),
-            // TODO map this to a new `Type::TypeVar` variant
-            Type::KnownInstance(KnownInstanceType::TypeVar(_)) => Ok(*self),
-            Type::KnownInstance(KnownInstanceType::TypeAliasType(alias)) => {
-                Ok(alias.value_type(db))
-            }
-            Type::KnownInstance(KnownInstanceType::Never | KnownInstanceType::NoReturn) => {
-                Ok(Type::Never)
-            }
-            Type::KnownInstance(KnownInstanceType::LiteralString) => Ok(Type::LiteralString),
-            Type::KnownInstance(KnownInstanceType::Any) => Ok(Type::any()),
-            Type::KnownInstance(KnownInstanceType::Annotated) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareAnnotated],
-                fallback_type: Type::unknown(),
-            }),
-            Type::KnownInstance(KnownInstanceType::ClassVar) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![
-                    InvalidTypeExpression::ClassVarInTypeExpression
-                ],
-                fallback_type: Type::unknown(),
-            }),
-            Type::KnownInstance(KnownInstanceType::Final) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![
-                    InvalidTypeExpression::FinalInTypeExpression
-                ],
-                fallback_type: Type::unknown(),
-            }),
-            Type::KnownInstance(KnownInstanceType::Unknown) => Ok(Type::unknown()),
-            Type::KnownInstance(KnownInstanceType::AlwaysTruthy) => Ok(Type::AlwaysTruthy),
-            Type::KnownInstance(KnownInstanceType::AlwaysFalsy) => Ok(Type::AlwaysFalsy),
-            Type::KnownInstance(KnownInstanceType::Callable) => {
-                // TODO: Use an opt-in rule for a bare `Callable`
-                Ok(Type::Callable(CallableType::General(
-                    GeneralCallableType::unknown(db),
-                )))
-            }
-            Type::KnownInstance(
-                KnownInstanceType::Literal
-                | KnownInstanceType::Union
-                | KnownInstanceType::Intersection,
-            ) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![InvalidTypeExpression::RequiresArguments(
-                    *self
-                )],
-                fallback_type: Type::unknown(),
-            }),
-            Type::KnownInstance(
-                KnownInstanceType::Optional
-                | KnownInstanceType::Not
-                | KnownInstanceType::TypeOf
-                | KnownInstanceType::CallableTypeFromFunction,
-            ) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![
-                    InvalidTypeExpression::RequiresOneArgument(*self)
-                ],
-                fallback_type: Type::unknown(),
-            }),
-            Type::KnownInstance(KnownInstanceType::Protocol) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![
-                    InvalidTypeExpression::ProtocolInTypeExpression
-                ],
-                fallback_type: Type::unknown(),
-            }),
-            Type::KnownInstance(
-                KnownInstanceType::TypingSelf
-                | KnownInstanceType::ReadOnly
-                | KnownInstanceType::TypeAlias
-                | KnownInstanceType::NotRequired
-                | KnownInstanceType::Concatenate
-                | KnownInstanceType::TypeIs
-                | KnownInstanceType::TypeGuard
-                | KnownInstanceType::Unpack
-                | KnownInstanceType::Required,
-            ) => Ok(todo_type!(
-                "Invalid or unsupported `KnownInstanceType` in `Type::to_type_expression`"
-            )),
+
             Type::Instance(_) => Ok(todo_type!(
                 "Invalid or unsupported `Instance` in `Type::to_type_expression`"
             )),
+
             Type::Intersection(_) => Ok(todo_type!("Type::Intersection.in_type_expression")),
         }
     }
@@ -3593,18 +3587,20 @@ impl<'db> InvalidTypeExpressionError<'db> {
 /// Enumeration of various types that are invalid in type-expression contexts
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 enum InvalidTypeExpression<'db> {
-    /// `x: Annotated` is invalid as an annotation
-    BareAnnotated,
-    /// Some types always require at least one argument when used in a type expression
-    RequiresArguments(Type<'db>),
     /// Some types always require exactly one argument when used in a type expression
     RequiresOneArgument(Type<'db>),
+    /// Some types always require at least one argument when used in a type expression
+    RequiresArguments(Type<'db>),
+    /// Some types always require at least two arguments when used in a type expression
+    RequiresTwoArguments(Type<'db>),
     /// The `Protocol` type is invalid in type expressions
-    ProtocolInTypeExpression,
-    /// The `ClassVar` type qualifier was used in a type expression
-    ClassVarInTypeExpression,
-    /// The `Final` type qualifier was used in a type expression
-    FinalInTypeExpression,
+    Protocol,
+    /// Type qualifiers are always invalid in *type expressions*,
+    /// but these ones are okay with 0 arguments in *annotation expressions*
+    TypeQualifier(KnownInstanceType<'db>),
+    /// Type qualifiers that are invalid in type expressions,
+    /// and which would require exactly one argument even if they appeared in an annotation expression
+    TypeQualifierRequiresOneArgument(KnownInstanceType<'db>),
     /// Some types are always invalid in type expressions
     InvalidType(Type<'db>),
 }
@@ -3619,26 +3615,33 @@ impl<'db> InvalidTypeExpression<'db> {
         impl std::fmt::Display for Display<'_> {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
                 match self.error {
-                    InvalidTypeExpression::BareAnnotated => f.write_str(
-                        "`Annotated` requires at least two arguments when used in an annotation or type expression"
-                    ),
                     InvalidTypeExpression::RequiresOneArgument(ty) => write!(
                         f,
                         "`{ty}` requires exactly one argument when used in a type expression",
-                        ty = ty.display(self.db)),
+                        ty = ty.display(self.db)
+                    ),
                     InvalidTypeExpression::RequiresArguments(ty) => write!(
                         f,
                         "`{ty}` requires at least one argument when used in a type expression",
                         ty = ty.display(self.db)
                     ),
-                    InvalidTypeExpression::ProtocolInTypeExpression => f.write_str(
+                    InvalidTypeExpression::RequiresTwoArguments(ty) => write!(
+                        f,
+                        "`{ty}` requires at least two arguments when used in a type expression",
+                        ty = ty.display(self.db)
+                    ),
+                    InvalidTypeExpression::Protocol => f.write_str(
                         "`typing.Protocol` is not allowed in type expressions"
                     ),
-                    InvalidTypeExpression::ClassVarInTypeExpression => f.write_str(
-                        "Type qualifier `typing.ClassVar` is not allowed in type expressions (only in annotation expressions)"
+                    InvalidTypeExpression::TypeQualifier(qualifier) => write!(
+                        f,
+                        "Type qualifier `{q}` is not allowed in type expressions (only in annotation expressions)",
+                        q = qualifier.repr(self.db)
                     ),
-                    InvalidTypeExpression::FinalInTypeExpression => f.write_str(
-                        "Type qualifier `typing.Final` is not allowed in type expressions (only in annotation expressions)"
+                    InvalidTypeExpression::TypeQualifierRequiresOneArgument(qualifier) => write!(
+                        f,
+                        "Type qualifier `{q}` is not allowed in type expressions (only in annotation expressions, and only with exactly one argument)",
+                        q = qualifier.repr(self.db)
                     ),
                     InvalidTypeExpression::InvalidType(ty) => write!(
                         f,