Add "why not promoted" support for method invocations to CFE.

This required moving the logic for computing the context messages from
the inference visitor to the type inferrer.

This includes support for extension method invocations and invocation
of `.call` on a function type.

Bug: #44898
Change-Id: Icda160f69cc815953c43edc92be910c5f49f9401
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184842
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: stereotype441

pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data/nullable_error_scenarios.dart

@@ -0,0 +1,82 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This test contains a test case for each kind of error that can arise from an
+// expression being nullable, for which we wish to report "why not promoted"
+// errors.
+
+class C {
+  int? i;
+  void Function()? f;
+}
+
+extension on int {
+  get propertyOnNonNullInt => null;
+  void methodOnNonNullInt() {}
+}
+
+extension on int? {
+  get propertyOnNullableInt => null;
+  void methodOnNullableInt() {}
+}
+
+property_get_of_variable(int? i, int? j) {
+  if (i == null) return;
+  /*cfe.update: explicitWrite*/ /*analyzer.explicitWrite*/ i = j;
+  i. /*notPromoted(explicitWrite)*/ isEven;
+}
+
+extension_property_get_of_variable(int? i, int? j) {
+  if (i == null) return;
+  /*cfe.update: explicitWrite*/ /*analyzer.explicitWrite*/ i = j;
+  i.propertyOnNullableInt;
+  i.
+      /*cfe.invoke: notPromoted(explicitWrite)*/
+      /*analyzer.notPromoted(explicitWrite)*/
+      propertyOnNonNullInt;
+}
+
+property_get_of_expression(C c) {
+  if (c.i == null) return;
+  c.i. /*notPromoted(propertyNotPromoted(member:C.i))*/ isEven;
+}
+
+extension_property_get_of_expression(C c) {
+  if (c.i == null) return;
+  c.i.propertyOnNullableInt;
+  c
+      .i
+      .
+      /*cfe.invoke: notPromoted(propertyNotPromoted(member:C.i))*/
+      /*analyzer.notPromoted(propertyNotPromoted(member:C.i))*/
+      propertyOnNonNullInt;
+}
+
+method_invocation(C c) {
+  if (c.i == null) return;
+  c.i
+      .
+      /*cfe.invoke: notPromoted(propertyNotPromoted(member:C.i))*/
+      /*analyzer.notPromoted(propertyNotPromoted(member:C.i))*/
+      abs();
+}
+
+extension_method_invocation(C c) {
+  if (c.i == null) return;
+  c.i.methodOnNullableInt();
+  c.i
+      .
+      /*cfe.invoke: notPromoted(propertyNotPromoted(member:C.i))*/
+      /*analyzer.notPromoted(propertyNotPromoted(member:C.i))*/
+      methodOnNonNullInt();
+}
+
+call_invocation(C c) {
+  if (c.f == null) return;
+  c.f
+      .
+      /*cfe.invoke: notPromoted(propertyNotPromoted(member:C.f))*/
+      /*analyzer.notPromoted(propertyNotPromoted(member:C.f))*/
+      call();
+}

pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart

@@ -5,13 +5,9 @@
 // @dart = 2.9
 
 import 'dart:core' hide MapEntry;
-import 'dart:core' as core;
 
-import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
-import 'package:_fe_analyzer_shared/src/testing/id.dart';
 import 'package:_fe_analyzer_shared/src/util/link.dart';
 import 'package:front_end/src/api_prototype/lowering_predicates.dart';
-import 'package:front_end/src/testing/id_extractor.dart';
 import 'package:kernel/ast.dart'
     hide Reference; // Work around https://github.com/dart-lang/sdk/issues/44667
 import 'package:kernel/src/legacy_erasure.dart';
@@ -4739,36 +4735,13 @@ class InferenceVisitor
 
     readResult ??= new ExpressionInferenceResult(readType, read);
     if (!inferrer.isTopLevel && readTarget.isNullable) {
-      Map<DartType, NonPromotionReason> whyNotPromoted =
-          inferrer.flowAnalysis?.whyNotPromoted(receiver);
-      List<LocatedMessage> context;
-      if (whyNotPromoted != null && whyNotPromoted.isNotEmpty) {
-        _WhyNotPromotedVisitor whyNotPromotedVisitor =
-            new _WhyNotPromotedVisitor(inferrer, receiver);
-        for (core.MapEntry<DartType, NonPromotionReason> entry
-            in whyNotPromoted.entries) {
-          if (entry.key.isPotentiallyNullable) continue;
-          LocatedMessage message = entry.value.accept(whyNotPromotedVisitor);
-          if (inferrer.dataForTesting != null) {
-            String nonPromotionReasonText = entry.value.shortName;
-            if (whyNotPromotedVisitor.propertyReference != null) {
-              Id id = computeMemberId(whyNotPromotedVisitor.propertyReference);
-              nonPromotionReasonText += '($id)';
-            }
-            inferrer.dataForTesting.flowAnalysisResult
-                .nonPromotionReasons[read] = nonPromotionReasonText;
-          }
-          context = [message];
-          break;
-        }
-      }
       readResult = inferrer.wrapExpressionInferenceResultInProblem(
           readResult,
           templateNullablePropertyAccessError.withArguments(
               propertyName.text, receiverType, inferrer.isNonNullableByDefault),
           read.fileOffset,
           propertyName.text.length,
-          context: context);
+          context: inferrer.getWhyNotPromotedContext(receiver, read));
     }
     return readResult;
   }
