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

New check: break at the top of while True loop #8015

Closed
nickdrozd opened this issue Jan 2, 2023 · 1 comment · Fixed by #8021
Closed

New check: break at the top of while True loop #8015

nickdrozd opened this issue Jan 2, 2023 · 1 comment · Fixed by #8021
Assignees
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@nickdrozd
Copy link
Collaborator

Current problem

A not-uncommon pattern is to check some condition at the top of a while True loop and break if the condition is met:

while True:
    if condition:
        break

    do_stuff()

There is an instance of this in the Pylint codebase.

Desired solution

It would be more straightforward and less verbose to negate the condition and move it into the while expression:

while not condition:
    do_stuff()

Add a new check for the following circumstance:

  1. There is a while loop with True condition (or more generally, constant condition);
  2. the first statement of the loop body is an if statement; and
  3. the first statement of the if body is a break statement.

In this case, emit a suggestion to negate the condition and move it to the while.

I don't know what the name of the warning should be. Suggestions welcome!

Additional context

No response

@nickdrozd nickdrozd added Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 2, 2023
@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jan 2, 2023
@yushao2 yushao2 self-assigned this Jan 4, 2023
@yushao2
Copy link
Collaborator

yushao2 commented Jan 27, 2023

it's finally done! thanks for this suggestion :)

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.16.0 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants