Missed nullness pattern in migration

[johnniwinther]

When the migration tries to migrate code like

class Class {
  Foo field;

  Bar toBar() {
    if (field == null) {
      throw 'Field is null';
    }
    return field as Bar;
  }
}

it proposes

class Class {
  Foo? field;

  Bar? toBar() {
    if (field == null) {
      throw 'Field is null';
    }
    return field as Bar?; // This should be `as Bar`.
  }
}

that is, failing to see that the cast must be to a non-nullable type.

[lrhn]

Likely missed that because we don't do promotion of instance variables. It's not safe to assume that field has the same value in field as Bar as it had in field == null - a subclass could override field and return something new each time.
(Probably won't, but compilers can't rely on "probably").

Not saying that migration tools can't assume that the code is reasonable, though. It might make sense for the migration tool to assume that checks do promote field, and so field as Bar could assume that field is not null and not make it as Bar?.
(Not sure how easy that would be, though. It won't be able to piggy-back on the existing analysis for that because we don't do that for instance fields).

[johnniwinther]

cc/ @stereotype441

[stereotype441]

Likely missed that because we don't do promotion of instance variables. It's not safe to assume that field has the same value in field as Bar as it had in field == null - a subclass could override field and return something new each time.
(Probably won't, but compilers can't rely on "probably").

Not saying that migration tools can't assume that the code is reasonable, though. It might make sense for the migration tool to assume that checks do promote field, and so field as Bar could assume that field is not null and not make it as Bar?.
(Not sure how easy that would be, though. It won't be able to piggy-back on the existing analysis for that because we don't do that for instance fields).

Agreed. This is one of the prinicpal improvements discussed in #40566. Glad to hear I'm not the only one who thinks this would be beneficial.

[stereotype441]

Duplicate of #40566