pylint
pylint copied to clipboard
Check For @singledispatch Decorator in classes without `@staticmethod`
Current problem
The representative code implements single dispatching with the @singledispatch
and @staticmethod
decorators in a class.
Class Board():
@singledispatch
@staticmethod
def convert_position(position):
[...]
@convert_position.register(str)
@staticmethod
def _(position: str) -> tuple:
[...]
@convert_position.register(tuple)
@staticmethod
def _(position: tuple) -> str:
[...]
Pylint reports nothing unusual about using @singledispatch
inside a class.
Desired solution
Pylint should check for the @singledispatch
decorator inside a class. According to the documentation, the @singledispatchmethod
decorator is for class methods.
Additional context
The code works with the @singledispatch
decorator as long as the single dispatch methods have the @staticmethod
decorator (i.e., stateless methods). If the @staticmethod
decorators were removed or replaced with the @classmethod
decorator, the code stops working and the resulting error messages will indirectly point out the wrong decorator. A pylint check would prevent that from happening.
Thanks for the clear report. This could be added to the StdlibChecker
. No obligation, but if you have (or anyone reading has) an interest in preparing a patch yourself I'd be glad to answer questions as you go.
Hey @jacobtylerwalls - landing here from the hacktoberfest
label and not familiar with pylint's codebase. Is this a similar implementation as _check_lru_cache_decorators
in StdlibChecker
?
@ramonsaraiva That should indeed be relatively similar. In fact, I think we can build upon some of the code that is already there. You can probably also look at https://github.com/PyCQA/pylint/blob/main/tests/functional/m/method_cache_max_size_none.py for some inspiration for functional tests.
Awesome, I'll be reading more about how everything works and attempt to come up with a draft patch for this. A few review interactions will most likely be needed. Thanks!
Created that draft PR and added in its description a bit of my thought process and things that I believe can be improved and simplified. I just want to have a better sense if I'm going in the right direction.
I'll be digging more on utilities and built-in functionality that I can use to simplify all of that but I thought it'd be helpful to have a draft available for you to take a look.
I think https://github.com/PyCQA/pylint/commit/4d95d21ad41e1a219cc5917181b102e5de5832bd and https://github.com/PyCQA/pylint/commit/cc64ded7c6c5e5fd053e5a58fbbeebce0cff93c8 bring a much cleaner version that https://github.com/PyCQA/pylint/commit/3dc361624cd805e32972372257e3ee48ef1ac84a had - still open to suggestions.
I added a new util function for singledispatchmethod
(similar to the singledispatch
one) and moved some reusable logic to a separate one. There was a minor modification to the way we check for register calls, cause now with annotated types register can also be an attribute and not strictly a call.
Manually closing since MR is merged.