Analyzer and CFE disagree on promotion of this.

[leafpetersen]

With the non-nullable experiment on, I see errors from the analyzer on the return this statements in the extension and class below. The CFE gives no errors.

extension Test on num {
  int get foo {
    num self = this;
    if (this is int) return this;
    if (self is int) return self;
    return 3;
  }
}

class A {
  B get foo {
    A self = this;
    if (this is B) return this;
    if (self is B) return self;
    return B();
  }
}

class B extends A {}


void main() {}

I believe this may be a long-standing issue, should we try to resolve this in the NNBD release?

cc @scheglov @johnniwinther @lrhn @munificent @eernstg @stereotype441

[lrhn]

I do want promotion of this to work like it does for (final) local variables. There is no good reason not to allow it.
For extensions, the this really is just a variable in the desugared code.

So, yes please, let's fix this now.

[scheglov]

Oh, I missed the part about classes.
It totally makes sense for extensions, where this is already synthetic.

Do we do promotion of this in classes as well?

[eernstg]

For code in the body of a method which is inherited, a promotion could be both safe and useful. So, yes, I think classes should support promotion of this as well.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/139063

[leafpetersen]

@johnniwinther seems to think that this is actually not supported by the front end, and that presumably the reason I'm not seeing errors for the example above is because errors are suppressed in NNBD mode, and implicit casts are allowed in non-NNBD mode. The modified example below which makes the implicit downcast a sidecast seems to validate this.

Given that this is not implemented in the CFE either, we should re-evaluate whether to spend time on this now.

extension Test on A {
  C get foo {
    A self = this;
    if (this is C) return this;
    if (self is C) return self;
    return C();
  }
}

class A {
  C get foo {
    A self = this;
    if (this is C) return this;
    if (self is C) return self;
    return C();
  }
}

class B extends A implements C {}

class C {}

void main() {}

[lrhn]

Would there be any risk of lock-in if we don't do this promotion now, but want to do it later?

Since our promotion is only to subtypes, invocations should stay sound, but extension methods might get shadowed if we start promotiong. The biggest risk is that someone does

class Foo { 
  if (this is Bar) {
    var list = [this];
  }
}

and starts getting a List<Bar> instead of a List<Foo>.

So, it's not completely non-breaking to promote this later. If at all possible, I'd be happiest if we do it now (just treat this like a final variable with the current class as declared type, and do that everywhere).

[johnniwinther]

Just note that the "just treat this like a final variable ... " entails a lot of work in the CFE and maybe even the backends. That an expression is this holds valuable information that is not conveyed in an alias.

[leafpetersen]

This is consistent in the two front ends, and I don't think this is something we should try to push into the null safety release. I filed a language issue to track the desire to do better here, closing this out.