no error when setting `init=False` on `Final` field in dataclass

[DetachHead]

from typing import Final
from dataclasses import dataclass, field

@dataclass
class Foo:
    a: Final[int] = field(init=False) # no error

Foo().a # runtime error

playground

related: #13118

[odesenfans]

There is no error either when the member is not final. IMO the whole premise is dubious when it comes to OO, having a parameter that you cannot initialize through the constructor and that has no value will result in an incomplete object.

The solution could be to reject fields where init=False and the user does not specify a default value.

[odesenfans]

Just checked, the error also occurs for normal classes:

class Bar:
    b: int
    def __init__(self):
        ...

Bar().b  # no error with mypy, obvious error at runtime

[DetachHead]

i guess that's the same issue as #686

[odesenfans]

i guess that's the same issue as https://github.com/python/mypy/issues/686

I don't think it is, this issue has to do with mypy being unable to determine if this variable can be initialized.

I tried to fix this issue but it seems not trivial to resolve properly. Use cases like the one below show that the field can be initialized in methods like __post_init__.

[case testDataclassesInitVarPostInitOverride]
@dataclasses.dataclass
class A:
    a: dataclasses.InitVar[int]
    _a: int = dataclasses.field(init=False)

    def __post_init__(self, a: int) -> None:
        self._a = a

[JukkaL]

What if we'd only flag the error if there is no __post_init__? It's not used very commonly, so we might still be able to handle a decent fraction of all use cases.

[odesenfans]

I checked, but there are other use cases like this one:

[case testDataclassesInitVarOverride]
# flags: --python-version 3.7
import dataclasses

@dataclasses.dataclass
class A:
    a: dataclasses.InitVar[int]
    _a: int = dataclasses.field(init=False)

    def __post_init__(self, a):
        self._a = a

@dataclasses.dataclass(init=False)
class B(A):
    b: dataclasses.InitVar[int]
    _b: int = dataclasses.field(init=False)

    def __init__(self, b):
        super().__init__(b+1)
        self._b = b

which are considered legitimate code (here: overriding __init__ in the dataclass). I don't know how easy/hard it is to check inside __init__ and __post_init__ to see if attributes are correctly assigned.

[jakezych]

Hi! I'm interested in working on this issue. I've spent some time trying to familiarize myself with dataclasses.py and default.py as I think this is where most of the implementation would be.

I'm thinking that in collect_attributes, we extract the init argument and its value in is_in_init, but I'm not exactly sure where we can access all of the statements belonging to the __init__ or __post_init__ method to check if the attribute is actually being initialized. Would that also be in the for loop on line 415?

Also, I'm also not exactly sure where the actual error would get thrown. I noticed that Plugin base class has a method get_attribute_hook which could be implemented in default.py to return a new callback method written in dataclasses.py that could throw the error when an attribute that has not been initialized is accessed. Is this the right idea?

I'm new to this project so any guidance on this issue would be super helpful, thank you!

[KotlinIsland]

@odesenfans I think the difference is that in the OP Final is used, in your examples you are omitting the Final.

I found another case of high suspicion:

from typing import Final
from dataclasses import dataclass, field

@dataclass
class Bar:
    a: Final[int] = field()
    b: Final[int]

    def __init__(self) -> None:
        self.a = 1  # error: Cannot assign to final attribute "a" 
        self.b = 1

This error is a sus imposter false positive.

[JukkaL]

14285 is an attempt at this, but unfortunately it at least currently seems too complex to merge as is. It would be great if somebody can come up with a simpler way to implement this functionality. #14285 is still a useful first step and the test cases can be useful, in particular (remember to credit @jakezych if you reuse parts of the PR).

[ikonst]

I think @JukkaL just meant to check for __post_init__ existing (i.e. has_method which should take MRO into account) rather than analyzing its statements.