Skip to content

Refactor and deprecate resolving of interface implementations in pyreverse #6713

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

Merged
merged 6 commits into from
May 27, 2022

Conversation

DudeNr33
Copy link
Collaborator

  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.
  • If you used multiple emails or multiple names when contributing, add your mails
    and preferred name in script/.contributors_aliases.json

Type of Changes

Type
🔨 Refactoring

Description

Ref #6582
As we also deprecated the use of __implements__ in our own codebase, deprecating the pyreverse functionality to resolve and display implemented interfaces is the right consequence.
To make this change transparent for users, a DeprecationWarning will be issued when such an __implements__ attribute has been found.

Additionally, I removed parameters and checks that were always constant in production use of the code.

@DudeNr33 DudeNr33 added pyreverse Related to pyreverse component Maintenance Discussion or action around maintaining pylint or the dev workflow labels May 27, 2022
@DudeNr33 DudeNr33 added this to the 2.14.0 milestone May 27, 2022
@coveralls
Copy link

coveralls commented May 27, 2022

Pull Request Test Coverage Report for Build 2397637286

  • 9 of 10 (90.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.007%) to 95.51%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pylint/pyreverse/inspector.py 9 10 90.0%
Totals Coverage Status
Change from base Build 2397612156: -0.007%
Covered Lines: 16230
Relevant Lines: 16993

💛 - Coveralls

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Great clean up, apparently it was 16 years overdue 😂

Comment on lines +386 to +388
* ``pyreverse``: Resolving and displaying implemented interfaces that are defined by the ``__implements__``
attribute has been deprecated and will be removed in 3.0.

Copy link
Member

Choose a reason for hiding this comment

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

Could you put it alongside the changelog for #6712, please ? There is going to be a conflict in the Changelog file in #6688 during rebase (as we burst the changelog file), it would be nice to regroup new changelog entry together until this MR is merged 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done 😊

warnings.warn(
"pyreverse will drop support for resolving and displaying implemented interfaces in pylint 3.0. "
"The implementation relies on the '__implements__' attribute proposed in PEP 245, which was rejected "
"in 2006.",
Copy link
Member

Choose a reason for hiding this comment

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

Wow, 2006 😄 I was copy pasting javascript and basic for high school calculator at the time.

Comment on lines -284 to -287
* ``pylint.pyreverse.ASTWalker`` has been removed, as it was only used internally by a single child class.

Ref #6712

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is now under "Deprecations" in order to be grouped with #6713, although this is technically not a deprecation but a hard removal. Let me know if that's OK for you to put it there.

@@ -110,7 +110,7 @@ class Concrete23(Concrete1): pass
("Concrete0", ["MyIFace"]),
("Concrete1", ["MyIFace", "AnotherIFace"]),
("Concrete2", ["MyIFace", "AnotherIFace"]),
("Concrete23", ["MyIFace", "AnotherIFace"]),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason this test changed is that the unit test actually used the default value of True for the now removed herited parameter, but in production this parameter was always False. So after removing the parameter from the function signature I had to update this test case accordingly.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

👌

@DudeNr33 DudeNr33 merged commit 4fb07df into pylint-dev:main May 27, 2022
@DudeNr33 DudeNr33 deleted the refactor-pyreverse-inspector branch May 27, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow pyreverse Related to pyreverse component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants