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: https://github.com/dart-lang/sdk/issues/47669
Change-Id: Idd7620cb5bb682c5fe235a4b088b94cd2208ebbd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220060
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/nnbd_migration/lib/src/fix_builder.dart b/pkg/nnbd_migration/lib/src/fix_builder.dart
index afb6393..44f4d37 100644
--- a/pkg/nnbd_migration/lib/src/fix_builder.dart
+++ b/pkg/nnbd_migration/lib/src/fix_builder.dart
@@ -1030,8 +1030,22 @@
if (explicitTypeNeeded) {
var firstNeededType = neededTypes[0];
if (neededTypes.any((t) => t != firstNeededType)) {
- throw UnimplementedError(
- 'Different explicit types needed in multi-variable declaration');
+ // Different variables need different types. We handle this by
+ // introducing casts, which is not great but gets the job done.
+ for (int i = 0; i < node.variables.length; i++) {
+ if (neededTypes[i] != inferredTypes[i]) {
+ // We only have to worry about variables with initializers because
+ // variables without initializers will get the type `dynamic`.
+ var initializer = node.variables[i].initializer;
+ if (initializer != null) {
+ (_fixBuilder._getChange(initializer) as NodeChangeForExpression)
+ .addExpressionChange(
+ IntroduceAsChange(neededTypes[i], isDowncast: false),
+ AtomicEditInfo(
+ NullabilityFixDescription.otherCastExpression, {}));
+ }
+ }
+ }
} else {
(_fixBuilder._getChange(node) as NodeChangeForVariableDeclarationList)
.addExplicitType = firstNeededType;
diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart
index 69a8936..286aa4e 100644
--- a/pkg/nnbd_migration/test/api_test.dart
+++ b/pkg/nnbd_migration/test/api_test.dart
@@ -8952,6 +8952,24 @@
await _checkSingleFileChanges(content, expected);
}
+ Future<void> test_var_with_different_types_becoming_explicit() async {
+ // When types need to be added to some variables in a declaration but not
+ // others, we handle it by introducing `as` casts.
+ var content = '''
+f(int i, String s) {
+ var x = i, y = s;
+ x = null;
+}
+''';
+ var expected = '''
+f(int i, String s) {
+ var x = i as int?, y = s;
+ x = null;
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
Future<void> test_weak_if_visit_weak_subexpression() async {
var content = '''
int f(int x, int/*?*/ y) {
