Skip to content

Commit 89782c9

Browse files
scheglovcommit-bot@chromium.org
authored andcommitted
Implement changes to return type verification.
#41803 https://dart-review.googlesource.com/c/sdk/+/149163 dart-lang/language#989 Change-Id: I13caf53a3e96c7b266dd3b3bf511002addcf83b6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149373 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent ad53668 commit 89782c9

File tree

9 files changed

+284
-30
lines changed

9 files changed

+284
-30
lines changed

pkg/analysis_server/test/src/services/correction/fix/replace_return_type_future_test.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,6 @@ int main() async {
142142
library main;
143143
Future<int> main() async {
144144
}
145-
''', errorFilter: (error) {
146-
return error.errorCode == StaticTypeWarningCode.ILLEGAL_ASYNC_RETURN_TYPE;
147-
});
145+
''');
148146
}
149147
}

pkg/analyzer/lib/src/dart/resolver/body_inference_context.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,10 @@ class BodyInferenceContext {
188188
}
189189
}
190190

191-
// Otherwise the context type is `FutureOr<flatten(T)>` where `T` is the
192-
// imposed return type.
191+
// Otherwise the context type is `FutureOr<futureValueTypeSchema(S)>`,
192+
// where `S` is the imposed return type.
193193
return typeSystem.typeProvider.futureOrType2(
194-
typeSystem.flatten(imposedType),
194+
typeSystem.futureValueType(imposedType),
195195
);
196196
}
197197
}

pkg/analyzer/lib/src/error/return_type_verifier.dart

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ class ReturnTypeVerifier {
126126
return;
127127
}
128128

