sdk icon indicating copy to clipboard operation
sdk copied to clipboard

[Extension type] Wrong error messages in CFE in case of invalid assignment

Open sgrekhov opened this issue 7 months ago • 4 comments

CFE produces the following errors below.

extension type ET1(int v) {}
extension type ET2(int v) implements Object {}

test1() {
  ET1 et1 = ET1(42);
  et1 = null; // Error: The value 'null' can't be assigned to a variable of type 'ET1' because 'ET1' is not nullable.

  ET2 et2 = ET2(42);
  et2 = null; // Error: The value 'null' can't be assigned to a variable of type 'ET2' because 'ET2' is not nullable.
}

test2() {
  ET1 et1 = ET1(42);
  Object o1 = et1; // Error: A value of type 'ET1' can't be assigned to a variable of type 'Object' because 'ET1' is nullable and 'Object' isn't.

  ET2 et2 = ET2(42);
  Object o2 = et2; // No error
}

main() {
  test1();
  test2();
}

So, according to these errors in test1() ET1 is not nullable but in test2() it is nullable. Analyzer's error messages for this case are better.

For test1() they are "A value of type 'Null' can't be assigned to a variable of type 'ET1'. Try changing the type of the variable, or casting the right-hand type to 'ET1'. • invalid_assignment"

For test2() the error in the analyzer is "A value of type 'ET1' can't be assigned to a variable of type 'Object'. Try changing the type of the variable, or casting the right-hand type to 'Object'. • invalid_assignment"

Dart SDK version: 3.9.0-74.0.dev (dev) (Tue Apr 29 21:02:55 2025 -0700) on "windows_x64"

sgrekhov avatar May 05 '25 13:05 sgrekhov

Good catch!

I think it would suffice if the CFE were to say "..because 'ET1' is potentially nullable..".

eernstg avatar May 05 '25 14:05 eernstg

Indeed. It's not that it's nullable, it's that it's not non-nullable. Or just "not a subtype of Object".

lrhn avatar May 07 '25 07:05 lrhn

cc @chloestefantsova

johnniwinther avatar May 07 '25 09:05 johnniwinther

When we fix this we should also improve the diagnostics from the analyzer.

So, according to these errors in test1() ET1 is not nullable but in test2() it is nullable. Analyzer's error messages for this case are better.

They don't say anything about nullability, so they don't contradict each other (which is better in one sense), but I'd propose two changes.

  1. They ought to say "The value 'null'" rather than "A value of type 'Null'". There's only one value of type Null and users are more familiar/comfortable with the lowercase spelling than they are the uppercase.

  2. They ought to say something about nullability. I think it's helpful to users to point out when the problem is strictly related to nullability because the way you solve a nullability issue is different than the way you solve other type issues.

And, although it wasn't mentioned above, the third diagnostic from the analyzer also doesn't say anything about nullability, and it should.

bwilkerson avatar Jun 06 '25 16:06 bwilkerson