diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_async.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_async.dart index 66d22f5d744e..cc164b5a4d8e 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_async.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_async.dart @@ -7,20 +7,23 @@ import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/dart/element/type_system.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; class AddAsync extends ResolvedCorrectionProducer { // TODO(pq): consider adding a variation that adds an `await` as well. - /// A flag indicating whether this producer is producing a fix in the case - /// where a function is missing a return at the end. - final bool isForMissingReturn; + final _Type _type; /// Initialize a newly created producer. - AddAsync({required super.context}) : isForMissingReturn = false; + AddAsync({required super.context}) : _type = _Type.others; + + AddAsync.missingReturn({required super.context}) + : _type = _Type.missingReturn; - AddAsync.missingReturn({required super.context}) : isForMissingReturn = true; + AddAsync.wrongReturnType({required super.context}) + : _type = _Type.wrongReturnType; @override CorrectionApplicability get applicability => @@ -33,43 +36,100 @@ class AddAsync extends ResolvedCorrectionProducer { @override Future compute(ChangeBuilder builder) async { - if (isForMissingReturn) { - var node = this.node; - FunctionBody? body; - DartType? returnType; - switch (node) { - case FunctionDeclaration(): - body = node.functionExpression.body; - if (node.declaredFragment?.element case var declaredElement?) { - returnType = declaredElement.returnType; - } else if (node.declaredFragment case var declaredFragment?) { - returnType = declaredFragment.element.returnType; - } - case MethodDeclaration(): - body = node.body; - returnType = node.declaredFragment!.element.returnType; - } - if (body == null || returnType == null) { - return; - } - if (_isFutureVoid(returnType) && _hasNoReturns(body)) { - var final_body = body; - await builder.addDartFileEdit(file, (builder) { - builder.addSimpleInsertion(final_body.offset, 'async '); - }); - } - } else { - var body = node.thisOrAncestorOfType(); - if (body != null && body.keyword == null) { - await builder.addDartFileEdit(file, (builder) { - builder.convertFunctionFromSyncToAsync( - body: body, - typeSystem: typeSystem, - typeProvider: typeProvider, - ); - }); - } + switch (_type) { + case _Type.missingReturn: + var node = this.node; + FunctionBody? body; + DartType? returnType; + switch (node) { + case FunctionDeclaration(): + body = node.functionExpression.body; + if (node.declaredFragment?.element case var declaredElement?) { + returnType = declaredElement.returnType; + } else if (node.declaredFragment case var declaredFragment?) { + returnType = declaredFragment.element.returnType; + } + case MethodDeclaration(): + body = node.body; + returnType = node.declaredFragment!.element.returnType; + } + if (body == null || returnType == null) { + return; + } + if (_isFutureVoid(returnType) && _hasNoReturns(body)) { + var final_body = body; + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleInsertion(final_body.offset, 'async '); + }); + } + case _Type.wrongReturnType: + var body = node.thisOrAncestorOfType(); + if (body == null) { + return; + } + if (body.keyword != null) { + return; + } + var expectedReturnType = _getStaticFunctionType(body); + if (expectedReturnType == null) { + return; + } + + if (expectedReturnType is! InterfaceType || + !expectedReturnType.isDartAsyncFuture) { + return; + } + + var visitor = _ReturnTypeTester( + typeSystem, + expectedReturnType.typeArguments.first, + ); + if (visitor.returnsAreAssignable(body)) { + await builder.addDartFileEdit(file, (builder) { + builder.convertFunctionFromSyncToAsync( + body: body, + typeSystem: typeSystem, + typeProvider: typeProvider, + ); + }); + } + case _Type.others: + var body = node.thisOrAncestorOfType(); + if (body != null && body.keyword == null) { + await builder.addDartFileEdit(file, (builder) { + builder.convertFunctionFromSyncToAsync( + body: body, + typeSystem: typeSystem, + typeProvider: typeProvider, + ); + }); + } + } + } + + DartType? _getStaticFunctionType(FunctionBody body) { + // Gets return type for expression functions. + if (body case ExpressionFunctionBody(:FunctionExpression parent)) { + return parent.declaredFragment?.element.returnType; + } + + if (body is! BlockFunctionBody) { + return null; + } + + // Gets return type for methods. + if (node.thisOrAncestorOfType() case var method? + when method.body == body) { + return method.declaredFragment?.element.returnType; + } + + // Gets return type for functions. + if (node.thisOrAncestorOfType() case var function? + when function.functionExpression.body == body) { + return function.declaredFragment?.element.returnType; } + + return null; } /// Return `true` if there are no return statements in the given function @@ -107,3 +167,83 @@ class _ReturnFinder extends RecursiveAstVisitor { foundReturn = true; } } + +/// An AST visitor used to test if all return statements in a function body +/// are assignable to a given type. +class _ReturnTypeTester extends RecursiveAstVisitor { + /// A flag indicating whether a return statement was visited. + bool _foundOneReturn = false; + + /// A flag indicating whether the return type is assignable considering + /// [isAssignable]. + bool _returnsAreAssignable = true; + + /// The type system used to check assignability. + final TypeSystem typeSystem; + + /// The type that the return statements should be assignable to. + final DartType futureOf; + + /// Initialize a newly created visitor. + _ReturnTypeTester(this.typeSystem, this.futureOf); + + /// Tests whether a type is assignable to the [futureOf] type. + bool isAssignable(DartType type) { + if (typeSystem.isAssignableTo(type, futureOf)) { + return true; + } + if (type is InterfaceType && type.isDartAsyncFuture) { + return typeSystem.isAssignableTo(type.typeArguments.first, futureOf); + } + return false; + } + + /// Returns `true` if all return statements in the given [node] are + /// assignable to the [futureOf] type. + bool returnsAreAssignable(AstNode node) { + _returnsAreAssignable = true; + _foundOneReturn = false; + node.accept(this); + return _returnsAreAssignable && _foundOneReturn; + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + _foundOneReturn = true; + if (node.expression.staticType case var type?) { + _returnsAreAssignable &= isAssignable(type); + } else { + _returnsAreAssignable = false; + } + } + + @override + void visitFunctionExpression(FunctionExpression node) { + // Return statements within closures aren't counted. + } + + @override + void visitReturnStatement(ReturnStatement node) { + _foundOneReturn = true; + if (node.expression?.staticType case var type?) { + _returnsAreAssignable &= isAssignable(type); + } else { + _returnsAreAssignable = false; + } + } +} + +enum _Type { + /// Indicates whether the producer is producing a fix in the case + /// where a function has a return statement with the wrong return type. + wrongReturnType, + + /// Indicates whether the producer is producing a fix in the case + /// where a function is missing a return at the end. + missingReturn, + + /// Indicates whether the producer is producing a fix that adds `async` + /// to a function that is missing it. In cases where the error/lint is + /// good enough to suggest adding `async` to a function, this is valid. + others, +} diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index 69f79d955813..071ad398a241 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml @@ -1427,7 +1427,7 @@ CompileTimeErrorCode.RETURN_IN_GENERATOR: notes: |- Replace with a yield. CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE: - status: noFix + status: hasFix CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CONSTRUCTOR: status: noFix CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION: diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 07e97880e59a..57b2493bbcf4 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -947,11 +947,16 @@ final _builtInNonLintProducers = >{ CompileTimeErrorCode.RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA: [ AddTrailingComma.new, ], + CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE: [ + AddAsync.wrongReturnType, + ], CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION: [ + AddAsync.wrongReturnType, MakeReturnTypeNullable.new, ReplaceReturnType.new, ], CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD: [ + AddAsync.wrongReturnType, MakeReturnTypeNullable.new, ReplaceReturnType.new, ], diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_async_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_async_test.dart index 2d661d0772ef..0839f63a3066 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/add_async_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/add_async_test.dart @@ -107,53 +107,228 @@ f() async => await foo(); '''); } - Future test_localFunction() async { + Future test_futureOrString() async { await resolveTestCode(''' -void f() async { - Future g() {} - await g(); +Future f() { + if (1 == 2) { + return ''; + } + return Future.value(''); } '''); await assertHasFix(''' -void f() async { - Future g() async {} - await g(); +Future f() async { + if (1 == 2) { + return ''; + } + return Future.value(''); +} +'''); + } + + Future test_futureString_closure() async { + await resolveTestCode(''' +void f(Future Function() p) async {} + +void g() => f(() => ''); +'''); + await assertHasFix(''' +void f(Future Function() p) async {} + +void g() => f(() async => ''); +'''); + } + + Future test_futureString_function() async { + await resolveTestCode(''' +Future f() { + return ''; +} +'''); + await assertHasFix(''' +Future f() async { + return ''; +} +'''); + } + + Future test_futureString_innerClosure_closure() async { + await resolveTestCode(''' +void f(Future Function() p) async {} + +void g() => f(() => () => ''); +'''); + await assertNoFix(); + } + + Future test_futureString_innerClosure_functionBody() async { + await resolveTestCode(''' +Future f() { + () => ''; + return 0; +} +'''); + await assertHasFix(''' +Future f() async { + () => ''; + return 0; } '''); } - Future test_missingReturn_hasReturn() async { + Future test_futureString_innerClosureCall_closure() async { await resolveTestCode(''' -Future f(bool b) { - if (b) { +void f(Future Function() p) async {} + +void g() => f(() => (() => '')()); +'''); + await assertHasFix(''' +void f(Future Function() p) async {} + +void g() => f(() async => (() => '')()); +'''); + } + + Future test_futureString_innerClosureCall_functionBody() async { + await resolveTestCode(''' +Future f() { + return (() => '')(); +} +'''); + await assertHasFix(''' +Future f() async { + return (() => '')(); +} +'''); + } + + Future test_futureString_innerFunction_closure() async { + await resolveTestCode(''' +void f(Future Function() p) async {} + +void g() => f(() => () { + return ''; +}); +'''); + await assertNoFix(); + } + + Future test_futureString_innerFunction_functionBody() async { + await resolveTestCode(''' +Future f() { + int g() { return 0; } + g(); + return ''; +} +'''); + await assertHasFix(''' +Future f() async { + int g() { + return 0; + } + g(); + return ''; } '''); - await assertNoFix( - errorFilter: (error) { - return error.errorCode == - CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION; - }, - ); } - Future test_missingReturn_method_hasReturn() async { + Future test_futureString_innerFunctionCall_closure() async { + await resolveTestCode(''' +void f(Future Function() p) async {} + +void g() => f(() => (() { + return ''; +})()); +'''); + await assertHasFix(''' +void f(Future Function() p) async {} + +void g() => f(() async => (() { + return ''; +})()); +'''); + } + + Future test_futureString_innerFunctionCall_functionBody() async { + await resolveTestCode(''' +Future f() { + String g() { + return ''; + } + return g(); +} +'''); + await assertHasFix(''' +Future f() async { + String g() { + return ''; + } + return g(); +} +'''); + } + + Future test_futureString_int_closure() async { + await resolveTestCode(''' +void f(Future Function() p) async {} + +void g() => f(() => 0); +'''); + await assertNoFix(); + } + + Future test_futureString_int_function() async { + await resolveTestCode(''' +Future f() { + return 0; +} +'''); + await assertNoFix(); + } + + Future test_futureString_int_method() async { await resolveTestCode(''' class C { - Future m(bool b) { - if (b) { - return 0; - } + Future f() { + return 0; + } +} +'''); + await assertNoFix(); + } + + Future test_futureString_method() async { + await resolveTestCode(''' +class C { + Future f() { + return ''; + } +} +'''); + await assertHasFix(''' +class C { + Future f() async { + return ''; } } '''); - await assertNoFix( - errorFilter: (error) { - return error.errorCode == - CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD; - }, - ); + } + + Future test_localFunction() async { + await resolveTestCode(''' +void f() async { + Future g() {} + await g(); +} +'''); + await assertHasFix(''' +void f() async { + Future g() async {} + await g(); +} +'''); } Future test_missingReturn_method_notVoid() async { @@ -233,22 +408,6 @@ Future f() { await assertNoFix(); } - Future test_missingReturn_topLevel_hasReturn() async { - await resolveTestCode(''' -Future f(bool b) { - if (b) { - return 0; - } -} -'''); - await assertNoFix( - errorFilter: (error) { - return error.errorCode == - CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION; - }, - ); - } - Future test_missingReturn_topLevel_notVoid() async { await resolveTestCode(''' Future f() {