Skip to content

Commit 3793a40

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Migration: stop using always for decorating the type of explicit nulls.
Change-Id: I950c9b8e80bc647c5a42b5cb5c66babd7c4d41e8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125008 Reviewed-by: Mike Fairhurst <mfairhurst@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
1 parent 12265c4 commit 3793a40

File tree

4 files changed

+93
-48
lines changed

4 files changed

+93
-48
lines changed

pkg/nnbd_migration/lib/src/edge_builder.dart

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,6 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
128128
/// For convenience, a [DecoratedType] representing non-nullable `Type`.
129129
final DecoratedType _nonNullableTypeType;
130130

131-
/// For convenience, a [DecoratedType] representing `Null`.
132-
final DecoratedType _nullType;
133-
134131
/// For convenience, a [DecoratedType] representing `dynamic`.
135132
final DecoratedType _dynamicType;
136133

@@ -203,7 +200,6 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
203200
DecoratedType(typeProvider.boolType, _graph.never),
204201
_nonNullableTypeType =
205202
DecoratedType(typeProvider.typeType, _graph.never),
206-
_nullType = DecoratedType(typeProvider.nullType, _graph.always),
207203
_dynamicType = DecoratedType(typeProvider.dynamicType, _graph.always);
208204

209205
/// Gets the decorated type of [element] from [_variables], performing any
@@ -970,7 +966,11 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
970966
@override
971967
DecoratedType visitNullLiteral(NullLiteral node) {
972968
_flowAnalysis.nullLiteral(node);
973-
return _nullType;
969+
var decoratedType =
970+
DecoratedType.forImplicitType(typeProvider, node.staticType, _graph);
971+
_graph.makeNullable(
972+
decoratedType.node, AlwaysNullableTypeOrigin(source, node));
973+
return decoratedType;
974974
}
975975

976976
@override
@@ -1094,8 +1094,12 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
10941094
Expression returnValue = node.expression;
10951095
final isAsync = node.thisOrAncestorOfType<FunctionBody>().isAsynchronous;
10961096
if (returnValue == null) {
1097+
var implicitNullType = DecoratedType.forImplicitType(
1098+
typeProvider, typeProvider.nullType, _graph);
1099+
_graph.makeNullable(
1100+
implicitNullType.node, AlwaysNullableTypeOrigin(source, node));
10971101
_checkAssignment(null,
1098-
source: isAsync ? _futureOf(_nullType) : _nullType,
1102+
source: isAsync ? _futureOf(implicitNullType) : implicitNullType,
10991103
destination: returnType,
11001104
hard: false);
11011105
} else {

pkg/nnbd_migration/test/edge_builder_test.dart

Lines changed: 56 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -342,11 +342,12 @@ class EdgeBuilderTest extends EdgeBuilderTestBase {
342342
assertEdge(node, right, hard: false);
343343
}
344344

345-
void assertLUB(
346-
NullabilityNode node, NullabilityNode left, NullabilityNode right) {
345+
void assertLUB(NullabilityNode node, Object left, Object right) {
347346
var conditionalNode = node as NullabilityNodeForLUB;
348-
expect(conditionalNode.left, same(left));
349-
expect(conditionalNode.right, same(right));
347+
var leftMatcher = NodeMatcher(left);
348+
var rightMatcher = NodeMatcher(right);
349+
expect(leftMatcher.matches(conditionalNode.left), true);
350+
expect(rightMatcher.matches(conditionalNode.right), true);
350351
}
351352

352353
/// Checks that there are no nullability nodes upstream from [node] that could
@@ -568,7 +569,8 @@ main() {
568569
}
569570
''');
570571
// TODO(paulberry): edge should be hard.
571-
assertEdge(always, decoratedTypeAnnotation('List').node, hard: false);
572+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('List').node,
573+
hard: false);
572574
}
573575

574576
test_assign_type_parameter_to_bound() async {
@@ -1566,7 +1568,7 @@ int f(bool b, int i) {
15661568

15671569
var nullable_i = decoratedTypeAnnotation('int i').node;
15681570
var nullable_conditional = decoratedExpressionType('(b ?').node;
1569-
assertLUB(nullable_conditional, always, nullable_i);
1571+
assertLUB(nullable_conditional, inSet(alwaysPlus), nullable_i);
15701572
}
15711573

15721574
test_conditionalExpression_right_non_null() async {
@@ -1593,7 +1595,7 @@ int f(bool b, int i) {
15931595

15941596
var nullable_i = decoratedTypeAnnotation('int i').node;
15951597
var nullable_conditional = decoratedExpressionType('(b ?').node;
1596-
assertLUB(nullable_conditional, nullable_i, always);
1598+
assertLUB(nullable_conditional, nullable_i, inSet(alwaysPlus));
15971599
}
15981600

15991601
test_constructor_default_parameter_value_bool() async {
@@ -2063,7 +2065,8 @@ void f({int i = 1}) {}
20632065
void f({int i = null}) {}
20642066
''');
20652067

2066-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false);
2068+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
2069+
hard: false);
20672070
}
20682071

