mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Fixes no error thrown for Final fields declared with init=False and not assigned inside __init__

Open jakezych opened this issue 3 years ago • 11 comments

Fixes #13119

For all final field assignments to class variables in a dataclass i.e. x: Final[int] = field(), this PR checks whether this field is being properly assigned in the __init__ or __post_init__ methods. In the case where the field has the argument init=True, no error is thrown if the field is assigned inside __init__ or __post_init__ which was previously a false positive. This addresses the last example given in the issue thread.

In the case where the field has the argument init=False and it is not being assigned in __init__ or __post_init__, on the line where it is a declared, the error "Final name must be initialized with a value" is now thrown. A new error was added and is thrown if that same field is accessed through the class "Final field "a" not set". This addresses the original false negative given in the issue thread.

Tests Added (all in check-dataclasses.test): testErrorFinalFieldNoInitNoArgumentPassed testNoErrorFinalFieldDelayedInit testErrorFinalFieldInitNoArgumentPassed testFinalFieldGeneratedInitArgumentPassed testFinalFieldInit testFinalFieldPostInit

testFinalDefiningNoRhs in check-final.test was modified to now throw the new error when an unset final attribute is accessed

jakezych avatar Dec 12 '22 02:12 jakezych

Diff from mypy_primer, showing the effect of this PR on open source code:

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/helpers.py:181: error: Final field "user" not set  [misc]
+ aiohttp/helpers.py:183: error: Final field "user" not set  [misc]
+ aiohttp/helpers.py:183: error: Final field "password" not set  [misc]
+ aiohttp/helpers.py:258: error: Final field "scheme" not set  [misc]
+ aiohttp/helpers.py:260: error: Final field "scheme" not set  [misc]
+ aiohttp/helpers.py:265: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:266: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:280: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:281: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:285: error: Final field "scheme" not set  [misc]
+ aiohttp/cookiejar.py:160: error: Final field "raw_host" not set  [misc]
+ aiohttp/cookiejar.py:200: error: Final field "path" not set  [misc]
+ aiohttp/cookiejar.py:251: error: Final field "raw_host" not set  [misc]
+ aiohttp/cookiejar.py:257: error: Final field "scheme" not set  [misc]
+ aiohttp/cookiejar.py:279: error: Final field "path" not set  [misc]
+ aiohttp/connector.py:983: error: Final field "scheme" not set  [misc]
+ aiohttp/connector.py:1086: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:226: error: Final field "query" not set  [misc]
+ aiohttp/client_reqrep.py:228: error: Final field "query" not set  [misc]
+ aiohttp/client_reqrep.py:266: error: Final field "scheme" not set  [misc]
+ aiohttp/client_reqrep.py:291: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:297: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:307: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:311: error: Final field "user" not set  [misc]
+ aiohttp/client_reqrep.py:311: error: Final field "password" not set  [misc]
+ aiohttp/client_reqrep.py:335: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:338: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:339: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:563: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:567: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:571: error: Final field "raw_path" not set  [misc]
+ aiohttp/client_reqrep.py:572: error: Final field "raw_query_string" not set  [misc]
+ aiohttp/client_reqrep.py:573: error: Final field "raw_query_string" not set  [misc]
+ aiohttp/client_reqrep.py:729: error: Final field "host" not set  [misc]
+ aiohttp/client_reqrep.py:730: error: Final field "host" not set  [misc]
+ aiohttp/client.py:323: error: Final field "path" not set  [misc]
+ aiohttp/client.py:590: error: Final field "scheme" not set  [misc]
+ aiohttp/client.py:598: error: Final field "host" not set  [misc]
+ aiohttp/client.py:599: error: Final field "scheme" not set  [misc]
+ aiohttp/client.py:600: error: Final field "scheme" not set  [misc]
+ aiohttp/web_request.py:181: error: Final field "host" not set  [misc]
+ aiohttp/web_request.py:182: error: Final field "scheme" not set  [misc]
+ aiohttp/web_request.py:450: error: Final field "path" not set  [misc]
+ aiohttp/web_request.py:473: error: Final field "query" not set  [misc]
+ aiohttp/web_request.py:481: error: Final field "query_string" not set  [misc]
+ aiohttp/web_urldispatcher.py:350: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:617: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:736: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:737: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:795: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:796: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:798: error: Final field "port" not set  [misc]
+ aiohttp/web_urldispatcher.py:799: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:800: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:800: error: Final field "port" not set  [misc]
+ aiohttp/web_urldispatcher.py:1187: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:1191: error: Final field "path" not set  [misc]

