Analyzer does not properly handle flow analysis of null-aware extension method invocations

[stereotype441]

The following code is accepted by the CFE but rejected by the analyzer:

extension E on Null {
  int f(dynamic d) => 0;
}

int test(int? i) {
  i = 0; // promotes `i` to `int`
  E(null)?.f(i = null); // doesn't demote because `i = null` is unreachable
  return i; // ok because `i` is promoted to `int`
}

main() {}

The analyzer's error message is:

error • test.dart:8:10 • A value of type 'int?' can't be returned from the function 'test' because it has a return type of 'int'. • return_of_invalid_type

I believe what's happening is that the CFE is (correctly) passing the type of the target of the extension override (Null in this case) to FlowAnalysis.nullAwareAccess_rightBegin. Therefore, flow analysis concludes that i = null is unreachable, so i is not demoted. Whereas the analyzer is passing the type dynamic to FlowAnalysis.nullAwareAccess_rightBegin, so flow analysis doesn't know that i = null is unreachable, and so it demotes i to int?, and the return statement is in error.

I'll work on a fix.

[stereotype441]

I spoke to @bwilkerson and @scheglov about this. Here's some more info about what's going on:

When the analyzer tells flow analysis about a null-aware invocation, one of the pieces of information it has to provide is the static type of the target. The analyzer does this by querying .staticType on the expression to the left of the ?.. However, since the analyzer doesn't consider an extension override like E(null) to be a true expression, ExtensionOverride.staticType returns null. So the analyzer has some extra logic to substitute the type typeProvider.dynamicType in place of null. This prevents a crash, but it also prevents flow analysis from understanding that the target of the null-aware invocation is always null.

We think the easiest way to fix this is probably to change the analyzer so that ExtensionOverride.staticType no returns the same type as the type of the wrapped expression, rather than null.

I'll try that approach and see how it goes.

[stereotype441]

Upon further investigation, I found a manifestation of this issue that isn't so contrived:

class C {
  int f() => 0;
}

extension E on C {
  // Note: return type of `int` to work aorund https://github.com/dart-lang/sdk/issues/59690
  int g(dynamic d) => 0;
}

void test(C? c) {
  E(c)?.g(c.f()); // `c` is promoted to `C` on the RHS of `?.`, so `c.f()` should be ok
}

main() {}

[eernstg]

I think E(c)?.g... is a compile-time error because E(c) is an 'extension application' where the static type of c (that is, C?) isn't assignable to the instantiated on-type of E (which is C). This might simplify the treatment of cases which are not errors.

[stereotype441]

@eernstg the spec proposal for extensions specifically calls out null-aware extension applications as something that's expected to work (https://github.com/dart-lang/language/blob/main/accepted/2.7/static-extension-methods/feature-specification.md#null-aware-member-invocations). Perhaps this aspect of the feature got missed when merging the feature into the formal spec.

[eernstg]

The situation might be a bit underspecified, but we do have some pieces.

As you indicate, the document language/accepted/2.7/static-extension-methods
/feature-specification.md is a background document at this time, so we should rely on the language specification where this feature is specified today. Of course, we might still detect a mistake and fix it in the language specification.

However, the rule which makes E(c) a compile time error occurs in the feature specification as well, here:

It is a compile-time error if the corresponding class constructor invocation would be a compile-time error.

This doesn't prevent every null-aware explicit extension member invocation from taking place, because we could have an instantiated on-type which is nullable:

class C {}

extension E2 on C? {
  String foo() => 'Foo!';
}

void test(C? c) {
  print(E2(c)?.foo());
}

void main() => test(null);

However, the implicit form is more flexible, based on the language specification, as described here: https://github.com/dart-lang/language/blob/9f594cebb9fd089f71556d0e68bbb11aef8bf948/specification/dartLangSpec.tex#L13908. So c?.g(c.f()) should be OK in the original example.

It could be considered to be an inconsistency that E(c)?.g... is a compile-time error in the original example, but E2(c)?.foo() is not an error, but it does make sense to me that we simply insist on E(c) being subject to a type check based on the instantiated on-type. It is somewhat "magical" if the ? on the outside of E(c) can eliminate an error inside E(c).

[stereotype441]

@eernstg Ok, I've created dart-lang/language#4201 to track the issue you've raised, so that we can keep it separate from this issue (which is about an inaccuracy in the analyzer's use of flow analysis)