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

How to disable "code is not reachable" warning and greying out the code? I find this feature to be unreliable quite often. #248

Closed
yury-tokpanov opened this issue Aug 16, 2020 · 49 comments
Labels
bug Something isn't working

Comments

@yury-tokpanov
Copy link

No description provided.

@yury-tokpanov yury-tokpanov changed the title How to disable "code is not reachable" greying out? Also, how to disable auto imports? How to disable "code is not reachable" warning and greying out the code? I find this feature to be unreliable quite often. Aug 16, 2020
@jakebailey
Copy link
Member

We don't have any way to disable that, no. Can you provide some examples of how this feature is unreliable?

For auto-imports, see: #64 (comment)

@jakebailey jakebailey added the waiting for user response Requires more information from user label Aug 17, 2020
@github-actions github-actions bot removed the triage label Aug 17, 2020
@yury-tokpanov
Copy link
Author

Thanks for the reply! Unfortunately, I cannot provide details, since I'm working on a proprietary code. I'm pretty sure "code is unreachable" warning is generated due to how the code is structured, and, unfortunately, pylance analyzer doesn't dig deep enough.

@amachanic
Copy link

Here is one example I'm hitting. We have a base class with a method that's expected to be overridden by children, and the base class also has another method that calls the (presumably overridden) base method. Herein lies the issue: PyLance seems to see our NotImplementedError on the base/non-overridden version of the method and doesn't realize that what we're referencing is in fact the overridden version. See the comment in function() for where I would expect things to properly work -- or to be able to disable the check so I don't have to see the greyed out code.

class MyBaseClass:
    @classmethod
    def get(cls):
         raise NotImplementedError

    @classmethod
    def get2(cls):
        blat = cls.get()
        # First hint: Code below here is "unreachable"
        # ... technically true out of context, but ...
        return blat

class MyClass(MyBaseClass):
    @classmethod
    def get(cls):
        return None

def function():
    r = MyClass.get2()
    # Anything from here down is "unreachable"
    return r

@erictraut
Copy link
Contributor

Thanks for providing the example. This is the most common cause of code being marked as unreachable. In your example, the get method in the MyBaseClass has no return type annotation, so Pylance is forced to infer its type based on code analysis. The method always raises an exception, and it's not properly marked as abstract (using an @abstractmethod decorator). So Pylance concludes that the return type is NoReturn.

There are two ways to work around this issue:

  1. Add a return type annotation to the get method so Pylance doesn't need to infer the return type.
  2. Mark MyBaseClass as abstract by deriving from abc.ABC and marking get as abstract using the @abc.abstractmethod decorator.

@yury-tokpanov
Copy link
Author

Thanks for a constructive reply!

We have a lot of similar to @amachanic 's situations, and it would be a pain to rework everything to accommodate PyLance reachability analyzer. I'd definitely prefer to just disable it for these kind of projects. I'm sure a lot of people would welcome an additional PyLance option to control this setting. :)

@erictraut
Copy link
Contributor

The reachability analyzer doesn't ever produce errors or warnings, so the worst thing that happens if you decide not to change your code is that you will see some text grayed out in your editor.

@amachanic
Copy link

Thanks @erictraut. I went with the return annotation and it did the trick. Probably should have had one on there anyway.

@yury-tokpanov, it took me less than five minutes to change a few impacted base classes, and it's a nearly zero-risk modification. I highly recommend giving it a shot if the gray text bothers you.

@yury-tokpanov
Copy link
Author

@erictraut Sometimes not just some text, but all the text, if Pylance didn't correctly analyze beginning portion of the code.

@amachanic There are too many cases in my project, and I don't feel saying "adding support for Pylance" is a good justification to other people working on it.

@sabbyX
Copy link

sabbyX commented Aug 21, 2020

Happens when you are using

if typing.TYPE_CHECKING:
    annotation = str
# below code is marked as "unreachable"
else:
    annotation = ... # some function, these are used for validating data in pydantic as 

I am forced to do like this as MyPy throws invalid annotation

@erictraut
Copy link
Contributor

Yes, this is as intended. Pylance is showing you the code that is unreachable from the perspective of its code analysis. The "else" clause in your example above should appear as gray. This gives you a visual clue that it is not being analyzed, and errors within this block will not be reported because it's conditionalized on TYPE_CHECKING.

Out of curiosity, why do you need to conditionalize this code? It should be fine to execute the statement annotation = str at runtime.

@sabbyX
Copy link

sabbyX commented Aug 23, 2020

Out of curiosity, why do you need to conditionalize this code? It should be fine to execute the statement annotation = str at runtime.

