Skip to content

Commit 5412f2c

Browse files
FMorschelCommit Queue
FMorschel
authored and
Commit Queue
committed
[DAS] Fixes wrong return type - add async
[email protected] Fixes #50460 Change-Id: I0761364ec0292696ddcdba81213ab4d65a2a5e14 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/394361 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Auto-Submit: Felipe Morschel <[email protected]>
1 parent cc6ef3a commit 5412f2c

File tree

4 files changed

+389
-85
lines changed

4 files changed

+389
-85
lines changed

‎pkg/analysis_server/lib/src/services/correction/dart/add_async.dart‎

Lines changed: 181 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,23 @@ import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
77
import 'package:analyzer/dart/ast/ast.dart';
88
import 'package:analyzer/dart/ast/visitor.dart';
99
import 'package:analyzer/dart/element/type.dart';
10+
import 'package:analyzer/dart/element/type_system.dart';
1011
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
1112
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
1213

1314
class AddAsync extends ResolvedCorrectionProducer {
1415
// TODO(pq): consider adding a variation that adds an `await` as well.
1516

16-
/// A flag indicating whether this producer is producing a fix in the case
17-
/// where a function is missing a return at the end.
18-
final bool isForMissingReturn;
17+
final _Type _type;
1918

2019
/// Initialize a newly created producer.
21-
AddAsync({required super.context}) : isForMissingReturn = false;
20+
AddAsync({required super.context}) : _type = _Type.others;
21+
22+
AddAsync.missingReturn({required super.context})
23+
: _type = _Type.missingReturn;
2224

23-
AddAsync.missingReturn({required super.context}) : isForMissingReturn = true;
25+
AddAsync.wrongReturnType({required super.context})
26+
: _type = _Type.wrongReturnType;
2427

2528
@override
2629
CorrectionApplicability get applicability =>
@@ -33,43 +36,100 @@ class AddAsync extends ResolvedCorrectionProducer {
3336

3437
@override
3538
Future<void> compute(ChangeBuilder builder) async {
36-
if (isForMissingReturn) {
37-
var node = this.node;
38-
FunctionBody? body;
39-
DartType? returnType;
40-
switch (node) {
41-
case FunctionDeclaration():
42-
body = node.functionExpression.body;
43-
if (node.declaredFragment?.element case var declaredElement?) {
44-
returnType = declaredElement.returnType;
45-
} else if (node.declaredFragment case var declaredFragment?) {
46-
returnType = declaredFragment.element.returnType;
47-
}
48-
case MethodDeclaration():
49-
body = node.body;
50-
returnType = node.declaredFragment!.element.returnType;
51-
}
52-
if (body == null || returnType == null) {
53-
return;
54-
}
55-
if (_isFutureVoid(returnType) && _hasNoReturns(body)) {
56-
var final_body = body;
57-
await builder.addDartFileEdit(file, (builder) {
58-
builder.addSimpleInsertion(final_body.offset, 'async ');
59-
});
60-
}
61-
} else {
62-
var body = node.thisOrAncestorOfType<FunctionBody>();
63-
if (body != null && body.keyword == null) {
64-
await builder.addDartFileEdit(file, (builder) {
65-
builder.convertFunctionFromSyncToAsync(
66-
body: body,
67-
typeSystem: typeSystem,
68-
typeProvider: typeProvider,
69-
);
70-
});
71-
}
39+
switch (_type) {
40+
case _Type.missingReturn:
41+
var node = this.node;
42+
FunctionBody? body;
43+
DartType? returnType;
44+
switch (node) {
45+
case FunctionDeclaration():
46+
body = node.functionExpression.body;
47+
if (node.declaredFragment?.element case var declaredElement?) {
48+
returnType = declaredElement.returnType;
49+
} else if (node.declaredFragment case var declaredFragment?) {
50+
returnType = declaredFragment.element.returnType;
51+
}
52+
case MethodDeclaration():
53+
body = node.body;
54+
returnType = node.declaredFragment!.element.returnType;
55+
}
56+
if (body == null || returnType == null) {
57+
return;
58+
}
59+
if (_isFutureVoid(returnType) && _hasNoReturns(body)) {
60+
var final_body = body;
61+
await builder.addDartFileEdit(file, (builder) {
62+
builder.addSimpleInsertion(final_body.offset, 'async ');
63+
});
64+
}
65+
case _Type.wrongReturnType:
66+
var body = node.thisOrAncestorOfType<FunctionBody>();
67+
if (body == null) {
68+
return;
69+
}
70+
if (body.keyword != null) {
71+
return;
72+
}
73+
var expectedReturnType = _getStaticFunctionType(body);
74+
if (expectedReturnType == null) {
75+
return;
76+
}
77+
78+
if (expectedReturnType is! InterfaceType ||
79+
!expectedReturnType.isDartAsyncFuture) {
80+
return;
81+
}
82+
83+
var visitor = _ReturnTypeTester(
84+
typeSystem,
85+
expectedReturnType.typeArguments.first,
86+
);
87+
if (visitor.returnsAreAssignable(body)) {
88+
await builder.addDartFileEdit(file, (builder) {
89+
builder.convertFunctionFromSyncToAsync(
90+
body: body,
91+
typeSystem: typeSystem,
92+
typeProvider: typeProvider,
93+
);
94+
});
95+
}
96+
case _Type.others:
97+
var body = node.thisOrAncestorOfType<FunctionBody>();
98+
if (body != null && body.keyword == null) {
99+
await builder.addDartFileEdit(file, (builder) {
100+
builder.convertFunctionFromSyncToAsync(
101+
body: body,
102+
typeSystem: typeSystem,
103+
typeProvider: typeProvider,
104+
);
105+
});
106+
}
107+
}
108+
}
109+
110+
DartType? _getStaticFunctionType(FunctionBody body) {
111+
// Gets return type for expression functions.
112+
if (body case ExpressionFunctionBody(:FunctionExpression parent)) {
113+
return parent.declaredFragment?.element.returnType;
114+
}
115+
116+
if (body is! BlockFunctionBody) {
117+
return null;
118+
}
119+
120+
// Gets return type for methods.
121+
if (node.thisOrAncestorOfType<MethodDeclaration>() case var method?
122+
when method.body == body) {
123+
return method.declaredFragment?.element.returnType;
124+
}
125+
126+
// Gets return type for functions.
127+
if (node.thisOrAncestorOfType<FunctionDeclaration>() case var function?
128+
when function.functionExpression.body == body) {
129+
return function.declaredFragment?.element.returnType;
72130
}
131+
132+
return null;
73133
}
74134

75135
/// Return `true` if there are no return statements in the given function
@@ -107,3 +167,83 @@ class _ReturnFinder extends RecursiveAstVisitor<void> {
107167
foundReturn = true;
108168
}
109169
}
170+
171+
/// An AST visitor used to test if all return statements in a function body
172+
/// are assignable to a given type.
173+
class _ReturnTypeTester extends RecursiveAstVisitor<void> {
174+
/// A flag indicating whether a return statement was visited.
175+
bool _foundOneReturn = false;
176+
177+
/// A flag indicating whether the return type is assignable considering
178+
/// [isAssignable].
179+
bool _returnsAreAssignable = true;
180+
181+
/// The type system used to check assignability.
182+
final TypeSystem typeSystem;
183+
184+
/// The type that the return statements should be assignable to.
185+
final DartType futureOf;
186+
187+
/// Initialize a newly created visitor.
188+
_ReturnTypeTester(this.typeSystem, this.futureOf);
189+
190+
/// Tests whether a type is assignable to the [futureOf] type.
191+
bool isAssignable(DartType type) {
192+
if (typeSystem.isAssignableTo(type, futureOf)) {
193+
return true;
194+
}
195+
if (type is InterfaceType && type.isDartAsyncFuture) {
196+
return typeSystem.isAssignableTo(type.typeArguments.first, futureOf);
197+
}
198+
return false;
199+
}
200+
201+
/// Returns `true` if all return statements in the given [node] are
202+
/// assignable to the [futureOf] type.
203+
bool returnsAreAssignable(AstNode node) {
204+
_returnsAreAssignable = true;
205+
_foundOneReturn = false;
206+
node.accept(this);
207+
return _returnsAreAssignable && _foundOneReturn;
208+
}
209+
210+
@override
211+
void visitExpressionFunctionBody(ExpressionFunctionBody node) {
212+
_foundOneReturn = true;
213+
if (node.expression.staticType case var type?) {
214+
_returnsAreAssignable &= isAssignable(type);
215+
} else {
216+
_returnsAreAssignable = false;
217+
}
218+
}
219+
220+
@override
221+
void visitFunctionExpression(FunctionExpression node) {
222+
// Return statements within closures aren't counted.
223+
}
224+
225+
@override
226+
void visitReturnStatement(ReturnStatement node) {
227+
_foundOneReturn = true;
228+
if (node.expression?.staticType case var type?) {
229+
_returnsAreAssignable &= isAssignable(type);
230+
} else {
231+
_returnsAreAssignable = false;
232+
}
233+
}
234+
}
235+
236+
enum _Type {
237+
/// Indicates whether the producer is producing a fix in the case
238+
/// where a function has a return statement with the wrong return type.
239+
wrongReturnType,
240+
241+
/// Indicates whether the producer is producing a fix in the case
242+
/// where a function is missing a return at the end.
243+
missingReturn,
244+
245+
/// Indicates whether the producer is producing a fix that adds `async`
246+
/// to a function that is missing it. In cases where the error/lint is
247+
/// good enough to suggest adding `async` to a function, this is valid.
248+
others,
249+
}

‎pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1427,7 +1427,7 @@ CompileTimeErrorCode.RETURN_IN_GENERATOR:
14271427
notes: |-
14281428
Replace with a yield.
14291429
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE:
1430-
status: noFix
1430+
status: hasFix
14311431
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CONSTRUCTOR:
14321432
status: noFix
14331433
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION:

‎pkg/analysis_server/lib/src/services/correction/fix_internal.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,11 +947,16 @@ final _builtInNonLintProducers = <ErrorCode, List<ProducerGenerator>>{
947947
CompileTimeErrorCode.RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA: [
948948
AddTrailingComma.new,
949949
],
950+
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE: [
951+
AddAsync.wrongReturnType,
952+
],
950953
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION: [
954+
AddAsync.wrongReturnType,
951955
MakeReturnTypeNullable.new,
952956
ReplaceReturnType.new,
953957
],
954958
CompileTimeErrorCode.RETURN_OF_INVALID_TYPE_FROM_METHOD: [
959+
AddAsync.wrongReturnType,
955960
MakeReturnTypeNullable.new,
956961
ReplaceReturnType.new,
957962
],

0 commit comments

Comments
 (0)