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

[pylint] Don't recommend decorating staticmethods with @singledispatch (PLE1519, PLE1520) #10637

Merged
merged 2 commits into from Apr 2, 2024

Conversation

alex-700
Copy link
Contributor

…andling

Summary

Fixes #10619

The behaviour, introduced in this PR, is different from original pylint's one, but this one is more natural.

According to the docs of staticmethod, the method marked with @staticmethod can be called via class (C.foo()) and via the instance (C().foo() (UFCS: C.foo(C())). Marking such method with @singledispatch allows only call via class, but not via the instance. Marking such method with @singledispatchmethod supports both C.foo() and C().foo(). The latter also is supported by singledispatchmethod docs via

@singledispatchmethod supports nesting with other decorators such as @classmethod.
...
The same pattern can be used for other similar decorators: @staticmethod, @abstractmethod, and others.

Example

class C:
    @singledispatchmethod
    @staticmethod
    def foo(x): raise NotImplementedError()

    @foo.register
    @staticmethod
    def _(x: int) -> int: return x


print(C.foo(1))
print(C().foo(1))

prints

1
1

but

class C:
    @singledispatch
    @staticmethod
    def foo(x): raise NotImplementedError()

    @foo.register
    @staticmethod
    def _(x: int) -> int: return x


print(C.foo(1))
print(C().foo(1))

fails with

C.foo() takes 1 positional argument but 2 were given

Test Plan

cargo test

Copy link
Contributor

github-actions bot commented Mar 27, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood self-assigned this Apr 2, 2024
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alex-700!

@AlexWaygood AlexWaygood changed the title [pylint] Fix singledispatchmethod[_function] in @staticmethod h… [pylint] Don't recommend decorating staticmethods with @singledispatch (PLE1519, PLE1520) Apr 2, 2024
@AlexWaygood AlexWaygood enabled auto-merge (squash) April 2, 2024 16:40
@AlexWaygood AlexWaygood merged commit 0de2376 into astral-sh:main Apr 2, 2024
17 checks passed
@dhruvmanila dhruvmanila added the bug Something isn't working label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PLE1520 false positive when using singledispatchmethod with staticmethod
3 participants