Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix crash in modified_iterating checker for set defined as a class attribute #7541

Merged
merged 3 commits into from
Sep 30, 2022
Merged

Fix crash in modified_iterating checker for set defined as a class attribute #7541

merged 3 commits into from
Sep 30, 2022

Conversation

clavedeluna
Copy link
Contributor

Type of Changes

Type
✓ 🐛 Bug fix

Description

Add functional test that demonstrates we forgot to correctly call the attrname attribute for an Attribute node. This fixes that.
Could potentially do some refactoring because we do this operation three times

            if isinstance(iter_obj, nodes.Attribute):
                obj_name = iter_obj.attrname
            else:
                obj_name = iter_obj.name

but the downside would be passing in as an argument. It could be best to leave as is in some cases.

Closes #7528

@DanielNoord DanielNoord self-requested a review September 29, 2022 16:13
@clavedeluna clavedeluna marked this pull request as ready for review September 29, 2022 16:15
@coveralls
Copy link

coveralls commented Sep 29, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 3158149961

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0003%) to 95.315%
Totals Coverage Status
Change from base Build 3146730788: 0.0003%
Covered Lines: 17110
Relevant Lines: 17951
💛 - Coveralls

@github-actions

This comment has been minimized.

Sign in to view
@Pierre-Sassoulas Pierre-Sassoulas added the Crash 💥 A bug that makes pylint crash label Sep 29, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.4 milestone Sep 29, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer label Sep 29, 2022
Pierre-Sassoulas
Pierre-Sassoulas requested changes Sep 29, 2022
View reviewed changes
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.

LGTM, thank you. I opened pylint-dev/astroid#1811 in astroid, because this problem happens A LOT.

doc/whatsnew/fragments/7528.bugfix Outdated Show resolved Hide resolved
@DanielNoord @Pierre-Sassoulas
Apply suggestions from code review
4d8611f 
Co-authored-by: Pierre Sassoulas <[email protected]>
@DanielNoord DanielNoord enabled auto-merge (squash) September 30, 2022 10:42
@DanielNoord DanielNoord merged commit aca8dd5 into pylint-dev:main Sep 30, 2022
@github-actions
Copy link
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 4d8611f

orSolocate pushed a commit to orSolocate/pylint that referenced this pull request Oct 1, 2022
@clavedeluna @DanielNoord
@Pierre-Sassoulas @orSolocate
Fix crash in modified_iterating checker for set defined as a clas…
e5e30c1 
…s attribute (pylint-dev#7541)

Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
@Pierre-Sassoulas Pierre-Sassoulas changed the title bugfix for Attribute.attrname Fix crash in modified_iterating checker for set defined as a class attribute Oct 9, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Backported and removed Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer labels Oct 10, 2022
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this pull request Oct 10, 2022
@clavedeluna @DanielNoord @Pierre-Sassoulas
Fix crash in modified_iterating checker for set defined as a clas…
d4fc390 
…s attribute (pylint-dev#7541)

Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
Pierre-Sassoulas added a commit that referenced this pull request Oct 10, 2022
@clavedeluna @DanielNoord @Pierre-Sassoulas
Fix crash in modified_iterating checker for set defined as a clas…
983d5fc 
…s attribute (#7541)

Co-authored-by: Daniël van Noord <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
@rohitthakur2590 rohitthakur2590 mentioned this pull request Oct 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielNoord DanielNoord DanielNoord approved these changes

@Pierre-Sassoulas Pierre-Sassoulas Pierre-Sassoulas approved these changes

Assignees
No one assigned
Labels
Backported Crash 💥 A bug that makes pylint crash
Projects
None yet
Milestone
2.15.4
Development

Successfully merging this pull request may close these issues.

regression: modified_iterating_checker exception
4 participants
@clavedeluna @coveralls @Pierre-Sassoulas @DanielNoord