-
Notifications
You must be signed in to change notification settings - Fork 767
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
contextlib.suppress results in following code being marked as unreachable #494
Comments
@erictraut I may be misremembering, but I had thought the code flow analysis had been modified to treat context managers with an |
Yeah, that's not the case. That's a very large change and will have impacts on perf and (potentially) correctness in some cases. It effectively means that every Since this convention was never ratified in a PEP — and because of the tradeoffs it entails, I haven't implemented it. |
If the grayed-out code bothers users, there is an easy workaround — use a try/catch block rather than relying on a context manager to handle exceptions. |
The catch here to me is that because the code flow is "proving" that the code below is unreachable, it's not just a visual problem but impacts the code below. It was my impression that LSP features, inference, additional flow, etc, inside grayed-out code is limited because the analysis believes the code to be dead. |
Yeah, there are potentially false negatives — actual errors in that code that are not reported. In general, false positives are worse than false negatives, but I agree that both are undesirable. |
I'll mark it as an enhancement for now; I think there's some discussion across type checkers to be had here as to what is expected from them, especially if this isn't in any specification. |
Adding a "Me too" here. and yes using a try/except is a workaround, but just dropping exceptions with |
This will be addressed in the next release. |
This issue has been fixed in version 2020.12.1, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#2020121-9-december-2020 |
This issue just came back for me with version I downloaded the VSIX file for the last 5 versions and from |
@Cielquan, I agree with you. It reappeared in version |
Thanks for the bug report. You are correct, I introduced a regression in this code when fixing another bug. The nature of the regression was such that it eluded our unit tests. I've fixed the regression and improved our unit tests so this won't happen again in the future. This will be fixed in the next release. Apologies for any inconvenience. |
This issue has been fixed in version 2021.1.2, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202112-20-january-2021 |
Environment data
Expected behaviour
Using
contextlib.suppress
should not result in following code being marked as unreachable.Actual behaviour
Code following the suppress context block is marked as unreachable if there is a
return
statement in the block.Code Snippet / Additional information
Logs
The text was updated successfully, but these errors were encountered: