Migration tool doesn't recognize Never in flow control

[nex3]

The migration tool doesn't recognize the effect functions that return Never have on the control flow of a function. Consider the following unmigrated code:

Never bail() => throw "oh no";

int foo() {
  int a;
  if (a == null) bail();
  return a;
}

The migrator correctly tries to change int a to int? a but it also tries to change int foo() to int? foo(), which is unnecessary.

[eernstg]

Note the discussion in dart-lang/language#1143 about soundness issues that arise because a certain location in code is considered to be unreachable. The point is that (as long as mixed-version programs and the associated unsound null checking execution mode exist) it is possible for an expression whose static type is non-nullable to evaluate to null. For instance, bail() could return null. You could also have used int a = bar(); where bar is a function in legacy code (not under migration) that has return type int, and it could return null.

There is no guarantee that these nulls will be detected immediately, because it would be too costly in terms of run-time performance to enforce checks in every situation where it could occur. So the static analysis relaxes the analysis slightly, in order to consider certain otherwise-unreachable locations reachable, and that might be the reason for this behavior.

[stereotype441]

Note the discussion in dart-lang/language#1143 about soundness issues that arise because a certain location in code is considered to be unreachable. The point is that (as long as mixed-version programs and the associated unsound null checking execution mode exist) it is possible for an expression whose static type is non-nullable to evaluate to null. For instance, bail() could return null. You could also have used int a = bar(); where bar is a function in legacy code (not under migration) that has return type int, and it could return null.

Actually, the final resolution we chose for dart-lang/language#1143 addresses things slightly differently (see dart-lang/language#1143 (comment)). There are several parts to the solution we chose, but the part that's relevant here is that in unsound null checking mode, the compiler inserts an invisible throw right after evaluating an expression whose static type is Never. So even with unsound null checking, the user can rely on the fact that a call to a function with static type Never will never return, and so can flow analysis. Which means that this is a perfectly acceptable program:

Never bail() => throw "oh no";

int foo() {
  int? a;
  if (a == null) bail();
  return a;
}

And I think it's what the migration tool ought to output.

I think what actually happened here was just an oversight: when writing the EdgeBuilder phase of the migration tool, we forgot to implement the rule that expressions of type Never will never return 😃

[stereotype441]

As of 1c7fe71, the null safety migration tool has been removed from active development and retired. No further work on the tool is planned.

If you still need help, or you believe this issue has been closed in error, please feel free to reopen.