Skip to content

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Jan 27, 2025

Summary

It was recently clarified in the typing spec that bare ClassVar annotations are allowed. For annotated assignments with a right hand side value, the spec requires type checkers to infer the type as something "to which [the] value is assignable". For a value of 2, the spec suggests int, Literal[2], or Any as examples. Here, we choose Unknown | Literal[2] instead, conforming with out usual treatment of attribute types.

closes astral-sh/ty#211

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Jan 27, 2025
@sharkdp sharkdp force-pushed the david/bare-classvar-annotation branch from 538b8ff to 5b6d2d7 Compare July 7, 2025 11:25
@sharkdp sharkdp force-pushed the david/bare-classvar-annotation branch from 5b6d2d7 to 0580abf Compare July 7, 2025 11:28
@sharkdp sharkdp changed the title [red-knot] Treat bare ClassVar annotation as ClassVar[Unknown] [red-knot] Bare ClassVar annotations Jul 7, 2025
Copy link
Contributor

github-actions bot commented Jul 7, 2025

mypy_primer results

Changes were detected when running on open source projects
bidict (https://github.com/jab/bidict)
-     memo fields = ~17MB
+     memo fields = ~15MB

parso (https://github.com/davidhalter/parso)
-     memo fields = ~54MB
+     memo fields = ~49MB

black (https://github.com/psf/black)
- TOTAL MEMORY USAGE: ~142MB
+ TOTAL MEMORY USAGE: ~129MB

porcupine (https://github.com/Akuli/porcupine)
-     memo fields = ~106MB
+     memo fields = ~97MB

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- TOTAL MEMORY USAGE: ~97MB
+ TOTAL MEMORY USAGE: ~88MB

schemathesis (https://github.com/schemathesis/schemathesis)
-     memo fields = ~129MB
+     memo fields = ~142MB

flake8 (https://github.com/pycqa/flake8)
-     memo fields = ~60MB
+     memo fields = ~66MB

jinja (https://github.com/pallets/jinja)
-     memo fields = ~80MB
+     memo fields = ~88MB

cki-lib (https://gitlab.com/cki-project/cki-lib)
-     memo fields = ~80MB
+     memo fields = ~72MB

rich (https://github.com/Textualize/rich)
-     memo fields = ~106MB
+     memo fields = ~117MB

altair (https://github.com/vega/altair)
-     memo fields = ~228MB
+     memo fields = ~251MB

openlibrary (https://github.com/internetarchive/openlibrary)
-     memo fields = ~171MB
+     memo fields = ~189MB

@sharkdp sharkdp marked this pull request as ready for review July 7, 2025 11:33
@AlexWaygood
Copy link
Member

What's our behaviour for, e.g.

from typing import Final, ClassVar

class Foo:
    X: Final[ClassVar] = 1

Do we have any tests for that? Does the spec say anything about it? What are mypy/pyright's behaviour there?

It would be pretty unusual to see something like that since a Final declaration in a class body usually implies that it is also a ClassVar. But there are a few edge cases, for e.g. dataclasses.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 7, 2025

Regarding dataclasses: this is what the spec says:

  • ClassVar attributes are not considered dataclass fields and are ignored by dataclass mechanisms.

  • A dataclass field may be annotated with Final[...]. For example, x: Final[int] in a dataclass body specifies a dataclass field x, which will be initialized in the generated __init__ and cannot be assigned to thereafter. A Final dataclass field initialized in the class body is not a class attribute unless explicitly annotated with ClassVar. For example, x: Final[int] = 3 is a dataclass field named x with a default value of 3 in the generated __init__ method. A final class variable on a dataclass must be explicitly annotated as e.g. x: ClassVar[Final[int]] = 3.

@MichaReiser MichaReiser changed the title [red-knot] Bare ClassVar annotations [ty] Bare ClassVar annotations Jul 7, 2025
@sharkdp
Copy link
Contributor Author

sharkdp commented Jul 7, 2025

What's our behaviour for, e.g.

from typing import Final, ClassVar

class Foo:
    X: Final[ClassVar] = 1

What are mypy/pyright's behaviour there?

Mypy infers Any. Pyright and pyrefly say that ClassVar is not allowed in that context(?).

When we change that to X: ClassVar[Final] = 1, Mypy still infers Any. And Pyright and pyrefly infer Literal[1].

We infer Literal[1] in both cases, which seems reasonable to me. Final should take precedence and we should not union with Unknown here.

Do we have any tests for that?

No, added! Thanks.

Does the spec say anything about it?

According to the type annotation syntax, both forms should be allowed.

It would be pretty unusual to see something like that since a Final declaration in a class body usually implies that it is also a ClassVar

... because you would expect to see the assignment right there in the class body? Right..

Regarding dataclasses: this is what the spec says:

Interesting! This behavior is certainly not yet implemented, so this will require some more changes. I'll add tests for this (and an implementation, if it's easy) in another PR. It's not really related to the change here.

@AlexWaygood
Copy link
Member

Regarding dataclasses: this is what the spec says:

Interesting! This behavior is certainly not yet implemented, so this will require some more changes. I'll add tests for this (and an implementation, if it's easy) in another PR. It's not really related to the change here.

I know -- but this PR reminded me of it, and it's certainly related to the theme of your work today ;)

@sharkdp sharkdp merged commit e7fb368 into main Jul 7, 2025
36 checks passed
@sharkdp sharkdp deleted the david/bare-classvar-annotation branch July 7, 2025 13:04
UnboundVariable pushed a commit to UnboundVariable/ruff that referenced this pull request Jul 7, 2025
…c_tokens

* 'main' of https://github.com/astral-sh/ruff: (27 commits)
  [ty] First cut at semantic token provider (astral-sh#19108)
  [`flake8-simplify`] Make example error out-of-the-box (`SIM116`) (astral-sh#19111)
  [`flake8-use-pathlib`] Make example error out-of-the-box (`PTH210`) (astral-sh#19189)
  [`flake8-use-pathlib`] Add autofixes for `PTH203`, `PTH204`, `PTH205` (astral-sh#18922)
  [`flake8-type-checking`] Fix syntax error introduced by fix (`TC008`) (astral-sh#19150)
  [`flake8-pyi`] Make example error out-of-the-box (`PYI007`, `PYI008`) (astral-sh#19103)
  Update Rust crate indicatif to 0.18.0 (astral-sh#19165)
  [ty] Add separate CI job for memory usage stats (astral-sh#19134)
  [ty] Add documentation for server traits (astral-sh#19137)
  Rename to `SessionSnapshot`, move unwind assertion closer (astral-sh#19177)
  [`flake8-type-checking`] Make example error out-of-the-box (`TC001`) (astral-sh#19151)
  [ty] Bare `ClassVar` annotations (astral-sh#15768)
  [ty] Re-enable multithreaded pydantic benchmark (astral-sh#19176)
  [ty] Implement equivalence for protocols with method members (astral-sh#18659)
  [ty] Use RHS inferred type for bare `Final` symbols (astral-sh#19142)
  [ty] Support declaration-only attributes (astral-sh#19048)
  [ty] Sync vendored typeshed stubs (astral-sh#19174)
  Update dependency pyodide to ^0.28.0 (astral-sh#19164)
  Update NPM Development dependencies (astral-sh#19170)
  Update taiki-e/install-action action to v2.56.7 (astral-sh#19169)
  ...
@sharkdp
Copy link
Contributor Author

sharkdp commented Jul 8, 2025

Regarding dataclasses: this is what the spec says:

Interesting! This behavior is certainly not yet implemented, so this will require some more changes. I'll add tests for this (and an implementation, if it's easy) in another PR. It's not really related to the change here.

I know -- but this PR reminded me of it, and it's certainly related to the theme of your work today ;)

I don't know what I read yesterday, but now that I read it again, that all seems very logical — and in fact we already implement everything correctly. Added some tests here: #19202

sharkdp added a commit that referenced this pull request Jul 8, 2025
## Summary

Adds some tests for dataclass fields that are annotated with `Final`
(see comment
[here](#15768 (comment))).
Turns out that nothing is needed here, everything already works as
expected (apart from the fact that we can assign to `Final` fields,
which is tracked in astral-sh/ty#158

## Test Plan

New Markdown tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bare ClassVar annotations
2 participants