pylance-release icon indicating copy to clipboard operation
pylance-release copied to clipboard

"Code is Unreachable" is showed when code is actually working

Open Qihai-Weng opened this issue 3 years ago • 25 comments

Environment data

  • Language Server version: python v2021.11.2
  • OS and version: macos 12.0.1
  • Python version (& distribution if applicable, e.g. Anaconda): 3.8.5

Expected behaviour

Should act normally since the code doesn't have problem

Actual behaviour

report code unreachable

Logs

XXX

Code Snippet / Additional information


![Screen Shot 2021-11-19 at 5 34 06 PM](https://user-images.githubusercontent.com/71131478/142699934-01fd53be-29d7-4371-bc24-d0c8b20ce432.png)

Qihai-Weng avatar Nov 19 '21 22:11 Qihai-Weng

Can u share your whole code?

yvvt0379 avatar Nov 20 '21 02:11 yvvt0379

Sure.

On Nov 19, 2021, at 9:20 PM, CodeCrazy-ywt @.***> wrote:

Can u share your whole code?

You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_microsoft_pylance-2Drelease_issues_2092-23issuecomment-2D974577220&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=vhci8QKwzO_aQE-C3JoQYQ&m=2skxzB8qNsclE8oflC8oNNGB_BNKr7um8wcbPZvFYs42UVaI93NTvGpSk1UeYgNu&s=thBl2ypSct0dtCYIyiDKIOT0TQ51gLZfgxxJwUnhKK0&e=, or unsubscribe https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AQ6WCVVHHWC6Y7BP3DWC5BDUM4AXPANCNFSM5INEYSJQ&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=vhci8QKwzO_aQE-C3JoQYQ&m=2skxzB8qNsclE8oflC8oNNGB_BNKr7um8wcbPZvFYs42UVaI93NTvGpSk1UeYgNu&s=zbAxTmiLTtq1zLARnX0thzFnTaZQMB9EoZsBkRDZ8Ww&e=. Triage notifications on the go with GitHub Mobile for iOS https://urldefense.proofpoint.com/v2/url?u=https-3A__apps.apple.com_app_apple-2Dstore_id1477376905-3Fct-3Dnotification-2Demail-26mt-3D8-26pt-3D524675&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=vhci8QKwzO_aQE-C3JoQYQ&m=2skxzB8qNsclE8oflC8oNNGB_BNKr7um8wcbPZvFYs42UVaI93NTvGpSk1UeYgNu&s=XWxjhSI1nGREWJ8dgXHrkP-j6-n0zw-FYJuxgPfA7yc&e= or Android https://urldefense.proofpoint.com/v2/url?u=https-3A__play.google.com_store_apps_details-3Fid-3Dcom.github.android-26referrer-3Dutm-5Fcampaign-253Dnotification-2Demail-2526utm-5Fmedium-253Demail-2526utm-5Fsource-253Dgithub&d=DwMCaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=vhci8QKwzO_aQE-C3JoQYQ&m=2skxzB8qNsclE8oflC8oNNGB_BNKr7um8wcbPZvFYs42UVaI93NTvGpSk1UeYgNu&s=cItkxteMnqAorMMcFll8yRZ_57YxdzD5h09U8GrA2d8&e=.

Qihai-Weng avatar Nov 20 '21 02:11 Qihai-Weng

You need to send the screenshot in the github page, not via email, or we cannot see the screenshot.

yvvt0379 avatar Nov 20 '21 08:11 yvvt0379

https://user-images.githubusercontent.com/71131478/142797739-cafe9c34-5261-4350-b5be-302258f3a760.png

Qihai-Weng avatar Nov 22 '21 03:11 Qihai-Weng

This is the same issue as https://github.com/microsoft/pylance-release/issues/2093, and it should be addressed in the next release.

erictraut avatar Nov 22 '21 03:11 erictraut

I have similar issue after class constructors Pylance v2022.1. pylance code unrechable

slavugan avatar Jan 10 '22 10:01 slavugan

Also seeing the same as @slavugan , with v2022.1.3.

brettdh avatar Jan 26 '22 19:01 brettdh

@slavugan, @brettdh, we'll need more context to investigate the issue you're seeing. Could you please post a minimal, self-contained sample that demonstrates the problem?

erictraut avatar Jan 26 '22 19:01 erictraut

Here's one occurrence (edit: wrong call, sorry)

image

brettdh avatar Jan 26 '22 20:01 brettdh

Just code:

class Foo:
    def __init__(self):
        self.must_override()

    def must_override(self):
        raise AssertionError("Subclass must implement")


class Bar(Foo):
    def must_override(self):
        print("Bar.must_override")


def main():
    bar = Bar()
    print("definitely reachable")  # pylance says this is unreachable

main()

brettdh avatar Jan 26 '22 20:01 brettdh

This is expected behavior. You haven't provided pylance with any clues that Foo will be overridden. You can do this in any of the following ways.

  1. Mark Foo as an abstract base class and must_override as an abstract method. This technique has the added benefit that pylance can warn you if you attempt to instantiate a subclass that doesn't properly override the required method(s).
class Foo(ABC):
    def __init__(self):
        self.must_override()

    @abstractmethod
    def must_override(self):
        raise AssertionError("Subclass must implement")
  1. Use a NotImplementedError rather than AssertionError. The former is the standard and recommended way to indicate that a method is not implemented in a base class, and pylance special-cases this exception type accordingly.
    def must_override(self):
        raise NotImplementedError("Subclass must implement")
  1. Add an explicit return type annotation for must_override so pylance doesn't need to fall back on type inference to infer the return type. This has the side benefit that if a subclass that overrides the method with an incorrect return type, pylance can warn you (if you enable the reportIncompatibleMethodOverride diagnostic check).
    def must_override(self) -> None:
        raise AssertionError("Subclass must implement")

erictraut avatar Jan 26 '22 20:01 erictraut

Can we agree that "Code is unreachable" is a bit misleading in this scenario, and that pylance could be telling me something more helpful?

brettdh avatar Jan 26 '22 21:01 brettdh

I'm also facing the same issue on Pylance v2022.2.1, here is some code to reproduce it:

from influxdb import DataFrameClient

dataframe_client = DataFrameClient()
print("This statement is reachable but marked unreachable")

DataFrameClient is from here.

Since this is from a library, I don't have the liberty of editing the class as suggested by @erictraut. Is there anything else I can do to disable the problem?

melvinkokxw avatar Feb 11 '22 14:02 melvinkokxw

Odd behavior I'm having with the same issue, on Pylance v2022.2.1 with Python 3.10.2 .

TL;DR: the simplest example showing a false positive is with pytest.raises:

def test_the_thing():
    def bar():
        raise ValueError("Whoops")
    
    with pytest.raises(ValueError):
        bar()
    
    # Marked unreachable
    assert False, "reached here"

The last line is reachable, though Pylance says it is not. Likely to do with the return value for pytest.raises context manager.

The odd behavior I'm having is more complicated, as I'm setting up a decorator for class methods to capture, log, and re-raise exceptions:

from typing import Callable
import functools

def method_exception_logger(
    _func: Callable = None,
    event: str = "Something bad happened",
    **data,
):
    def inner(func: Callable):
        @functools.wraps(func)
        def wrapper(self, *args, **kwargs):
            try:
                return func(*args, **kwargs)
            except Exception:
                # No, I'm not using print logging: this is just for demonstration!
                print(f"{event}: {data}")
                raise
        return wrapper
    if _func is None:
        return inner
    return inner(_func)

This decorator works both with and without parentheses:

# without parentheses
@method_exception_logger
def foo():
    pass

# with empty parentheses
@method_exception_logger()
def foo():
    pass

# changing kwargs
@method_exception_logger(event="Something ELSE happened")
def foo():
    pass

When testing, I find that the no-parentheses method somehow tricks Pylance into thinking the code is now reachable:

import pytest

def test_the_thing():
    @method_exception_logger
    def bar():
        raise ValueError("Whoops")
    
    with pytest.raises(ValueError):
        bar()

    # Pylance marks as reachable, no problems here
    assert False, "reached here"

But any time I use parentheses with the decorator, again it's marked unreachable:

def test_the_thing():
    @method_exception_logger()
    def bar():
        raise ValueError("Whoops")
    
    with pytest.raises(ValueError):
        bar()
    
    # Pylance marks as unreachable, even though it is
    assert False, "reached here"

In all cases above, Python correctly reaches the False assertions in tests, since pytest.raises allows the program to continue if the expected exception is caught. Pylance seems to think the exceptions bubble out and stop the program.

GriceTurrble avatar Feb 11 '22 19:02 GriceTurrble

@GriceTurrble, thanks for the clear instructions. I was able to reproduce the problem you're seeing, and I have a fix that will be in the next release.

@melvinkokxw, the problem with influxdb is more difficult to solve because it's using a non-standard dynamic code pattern that defines DataFrameClient in a way that always raises an exception from its constructor. If you want to reach out to the maintainer of this library, there are some simple changes they could make to improve the experience with static type analyzers like pylance. I'd be happy to work with the maintainers if they want to reach out. In the meantime, the best idea I can offer you is to use a local type stub file. Create a file called influxdb.pyi in your local project director and add the following line to it:

from influxdb._dataframe_client import DataFrameClient as DataFrameClient

erictraut avatar Feb 12 '22 02:02 erictraut

I just loaded back up with release 2022.2.3, and still seeing the false positives (unreachable code) that I noted before.

Going to assume that fix is coming in the following update, then? 🙂

GriceTurrble avatar Feb 19 '22 02:02 GriceTurrble

I needed to back out my previous fix because it had a significant performance impact that negatively affected many users. I'll need to come up with a different fix. I've created a tracking bug in the pyright project: https://github.com/microsoft/pyright/issues/3080.

erictraut avatar Feb 19 '22 13:02 erictraut

@erictraut Fair enough. Thanks for the update. :)

