Reland fix for #28233: add hint for missing returns to function expressions
The bulk of this change is actually correcting missing returns in the Dart SDK.
Bug: https://github.com/dart-lang/sdk/issues/28233
Change-Id: I52bcbc6c6f4a129d3fc22003f4448a7c3d4487ad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100301
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 0382710..91657b9 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -473,6 +473,14 @@
}
@override
+ void visitFunctionExpression(FunctionExpression node) {
+ if (node.parent is! FunctionDeclaration) {
+ _checkForMissingReturn(null, node.body, node.element, node);
+ }
+ super.visitFunctionExpression(node);
+ }
+
+ @override
void visitImportDirective(ImportDirective node) {
_checkForDeprecatedMemberUse(node.uriElement, node);
ImportElement importElement = node.element;
@@ -1034,7 +1042,27 @@
var flattenedType =
body.isAsynchronous ? _typeSystem.flatten(returnType) : returnType;
- // dynamic/Null/void are allowed to omit a return.
+ // Function expressions without a return will have their return type set
+ // to `Null` regardless of their context type. So we need to figure out
+ // if a return type was expected from the original downwards context.
+ //
+ // This helps detect hint cases like `int Function() f = () {}`.
+ // See https://github.com/dart-lang/sdk/issues/28233 for context.
+ if (flattenedType.isDartCoreNull && functionNode is FunctionExpression) {
+ var contextType = InferenceContext.getContext(functionNode);
+ if (contextType is FunctionType) {
+ returnType = contextType.returnType;
+ flattenedType = body.isAsynchronous
+ ? returnType.flattenFutures(_typeSystem)
+ : returnType;
+ }
+ }
+
+ // dynamic, Null, void, and FutureOr<T> where T is (dynamic, Null, void)
+ // are allowed to omit a return.
+ if (flattenedType.isDartAsyncFutureOr) {
+ flattenedType = (flattenedType as InterfaceType).typeArguments[0];
+ }
if (flattenedType.isDynamic ||
flattenedType.isDartCoreNull ||
flattenedType.isVoid) {
diff --git a/pkg/analyzer/test/src/diagnostics/missing_return_test.dart b/pkg/analyzer/test/src/diagnostics/missing_return_test.dart
index 41042fb..902c4f3 100644
--- a/pkg/analyzer/test/src/diagnostics/missing_return_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/missing_return_test.dart
@@ -79,11 +79,96 @@
''', [HintCode.MISSING_RETURN]);
}
+ test_functionExpression_declared() async {
+ await assertNoErrorsInCode(r'''
+main() {
+ f() {} // no hint
+}
+''');
+ }
+
+ test_functionExpression_expression() async {
+ await assertNoErrorsInCode(r'''
+main() {
+ int Function() f = () => null; // no hint
+}
+''');
+ }
+
+ test_functionExpression_futureOrDynamic() async {
+ await assertNoErrorsInCode(r'''
+import 'dart:async';
+main() {
+ FutureOr<dynamic> Function() f = () { print(42); };
+}
+''');
+ }
+
+ test_functionExpression_futureOrInt() async {
+ await assertErrorCodesInCode(r'''
+import 'dart:async';
+main() {
+ FutureOr<int> Function() f = () { print(42); };
+}
+''', [HintCode.MISSING_RETURN]);
+ }
+
+ test_functionExpression_inferred() async {
+ await assertErrorCodesInCode(r'''
+main() {
+ int Function() f = () { print(42); };
+}
+''', [HintCode.MISSING_RETURN]);
+ }
+
+ test_functionExpression_inferred_dynamic() async {
+ await assertNoErrorsInCode(r'''
+main() {
+ Function() f = () { print(42); }; // no hint
+}
+''');
+ }
+
+ test_functionExpressionAsync_inferred() async {
+ await assertErrorCodesInCode(r'''
+main() {
+ Future<int> Function() f = () async { print(42); };
+}
+''', [HintCode.MISSING_RETURN]);
+ }
+
+ test_functionExpressionAsync_inferred_dynamic() async {
+ await assertNoErrorsInCode(r'''
+main() {
+ Future Function() f = () async { print(42); }; // no hint
+}
+''');
+ }
+
test_method() async {
await assertErrorCodesInCode(r'''
class A {
int m() {}
-}''', [HintCode.MISSING_RETURN]);
+}
+''', [HintCode.MISSING_RETURN]);
+ }
+
+ test_method_futureOrDynamic() async {
+ await assertNoErrorsInCode(r'''
+import 'dart:async';
+class A {
+ FutureOr<dynamic> m() {}
+}
+''');
+ }
+
+ test_method_futureOrInt() async {
+ await assertErrorCodesInCode(r'''
+import 'dart:async';
+class A {
+ FutureOr<int> m() {}
+}
+''', [HintCode.MISSING_RETURN]);
}
test_method_inferred() async {
diff --git a/pkg/compiler/lib/src/js/rewrite_async.dart b/pkg/compiler/lib/src/js/rewrite_async.dart
index b3768c4..01cad9d 100644
--- a/pkg/compiler/lib/src/js/rewrite_async.dart
+++ b/pkg/compiler/lib/src/js/rewrite_async.dart
@@ -1385,8 +1385,9 @@
if (clause is js.Case) {
return new js.Case(
clause.expression, translateToBlock(clause.body));
- } else if (clause is js.Default) {
- return new js.Default(translateToBlock(clause.body));
+ } else {
+ return new js.Default(
+ translateToBlock((clause as js.Default).body));
}
}).toList();
addStatement(new js.Switch(key, cases));
diff --git a/pkg/compiler/lib/src/js_backend/interceptor_data.dart b/pkg/compiler/lib/src/js_backend/interceptor_data.dart
index cc38bab..ee38c00 100644
--- a/pkg/compiler/lib/src/js_backend/interceptor_data.dart
+++ b/pkg/compiler/lib/src/js_backend/interceptor_data.dart
@@ -12,6 +12,7 @@
import '../inferrer/abstract_value_domain.dart';
import '../js/js.dart' as jsAst;
import '../serialization/serialization.dart';
+import '../universe/class_set.dart';
import '../universe/selector.dart';
import '../world.dart' show JClosedWorld;
import 'namer.dart' show ModularNamer, suffixForGetInterceptor;
@@ -252,6 +253,7 @@
if (result == null) result = new Set<ClassEntity>();
result.add(subclass);
}
+ return IterationStep.CONTINUE;
});
}
return result;
diff --git a/pkg/compiler/lib/src/js_backend/runtime_types.dart b/pkg/compiler/lib/src/js_backend/runtime_types.dart
index 3dd3633..fa7a7d1 100644
--- a/pkg/compiler/lib/src/js_backend/runtime_types.dart
+++ b/pkg/compiler/lib/src/js_backend/runtime_types.dart
@@ -22,6 +22,7 @@
import '../options.dart';
import '../serialization/serialization.dart';
import '../universe/class_hierarchy.dart';
+import '../universe/class_set.dart';
import '../universe/codegen_world_builder.dart';
import '../universe/feature.dart';
import '../universe/selector.dart';
@@ -1525,6 +1526,7 @@
closedWorld.classHierarchy.forEachStrictSubtypeOf(cls,
(ClassEntity sub) {
potentiallyNeedTypeArguments(sub);
+ return IterationStep.CONTINUE;
});
} else if (entity is FunctionEntity) {
methodsNeedingTypeArguments.add(entity);
diff --git a/pkg/compiler/lib/src/js_model/js_world_builder.dart b/pkg/compiler/lib/src/js_model/js_world_builder.dart
index fa4c4d9..3785a41 100644
--- a/pkg/compiler/lib/src/js_model/js_world_builder.dart
+++ b/pkg/compiler/lib/src/js_model/js_world_builder.dart
@@ -108,6 +108,7 @@
.getClassHierarchyNode(closedWorld.commonElements.objectClass)
.forEachSubclass((ClassEntity cls) {
convertClassSet(closedWorld.classHierarchy.getClassSet(cls));
+ return IterationStep.CONTINUE;
}, ClassHierarchyNode.ALL);
Set<MemberEntity> liveInstanceMembers =
diff --git a/pkg/compiler/lib/src/old_to_new_api.dart b/pkg/compiler/lib/src/old_to_new_api.dart
index 808e242..a5d7c45 100644
--- a/pkg/compiler/lib/src/old_to_new_api.dart
+++ b/pkg/compiler/lib/src/old_to_new_api.dart
@@ -23,6 +23,8 @@
@override
Future<Input> readFromUri(Uri uri, {InputKind inputKind: InputKind.UTF8}) {
+ // The switch handles all enum values, but not null.
+ // ignore: missing_return
return _inputProvider(uri).then((/*String|List<int>*/ data) {
switch (inputKind) {
case InputKind.UTF8:
diff --git a/pkg/compiler/lib/src/universe/class_hierarchy.dart b/pkg/compiler/lib/src/universe/class_hierarchy.dart
index 776b4f1..49a899e 100644
--- a/pkg/compiler/lib/src/universe/class_hierarchy.dart
+++ b/pkg/compiler/lib/src/universe/class_hierarchy.dart
@@ -195,10 +195,12 @@
getClassHierarchyNode(_commonElements.objectClass);
node.forEachSubclass((ClassEntity cls) {
getClassHierarchyNode(cls).writeToDataSink(sink);
+ return null;
}, ClassHierarchyNode.ALL);
ClassSet set = getClassSet(_commonElements.objectClass);
set.forEachSubclass((ClassEntity cls) {
getClassSet(cls).writeToDataSink(sink);
+ return null;
}, ClassHierarchyNode.ALL);
sink.end(tag);
}
@@ -836,6 +838,7 @@
if (builder._isSubtypeOf(z, y)) {
classes.add(z);
}
+ return null;
}, ClassHierarchyNode.ALL, strict: strict);
}
diff --git a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
index 51d5070..b97c08b 100644
--- a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
@@ -1591,6 +1591,7 @@
} else {
reportInvalid(
node, 'No support for ${target.runtimeType} in a static-get.');
+ return null;
}
});
}
diff --git a/pkg/front_end/test/fasta/generator_to_string_test.dart b/pkg/front_end/test/fasta/generator_to_string_test.dart
index 6987266..7f59d4b 100644
--- a/pkg/front_end/test/fasta/generator_to_string_test.dart
+++ b/pkg/front_end/test/fasta/generator_to_string_test.dart
@@ -241,5 +241,6 @@
"offset: 4, prefixGenerator: , isInitializer: false, isSuper: false)",
new KernelUnexpectedQualifiedUseGenerator(
helper, token, generator, false));
+ return Future<void>.value();
});
}
diff --git a/pkg/front_end/test/fasta/unlinked_scope_test.dart b/pkg/front_end/test/fasta/unlinked_scope_test.dart
index e58294c..fb933f8 100644
--- a/pkg/front_end/test/fasta/unlinked_scope_test.dart
+++ b/pkg/front_end/test/fasta/unlinked_scope_test.dart
@@ -108,5 +108,6 @@
testExpression("unresolved");
testExpression("a + b", "a.+(b)");
testExpression("a = b");
+ return Future<void>.value();
});
}
diff --git a/pkg/front_end/test/src/base/processed_options_test.dart b/pkg/front_end/test/src/base/processed_options_test.dart
index 605b5ac..0abc4cb 100644
--- a/pkg/front_end/test/src/base/processed_options_test.dart
+++ b/pkg/front_end/test/src/base/processed_options_test.dart
@@ -22,6 +22,7 @@
defineReflectiveSuite(() {
defineReflectiveTests(ProcessedOptionsTest);
});
+ return Future<void>.value();
});
}
diff --git a/pkg/front_end/tool/_fasta/entry_points.dart b/pkg/front_end/tool/_fasta/entry_points.dart
index a3cd069..18b7445 100644
--- a/pkg/front_end/tool/_fasta/entry_points.dart
+++ b/pkg/front_end/tool/_fasta/entry_points.dart
@@ -192,6 +192,7 @@
// TODO(ahe): Extend this entry point so it can replace
// batchEntryPoint.
new IncrementalCompiler(c);
+ return Future<void>.value();
});
}
diff --git a/pkg/vm/test/frontend_server_test.dart b/pkg/vm/test/frontend_server_test.dart
index 5310517..a56d500 100644
--- a/pkg/vm/test/frontend_server_test.dart
+++ b/pkg/vm/test/frontend_server_test.dart
@@ -83,11 +83,12 @@
new StreamController<List<int>>();
final ReceivePort compileCalled = new ReceivePort();
when(compiler.compile(any, any, generator: anyNamed('generator')))
- .thenAnswer((Invocation invocation) {
+ .thenAnswer((Invocation invocation) async {
expect(invocation.positionalArguments[0], equals('server.dart'));
expect(
invocation.positionalArguments[1]['sdk-root'], equals('sdkroot'));
compileCalled.sendPort.send(true);
+ return true;
});
Future<int> result = starter(
@@ -116,11 +117,12 @@
new StreamController<List<int>>();
final ReceivePort compileCalled = new ReceivePort();
when(compiler.compile(any, any, generator: anyNamed('generator')))
- .thenAnswer((Invocation invocation) {
+ .thenAnswer((Invocation invocation) async {
expect(invocation.positionalArguments[0], equals('server.dart'));
expect(
invocation.positionalArguments[1]['sdk-root'], equals('sdkroot'));
compileCalled.sendPort.send(true);
+ return true;
});
Future<int> result = starter(
@@ -141,12 +143,13 @@
final ReceivePort compileCalled = new ReceivePort();
int counter = 1;
when(compiler.compile(any, any, generator: anyNamed('generator')))
- .thenAnswer((Invocation invocation) {
+ .thenAnswer((Invocation invocation) async {
expect(invocation.positionalArguments[0],
equals('server${counter++}.dart'));
expect(
invocation.positionalArguments[1]['sdk-root'], equals('sdkroot'));
compileCalled.sendPort.send(true);
+ return true;
});
Future<int> result = starter(
@@ -180,7 +183,7 @@
final ReceivePort recompileCalled = new ReceivePort();
when(compiler.recompileDelta(entryPoint: null))
- .thenAnswer((Invocation invocation) {
+ .thenAnswer((Invocation invocation) async {
recompileCalled.sendPort.send(true);
});
Future<int> result = starter(
@@ -208,7 +211,7 @@
final ReceivePort recompileCalled = new ReceivePort();
when(compiler.recompileDelta(entryPoint: 'file2.dart'))
- .thenAnswer((Invocation invocation) {
+ .thenAnswer((Invocation invocation) async {
recompileCalled.sendPort.send(true);
});
Future<int> result = starter(
@@ -275,7 +278,7 @@
final ReceivePort recompileCalled = new ReceivePort();
when(compiler.recompileDelta(entryPoint: null))
- .thenAnswer((Invocation invocation) {
+ .thenAnswer((Invocation invocation) async {
recompileCalled.sendPort.send(true);
});
Future<int> result = starter(
diff --git a/tests/compiler/dart2js/model/class_set_test.dart b/tests/compiler/dart2js/model/class_set_test.dart
index 5ceb1e8..ea78222 100644
--- a/tests/compiler/dart2js/model/class_set_test.dart
+++ b/tests/compiler/dart2js/model/class_set_test.dart
@@ -344,6 +344,7 @@
List<ClassEntity> visited = <ClassEntity>[];
classSet.forEachSubclass((cls) {
visited.add(cls);
+ return IterationStep.CONTINUE;
}, ClassHierarchyNode.ALL);
Expect.listEquals(
@@ -381,6 +382,7 @@
List<ClassEntity> visited = <ClassEntity>[];
classSet.forEachSubtype((cls) {
visited.add(cls);
+ return IterationStep.CONTINUE;
}, ClassHierarchyNode.ALL);
Expect.listEquals(
diff --git a/tests/compiler/dart2js/model/receiver_type_test.dart b/tests/compiler/dart2js/model/receiver_type_test.dart
index 663c6e3..3ad5042 100644
--- a/tests/compiler/dart2js/model/receiver_type_test.dart
+++ b/tests/compiler/dart2js/model/receiver_type_test.dart
@@ -6,6 +6,7 @@
import 'package:async_helper/async_helper.dart';
import 'package:compiler/src/elements/entities.dart';
import 'package:compiler/src/inferrer/typemasks/masks.dart';
+import 'package:compiler/src/universe/class_set.dart';
import 'package:compiler/src/universe/selector.dart';
import 'package:compiler/src/world.dart';
import 'package:expect/expect.dart';
@@ -49,7 +50,8 @@
Selector callSelector = new Selector.callClosure(0);
closedWorld.classHierarchy.forEachStrictSubclassOf(
closedWorld.commonElements.objectClass, (ClassEntity cls) {
- if (cls.library.canonicalUri.scheme != 'memory') return;
+ if (cls.library.canonicalUri.scheme != 'memory')
+ return IterationStep.CONTINUE;
TypeMask mask = new TypeMask.nonNullSubclass(cls, closedWorld);
TypeMask receiverType = closedWorld.computeReceiverType(callSelector, mask);
@@ -65,6 +67,7 @@
Expect.equals(expected, '$receiverType',
"Unexpected receiver type for $callSelector on $mask");
}
+ return IterationStep.CONTINUE;
});
Expect.equals(2, closureCount);
diff --git a/tests/compiler/dart2js/model/world_test.dart b/tests/compiler/dart2js/model/world_test.dart
index c49ff3d..b2106a8 100644
--- a/tests/compiler/dart2js/model/world_test.dart
+++ b/tests/compiler/dart2js/model/world_test.dart
@@ -99,6 +99,7 @@
List<ClassEntity> visited = <ClassEntity>[];
forEach(cls, (ClassEntity c) {
visited.add(c);
+ return IterationStep.CONTINUE;
});
checkClasses('forEach($property)', cls, visited, expectedClasses,
exact: exact);
@@ -424,6 +425,7 @@
if (allClasses.contains(other)) {
strictSubclasses.add(other);
}
+ return IterationStep.CONTINUE;
});
Expect.setEquals(subclasses, strictSubclasses,
"Unexpected strict subclasses of $cls: ${strictSubclasses}.");
@@ -433,6 +435,7 @@
if (allClasses.contains(other)) {
strictSubtypes.add(other);
}
+ return IterationStep.CONTINUE;
});
Expect.setEquals(subtypes, strictSubtypes,
"Unexpected strict subtypes of $cls: $strictSubtypes.");
diff --git a/tests/compiler/dart2js/sourcemaps/helpers/sourcemap_helper.dart b/tests/compiler/dart2js/sourcemaps/helpers/sourcemap_helper.dart
index f38c7a6..9e4273d 100644
--- a/tests/compiler/dart2js/sourcemaps/helpers/sourcemap_helper.dart
+++ b/tests/compiler/dart2js/sourcemaps/helpers/sourcemap_helper.dart
@@ -262,6 +262,7 @@
}
return null;
}
+ return null;
});
}
}
