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

False-positive inconsistent-mro with PySide6 objects #8704

Open
bersbersbers opened this issue May 19, 2023 · 3 comments
Open

False-positive inconsistent-mro with PySide6 objects #8704

bersbersbers opened this issue May 19, 2023 · 3 comments
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@bersbersbers
Copy link

Bug description

This is a follow-up to #8698, so I am using pylint from master, see version info below. Similar to #1377, pylint warns about inconsistent MRO with PySide6 objects although the code works fine. The error message is correct for other classes such as Path, see second part of the example.

"""False-positive 'inconsistent MRO'."""
from pathlib import Path
from typing import Protocol

from PySide6.QtWidgets import QApplication, QMainWindow


class Appear(Protocol):
    def appear(self) -> None:
        ...


# Here, we get a false-positive "inconsistent MRO" in `pylint` ...
class WindowMeta(type(QMainWindow), type(Appear)):
    pass

class MyWindow(QMainWindow, Appear, metaclass=WindowMeta):
    def appear(self) -> None:
        self.show()

# ... as evidenced by this code working fine in `python`:
application = QApplication()
my_window = MyWindow()
my_window.appear()
application.exec()


# By contrast, this "inconsistent MRO" error in `pylint` ...
class PathMeta(type(Path), type(Appear)):
    pass

class MyPath(Path, Appear, metaclass=PathMeta):
    def appear(self) -> None:
        print(self)

# ... matches the (failing) behavior in `python`:
my_path = MyPath()
my_path.appear()

Configuration

[tool.pylint]
enable = ["useless-suppression"]
disable = [
    "missing-class-docstring",
    "missing-function-docstring",
    "too-many-lines",
    "design",
]

Command used

pylint bug.py

Pylint output

(project_3.11) C:\Code\project>pylint bug.py
************* Module bug
bug.py:14:0: E0240: Inconsistent method resolution order for class 'WindowMeta' (inconsistent-mro)
bug.py:17:0: E1139: Invalid metaclass 'WindowMeta' used (invalid-metaclass)
bug.py:29:0: E0240: Inconsistent method resolution order for class 'PathMeta' (inconsistent-mro)
bug.py:32:0: E1139: Invalid metaclass 'PathMeta' used (invalid-metaclass)

------------------------------------------------------------------
Your code has been rated at 0.91/10 (previous run: 0.00/10, +0.91)

Expected behavior

No pylint error for WindowMeta/MyWindow

bug.py:29:0: E0240: Inconsistent method resolution order for class 'PathMeta' (inconsistent-mro)
bug.py:32:0: E1139: Invalid metaclass 'PathMeta' used (invalid-metaclass)

Pylint version

pylint 3.0.0b1
astroid 3.0.0a3
Python 3.11.3 (tags/v3.11.3:f3909b8, Apr  4 2023, 23:49:59) [MSC v.1934 64 bit (AMD64)]

OS / Environment

Windows 10 21H2

Additional dependencies

pylint==3.0.0b1

  • astroid [required: >=3.0.0a2,<=3.1.0-dev0, installed: 3.0.0a3]
  • colorama [required: >=0.4.5, installed: 0.4.6]
  • dill [required: >=0.3.6, installed: 0.3.6]
  • isort [required: >=4.2.5,<6, installed: 5.12.0]
  • mccabe [required: >=0.6,<0.8, installed: 0.7.0]
  • platformdirs [required: >=2.2.0, installed: 3.5.1]
  • tomlkit [required: >=0.10.1, installed: 0.11.8]
    PySide6-Essentials==6.4.3
  • shiboken6 [required: ==6.4.3, installed: 6.4.3]
@bersbersbers bersbersbers added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label May 19, 2023
@jacobtylerwalls jacobtylerwalls added False Positive 🦟 A message is emitted but nothing is wrong with the code Unreleased Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels May 19, 2023
@jacobtylerwalls jacobtylerwalls added this to the 3.0.0a7 milestone May 19, 2023
@jacobtylerwalls
Copy link
Member

Thanks! Looks like #8698 was a little too eager with the InconsistentMroError handling. What do you think of this?

diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index ce1c9dd5c..7d8676079 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -749,8 +749,10 @@ def _no_context_variadic(
 def _is_invalid_metaclass(metaclass: nodes.ClassDef) -> bool:
     try:
         mro = metaclass.mro()
-    except (astroid.DuplicateBasesError, astroid.InconsistentMroError):
+    except astroid.DuplicateBasesError:
         return True
+    except astroid.InconsistentMroError:
+        return False
     return not any(is_builtin_object(cls) and cls.name == "type" for cls in mro)

Would you like to prepare a PR?

@jacobtylerwalls jacobtylerwalls added the Good first issue Friendly and approachable by new contributors label May 19, 2023
@bersbersbers
Copy link
Author

I tried this locally, and I am not sure this fixes the issue. At least it's not enough. (Ignore exact line numbers below.)

  1. After Fix crash in invalid-metaclass check #8699, we get inconsistent-mro and invalid-metaclass for both tests:
    bug.py:14:0: E0240: Inconsistent method resolution order for class 'WindowMeta' (inconsistent-mro)
    bug.py:17:0: E1139: Invalid metaclass 'WindowMeta' used (invalid-metaclass)
    bug.py:29:0: E0240: Inconsistent method resolution order for class 'PathMeta' (inconsistent-mro)
    bug.py:32:0: E1139: Invalid metaclass 'PathMeta' used (invalid-metaclass)
    
  2. After your proposed fix, I get no more invalid-metaclass, but still inconsistent-mro for both:
    bug.py:14:0: E0240: Inconsistent method resolution order for class 'WindowMeta' (inconsistent-mro)
    bug.py:31:0: E0240: Inconsistent method resolution order for class 'PathMeta' (inconsistent-mro)
    
  3. What we need (I think) is either
    bug.py:29:0: E0240: Inconsistent method resolution order for class 'PathMeta' (inconsistent-mro)
    bug.py:32:0: E1139: Invalid metaclass 'PathMeta' used (invalid-metaclass)
    
    or even
    bug.py:29:0: E0240: Inconsistent method resolution order for class 'PathMeta' (inconsistent-mro)
    
    (not sure about the invalid-metaclass for PathMeta).

Either way, there is at least one other inconsistent-mro to be fixed.

@jacobtylerwalls
Copy link
Member

Right, got it. Okay. The tricky part is that one is downstream of the other (if it's inconsistent-mro, it's also invalid as a metaclass). I guess we can leave this open to track dealing with the fp inconsistent-mro. This probably needs an update in the astroid brain for pyside6.

@jacobtylerwalls jacobtylerwalls added Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) and removed Good first issue Friendly and approachable by new contributors Unreleased labels May 19, 2023
@jacobtylerwalls jacobtylerwalls removed this from the 3.0.0a7 milestone May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants