Introduce B030; fix crash on weird except handlers

[JelleZijlstra]

Fixes #345.

This introduces a new bugbear check against things like except call(): or except 1:. This could be legitimate code (if you have a function that returns an exception class), but I'm not sure that's something that would ever come up in practice.

bugbear would previously crash on any except handler that is not a Tuple, Name, Attribute, or Call.

[cooperlees]

haha - I always feel bad when I push something somewhere and black is upset at my formatting! I can try fix before merge eventually too if you don't beat me.

[cooperlees]

LGTM - Thanks mate. One of us can just fix the formatting.

[JelleZijlstra]

Looks like you just merged a different B029 so there's a little more work to do :)

I turned off my editor's format-on-save because it's annoying when writing test cases for Black, lol. Should probably figure out a way to keep it on for other projects.

[JelleZijlstra]

pre-commit.ci autofix

[JelleZijlstra]

All green now.

[FozzieHi]

LGTM, but do we also want to allow function calls (by allowing ast.Call)?

For example:

def exceptions():
    return ValueError, KeyError


try:
    pass
except exceptions():
    pass

Not entirely sure how common it is, and we obviously can't check the return values of the function, but I think it'd still be useful for some use cases.

[JelleZijlstra]

I guess we could allow it. People could also plausibly write except ValueError(): pass instead of except ValueError: pass, which this check would help catch.

@cooperlees what do you think, should we allow calls in except handlers?

[FozzieHi]

That's a good point. I think we definitely want to identify that except ValueError() case, and I think anyone using the legitimate function call can simply just noqa it. I'm fine with keeping it how it is.

[cooperlees]

This is a strange pattern, but I am sure valid. Is there any harm doing a function call here? I don't see any so probably best we handle this edge case ... But happy for another PR to do that ... Was going to release a new version later today.

[FozzieHi]

What Jelle mentioned above, except ValueError() results in a TypeError. In this current version we would catch that. If we added ast.Call to the list of permitted nodes then we wouldn't catch that, but we would allow for this, which is valid:

def exceptions():
    return ValueError, KeyError


try:
    pass
except exceptions():
    pass

I think that we would want to catch the except ValueError() over allowing the above valid code (so we shouldn't add ast.Call to the list of permitted nodes), with the user being able to noqa any valid code.

[cooperlees]

Yeah, lets see if it's a common pattern (via issues opened) and if so address it then. Thanks again all!