discord.py (https://github.com/Rapptz/discord.py)
+ discord/utils.py:834: error: Final field "parts" not set  [misc]
+ discord/utils.py:835: error: Final field "query" not set  [misc]
+ discord/asset.py:173: error: Final field "name" not set  [misc]
+ discord/asset.py:399: error: Final field "path" not set  [misc]
+ discord/asset.py:420: error: Final field "raw_query_string" not set  [misc]
+ discord/asset.py:484: error: Final field "path" not set  [misc]
+ discord/asset.py:485: error: Final field "raw_query_string" not set  [misc]

pytest (https://github.com/pytest-dev/pytest)
+ testing/_py/test_local.py:850: error: Final field "strpath" not set  [misc]
+ testing/_py/test_local.py:851: error: Final field "strpath" not set  [misc]

yarl (https://github.com/aio-libs/yarl)
+ tests/test_url.py:202: error: Final field "raw_authority" not set  [misc]
+ tests/test_url.py:203: error: Final field "authority" not set  [misc]
+ tests/test_url.py:208: error: Final field "raw_authority" not set  [misc]
+ tests/test_url.py:213: error: Final field "raw_authority" not set  [misc]
+ tests/test_url.py:217: error: Final field "authority" not set  [misc]

github-actions[bot] avatar Dec 12 '22 03:12 github-actions[bot]

From the mypy primer output it seems like you need to exclude class definitions in stub files from your check.

tmke8 avatar Dec 12 '22 11:12 tmke8

I think I fixed the issue. Could someone rerun the workflows?

jakezych avatar Dec 15 '22 15:12 jakezych

Diff from mypy_primer, showing the effect of this PR on open source code:

yarl (https://github.com/aio-libs/yarl)
+ tests/test_url.py:202: error: Final field "raw_authority" not set  [misc]
+ tests/test_url.py:203: error: Final field "authority" not set  [misc]
+ tests/test_url.py:208: error: Final field "raw_authority" not set  [misc]
+ tests/test_url.py:213: error: Final field "raw_authority" not set  [misc]
+ tests/test_url.py:217: error: Final field "authority" not set  [misc]

discord.py (https://github.com/Rapptz/discord.py)
+ discord/utils.py:834: error: Final field "parts" not set  [misc]
+ discord/utils.py:835: error: Final field "query" not set  [misc]
+ discord/asset.py:173: error: Final field "name" not set  [misc]
+ discord/asset.py:399: error: Final field "path" not set  [misc]
+ discord/asset.py:420: error: Final field "raw_query_string" not set  [misc]
+ discord/asset.py:484: error: Final field "path" not set  [misc]
+ discord/asset.py:485: error: Final field "raw_query_string" not set  [misc]

pytest (https://github.com/pytest-dev/pytest)
+ testing/_py/test_local.py:850: error: Final field "strpath" not set  [misc]
+ testing/_py/test_local.py:851: error: Final field "strpath" not set  [misc]

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/helpers.py:181: error: Final field "user" not set  [misc]
+ aiohttp/helpers.py:183: error: Final field "user" not set  [misc]
+ aiohttp/helpers.py:183: error: Final field "password" not set  [misc]
+ aiohttp/helpers.py:258: error: Final field "scheme" not set  [misc]
+ aiohttp/helpers.py:260: error: Final field "scheme" not set  [misc]
+ aiohttp/helpers.py:265: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:266: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:280: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:281: error: Final field "host" not set  [misc]
+ aiohttp/helpers.py:285: error: Final field "scheme" not set  [misc]
+ aiohttp/cookiejar.py:160: error: Final field "raw_host" not set  [misc]
+ aiohttp/cookiejar.py:200: error: Final field "path" not set  [misc]
+ aiohttp/cookiejar.py:251: error: Final field "raw_host" not set  [misc]
+ aiohttp/cookiejar.py:257: error: Final field "scheme" not set  [misc]
+ aiohttp/cookiejar.py:279: error: Final field "path" not set  [misc]
+ aiohttp/connector.py:983: error: Final field "scheme" not set  [misc]
+ aiohttp/connector.py:1086: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:226: error: Final field "query" not set  [misc]
+ aiohttp/client_reqrep.py:228: error: Final field "query" not set  [misc]
+ aiohttp/client_reqrep.py:266: error: Final field "scheme" not set  [misc]
+ aiohttp/client_reqrep.py:291: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:297: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:307: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:311: error: Final field "user" not set  [misc]
+ aiohttp/client_reqrep.py:311: error: Final field "password" not set  [misc]
+ aiohttp/client_reqrep.py:335: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:338: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:339: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:563: error: Final field "raw_host" not set  [misc]
+ aiohttp/client_reqrep.py:567: error: Final field "port" not set  [misc]
+ aiohttp/client_reqrep.py:571: error: Final field "raw_path" not set  [misc]
+ aiohttp/client_reqrep.py:572: error: Final field "raw_query_string" not set  [misc]
+ aiohttp/client_reqrep.py:573: error: Final field "raw_query_string" not set  [misc]
+ aiohttp/client_reqrep.py:729: error: Final field "host" not set  [misc]
+ aiohttp/client_reqrep.py:730: error: Final field "host" not set  [misc]
+ aiohttp/client.py:323: error: Final field "path" not set  [misc]
+ aiohttp/client.py:590: error: Final field "scheme" not set  [misc]
+ aiohttp/client.py:598: error: Final field "host" not set  [misc]
+ aiohttp/client.py:599: error: Final field "scheme" not set  [misc]
+ aiohttp/client.py:600: error: Final field "scheme" not set  [misc]
+ aiohttp/web_request.py:181: error: Final field "host" not set  [misc]
+ aiohttp/web_request.py:182: error: Final field "scheme" not set  [misc]
+ aiohttp/web_request.py:450: error: Final field "path" not set  [misc]
+ aiohttp/web_request.py:473: error: Final field "query" not set  [misc]
+ aiohttp/web_request.py:481: error: Final field "query_string" not set  [misc]
+ aiohttp/web_urldispatcher.py:350: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:617: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:736: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:737: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:795: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:796: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:798: error: Final field "port" not set  [misc]
+ aiohttp/web_urldispatcher.py:799: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:800: error: Final field "raw_host" not set  [misc]
+ aiohttp/web_urldispatcher.py:800: error: Final field "port" not set  [misc]
+ aiohttp/web_urldispatcher.py:1187: error: Final field "raw_path" not set  [misc]
+ aiohttp/web_urldispatcher.py:1191: error: Final field "path" not set  [misc]

github-actions[bot] avatar Dec 15 '22 16:12 github-actions[bot]

I'll see if I can set up mypy primer locally to get a better idea of what's going wrong.

jakezych avatar Dec 15 '22 16:12 jakezych

I tested the changes locally on yarl and discord.py and it was not throwing the errors anymore.

jakezych avatar Dec 20 '22 01:12 jakezych

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Dec 20 '22 02:12 github-actions[bot]

If the mistake is in a third-party library, does it still show an error in my own code? So, for example:

some_library/__init__.py:

from dataclasses import dataclass, field
from typing import Final

@dataclass
class C:
    x: Final[int] = field(init=False)

    def __post_init__(self) -> None:
        self.y = 3  # typo here

my_code.py:

from some_library import C

C().x  # an error here?

tmke8 avatar Dec 20 '22 13:12 tmke8

~~I'm a bit confused about the self.y = 3 #typo here line. Shouldn't it be okay to assign a final field in the __post_init__ method?~~

I'm silly and didn't read the difference between x and y.

I do understand your main point here though, and I'll take a look at how my code handles errors in imported libraries.

jakezych avatar Dec 20 '22 16:12 jakezych

It does show errors from third-party libraries, I assume we want to suppress those?

jakezych avatar Dec 20 '22 16:12 jakezych

It does show errors from third-party libraries, I assume we want to suppress those?

Nevermind actually, I realized that a lot of other mypy checks have the same problem. So it should be fine!

tmke8 avatar Dec 22 '22 09:12 tmke8

This PR is going in the right direction, but it feels to me like the implementation is too complicated, especially considering this is a relatively minor edge case. In particular, iterating over the body of a function in the dataclass plugin is not the best way to approach this, and it seems to duplicate functionality in the semantic analyzer which will make things harder to maintain and understand. Also I'd prefer it checkmember.py wouldn't need to be changed.

JukkaL avatar Apr 22 '23 13:04 JukkaL