Skip to content

Commit 5d2c17d

Browse files
MichaelRFairhurstcommit-bot@chromium.org
MichaelRFairhurst
authored and
[email protected]
committed
[nnbd_migration] Handle if elements in lists.
Change-Id: Ia892668cc31db2453e24778067b7b2c857813482 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112644 Reviewed-by: Paul Berry <[email protected]> Commit-Queue: Mike Fairhurst <[email protected]>
1 parent c536510 commit 5d2c17d

File tree

3 files changed

+176
-9
lines changed

3 files changed

+176
-9
lines changed

‎pkg/nnbd_migration/lib/src/edge_builder.dart‎

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
111111
/// return statements.
112112
DecoratedType _currentFunctionType;
113113

114+
/// The [DecoratedType] of the innermost list or set literal being visited, or
115+
/// `null` if the visitor is not inside any function or method.
116+
///
117+
/// This is needed to construct the appropriate nullability constraints for
118+
/// ui as code list elements.
119+
DecoratedType _currentLiteralType;
120+
114121
/// Information about the most recently visited binary expression whose
115122
/// boolean value could possibly affect nullability analysis.
116123
_ConditionInfo _conditionInfo;
@@ -555,6 +562,44 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
555562
node.typeArguments, calleeType, null);
556563
}
557564

