-
-
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
Added deprecated decorators check #4513
Added deprecated decorators check #4513
Conversation
Hm checks are failing. I need to investigate. For now I will put MR as draft... |
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.
Thank you for this ! I nitpicked a little about typing and formatting. Maybe you could add a little functional tests with an @abc.abstractclassmethod
too. But mostly worry about your work environment :) So many deprecations to take care of ! Are you okay with the legacy code π ?
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.
Thanks for working on that! I did look at the pytest errors. Most fail because they use abc.abstractproperty
π In two cases the decorator is uninferable which cause an error.
class Foo:
def __init__(self):
self._baz = 84
def method(self):
return self._baz
@method.setter # Invalid decorator
def method(self, value):
self._baz = value
This isn't valid python as method
doesn't have a setter
(it would require the @property
decorator), but I think that we shouldn't raise an unexpected error for it never the less.
I would appreciate if you could add this as a test case as well.
pylint/checkers/deprecated.py
Outdated
if isinstance(next(node.get_children()), astroid.Call): | ||
qname = next(node.get_children()).func.inferred()[0].qname() | ||
else: | ||
qname = next(next(node.get_children()).infer()).qname() |
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'm not sure you can be certain that the result will always have a qname()
method here.
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.
Instead of .infer()
try safe_infer
from the pylint/checkers/utils.py
module.
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
for more information, see https://pre-commit.ci
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.
Thank you for this checker ! It will be available in 2.9.0 which should release shortly.
Merging as @cdce8p comments were taken into account. |
Thank you @Pierre-Sassoulas ! |
Steps
doc/whatsnew/<current release.rst>
.Description
This PR introduces new check for deprecated decorators:
Type of Changes
Related Issue
Closes #4429