@@ -6988,66 +6961,6 @@ class InferenceVisitor
   }
 }
 
-class _WhyNotPromotedVisitor
-    implements
-        NonPromotionReasonVisitor<LocatedMessage, Node, Expression,
-            VariableDeclaration> {
-  final TypeInferrerImpl inferrer;
-
-  final Expression receiver;
-
-  Member propertyReference;
-
-  _WhyNotPromotedVisitor(this.inferrer, this.receiver);
-
-  @override
-  LocatedMessage visitDemoteViaExplicitWrite(
-      DemoteViaExplicitWrite<VariableDeclaration, Expression> reason) {
-    if (inferrer.dataForTesting != null) {
-      inferrer.dataForTesting.flowAnalysisResult
-          .nonPromotionReasonTargets[reason.writeExpression] = reason.shortName;
-    }
-    int offset = reason.writeExpression.fileOffset;
-    return templateVariableCouldBeNullDueToWrite
-        .withArguments(reason.variable.name)
-        .withLocation(inferrer.helper.uri, offset, noLength);
-  }
-
-  @override
-  LocatedMessage visitDemoteViaForEachVariableWrite(
-      DemoteViaForEachVariableWrite<VariableDeclaration, Node> reason) {
-    int offset = (reason.node as TreeNode).fileOffset;
-    return templateVariableCouldBeNullDueToWrite
-        .withArguments(reason.variable.name)
-        .withLocation(inferrer.helper.uri, offset, noLength);
-  }
-
-  @override
-  LocatedMessage visitPropertyNotPromoted(PropertyNotPromoted reason) {
-    Member member;
-    Expression receiver = this.receiver;
-    if (receiver is InstanceGet) {
-      member = receiver.interfaceTarget;
-    } else if (receiver is SuperPropertyGet) {
-      member = receiver.interfaceTarget;
-    } else if (receiver is StaticInvocation) {
-      member = receiver.target;
-    } else if (receiver is PropertyGet) {
-      member = receiver.interfaceTarget;
-    } else {
-      assert(false, 'Unrecognized receiver: ${receiver.runtimeType}');
-    }
-    if (member != null) {
-      propertyReference = member;
-      return templateFieldNotPromoted
-          .withArguments(reason.propertyName)
-          .withLocation(member.fileUri, member.fileOffset, noLength);
-    } else {
-      return null;
-    }
-  }
-}
-
 class ForInResult {
   final VariableDeclaration variable;
   final Expression iterable;

pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart

@@ -5,14 +5,17 @@
 // @dart = 2.9
 
 import 'dart:core' hide MapEntry;
+import 'dart:core' as core;
 
 import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart'
     hide Reference; // Work around https://github.com/dart-lang/sdk/issues/44667
 
+import 'package:_fe_analyzer_shared/src/testing/id.dart';
 import 'package:_fe_analyzer_shared/src/util/link.dart';
 
 import 'package:front_end/src/fasta/kernel/internal_ast.dart';
 import 'package:front_end/src/fasta/type_inference/type_demotion.dart';
+import 'package:front_end/src/testing/id_extractor.dart';
 
 import 'package:kernel/ast.dart'
     hide Reference; // Work around https://github.com/dart-lang/sdk/issues/44667
@@ -301,6 +304,37 @@ class TypeInferrerImpl implements TypeInferrer {
       ..fileOffset = fileOffset;
   }
 
+  /// Computes a list of context messages explaining why [receiver] was not
+  /// promoted, to be used when reporting an error for a larger expression
+  /// containing [receiver].  [expression] is the containing expression.
+  List<LocatedMessage> getWhyNotPromotedContext(
+      Expression receiver, Expression expression) {
+    Map<DartType, NonPromotionReason> whyNotPromoted =
+        flowAnalysis?.whyNotPromoted(receiver);
+    List<LocatedMessage> context;
+    if (whyNotPromoted != null && whyNotPromoted.isNotEmpty) {
+      _WhyNotPromotedVisitor whyNotPromotedVisitor =
+          new _WhyNotPromotedVisitor(this, receiver);
+      for (core.MapEntry<DartType, NonPromotionReason> entry
+          in whyNotPromoted.entries) {
+        if (entry.key.isPotentiallyNullable) continue;
+        LocatedMessage message = entry.value.accept(whyNotPromotedVisitor);
+        if (dataForTesting != null) {
+          String nonPromotionReasonText = entry.value.shortName;
+          if (whyNotPromotedVisitor.propertyReference != null) {
+            Id id = computeMemberId(whyNotPromotedVisitor.propertyReference);
+            nonPromotionReasonText += '($id)';
+          }
+          dataForTesting.flowAnalysisResult.nonPromotionReasons[expression] =
+              nonPromotionReasonText;
+        }
+        context = [message];
+        break;
+      }
+    }
+    return context;
+  }
+
   /// Returns `true` if exceptions should be thrown in paths reachable only due
   /// to unsoundness in flow analysis in mixed mode.
   bool get shouldThrowUnsoundnessException =>
@@ -2754,7 +2788,8 @@ class TypeInferrerImpl implements TypeInferrer {
               templateNullableMethodCallError.withArguments(
                   name.text, receiverType, isNonNullableByDefault),
               fileOffset,
-              name.text.length);
+              name.text.length,
+              context: getWhyNotPromotedContext(receiver, staticInvocation));
         }
       }
       return createNullAwareExpressionInferenceResult(
@@ -2831,7 +2866,8 @@ class TypeInferrerImpl implements TypeInferrer {
             templateNullableMethodCallError.withArguments(
                 callName.text, receiverType, isNonNullableByDefault),
             fileOffset,
-            callName.text.length);
+            callName.text.length,
+            context: getWhyNotPromotedContext(receiver, expression));
       }
     }
     // TODO(johnniwinther): Check that type arguments against the bounds.
