promote locals without an upcast first

[matanlurey]

class Model<T> {}

class Interface<T> {
  void hasMethod() {}
}

class Service {
  Model model;
  
  void tryThis() {
    final model = this.model;
    if (model is Interface) {
      // ERROR: The method 'hasMethod' isn't defined for the class 'Model'.
      model.hasMethod();
    }
  }
}

Naming final model to final modelX works as expected.

[matanlurey]

Actually, it seems to fail more broadly. Consider:

class Selectable<T> {
  void doThing() {}
}

class SelectionModel<T> {
  
}

class Controller<T> {
  SelectionModel<T> model;
  
  void doThing() {
    final model$ = model;
    if (model$ is Selectable) {
      model$.doThing();
    }
  }
}

[matanlurey]

This works though:

class Selectable<T> {
  void doThing() {}
}

class SelectionModel<T> {
  
}

class Controller<T> {
  SelectionModel<T> model;
  
  void doThing() {
    final Object model$ = model;
    if (model$ is Selectable) {
      model$.doThing();
    }
  }
}

[lexaknyazev]

In the very similar #26576, the answer was that analyzer works as intended.

[matanlurey]

I'm hoping that was related to Dart1, and we can fix that now (the spec has changed). If the compilers think model is Selectable is valid code then the analyzer should think it is valid too (and allow type promotion).

Related issue is #33933, FWIW.

/cc @lrhn @eernstg

[jmesserly]

this was considered this a bunch of times for strong mode/Dart 2, but we couldn't do it without a notion of interface types. There's probably another issue filed about this, though I can't find it at the moment. In the meantime, moving to area-language. It would be neat to see an improvement here in a future version of Dart. (or some alternate way of expressing this, like pattern matching)

edit: to add more explanation, Dart 1/2 can only promote when the promoted type is a subtype of the original type. This is because otherwise, you'd lose access to some APIs. In the original example, if model becomes type Interface<dynamic>, it will lose its type Selection. The only way to track both types is to represent it as Interface<dynamic> & Selection

[matanlurey]

Thanks! That makes sense.

That being said if someone writes:

Model x;

if (x is SomethingElse) {
  // OK to "lose" capabilities of Model.
}

... seems fine with me.

[lrhn]

Losing information is most likely going to be very annoying in practice. It might seem fine to you currently, because the examples you are looking at has that property, but in general it's not practical to forget that x is Model because users will expect it to be a Model (it says so, right there).

Unless we get (effectively) intersection types, I don't see a way to promote a variable of one type to an unrelated type.
It's possible that we can get intersection type behavior for promotion without actual intersection types in the type system.

[matanlurey]

Losing information is most likely going to be very annoying in practice

Could we do some actual investigation of this before claiming it? For example, we have a huge swath of both Flutter (/cc @Hixie) and internal code (/cc @davidmorgan) to look at, and we could see if indeed you are correct, or if the user always expects the new type, and is OK with losing capabilities.

In the short term though, can we do something about the usability issue?

Many users hit this and are very confused. A couple ideas:

1. Easier: Issue a hint when is won't perform a type promotion: (/cc @bwilkerson)

class A {}
class B {}

void example(A a) {
  // HINT: Variable "a" will not be promoted to type "B". Perform an upcast first.
  if (a is B) {

  }
}

... and if analyzer could offer an auto-fix, it would write, for you:

class A {}
class B {}

void example(A a) {
  final Object aUpcast = a;
  if (aUpcast is B) {
    // aUpcast is now a type of B
  }
}

If we deem this too disruptive, then maybe we could detect the following pattern:

class A {}
class B {}

void example(A a) {
  // HINT: Variable "a" will not be promoted to type "B". Perform an upcast first.
  if (a is B) {
    // This.
    (a as B);
    // Or this.
    B b = a;
  }
}

2. Harder: Have a variant of is that allows losing information:

class A {}
class B {}

void example(A a) {
  // I am not a language designer. But something that allows `is` to an unrelated type.
  if (a is~ B) {

  }
}

[jmesserly]

For example, we have a huge swath of both Flutter and internal code o look at, and we could see if indeed you are correct, or if the user always expects the new type, and is OK with losing capabilities.

it would be pretty easy to do something like: change Analyzer behavior to always promote, and then analyze all the code we have.

Even if that run came back pretty good, it would still be a breaking change to the language. (Also, mutating the variable's type to an unrelated type would be pretty weird from a language design perspective.)

Adding a hint would be fairly straightforward. Promotion is also disabled in other cases, such as if you mutate the variable in the block, or if you closed over it anywhere. So the hint could address that as well. But if you turn off implicit casts, it makes it more obvious when promotion failed. So perhaps pushing on implicit casts is the right way to go.

[eernstg]

The language team has considered a different mechanism that might be helpful here (and it might be helpful in other ways as well). Consider the original example (slightly modified):

class Model<T> {}

class Interface<T> {
  void hasMethod() {}
}

class Service {
  Model model;
  
  void tryThis() {
    final model = this.model;
    if (model is Interface interface) {
      interface.hasMethod();
    }
  }
}

There may be lots of things to sort out for this mechanism, but the basic idea is that e is T x declares a fresh, final variable x of type T which is in scope in the 'then' part of the enclosing if and whose value is obtained by an evaluation of e. We may or may not support such a construct elsewhere, but the main motivating case is when it's used as the guard of an if statement. This makes it possible to use an arbitrary expression e and snapshot its value in a fresh variable x (which is helpful in some cases in itself), and it provides access to that object under the tested type T (which is helpful in the given example). This eliminates the need for intersection types during promotion (more general than the special case that we already have for type variables).

[matanlurey]

Love that suggestion @eernstg! Huge 👍 from me.

[Aqluse]

I have encountered this moment and it seems tremendously annoying. Does this case really needs to be followed by the spec rule of promotion? I mean are there none of solutions with minimum losses due to extra types promoted for variable scope?