Skip to content

Commit 72f461b

Browse files
scheglovcommit-bot@chromium.org
scheglov
authored and
commit-bot@chromium.org
committed
Update FunctionExpression return type inference to NNBD.
This reverts some changes I did in LUB update CL. https://dart-review.googlesource.com/c/sdk/+/125761 Change-Id: I29e8896f6007ecb643c30ad98f2055fb917eeacc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125999 Reviewed-by: Paul Berry <paulberry@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
1 parent c9165dc commit 72f461b

File tree

5 files changed

+71
-42
lines changed

5 files changed

+71
-42
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,8 +1477,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
14771477
}
14781478
var returnType =
14791479
_inAsync ? _typeSystem.flatten(expectedReturnType) : expectedReturnType;
1480-
if (returnType.isBottom ||
1481-
returnType.isDynamic ||
1480+
if (returnType.isDynamic ||
14821481
returnType.isDartCoreNull ||
14831482
returnType.isVoid) {
14841483
return;

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

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2232,8 +2232,12 @@ class InferenceContext {
22322232
}
22332233

22342234
DartType inferred = _inferredReturn.last;
2235-
inferred = _typeSystem.getLeastUpperBound(type, inferred);
2236-
inferred = _resolver.toLegacyTypeIfOptOut(inferred);
2235+
if (inferred == null) {
2236+
inferred = type;
2237+
} else {
2238+
inferred = _typeSystem.getLeastUpperBound(type, inferred);
2239+
inferred = _resolver.toLegacyTypeIfOptOut(inferred);
2240+
}
22372241
_inferredReturn[_inferredReturn.length - 1] = inferred;
22382242
}
22392243

@@ -2244,8 +2248,21 @@ class InferenceContext {
22442248
/// bound of all types added with [addReturnOrYieldType].
22452249
void popReturnContext(FunctionBody node) {
22462250
if (_returnStack.isNotEmpty && _inferredReturn.isNotEmpty) {
2247-
DartType context = _returnStack.removeLast() ?? DynamicTypeImpl.instance;
2251+
// If NNBD, and the function body end is reachable, infer nullable.
2252+
// If legacy, we consider the end as always reachable, and return Null.
2253+
if (_resolver._nonNullableEnabled) {
2254+
var flow = _resolver._flowAnalysis?.flow;
2255+
if (flow != null && flow.isReachable) {
2256+
addReturnOrYieldType(_typeProvider.nullType);
2257+
}
2258+
} else {
2259+
addReturnOrYieldType(_typeProvider.nullType);
2260+
}
2261+
2262+
DartType context = _returnStack.removeLast();
22482263
DartType inferred = _inferredReturn.removeLast();
2264+
context ??= DynamicTypeImpl.instance;
2265+
inferred ??= DynamicTypeImpl.instance;
22492266

22502267
if (_typeSystem.isSubtypeOf(inferred, context)) {
22512268
setType(node, inferred);
@@ -2258,11 +2275,7 @@ class InferenceContext {
22582275
/// Push a block function body's return type onto the return stack.
22592276
void pushReturnContext(FunctionBody node) {
22602277
_returnStack.add(getContext(node));
2261-
if (_resolver._nonNullableEnabled) {
2262-
_inferredReturn.add(_typeProvider.neverType);
2263-
} else {
2264-
_inferredReturn.add(_typeProvider.nullType);
2265-
}
2278+
_inferredReturn.add(null);
22662279
}
22672280

22682281
/// Place an info node into the error stream indicating that a

‎pkg/analyzer/lib/src/summary2/ast_resolver.dart

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/element/element.dart';
77
import 'package:analyzer/error/listener.dart';
8+
import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart';
89
import 'package:analyzer/src/dart/resolver/resolution_visitor.dart';
910
import 'package:analyzer/src/generated/resolver.dart';
1011
import 'package:analyzer/src/summary2/link.dart';
@@ -15,7 +16,17 @@ class AstResolver {
1516
final CompilationUnitElement _unitElement;
1617
final Scope _nameScope;
1718

18-
AstResolver(this._linker, this._unitElement, this._nameScope);
19+
/// This field is set if the library is non-nullable by default.
20+
FlowAnalysisHelper flowAnalysis;
21+
22+
AstResolver(this._linker, this._unitElement, this._nameScope) {
23+
if (_unitElement.library.isNonNullableByDefault) {
24+
flowAnalysis = FlowAnalysisHelper(
25+
_unitElement.library.typeSystem,
26+
false,
27+
);
28+
}
29+
}
1930

2031
void resolve(
2132
AstNode node,
@@ -56,6 +67,7 @@ class AstResolver {
5667
nameScope: _nameScope,
5768
propagateTypes: false,
5869
reportConstEvaluationErrors: false,
70+
flowAnalysisHelper: flowAnalysis,
5971
);
6072
resolverVisitor.prepareEnclosingDeclarations(
6173
enclosingClassElement: enclosingClassElement,

‎pkg/analyzer/lib/src/summary2/top_level_inference.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,7 @@ class _VariableInferenceNode extends _InferenceNode {
456456

457457
void _resolveInitializer() {
458458
var astResolver = AstResolver(_walker._linker, _unitElement, _scope);
459+
astResolver.flowAnalysis?.topLevelDeclaration_enter(_node, null, null);
459460
astResolver.resolve(_node.initializer, () => _node.initializer);
460461
}
461462
}

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

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -17,43 +17,47 @@ main() {
1717

1818
@reflectiveTest
1919
class FunctionExpressionTest extends DriverResolutionTest {
20-
test_closure_returnType() async {
21-
await assertNoErrorsInCode('''
22-
typedef ReturnsVoid = void Function(int a);
23-
24-
void setClosureContext(ReturnsVoid a) {}
25-
26-
void fail(String message) {
27-
throw message;
28-
}
20+
test_returnType_notNullable() async {
21+
await resolveTestCode('''
22+
var v = (bool b) {
23+
if (b) return 0;
24+
return 1.2;
25+
};
26+
''');
27+
var element = findNode.functionExpression('(bool').declaredElement;
28+
assertElementTypeString(element.returnType, 'num');
29+
}
2930

30-
void main() {
31-
setClosureContext((a) {
32-
if (a == 42) return;
33-
});
34-
}
31+
test_returnType_null_hasReturn() async {
32+
await resolveTestCode('''
33+
var v = (bool b) {
34+
if (b) return;
35+
};
3536
''');
36-
var element = findNode.functionExpression('(a)').declaredElement;
37-
if (typeToStringWithNullability) {
38-
assertElementTypeString(element.returnType, 'Never');
39-
} else {
40-
assertElementTypeString(element.returnType, 'Null');
41-
}
37+
var element = findNode.functionExpression('(bool').declaredElement;
38+
assertElementTypeString(element.returnType, 'Null');
4239
}
4340

44-
test_return() async {
41+
test_returnType_null_noReturn() async {
4542
await resolveTestCode('''
46-
var f = (bool b) {
47-
if (b) {
48-
return 0;
43+
var v = () {};
44+
''');
45+
var element = findNode.functionExpression('() {}').declaredElement;
46+
assertElementTypeString(element.returnType, 'Null');
4947
}
50-
return 1.2;
51-
}
48+
49+
test_returnType_nullable() async {
50+
await resolveTestCode('''
51+
var v = (bool b) {
52+
if (b) return 0;
53+
};
5254
''');
53-
assertElementTypeString(
54-
findElement.topVar('f').type,
55-
'num Function(bool)',
56-
);
55+
var element = findNode.functionExpression('(bool').declaredElement;
56+
if (typeToStringWithNullability) {
57+
assertElementTypeString(element.returnType, 'int?');
58+
} else {
59+
assertElementTypeString(element.returnType, 'int');
60+
}
5761
}
5862
}
5963

@@ -62,7 +66,7 @@ class FunctionExpressionWithNnbdTest extends FunctionExpressionTest {
6266
@override
6367
AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
6468
..contextFeatures = new FeatureSet.forTesting(
65-
sdkVersion: '2.3.0', additionalFeatures: [Feature.non_nullable]);
69+
sdkVersion: '2.6.0', additionalFeatures: [Feature.non_nullable]);
6670

6771
@override
6872
bool get typeToStringWithNullability => true;

0 commit comments

Comments
 (0)