-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Avoid treating pass and ... as no-op for reachability
#20488
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
Conversation
Fixes python#16232 The inclusion of these statements dates back to the original `--warn-unreachable` PR (and doesn't provide justification) I've been advocating for more consistent heuristics for allowable unreachable code across type checkers and I can't come up with a good reason that this should be allowed
This comment has been minimized.
This comment has been minimized.
|
The aiohttp one seems kind of useful to know about, the other two are maybe a little annoying to see change on, but defensible |
|
Would half-fix #20462, & docunenting the consensus reached at microsoft/pyright#11041 would fix the other half. |
A5rocks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are (weak) justifications for these, but I agree with removing it.
Maybe we can remove the logic that checks multiple statements now? Maybe that's a followup.
JukkaL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test?
This looks good to me.
This comment has been minimized.
This comment has been minimized.
7e4ff0c to
773289b
Compare
|
Diff from mypy_primer, showing the effect of this PR on open source code: aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/client.py:870:17: error: Statement is unreachable [unreachable]
+ aiohttp/client.py:870:17: note: See https://mypy.rtfd.io/en/stable/_refs.html#code-unreachable for more info
Tanjun (https://github.com/FasterSpeeding/Tanjun)
+ tanjun/abc.py:2774: error: Statement is unreachable [unreachable]
pytest (https://github.com/pytest-dev/pytest)
+ testing/code/test_code.py:151: error: Statement is unreachable [unreachable]
+ testing/code/test_code.py:167: error: Statement is unreachable [unreachable]
|
There is a special case about unreachability, `return NotImplemented`, which we did not until now mention, which we hereby now mention in the documentation of `--warn-unreachable`. I also add a test to test this claimed behavior thoroughly. The current test tests its desired behavior, in a class, but the actual behavior of mypy is much broader than that, and applies all the time, it seems. Fixes #20462. That issue is not correct anymore, as it was obsoleted by #20488. However, during its discussion @sterliakov brought up this `NotImplemented` edge case as something that should be documented. I think the documentation after this PR will be satisfactory by everyone's standards. Similarly, this PR closes #20558. Theoretically, there could be further special cases of unreachability of which I am not aware, but these can be noted by later PRs if so.
Fixes #16232
The inclusion of these statements dates back to the original
--warn-unreachablePR (and doesn't provide justification)I've been advocating for more consistent heuristics for allowable unreachable code across type checkers and I can't come up with a good reason that this should be allowed