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.

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

Fixed false negative of consider-using-enumerate on attributes #4508

Merged
merged 5 commits into from
May 26, 2021

Conversation

yushao2
Copy link
Collaborator

@yushao2 yushao2 commented May 25, 2021

Steps

  • Add a ChangeLog entry describing what your PR does.
  • Write a good description on what the PR does.

Description

Fixed false negative of consider-using-enumerate not emitting when iterating over an attribute.

Also, included test scenario described in #3657

Type of Changes

Type
🐛 Bug fix

Related Issue

Closes #3657

@coveralls
Copy link

coveralls commented May 25, 2021

Coverage Status

Coverage increased (+0.008%) to 91.835% when pulling 942aa5a on yushao2:bugfix-3657 into d7c904f on PyCQA:master.

@Pierre-Sassoulas Pierre-Sassoulas added the False Negative 🦋 No message is emitted but something is wrong with the code label May 25, 2021
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.

👍

Comment on lines 84 to 87
for i, _ in enumerate(self.options_providers):
if provider.priority > self.options_providers[i].priority:
self.options_providers.insert(i, provider)
break
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for i, _ in enumerate(self.options_providers):
if provider.priority > self.options_providers[i].priority:
self.options_providers.insert(i, provider)
break
for i, options_provider in enumerate(self.options_providers):
if provider.priority > options_provider.priority:
self.options_providers.insert(i, provider)
break

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops, oversight on my part -- will accept this suggestion. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

We can probably add a new checker for it ;)
Something like list-index-lookup?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sounds good, was thinking something along the lines too

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.8.3 milestone May 26, 2021
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks 🐬

@cdce8p cdce8p merged commit 12af1ce into pylint-dev:master May 26, 2021
@yushao2 yushao2 deleted the bugfix-3657 branch May 26, 2021 10:24
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.8.3, 2.9.0 May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Negative 🦋 No message is emitted but something is wrong with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

consider-using-enumerate doesn't trigger if iterated object is an attribute
4 participants