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

Provide a way to disable unreachability hints #6106

Closed
SmartLamScott opened this issue Jul 8, 2024 · 12 comments
Closed

Provide a way to disable unreachability hints #6106

SmartLamScott opened this issue Jul 8, 2024 · 12 comments
Assignees
Labels
enhancement New feature or request fixed in next version (release) A fix has been implemented and will appear in an upcoming version

Comments

@SmartLamScott
Copy link

SmartLamScott commented Jul 8, 2024

Environment data

  • Language Server version: 2024.6.104
  • OS and version: win32 x64
  • Python version (and distribution if applicable, e.g. Anaconda): Python 3.12.4
  • python.analysis.indexing: true
  • python.analysis.typeCheckingMode: off

Code Snippet

class MyClass(object):
    def __init__(self, settings: dict) -> None:
        if not isinstance(settings, dict):
            raise TypeError("settings must be of type dict.")

Repro Steps

  1. Write a function with a parameter that has a type hint.
  2. Write an if statement using not isinstance(parameters, hinted_type).
  3. Observe that the contents of the if statement are grayed out as if they cannot be executed. The are tagged with the hover text, "Code is unreachable".

Expected behavior

The body of the if statement should have the default Pylance colors. It should not be tagged with the hover text, "Code is unreachable". Pylance behaves correctly in v2024.6.1, but not pre-release v2024.6.104.

Actual behavior

The body of the if statement is grayed out. It is tagged with the hover text, "Code is unreachable". If the type in the hint and the isinstance statement are different, Pylance functions as expected.

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Jul 8, 2024
@erictraut
Copy link
Contributor

The current (new) behavior is correct. Reachability determination is based on type analysis. In your example, you've indicated that settings must be an instance of dict (or a subclass thereof). If you enable type checking diagnostics, you will see that this is enforced — i.e. you'll seen an error if you attempt to call the method with an argument of the wrong type. If settings is always a dict, then the if not instance(settings, dict) is always false, and you will never reach the raise statement.

If your intent is that settings can be something other than a dict, you should adjust the type annotation accordingly. For example: settings: dict | Any or settings: object.

@debonte debonte added by design and removed needs repro Issue has not been reproduced yet labels Jul 8, 2024
@debonte debonte closed this as completed Jul 8, 2024
@SmartLamScott
Copy link
Author

Thanks for the quick response, Eric. I'm glad to see that it isn't a bug, but I personally disagree with the design choice. I use isinstance calls in conjunction with type hinting to perform runtime type checks. My codebase contains dozens, if not hundreds of instances of this pattern. I'll then handle the result of the isinstance evaluation (almost always raising a TypeError) because it's still possible to call the __init__ function in my example using a parameter with an incorrect type. I would prefer if this is an option that could be toggled, not just a change in the update.

Is there a preferred way that I could enforce types in Python? I know that Python is not a statically typed language, nor do I need that feature. I just need to avoid runtime errors in the event that a user calls my function using an incorrectly typed parameter. I use Pylance only for type annotations and similar development features. Does it provide other capabilities to help me with types as the code runs?

@erictraut
Copy link
Contributor

erictraut commented Jul 8, 2024

Some static analysis tools (type checkers and linters) treat unreachable code as an error. Our philosophy with pyright & pylance is informed by the behavior of TypeScript, which doesn't treat unreachable code as an error. Instead, it treats it as potentially (but not necessarily) a bug. We therefore use "tagged hints" to tell VS Code to display these regions in a subtle gray color. As a developer, you can easily see that static analysis has determined that code is unreachable. You can then make a determination for yourself whether this is something that you expect or not. If not, you can investigate it as a potential bug. In your case, you would expect this to be unreachable from the perspective of a static code analyzer, so you can ignore the fact that it's in gray. In other cases, you may see something in gray and say to yourself, "hey, that looks wrong; I should investigate more closely".

This is the same philosophy pyright and pylance have been using since their inception. However, there was a bug that caused pyright to behave inconsistently and not detect unreachable code in certain cases. I recently fixed this bug, which explains why you saw a change in behavior.

If you don't want to see any of the "grayed out" text in your Python code, you can set "pyright.disableTaggedHints" to true in your VS Code settings.

@debonte
Copy link
Contributor

debonte commented Jul 8, 2024

you can set "pyright.disableTaggedHints" to true in your VS Code settings.

This setting is currently not supported in Pylance, but we could add it.

@debonte
Copy link
Contributor

debonte commented Jul 8, 2024

Related discussion: #5647

@bdpedigo
Copy link

bdpedigo commented Jul 9, 2024

I am in the same boat as @SmartLamScott: because of the lack of runtime type checking in Python, I have a ton of statements with this pattern where I check if a variable is not of an allowed type, and then do something.

I have found the "code is unreachable" greying out quite helpful for things like code after a return statement, but doing this inference based on unenforceable types feels fundamentally different in kind. I wonder if there is some way to treat these cases differently, or some other middle ground?

you can set "pyright.disableTaggedHints" to true in your VS Code settings.

This setting is currently not supported in Pylance, but we could add it.

If not, then I would greatly appreciate this feature for VS Code. Currently finding it very distracting to have large sections of code greyed out, but I otherwise love Pylance!

@debonte debonte added enhancement New feature or request and removed by design labels Jul 13, 2024
@debonte debonte changed the title If block contents are incorrectly grayed out as if they are inaccessible Provide a way to disable unreachability hints Jul 13, 2024
@debonte
Copy link
Contributor

debonte commented Jul 13, 2024

Reopening to track adding support for pyright.disableTaggedHints in Pylance.

@debonte debonte reopened this Jul 13, 2024
@nmoreaud
Copy link

Hello
Could you please add an option to disable the detection of code reachability when a variable is not marked as nullable? (equivalent to mypy's no-strict-optional option).
I've seen a lot of tickets about that (ex #6152 #6131 #5857 #5776...)

@erictraut
Copy link
Contributor

Pyright (the type checker that underlies pylance) offers a backward-compatibility option called strictParameterNoneValue. This is roughly equivalent to no-strict-optional in mypy, although it affects only function parameters, not variables in general. When set to false, this option allows parameters with None default values to be treated as though their types are implicitly unioned with None. You can specify this in a pyproject.toml or pyrightconfig.json file. However, I'd recommend against doing this because it violates the Python static typing spec. It is better to update your type annotations so they are accurate and correct.

@bschnurr bschnurr added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Jul 23, 2024
@heejaechang heejaechang added fixed in next version (release) A fix has been implemented and will appear in an upcoming version and removed fixed in next version (main) A fix has been implemented and will appear in an upcoming version labels Jul 23, 2024
@heejaechang
Copy link
Contributor

This issue has been fixed in prerelease version 2024.7.102, which we've just released. You can find the changelog here: CHANGELOG.md

@SmartLamScott
Copy link
Author

Which setting should I change to disable unreachability hints?

@rchiodo
Copy link
Contributor

rchiodo commented Jul 25, 2024

Which setting should I change to disable unreachability hints?

python.analysis.disableTaggedHints.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed in next version (release) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

8 participants