Flow analysis: expand analyzer support for "why not promoted" messages.

This CL adds support for the following scenarios to the analyzer:

- Attempt to use a non-promoted nullable expression as the iterable of
  a for-in loop

- Attempt to use a non-promoted nullable expression as the argument of
  a `yield *` statement

- Attempt to implicitly invoke `.call` on a non-promoted nullable
  expression

- Attempt to use a non-promoted nullable expression as the argument of
  a spread operator (`...`) that is not null-aware

Some of these cases are already handled by the CFE.  Others will be
addressed in a follow-up CL.

Change-Id: I3cf31b1496e1bd92fdd3f8192f04c98dff15077c
Bug: #44898
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186320
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Paul Berry <[email protected]>

Author: stereotype441

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

@@ -0,0 +1,21 @@
+// 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 condition that can lead to the front
+// end's `ForInLoopTypeNotIterableNullability` or
+// `ForInLoopTypeNotIterablePartNullability` errors, for which we wish to report
+// "why not promoted" context information.
+
+// TODO(paulberry): get this to work with the CFE and add additional test cases
+// if needed.
+
+class C1 {
+  List<int>? bad;
+}
+
+test(C1 c) {
+  if (c.bad == null) return;
+  for (var x
+      in /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad) {}
+}

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

@@ -0,0 +1,20 @@
+// 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 condition that can lead to the front
+// end's `InvalidAssignmentErrorNullability` or
+// `InvalidAssignmentErrorPartNullability` errors, for which we wish to report
+// "why not promoted" context information.
+
+// TODO(paulberry): get this to work with the CFE and add additional test cases
+// if needed.
+
+class C1 {
+  List<int>? bad;
+}
+
+test(C1 c) sync* {
+  if (c.bad == null) return;
+  yield* /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad;
+}

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

@@ -16,7 +16,7 @@ class C2 {
 
 instance_method_invocation(C1 c) {
   if (c.bad == null) return;
-  c.bad
+  /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad
       /*cfe.invoke: notPromoted(propertyNotPromoted(member:C1.bad))*/
       ();
 }
@@ -42,7 +42,7 @@ extension_invocation_method(C3 c) {
   if (c.ok == null) return;
   c.ok();
   if (c.bad == null) return;
-  c.bad
+  /*analyzer.notPromoted(propertyNotPromoted(member:C3.bad))*/ c.bad
       /*cfe.invoke: notPromoted(propertyNotPromoted(member:C3.bad))*/
       ();
 }
@@ -57,7 +57,7 @@ class C7 {
 
 instance_getter_invocation(C6 c) {
   if (c.bad == null) return;
-  c.bad
+  /*analyzer.notPromoted(propertyNotPromoted(member:C6.bad))*/ c.bad
       /*cfe.invoke: notPromoted(propertyNotPromoted(member:C6.bad))*/
       ();
 }
@@ -83,7 +83,7 @@ extension_invocation_getter(C8 c) {
   if (c.ok == null) return;
   c.ok();
   if (c.bad == null) return;
-  c.bad
+  /*analyzer.notPromoted(propertyNotPromoted(member:C8.bad))*/ c.bad
       /*cfe.invoke: notPromoted(propertyNotPromoted(member:C8.bad))*/
       ();
 }
@@ -94,7 +94,7 @@ class C11 {
 
 function_invocation(C11 c) {
   if (c.bad == null) return;
-  c.bad
+  /*analyzer.notPromoted(propertyNotPromoted(member:C11.bad))*/ c.bad
       /*cfe.invoke: notPromoted(propertyNotPromoted(member:C11.bad))*/
       ();
 }

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

@@ -0,0 +1,21 @@
+// 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 condition that can lead to the front
+// end's `NullableSpreadError` error, for which we wish to report "why not
+// promoted" context information.
+
+// TODO(paulberry): get this to work with the CFE and add additional test cases
+// if needed.
+
+class C1 {
+  List<int>? bad;
+}
+
+test(C1 c) {
+  if (c.bad == null) return;
+  return [
+    ... /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad
+  ];
+}

pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart

@@ -240,6 +240,19 @@ class FlowAnalysisHelper {
     flow!.finish();
   }
 
+  /// Transfers any test data that was recorded for [oldNode] so that it is now
+  /// associated with [newNode].  We need to do this when doing AST rewriting,
+  /// so that test data can be found using the rewritten tree.
+  void transferTestData(AstNode oldNode, AstNode newNode) {
+    var dataForTesting = this.dataForTesting;
+    if (dataForTesting != null) {
+      var oldNonPromotionReasons = dataForTesting.nonPromotionReasons[oldNode];
+      if (oldNonPromotionReasons != null) {
+        dataForTesting.nonPromotionReasons[newNode] = oldNonPromotionReasons;
+      }
+    }
+  }
+
   void variableDeclarationList(VariableDeclarationList node) {
     if (flow != null) {
       var variables = node.variables;

pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart

@@ -42,20 +42,24 @@ class FunctionExpressionInvocationResolver {
       return;
     }
 
+    var receiverType = function.staticType;
+    if (receiverType is InterfaceType) {
+      // Note: in this circumstance it's not necessary to call
+      // `_nullableDereferenceVerifier.expression` because
+      // `_resolveReceiverInterfaceType` calls `TypePropertyResolver.resolve`,
+      // which does the necessary null checking.
+      _resolveReceiverInterfaceType(node, function, receiverType);
+      return;
+    }
+
     _nullableDereferenceVerifier.expression(function,
         errorCode: CompileTimeErrorCode.UNCHECKED_INVOCATION_OF_NULLABLE_VALUE);
 
-    var receiverType = function.staticType;
     if (receiverType is FunctionType) {
       _resolve(node, receiverType);
       return;
     }
 
-    if (receiverType is InterfaceType) {
-      _resolveReceiverInterfaceType(node, function, receiverType);
-      return;
-    }
-
     if (identical(receiverType, NeverTypeImpl.instance)) {
       _unresolved(node, NeverTypeImpl.instance);
       return;

pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart

@@ -752,6 +752,8 @@ class MethodInvocationResolver {
           node.methodName,
         );
       }
+      _resolver.flowAnalysis?.flow
+          ?.propertyGet(functionExpression, target, node.methodName.name);
       functionExpression.staticType = targetType;
     }
 
@@ -763,6 +765,7 @@ class MethodInvocationResolver {
     NodeReplacer.replace(node, invocation);
     node.setProperty(_rewriteResultKey, invocation);
     InferenceContext.setTypeFromNode(invocation, node);
+    _resolver.flowAnalysis?.transferTestData(node, invocation);
   }
 
   void _setDynamicResolution(MethodInvocation node,

pkg/analyzer/lib/src/dart/resolver/type_property_resolver.dart

@@ -2,7 +2,6 @@
 // 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.
 
-import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/syntactic_entity.dart';
 import 'package:analyzer/dart/element/element.dart';
@@ -13,13 +12,9 @@ import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
 import 'package:analyzer/src/dart/element/type_provider.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
 import 'package:analyzer/src/dart/resolver/extension_member_resolver.dart';
-import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart';
 import 'package:analyzer/src/dart/resolver/resolution_result.dart';
-import 'package:analyzer/src/diagnostic/diagnostic.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/generated/resolver.dart';
-import 'package:analyzer/src/generated/source.dart';
-import 'package:analyzer/src/util/ast_data_extractor.dart';
 
 /// Helper for resolving properties (getters, setters, or methods).
 class TypePropertyResolver {
@@ -120,35 +115,8 @@ class TypePropertyResolver {
         }
       }
 
-      List<DiagnosticMessage> messages = [];
-      if (receiver != null) {
-        var whyNotPromoted =
-            _resolver.flowAnalysis?.flow?.whyNotPromoted(receiver);
-        if (whyNotPromoted != null) {
-          for (var entry in whyNotPromoted.entries) {
-            var whyNotPromotedVisitor = _WhyNotPromotedVisitor(_resolver.source,
-                receiver, _resolver.flowAnalysis!.dataForTesting);
-            if (_typeSystem.isPotentiallyNullable(entry.key)) continue;
-            var message = entry.value.accept(whyNotPromotedVisitor);
-            if (message != null) {
-              if (_resolver.flowAnalysis!.dataForTesting != null) {
-                var nonPromotionReasonText = entry.value.shortName;
-                if (whyNotPromotedVisitor.propertyReference != null) {
-                  var id =
-                      computeMemberId(whyNotPromotedVisitor.propertyReference!);
-                  nonPromotionReasonText += '($id)';
-                }
-                _resolver.flowAnalysis!.dataForTesting!
-                        .nonPromotionReasons[nameErrorEntity] =
-                    nonPromotionReasonText;
-              }
-              messages = [message];
-            }
-            break;
-          }
-        }
-      }
-
+      List<DiagnosticMessage> messages =
+          _resolver.computeWhyNotPromotedMessages(receiver, nameErrorEntity);
       _resolver.nullableDereferenceVerifier.report(
           receiverErrorNode, receiverType,
           errorCode: errorCode, arguments: [name], messages: messages);
@@ -299,102 +267,3 @@ class TypePropertyResolver {
     );
   }
 }
-
-class _WhyNotPromotedVisitor
-    implements
-        NonPromotionReasonVisitor<DiagnosticMessage?, AstNode, Expression,
-            PromotableElement> {
-  final Source source;
-
-  final Expression _receiver;
-
-  final FlowAnalysisDataForTesting? _dataForTesting;
-
-  PropertyAccessorElement? propertyReference;
-
-  _WhyNotPromotedVisitor(this.source, this._receiver, this._dataForTesting);
-
-  @override
-  DiagnosticMessage? visitDemoteViaExplicitWrite(
-      DemoteViaExplicitWrite<PromotableElement, Expression> reason) {
-    var writeExpression = reason.writeExpression;
-    if (_dataForTesting != null) {
-      _dataForTesting!.nonPromotionReasonTargets[writeExpression] =
-          reason.shortName;
-    }
-    var variableName = reason.variable.name;
-    if (variableName == null) return null;
-    return _contextMessageForWrite(variableName, writeExpression);
-  }
-
-  @override
-  DiagnosticMessage? visitDemoteViaForEachVariableWrite(
-      DemoteViaForEachVariableWrite<PromotableElement, AstNode> reason) {
-    var node = reason.node;
-    var variableName = reason.variable.name;
-    if (variableName == null) return null;
-    ForLoopParts parts;
-    if (node is ForStatement) {
-      parts = node.forLoopParts;
-    } else if (node is ForElement) {
-      parts = node.forLoopParts;
-    } else {
-      assert(false, 'Unexpected node type');
-      return null;
-    }
-    if (parts is ForEachPartsWithIdentifier) {
-      var identifier = parts.identifier;
-      if (_dataForTesting != null) {
-        _dataForTesting!.nonPromotionReasonTargets[identifier] =
-            reason.shortName;
-      }
-      return _contextMessageForWrite(variableName, identifier);
-    } else {
-      assert(false, 'Unexpected parts type');
-      return null;
-    }
-  }
-
-  @override
-  DiagnosticMessage? visitPropertyNotPromoted(PropertyNotPromoted reason) {
-    var receiver = _receiver;
-    Element? receiverElement;
-    if (receiver is SimpleIdentifier) {
-      receiverElement = receiver.staticElement;
-    } else if (receiver is PropertyAccess) {
-      receiverElement = receiver.propertyName.staticElement;
-    } else if (receiver is PrefixedIdentifier) {
-      receiverElement = receiver.identifier.staticElement;
-    } else {
-      assert(false, 'Unrecognized receiver: ${receiver.runtimeType}');
-    }
-    if (receiverElement is PropertyAccessorElement) {
-      propertyReference = receiverElement;
-      return _contextMessageForProperty(receiverElement, reason.propertyName);
-    } else {
-      assert(receiverElement == null,
-          'Unrecognized receiver element: ${receiverElement.runtimeType}');
-      return null;
-    }
-  }
-
-  DiagnosticMessageImpl _contextMessageForProperty(
-      PropertyAccessorElement property, String propertyName) {
-    return DiagnosticMessageImpl(
-        filePath: property.source.fullName,
-        message:
-            "'$propertyName' refers to a property so it could not be promoted.",
-        offset: property.nameOffset,
-        length: property.nameLength);
-  }
-
-  DiagnosticMessageImpl _contextMessageForWrite(
-      String variableName, Expression writeExpression) {
-    return DiagnosticMessageImpl(
-        filePath: source.fullName,
-        message: "Variable '$variableName' could be null due to an intervening "
-            "write.",
-        offset: writeExpression.offset,
-        length: writeExpression.length);
-  }
-}

pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart

@@ -11,17 +11,23 @@ import 'package:analyzer/src/dart/ast/extensions.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
 import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/resolver.dart';
 
 /// Helper for checking potentially nullable dereferences.
 class NullableDereferenceVerifier {
   final TypeSystemImpl _typeSystem;
   final ErrorReporter _errorReporter;
 
+  /// The resolver driving this participant.
+  final ResolverVisitor _resolver;
+
   NullableDereferenceVerifier({
     required TypeSystemImpl typeSystem,
     required ErrorReporter errorReporter,
+    required ResolverVisitor resolver,
   })   : _typeSystem = typeSystem,
-        _errorReporter = errorReporter;
+        _errorReporter = errorReporter,
+        _resolver = resolver;
 
   bool expression(Expression expression,
       {DartType? type, ErrorCode? errorCode}) {
@@ -59,7 +65,11 @@ class NullableDereferenceVerifier {
       return false;
     }
 
-    report(errorNode, receiverType, errorCode: errorCode);
+    List<DiagnosticMessage>? messages;
+    if (errorNode is Expression) {
+      messages = _resolver.computeWhyNotPromotedMessages(errorNode, errorNode);
+    }
+    report(errorNode, receiverType, errorCode: errorCode, messages: messages);
     return true;
   }
 }