Pydantic uses annotations for validating data, and if you want to use custom validation for the field without making type-checkers screaming ;)

@krooq
Copy link

krooq commented Aug 26, 2020

If the library you are using doesn't use type hints then you are reduced to 2 options:

  1. use type hints in your code and deal with the greyed out text
  2. don't use the type hints in your code when referencing the library code

Seems a bit odd to me that Pylance would make an opinionated decision on this sort of thing.
Interested to know how Typescript handles this situation.

EDIT: There is actually a 3rd option
3. try/except around the calling code

@erictraut
Copy link
Contributor

erictraut commented Aug 26, 2020

Library code doesn't need to contain type annotations to avoid this condition. If it makes proper use of @abstractmethod decorators, that will suffice. There are advantages to using both type annotations and @abstractmethod decorators, so I would encourage library authors to use both. But one or the other is sufficient in this case. Please submit bug reports or PRs to fix these issues in your favorite libraries. The entire Python community will benefit from such improvements.

It is important for Pylance to infer when a method or function is a "NoReturn" type. Failure to do this will result in false-positive errors because it will analyze code that is not meant to be executed. It's a tradeoff between false-positive errors (something that we try hard to avoid) versus grayed-out text when using a library that is missing annotations and proper use of @abstractmethod.

@yury-tokpanov
Copy link
Author

yury-tokpanov commented Aug 26, 2020

@erictraut I understand the motives, but it's infeasible to do what you suggest in all of the Python code out there. Why not just give a user an option whether to show results of reachability analysis in UI or not (meaning disabling graying out)? Otherwise, Pylance is great, it's fast and gives useful suggestions, but having sometimes significant portion of the code greyed out is annoying at least.

@erictraut
Copy link
Contributor

As I mentioned, disabling reachability analysis will produce more problems than it solves.

One potential solution is to provide a setting that allows you to disable the reporting of unreachable code (i.e. don't show any gray text), but you can already effectively do that by adjusting your theme settings if the gray text really bothers you.

@krooq
Copy link

krooq commented Aug 30, 2020

That was an really clear explanation :)

It seems that the main issue is that NotImplementedError is a visual marker to developers that some class will implement the method but Pylance only considers @abstractmethod to be the marker.
Fair enough, they can have different semantic meanings, the just usually don't, which makes the grey text somewhat confusing to the developer.

If there was a setting, it would be to make NotImplementedError work the same as @abstractmethod.
But I don't think it's worth the extra maintainence burden.

@erictraut
Copy link
Contributor

After thinking about this further, I think it's reasonable for Pyright to assume that if a function meets all of the criteria below that it should be considered an abstract method for purposes of type inference:

  1. Is a method (as opposed to a function declared outside of a class)
  2. Has no return statements
  3. Has no yield statements
  4. Contains one or more raise statements
  5. All raise statements raise NotImplementedError exception types

I've changed Pyright to infer the return type of such methods as "Unknown" rather than "NoReturn". This should address most of the cases that have been cited.

This change will be in the next version of Pyright and Pylance.

@erictraut erictraut added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed waiting for user response Requires more information from user labels Aug 30, 2020
@jakebailey
Copy link
Member

This issue has been fixed in version 2020.9.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#202090-3-september-2020

@daviskirk
Copy link

daviskirk commented Oct 15, 2020

Another example is when using contextlib.suppress:

with suppress(AttributeError):
    return something.a 
return something_else

pylance will not understand that the suppress context manager is basically doing a try/catch pass and mark return something_else as unreachable.

@sect2k
Copy link

sect2k commented Feb 3, 2021

After thinking about this further, I think it's reasonable for Pyright to assume that if a function meets all of the criteria below that it should be considered an abstract method for purposes of type inference:

1. Is a method (as opposed to a function declared outside of a class)

2. Has no return statements

3. Has no yield statements

4. Contains one or more raise statements

5. All raise statements raise NotImplementedError exception types

I've changed Pyright to infer the return type of such methods as "Unknown" rather than "NoReturn". This should address most of the cases that have been cited.

This change will be in the next version of Pyright and Pylance.

Not sure if this is by design, or a bug, but this doesn't work for methods marked as @classmethod.

Example:

class Test(object):
    @classmethod
    def do_something(cls):
        val = cls.get_value()
        print(val)

    @classmethod
    def get_value(cls):
        raise NotImplementedError

print(val) is still grayed out, if decorator is removed, then it works fine.

@erictraut
Copy link
Contributor

I'm able to repro the problem using the above code snippet, but it only happens with Pylance (not with Pyright), and it seems to happen intermittently. In other words, I can make it go away by making a small edit in the file.

