RET504 invalid autofix

[tylerlaprade]

def foo(data):
    with contextlib.suppress(UnicodeDecodeError):
        data = data.decode()
    return data

is autofixed to

def foo(data):
    with contextlib.suppress(UnicodeDecodeError):
        return data.decode()

These are not equivalent. The first one will always return data, either decoded or the original. The second will return the decoded data if there's no exception, otherwise, it will return None. This introduced a bug into my code that I only caught from some miscellaneous Pyright type-checking far away from the autofix.

[zanieb]

Thanks for the report @tylerlaprade. It looks like perhaps we should not raise RET504 inside context managers?

[dhruvmanila]

I think this is a special case as data is defined outside (before) the context manager and/or that the contextlib.suppress context manager is used here.

[zanieb]

@dhruvmanila the problem is that any context manager could suppress exceptions, we can't really know. I agree that data is defined before the context is important as well though.

[charliermarsh]

Oh jeez what an interesting case.

[tylerlaprade]

Sourcery has been autofixing all my try-catch-pass blocks to the more concise contextlib.suppress (which I'm happy about!), but it opens up potential opportunities for this bug.

[charliermarsh]

Shameless plug that Ruff can do it too 😂 https://beta.ruff.rs/docs/rules/suppressible-exception/

[tylerlaprade]

Ah, I do have SIM enabled already, so I guess I have two sources of the autofixes!

[tylerlaprade]

This appears to be fixed in 0.1.15 so I will close this, thanks!