TRIO104: return/yield in finally

[jakkdl]

def foo():
  try:
    raise trio.Cancelled()
  finally:
    return

will silently eat the error without re-raising. I'm not sure how often people do this, but it seems crazy bad to ever return/yield in finally. Should be a pretty simple addition.

edit: or if another exception is raised inside the finally

[jakkdl]

oh, return/yield in finally always eats the exception regardless of if there's a handler for it. So this will also fail to re-raise. Is there any other checker out there that warns about return in finally?

def foo():
  try:
    await trio.sleep(99)
  except trio.Cancelled:
    raise
  finally:
    return

[jakkdl]

Oh and a separate problem is that 103 doesn't check for yield at all:

try:
  ...
except:
  if ...:
    yield
  raise

[Zac-HD]

oh, return/yield in finally always eats the exception regardless of if there's a handler for it. So this will also fail to re-raise. Is there any other checker out there that warns about return in finally?

Yes, flake8-bugbear's B012 warning is for these cases.

[jakkdl]

Ah okay, nice. I forgot that flake8 excludes tests/trio which sort of made me think bugbear didn't check that. Since bugbear is mentioned in the readme it's probably not worth to duplicate that check then? But mostly just a tradeoff between assuming bugbear vs duplicate warnings, implementing the check itself is easy. Implementing it with an option maybe the "best", but that feels a bit overkill.

[Zac-HD]

It's safe to assume that everyone using flake8-trio is also using flake8-bugbear; at most we could make the note in the readme more explicit.

[jakkdl]

Could remove some 107 logic that checks for that if you want cleaner code in that case, but will at least keep that in mind for any future stuff.