Error messages with lack of property promotion could be improved

[stereotype441]

@mit-mit points out that the errors that occur when the user attempts to promote a property could be clearer. For example, this code:

class C {
  int? x;
  test() {
    if (x != null) {
      print(x.isEven);
    }
  }
}

Produces the error message:

The property 'isEven' can't be unconditionally accessed because the receiver can be 'null'.

with the following suggested fix:

Try making the access conditional (using '?.') or adding a null check to the target ('!').

and then the following context message (pointing to the declaration int? x;):

'x' refers to a property so it couldn't be promoted.  See http://dart.dev/go/non-promo-property

Two things are unfortunate about this: (1) The user has to read through a lot of error message text before they get to the meat of the problem (properties can't be promoted). (2) The suggested fix isn't great.

It would be better to have an error message like this:

Cannot access 'isEven' because 'x' can be null. Because 'x' is a property, it cannot be promoted to non-null.  See http://dart.dev/go/non-promo-property

and a suggested fix like this:

Try assigning 'x' to a local variable, for example: final x = this.x;

It probably would still be useful to have a context message pointing to the declaration int? x;. Maybe it could say something like:

The property 'x' is declared here.

Note that this isn't the only error message that could potentially be improved; it's just an example.

[InMatrix]

Here is how this error message shows up in VS Code:

Only the first part of the full message is included for some reason.

[stereotype441]

Assigning to Konstantin. As discussed in the meeting, it would also be good to have a quick fix (or assist) that introduces a local variable.

[scheglov]

Hm... Currently we report "not promoted" as attached DiagnosticMessages. This way we can use a smaller number of error codes, and attached reasons why the receiver was not promoted to non-nullable. We could specialize PropertyNotPromoted to report a more specific ErrorCode with a better message, although it looks that we would have to move the attached message into the main message (the "because" part). Is this what you have in mind?

PropertyNotPromoted does not tell us where the promotion was attempted. Or do I miss it? If it is not there, we still can support a quick fix for probably the most often case of if (o.x != null) { o.x.foo }.

[stereotype441]

PropertyNotPromoted does not tell us where the promotion was attempted.

You're right. I'll do some digging to see how difficult it would be to add this functionality.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/227900 will add a fix for a few cases.