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

Enhancement/add checker consider using min max builtin #4359

Conversation

Qwiddle13
Copy link
Contributor

@Qwiddle13 Qwiddle13 commented Apr 15, 2021

Steps

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • 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.

Description

Adds a warning to replace code like

if value < 10:
    value = 10

with its respective

value = max(value, 10)

Type of Changes

Type
✨ New feature

Related Issue

Closes #3406

@Qwiddle13
Copy link
Contributor Author

Qwiddle13 commented Apr 15, 2021

Almost all of the changes here are from previous commits to #3406 by user manderj. I mostly just polished it up and got the test cases working with test_functional.py and they seem to pass now.

Is this a big enough change that Step 3 (doc/whatsnew/<current release.rst>) needs to be be done too?

@coveralls
Copy link

coveralls commented Apr 15, 2021

Coverage Status

Coverage increased (+0.01%) to 91.62% when pulling 058d609 on Qwiddle13:enhancement/add-checker-consider-using-min-max-builtin into c7ac7c0 on PyCQA:master.

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.

Nice change and good functional tests, I left some remarks (that will requires to regenerate the output of functional test) but I'd merge this.

pylint/checkers/refactoring/refactoring_checker.py Outdated Show resolved Hide resolved
pylint/checkers/refactoring/refactoring_checker.py Outdated Show resolved Hide resolved
pylint/checkers/refactoring/refactoring_checker.py Outdated Show resolved Hide resolved
pylint/checkers/refactoring/refactoring_checker.py Outdated Show resolved Hide resolved
Qwiddle13 and others added 4 commits April 17, 2021 12:24
Co-authored-by: Pierre Sassoulas <[email protected]>
The broadest type should be astroid.If since _check_consider_using_min_max_builtin is only called by visit_if. This aligns with what I saw at runtime.
@Qwiddle13
Copy link
Contributor Author

I think I'm good with this as it stands. @Pierre-Sassoulas do you have or know who has write access and can merge?

@Pierre-Sassoulas
Copy link
Member

Thank you for this new checker, it will be included in the next version that we'll release soonish :)

@Pierre-Sassoulas Pierre-Sassoulas merged commit bafb149 into pylint-dev:master Apr 17, 2021
@Qwiddle13 Qwiddle13 deleted the enhancement/add-checker-consider-using-min-max-builtin branch April 17, 2021 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggesting to use min()/max()
3 participants