There's no good way to promote a field

[Hixie]

(This bug assumes a world with implicit-casts: false.)

If I have a class with a field of unknown type, there's currently no clean way to write the code that handles that field in a type-safe way, as far as I can tell.

class Bar<T, R, S> {
  Bar(this.value);
  final Object value;

  void foo() {
    if (value.runtimeType == runtimeType) {
      // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
      final Bar<T, R, S> typedValue = value as Bar<T, R, S>; // this line is really ugly.
      // not we can use typedValue, but that's ugly.
      typedValue.la(1);
      typedValue.la(2);
      typedValue.la(3);
    }
    // ...
  }
  void la(int x) { }
}

class Bar<T, R, S> {
  Bar(this.value);
  final dynamic value;

  void foo() {
    if (value.runtimeType == runtimeType) {
      // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
      // so we get no type checking here
      value.la(1);
      value.la(2);
      value.laa(3); // oops typo, nobody will notice until runtime
    }
    // ...
  }
  void la(int x) { }
}

class Bar<T, R, S> {
  Bar(this.value);
  final Object value;

  void foo() {
    if (value.runtimeType == runtimeType) {
      // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
      assert(value is Bar<T, R, S>); // this doesn't do anything for the compiler or the analyzer
      value.la(1); // the compiler doesn't allow this
      value.la(2);
      value.la(3);
    }
    // ...
  }
  void la(int x) { }
}

class Bar<T, R, S> {
  Bar(this.value);
  final Object value;

  void foo() {
    if (value.runtimeType == runtimeType) {
      // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
      if (value is Bar<T, R, S>) { // this doesn't do anything for the compiler or the analyzer
        value.la(1); // the compiler doesn't allow this
        value.la(2);
        value.la(3);
      }
    }
    // ...
  }
  void la(int x) { }
}

What is the preferred way to write this code?

I've run into this in various places but most recently when writing a class to represent Json data.

[vsmenon]

Note, runtimeType isn't robust for this kind of use ... because the following is legal Dart:

var _types = [int, String, bool];
var _i = 0;

class Foo {
  Type get runtimeType {
    _i = (_i + 1) % _types.length;
    return _types[_i];
  }
}

void main() {
  var f = Foo();
  print(f.runtimeType);
  print(f.runtimeType);
}

It would probably make sense to seal runtimeType, but we don't have sealing yet. :-)

[vsmenon]

This would probably be the most concise way to write this today:

class Bar<T, R, S> {
  Bar(this.value);
  final Object value;

  void foo() {
    var v = value;
    if (v is Bar<T, R, S>) {
      v.la(1);
      v.la(2);
      v.la(3);
    }
  }
  void la(int x) { }
}

Promoting the field directly is tricky as v.la (including overrides) could change value to a different type.

[lrhn]

Dart currently cannot promote any field because there is no analysis ensuring that it's not mutated between test and use.
Promoting a final field should be possible. At least the analysis is trivial, but the notion of a test "showing that something has type something" needs to be extended to expressions larger than a single variable. We are unlikely to do anything about this until we get a chance to do a proper overhaul of the promotion system (which is something that will have to happen in conjunction with non-nullable types).

(Personally, I like the idea of unsafe promotion: You check that some expression has type T, then you later use it at type T. That probably means that you assume that it doesn't change, so we allow you to do it, we just add an extra check at the use point. It's effectively an implicit down-cast, so you can disallow it if you don't want those, but implementations can be more efficient by first checking that the value hasn't changed).

Rather than "sealing" the runtimeType getter, we should just make it a static function, probably in dart:mirrors where it belongs.

[leafpetersen]

Promoting a final field should be possible. At least the analysis is trivial

No, it isn't. Not without sealing (or mandatory whole program analysis). Because I can implement that final field in a subclass with a getter that returns a different value every time based on the time of the day.

[leafpetersen]

@Hixie Leaving aside the runtimeType bit for the moment, how would you feel about something like:

      if (Bar<T, R, S> v = value) { // like an implicit cast on value, but failed cast is just treated as false 
        v.la(1);
        v.la(2);
        v.la(3);
      }

[Hixie]

The general pattern is:

  if (some out of band condition that Dart can't understand) {
    // we now know that a particular final or private property is a particular subtype
    // of its static type
    ...
  }

Using if doesn't work because it implies that the condition might not be true, so the code is confusing:

  if (some out of band condition that Dart can't understand) {
    // we now know that a particular final or private property is a particular subtype
    // of its static type
    if (magic that we know will always be true but happens to trick the compiler/analyzer) {
      ...
    }
    // else what?
  }

[nex3]

This is particularly painful and confusing with NNBD. I haven't done a lot of migration work yet, but so far this accounts for about half of the total new analysis errors I'm seeing, and almost all of the ! operations I've added. I don't know if the promotion system overhaul that @lrhn mentioned ended up happening, but his proposed "unsafe promotion" semantics would be pretty helpful here.

It may be worth noting that in all the cases I'm dealing with, the field in question is private, so local analysis (for top-level fields) or library-level analysis (for class fields) would show that there's no way the value changed between references.

[leafpetersen]

It may be worth noting that in all the cases I'm dealing with, the field in question is private

@nex3 are these all final as well? I ask because I'm interested in pushing on the local analysis approach and have some initial thoughts about how to approach this, but I think doing escape analysis to see if there's a possibility that a field is assigned is probably a non-starter.

Would you give up the ability to implement and extend the interfaces in question, at least non-locally?

Would you give up the ability to access these fields through a dynamic invocation?

What about testing? Would you be ok not being able to mock these fields via a getter or NSM? Or would you need an unsound escape hatch for tests?

[nex3]

• The fields are generally not final. The use-case is often around caching data (although sometimes late works for that).
• I'm usually happy making my classes sealed.
• I try to avoid ever doing dynamic invocations, although no-implicit-dynamic mode is still not quite usable enough that I feel confident in knowing exactly which invocations are or aren't dynamic.
• I never use mocks.

[tomyeh]

Agree with @nex3 It is really painful to upgrade this kind statements for NNBD (with a local variable or other approaches). It can be error-prone if the code is complicated.

if (_member != null) {
  _member.foo();
...

Worse of all, almost of zero of code will return null in the 2nd invocation (if setter is never called). It is kind of treating every citizen as criminal while the crime rate is almost negligible.

Is it possible to introduce a keyword to mark a getter that can return a different value in different invocations? Otherwise, consider it is immutable before setter is called. Then, it will be much easier to upgrade to NNBD for us -- we got hundreds thousands of lines of code! For example,

volatile int? foo;

[lrhn]

A getter can always return a different value in different invocations.
Some getters don't, but it's a non-breaking change (from the type system's view) to change one getter to another, so the compiler cannot assume anything about the behavior of any getter. Depending on the implementation of the getter makes it a breaking change to change the implementation, which is what the getter abstraction exist to avoid.

If we need anything, it's a marker to say that a getter cannot return a different value in a second invocation. It would have to be something you opt in to as an author, because will be a breaking change to remove that marker in a later change.
See dart-lang/language#1518 for one such proposal.

It will still not help with getters which might change at some later time, but you just happen to know won't change between this test and the following use. I generally find that nullable fields are fairly likely to be mutable, and mutable fields can't be stable.