129+
if (_typeSystem.isNonNullableByDefault) {
130+
_checkReturnExpression_nullSafety(expression);
131+
} else {
132+
_checkReturnExpression_legacy(expression);
133+
}
134+
}
135+
136+
void _checkReturnExpression_legacy(Expression expression) {
129137
// `T` is the declared return type.
130138
// `S` is the static type of the expression.
131139
var T = enclosingExecutable.returnType;
@@ -188,6 +196,11 @@ class ReturnTypeVerifier {
188196
var flatten_S = _typeSystem.flatten(S);
189197
// It is a compile-time error if `T` is `void`,
190198
// and `flatten(S)` is neither `void`, `dynamic`, nor `Null`.
199+
//
200+
// Note, the specification was not implemented correctly, and
201+
// implementing it now would be a breaking change. So, the code below
202+
// intentionally does not implement the specification.
203+
// https://github.com/dart-lang/sdk/issues/41803#issuecomment-635852474
191204
if (T.isVoid) {
192205
if (!_isVoidDynamicOrNull(flatten_S)) {
193206
reportTypeError();
@@ -216,12 +229,117 @@ class ReturnTypeVerifier {
216229
}
217230
}
218231

219-
void _checkReturnWithoutValue(ReturnStatement statement) {
220-
var returnType = _flattenedReturnType;
221-
if (_isVoidDynamicOrNull(returnType)) {
232+
void _checkReturnExpression_nullSafety(Expression expression) {
233+
// `T` is the declared return type.
234+
// `S` is the static type of the expression.
235+
var T = enclosingExecutable.returnType;
236+
var S = getStaticType(expression);
237+
238+
void reportTypeError() {
239+
String displayName = enclosingExecutable.element.displayName;
240+
if (displayName.isEmpty) {
241+
_errorReporter.reportErrorForNode(
242+
StaticTypeWarningCode.RETURN_OF_INVALID_TYPE_FROM_CLOSURE,
243+
expression,
244+
[S, T],
245+
);
246+
} else if (enclosingExecutable.isMethod) {
247+
_errorReporter.reportErrorForNode(
248+
StaticTypeWarningCode.RETURN_OF_INVALID_TYPE_FROM_METHOD,
249+
expression,
250+
[S, T, displayName],
251+
);
252+
} else {
253+
_errorReporter.reportErrorForNode(
254+
StaticTypeWarningCode.RETURN_OF_INVALID_TYPE_FROM_FUNCTION,
255+
expression,
256+
[S, T, displayName],
257+
);
258+
}
259+
}
260+
261+
if (enclosingExecutable.isSynchronous) {
262+
// It is a compile-time error if `T` is `void`,
263+
// and `S` is neither `void`, `dynamic`, nor `Null`.
264+
if (T.isVoid) {
265+
if (!_isVoidDynamicOrNull(S)) {
266+
reportTypeError();
267+
return;
268+
}
269+
}
270+
// It is a compile-time error if `S` is `void`,
271+
// and `T` is neither `void` nor `dynamic`.
272+
if (S.isVoid) {
273+
if (!_isVoidDynamic(T)) {
274+
reportTypeError();
275+
return;
276+
}
277+
}
278+
// It is a compile-time error if `S` is not `void`,
279+
// and `S` is not assignable to `T`.
280+
if (!S.isVoid) {
281+
if (!_typeSystem.isAssignableTo2(S, T)) {
282+
reportTypeError();
283+
return;
284+
}
285+
}
286+
// OK
222287
return;
223288
}
224289

290+
if (enclosingExecutable.isAsynchronous) {
291+
var T_v = _typeSystem.futureValueType(T);
292+
var flatten_S = _typeSystem.flatten(S);
293+
// It is a compile-time error if `flatten(T)` is `void`,
294+
// and `flatten(S)` is neither `void`, `dynamic`, nor `Null`.
295+
if (T_v.isVoid) {
296+
if (!_isVoidDynamicOrNull(flatten_S)) {
297+
reportTypeError();
298+
return;
299+
}
300+
}
301+
// It is a compile-time error if `flatten(S)` is `void`,
302+
// and `flatten(T)` is neither `void`, `dynamic`.
303+
if (flatten_S.isVoid) {
304+
if (!_isVoidDynamic(T_v)) {
305+
reportTypeError();
306+
return;
307+
}
308+
}
309+
// It is a compile-time error if `flatten(S)` is not `void`,
310+
// and `Future<flatten(S)>` is not assignable to `T`.
311+
if (!flatten_S.isVoid) {
312+
if (!_typeSystem.isAssignableTo2(S, T_v) &&
313+
!_typeSystem.isSubtypeOf2(flatten_S, T_v)) {
314+
reportTypeError();
315+
return;
316+
}
317+
// OK
318+
return;
319+
}
320+
}
321+
}
322+
323+
void _checkReturnWithoutValue(ReturnStatement statement) {
324+
if (_typeSystem.isNonNullableByDefault) {
325+
var T = enclosingExecutable.returnType;
326+
if (enclosingExecutable.isSynchronous) {
327+
if (_isVoidDynamicOrNull(T)) {
328+
return;
329+
}
330+
} else {
331+
var T_v = _typeSystem.futureValueType(T);
332+
if (_isVoidDynamicOrNull(T_v)) {
333+
return;
334+
}
335+
}
336+
} else {
337+
var returnType = _flattenedReturnType;
338+
if (_isVoidDynamicOrNull(returnType)) {
339+
return;
340+
}
341+
}
342+
225343
_errorReporter.reportErrorForToken(
226344
StaticWarningCode.RETURN_WITHOUT_VALUE,
227345
statement.returnKeyword,
@@ -268,6 +386,10 @@ class ReturnTypeVerifier {
268386
return type;
269387
}
270388

389+
static bool _isVoidDynamic(DartType type) {
390+
return type.isVoid || type.isDynamic;
391+
}
392+
271393
static bool _isVoidDynamicOrNull(DartType type) {
272394
return type.isVoid || type.isDynamic || type.isDartCoreNull;
273395
}

pkg/analyzer/lib/src/generated/type_system.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,11 @@ class TypeSystemImpl extends TypeSystem {
655655
}
656656
}
657657

658+
// futureValueType(`dynamic`) = `dynamic`.
659+
if (identical(T, DynamicTypeImpl.instance)) {
660+
return T;
661+
}
662+
658663
// futureValueType(`void`) = `void`.
659664
if (identical(T, VoidTypeImpl.instance)) {
660665
return T;

pkg/analyzer/test/src/dart/element/future_value_type_test.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ main() {
1616

1717
@reflectiveTest
1818
class FutureValueTypeTest extends AbstractTypeSystemNullSafetyTest {
19+
/// futureValueType(`dynamic`) = `dynamic`.
20+
test_dynamic() {
21+
_check(dynamicNone, 'dynamic');
22+
}
23+
1924
/// futureValueType(Future<`S`>) = `S`, for all `S`.
2025
test_future() {
2126
void check(DartType S, String expected) {

pkg/analyzer/test/src/dart/resolution/type_inference/function_expression_test.dart

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,24 @@ T foo<T>() => throw 0;
5050
5151
Object Function() v = () async => foo();
5252
''');
53+
5354
assertTypeArgumentTypes(
5455
findNode.methodInvocation('foo();'),
55-
['FutureOr<Object>'],
56+
[
57+
typeStringByNullability(
58+
nullable: 'FutureOr<Object?>',
59+
legacy: 'FutureOr<Object>',
60+
),
61+
],
62+
);
63+
64+
_assertReturnType(
65+
'() async => foo',
66+
typeStringByNullability(
67+
nullable: 'Future<Object?>',
68+
legacy: 'Future<Object>',
69+
),
5670
);
57-
_assertReturnType('() async => foo', 'Future<Object>');
5871
}
5972

6073
test_contextFunctionType_returnType_asyncStar_blockBody() async {

pkg/analyzer/test/src/diagnostics/illegal_async_return_type_test.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ class IllegalAsyncReturnTypeTest extends DriverResolutionTest {
2020
int f() async {}
2121
''', [
2222
error(StaticTypeWarningCode.ILLEGAL_ASYNC_RETURN_TYPE, 0, 3),
23-
error(HintCode.MISSING_RETURN, 4, 1),
2423
]);
2524
}
2625

@@ -52,7 +51,6 @@ class C {
5251
int m() async {}
5352
}
5453
''', [
55-
error(HintCode.MISSING_RETURN, 16, 1),
5654
error(StaticTypeWarningCode.ILLEGAL_ASYNC_RETURN_TYPE, 12, 3),
5755
]);
5856
}

0 commit comments

Comments
 (0)