add b040: exception with note added not reraised or used

[jakkdl]

Fixes #474

There's some alternatives when it comes to the scope of this check, namely whether these two should be caught

1

def foo():
  my_exc = ValueError()
  my_exc.add_note("I want to use this variable")
  # ... but then completely forgets to raise

I.e. arbitrary exception created anywhere, that appears to only be used for having a note added. This one gets tricky as its complexity approaches that of a type checker, which usually handles unused variables. If we assume that only exceptions will ever have a method called add_note we can sidestep trying to do any type inference.
Though this would hit false alarms with stuff like

def my_add_note(e: Exception) -> None:
  e.add_note("foo")

so it probably isn't worth it.

2

try:
  ...
except:
  f = ValueError()
  f.add_note("")

This is essentially the same as the first one, except it's happening within an except scope and much more clearly looks like a bug.

Handling either case will require exceptions_tracked to be a full dict, and the logic in a bunch of places to be beefed up a bit. If we don't care to go with that, I will simplify the type (and rename it, it's an awful name atm) of exceptions_tracked and simplify/clean up some code handling it.

Handling the lambda case, or other functions defined inside an except handler, wouldn't be super complicated - but I don't really think it's worth even the slight complexity.

[cooperlees]

Thanks!

In regards to the questions:

1. Agree, don't think it's worth it - especially with false positives
2. I think since it's in an except clause something should be done with the exception there .,.. so it's a bug we should care about. Up to you how far you want to go in regards to lambdas and other

• Happy to start catching the blatant bugs - But let's note possible enhancements in the issue as someone else might want to implement.

Will request changes so you can continue on here :)

[cooperlees]

This highlights we should type annotate all this code. #478

[cooperlees]

Wonder if it's worth creating an issue to break this up ...

[jakkdl]

It is quite messy, with code for B013, B029, B030, B036 and B040 interwoven. Fairly straightforward to split out B013, B029 and B030 though - so I'll just go ahead and do that.

[jakkdl]

2. I think since it's in an except clause something should be done with the exception there .,.. so it's a bug we should care about.

This one is tricky if we want to avoid a false alarm for

except:
  f = ValueError()
  my_exceptions.append(f)
  f.add_note("")

We don't want to try to detect creation of exceptions, as handling anything other than built-in exceptions would require full-on type tracking. So what we can do is detect when add_note is used on an object. But in this example we would need to write a custom visitor to visit twice to pick up that f is being used because it happens before add_note. I guess doing an ast.walk for .add_note() calls is a decent solution though.

And there's other thorny cases that will raise false alarms even if we limit it to "the add_note is in an ExceptHandler".

my_exc = ValueError()
try:
  do_thing()
except ValueError as e:
  my_exc.add_note(str(e))
raise my_exc

The reason the straightforward case is not vulnerable to this kind of stuff is that the caught exception gets deleted at the end of the exception handler, so we know for sure it's limited to the scope of the ExceptHandler.
So unless we're fine with the occasional false alarm I suspect I shouldn't bother trying to handle this case either. Maybe the ruff folks have sufficient infra that it's plausible for them to do so if/when they reimplement it.

[cooperlees]

Hmm, yeah. Some interesting false positives here. I'm happy with not covering this and starting simpler. You do what you want here. I think it's better to start with less false positives and do dedicated PRs handling more complex edge cases in general.

You have merge conflicts now to work out now too.

[jakkdl]

simplified and cleaned up the code, should be ready for final review.

[cooperlees]

Many thanks and very nice. Love the tests with nice comments to see what you're aiming for over the file.