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

False positive: "<variable>" is not defined Pylance(reportUndefinedVariable)

Open shoffmeister opened this issue 3 years ago • 3 comments

Environment data

Pylance language server 2022.8.30 (pyright ed13daf1) starting Python 3.10.6 (distribution bundled) Fedora Linux 36

Code Snippet

Rough sketch:

        if key:  # Enable
            self.key = next((k for k in device.keys if k.key == key), None)
            if self.key:
                self.active = True
                if divertSetting:
                    divertSetting.write_key_value(int(self.key.key), 1)
                device.add_notification_handler(self.name, handler)
                from solaar.ui import status_changed as _status_changed        # <----- this import is good
                self.activate_action()
                _status_changed(device, refresh=True)  # update main window
            else:
                _log.error('cannot enable %s on %s for key %s', self.name, device, key)
        else:  # Disable
            if self.active:
                self.active = False
                if divertSetting:
                    divertSetting.write_key_value(int(self.key.key), 0)
                from solaar.ui import status_changed as _status_changed            # <----- diagnostic on exactly the same import
                _status_changed(device, refresh=True)  # update main window
                try:
                    device.remove_notification_handler(self.name)
                except Exception:
                    if _log.isEnabledFor(_WARNING):
                        _log.warn('cannot disable %s on %s', self.name, device)
                self.deactivate_action()
        return True

See actual screenshot below for more context; I am not sure how to approach efficiently trimming down this (to be unfamiliar) code base - my apologies. Pylance implementation knowledge may help to construct a much smaller repro.

Repro Steps

  • git clone https://github.com/pwr-Solaar/Solaar.git
  • open working copy in vscode
  • Go to File: lib/logitech_receiver/settings.py

-> get warning: "status_changed" is not defined Pylance(reportUndefinedVariable)

You may have to install Python modules to (only) see this diagnostic; in my case, I have the baseline distribution package installed which got me system-wide copies. According to docs/installation.md installing the following modules might suffice in a venv:

pyudev
psutil
xlib
evdev
yaml
pyyaml

Expected behavior

No warning that status_changed is undefined.

Actual behavior

"status_changed" is not definedPylancereportUndefinedVariable

The screenshot of the snippet below demonstrates the challenge, too: Note the Pylance warning on status_changed at the bottom; the block above mirrors exactly the from solaar.ui import status_changed as _status_changed line diagnostic, without any diagnostic. Browsing to that symbol also works from that location.

image

shoffmeister avatar Aug 19 '22 16:08 shoffmeister

Thanks for posting the quest.

Could you provide a minimal, self-contained code sample that exhibits the issue? It's difficult to repro problems with incomplete code samples and screen shots.

erictraut avatar Aug 19 '22 17:08 erictraut

I would love to provide better instructions, but I myself am totally unfamiliar with that code base and wouldn't know where to start trimming.

My hope would be that by looking at https://github.com/pwr-Solaar/Solaar/blob/master/lib/logitech_receiver/settings.py#L1403, and with awareness of the implementation details of Pylance, it would be much much easier to trim this down - perhaps even with a breakpoint on a debugger for pylance?

The one thing I can contribute right now is: If the use of status_change on https://github.com/pwr-Solaar/Solaar/blob/master/lib/logitech_receiver/settings.py#L1392 and subsequent lines is removed, then warning disappears.

So pylance seems to get confused by the multiple, identical imports, in the different branches, it would seem. That's the best I can offer for now.

shoffmeister avatar Aug 19 '22 17:08 shoffmeister

For the record, while trying to trim this, I had to restart the Python language server to make this problem appear again, after resetting any modifications.

So in case suddenly the diagnostic disappears - restart the Python language server...

shoffmeister avatar Aug 19 '22 17:08 shoffmeister

I can repro opening that same settings.py:

Image

Not sure why it's undefined, but should allow me to debug it.

rchiodo avatar Nov 11 '22 00:11 rchiodo

This code repros the issue:

def foo():
    if (Bar()):
        from sys import excepthook as _excepthook
        print(_excepthook.__class__)
    else:
        from sys import excepthook as _excepthook
        print(_excepthook.__class__)

rchiodo avatar Nov 11 '22 23:11 rchiodo

Image

rchiodo avatar Nov 11 '22 23:11 rchiodo

I think bug is in type eval. at line 4181

addDiagnostic(
                        fileInfo.diagnosticRuleSet.reportUndefinedVariable,
                        DiagnosticRule.reportUndefinedVariable,
                        Localizer.Diagnostic.symbolIsUndefined().format({ name }),
                        node
                    );

basically, when I took a look last time, for code like

from M import S as A

pyright will put A in symbol table but won't put S. so type eval will hit the code above.

I think we should check the name node given whether it is for S, if it is, we should skip showing error.

heejaechang avatar Nov 14 '22 18:11 heejaechang

The 'S' in the example above is a special case. You can see that special case here:

https://github.com/microsoft/pyrx/blob/main/packages/pyright/packages/pyright-internal/src/analyzer/typeEvaluator.ts#L19095

I thought it was a missing symbol too, but there shouldn't be a symbol for that item.

rchiodo avatar Nov 14 '22 19:11 rchiodo

With the range fix, that special code will find the declaration correctly.

rchiodo avatar Nov 14 '22 19:11 rchiodo

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

rchiodo avatar Nov 17 '22 00:11 rchiodo