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

E1520 recommends an invalid combination of @singledispatch and @staticmethod #9531

Closed
AlexWaygood opened this issue Apr 2, 2024 · 0 comments · Fixed by #9599
Closed

E1520 recommends an invalid combination of @singledispatch and @staticmethod #9531

AlexWaygood opened this issue Apr 2, 2024 · 0 comments · Fixed by #9599
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@AlexWaygood
Copy link
Contributor

AlexWaygood commented Apr 2, 2024

Bug description

Hi from Ruff!

We recently received a bug report at Ruff stating that our reimplementation of E1520 was recommending an invalid combination of @functools.singledispatch and @staticmethod. For example, this code runs fine, but ruff's PLE1520 rule was telling the issue reporter that they should use @singledispatch here instead of @singledispatchmethod. If you do so, the code raises an exception at runtime, as @staticmethod is a descriptor, and @singledispatch isn't designed to be stacked on top of descriptor instances (that's what @singledispatchmethod is for). The Python docs also state that @singledispatchmethod should be used for static methods: https://docs.python.org/3/library/functools.html#functools.singledispatchmethod.

from functools import singledispatch, singledispatchmethod

class Foo:
    @singledispatchmethod
    @staticmethod
    def bar(value):
        raise NotImplementedError() 

    @bar.register
    @staticmethod
    def _(value: int) -> int:
        return 42 * value

    def method(self) -> int:
        return self.bar(2)

print(Foo().method())

After investigating the bug, we decided to change our PLE1520 rule so that it no longer complained about @singledispatchmethod on top of @staticmethod. It looks like pylint has the same incorrect behaviour here in the original PLE1520 rule, though, so I thought I'd open an issue to give you a heads-up as well!

In case it's helpful, here's the fix that was contributed to Ruff to fix the bug: astral-sh/ruff#10637. The fix also involved some small changes to PLE1519 (E1519 at pylint) as well as PLE1520 (E1520 at pylint).

Command used

pylint demo.py

Pylint output

************* Module demo
demo.py:7:35: C0303: Trailing whitespace (trailing-whitespace)
demo.py:1:0: C0114: Missing module docstring (missing-module-docstring)
demo.py:3:0: C0115: Missing class docstring (missing-class-docstring)
demo.py:6:4: C0116: Missing function or method docstring (missing-function-docstring)
demo.py:6:4: C0104: Disallowed name "bar" (disallowed-name)
demo.py:4:5: E1520: singledispatchmethod decorator should not be used with functions, use singledispatch instead. (singledispatchmethod-function)
demo.py:9:5: E1520: singledispatchmethod decorator should not be used with functions, use singledispatch instead. (singledispatchmethod-function)
demo.py:14:4: C0116: Missing function or method docstring (missing-function-docstring)
demo.py:1:0: W0611: Unused singledispatch imported from functools (unused-import)

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

Expected behavior

************* Module demo
demo.py:7:35: C0303: Trailing whitespace (trailing-whitespace)
demo.py:1:0: C0114: Missing module docstring (missing-module-docstring)
demo.py:3:0: C0115: Missing class docstring (missing-class-docstring)
demo.py:6:4: C0116: Missing function or method docstring (missing-function-docstring)
demo.py:6:4: C0104: Disallowed name "bar" (disallowed-name)
demo.py:14:4: C0116: Missing function or method docstring (missing-function-docstring)
demo.py:1:0: W0611: Unused singledispatch imported from functools (unused-import)

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

Pylint version

pylint 3.1.0
astroid 3.1.0
Python 3.12.2 (main, Feb 15 2024, 19:30:27) [Clang 15.0.0 (clang-1500.1.0.2.5)]

OS / Environment

MacOS; Python 3.12.2

Additional dependencies

None

@AlexWaygood AlexWaygood added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Apr 2, 2024
@DanielNoord DanielNoord added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation Help wanted 🙏 Outside help would be appreciated, good for new contributors and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 19, 2024
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue May 5, 2024
…thod is decorated with both ``functools.singledispatchmethod`` and ``staticmethod``.

Closes pylint-dev#9531
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.1.1 milestone May 6, 2024
mbyrnepr2 added a commit that referenced this issue May 7, 2024
* Fix a false positive with ``singledispatchmethod-function`` when a method is decorated with both ``functools.singledispatchmethod`` and ``staticmethod``.

Closes #9531
github-actions bot pushed a commit that referenced this issue May 7, 2024
* Fix a false positive with ``singledispatchmethod-function`` when a method is decorated with both ``functools.singledispatchmethod`` and ``staticmethod``.

Closes #9531

(cherry picked from commit 6df4e1d)
Pierre-Sassoulas pushed a commit that referenced this issue May 7, 2024
…9605)

* Fix a false positive with ``singledispatchmethod-function`` when a method is decorated with both ``functools.singledispatchmethod`` and ``staticmethod``.

Closes #9531

(cherry picked from commit 6df4e1d)

Co-authored-by: Mark Byrne <[email protected]>
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 Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants