[red-knot] Make `Type::in_type_expression()` exhaustive

[AlexWaygood]

Type::in_type_expression() already errors for some types that are invalid in type expression contexts:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 2031 to 2043 in ed2bce6

	// TODO: Should emit a diagnostic
	Type::KnownInstance(KnownInstanceType::Annotated) => Err(InvalidTypeExpressionError {
	invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareAnnotated],
	fallback_type: Type::Unknown,
	}),
	Type::KnownInstance(KnownInstanceType::Literal) => Err(InvalidTypeExpressionError {
	invalid_expressions: smallvec::smallvec![InvalidTypeExpression::BareLiteral],
	fallback_type: Type::Unknown,
	}),
	Type::Todo(_) => Ok(*self),
	_ => Ok(todo_type!(
	"Unsupported or invalid type in a type expression"
	)),

However, there are many other types that are invalid in type expression contexts. We should remove the final fallback branch from this match statement and make it exhaustive so that all typing errors are correctly reported.

I expect that many invalid types can be covered by a single branch. Doing so might require changing this method so that its return type is std::fmt::Arguments rather than &'static str:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 2208 to 2215 in ed2bce6

	impl InvalidTypeExpression {
	const fn reason(self) -> &'static str {
	match self {
	Self::BareAnnotated => "`Annotated` requires at least two arguments when used in an annotation or type expression",
	Self::BareLiteral => "`Literal` requires at least one argument when used in a type expression",
	}
	}
	}

[brandonsorensen]

Hi. I'm interested in contributing to red_knot. I'm happy to give this issue a go.

[AlexWaygood]

Hi. I'm interested in contributing to red_knot. I'm happy to give this issue a go.

Fantastic! Feel free to give me a ping if you need any help on how to tackle it :-D

[carljm]

It looks like this match is now exhaustive, but some branches still return Todo types, which could be addressed.

@brandonsorensen are you still taking a look at this, or should I un-assign for now?

[AlexWaygood]

It looks like this match is now exhaustive

Yes, sorry! I should have updated the status of this issue following #16427.

but some branches still return Todo types, which could be addressed.

I think this Todo is somewhat hard to address without an understanding of generics (we'd start erroring on "instances of typing.TypeVar being used in type expressions"):

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 3282 to 3284 in 2de8455

	Type::Instance(_) => Ok(todo_type!(
	"Invalid or unsupported `Instance` in `Type::to_type_expression`"
	)),

But I think this one is doable right now, and would be a great contributor task -- we just need to expand the match so that it is exhaustive over all KnownInstanceType variants:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 3279 to 3281 in 2de8455

	Type::KnownInstance(_) => Ok(todo_type!(
	"Invalid or unsupported `KnownInstanceType` in `Type::to_type_expression`"
	)),

This one is also possibly doable, but might be trickier for a contributor to take on:

ruff/crates/red_knot_python_semantic/src/types.rs

Line 3285 in 2de8455

Type::Intersection(_) => Ok(todo_type!("Type::Intersection.in_type_expression")),

[MatthewMckee4]

But I think this one is doable right now, and would be a great contributor task -- we just need to expand the match so that it is exhaustive over all KnownInstanceType variants:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 3279 to 3281 in 2de8455
Type::KnownInstance(_) => Ok(todo_type!(
"Invalid or unsupported KnownInstanceType in Type::to_type_expression"
)),

@AlexWaygood do you think this is something i could take a look into?

[AlexWaygood]

@AlexWaygood do you think this is something i could take a look into?

@MatthewMckee4 yes absolutely! To make the match exhaustive, for each KnownInstanceType variant you'll need to either:

• emit an appropriate error message (if the KnownInstanceType variant is only valid in a type expression when it's parameterized with a type argument), OR
• return Ok(todo_type!()) with a precise TODO error message that states exactly what the missing feature is

You may find the "type expression grammar" in the typing spec helpful for this task: https://typing.python.org/en/latest/spec/annotations.html#type-and-annotation-expressions

[AlexWaygood]

@brandonsorensen, I'll unassign you for now so that others can work on this :-) but feel free to work on any of our other "help wanted" tasks!

[MatthewMckee4]

For example, something like

from typing import Optional

def f(x: Optional) -> None:
    reveal_type(x)  # revealed: Unknown

I could catch this and say "Variable of type typing.Optional is not allowed in a type expression", with InvalidTypeExpression::InvalidType. But this is not a great message, so should i create a new enum type "BareOptional"?

[MatthewMckee4]

Or would it be better to make a enum type like InvalidBareType, so we're not duplicating tons of code. This type should take the known instance type too I think

[carljm]

It looks to me like InvalidTypeExpression enum already has BareAnnotated and BareLiteral variants; I think adding BareOptional makes sense there.

If the list of Bare* variants grows very large, we could consider the commonality between their error messages and see if they are similar enough that we can reasonably share a single BareSpecialForm variant, but I think it would be premature to do that now.