20692072
test_functionDeclaration_parameter_named_no_default() async {
@@ -2097,7 +2100,8 @@ void f([int i = 1]) {}
20972100
void f([int i = null]) {}
20982101
''');
20992102

2100-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false);
2103+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
2104+
hard: false);
21012105
}
21022106

21032107
test_functionDeclaration_parameter_positionalOptional_no_default() async {
@@ -2198,8 +2202,10 @@ void test() {
21982202
}
21992203
''');
22002204

2201-
assertNullCheck(checkExpression('null'),
2202-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false));
2205+
assertNullCheck(
2206+
checkExpression('null'),
2207+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
2208+
hard: false));
22032209
}
22042210

22052211
test_functionInvocation_return() async {
@@ -2932,7 +2938,7 @@ List<String> f() {
29322938
final returnTypeEdge = returnTypeEdges.single;
29332939

29342940
final listArgType = returnTypeEdge.sourceNode;
2935-
assertEdge(always, listArgType, hard: false);
2941+
assertEdge(inSet(alwaysPlus), listArgType, hard: false);
29362942
}
29372943

29382944
test_listLiteral_typeArgument_noNullableElements() async {
@@ -2955,7 +2961,8 @@ List<String> f() {
29552961
}
29562962
''');
29572963
assertNoUpstreamNullability(decoratedTypeAnnotation('List').node);
2958-
assertEdge(always, decoratedTypeAnnotation('String>[').node, hard: false);
2964+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('String>[').node,
2965+
hard: false);
29592966
}
29602967

29612968
test_localVariable_type_inferred() async {
@@ -3615,8 +3622,10 @@ int f() {
36153622
}
36163623
''');
36173624

3618-
assertNullCheck(checkExpression('(null)'),
3619-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false));
3625+
assertNullCheck(
3626+
checkExpression('(null)'),
3627+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
3628+
hard: false));
36203629
}
36213630

36223631
test_part_metadata() async {
@@ -4065,7 +4074,8 @@ void test() {
40654074
// i1.toDouble() cannot be a hard edge or i2 will fail assignment
40664075
assertEdge(decoratedTypeAnnotation('int i').node, never, hard: false);
40674076
// i2 gets a soft edge to always due to null assignment
4068-
assertEdge(always, decoratedTypeAnnotation('int i').node, hard: false);
4077+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int i').node,
4078+
hard: false);
40694079
}
40704080

40714081
test_postDominators_questionQuestionOperator() async {
@@ -4769,7 +4779,8 @@ int f() {
47694779
}
47704780
''');
47714781

4772-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false);
4782+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
4783+
hard: false);
47734784
}
47744785

47754786
test_return_null() async {
@@ -4779,8 +4790,10 @@ int f() {
47794790
}
47804791
''');
47814792

4782-
assertNullCheck(checkExpression('null'),
4783-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false));
4793+
assertNullCheck(
4794+
checkExpression('null'),
4795+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
4796+
hard: false));
47844797
}
47854798

