commit 5594981c2a03a8f5587f488ee5245d6038d21a7a
author Paul Berry <paulberry@google.com> Sat Oct 12 03:11:03 2019 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Sat Oct 12 03:11:03 2019 +0000
tree 90fd16aae14eabfeba53f8491d36b098b230277a
parent 13f6d8213bc1f2f0a4601a66bbddeb26b98abd49

Migration: add isCompound parameter to FixBuilder.visitAssignmentTarget.

When visiting an assignment target that is an index expression
(`x[y]`) we'll need to know whether we are doing a compound assignment
or not, because this will influence whether the type context for the
index expression (`y`) should come from `operator[]` or `operator[]=`.

So before adding index expression support to FixBuilder, let's add an
`isCompound` boolean to indicate whether the assignment context is a
compound assignment or not.

Change-Id: I26a41544e10ef9c9ba042c1d7862563bd9d18b68
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121407
Reviewed-by: Mike Fairhurst <mfairhurst@google.com>

pkg/nnbd_migration/lib/src/fix_builder.dart

diff --git a/pkg/nnbd_migration/lib/src/fix_builder.dart b/pkg/nnbd_migration/lib/src/fix_builder.dart
index f3abc57..6322c06 100644
--- a/pkg/nnbd_migration/lib/src/fix_builder.dart
+++ b/pkg/nnbd_migration/lib/src/fix_builder.dart
@@ -126,10 +126,12 @@
 
   @override
   DartType visitAssignmentExpression(AssignmentExpression node) {
-    var targetInfo = visitAssignmentTarget(node.leftHandSide);
-    if (node.operator.type == TokenType.EQ) {
+    var operatorType = node.operator.type;
+    var targetInfo =
+        visitAssignmentTarget(node.leftHandSide, operatorType != TokenType.EQ);
+    if (operatorType == TokenType.EQ) {
       return visitSubexpression(node.rightHandSide, targetInfo.writeType);
-    } else if (node.operator.type == TokenType.QUESTION_QUESTION_EQ) {
+    } else if (operatorType == TokenType.QUESTION_QUESTION_EQ) {
       // TODO(paulberry): if targetInfo.readType is non-nullable, then the
       // assignment is dead code.
       // See https://github.com/dart-lang/sdk/issues/38678
@@ -166,14 +168,17 @@
 
   /// Recursively visits an assignment target, returning information about the
   /// target's read and write types.
-  AssignmentTargetInfo visitAssignmentTarget(Expression node) {
+  ///
+  /// If [isCompound] is true, the target is being both read from and written
+  /// to.  If it is false, then only the write type is needed.
+  AssignmentTargetInfo visitAssignmentTarget(Expression node, bool isCompound) {
     if (node is SimpleIdentifier) {
       var writeType = _computeMigratedType(node.staticElement);
       var auxiliaryElements = node.auxiliaryElements;
       var readType = auxiliaryElements == null
           ? writeType
           : _computeMigratedType(auxiliaryElements.staticElement);
-      return AssignmentTargetInfo(readType, writeType);
+      return AssignmentTargetInfo(isCompound ? readType : null, writeType);
     } else {
       throw UnimplementedError('TODO(paulberry)');
     }
@@ -341,7 +346,7 @@
           'TODO(paulberry): re-migration of already migrated code not '
           'supported yet');
     } else {
-      var targetInfo = visitAssignmentTarget(node.operand);
+      var targetInfo = visitAssignmentTarget(node.operand, true);
       _handleIncrementOrDecrement(node.staticElement, targetInfo, node);
       return targetInfo.readType;
     }
@@ -371,7 +376,7 @@
       case TokenType.PLUS_PLUS:
       case TokenType.MINUS_MINUS:
         return _handleIncrementOrDecrement(
-            node.staticElement, visitAssignmentTarget(operand), node);
+            node.staticElement, visitAssignmentTarget(operand, true), node);
       default:
         throw StateError('Unexpected prefix operator: ${node.operator}');
     }

pkg/nnbd_migration/test/fix_builder_test.dart

diff --git a/pkg/nnbd_migration/test/fix_builder_test.dart b/pkg/nnbd_migration/test/fix_builder_test.dart
index f25ba0b..143c4a7 100644
--- a/pkg/nnbd_migration/test/fix_builder_test.dart
+++ b/pkg/nnbd_migration/test/fix_builder_test.dart
@@ -338,7 +338,7 @@
   _f() => x = 0;
 }
 ''');
-    visitAssignmentTarget(findNode.simple('x '), 'int?', 'int?');
+    visitAssignmentTarget(findNode.simple('x '), null, 'int?');
   }
 
   test_binaryExpression_ampersand_ampersand() async {
@@ -1203,9 +1203,14 @@
       {Set<Expression> nullChecked = const <Expression>{},
       Map<AstNode, Set<Problem>> problems = const <AstNode, Set<Problem>>{}}) {
     _FixBuilder fixBuilder = _createFixBuilder(node);
-    var targetInfo = fixBuilder.visitAssignmentTarget(node);
-    expect((targetInfo.readType as TypeImpl).toString(withNullability: true),
-        expectedReadType);
+    var targetInfo =
+        fixBuilder.visitAssignmentTarget(node, expectedReadType != null);
+    if (expectedReadType == null) {
+      expect(targetInfo.readType, null);
+    } else {
+      expect((targetInfo.readType as TypeImpl).toString(withNullability: true),
+          expectedReadType);
+    }
     expect((targetInfo.writeType as TypeImpl).toString(withNullability: true),
         expectedWriteType);
     expect(fixBuilder.nullCheckedExpressions, nullChecked);