@@ -2993,7 +3029,8 @@ class TypeInferrerImpl implements TypeInferrer {
             templateNullableMethodCallError.withArguments(
                 methodName.text, receiverType, isNonNullableByDefault),
             fileOffset,
-            methodName.text.length);
+            methodName.text.length,
+            context: getWhyNotPromotedContext(receiver, expression));
       }
     }
 
@@ -4836,3 +4873,63 @@ class InferredFunctionBody {
 
   InferredFunctionBody(this.body, this.futureValueType);
 }
+
+class _WhyNotPromotedVisitor
+    implements
+        NonPromotionReasonVisitor<LocatedMessage, Node, Expression,
+            VariableDeclaration> {
+  final TypeInferrerImpl inferrer;
+
+  final Expression receiver;
+
+  Member propertyReference;
+
+  _WhyNotPromotedVisitor(this.inferrer, this.receiver);
+
+  @override
+  LocatedMessage visitDemoteViaExplicitWrite(
+      DemoteViaExplicitWrite<VariableDeclaration, Expression> reason) {
+    if (inferrer.dataForTesting != null) {
+      inferrer.dataForTesting.flowAnalysisResult
+          .nonPromotionReasonTargets[reason.writeExpression] = reason.shortName;
+    }
+    int offset = reason.writeExpression.fileOffset;
+    return templateVariableCouldBeNullDueToWrite
+        .withArguments(reason.variable.name)
+        .withLocation(inferrer.helper.uri, offset, noLength);
+  }
+
+  @override
+  LocatedMessage visitDemoteViaForEachVariableWrite(
+      DemoteViaForEachVariableWrite<VariableDeclaration, Node> reason) {
+    int offset = (reason.node as TreeNode).fileOffset;
+    return templateVariableCouldBeNullDueToWrite
+        .withArguments(reason.variable.name)
+        .withLocation(inferrer.helper.uri, offset, noLength);
+  }
+
+  @override
+  LocatedMessage visitPropertyNotPromoted(PropertyNotPromoted reason) {
+    Member member;
+    Expression receiver = this.receiver;
+    if (receiver is InstanceGet) {
+      member = receiver.interfaceTarget;
+    } else if (receiver is SuperPropertyGet) {
+      member = receiver.interfaceTarget;
+    } else if (receiver is StaticInvocation) {
+      member = receiver.target;
+    } else if (receiver is PropertyGet) {
+      member = receiver.interfaceTarget;
+    } else {
+      assert(false, 'Unrecognized receiver: ${receiver.runtimeType}');
+    }
+    if (member != null) {
+      propertyReference = member;
+      return templateFieldNotPromoted
+          .withArguments(reason.propertyName)
+          .withLocation(member.fileUri, member.fileOffset, noLength);
+    } else {
+      return null;
+    }
+  }
+}

pkg/front_end/test/lint_test.status

@@ -20,7 +20,6 @@ front_end/lib/src/fasta/incremental_compiler/ImportsTwice: Fail
 front_end/lib/src/fasta/kernel/body_builder/ImportsTwice: Fail
 front_end/lib/src/fasta/kernel/constant_evaluator/ExplicitType: Pass
 front_end/lib/src/fasta/kernel/expression_generator_helper/ImportsTwice: Fail
-front_end/lib/src/fasta/kernel/inference_visitor/ImportsTwice: Fail
 front_end/lib/src/fasta/kernel/kernel_api/Exports: Fail
 front_end/lib/src/fasta/kernel/kernel_ast_api/Exports: Fail
 front_end/lib/src/fasta/kernel/kernel_builder/Exports: Fail

pkg/front_end/test/spell_checking_list_code.txt

@@ -390,6 +390,7 @@ existentially
 exp
 expando
 expense
+explaining
 exportable
 exportee
 exportees