Analyzer infers promoted type on locals

[johnniwinther]

The analyzer wrongfully infers promoted types on locals.

Consider this example:

method<T>(T a, T b) {
  if (a is String) {
    var o = a;
    o = b;
    o.length; // Missing error
  }
}

The inferred type for o is T & String which leads to a missing error in the o.length access.

This issue was only recently fixed in the CFE: https://dart-review.googlesource.com/c/sdk/+/133588

cc @stereotype441, @lrhn, @eernstg

[stereotype441]

Interesting corner case! Even though it's not strictly speaking a null safety issue, I'm adding it to the analyzer team's null safety epic, since null safety causes type promotion to happen more frequently, so it's likely to crop up more often in opted in libraries.

[eernstg]

Extra info which may be helpful for others: @johnniwinther just mentioned that the difficulty is demotion of the variable, and also that a promising approach is the following:

Allow the local variable o to get an inferred type which is taken from the intersection type by stripping off the intersection (in the example the inferred type of o would then be T, not T & String), and then o is immediately promoted based on a step where the initialization is considered as a promoting assignment. Then o can have type T & String until the assignment o = b, which will demote it to T (which is now possible), and then o.length will be an error.

[sigmundch]

Just wanted to point to code that started breaking because of this change: #40485

The local variable with the promoted type seems to be final and only assigned in that initialization. Should that be allowed? (I believe it would follow from the alternative approach suggested by @eernstg in the comment above)

[eernstg]

I think the alternative approach would work just as well for a final variable. It might seem slightly convoluted in that case, and maybe we could get away with a rule that says that the inferred type of a final variable can be an intersection type. But as long as it's consistent with the treatment of non-final variables the complexity of the approach in general doesn't grow, so we might as well use this approach for final variables as well.

PS: The alternative approach came from @johnniwinter, but I think it's a good idea. ;)

[stereotype441]

Note: since this is a breaking change, it needs to be pre-announced and pre-tested internally before it's landed.

[johnniwinther]

Also note: the change has already been landed by the CFE seeing only one problem internally.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/136360