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]>

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;