Flow analysis: begin tracking non-promotion reasons.

This CL implements the core flow analysis infrastructure for tracking
reasons why an expression was not promoted.  It supports the following
reasons:

- Expression was a property access
- Expression has been written to since it was promoted

I expect to add support for other non-promotion reasons in the future,
for example:

- `this` cannot be promoted
- Expression has been write captured
- Expression was a reference to a static field or top level variable

These non-promotion reasons are plumbed through to the CFE and
analyzer for the purpose of making errors easier for the user to
understand.  For example, given the following code:

  class C {
    int? i;
    f() {
      if (i == null) return;
      print(i.isEven);
    }
  }

The front end now prints:

  ../../tmp/test.dart:5:13: Error: Property 'isEven' cannot be accessed on 'int?' because it is potentially null.
  Try accessing using ?. instead.
      print(i.isEven);
              ^^^^^^
  Context: 'i' refers to a property so it could not be promoted.

Much work still needs to be done to round out this feature, for example:

- Currently the analyzer only shows the new "why not promoted"
  messages when the "--verbose" flag is specified; this means the
  feature is unlikely to be noticed by users.

- Currently the analyzer doesn't show a "why not promoted" message
  when the non-promotion reason is that the expression is a property
  access.

- We need one or more web pages explaining non-promotion reasons in
  more detail so that the error messages can contain pointers to them.

- The analyzer and front end currently only show non-promotion reasons
  for expressions of the form `x.y` where `x` fails to be promoted to
  non-nullable.  There are many other scenarios that should be
  handled.

Change-Id: I0a12df74d0fc6274dfb3cb555abea81a75884231
Bug: #38773
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/181741
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: stereotype441

.dart_tool/package_config.json

@@ -55,6 +55,11 @@
       "rootUri": "../pkg/_fe_analyzer_shared/test/flow_analysis/type_promotion",
       "packageUri": ".nonexisting/"
     },
+    {
+      "name": "_fe_analyzer_shared_why_not_promoted",
+      "rootUri": "../pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted",
+      "packageUri": ".nonexisting/"
+    },
     {
       "name": "_js_interop_checks",
       "rootUri": "../pkg/_js_interop_checks",

pkg/_fe_analyzer_shared/analysis_options_no_lints.yaml

@@ -15,4 +15,5 @@ analyzer:
     - test/flow_analysis/nullability/data/**
     - test/flow_analysis/reachability/data/**
     - test/flow_analysis/type_promotion/data/**
+    - test/flow_analysis/why_not_promoted/data/**
     - test/inheritance/data/**

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

@@ -4032,6 +4032,29 @@ const MessageCode messageFieldInitializerOutsideConstructor = const MessageCode(
     message: r"""Field formal parameters can only be used in a constructor.""",
     tip: r"""Try removing 'this.'.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<Message Function(String name)> templateFieldNotPromoted =
+    const Template<Message Function(String name)>(
+        messageTemplate:
+            r"""'#name' refers to a property so it could not be promoted.""",
+        withArguments: _withArgumentsFieldNotPromoted);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name)> codeFieldNotPromoted =
+    const Code<Message Function(String name)>(
+  "FieldNotPromoted",
+);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsFieldNotPromoted(String name) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  return new Message(codeFieldNotPromoted,
+      message:
+          """'${name}' refers to a property so it could not be promoted.""",
+      arguments: {'name': name});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeFinalAndCovariant = messageFinalAndCovariant;
 
@@ -9620,6 +9643,34 @@ const MessageCode messageVarReturnType = const MessageCode("VarReturnType",
     tip:
         r"""Try removing the keyword 'var', or replacing it with the name of the return type.""");
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<
+    Message Function(
+        String
+            name)> templateVariableCouldBeNullDueToWrite = const Template<
+        Message Function(String name)>(
+    messageTemplate:
+        r"""Variable '#name' could be null due to a write occurring here.""",
+    tipTemplate: r"""Try null checking the variable after the write.""",
+    withArguments: _withArgumentsVariableCouldBeNullDueToWrite);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name)> codeVariableCouldBeNullDueToWrite =
+    const Code<Message Function(String name)>(
+  "VariableCouldBeNullDueToWrite",
+);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsVariableCouldBeNullDueToWrite(String name) {
+  if (name.isEmpty) throw 'No name provided';
+  name = demangleMixinApplicationName(name);
+  return new Message(codeVariableCouldBeNullDueToWrite,
+      message:
+          """Variable '${name}' could be null due to a write occurring here.""",
+      tip: """Try null checking the variable after the write.""",
+      arguments: {'name': name});
+}
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Code<Null> codeVerificationErrorOriginContext =
     messageVerificationErrorOriginContext;

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -16,6 +16,7 @@ main() {
   testDir('pkg/_fe_analyzer_shared/test/flow_analysis/nullability/data');
   testDir('pkg/_fe_analyzer_shared/test/flow_analysis/reachability/data');
   testDir('pkg/_fe_analyzer_shared/test/flow_analysis/type_promotion/data');
+  testDir('pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data');
   testDir('pkg/_fe_analyzer_shared/test/inheritance/data');
 }
 

pkg/_fe_analyzer_shared/test/annotated_code_helper_test.dart

@@ -154,6 +154,11 @@ Statement switch_(Expression expression, List<SwitchCase> cases,
         {required bool isExhaustive}) =>
     new _Switch(expression, cases, isExhaustive);
 
+Expression this_(String type) => new _This(Type(type));
+
+Expression thisOrSuperPropertyGet(String name, {String type = 'Object?'}) =>
+    new _ThisOrSuperPropertyGet(name, type);
+
 Expression throw_(Expression operand) => new _Throw(operand);
 
 Statement tryCatch(List<Statement> body, List<CatchClause> catches) =>
@@ -273,12 +278,24 @@ abstract class Expression extends Node implements _Visitable<Type> {
   /// If `this` is an expression `x`, creates the expression `x || other`.
   Expression or(Expression other) => new _Logical(this, other, isAnd: false);
 
+  /// If `this` is an expression `x`, creates the expression `x.name`.
+  Expression propertyGet(String name, {String type = 'Object?'}) =>
+      new _PropertyGet(this, name, type);
+
   /// If `this` is an expression `x`, creates a pseudo-expression that models
   /// evaluation of `x` followed by execution of [stmt].  This can be used to
   /// test that flow analysis is in the correct state after an expression is
   /// visited.
   Expression thenStmt(Statement stmt) =>
       new _WrappedExpression(null, this, stmt);
+
+  /// Creates an [Expression] that, when analyzed, will behave the same as
+  /// `this`, but after visiting it, will cause [callback] to be passed the
+  /// non-promotion info associated with it.  If the expression has no
+  /// non-promotion info, an empty map will be passed to [callback].
+  Expression whyNotPromoted(
+          void Function(Map<Type, NonPromotionReason>) callback) =>
+      new _WhyNotPromoted(this, callback);
 }
 
 /// Test harness for creating flow analysis tests.  This class implements all
@@ -311,8 +328,10 @@ class Harness extends TypeOperations<Var, Type> {
     'int? <: num?': true,
     'int? <: Object': false,
     'int? <: Object?': true,
+    'Never <: Object?': true,
     'Null <: int': false,
     'Null <: Object': false,
+    'Null <: Object?': true,
     'num <: int': false,
     'num <: Iterable': false,
     'num <: List': false,
@@ -347,13 +366,15 @@ class Harness extends TypeOperations<Var, Type> {
     'Object <: int': false,
     'Object <: int?': false,
     'Object <: List': false,
+    'Object <: Null': false,
     'Object <: num': false,
     'Object <: num?': false,
     'Object <: Object?': true,
     'Object <: String': false,
     'Object? <: Object': false,
     'Object? <: int': false,
     'Object? <: int?': false,
+    'Object? <: Null': false,
     'String <: int': false,
     'String <: int?': false,
     'String <: num?': false,
@@ -364,6 +385,8 @@ class Harness extends TypeOperations<Var, Type> {
   static final Map<String, Type> _coreFactors = {
     'Object? - int': Type('Object?'),
     'Object? - int?': Type('Object'),
+    'Object? - Never': Type('Object?'),
+    'Object? - Null': Type('Object'),
     'Object? - num?': Type('Object'),
     'Object? - Object?': Type('Never?'),
     'Object? - String': Type('Object?'),
@@ -410,6 +433,9 @@ class Harness extends TypeOperations<Var, Type> {
 
   Harness({this.allowLocalBooleanVarsToPromote = false, this.legacy = false});
 
+  @override
+  Type get topType => Type('Object?');
+
   /// Updates the harness so that when a [factor] query is invoked on types
   /// [from] and [what], [result] will be returned.
   void addFactor(String from, String what, String result) {
@@ -597,15 +623,30 @@ class SwitchCase implements _Visitable<void> {
 /// testing.  This is essentially a thin wrapper around a string representation
 /// of the type.
 class Type {
+  static bool _allowComparisons = false;
+
   final String type;
 
   Type(this.type);
 
+  @override
+  int get hashCode {
+    if (!_allowComparisons) {
+      // The flow analysis engine should not hash types using hashCode.  It
+      // should compare them using TypeOperations.
+      fail('Unexpected use of operator== on types');
+    }
+    return type.hashCode;
+  }
+
   @override
   bool operator ==(Object other) {
-    // The flow analysis engine should not compare types using operator==.  It
-    // should compare them using TypeOperations.
-    fail('Unexpected use of operator== on types');
+    if (!_allowComparisons) {
+      // The flow analysis engine should not compare types using operator==.  It
+      // should compare them using TypeOperations.
+      fail('Unexpected use of operator== on types');
+    }
+    return other is Type && this.type == other.type;
   }
 
   @override
@@ -1424,6 +1465,29 @@ class _PlaceholderExpression extends Expression {
       type;
 }
 
+class _PropertyGet extends Expression {
+  final Expression target;
+
+  final String propertyName;
+
+  final String type;
+
+  _PropertyGet(this.target, this.propertyName, this.type);
+
+  @override
+  void _preVisit(AssignedVariables<Node, Var> assignedVariables) {
+    target._preVisit(assignedVariables);
+  }
+
+  @override
+  Type _visit(
+      Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
+    target._visit(h, flow);
+    flow.propertyGet(this, target, propertyName);
+    return Type(type);
+  }
+}
+
 class _Return extends Statement {
   _Return() : super._();
 
@@ -1481,6 +1545,43 @@ class _Switch extends Statement {
   }
 }
 
+class _This extends Expression {
+  final Type type;
+
+  _This(this.type);
+
+  @override
+  String toString() => 'this';
+
+  @override
+  void _preVisit(AssignedVariables<Node, Var> assignedVariables) {}
+
+  @override
+  Type _visit(
+      Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
+    flow.thisOrSuper(this);
+    return type;
+  }
+}
+
+class _ThisOrSuperPropertyGet extends Expression {
+  final String propertyName;
+
+  final String type;
+
+  _ThisOrSuperPropertyGet(this.propertyName, this.type);
+
+  @override
+  void _preVisit(AssignedVariables<Node, Var> assignedVariables) {}
+
+  @override
+  Type _visit(
+      Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
+    flow.thisOrSuperPropertyGet(this, propertyName);
+    return Type(type);
+  }
+}
+
 class _Throw extends Expression {
   final Expression operand;
 
@@ -1622,6 +1723,37 @@ class _While extends Statement {
   }
 }
 
+class _WhyNotPromoted extends Expression {
+  final Expression target;
+
+  final void Function(Map<Type, NonPromotionReason>) callback;
+
+  _WhyNotPromoted(this.target, this.callback);
+
+  @override
+  String toString() => '$target (whyNotPromoted)';
+
+  @override
+  void _preVisit(AssignedVariables<Node, Var> assignedVariables) {
+    target._preVisit(assignedVariables);
+  }
+
+  @override
+  Type _visit(
+      Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
+    var type = target._visit(h, flow);
+    flow.forwardExpression(this, target);
+    assert(!Type._allowComparisons);
+    Type._allowComparisons = true;
+    try {
+      callback(flow.whyNotPromoted(this));
+    } finally {
+      Type._allowComparisons = false;
+    }
+    return type;
+  }
+}
+
 class _WrappedExpression extends Expression {
   final Statement? before;
   final Expression expr;
@@ -1681,7 +1813,7 @@ class _Write extends Expression {
       Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
     var rhs = this.rhs;
     var type = rhs == null ? variable.type : rhs._visit(h, flow);
-    flow.write(variable, type, rhs);
+    flow.write(this, variable, type, rhs);
     return type;
   }
 }