Skip to content

Commit e610dd4

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Migration: handle the multiple variable declarations in one line.
When the source code contains a declaration like `var x = ..., y = ...;`, and the variables need to be given different explicit types, there's no easy way to do so because replacing the `var` with a type causes the same type to be applied to *all* variables in the declaration. In an ideal world, the migration tool would split the declaration up into several declarations and give each one the proper type. But doing so would require a lot of coding effort, and the problem is extremely rare in practice (as witnessed by the fact that the issue hasn't been reported until now). So we opt for the much simpler fix of introducing an `as` cast to the initializer of each variable whose type needs to be changed. This is less ideal (since the `as` expressions bypass compile-time type safety), but considering how rarely this problem crops up it seems like a worthwhile tradeoff. Since the `as` expressions that are introduced are not downcasts, they will show up near the top of the list of changes in the migration tool's UI, so hopefully this will encourage the user to manually split up the declaration into multiple lines and replace the casts with explicit types. Fixes #47669. Bug: #47669 Change-Id: Idd7620cb5bb682c5fe235a4b088b94cd2208ebbd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220060 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent d21897b commit e610dd4

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

pkg/nnbd_migration/lib/src/fix_builder.dart

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,8 +1030,22 @@ class _FixBuilderPostVisitor extends GeneralizingAstVisitor<void>
10301030
if (explicitTypeNeeded) {
10311031
var firstNeededType = neededTypes[0];
10321032
if (neededTypes.any((t) => t != firstNeededType)) {
1033-
throw UnimplementedError(
1034-
'Different explicit types needed in multi-variable declaration');
1033+
// Different variables need different types. We handle this by
1034+
// introducing casts, which is not great but gets the job done.
1035+
for (int i = 0; i < node.variables.length; i++) {
1036+
if (neededTypes[i] != inferredTypes[i]) {
1037+
// We only have to worry about variables with initializers because
1038+
// variables without initializers will get the type `dynamic`.
1039+
var initializer = node.variables[i].initializer;
1040+
if (initializer != null) {
1041+
(_fixBuilder._getChange(initializer) as NodeChangeForExpression)
1042+
.addExpressionChange(
1043+
IntroduceAsChange(neededTypes[i], isDowncast: false),
1044+
AtomicEditInfo(
1045+
NullabilityFixDescription.otherCastExpression, {}));
1046+
}
1047+
}
1048+
}
10351049
} else {
10361050
(_fixBuilder._getChange(node) as NodeChangeForVariableDeclarationList)
10371051
.addExplicitType = firstNeededType;

pkg/nnbd_migration/test/api_test.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8952,6 +8952,24 @@ f(bool b, List<C<int>> cs) {
89528952
await _checkSingleFileChanges(content, expected);
89538953
}
89548954

8955+
Future<void> test_var_with_different_types_becoming_explicit() async {
8956+
// When types need to be added to some variables in a declaration but not
8957+
// others, we handle it by introducing `as` casts.
8958+
var content = '''
8959+
f(int i, String s) {
8960+
var x = i, y = s;
8961+
x = null;
8962+
}
8963+
''';
8964+
var expected = '''
8965+
f(int i, String s) {
8966+
var x = i as int?, y = s;
8967+
x = null;
8968+
}
8969+
''';
8970+
await _checkSingleFileChanges(content, expected);
8971+
}
8972+
89558973
Future<void> test_weak_if_visit_weak_subexpression() async {
89568974
var content = '''
89578975
int f(int x, int/*?*/ y) {

0 commit comments

Comments
 (0)