Skip to content

Commit 5594981

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
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>
1 parent 13f6d82 commit 5594981

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

pkg/nnbd_migration/lib/src/fix_builder.dart

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,12 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
126126

127127
@override
128128
DartType visitAssignmentExpression(AssignmentExpression node) {
129-
var targetInfo = visitAssignmentTarget(node.leftHandSide);
130-
if (node.operator.type == TokenType.EQ) {
129+
var operatorType = node.operator.type;
130+
var targetInfo =
131+
visitAssignmentTarget(node.leftHandSide, operatorType != TokenType.EQ);
132+
if (operatorType == TokenType.EQ) {
131133
return visitSubexpression(node.rightHandSide, targetInfo.writeType);
132-
} else if (node.operator.type == TokenType.QUESTION_QUESTION_EQ) {
134+
} else if (operatorType == TokenType.QUESTION_QUESTION_EQ) {
133135
// TODO(paulberry): if targetInfo.readType is non-nullable, then the
134136
// assignment is dead code.
135137
// See https://github.com/dart-lang/sdk/issues/38678
@@ -166,14 +168,17 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
166168

167169
/// Recursively visits an assignment target, returning information about the
168170
/// target's read and write types.
169-
AssignmentTargetInfo visitAssignmentTarget(Expression node) {
171+
///
172+
/// If [isCompound] is true, the target is being both read from and written
173+
/// to. If it is false, then only the write type is needed.
174+
AssignmentTargetInfo visitAssignmentTarget(Expression node, bool isCompound) {
170175
if (node is SimpleIdentifier) {
171176
var writeType = _computeMigratedType(node.staticElement);
172177
var auxiliaryElements = node.auxiliaryElements;
173178
var readType = auxiliaryElements == null
174179
? writeType
175180
: _computeMigratedType(auxiliaryElements.staticElement);
176-
return AssignmentTargetInfo(readType, writeType);
181+
return AssignmentTargetInfo(isCompound ? readType : null, writeType);
177182
} else {
178183
throw UnimplementedError('TODO(paulberry)');
179184
}
@@ -341,7 +346,7 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
341346
'TODO(paulberry): re-migration of already migrated code not '
342347
'supported yet');
343348
} else {
344-
var targetInfo = visitAssignmentTarget(node.operand);
349+
var targetInfo = visitAssignmentTarget(node.operand, true);
345350
_handleIncrementOrDecrement(node.staticElement, targetInfo, node);
346351
return targetInfo.readType;
347352
}
@@ -371,7 +376,7 @@ abstract class FixBuilder extends GeneralizingAstVisitor<DartType> {
371376
case TokenType.PLUS_PLUS:
372377
case TokenType.MINUS_MINUS:
373378
return _handleIncrementOrDecrement(
374-
node.staticElement, visitAssignmentTarget(operand), node);
379+
node.staticElement, visitAssignmentTarget(operand, true), node);
375380
default:
376381
throw StateError('Unexpected prefix operator: ${node.operator}');
377382
}

pkg/nnbd_migration/test/fix_builder_test.dart

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ class _C {
338338
_f() => x = 0;
339339
}
340340
''');
341-
visitAssignmentTarget(findNode.simple('x '), 'int?', 'int?');
341+
visitAssignmentTarget(findNode.simple('x '), null, 'int?');
342342
}
343343

344344
test_binaryExpression_ampersand_ampersand() async {
@@ -1203,9 +1203,14 @@ void _f(bool/*?*/ x, bool/*?*/ y) {
12031203
{Set<Expression> nullChecked = const <Expression>{},
12041204
Map<AstNode, Set<Problem>> problems = const <AstNode, Set<Problem>>{}}) {
12051205
_FixBuilder fixBuilder = _createFixBuilder(node);
1206-
var targetInfo = fixBuilder.visitAssignmentTarget(node);
1207-
expect((targetInfo.readType as TypeImpl).toString(withNullability: true),
1208-
expectedReadType);
1206+
var targetInfo =
1207+
fixBuilder.visitAssignmentTarget(node, expectedReadType != null);
1208+
if (expectedReadType == null) {
1209+
expect(targetInfo.readType, null);
1210+
} else {
1211+
expect((targetInfo.readType as TypeImpl).toString(withNullability: true),
1212+
expectedReadType);
1213+
}
12091214
expect((targetInfo.writeType as TypeImpl).toString(withNullability: true),
12101215
expectedWriteType);
12111216
expect(fixBuilder.nullCheckedExpressions, nullChecked);

0 commit comments

Comments
 (0)