Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 <31762852+mbyrnepr2@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # 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