[red-knot] Eagerly normalize type[] types

[AlexWaygood]

Summary

This PR refactors SubclassOfType so that certain type[] types are eagerly normalized in our model when they are constructed:

• type[object] is eagerly normalized to Type::Instance(<builtins.type>) rather than Type::SubclassOf(<builtins.object>)
• type[<final class>] is eagerly normalized to Type::ClassLiteral(<final class>) rather than Type::SubclassOf(<final class>)

This already allows us to remove several special cases that we carve out for type[object], in Type::is_equivalent_to() and Type::is_subtype_of(). The logic applied in these special cases just now falls out naturally from our handling of instance types. The approach will also make it easier to apply knowledge of @final classes to Type::is_equivalent_to(), Type::is_subtype_of() and Type::is_disjoint_from(). Without this refactor, we'd need to carefully remember to check whether the wrapped class was @final in every branch for Type::SubclassOf; with this change, however, it's no longer necessary to do so. Several TODO comments are therefore also removed as part of this PR.

Details of implementation

The SubclassOfType struct is moved to a new types submodule, so that it is impossible to construct instances of the struct from other modules without using the "constructor" function that eagerly normalizes types. The "constructor" function is called SubclassOfType::from, and is analogous to our existing methods UnionType::from_elements (which can return any variant of Type, not just Type::Union) and TupleType::from_elements (which can return Type::Never as well as Type::Tuple()). I wasn't really sure what the best name was for the "constructor" function, though -- I initially used SubclassOfType::new(), but Clippy (reasonably) complained that new() methods are meant to return Self. Suggestions are welcome!

Test Plan

• Existing mdtests tweaked
• New mdtests added
• New unit test added for Type::is_singleton(), ensuring that type[<final class>] is understood to be a singleton type
• QUICKCHECK_TESTS=200000 cargo test -p red_knot_python_semantic -- --ignored types::property_tests::stable reveals no new property test failures

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[InSyncWithFoo]

What does this PR say about the problem discovered in #15271?

Currently report_invalid_assignment() is defined as:

pub(super) fn report_invalid_assignment(..., declared_ty: Type, ...) {
    match declared_ty {
        Type::ClassLiteral(ClassLiteralType { class }) => {
            context.report_lint(&INVALID_ASSIGNMENT, node, format_args!(
                    "Implicit shadowing of class `{}`; annotate to make it explicit if this is intentional",
                    class.name(context.db())));
        }
        // ...
    }
}

...which seems to rely on the fact that a class definition always results in a ClassLiteral type being declared_ty.

from types import EllipsisType

class SomeClass: ...
#     ^^^^^^^^^
# `declared_ty`: `ClassLiteral(SomeClass)`

#                                   vvvvvvvvv `inferred_ty`: `InstanceOf(Type)` (irrelevant)
some_variable: type[EllipsisType] = type(...)
#              ^^^^^^^^^^^^^^^^^^
# `declared_ty`: previously `SubclassOf(EllipsisType)`, now `ClassLiteral(EllipsisType)`

No diagnostics should be emitted here, as some_variable is by no mean a redefinition.

[AlexWaygood]

What does this PR say about the problem discovered in #15271?

Yeah, I'll investigate that a bit more tomorrow. My suspicion is that it's a pre-existing issue exposed by this PR, rather than a regression caused by this PR, per se. But anyway, it's late here, so a full investigation shall have to wait :-)

[carljm]

Haven't finished reviewing, but have to head out, so submitting what I have so far.

[carljm]

I think this is probably not acceptable? That is, Literal[X] for classes X is already a notation we are inventing; I don't think we can round-trip explicit user annotations of type[X] as Literal[X] in our type display, even when they are equivalent.

We could always display Literal[X] as type[X] in general. The downside here is that then we are conflating two types that are meaningfully different in how we handle them internally, with identical text representations.

I'm not sure what the alternative is, though.

[AlexWaygood]

A few points.

