Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
Contributor

@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
Contributor 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
Contributor 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
# for free to join this conversation on GitHub. Already have an account? # 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