sourcery-ai / sourcery

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

Inconsistent `simplify-boolean-comparison` refactoring #232

Open diceroll123 opened 2 years ago

diceroll123 commented 2 years ago

Issue description or question

Sourcery decides not to refactor what is literally the same line. 🤔 Commenting out the test_1 method changes nothing, still won't want to refactor test_2

class Dummy:
    def __init__(self, test: bool = False):
        self._test = test

    @property
    def test(self) -> bool:
        return self._test

    @test.setter
    def test(self, val: bool):
        self._test = val

def test_1():
    m = Dummy()
    m.test = True
    assert m.test == True # Sourcery wants to refactor this line

def test_2():
    m = Dummy(test=True)
    assert m.test == True # Sourcery does not want to refactor this line

Sourcery Version

v0.11.2

Code editor or IDE name and version

VSCode Version: 1.66.2 (user setup) Commit: dfd34e8260c270da74b5c2d86d61aee4b6d56977 Date: 2022-04-11T07:46:01.075Z Electron: 17.2.0 Chromium: 98.0.4758.109 Node.js: 16.13.0 V8: 9.8.177.11-electron.0 OS: Windows_NT x64 10.0.22598

brendanator commented 2 years ago

Thanks for the report @diceroll123

We don't yet analyse classes, which means that we don't know that Dummy(test=True) sets the property m.test. On the other hand we assume that setting an attr on an object means that we know what its type is.

This is something that we will support in the future.

MinekPo1 commented 2 years ago

Maybe using static type checking would be useful here?

In that case if a function returns a bool, it could also apply the refactoring:

def foo() -> bool:
    return True

if foo() == True:  # <-- replace with `if foo():`
    print("foo")