Firstly, we're far from unique in inventing our own notation for type display in some cases and using it in the output of reveal_type. E.g. take a look at the mypy output here. There have been a couple of complaints about some of the stranger symbols in mypy's reveal_type output being confusing, but I don't think it's really been a major issue at all. I can only remember one issue about it, and it's not highly upvoted.

Secondly, we're also far from unique in eagerly simplifying types that we see in user annotations and using the simplified types in our type display. We already do this elsewhere when we simplify unions (removing duplicate elements and elements that are subtypes of each other); so does pyright, to some extent.

Notwithstanding those two points, I have been thinking for a while that our current display for class-literal and function-literal types is a little confusing for users. It looks too close to something that would be valid in a type annotation in user code, and users will probably think that we're claiming that Literal[SomeClass] is valid in a user type annotation. It'll be confusing for them when we then reject them using Literal[SomeClass] in their own code.

Prior to this PR, I was wondering about Literal[<class 'SomeClass'>] and Literal[<function 'some_function'>] as an alternative display for class-literal and function-literal types. But your comments here make me wonder if we could also consider type[SomeClass(final=True)] or type[SomeClass(@final=True)] for class-literal types

[carljm]

Yes, I agree that both displaying our own notation for some types, and simplifying user-provided types, are things we can do. It just feels a bit like crossing another line to combine those two things in such a way that we eagerly replace a spellable user-provided type with a non-spellable type display. But maybe this isn't actually a problem in practice. I'm OK with deferring that question from this PR, and at some point following up more holistically on type display. There are a lot of interesting questions here, including how we balance conciseness/readability vs clarity (to first time readers without needing to refer to docs) in the notations we choose. I won't get too deep into the details here, I'll just say that a) I kind of like the idea of representing ClassLiteral types in some way using type[] notation, since they are similar, and b) I'm not sure we should use the word "final" in that display, because I think it's too confusing in cases where we have a ClassLiteral type but for a non-final class.

[carljm]

a pre-existing issue exposed by this PR

Yes, it looks that way to me (though I haven't investigated it thoroughly). It looks like we probably need to actually check the reaching definitions, rather than just the declared type, in deciding whether to emit the special class-shadowing diagnostic. (Or we could just abandon that special case and just decide that the regular invalid-assignment diagnostic is adequate.)

[carljm]

Looks good! I think it makes sense to go with this direction.

[carljm]

Is it worth pulling this into an SubclassOfType::is_instance_of method?

[AlexWaygood]

I'm reluctant to add a SubclassOfType::is_instance_of method exactly, because I think there's an important-yet-subtle distinction that needs to be made here: the subtype-of relationship is a relationship between types (is this instance type a subtype of the other instance type? is this subclass-of type a subtype of that instance type?) but the subclass-of and instance-of relationships are relationships between runtime objects rather than types (is this class object an instance of that class object? is this instance an instance of that class object? is this class object a subclass of that class object?). So we can ask the question "are all inhabitants of <some SubclassOfType> instances of <class X>?", but I don't think we should allow ourselves to ask the question "is <some SubclassOfType> an instance of <class X>?"; it blurs the distinction between the concepts in an unfortunate way.

I suppose I already blurred the line recently by adding the KnownInstanceType::is_instance_of() method:

ruff/crates/red_knot_python_semantic/src/types.rs

Lines 2769 to 2771 in 0dc00e6

	pub fn is_instance_of(self, db: &'db dyn Db, class: Class<'db>) -> bool {
	self.class().is_subclass_of(db, class)
	}

it felt okay because this type in particular only has exactly one inhabitant at runtime, so in this particular case the distinction between "the type" and "all inhabitants of the type" is really a distinction without a difference. Maybe it was still a mistake, though; maybe we should get rid of that method.

I'd be open to adding APIs such as SubclassOfType::inhabitants_are_instances_of() and InstanceType::inhabitants_are_instances_of(), but the longer names make it slightly more unwieldy. For now I'll leave it, but I'm happy to tackle it in a follow-up if you think it would be worth it!