astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
29.91k stars 979 forks source link

RUF012 false positive with annotations set as strings #12288

Open ZeeD opened 1 month ago

ZeeD commented 1 month ago

minimal case:

from typing import ClassVar
from typing import Final

class C:
    cv_orig: ClassVar[list[int]] = []
    cv_str: 'ClassVar[list[int]]' = []

    cv_final_orig: Final[list[int]] = []
    cv_final_str: 'Final[list[int]]' = []

    use_case: 'Final[list[C]]' = []

with ruff 0.5.1 I got the error

RUF012 Mutable class attributes should be annotated with `typing.ClassVar`

for cv_str, cv_final_str and use_case (that was my initial scenario...)

dhruvmanila commented 1 month ago

I think this is the case for other rules as well like RUF008, RUF009, etc.

charliermarsh commented 1 month ago

👍 Yeah we would need to parse these. I wonder if we should consider parsing these in the parser and adding them to the AST...

charliermarsh commented 1 month ago

Actually, we probably can't do that because you often need semantic information to know if a string is a type annotation.

dhruvmanila commented 1 month ago

We do parse it: https://github.com/astral-sh/ruff/blob/ecd6865d2800a574baf122583b6e463605af2ef5/crates/ruff_python_parser/src/typing.rs; I guess we need to provide some API to resolve it to the AST node if the model is in a type annotation context.

charliermarsh commented 1 month ago

Yeah we just parse it much later, at the end IIRC.

charliermarsh commented 1 month ago

Perhaps we could parse these on-demand and store them on the semantic model.

charliermarsh commented 1 month ago

I guess that might not work because in theory these need to be evaluated lazily, at the very end (e.g., they can reference symbols that are imported afterwards, IIRC).

dhruvmanila commented 1 month ago

At least for these 3 rules, they all require a class node which means we can store them in Analyze struct (https://github.com/astral-sh/ruff/blob/ecd6865d2800a574baf122583b6e463605af2ef5/crates/ruff_linter/src/checkers/ast/deferred.rs#L30-L37) and then later use deferred_class.rs (similar to deferred_lambdas.rs) to check for violations for these rules. We might want to update existing logic to not raise a violation if the annotation is a string.