Open matanlurey opened 8 months ago
The lint description does not say whether throw
counts as control flow.
Since any function call can throw, it's hard to disallow completely, and a function returning Never
is basically the same as a throw
.
If a second error happens, it's perhaps fair that an original error gets clobbered and lost. After all, there is still some error.
Also, it sometimes makes sense to throw during clean-up, like
var resource = alloc();
try {
...
} finally {
respurce.dispose();
if (!resouce.isDisposed) throw SomeException("Resource not disposed");
}
or similar. So we probably don't want to warn about just any throw.
So, if anything, it might make sense to warn if a finally block always throws.
Warn if control flow cannot reach the end of the finally block. That should catch any onconditional control flow or Never
-function-calls.
Any non-error control flow should still be warned about, because those can also hide an error, and not behind a second error.
I've proposed the idea of explicit control flow
here: https://github.com/dart-lang/linter/issues/4054.
Under a policy that enforces "explicit control flow" exit(...)
would have been highlighted as implicit control flow. Fixing implicit control flow would have caused throw_in_finally
/control_flow_in_finally
to kick in which would have prevented this bug.
Any type of implicit control flow can, in my experience, lead to similar bugs like the one that motivated this issue. I claim, that the issue is not with control_flow_in_finally
, but with allowing Never-related implicit control flow.
In https://github.com/flutter/engine/pull/50255 @jonahwilliams and I debugged a tough "this program doesn't catch errors" bug.
It was basically narrowed down to this minimal program:
https://dart.dev/tools/linter-rules/control_flow_in_finally appears to be able to philosophically catch this bug.
I wonder if we could enhance it to lint functions that return
Never
, i.e.: