fix false positive W0238 with private staticmethod used from classmethod #4949
Conversation
| @@ -926,9 +926,12 @@ def _check_unused_private_functions(self, node: nodes.ClassDef) -> None: | |||
| continue | |||
| if isinstance(attribute.expr, nodes.Name) and attribute.expr.name in ( | |||
| "self", | |||
| "cls", | |||
There was a problem hiding this comment.
I think we already fixed something really similar in another MR, we have a duplication between this and child.expr.name in ("self", "cls", node.name) line 969/972.
There was a problem hiding this comment.
I can still reproduce #4849 against master. I don't quite understand what the lines you reference fixed.
There was a problem hiding this comment.
A previous false positive for unused-private-member. I was wondering if we need to refactor that check to have a single piece of code to check that a name represent the current class. It's probably a little late now 😄
There was a problem hiding this comment.
Ok. Yes, it seems like the possibility to access attributes/methods on cls representing the class is beeing added by parts now.
I'm still unsure of hardcoding "cls" into the checks in general without further specification. While pylint checks, that the convention with "cls" as the first parameter of a classmethod is followed, it's still no guarantee. And if this rule is disabled on a case by case basis then all these "fixes" will fail again.
But I guess that's the same for the "self" parameter/word.
There was a problem hiding this comment.
#4645 found the MR I was talking about.
There was a problem hiding this comment.
@Pierre-Sassoulas kinda rusty, let me refresh my brain for a bit
There was a problem hiding this comment.
@Pierre-Sassoulas if it can be refactored now, it would be great (since i foresee that it might be refactored again in the future)
Just my two-cents to try to make code more concise and DRY early on rather than doing it later. Could you give it a shot @yumasheta ?
There was a problem hiding this comment.
I'm humbled, but as this is my first contribution to pylint source code (and first ever look at it), I reckon there are better qualified people out there.
Also the earliest I could do would be Okt/Nov.
There was a problem hiding this comment.
@Pierre-Sassoulas lets do that, i will check on the compatibility once this is merged with my branch
Pierre-Sassoulas
added
Bug 🪲
False Positive 🦟
Pull Request Test Coverage Report for Build 1209376769
💛 - Coveralls |
Type of Changes
Description
This PR fixes false positive
unusede-private-memberW0238 when using a private staticmethod from a classmethod, ie.cls.__private_static_method().Also a regression test is added to the functional tests.
Closes #4849