sourcery-ai / sourcery

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

Erroneous "remove redundant pass statement" suggestion #237

Closed diceroll123 closed 2 years ago

diceroll123 commented 2 years ago

Issue description or question

Looks like pass is being detected from a method invoke

def blah():
    return None

def test_do_thing():
    blah() # Sourcery suggests Remove redundant pass statement, Remove `assert True` statements
    assert True

def test_do_thing2():
    # blah() # Sourcery suggests Remove `assert True` statements
    assert True

Sourcery Version

v0.11.4

Code editor or IDE name and version

VSCode Version: 1.67.0 (user setup) Commit: 57fd6d0195bb9b9d1b49f6da5db789060795de47 Date: 2022-05-04T12:06:02.889Z Electron: 17.4.1 Chromium: 98.0.4758.141 Node.js: 16.13.0 V8: 9.8.177.13-electron.0 OS: Windows_NT x64 10.0.22616

bm424 commented 2 years ago

Hi @diceroll123!

Took a quick look at this just now, and I think this is expected behaviour. Here's what's happening: assert True (a redundant statement) is being refactored into pass. Then the pass, in test_do_thing, is removed. We do this in two steps because (as you've shown in your example) in some cases it's incorrect to completely remove assert True - we have to leave some statement in place. The reason this is showing up now is due to a bug fix to address this exact issue in other contexts, such as for loops.

blah is actually irrelevant here - you can remove the function entirely or replace it with some other statement and you'll still see this behaviour. A silly example:

def what_is_this():
    assert False
    assert True  # sourcery removes this line in two steps

def please_no_why():
    assert True  # sourcery replaces this with `pass` - the first step in the refactoring above