`control_flow_in_finally` could be enhanced to consider functions that return `Never`

[matanlurey]

In flutter/engine#50255 @jonahwilliams and I debugged a tough "this program doesn't catch errors" bug.

It was basically narrowed down to this minimal program:

void main() async {
  try {
    innerProgram();
  } catch (e, s) {
    exit(1);
  }
}

void innerProgram() async {
  try {
    await runAndFail();
  } finally {
    // BUG: Even if a failure happens, the program will terminate.
    exit(0);
  }
}

void runAndFail() async {
  throw 'Fail';
}

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.:

void main() {
  try {} finally {
    // LINT
    throw 'Big Bad';
  }
}

void main() {
  Never panic() => throw 'Big Bad';

  try {} finally {
    // LINT
    panic();
  }  
}

void main() {
  try {} finally {
    // LINT
    exit(0);
  }
}

[lrhn]

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.

[modulovalue]

I've proposed the idea of explicit control flow here: #59032.

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.