`reportUnannotatedClassAttribute` false positive on non-declarations

[DetachHead]

Description

Code sample in basedpyright playground

class Foo:
    asdf: int = 1

class Bar(Foo):
    def foo(self):
        self.asdf = 1 # error: reportUnannotatedClassAttribute

[thomas-mckay]

Same for me with this simple code (no inheritance):

class Foo:
    def __init__(self, bar: str) -> None:
        self.bar = bar  # Type annotation for attribute `bar` is required because this class is not decorated with `@final`  (reportUnannotatedClassAttribute)

[KotlinIsland]

Same for me with this simple code (no inheritance):

class Foo: def init(self, bar: str) -> None: self.bar = bar

@thomas-mckay that's intentional, see the waring here: https://docs.basedpyright.com/latest/benefits-over-pyright/new-diagnostic-rules/#reportincompatibleunannotatedoverride

you can try disabling reportUnannotatedClassAttribute, and enable reportincompatibleunannotatedoverride instead

@DetachHead maybe we want to update the error message to mention the other rule? i don't think it's currently super discoverable

[DetachHead]

we should probably just switch to having reportIncompatibleUnannotatedOverride enabled by default instead. i've been using it for ages and haven't noticed any performance issues with it

[thomas-mckay]

@thomas-mckay that's intentional, see the waring here: https://docs.basedpyright.com/latest/benefits-over-pyright/new-diagnostic-rules/#reportincompatibleunannotatedoverride

you can try disabling reportUnannotatedClassAttribute, and enable reportincompatibleunannotatedoverride instead

@DetachHead maybe we want to update the error message to mention the other rule? i don't think it's currently super discoverable

I'm sorry, I don't understand why reportUnannotatedClassAttribute refers to a class attribute, but the rule flags an instance attribute (declared in self). I haven't followed the history of why this rule was implemented in the first place, but from an external POV, either the name or the intent is unclear (I have read the doc, including the one you linked, but I'm still not sure why this example is flagged). Sorry if I'm being daft or not seeing something obvious (it's very late) :)

[DetachHead]

i don't remember why i called it that. reportUnannotatedAttribute probably would've been a better name

[thomas-mckay]

Thanks for the answers. I agree that a name change could make it clearer. Though I'm still not quite sure why, in the example I posted, self.bar isn't considered annotated simply by inference from the __init__ signature. It's just my POV, but it seems redundant to have to write

class Foo:
    def __init__(self, bar: str) -> None:
        self.bar: str = bar

when

class Foo:
    def __init__(self, bar: str) -> None:
        self.bar = bar

class FooChild(Foo):
    def __init__(self, bar: int) -> None:     # should be a violation regardless (signature doesn't match parent class)
        super().__init__(bar)   # should be a violation regardless (invalid argument type: expected str)

I freely admit, I'm not privy enough to the inner workings of the type-checker to see the full picture, so I may be completely off-base here. If there is a good reason I'm not seeing to force annotation of instance attributes declared in the __init__, then if I may make a suggestion, would it be possible to exclude "private" attributes from this rule:

class Foo:
    def __init__(self, bar: str, baz: int) -> None:
        self.bar = bar  # flagged
        self._bat = baz  # not flagged
        self._baz = 5.0  # not flagged

PS: It may very well be that, in my case, reportIncompatibleUnannotatedOverride is enough and I should just disable reportUnannotatedClassAttribute (though from the name alone, it sounded like something I wanted to enforce).

[DetachHead]

reportUnannotatedClassAttribute and reportIncompatibleUnannotatedOverride are just two different solutions to the same problem:

from typing import reveal_type


class Foo:
    def __init__(self, bar: str) -> None:
        self.bar = bar # error here if reportUnannotatedClassAttribute is enabled

class FooChild(Foo):
    def __init__(self) -> None:
        super().__init__("")
        self.bar = 1 # error here if reportIncompatibleUnannotatedOverride is enabled

def fn(foo: Foo):
    reveal_type(foo.bar) # basedpyright: str, runtime: int

fn(FooChild())

(playground)

it's unlikely you'd want both of them enabled. the only reason i made reportUnannotatedClassAttribute was because i was afraid to add reportIncompatibleUnannotatedOverride initially because i saw a comment in the pyright codebase that said such a check doesn't exist for performance reasons.

so which one should you use?

my advice is:

use reportIncompatibleUnannotatedOverride if...

• you're developing an application ie. will only ever be used with the type checker used in the project (basedpyright)

use reportUnannotatedClassAttribute if...

• you're developing a library that may be used by people with pyright instead of basedpyright, because it prevents you from creating classes with attributes that users can unsafely override since they don't have the reportIncompatibleUnannotatedOverride rule to warn them against doing it.
• or you notice basedpyright running slower with reportIncompatibleUnannotatedOverride enabled

[thomas-mckay]

reportUnannotatedClassAttribute and reportIncompatibleUnannotatedOverride are just two different solutions to the same problem:

from typing import reveal_type

class Foo: def init(self, bar: str) -> None: self.bar = bar # error here if reportUnannotatedClassAttribute is enabled

class FooChild(Foo): def init(self) -> None: super().init("") self.bar = 1 # error here if reportIncompatibleUnannotatedOverride is enabled

def fn(foo: Foo): reveal_type(foo.bar) # basedpyright: str, runtime: int

fn(FooChild())

(playground)

it's unlikely you'd want both of them enabled. the only reason i made reportUnannotatedClassAttribute was because i was afraid to add reportIncompatibleUnannotatedOverride initially because i saw a comment in the pyright codebase that said such a check doesn't exist for performance reasons.

so which one should you use?

my advice is:

use reportIncompatibleUnannotatedOverride if...

* you're developing an application ie. will only ever be used with the type checker used in the project (basedpyright)

use reportUnannotatedClassAttribute if...

* you're developing a library that may be used by people with pyright instead of basedpyright, because it prevents you from creating classes with attributes that users can unsafely override since they don't have the `reportIncompatibleUnannotatedOverride` rule to warn them against doing it.

* or you notice basedpyright running slower with `reportIncompatibleUnannotatedOverride` enabled

Thank you. I understand what the rule is trying to prevent at the instance level now (that last example helped). I still think the double annotation it forces is not ideal (and I'm not sure it should be encouraged), but I understand the reason behind it.

If I may suggest, you could add the last example to the doc. Short of changing the name of the rule, it might help others like me have more context.

PS: BTW, thank you for basedpyright. I use it as an LSP with zed. It's plenty fast and I love it.