These symptoms indicate there's a bug related to type evaluation ordering. Pylance's semantic token highlighting feature tends to cause the types of identifiers to be evaluated in a different order than they otherwise would. In theory, the order of evaluation shouldn't affect the type evaluation results, but we have had bugs in the past where this is not the case. I'll need to look into the problem more deeply.

@erictraut
Copy link
Contributor

As I suspected, there was a bug that resulted in different results depending on the order of evaluation. The fix will be in the next release. Thanks for reporting the problem.

@erictraut erictraut added the bug Something isn't working label Feb 4, 2021
@jakebailey
Copy link
Member

This issue has been fixed in version 2021.2.1, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202121-10-february-2021

@StephanDDX
Copy link

As I mentioned, disabling reachability analysis will produce more problems than it solves.

One potential solution is to provide a setting that allows you to disable the reporting of unreachable code (i.e. don't show any gray text), but you can already effectively do that by adjusting your theme settings if the gray text really bothers you.

@jakebailey hey, was there any working done on providing such a setting? Or how do I can adjust my color-setting of my theme to not show greyed text? unfortunately the greyed text influence the readabilty of the code (color-wise)
I would be very happy to able to disable the "greyed out text". the "information/warning" is still a good thing to have.
for me it is just a color-problem. (and i am quite happy with my current theme so far, so if possible i would like to stay with it)

@erictraut
Copy link
Contributor

We don't have plans to provide a setting to disable this.

@judej judej removed the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Feb 15, 2022
@luabud
Copy link
Member

luabud commented Feb 15, 2022

@StephanDDX it seems VS Code offers this setting called Editor: Show Unused that allows you to enable/disable the greyed out text.

image

@StephanDDX
Copy link

@luabud thank you!

@luabud
Copy link
Member

luabud commented Feb 16, 2022

for those interested in making the setting specifically applied for Python only, opening the settings.json file (View > Command Palette... an run "Preferences: Open Settings (JSON)) you can add the following:

"[python]": {
        "editor.showUnused": false,
    },

@meropis
Copy link

meropis commented Feb 18, 2022

@luabud This is just what I was looking for! Thanks so much!

ps: edit.showUnused should be false, if you are looking to turn it off.

@amunozj
Copy link

amunozj commented Jul 29, 2022

@luabud Thank you so much for this!!! I also find this an issue and it is not something I can fix because the problem comes from a python module I have no control over. I'm glad there is an alterantive to the our way or the highway seen above.

@ielenik
Copy link

ielenik commented Nov 1, 2022

What is wrong with this code:

image

@rchiodo
Copy link
Contributor

rchiodo commented Nov 1, 2022

@ielenik what is your version of numpy? This might be a bug in the numpy type stubs. Works for me with 1.23.3

image

@erictraut
Copy link
Contributor

Yes, this is a duplicate of #3373, #3302, #3295, #3288, and #3277. It is a bug in earlier versions of numpy.

@OmarAI2003
Copy link

Capture
Capt3ure

why can't declare a variable after a while loop but when I write break it works

@debonte
Copy link
Contributor

debonte commented Mar 27, 2023

why can't declare a variable after a while loop but when I write break it works

@OmarAI2003, without the break your while True loop is an infinite loop so the code after the loop is unreachable. If you believe there's a problem here with Pylance's behavior, please open a new issue. But if the code after the loop is marked as unreachable, that would be by design.

@OmarAI2003
Copy link

why can't declare a variable after a while loop but when I write break it works

@OmarAI2003, without the break your while True loop is an infinite loop so the code after the loop is unreachable. If you believe there's a problem here with Pylance's behavior, please open a new issue. But if the code after the loop is marked as unreachable, that would be by design.

Yes, it should have been written before the while loop. Thank you for your assistance

@Kagee
Copy link

Kagee commented Mar 31, 2023

This feels like a place it would be nice to disable this:

image

This code is just unreachable if run on Windows, no ?

@erictraut
Copy link
Contributor

By default, pyright (the type checker that underlies pylance) analyzes your code assuming the current platform. If you want it to analyze your code for a different platform, you can specify the desired platform via the pythonPlatform configuration setting. If you want it to assume that your code will run on all platforms, you can specify a pythonPlatform of "" (empty string).

Here's a simple pyproject.toml file that will accomplish this:

[tool.pyright]
pythonPlatform = ""

@NSQY
Copy link

NSQY commented Sep 22, 2023

Still broken.

else is reachable:

if type_checking:
    annotation = str
else:
    annotation = float

else is unreachable:

if TYPE_CHECKING:
    annotation = str
else:
    annotation = float

@rchiodo
Copy link
Contributor

rchiodo commented Sep 22, 2023

Still broken.

else is reachable:

if type_checking:
    annotation = str
else:
    annotation = float

else is unreachable:

if TYPE_CHECKING:
    annotation = str
else:
    annotation = float

That's by design. TYPE_CHECKING is known for the type checker. type_checking is not so it can't predict which branch to take.

@NSQY
Copy link

NSQY commented Sep 23, 2023

Still broken.
else is reachable:

if type_checking:
    annotation = str
else:
    annotation = float

else is unreachable:

if TYPE_CHECKING:
    annotation = str
else:
    annotation = float

That's by design. TYPE_CHECKING is known for the type checker. type_checking is not so it can't predict which branch to take.

How do you suggest that I read other projects code when they're structured like this? I have to remove the TYPE_CHECKING check each time to get types within the function.

if TYPE_CHECKING:
    def foo(arg: str = 'whatever'):
        ...
else:
    def foo(arg: str = 'whatever'):
        x = 5
        y = 2
        z = 1
        return x + y + z

@rchiodo
Copy link
Contributor

rchiodo commented Sep 25, 2023

How do you suggest that I read other projects code when they're structured like this?

I would assume most people write libraries with the intent that you don't look at their source. I would assume in that example you gave, they're defining the function under the TYPE_CHECKING branch because it's easier to describe the return value of foo, which is what they expect you to care about.

@NSQY
Copy link

NSQY commented Oct 25, 2023

How do you suggest that I read other projects code when they're structured like this?

I would assume most people write libraries with the intent that you don't look at their source. I would assume in that example you gave, they're defining the function under the TYPE_CHECKING branch because it's easier to describe the return value of foo, which is what they expect you to care about.

I was going to ignore this and just assume that I was in the wrong but after dealing with this constantly it has been driving me crazy. The library does not get to decide what I should and should not have access to. In fact, I first discovered this when trying to diagnose a bug where a method was mistakenly returning None.

It is beyond frustrating that perfectly working code is supposedly "unreachable".

image

@rchiodo
Copy link
Contributor

rchiodo commented Oct 25, 2023

You could log an issue on the library then. If the library author is not returning the correct types for their functions, it would be a bug on them to fix.

@NSQY
Copy link

NSQY commented Oct 26, 2023

You could log an issue on the library then. If the library author is not returning the correct types for their functions, it would be a bug on them to fix.

Everything in the image I posted is correctly typed. Enabling plyance reduces the functionality of vs-code since it completely disables all interaction with the function since it thinks the code is unreachable. I may as well be using Notepad.

@rchiodo
Copy link
Contributor

rchiodo commented Oct 26, 2023

Everything in the image I posted is correctly typed. Enabling plyance reduces the functionality of vs-code since it completely disables all interaction with the function since it thinks the code is unreachable. I may as well be using Notepad.

Sorry now I'm confused. Pylance is getting the wrong types then? Do you want to open another bug? I thought the original problem was the if TYPE_CHECKING code would mark items in the else case as unreachable. If you have a situation where there's a different reason for that and you're suspecting Pylance is not computing types correctly, please log another issue.

If the complaint that some code is marked as unreachable (and the types are computed correctly) but you want it to just skip the unreachable behavior, that would be an enhancement. You could also open a new issue for that as well. I don't think it would work though. If Pylance thinks it's unreachable, it can't compute the types of anything in that section. As far as Pylance is concerned, everything has a type of 'Never'.

@rchiodo
Copy link
Contributor

rchiodo commented Oct 26, 2023

I guess what I'm asking is do you think Pylance is computing types incorrectly or not?

@NSQY
Copy link

NSQY commented Oct 29, 2023

The problem is when inside a if TYPE_CHECKING block half of the features either stop working or become unavailable.

if TYPE_CHECKING:
    def foo(exp: float = 5.0) -> float:
        ...
else:
    def foo(exp: float = 5.0) -> float:
        x = int(5)
        return x * exp

x is an int here:

if TYPE_CHECKING:
image

Without:
image

if TYPE_CHECKING but with Pylance extension disabled:
image

@NSQY
Copy link

NSQY commented Oct 29, 2023

Using pylance with projects that make use of if TYPE_CHECKING results in significantly degraded functionality compared to vs-code with the extension disabled. I can't jump between files, I can't jump to definitions, I can't see how many times it has been referenced.

if TYPE_CHECKING

image

Without:

image

if TYPE_CHECKING but with Pylance extension disabled:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests