Reland "Flow analysis: track property get targets."

This is a reland of bc6cdf5

Original change's description:
> Flow analysis: track property get targets.
>
> These targets are needed for "why not promoted" error messages, and
> it's easier to have flow analysis keep track of them than to have the
> analyzer and CFE try to reconstruct them at the time of reporting the
> error.
>
> Bug: #44898
> Change-Id: Ia8ef4a7ce13cc30860e59b7369e6230d233e252d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193832
> Commit-Queue: Paul Berry <[email protected]>
> Reviewed-by: Johnni Winther <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>

Bug: #44898
Change-Id: Ib5e6a0eafb381f3a9c8770c6c1c3e55581e44b01
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194105
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: William Hesse <[email protected]>

Author: stereotype441

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

@@ -736,8 +736,13 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
   /// expression to the left hand side of the `.`, and [propertyName] should be
   /// the identifier to the right hand side of the `.`.  [staticType] should be
   /// the static type of the value returned by the property get.
+  ///
+  /// [propertyMember] should be whatever data structure the client uses to keep
+  /// track of the field or property being accessed.  In the event of
+  /// non-promotion of a property get, this value can be retrieved from
+  /// [PropertyNotPromoted.propertyMember].
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Type staticType);
+      String propertyName, Object? propertyMember, Type staticType);
 
   /// Retrieves the SSA node associated with [variable], or `null` if [variable]
   /// is not associated with an SSA node because it is write captured.  For
@@ -788,8 +793,13 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
   /// the whole property get, and [propertyName] should be the name of the
   /// property being read.  [staticType] should be the static type of the value
   /// returned by the property get.
-  void thisOrSuperPropertyGet(
-      Expression expression, String propertyName, Type staticType);
+  ///
+  /// [propertyMember] should be whatever data structure the client uses to keep
+  /// track of the field or property being accessed.  In the event of
+  /// non-promotion of a property get, this value can be retrieved from
+  /// [PropertyNotPromoted.propertyMember].
+  void thisOrSuperPropertyGet(Expression expression, String propertyName,
+      Object? propertyMember, Type staticType);
 
   /// Call this method just before visiting the body of a "try/catch" statement.
   ///
@@ -1339,11 +1349,12 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
 
   @override
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Type staticType) {
+      String propertyName, Object? propertyMember, Type staticType) {
     _wrap(
-        'propertyGet($wholeExpression, $target, $propertyName, $staticType)',
+        'propertyGet($wholeExpression, $target, $propertyName, '
+        '$propertyMember, $staticType)',
         () => _wrapped.propertyGet(
-            wholeExpression, target, propertyName, staticType));
+            wholeExpression, target, propertyName, propertyMember, staticType));
   }
 
   @override
@@ -1378,12 +1389,13 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
   }
 
   @override
-  void thisOrSuperPropertyGet(
-      Expression expression, String propertyName, Type staticType) {
+  void thisOrSuperPropertyGet(Expression expression, String propertyName,
+      Object? propertyMember, Type staticType) {
     _wrap(
-        'thisOrSuperPropertyGet($expression, $propertyName, $staticType)',
+        'thisOrSuperPropertyGet($expression, $propertyName, $propertyMember, '
+        '$staticType)',
         () => _wrapped.thisOrSuperPropertyGet(
-            expression, propertyName, staticType));
+            expression, propertyName, propertyMember, staticType));
   }
 
   @override
@@ -2334,12 +2346,17 @@ class PropertyNotPromoted<Type extends Object> extends NonPromotionReason {
   /// The name of the property.
   final String propertyName;
 
+  /// The field or property being accessed.  This matches a `propertyMember`
+  /// value that was passed to either [FlowAnalysis.propertyGet] or
+  /// [FlowAnalysis.thisOrSuperPropertyGet].
+  final Object? propertyMember;
+
   /// The static type of the property at the time of the access.  This is the
   /// type that was passed to [FlowAnalysis.whyNotPromoted]; it is provided to
   /// the client as a convenience for ID testing.
   final Type staticType;
 
-  PropertyNotPromoted(this.propertyName, this.staticType);
+  PropertyNotPromoted(this.propertyName, this.propertyMember, this.staticType);
 
   @override
   String get documentationLink => 'http://dart.dev/go/non-promo-property';
@@ -2517,8 +2534,10 @@ abstract class Reference<Variable extends Object, Type extends Object> {
 
   /// Creates a reference representing a get of a property called [propertyName]
   /// on the reference represented by `this`.
-  Reference<Variable, Type> propertyGet(String propertyName) =>
-      new _PropertyGetReference<Variable, Type>(this, propertyName);
+  Reference<Variable, Type> propertyGet(
+          String propertyName, Object? propertyMember) =>
+      new _PropertyGetReference<Variable, Type>(
+          this, propertyName, propertyMember);
 
   /// Stores info for this reference in [variableInfo].
   void storeInfo(Map<Variable?, VariableModel<Variable, Type>> variableInfo,
@@ -3952,14 +3971,14 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
 
   @override
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Type staticType) {
+      String propertyName, Object? propertyMember, Type staticType) {
     Reference<Variable, Type>? reference =
         _getExpressionReference(target)?.reference;
     if (reference != null) {
       _storeExpressionReference(
           wholeExpression,
           new ReferenceWithType<Variable, Type>(
-              reference.propertyGet(propertyName), staticType));
+              reference.propertyGet(propertyName, propertyMember), staticType));
     }
   }
 
@@ -4016,12 +4035,13 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
   }
 
   @override
-  void thisOrSuperPropertyGet(
-      Expression expression, String propertyName, Type staticType) {
+  void thisOrSuperPropertyGet(Expression expression, String propertyName,
+      Object? propertyMember, Type staticType) {
     _storeExpressionReference(
         expression,
         new ReferenceWithType<Variable, Type>(
-            new _ThisReference<Variable, Type>().propertyGet(propertyName),
+            new _ThisReference<Variable, Type>()
+                .propertyGet(propertyName, propertyMember),
             staticType));
   }
 
@@ -4686,7 +4706,7 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
 
   @override
   void propertyGet(Expression wholeExpression, Expression target,
-      String propertyName, Type staticType) {}
+      String propertyName, Object? propertyMember, Type staticType) {}
 
   @override
   SsaNode<Variable, Type>? ssaNodeForTesting(Variable variable) {
@@ -4706,8 +4726,8 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
   void thisOrSuper(Expression expression, Type staticType) {}
 
   @override
-  void thisOrSuperPropertyGet(
-      Expression expression, String propertyName, Type staticType) {}
+  void thisOrSuperPropertyGet(Expression expression, String propertyName,
+      Object? propertyMember, Type staticType) {}
 
   @override
   void tryCatchStatement_bodyBegin() {}
@@ -4908,7 +4928,12 @@ class _PropertyGetReference<Variable extends Object, Type extends Object>
   /// The name of the property.
   final String propertyName;
 
-  _PropertyGetReference(this.target, this.propertyName);
+  /// The field or property being accessed.  This matches a `propertyMember`
+  /// value that was passed to either [FlowAnalysis.propertyGet] or
+  /// [FlowAnalysis.thisOrSuperPropertyGet].
+  final Object? propertyMember;
+
+  _PropertyGetReference(this.target, this.propertyName, this.propertyMember);
 
   @override
   Map<Type, NonPromotionReason> Function() getNonPromotionReasons(
@@ -4920,7 +4945,8 @@ class _PropertyGetReference<Variable extends Object, Type extends Object>
       return () {
         Map<Type, NonPromotionReason> result = <Type, NonPromotionReason>{};
         for (Type type in promotedTypes) {
-          result[type] = new PropertyNotPromoted(propertyName, staticType);
+          result[type] =
+              new PropertyNotPromoted(propertyName, propertyMember, staticType);
         }
         return result;
       };

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -1510,7 +1510,7 @@ class _Property extends LValue {
       Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
     var targetType = target._visit(h, flow);
     var propertyType = h.getMember(targetType, propertyName);
-    flow.propertyGet(this, target, propertyName, propertyType);
+    flow.propertyGet(this, target, propertyName, propertyName, propertyType);
     return propertyType;
   }
 
@@ -1614,7 +1614,7 @@ class _ThisOrSuperPropertyGet extends Expression {
   @override
   Type _visit(
       Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
-    flow.thisOrSuperPropertyGet(this, propertyName, type);
+    flow.thisOrSuperPropertyGet(this, propertyName, propertyName, type);
     return type;
   }
 }

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

@@ -121,8 +121,7 @@ class AssignmentExpressionResolver {
       CompileTimeErrorCode.INVALID_ASSIGNMENT,
       right,
       [rightType, writeType],
-      _resolver.computeWhyNotPromotedMessages(
-          right, right, whyNotPromoted?.call()),
+      _resolver.computeWhyNotPromotedMessages(right, whyNotPromoted?.call()),
     );
   }
 

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

@@ -807,7 +807,11 @@ class MethodInvocationResolver {
         );
       }
       _resolver.flowAnalysis?.flow?.propertyGet(
-          functionExpression, target, node.methodName.name, getterReturnType);
+          functionExpression,
+          target,
+          node.methodName.name,
+          node.methodName.staticElement,
+          getterReturnType);
       functionExpression.staticType = targetType;
     }
 

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

@@ -203,8 +203,8 @@ class PropertyElementResolver {
       readElementRequested = readLookup.requested;
       if (readElementRequested is PropertyAccessorElement &&
           !readElementRequested.isStatic) {
-        _resolver.flowAnalysis?.flow?.thisOrSuperPropertyGet(
-            node, node.name, readElementRequested.returnType);
+        _resolver.flowAnalysis?.flow?.thisOrSuperPropertyGet(node, node.name,
+            readElementRequested, readElementRequested.returnType);
       }
       _resolver.checkReadOfNotAssignedLocalVariable(node, readElementRequested);
     }
@@ -373,7 +373,11 @@ class PropertyElementResolver {
       nameErrorEntity: propertyName,
     );
 
-    _resolver.flowAnalysis?.flow?.propertyGet(node, target, propertyName.name,
+    _resolver.flowAnalysis?.flow?.propertyGet(
+        node,
+        target,
+        propertyName.name,
+        result.getter,
         result.getter?.returnType ?? _typeSystem.typeProvider.dynamicType);
 
     if (hasRead && result.needsGetterError) {
@@ -653,6 +657,7 @@ class PropertyElementResolver {
             node,
             target,
             propertyName.name,
+            readElement,
             readElement?.returnType ?? _typeSystem.typeProvider.dynamicType);
       }
 

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

@@ -130,11 +130,11 @@ class TypePropertyResolver {
       if (flow != null) {
         if (receiver != null) {
           messages = _resolver.computeWhyNotPromotedMessages(
-              receiver, nameErrorEntity, flow.whyNotPromoted(receiver)());
+              nameErrorEntity, flow.whyNotPromoted(receiver)());
         } else {
           var thisType = _resolver.thisType;
           if (thisType != null) {
-            messages = _resolver.computeWhyNotPromotedMessages(receiver,
+            messages = _resolver.computeWhyNotPromotedMessages(
                 nameErrorEntity, flow.whyNotPromotedImplicitThis(thisType)());
           }
         }

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

@@ -57,7 +57,7 @@ class BoolExpressionVerifier {
             errorCode: CompileTimeErrorCode
                 .UNCHECKED_USE_OF_NULLABLE_VALUE_AS_CONDITION,
             messages: _resolver.computeWhyNotPromotedMessages(
-                expression, expression, whyNotPromoted?.call()));
+                expression, whyNotPromoted?.call()));
       } else {
         _errorReporter.reportErrorForNode(errorCode, expression, arguments);
       }

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

@@ -76,8 +76,8 @@ class NullableDereferenceVerifier {
 
     List<DiagnosticMessage>? messages;
     if (errorNode is Expression) {
-      messages = _resolver.computeWhyNotPromotedMessages(errorNode, errorNode,
-          _resolver.flowAnalysis?.flow?.whyNotPromoted(errorNode)());
+      messages = _resolver.computeWhyNotPromotedMessages(
+          errorNode, _resolver.flowAnalysis?.flow?.whyNotPromoted(errorNode)());
     }
     report(errorNode, receiverType, errorCode: errorCode, messages: messages);
     return true;

pkg/analyzer/lib/src/generated/error_detection_helpers.dart

@@ -85,8 +85,8 @@ mixin ErrorDetectionHelpers {
       }
       return;
     }
-    var messages = computeWhyNotPromotedMessages(
-        expression, expression, whyNotPromoted?.call());
+    var messages =
+        computeWhyNotPromotedMessages(expression, whyNotPromoted?.call());
     // report problem
     if (isConstConstructor) {
       // TODO(paulberry): this error should be based on the actual type of the
@@ -229,7 +229,6 @@ mixin ErrorDetectionHelpers {
   /// [whyNotPromoted] should be the non-promotion details returned by the flow
   /// analysis engine.
   List<DiagnosticMessage> computeWhyNotPromotedMessages(
-      Expression? expression,
       SyntacticEntity errorEntity,
       Map<DartType, NonPromotionReason>? whyNotPromoted);
 
@@ -313,8 +312,7 @@ mixin ErrorDetectionHelpers {
         errorCode,
         getErrorNode(expression),
         [actualStaticType, expectedStaticType],
-        computeWhyNotPromotedMessages(
-            expression, expression, whyNotPromoted?.call()),
+        computeWhyNotPromotedMessages(expression, whyNotPromoted?.call()),
       );
       return false;
     }

pkg/analyzer/lib/src/generated/error_verifier.dart

@@ -312,7 +312,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
 
   @override
   List<DiagnosticMessage> computeWhyNotPromotedMessages(
-      Expression? expression,
       SyntacticEntity errorEntity,
       Map<DartType, NonPromotionReason>? whyNotPromoted) {
     return [];

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

@@ -533,17 +533,13 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
 
   @override
   List<DiagnosticMessage> computeWhyNotPromotedMessages(
-      Expression? expression,
       SyntacticEntity errorEntity,
       Map<DartType, NonPromotionReason>? whyNotPromoted) {
-    if (expression is NamedExpression) {
-      expression = expression.expression;
-    }
     List<DiagnosticMessage> messages = [];
     if (whyNotPromoted != null) {
       for (var entry in whyNotPromoted.entries) {
         var whyNotPromotedVisitor = _WhyNotPromotedVisitor(
-            source, expression, errorEntity, flowAnalysis!.dataForTesting);
+            source, errorEntity, flowAnalysis!.dataForTesting);
         if (typeSystem.isPotentiallyNullable(entry.key)) continue;
         var message = entry.value.accept(whyNotPromotedVisitor);
         if (message != null) {
@@ -3447,10 +3443,6 @@ class _WhyNotPromotedVisitor
             PromotableElement, DartType> {
   final Source source;
 
-  /// The expression that was not promoted, or `null` if the thing that was not
-  /// promoted was an implicit `this`.
-  final Expression? _expression;
-
   final SyntacticEntity _errorEntity;
 
   final FlowAnalysisDataForTesting? _dataForTesting;
@@ -3459,8 +3451,7 @@ class _WhyNotPromotedVisitor
 
   DartType? propertyType;
 
-  _WhyNotPromotedVisitor(
-      this.source, this._expression, this._errorEntity, this._dataForTesting);
+  _WhyNotPromotedVisitor(this.source, this._errorEntity, this._dataForTesting);
 
   @override
   DiagnosticMessage? visitDemoteViaExplicitWrite(
@@ -3480,18 +3471,7 @@ class _WhyNotPromotedVisitor
   @override
   DiagnosticMessage? visitPropertyNotPromoted(
       PropertyNotPromoted<DartType> reason) {
-    var expression = _expression;
-    Element? receiverElement;
-    if (expression is SimpleIdentifier) {
-      receiverElement = expression.staticElement;
-    } else if (expression is PropertyAccess) {
-      receiverElement = expression.propertyName.staticElement;
-    } else if (expression is PrefixedIdentifier) {
-      receiverElement = expression.identifier.staticElement;
-    } else {
-      assert(false,
-          'Unrecognized property access expression: ${expression.runtimeType}');
-    }
+    var receiverElement = reason.propertyMember;
     if (receiverElement is PropertyAccessorElement) {
       propertyReference = receiverElement;
       propertyType = reason.staticType;