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

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

Open
DetachHead opened this issue Jul 14, 2022 · 10 comments · May be fixed by #14285
Open

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

DetachHead opened this issue Jul 14, 2022 · 10 comments · May be fixed by #14285

Comments

@DetachHead
Copy link
Contributor

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

@DetachHead DetachHead added the bug mypy got something wrong label Jul 14, 2022
@JelleZijlstra JelleZijlstra added feature and removed bug mypy got something wrong labels Jul 14, 2022
@odesenfans
Copy link

odesenfans commented Jul 16, 2022

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
Copy link

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
Copy link
Contributor Author

i guess that's the same issue as #686

@odesenfans
Copy link

i guess that's the same issue as #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
Copy link
Collaborator

JukkaL commented Jul 16, 2022

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
Copy link

odesenfans commented Jul 17, 2022

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
Copy link

jakezych commented Dec 6, 2022

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
Copy link
Contributor

KotlinIsland commented Dec 6, 2022

@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
Copy link
Collaborator

JukkaL commented Apr 22, 2023

#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
Copy link
Contributor

ikonst commented Apr 22, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants