Analyzer won't promote type of member variables

[Hixie]

The following code results in The getter 'x' isn't defined for the class 'A' • test.dart:13:17 • undefined_getter even though it can be statically proved that it is fine (since nothing can change the type of _a before we access the getter x).

class A	{ }
class A1 extends A { final String x = 'a1.x'; }
class A2 extends A { final String y = 'a2.y'; }

class B	{
  A _a; 
  void setA(A value) {
    _a = value;
  }
  String foo() {
    // A _a = this._a; // with a local variable, it works
    if (_a is A1)
      return _a.x;
    if (_a is A2)
      return _a.y;
    return null;
  }
}

void main() {
  final B b =	new B();
  b.setA(new A1());
  print(b.foo());
}

[bwilkerson]

I believe that this is working as specified. The specification explicitly defines type promotion to apply to local variables and formal parameters; fields are not promoted.

[Hixie]

I don't doubt that it's working as specified, I was asking for the specification to be changed. :-)

[matanlurey]

Might I suggest we track all requests for enhanced type promotion in one place/bug (i.e. #25565 or perhaps a new thread on dart-lang/language)? There are multiple, sometimes duplicate threads asking for similar capabilities. It would also be good to have someone on the language team sort of clarify what the team's stance is, since some of these requests (like this one) either require unsafe is checks or require potentially significant changes to the language.

Consider:

abstract class X {
  Object get id;
}

class Y extends X {
  @override
  Object get id => 'Y';

  void newMethod() {
    if (this.name is String) {
      print(name.toUpperCase());
    }
  }
}

class Z extends Y {
  var _returnString = true;
 
  // Evil, but completely valid in the language.
  @override
  Object get id {
    if (_returnString) {
      _returnString = false;
      return 'Hello';
    }
    _returnString = true;
    return 5;
  }
}

void main() {
  // Error: Type int does not have an instance method 'toUpperCase()'.
  Z().newMethod();
}

To be able to promote class-level members, we would need either:

• (Breaking) Consider all operators to have an implicit as cast in the context
• (New) An unsafe is operator, i.e. is~, where an as cast is automatically inserted in the context
• (New) Non-virtual (non-overridable) class members
• (New) Sealed classes

For example, for the above we could either:

sealed class Y extends X { ... } 

or

class Y extends X {
  @override
  final Object get id => 'Y';
}

... that would prevent the patterns in Z entirely, and allow the safe is check.

[Hixie]

The previous comment seems to be about getters, which (as noted in that comment) you can't promote sanely. The original comment was only about member variables (fields), in a case where you statically prove that there's no possibility of shenanigans.

[matanlurey]

@Hixie:

The original comment was only about member variables (fields), in a case where you statically prove that there's no possibility of shenanigans.

That unfortunately is not true. Originally @leafpetersen and @vsmenon tried to make fields non-virtual by default (see @virtual), but that was decided, for better or worse, to be too disruptive of a change for Dart 1.x -> 2.x. This is totally legal in Dart 2.x:

class X {
  final x = 'Hello';
}

class Y extends X {
  bool _returnNull = true;
  get x => {
    _returnNull = !returnNull;
    return _returnNull ? null : 'Hello';
  }
}

void main() {
  X x = Y();
  if (x.x is String) {
    print(x.x is String); // false
  }
}

Even then, it isn't clear how this would work for implements:

class X {
  final x = 'Hello';
}

class Y extends X {
  bool _returnNull = false;
  get x => {
    _returnNull = !returnNull;
    return _returnNull ? null : 'Hello';
  }
}

void main() {
  X x = Y();
  if (x.x is String) {
    print(x.x is String); // false
  }
}

[matanlurey]

I know @eernstg had a very informal proposal to help here though, something like:

void main() {
  X x = Y();
  // If x.x is a String, assign it to a new variable "localX"
  if (x.x is String : localX) {
    print(localX is String); // true
  }
}

[leafpetersen]

Yes, the example @matanlurey gives in #34480 (comment) is the germane one (in fact, instead of null you can make it return something of a completely different type by specifying the type of the field as Object on the second call).

The two options that we might consider are to automatically insert runtime checks on each use, or to provide better syntax for binding a new promoted variable in the scope of the promotion.

[Hixie]

I agree that in the example above, with a public member, it can't be proved, but in the original example the field is private, which means the analyzer has all the information it needs to determine whether it can be promoted.

[matanlurey]

@Hixie:

But in the original example the field is private

Yes, this seems like somewhere the specs/tools could enhance today. I do admit though that would be very confusing to users - for example making a private variable public would be a breaking change in this case.

[leafpetersen]

for example making a private variable public would be a breaking change in this case.

Worse, adding a mock of an interface would be a breaking change. That is, as @Hixie notes, the analyzer could notice that a private field is never implemented by a getter and allow promotion. But now you add a mock that implements the field as a getter, and some completely unrelated code breaks because promotion becomes disabled.

[matanlurey]

@leafpetersen Ok, we could do private class and private member :P