565+
@override
566+
DecoratedType visitIfElement(IfElement node) {
567+
_handleAssignment(node.condition, _notNullType);
568+
NullabilityNode trueGuard;
569+
NullabilityNode falseGuard;
570+
if (identical(_conditionInfo?.condition, node.condition)) {
571+
trueGuard = _conditionInfo.trueGuard;
572+
falseGuard = _conditionInfo.falseGuard;
573+
_variables.recordConditionalDiscard(_source, node,
574+
ConditionalDiscard(trueGuard, falseGuard, _conditionInfo.isPure));
575+
}
576+
if (trueGuard != null) {
577+
_guards.add(trueGuard);
578+
}
579+
try {
580+
_postDominatedLocals.doScoped(
581+
action: () => _handleCollectionElement(node.thenElement));
582+
} finally {
583+
if (trueGuard != null) {
584+
_guards.removeLast();
585+
}
586+
}
587+
if (node.elseElement != null) {
588+
if (falseGuard != null) {
589+
_guards.add(falseGuard);
590+
}
591+
try {
592+
_postDominatedLocals.doScoped(
593+
action: () => _handleCollectionElement(node.elseElement));
594+
} finally {
595+
if (falseGuard != null) {
596+
_guards.removeLast();
597+
}
598+
}
599+
}
600+
return null;
601+
}
602+
558603
@override
559604
DecoratedType visitIfStatement(IfStatement node) {
560605
// TODO(paulberry): should the use of a boolean in an if-statement be
@@ -681,15 +726,12 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
681726
if (typeArgumentType == null) {
682727
_unimplemented(node, 'Could not compute type argument type');
683728
}
684-
for (var element in node.elements) {
685-
if (element is Expression) {
686-
_handleAssignment(element, typeArgumentType);
687-
} else {
688-
// Handle spread and control flow elements.
689-
element.accept(this);
690-
// TODO(brianwilkerson)
691-
_unimplemented(node, 'Spread or control flow element');
692-
}
729+
final previousLiteralType = _currentLiteralType;
730+
try {
731+
_currentLiteralType = typeArgumentType;
732+
node.elements.forEach(_handleCollectionElement);
733+
} finally {
734+
_currentLiteralType = previousLiteralType;
693735
}
694736
return DecoratedType(listType, _graph.never,
695737
typeArguments: [typeArgumentType]);
@@ -1206,6 +1248,15 @@ $stackTrace''');
12061248
return sourceType;
12071249
}
12081250

1251+
DecoratedType _handleCollectionElement(CollectionElement element) {
1252+
if (element is Expression) {
1253+
assert(_currentLiteralType != null);
1254+
return _handleAssignment(element, _currentLiteralType);
1255+
} else {
1256+
return element.accept(this);
1257+
}
1258+
}
1259+
12091260
void _handleConstructorRedirection(
12101261
FormalParameterList parameters, ConstructorName redirectedConstructor) {
12111262
var callee = redirectedConstructor.staticElement;

‎pkg/nnbd_migration/test/api_test.dart‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1938,6 +1938,31 @@ main() {
19381938
await _checkSingleFileChanges(content, expected);
19391939
}
19401940

1941+
@failingTest
1942+
test_removed_if_element_doesnt_introduce_nullability() async {
1943+
// Failing for two reasons: 1. we don't add ! to recover(), and 2. we get
1944+
// an unimplemented error.
1945+
var content = '''
1946+
f(int x) {
1947+
<int>[if (x == null) recover(), 0];
1948+
}
1949+
int recover() {
1950+
assert(false);
1951+
return null;
1952+
}
1953+
''';
1954+
var expected = '''
1955+
f(int x) {
1956+
<int>[if (x == null) recover()!, 0];
1957+
}
1958+
int? recover() {
1959+
assert(false);
1960+
return null;
1961+
}
1962+
''';
1963+
await _checkSingleFileChanges(content, expected);
1964+
}
1965+
19411966
test_single_file_multiple_changes() async {
19421967
var content = '''
19431968
int f() => null;

‎pkg/nnbd_migration/test/edge_builder_test.dart‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,6 +1707,76 @@ void f(bool b, int i) {
17071707
assertNoEdge(always, decoratedTypeAnnotation('int i').node);
17081708
}
17091709

1710+
test_if_element() async {
1711+
await analyze('''
1712+
void f(bool b) {
1713+
int i1 = null;
1714+
int i2 = null;
1715+
<int>[if (b) i1 else i2];
1716+
}
1717+
''');
1718+
1719+
assertNullCheck(checkExpression('b) i1'),
1720+
assertEdge(decoratedTypeAnnotation('bool b').node, never, hard: true));
1721+
assertEdge(decoratedTypeAnnotation('int i1').node,
1722+
decoratedTypeAnnotation('int>[').node,
1723+
hard: false);
1724+
assertEdge(decoratedTypeAnnotation('int i2').node,
1725+
decoratedTypeAnnotation('int>[').node,
1726+
hard: false);
1727+
}
1728+
1729+
@failingTest
1730+
test_if_element_guard_equals_null() async {
1731+
// failing because of an unimplemented exception in conditional modification
1732+
await analyze('''
1733+
dynamic f(int i, int j, int k) {
1734+
<int>[if (i == null) j/*check*/ else k/*check*/];
1735+
}
1736+
''');
1737+
var nullable_i = decoratedTypeAnnotation('int i').node;
1738+
var nullable_j = decoratedTypeAnnotation('int j').node;
1739+
var nullable_k = decoratedTypeAnnotation('int k').node;
1740+
var nullable_itemType = decoratedTypeAnnotation('int>[').node;
1741+
assertNullCheck(
1742+
checkExpression('j/*check*/'),
1743+
assertEdge(nullable_j, nullable_itemType,
1744+
guards: [nullable_i], hard: false));
1745+
assertNullCheck(checkExpression('k/*check*/'),
1746+
assertEdge(nullable_k, nullable_itemType, hard: false));
1747+
var discard = statementDiscard('if (i == null)');
1748+
expect(discard.trueGuard, same(nullable_i));
1749+
expect(discard.falseGuard, null);
1750+
expect(discard.pureCondition, true);
1751+
}
1752+
1753+
test_if_element_nested() async {
1754+
await analyze('''
1755+
void f(bool b1, bool b2) {
1756+
int i1 = null;
1757+
int i2 = null;
1758+
int i3 = null;
1759+
<int>[if (b1) if (b2) i1 else i2 else i3];
1760+
}
1761+
''');
1762+
1763+
assertNullCheck(checkExpression('b1)'),
1764+
assertEdge(decoratedTypeAnnotation('bool b1').node, never, hard: true));
1765+
assertNullCheck(
1766+
checkExpression('b2) i1'),
1767+
assertEdge(decoratedTypeAnnotation('bool b2').node, never,
1768+
hard: false));
1769+
assertEdge(decoratedTypeAnnotation('int i1').node,
1770+
decoratedTypeAnnotation('int>[').node,
1771+
hard: false);
1772+
assertEdge(decoratedTypeAnnotation('int i2').node,
1773+
decoratedTypeAnnotation('int>[').node,
1774+
hard: false);
1775+
assertEdge(decoratedTypeAnnotation('int i3').node,
1776+
decoratedTypeAnnotation('int>[').node,
1777+
hard: false);
1778+
}
1779+
17101780
test_if_guard_equals_null() async {
17111781
await analyze('''
17121782
int f(int i, int j, int k) {
@@ -2695,6 +2765,27 @@ void test(bool b1, C c1, C c2, C c3) {
26952765
assertEdge(decoratedTypeAnnotation('C c3').node, never, hard: false));
26962766
}
26972767

2768+
test_postDominators_ifElement() async {
2769+
await analyze('''
2770+
class C {
2771+
int m() => 0;
2772+
}
2773+
void test(bool b, C c1, C c2, C c3) {
2774+
<int>[if (b) c1.m() else c2.m()];
2775+
c3.m();
2776+
}
2777+
''');
2778+
2779+
assertNullCheck(checkExpression('b)'),
2780+
assertEdge(decoratedTypeAnnotation('bool b').node, never, hard: true));
2781+
assertNullCheck(checkExpression('c1.m'),
2782+
assertEdge(decoratedTypeAnnotation('C c1').node, never, hard: false));
2783+
assertNullCheck(checkExpression('c2.m'),
2784+
assertEdge(decoratedTypeAnnotation('C c2').node, never, hard: false));
2785+
assertNullCheck(checkExpression('c3.m'),
2786+
assertEdge(decoratedTypeAnnotation('C c3').node, never, hard: true));
2787+
}
2788+
26982789
test_postDominators_ifStatement_conditional() async {
26992790
await analyze('''
27002791
class C {

0 commit comments

Comments
 (0)