Skip to content

Commit 058510e

Browse files
Jenny Messerlycommit-bot@chromium.org
Jenny Messerly
authored and
[email protected]
committed
fix #28233, add hint for missing returns to function expressions
If a function expression is used in a context that expects a return type other than dynamic/void/Null, issue a hint if that function has a block body and is missing a `return` statement. Change-Id: Ia55212abd84f5718343cf7401b87aba2891d6446 Reviewed-on: https://dart-review.googlesource.com/66340 Commit-Queue: Jenny Messerly <[email protected]> Reviewed-by: Leaf Petersen <[email protected]> Reviewed-by: Sigmund Cherem <[email protected]>
1 parent 4482d13 commit 058510e

File tree

12 files changed

+176
-4
lines changed

12 files changed

+176
-4
lines changed

‎pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,14 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
413413
}
414414
}
415415

416+
@override
417+
Object visitFunctionExpression(FunctionExpression node) {
418+
if (node.parent is! FunctionDeclaration) {
419+
_checkForMissingReturn(null, node.body, node.element, node);
420+
}
421+
return super.visitFunctionExpression(node);
422+
}
423+
416424
@override
417425
Object visitImportDirective(ImportDirective node) {
418426
_checkForDeprecatedMemberUse(node.uriElement, node);
@@ -1069,7 +1077,28 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
10691077
? returnType.flattenFutures(_typeSystem)
10701078
: returnType;
10711079

1072-
// dynamic/Null/void are allowed to omit a return.
1080+
// Function expressions without a return will have their return type set
1081+
// to `Null` regardless of their context type. So we need to figure out
1082+
// if a return type was expected from the original downwards context.
1083+
//
1084+
// This helps detect hint cases like `int Function() f = () {}`.
1085+
// See https://github.com/dart-lang/sdk/issues/28233 for context.
1086+
if (flattenedType.isDartCoreNull &&
1087+
functionNode is FunctionExpression) {
1088+
var contextType = InferenceContext.getContext(functionNode);
1089+
if (contextType is FunctionType) {
1090+
returnType = contextType.returnType;
1091+
flattenedType = body.isAsynchronous
1092+
? returnType.flattenFutures(_typeSystem)
1093+
: returnType;
1094+
}
1095+
}
1096+
1097+
// dynamic, Null, void, and FutureOr<T> where T is (dynamic, Null, void)
1098+
// are allowed to omit a return.
1099+
if (flattenedType.isDartAsyncFutureOr) {
1100+
flattenedType = (flattenedType as InterfaceType).typeArguments[0];
1101+
}
10731102
if (flattenedType.isDynamic ||
10741103
flattenedType.isDartCoreNull ||
10751104
flattenedType.isVoid) {

‎pkg/analyzer/test/generated/hint_code_kernel_test.dart

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,24 @@ class HintCodeTest_Kernel extends HintCodeTest_Driver {
143143
return super.test_missingJsLibAnnotation_variable();
144144
}
145145

146+
@failingTest
147+
@override
148+
test_missingReturn_functionExpression_futureOrInt() async {
149+
return super.test_missingReturn_functionExpression_futureOrInt();
150+
}
151+
152+
@failingTest
153+
@override
154+
test_missingReturn_functionExpression_inferred() async {
155+
return super.test_missingReturn_functionExpression_inferred();
156+
}
157+
158+
@failingTest
159+
@override
160+
test_missingReturn_functionExpressionAsync_inferred() async {
161+
return super.test_missingReturn_functionExpressionAsync_inferred();
162+
}
163+
146164
@failingTest
147165
@override
148166
test_mustCallSuper() async {

‎pkg/analyzer/test/generated/hint_code_test.dart

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2399,6 +2399,96 @@ class A {
23992399
verify([source]);
24002400
}
24012401

2402+
test_missingReturn_functionExpression_declared() async {
2403+
Source source = addSource(r'''
2404+
main() {
2405+
f() {} // no hint
2406+
}
2407+
''');
2408+
await computeAnalysisResult(source);
2409+
assertNoErrors(source);
2410+
}
2411+
2412+
test_missingReturn_functionExpression_expression() async {
2413+
Source source = addSource(r'''
2414+
main() {
2415+
int Function() f = () => null; // no hint
2416+
}
2417+
''');
2418+
await computeAnalysisResult(source);
2419+
assertNoErrors(source);
2420+
}
2421+
2422+
test_missingReturn_functionExpression_futureOrDynamic() async {
2423+
Source source = addSource(r'''
2424+
import 'dart:async';
2425+
main() {
2426+
FutureOr<dynamic> Function() f = () { print(42); };
2427+
}
2428+
''');
2429+
await computeAnalysisResult(source);
2430+
assertNoErrors(source);
2431+
verify([source]);
2432+
}
2433+
2434+
test_missingReturn_functionExpression_futureOrInt() async {
2435+
Source source = addSource(r'''
2436+
import 'dart:async';
2437+
main() {
2438+
FutureOr<int> Function() f = () { print(42); };
2439+
}
2440+
''');
2441+
await computeAnalysisResult(source);
2442+
assertErrors(source, [HintCode.MISSING_RETURN]);
2443+
verify([source]);
2444+
}
2445+
2446+
test_missingReturn_functionExpression_inferred() async {
2447+
Source source = addSource(r'''
2448+
main() {
2449+
int Function() f = () { print(42); };
2450+
}
2451+
''');
2452+
await computeAnalysisResult(source);
2453+
assertErrors(source, [HintCode.MISSING_RETURN]);
2454+
verify([source]);
2455+
}
2456+
2457+
test_missingReturn_functionExpression_inferred_dynamic() async {
2458+
Source source = addSource(r'''
2459+
main() {
2460+
Function() f = () { print(42); }; // no hint
2461+
}
2462+
''');
2463+
await computeAnalysisResult(source);
2464+
assertNoErrors(source);
2465+
verify([source]);
2466+
}
2467+
2468+
test_missingReturn_functionExpressionAsync_inferred() async {
2469+
Source source = addSource(r'''
2470+
import 'dart:async';
2471+
main() {
2472+
Future<int> Function() f = () async { print(42); };
2473+
}
2474+
''');
2475+
await computeAnalysisResult(source);
2476+
assertErrors(source, [HintCode.MISSING_RETURN]);
2477+
verify([source]);
2478+
}
2479+
2480+
test_missingReturn_functionExpressionAsync_inferred_dynamic() async {
2481+
Source source = addSource(r'''
2482+
import 'dart:async';
2483+
main() {
2484+
Future Function() f = () async { print(42); }; // no hint
2485+
}
2486+
''');
2487+
await computeAnalysisResult(source);
2488+
assertNoErrors(source);
2489+
verify([source]);
2490+
}
2491+
24022492
test_missingReturn_method() async {
24032493
Source source = addSource(r'''
24042494
class A {
@@ -2409,6 +2499,28 @@ class A {
24092499
verify([source]);
24102500
}
24112501

2502+
test_missingReturn_method_futureOrDynamic() async {
2503+
Source source = addSource(r'''
2504+
import 'dart:async';
2505+
class A {
2506+
FutureOr<dynamic> m() {}
2507+
}''');
2508+
await computeAnalysisResult(source);
2509+
assertNoErrors(source);
2510+
verify([source]);
2511+
}
2512+
2513+
test_missingReturn_method_futureOrInt() async {
2514+
Source source = addSource(r'''
2515+
import 'dart:async';
2516+
class A {
2517+
FutureOr<int> m() {}
2518+
}''');
2519+
await computeAnalysisResult(source);
2520+
assertErrors(source, [HintCode.MISSING_RETURN]);
2521+
verify([source]);
2522+
}
2523+
24122524
test_missingReturn_method_inferred() async {
24132525
Source source = addSource(r'''
24142526
abstract class A {

‎pkg/compiler/lib/src/js/rewrite_async.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1379,8 +1379,9 @@ abstract class AsyncRewriterBase extends js.NodeVisitor {
13791379
if (clause is js.Case) {
13801380
return new js.Case(
13811381
clause.expression, translateToBlock(clause.body));
1382-
} else if (clause is js.Default) {
1383-
return new js.Default(translateToBlock(clause.body));
1382+
} else {
1383+
return new js.Default(
1384+
translateToBlock((clause as js.Default).body));
13841385
}
13851386
}).toList();
13861387
addStatement(new js.Switch(key, cases));

‎pkg/compiler/lib/src/js_backend/interceptor_data.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ class InterceptorDataImpl implements InterceptorData {
188188
if (result == null) result = new Set<ClassEntity>();
189189
result.add(subclass);
190190
}
191+
return null;
191192
});
192193
}
193194
return result;

‎pkg/compiler/lib/src/js_backend/runtime_types.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,7 @@ class RuntimeTypesNeedBuilderImpl extends _RuntimeTypesBase
14731473
closedWorld.classHierarchy.forEachStrictSubtypeOf(cls,
14741474
(ClassEntity sub) {
14751475
potentiallyNeedTypeArguments(sub);
1476+
return null;
14761477
});
14771478
} else if (entity is FunctionEntity) {
14781479
methodsNeedingTypeArguments.add(entity);

‎pkg/compiler/lib/src/js_model/js_strategy.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,7 @@ class JsClosedWorldBuilder {
295295
.getClassHierarchyNode(closedWorld.commonElements.objectClass)
296296
.forEachSubclass((ClassEntity cls) {
297297
convertClassSet(closedWorld.classHierarchy.getClassSet(cls));
298+
return null;
298299
}, ClassHierarchyNode.ALL);
299300

300301
Set<MemberEntity> liveInstanceMembers =

‎pkg/compiler/lib/src/old_to_new_api.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class LegacyCompilerInput implements CompilerInput {
2323

2424
@override
2525
Future<Input> readFromUri(Uri uri, {InputKind inputKind: InputKind.UTF8}) {
26+
// The switch handles all enum values, but not null.
27+
// ignore: missing_return
2628
return _inputProvider(uri).then((/*String|List<int>*/ data) {
2729
switch (inputKind) {
2830
case InputKind.UTF8:

‎tests/compiler/dart2js/model/class_set_test.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ testForEach() async {
404404
List<ClassEntity> visited = <ClassEntity>[];
405405
classSet.forEachSubclass((cls) {
406406
visited.add(cls);
407+
return null;
407408
}, ClassHierarchyNode.ALL);
408409

409410
Expect.listEquals(
@@ -441,6 +442,7 @@ testForEach() async {
441442
List<ClassEntity> visited = <ClassEntity>[];
442443
classSet.forEachSubtype((cls) {
443444
visited.add(cls);
445+
return null;
444446
}, ClassHierarchyNode.ALL);
445447

446448
Expect.listEquals(

‎tests/compiler/dart2js/model/world_test.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ testClassSets() async {
9999
List<ClassEntity> visited = <ClassEntity>[];
100100
forEach(cls, (ClassEntity c) {
101101
visited.add(c);
102+
return null;
102103
});
103104
checkClasses('forEach($property)', cls, visited, expectedClasses,
104105
exact: exact);
@@ -424,6 +425,7 @@ testNativeClasses() async {
424425
if (allClasses.contains(other)) {
425426
strictSubclasses.add(other);
426427
}
428+
return null;
427429
});
428430
Expect.setEquals(subclasses, strictSubclasses,
429431
"Unexpected strict subclasses of $cls: ${strictSubclasses}.");
@@ -433,6 +435,7 @@ testNativeClasses() async {
433435
if (allClasses.contains(other)) {
434436
strictSubtypes.add(other);
435437
}
438+
return null;
436439
});
437440
Expect.setEquals(subtypes, strictSubtypes,
438441
"Unexpected strict subtypes of $cls: $strictSubtypes.");

0 commit comments

Comments
 (0)