sourcery icon indicating copy to clipboard operation
sourcery copied to clipboard

Improvement to be made between `use-contextlib-suppress` and `do-not-use-bare-except`

Open diceroll123 opened this issue 3 years ago • 2 comments

Issue description or question

In as few words as possible,

try:
    int("123.45")
except:
    pass

The above code is first suggested to use do-not-use-bare-except, and upon adding except Exception: on line 3, it leads me into the use-contextlib-suppress refactoring, which I kind of expected it to do in the first place given the narrowness of this use case.

Sourcery Version

v0.11.2

Code editor or IDE name and version

VSC Version: 1.66.1 (user setup) Commit: 8dfae7a5cd50421d10cd99cb873990460525a898 Date: 2022-04-06T14:50:12.141Z 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.22593

diceroll123 avatar Apr 13 '22 03:04 diceroll123

Hello @diceroll123! Thanks for opening this issue.

When we introduce Exception with the do-not-use-bare-except suggestion there's a small chance that the functionality of the code could be changed, so we want to make sure that you have a chance to review that change separately before then proposing refactoring it with use-contextlib-suppress.

In your ideal scenario would you be able to have Sourcery directly suggest a change from:

try:
    int("123.45")
except:
    pass

to

with contextlib.suppress(Exception):
    int("123.45")

?

ruancomelli avatar Apr 13 '22 14:04 ruancomelli

Correct! Sorry for the lack of clarity.

I'd like to think in this very very specific, strict case of a bare except + pass is when it could skip straight to the contextlib.suppress implementation, is what I'm trying to say here.

This is opposed to having any code in the except block, of course. I'm not here to disparage the do-not-use-bare-except solution, it's a good one too!

Overall, I'm not emotionally invested in this particular idea, and I trust your judgement! 😄

diceroll123 avatar Apr 13 '22 16:04 diceroll123