47864799
test_return_null_generic() async {
@@ -4792,9 +4805,9 @@ class C<T> {
47924805
}
47934806
''');
47944807
var tNode = decoratedTypeAnnotation('T f').node;
4795-
assertEdge(always, tNode, hard: false);
4796-
assertNullCheck(
4797-
checkExpression('null'), assertEdge(always, tNode, hard: false));
4808+
assertEdge(inSet(alwaysPlus), tNode, hard: false);
4809+
assertNullCheck(checkExpression('null'),
4810+
assertEdge(inSet(alwaysPlus), tNode, hard: false));
47984811
}
47994812

48004813
test_setOrMapLiteral_map_noTypeArgument_noNullableKeysAndValues() async {
@@ -4825,7 +4838,8 @@ Map<String, int> f() {
48254838
var mapNode = decoratedTypeAnnotation('Map').node;
48264839

48274840
assertNoUpstreamNullability(mapNode);
4828-
assertEdge(always, assertEdge(anyNode, keyNode, hard: false).sourceNode,
4841+
assertEdge(
4842+
inSet(alwaysPlus), assertEdge(anyNode, keyNode, hard: false).sourceNode,
48294843
hard: false);
48304844
assertNoUpstreamNullability(
48314845
assertEdge(anyNode, valueNode, hard: false).sourceNode);
@@ -4842,9 +4856,11 @@ Map<String, int> f() {
48424856
var mapNode = decoratedTypeAnnotation('Map').node;
48434857

48444858
assertNoUpstreamNullability(mapNode);
4845-
assertEdge(always, assertEdge(anyNode, keyNode, hard: false).sourceNode,
4859+
assertEdge(
4860+
inSet(alwaysPlus), assertEdge(anyNode, keyNode, hard: false).sourceNode,
48464861
hard: false);
4847-
assertEdge(always, assertEdge(anyNode, valueNode, hard: false).sourceNode,
4862+
assertEdge(inSet(alwaysPlus),
4863+
assertEdge(anyNode, valueNode, hard: false).sourceNode,
48484864
hard: false);
48494865
}
48504866

@@ -4861,7 +4877,8 @@ Map<String, int> f() {
48614877
assertNoUpstreamNullability(mapNode);
48624878
assertNoUpstreamNullability(
48634879
assertEdge(anyNode, keyNode, hard: false).sourceNode);
4864-
assertEdge(always, assertEdge(anyNode, valueNode, hard: false).sourceNode,
4880+
assertEdge(inSet(alwaysPlus),
4881+
assertEdge(anyNode, valueNode, hard: false).sourceNode,
48654882
hard: false);
48664883
}
48674884

@@ -4891,7 +4908,7 @@ Map<String, int> f() {
48914908
}
48924909
''');
48934910
assertNoUpstreamNullability(decoratedTypeAnnotation('Map').node);
4894-
assertEdge(always, decoratedTypeAnnotation('String, int>{').node,
4911+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('String, int>{').node,
48954912
hard: false);
48964913
assertNoUpstreamNullability(decoratedTypeAnnotation('int>{').node);
48974914
}
@@ -4903,9 +4920,10 @@ Map<String, int> f() {
49034920
}
49044921
''');
49054922
assertNoUpstreamNullability(decoratedTypeAnnotation('Map').node);
4906-
assertEdge(always, decoratedTypeAnnotation('String, int>{').node,
4923+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('String, int>{').node,
4924+
hard: false);
4925+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int>{').node,
49074926
hard: false);
4908-
assertEdge(always, decoratedTypeAnnotation('int>{').node, hard: false);
49094927
}
49104928

49114929
test_setOrMapLiteral_map_typeArguments_nullableValue() async {
@@ -4916,7 +4934,8 @@ Map<String, int> f() {
49164934
''');
49174935
assertNoUpstreamNullability(decoratedTypeAnnotation('Map').node);
49184936
assertNoUpstreamNullability(decoratedTypeAnnotation('String, int>{').node);
4919-
assertEdge(always, decoratedTypeAnnotation('int>{').node, hard: false);
4937+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int>{').node,
4938+
hard: false);
49204939
}
49214940

49224941
test_setOrMapLiteral_set_noTypeArgument_noNullableElements() async {
@@ -4943,7 +4962,8 @@ Set<String> f() {
49434962
var setNode = decoratedTypeAnnotation('Set').node;
49444963

49454964
assertNoUpstreamNullability(setNode);
4946-
assertEdge(always, assertEdge(anyNode, valueNode, hard: false).sourceNode,
4965+
assertEdge(inSet(alwaysPlus),
4966+
assertEdge(anyNode, valueNode, hard: false).sourceNode,
49474967
hard: false);
49484968
}
49494969

@@ -4967,7 +4987,8 @@ Set<String> f() {
49674987
}
49684988
''');
49694989
assertNoUpstreamNullability(decoratedTypeAnnotation('Set').node);
4970-
assertEdge(always, decoratedTypeAnnotation('String>{').node, hard: false);
4990+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('String>{').node,
4991+
hard: false);
49714992
}
49724993

49734994
test_simpleIdentifier_function() async {
@@ -5041,7 +5062,8 @@ main() {}
50415062
await analyze('''
50425063
int f() => null;
50435064
''');
5044-
assertEdge(always, decoratedTypeAnnotation('int').node, hard: false);
5065+
assertEdge(inSet(alwaysPlus), decoratedTypeAnnotation('int').node,
5066+
hard: false);
50455067
}
50465068

50475069
test_spread_element_list() async {
@@ -5237,7 +5259,7 @@ void set x(int value) {}
52375259
main() { x = null; }
52385260
''');
52395261
var setXType = decoratedTypeAnnotation('int value');
5240-
assertEdge(always, setXType.node, hard: false);
5262+
assertEdge(inSet(alwaysPlus), setXType.node, hard: false);
52415263
}
52425264

