-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add bad-dunder-name
checker
#7642
Add bad-dunder-name
checker
#7642
Conversation
This comment has been minimized.
This comment has been minimized.
b9a5b9c
to
ed43df2
Compare
9f8ca4b
to
e1e44dd
Compare
Pull Request Test Coverage Report for Build 3359652883
💛 - Coveralls |
e1e44dd
to
80363f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It starts to look pretty good 👍
# author likely unintentionally misspelled the correct init dunder. | ||
pass | ||
|
||
def _init_(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would raise with the change suggested earlier, I think it make sense.
This comment has been minimized.
This comment has been minimized.
80363f7
to
3bd3f61
Compare
This comment has been minimized.
This comment has been minimized.
I haven't followed discussion or review but was just triggered by the primer. Is this something for an extension? It seems many plugins and packages define non standard dunders (and that's technically allowed by the Python spec just without guarantee that it will always work). |
Ok cool, I can make it an extension. @Pierre-Sassoulas does that make the config for good dunder names moot, or do we want both? |
I think that if we want this check to be used at all, it needs an option to define what good dunder name are (django for example would probably add |
Makes sense, so...
? What's your decision? |
We need the option for good dunder name. |
3bd3f61
to
37ca3dd
Compare
This comment has been minimized.
This comment has been minimized.
9296f29
to
84f410e
Compare
This comment has been minimized.
This comment has been minimized.
658b28c
to
43ba022
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty nice checker @clavedeluna, I think the refactor to put in pylint.constants also make a lot of sense. I'll wait for another reviewer before merging, new checker are always what bring the most issue in a release.
return | ||
|
||
# Detect something that could be a bad dunder method | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed the discussion here, but is there a reason why we don't check for __
? Could _test_
be something private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A function starting with _
is protected, a function starting with __
is private those should not raise. But _init__
is a bad dunder name that we should detect so a single underscore is enough.
6c940a3
Thank you for your work on this checker @clavedeluna, I think this is a nice addition to pylint ! Let's see how it goes, but I'm pretty confident we won't have too much issues opened for this one. |
Type of Changes
Description
The new
bad-dunder-name
checker flags any potentially misspelled known dunder name method and flags any method defined with_..._
that's not one of the pre-defined ones.If you can think of more test cases, happy to add them.
Closes #3038