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

Can't ignore reportUnreachable when typeCheckingMode == "standard" #590

Closed
jose-elias-alvarez opened this issue Aug 18, 2024 · 5 comments
Closed
Labels
awaiting response waiting for more info from the author - will eventually close if they don't respond type checking / linting issues relating to existing diagnostic rules or proposals for new diagnostic rules unreachable issues about unreachable code (which doesn't get typechecked)

Comments

@jose-elias-alvarez
Copy link

Given this script (playground):

import sys

if sys.version_info < (3, 11):
    sys.stderr.write("script requires Python 3.11 or newer!" + "\n") # pyright:ignore[reportUnreachable]
    sys.exit(1)

I'd expect that the code in the condition would not be marked as unreachable. In fact, when typeCheckingMode == "all", the code is not marked (playground).

Pyright seems to suffer from the same issue, both in standard and strict mode (playground), but it'd be a nice improvement if it did work here (unless I'm misunderstanding something about the setting).

@DetachHead
Copy link
Owner

i can't seem to reproduce your issue. in both those basedpyright playground links you sent, the # pyright:ignore[reportUnreachable] comment seems to correctly suppress the error.

(ideally you wouldn't need to suppress the error though in cases where you expect your code to be run on multiple python versions: #8)

@DetachHead DetachHead added unreachable issues about unreachable code (which doesn't get typechecked) type checking / linting issues relating to existing diagnostic rules or proposals for new diagnostic rules awaiting response waiting for more info from the author - will eventually close if they don't respond labels Aug 18, 2024
@jose-elias-alvarez
Copy link
Author

That's strange – on my end, this is what I see on the first playground link when I hover over the code in the condition:

Screenshot 2024-08-18 at 11 41 30 AM

Whereas in the second link, the code is not faded out, and nothing shows on hover:

Screenshot 2024-08-18 at 11 43 21 AM

Is that not the case for you? It's consistent with what I see in the editor, too.

@KotlinIsland KotlinIsland added needs investigation awaiting verification by a maintainer that the issue is valid and removed awaiting response waiting for more info from the author - will eventually close if they don't respond labels Aug 18, 2024
@KotlinIsland
Copy link
Collaborator

KotlinIsland commented Aug 18, 2024

I believe the issue is that the ignore in the first example is invalid, and there is no way to suppress the unreachable effect

Additionally, with all the unreachable effect is not applied at all

@DetachHead
Copy link
Owner

I believe the issue is that the ignore in the first example is invalid, and there is no way to suppress the unreachable effect

weird, there seems to be some intermittent issue with the playground that prevented the unreachable diagnostic from appearing in some cases, which is why i thought both those playground links were showing the same thing. (DetachHead/basedpyright-playground#21)

Additionally, with all the unreachable effect is not applied at all

this is intentional and caused by #326. since "unreachable" is now a separate diagnostic level and an error can't be reported with multiple levels at a time.


as for why the # pyright:ignore[reportUnreachable] comment doesn't suppress diagnostics with the unreachable/deprecated/unused levels, that's probably a result of them not actually being an error but rather a "hint" displayed in the IDE. while this decision came from upstream, the way i see it, since the "hint" is not actually an error it doesn't really make sense to suppress it. i think our solution to allow users to report unreachable code as an error or turn it off entirely adequately solves this problem, but i'm open to discussion if you disagree.

you can turn off reportUnreachable entirely if you want by setting it to false or "none"

@DetachHead DetachHead added awaiting response waiting for more info from the author - will eventually close if they don't respond and removed needs investigation awaiting verification by a maintainer that the issue is valid labels Aug 19, 2024
@jose-elias-alvarez
Copy link
Author

Thanks for looking into it – it sounds like a complicated situation. The hint is pretty unobtrusive in VS Code, but in Neovim and Helix, it's treated the same as any other diagnostic, so it's a bit harder to ignore (it's possible to configure the editor itself to suppress it, but that's not ideal, either).

It'd be nice to tell the language server "I know you think this code is unreachable, but it's actually fine" and keep reporting other cases that haven't specifically been approved, but from my end, just turning off the rule is fine (and #8 would resolve this specific issue in an even better way). Feel free to close this!

@DetachHead DetachHead closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response waiting for more info from the author - will eventually close if they don't respond type checking / linting issues relating to existing diagnostic rules or proposals for new diagnostic rules unreachable issues about unreachable code (which doesn't get typechecked)
Projects
None yet
Development

No branches or pull requests

3 participants