52435265
test_topLevelVar_metadata() async {

pkg/nnbd_migration/test/instrumentation_test.dart

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,10 @@ int x = null;
458458
expect(always.isNullable, true);
459459
var xNode = explicitTypeNullability[findNode.typeAnnotation('int')];
460460
var edge = edges.where((e) => e.destinationNode == xNode).single;
461-
expect(edge.sourceNode, always);
461+
var edgeSource = edge.sourceNode;
462+
var upstreamEdge =
463+
edges.where((e) => e.destinationNode == edgeSource).single;
464+
expect(upstreamEdge.sourceNode, always);
462465
}
463466

464467
test_immutableNode_never() async {
@@ -774,7 +777,7 @@ List<int> f() => g(null);
774777
explicitTypeNullability[findNode.typeAnnotation('int')];
775778
expect(edges.where((e) {
776779
var destination = e.destinationNode;
777-
return e.sourceNode == always &&
780+
return _isPointedToByAlways(e.sourceNode) &&
778781
destination is SubstitutionNodeInfo &&
779782
destination.innerNode == implicitInvocationTypeArgumentNode;
780783
}), hasLength(1));
@@ -801,7 +804,7 @@ List<int> f(C c) => c.g(null);
801804
explicitTypeNullability[findNode.typeAnnotation('int')];
802805
expect(edges.where((e) {
803806
var destination = e.destinationNode;
804-
return e.sourceNode == always &&
807+
return _isPointedToByAlways(e.sourceNode) &&
805808
destination is SubstitutionNodeInfo &&
806809
destination.innerNode == implicitInvocationTypeArgumentNode;
807810
}), hasLength(1));
@@ -826,7 +829,7 @@ C<int> f() => C(null);
826829
explicitTypeNullability[findNode.typeAnnotation('int')];
827830
expect(edges.where((e) {
828831
var destination = e.destinationNode;
829-
return e.sourceNode == always &&
832+
return _isPointedToByAlways(e.sourceNode) &&
830833
destination is SubstitutionNodeInfo &&
831834
destination.innerNode == implicitInvocationTypeArgumentNode;
832835
}), hasLength(1));
@@ -847,7 +850,7 @@ List<int> f() => [null];
847850
explicitTypeNullability[findNode.typeAnnotation('int')];
848851
expect(
849852
edges.where((e) =>
850-
e.sourceNode == always &&
853+
_isPointedToByAlways(e.sourceNode) &&
851854
e.destinationNode == implicitListLiteralElementNode),
852855
hasLength(1));
853856
expect(
@@ -881,7 +884,7 @@ Map<int, String> f() => {1: null};
881884
hasLength(1));
882885
expect(
883886
edges.where((e) =>
884-
e.sourceNode == always &&
887+
_isPointedToByAlways(e.sourceNode) &&
885888
e.destinationNode == implicitMapLiteralValueNode),
886889
hasLength(1));
887890
expect(
@@ -901,7 +904,7 @@ Set<int> f() => {null};
901904
explicitTypeNullability[findNode.typeAnnotation('int')];
902905
expect(
903906
edges.where((e) =>
904-
e.sourceNode == always &&
907+
_isPointedToByAlways(e.sourceNode) &&
905908
e.destinationNode == implicitSetLiteralElementNode),
906909
hasLength(1));
907910
expect(
@@ -934,7 +937,7 @@ int x = null;
934937
var step = propagationSteps.where((s) => s.node == xNode).single;
935938
expect(step.newState, NullabilityState.ordinaryNullable);
936939
expect(step.reason, StateChangeReason.downstream);
937-
expect(step.edge.sourceNode, always);
940+
expect(_isPointedToByAlways(step.edge.sourceNode), true);
938941
expect(step.edge.destinationNode, xNode);
939942
}
940943

@@ -969,4 +972,9 @@ voig g(C<int> x, int y) {
969972
expect(sNode.outerNode,
970973
explicitTypeNullability[findNode.typeAnnotation('T t')]);
971974
}
975+
976+
bool _isPointedToByAlways(NullabilityNodeInfo node) {
977+
return edges
978+
.any((e) => e.sourceNode == always && e.destinationNode == node);
979+
}
972980
}

0 commit comments

Comments
 (0)