sourcery-ai / sourcery

Instant AI code reviews
https://sourcery.ai
MIT License
1.53k stars 66 forks source link

`simplify-boolean-comparison` should not be triggered by `is` operator #250

Open Czaki opened 2 years ago

Czaki commented 2 years ago

Issue description or question

Simplifying boolean comparison should not touch is operator usage as it directly checks type, not result of conversion to bool.

Zrzut ekranu z 2022-06-27 10-36-52

Sourcery Version

0.12.1

Code editor or IDE name and version

PyCharm 2022.1.3 (Professional Edition) Build #PY-221.5921.27, built on June 21, 2022

OS name and version

Linux

reka commented 2 years ago

Hi @Czaki ,

Thanks for bringing up this issue.

It's a good question :thinking: : I also think is False sounds natural, especially in a test.

But PEP 8 says not to compare boolean values to True or False using == or is, so we've decided to add this as a general rule.

Don’t compare boolean values to True or False using ==:

# Correct:
if greeting:
# Wrong:
if greeting == True:

Worse:

# Wrong:
if greeting is True:

https://peps.python.org/pep-0008/#programming-recommendations

Skipping This Recommendation

If you want to keep the syntax with is False, you can skip this recommendation by Sourcery:

# sourcery skip: simplify-boolean-comparison

This will deactivate all simplify-boolean-comparison recommendations in the current function.

Cheers, Reka

Czaki commented 2 years ago

But there is a syntax to disable it globally for giving a tree of files (whole test folder)?

I know that mypy is the best solve of this problem, but not all projects are ready for this.

reka commented 2 years ago

But there is a syntax to disable it globally for giving a tree of files (whole test folder)?

Unfortunately, we don't have this current configuration possibility yet. I see why it would sometimes make sense to disable a refactoring only for a subfolder, e.g. only for the tests. :thinking: We'll consider adding this configuration option.

The current options are:

The steps to disable simplify-boolean-comparison for the whole project:

  1. Create a .sourcery.yaml project config file in the root directory of the project.
  2. Add this snippet to .sourcery.yaml:
refactor:
  skip:
    - simplify-boolean-comparison

Related docs about the project config file

It's great to see a cool open source project like napari using Sourcery. :sunglasses: Would you mind having a quick chat with us? It would be awesome to hear which (further) improvements would make it more useful for you.