GriceTurrble avatar Feb 19 '22 15:02 GriceTurrble

I found a better fix that should have no measurable impact on performance. This will be included in the next release.

erictraut avatar Feb 19 '22 21:02 erictraut

Hi,

I have a similar issue, but with context manager, that is reproducible with the following minimal example:

from contextlib import contextmanager


class ContextManager:
    error_raised = False
    @contextmanager
    def update_context(self):
        try:
            yield
        except ValueError:
            self.error_raised = True


context = ContextManager()

with context.update_context():
    raise ValueError("Some error")

print("Statement marked as unreachable when it is reachable")
print(context.error_raised)

I just wanted to share it to see if the incoming release is also covering this case.

Regards,

Nateckert avatar Mar 04 '22 09:03 Nateckert

@Nateckert, your situation is different. You're relying on the @contextmanager decorator, which transforms the update_context method into a _GeneratorContextManager object. This class is defined within the stdlib typeshed type stubs, and its type information indicates to pylance that the context manager does not swallow exceptions, so when pylance sees an exception raised within the with code block, it assumes this exception will not be handled and that all subsequent code is unreachable.

The standard way to write a context manager is to supply an __enter__ and __exit__ method. If the exit method returns a value of True, then it indicates that it has handled exceptions. If it returns False or None, it doesn't. If you write your context manager in this standard way, pylance will be able to statically analyze the intended code flow.

