pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.29k stars 1.13k forks source link

no-member with Qt signals using PySide2 (starting from PySide2 5.15.2) #4040

Closed g-rocket closed 2 years ago

g-rocket commented 3 years ago

It looks like #2585 has popped back open.

Steps to reproduce

  1. Create signal_test.py containing:
    ''' bug '''
    from PySide2 import QtWidgets
    app = QtWidgets.QApplication()
    app.focusChanged.connect(lambda x: None)
  2. Run
    pylint ./signal_test.py --extension-pkg-whitelist=PySide2

Current behavior

signal_test.py:4:0: E1101: Method 'focusChanged' has no 'connect' member (no-member)

Expected behavior

No errors

pylint --version output

pylint 2.6.0
astroid 2.4.2
Python 3.9.1 (default, Dec 13 2020, 11:55:53)
[GCC 10.2.0]
g-rocket commented 3 years ago

Not sure if it's more appropriate to leave a comment on the old issue or open a new one; if the former feel free to re-open #2585 and close this as a duplicate. But that was fixed, and at some point afterwards seems to have started recurring with a slightly different error message.

I can confirm that with a slightly older version (exact version string below) there are no errors as expected:

pylint 2.4.4
astroid 2.3.3
Python 3.6.9 (default, Oct  8 2020, 12:12:24) 
[GCC 8.4.0]
hippo91 commented 3 years ago

@g-rocket thanks for opening a new issue. I find it clearer thant reopening the old one. I can reproduce the issue. However even with pylint 2.4.4 and astroid 2.3.3 the issue is present:

pylint bug_pylint_4040.py --extension-pkg-whitelist=PySide2
************* Module bug_pylint_4040
bug_pylint_4040.py:3:0: C0103: Constant name "app" doesn't conform to UPPER_CASE naming style (invalid-name)
bug_pylint_4040.py:4:0: E1101: Method 'focusChanged' has no 'connect' member (no-member)

----------------------------------------------------------------------
Your code has been rated at -10.00/10 (previous run: -10.00/10, +0.00)
pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.9.1 (default, Dec 30 2020, 11:18:33) 
[GCC 7.5.0]
g-rocket commented 3 years ago

Maybe it's related to the python version, or the pyside version?


The bug doesn't reproduce with python 3.6 / pyside2 5.13.0 (from pip).

pylint ./test.py --extension-pkg-whitelist=PySide2
************* Module test
test.py:3:0: C0103: Constant name "app" doesn't conform to UPPER_CASE naming style (invalid-name)

-------------------------------------------------------------------
Your code has been rated at 6.67/10 (previous run: 10.00/10, -3.33)
pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.6.9 (default, Oct  8 2020, 12:12:24) 
[GCC 8.4.0]
pip3 show pyside2
Name: PySide2
Version: 5.13.0
Summary: Python bindings for the Qt cross-platform application and UI framework
Home-page: https://www.pyside.org
Author: Qt for Python Team
Author-email: pyside@qt-project.org
License: LGPL
Location: /home/wren/reubenr/.local/lib/python3.6/site-packages
Requires: shiboken2

I'm able to reproduce it (see my first comment) with python 3.9 / pyside2 5.15.2-2 (from the arch repos).

hippo91 commented 3 years ago

@g-rocket i thnink it may indeed be linked to the pyside version. Maybe it could be interesting to test with python3.9 and pyside2 5.13.0.

g-rocket commented 3 years ago

I can reproduce the issue. However even with pylint 2.4.4 and astroid 2.3.3 the issue is present

@hippo91 -- what version of pyside2 were you testing with?

g-rocket commented 3 years ago

A bit more cross-version testing reveals it's definitely based on the pyside2 version.

$ pylint --version
pylint 2.6.0
astroid 2.4.2
Python 3.6.9 (default, Oct  8 2020, 12:12:24)
[GCC 8.4.0]

With pyside2==5.13.0 the issue doesn't reproduce. With pyside2==5.15.2, it shows up.

After doing some binary searching it looks like the issue only reproduces with pyside2==5.15.2 -- it does not reproduce with pyside2==5.15.1 or earlier. Unfortunately I can't seem to get pyside2 to build from source so I'm having trouble tracking down which commit actually broke it.

g-rocket commented 3 years ago

It looks like in pyside2==5.15.2, Signals are now detected as functions instead of classes. Adding

MANAGER.register_transform(
    nodes.FunctionDef,
    transform_pyside_signal,
    lambda node: _looks_like_signal(node, "Signal"),
)

to astroid/brain/brain_qt.py fixes the issue, but that looks like it'd also muck up any function named Signal, and I can't figure out how to make it module-specific. Could someone who's more familiar with Astroid suggest a better fix?

hippo91 commented 3 years ago

@g-rocket i think you are a the good way to solve this issue. What about making a PR in astroid? To be sure to be module specific you should use the qname attribute:

    lambda node: node.qname() in ("PySide.QtCore.Signal", "PySide2.QtCore.Signal"),
g-rocket commented 3 years ago

@hippo91 Unfortunately, that doesn't work. node.qname() is the function name, e.g. PySide2.QtWidgets.QtApplication.focusChanged for the given example file, but there are a bunch of different functions that we need to identify all have the class Signal.

Unfortunately, node.instance_attrs['__class__'].qname() is just Signal (no qualifications).

I suppose I could do

def _is_probably_pyside2_signal(node):
    if "__class__" in node.instance_attrs:
        try:
            cls = node.instance_attrs["__class__"][0]
            return cls.name == "Signal" and node.qname().startswith("PySide2.")
        except AttributeError:
            # return False if the cls does not have a name attribute
            pass
    return False

but that seems kinda hacky.