Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Bare ClassVar annotations #15767

Open
sharkdp opened this issue Jan 27, 2025 · 0 comments
Open

[red-knot] Bare ClassVar annotations #15767

sharkdp opened this issue Jan 27, 2025 · 0 comments
Labels
red-knot Multi-file analysis & type inference

Comments

@sharkdp
Copy link
Contributor

sharkdp commented Jan 27, 2025

Description

Consider a 'bare' ClassVar annotation, i.e. without an explicit ClassVar[…] type argument:

class C:
    var: ClassVar = 1

There are two questions here:

  • Should this be allowed?
  • If allowed, what should the public type of C.var be?

Should this be allowed?

Arguments for why it should be disallowed:

  • PEP 526 which introduced ClassVar doesn't mention the possibility of a bare ClassVar

  • The typing spec seems rather clear that this is not allowed:

    A type qualifier ClassVar[T] exists in the typing module. It accepts only a single argument that should be a valid type, and is used to annotate class variables that should not be set on class instances.

  • A bare Final is explicitly allowed in the spec, but no such clarification exists for ClassVar.

  • The syntax for type and annotations expressions does not seem to allow for a ClassVar without bracketed arguments, because it would fall under the type_expression branch and this only allows bare names if they "refer to a valid in-scope class, type alias, or TypeVar", but ClassVar is a special form:

    annotation_expression ::=  …
                              | <ClassVar> '[' annotation_expression ']'
                              | …
                              | type_expression
    
    type_expression       ::=  …
                              | name
                                    (where name must refer to a valid in-scope class,
                                     type alias, or TypeVar)
                              | …
    

    Although to be fair, the syntax does not seem to allow a bare Final either, which is explicitly allowed in the spec.

Arguments for why it should be allowed:

  • It is okay to declare the type of a (class) variable using var: int, or not to declare it at all. It therefore seems reasonable to allow both var: ClassVar[int] and var: ClassVar, because ClassVar is a type qualifier that adds information that is orthogonal to the declared type.
  • Mypy and Pyright both support a bare ClassVar annotation in the sense that they raise a diagnostic if you try to modify var through an instance.
  • ClassVar seems similar in spirit to Final, and a bare Final is explicitly allowed (but see below for why we probably don't want a bare ClassVar to have the same implication as a bare Final).

If allowed, what should the public type of C.var be?

A bare Final has the following meaning:

ID: Final = 1

The typechecker should apply its usual type inference mechanisms to determine the type of ID (here, likely, int). Note that unlike for generic classes this is not the same as Final[Any].

This suggests that the type should be inferred from the right-hand side of the definition. For Red Knot, this would mean that we infer Literal[1] which is both precise and correct for Final variables, as they can not be modified.

But for ClassVar, this seems undesirable. If we treat C.var from above as having type Literal[1], we would not be allowed to modify it. There are two other reasonable behaviors:

  1. Use Unknown | Literal[1] as the public type, which would also be the type of an un-annotated class variable
  2. Use Unknown as the public type, which would also be the public type of a class variable annotated with var: ClassVar[Unknown]

Option 1 is the behavior that is documented as being desirable in TODO comments ([1], [2]). Option 2 is our current behavior on main.

Implementing Option 1 is quite involved, as there is a difference (inconsistency?) between undeclared variables and variables declared with Unknown (as documented here. It would probably involve returning Option<Type<…>> instead of Type<…> in functions like infer_annotation_expression. It might also involve treating var: ClassVar = 1 as a pure binding?

If Option 2 seems like a possible alternative (for now?), we can simply remove some TODO comments (draft PR here).

@sharkdp sharkdp added the red-knot Multi-file analysis & type inference label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

No branches or pull requests

1 participant