Fix strict optional handling in dataclasses #15571
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks @ilevkivskyi!
I do wonder though if we should completely remove the --strict-optional flag rather sooner than later. It's the default already and this type of issue here seems to come up often. Also a small performance improvement as state.strict_optional_set wouldn't be needed.
| @@ -2420,3 +2420,16 @@ class Test(Protocol): | |||
| def reset(self) -> None: | |||
| self.x = DEFAULT | |||
| [builtins fixtures/dataclasses.pyi] | |||
|
|
|||
| [case testStrictOptionalAlwaysSet] | |||
| # flags: --strict-optional | |||
There was a problem hiding this comment.
| # flags: --strict-optional |
--strict-optional is the default. So the flag wouldn't be necessary.
There was a problem hiding this comment.
--strict-optional is actually off by default in tests (at least for testcheck.py).
There was a problem hiding this comment.
But this actually reminded me that this test should use real stubs, so it should be moved to pythoneval.test.
There was a problem hiding this comment.
Interesting, before writing the initial comment, I removed the flag from the test case and it still passed 🤔
--strict-optionalis actually off by default in tests (at least fortestcheck.py).
I enabled it for pythoneval tests in #15474, seems I should work on the rest as well.
There was a problem hiding this comment.
Interesting, before writing the initial comment, I removed the flag from the test case and it still passed
This is exactly why I moved it to pythoneval, otherwise it doesn't really test much.
This is an important flag. We can remove it at some point, but I don't think we are there yet. I bet multiple large mypy users still use it (likely on per-file basis), at least this is what I can say from recent personal experience. |
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
There were few cases when someone forgot to call
strict_optional_set()in dataclasses plugin, let's move the calls directly to two places where they are needed for typeops. This may cause a tiny perf regression, but is much more robust in terms of preventing bugs.