If you would like help with the details, please feel free to open a discussion thread in this project.

erictraut avatar Mar 04 '22 09:03 erictraut

Hi! Also have a similar issue, looks like pylance is triggered by the word cross (and it is treated just like break, i.e. not making unreachable everything after for loop block). Here is minimal example:

import numpy as np
for i in range(7):
    transform = np.eye(4)

    fwd_vector = np.cross(up_vectors[i], right_vector)
    transform[:3, 0] = fwd_vector # this line and lines below are considered unreachable but they are obviously not
    transform[:3, 1] = right_vector
    transform[:3, 2] = up_vectors[i]

    transforms.append(transform)

print("reachable here") # but not this one =)

If I change it to something else (like np.dot for example), the "Code is unreachable" plate disappears.

Version: 1.70.1 (user setup)
Commit: 6d9b74a70ca9c7733b29f0456fd8195364076dda
Date: 2022-08-10T06:08:33.642Z
Electron: 18.3.5
Chromium: 100.0.4896.160
Node.js: 16.13.2
V8: 10.0.139.17-electron.0
OS: Windows_NT x64 10.0.22000

Pylance ext: v2022.8.30
Python ext: v2022.12.1

kst179 avatar Aug 23 '22 23:08 kst179

@kst179 see issue https://github.com/microsoft/pylance-release/issues/3240

need to specify a type to up_vectors[i] so that you get the correct overload of cross

bschnurr avatar Aug 24 '22 16:08 bschnurr

I reverted pylance to v2022.8.10 and that addressed the problem for me.

I hope this comment helps people who cannot update the relevant source code (for whatever reason). For me, having this version of pylance is much better than not having it. (otherwise there's too much code marked as unreachable).

I looked into what changed since v2022.8.10. In v2022.8.12 I noticed that pylance upgraded to pyright==1.1.226. When I looked at the pyright change-log, this enhancement to pyright caught my attention:

Enhancement: Enhanced support for handling of functions that return NoReturn to include support for overloaded functions where some of the overloads return NoReturn and others do not.

In case it's helpful, here's where the code becomes "unreachable" (it all stems from a call to numpy's cross product function): https://github.com/pbrod/nvector/blob/bfd8ffce09e208e09648f4cede829e1709bf59c7/src/nvector/rotation.py#L461 I'm thankful for nvector and numpy but I'm not able to change these projects.

So far, I've only seen this problem when using an m1 mac. I'm using the remote containers extension with docker. When I tested pylance on Linux, version v2022.8.50 worked as expected.

murphym18 avatar Sep 05 '22 22:09 murphym18

Your analysis is correct. Versions of pyright prior to 1.1.266 were not honoring overloads that included NoReturn return types for some overload signatures. Fixing this bug exposed some bugs in existing libraries including numpy's cross function. That will need to be fixed by the numpy maintainers. In the meantime, you can work around the problem by providing type annotations that allow pyright to determine the types of the arguments you pass to cross. If the types of these arguments are Unknown or Any, pyright defaults to the first overload signature, and in the case of cross, that has a return type of NoReturn.

erictraut avatar Sep 06 '22 02:09 erictraut