Do not convert return type of enclosing function when adding async to nested closure (issue 30901)

Change-Id: I8293b7784f71d4ff7846a0cca6efbfa69847b7da Reviewed-on: https://dart-review.googlesource.com/37646 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
2024-09-16 00:39:49 +00:00 · 2018-01-30 21:57:10 +00:00 · 2018-01-30 21:57:10 +00:00 · 0916cca7d2
parent 16f8bbe68b
commit 0916cca7d2
4 changed files with 223 additions and 191 deletions
--- a/pkg/analysis_server/lib/src/services/correction/assist_internal.dart
+++ b/pkg/analysis_server/lib/src/services/correction/assist_internal.dart
@ -683,26 +683,14 @@ class AssistProcessor {
      _coverageMarker();
      return;
    }
-    TypeAnnotation returnType;
    AstNode parent = body.parent;
    if (parent is ConstructorDeclaration) {
      return;
    }
-    if (parent is FunctionExpression) {
-      AstNode grandParent = parent.parent;
-      if (grandParent is FunctionDeclaration) {
-        returnType = grandParent.returnType;
-      }
-    } else if (parent is MethodDeclaration) {
-      returnType = parent.returnType;
-    }

    DartChangeBuilder changeBuilder = new DartChangeBuilder(session);
    await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
-      if (returnType != null) {
-        builder.replaceTypeWithFuture(returnType, typeProvider);
-      }
-      builder.addSimpleInsertion(body.offset, 'async ');
+      builder.convertFunctionFromSyncToAsync(body, typeProvider);
    });
    _addAssistFromBuilder(
        changeBuilder, DartAssistKind.CONVERT_INTO_ASYNC_BODY);
--- a/pkg/analysis_server/test/services/correction/fix_test.dart
+++ b/pkg/analysis_server/test/services/correction/fix_test.dart
@ -211,6 +211,190 @@ bool test() {

@reflectiveTest
 class FixProcessorTest extends BaseFixProcessorTest {
+  test_addAsync_asyncFor() async {
+    await resolveTestUnit('''
+import 'dart:async';
+void main(Stream<String> names) {
+  await for (String name in names) {
+    print(name);
+  }
+}
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+import 'dart:async';
+Future main(Stream<String> names) async {
+  await for (String name in names) {
+    print(name);
+  }
+}
+''');
+  }
+
+  test_addAsync_BAD_nullFunctionBody() async {
+    await resolveTestUnit('''
+var F = await;
+''');
+    await assertNoFix(DartFixKind.ADD_ASYNC);
+  }
+
+  test_addAsync_blockFunctionBody() async {
+    await resolveTestUnit('''
+foo() {}
+main() {
+  await foo();
+}
+''');
+    List<AnalysisError> errors = await _computeErrors();
+    expect(errors, hasLength(2));
+    errors.sort((a, b) => a.message.compareTo(b.message));
+    // No fix for ";".
+    {
+      AnalysisError error = errors[0];
+      expect(error.message, "Expected to find ';'.");
+      List<Fix> fixes = await _computeFixes(error);
+      expect(fixes, isEmpty);
+    }
+    // Has fix for "await".
+    {
+      AnalysisError error = errors[1];
+      expect(error.message, startsWith("Undefined name 'await' in function"));
+      List<Fix> fixes = await _computeFixes(error);
+      // has exactly one fix
+      expect(fixes, hasLength(1));
+      Fix fix = fixes[0];
+      expect(fix.kind, DartFixKind.ADD_ASYNC);
+      // apply to "file"
+      List<SourceFileEdit> fileEdits = fix.change.edits;
+      expect(fileEdits, hasLength(1));
+      resultCode = SourceEdit.applySequence(testCode, fileEdits[0].edits);
+      // verify
+      expect(resultCode, '''
+foo() {}
+main() async {
+  await foo();
+}
+''');
+    }
+  }
+
+  test_addAsync_closure() async {
+    errorFilter = (AnalysisError error) {
+      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
+    };
+    await resolveTestUnit('''
+import 'dart:async';
+
+void takeFutureCallback(Future callback()) {}
+
+void doStuff() => takeFutureCallback(() => await 1);
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+import 'dart:async';
+
+void takeFutureCallback(Future callback()) {}
+
+void doStuff() => takeFutureCallback(() async => await 1);
+''');
+  }
+
+  test_addAsync_expressionFunctionBody() async {
+    errorFilter = (AnalysisError error) {
+      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
+    };
+    await resolveTestUnit('''
+foo() {}
+main() => await foo();
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+foo() {}
+main() async => await foo();
+''');
+  }
+
+  test_addAsync_returnFuture() async {
+    errorFilter = (AnalysisError error) {
+      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
+    };
+    await resolveTestUnit('''
+foo() {}
+int main() {
+  await foo();
+  return 42;
+}
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+import 'dart:async';
+
+foo() {}
+Future<int> main() async {
+  await foo();
+  return 42;
+}
+''');
+  }
+
+  test_addAsync_returnFuture_alreadyFuture() async {
+    errorFilter = (AnalysisError error) {
+      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
+    };
+    await resolveTestUnit('''
+import 'dart:async';
+foo() {}
+Future<int> main() {
+  await foo();
+  return 42;
+}
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+import 'dart:async';
+foo() {}
+Future<int> main() async {
+  await foo();
+  return 42;
+}
+''');
+  }
+
+  test_addAsync_returnFuture_dynamic() async {
+    errorFilter = (AnalysisError error) {
+      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
+    };
+    await resolveTestUnit('''
+foo() {}
+dynamic main() {
+  await foo();
+  return 42;
+}
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+foo() {}
+dynamic main() async {
+  await foo();
+  return 42;
+}
+''');
+  }
+
+  test_addAsync_returnFuture_noType() async {
+    errorFilter = (AnalysisError error) {
+      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
+    };
+    await resolveTestUnit('''
+foo() {}
+main() {
+  await foo();
+  return 42;
+}
+''');
+    await assertHasFix(DartFixKind.ADD_ASYNC, '''
+foo() {}
+main() async {
+  await foo();
+  return 42;
+}
+''');
+  }
+
  test_addFieldFormalParameters_hasRequiredParameter() async {
    await resolveTestUnit('''
 class Test {
@ -767,170 +951,6 @@ main() {
 ''');
  }

-  test_addSync_asyncFor() async {
-    await resolveTestUnit('''
-import 'dart:async';
-void main(Stream<String> names) {
-  await for (String name in names) {
-    print(name);
-  }
-}
-''');
-    await assertHasFix(DartFixKind.ADD_ASYNC, '''
-import 'dart:async';
-Future main(Stream<String> names) async {
-  await for (String name in names) {
-    print(name);
-  }
-}
-''');
-  }
-
-  test_addSync_BAD_nullFunctionBody() async {
-    await resolveTestUnit('''
-var F = await;
-''');
-    await assertNoFix(DartFixKind.ADD_ASYNC);
-  }
-
-  test_addSync_blockFunctionBody() async {
-    await resolveTestUnit('''
-foo() {}
-main() {
-  await foo();
-}
-''');
-    List<AnalysisError> errors = await _computeErrors();
-    expect(errors, hasLength(2));
-    errors.sort((a, b) => a.message.compareTo(b.message));
-    // No fix for ";".
-    {
-      AnalysisError error = errors[0];
-      expect(error.message, "Expected to find ';'.");
-      List<Fix> fixes = await _computeFixes(error);
-      expect(fixes, isEmpty);
-    }
-    // Has fix for "await".
-    {
-      AnalysisError error = errors[1];
-      expect(error.message, startsWith("Undefined name 'await' in function"));
-      List<Fix> fixes = await _computeFixes(error);
-      // has exactly one fix
-      expect(fixes, hasLength(1));
-      Fix fix = fixes[0];
-      expect(fix.kind, DartFixKind.ADD_ASYNC);
-      // apply to "file"
-      List<SourceFileEdit> fileEdits = fix.change.edits;
-      expect(fileEdits, hasLength(1));
-      resultCode = SourceEdit.applySequence(testCode, fileEdits[0].edits);
-      // verify
-      expect(resultCode, '''
-foo() {}
-main() async {
-  await foo();
-}
-''');
-    }
-  }
-
-  test_addSync_expressionFunctionBody() async {
-    errorFilter = (AnalysisError error) {
-      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
-    };
-    await resolveTestUnit('''
-foo() {}
-main() => await foo();
-''');
-    await assertHasFix(DartFixKind.ADD_ASYNC, '''
-foo() {}
-main() async => await foo();
-''');
-  }
-
-  test_addSync_returnFuture() async {
-    errorFilter = (AnalysisError error) {
-      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
-    };
-    await resolveTestUnit('''
-foo() {}
-int main() {
-  await foo();
-  return 42;
-}
-''');
-    await assertHasFix(DartFixKind.ADD_ASYNC, '''
-import 'dart:async';
-
-foo() {}
-Future<int> main() async {
-  await foo();
-  return 42;
-}
-''');
-  }
-
-  test_addSync_returnFuture_alreadyFuture() async {
-    errorFilter = (AnalysisError error) {
-      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
-    };
-    await resolveTestUnit('''
-import 'dart:async';
-foo() {}
-Future<int> main() {
-  await foo();
-  return 42;
-}
-''');
-    await assertHasFix(DartFixKind.ADD_ASYNC, '''
-import 'dart:async';
-foo() {}
-Future<int> main() async {
-  await foo();
-  return 42;
-}
-''');
-  }
-
-  test_addSync_returnFuture_dynamic() async {
-    errorFilter = (AnalysisError error) {
-      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
-    };
-    await resolveTestUnit('''
-foo() {}
-dynamic main() {
-  await foo();
-  return 42;
-}
-''');
-    await assertHasFix(DartFixKind.ADD_ASYNC, '''
-foo() {}
-dynamic main() async {
-  await foo();
-  return 42;
-}
-''');
-  }
-
-  test_addSync_returnFuture_noType() async {
-    errorFilter = (AnalysisError error) {
-      return error.errorCode == StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT;
-    };
-    await resolveTestUnit('''
-foo() {}
-main() {
-  await foo();
-  return 42;
-}
-''');
-    await assertHasFix(DartFixKind.ADD_ASYNC, '''
-foo() {}
-main() async {
-  await foo();
-  return 42;
-}
-''');
-  }
-
  test_boolean() async {
    await resolveTestUnit('''
 main() {
@ -6426,6 +6446,19 @@ class C {
 ''');
  }

+  test_replaceFinalWithConst_method() async {
+    String src = '''
+/*LINT*/final int a = 1;
+''';
+    await findLint(src, LintNames.prefer_const_declarations);
+
+    await applyFix(DartFixKind.REPLACE_FINAL_WITH_CONST);
+
+    verifyResult('''
+const int a = 1;
+''');
+  }
+
  test_replaceWithConditionalAssignment_withCodeBeforeAndAfter() async {
    String src = '''
 class Person {
@ -6741,19 +6774,6 @@ Function finalVar() {
 ''');
  }

-  test_replaceFinalWithConst_method() async {
-    String src = '''
-/*LINT*/final int a = 1;
-''';
-    await findLint(src, LintNames.prefer_const_declarations);
-
-    await applyFix(DartFixKind.REPLACE_FINAL_WITH_CONST);
-
-    verifyResult('''
-const int a = 1;
-''');
-  }
-
  void verifyResult(String expectedResult) {
    expect(resultCode, expectedResult);
  }
--- a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart
+++ b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart
@ -1348,6 +1348,10 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl
      if (node is FunctionDeclaration) {
        replaceTypeWithFuture(node.returnType, typeProvider);
        return;
+      } else if (node is FunctionExpression &&
+          node.parent is! FunctionDeclaration) {
+        // Closures don't have a return type.
+        return;
      } else if (node is MethodDeclaration) {
        replaceTypeWithFuture(node.returnType, typeProvider);
        return;
--- a/pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_dart_test.dart
+++ b/pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_dart_test.dart
@ -1350,7 +1350,27 @@ class DartFileEditBuilderImplTest extends AbstractContextTest
    return new TestTypeProvider(context);
  }

-  test_convertFunctionFromSyncToAsync() async {
+  test_convertFunctionFromSyncToAsync_closure() async {
+    String path = provider.convertPath('/test.dart');
+    addSource(path, '''var f = () {}''');
+
+    CompilationUnit unit = (await driver.getResult(path))?.unit;
+    TopLevelVariableDeclaration variable = unit.declarations[0];
+    FunctionBody body =
+        (variable.variables.variables[0].initializer as FunctionExpression)
+            .body;
+
+    DartChangeBuilderImpl builder = new DartChangeBuilder(session);
+    await builder.addFileEdit(path, (FileEditBuilder builder) {
+      (builder as DartFileEditBuilder)
+          .convertFunctionFromSyncToAsync(body, typeProvider);
+    });
+    List<SourceEdit> edits = getEdits(builder);
+    expect(edits, hasLength(1));
+    expect(edits[0].replacement, equalsIgnoringWhitespace('async'));
+  }
+
+  test_convertFunctionFromSyncToAsync_topLevelFunction() async {
    String path = provider.convertPath('/test.dart');
    addSource(path, 'String f() {}');