Make each NullabilityNode track those nodes that are downstream of it.
This allows more migration tool unit tests to be simplified so that
they don't have to refer to ConstraintVariable objects directly
anymore.
In order to make this work, a few other changes were also necessary:
- Make a derived class for NullabilityNodes that arise from
conditional expressions.
- Add a getter to reveal whether a NullabilityNode is known a priori
to be non-nullable.
Change-Id: I8a68b627ce9e807a50d371369985f52848903a2f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100922
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
index 7127e82..52ed8a5 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
@@ -206,7 +206,7 @@
assert(_isSimple(elseType)); // TODO(paulberry)
var overallType = DecoratedType(
node.staticType,
- NullabilityNode.forConditionalexpression(
+ NullabilityNode.forLUB(
node, thenType.node, elseType.node, _joinNullabilities));
_variables.recordDecoratedExpressionType(node, overallType);
return overallType;
@@ -502,7 +502,7 @@
/// Creates a constraint variable (if necessary) representing the nullability
/// of [node], which is the disjunction of the nullabilities [a] and [b].
ConstraintVariable _joinNullabilities(
- ConditionalExpression node, ConstraintVariable a, ConstraintVariable b) {
+ Expression node, ConstraintVariable a, ConstraintVariable b) {
if (a == null) return b;
if (b == null) return a;
if (identical(a, ConstraintVariable.always) ||
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_node.dart b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
index f993264..e244d0e 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_node.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
@@ -30,27 +30,15 @@
/// migrated) forces this type to be non-nullable.
final ConstraintVariable nullable;
+ /// List of all the nodes that are "downstream" of this one (i.e. if this node
+ /// is nullable, then all the nodes in [_downstreamNodes] will either have to
+ /// be nullable, or null checks will have to be added).
+ final _downstreamNodes = <NullabilityNode>[];
+
ConstraintVariable _nonNullIntent;
bool _isPossiblyOptional = false;
- /// Creates a [NullabilityNode] representing the nullability of a conditional
- /// expression which is nullable iff both [a] and [b] are nullable.
- ///
- /// The constraint variable contained in the new node is created using the
- /// [joinNullabilities] callback. TODO(paulberry): this should become
- /// unnecessary once constraint solving is performed directly using
- /// [NullabilityNode] objects.
- NullabilityNode.forConditionalexpression(
- ConditionalExpression conditionalExpression,
- NullabilityNode a,
- NullabilityNode b,
- ConstraintVariable Function(
- ConditionalExpression, ConstraintVariable, ConstraintVariable)
- joinNullabilities)
- : this._(
- joinNullabilities(conditionalExpression, a.nullable, b.nullable));
-
/// Creates a [NullabilityNode] representing the nullability of a variable
/// whose type is `dynamic` due to type inference.
///
@@ -58,6 +46,21 @@
/// inferred type rather than assuming `dynamic`.
NullabilityNode.forInferredDynamicType() : this._(ConstraintVariable.always);
+ /// Creates a [NullabilityNode] representing the nullability of an
+ /// expression which is nullable iff both [a] and [b] are nullable.
+ ///
+ /// The constraint variable contained in the new node is created using the
+ /// [joinNullabilities] callback. TODO(paulberry): this should become
+ /// unnecessary once constraint solving is performed directly using
+ /// [NullabilityNode] objects.
+ factory NullabilityNode.forLUB(
+ Expression conditionalExpression,
+ NullabilityNode a,
+ NullabilityNode b,
+ ConstraintVariable Function(
+ Expression, ConstraintVariable, ConstraintVariable)
+ joinNullabilities) = NullabilityNodeForLUB._;
+
/// Creates a [NullabilityNode] representing the nullability of a type
/// substitution where [outerNode] is the nullability node for the type
/// variable being eliminated by the substitution, and [innerNode] is the
@@ -85,9 +88,17 @@
/// annotate the nullability of that type.
String get debugSuffix => nullable == null ? '' : '?($nullable)';
+ /// Iterates through all nodes that are "downstream" of this node (i.e. if
+ /// this node is nullable, then all the nodes in [downstreamNodes] will either
+ /// have to be nullable, or null checks will have to be added).
+ Iterable<NullabilityNode> get downstreamNodes => _downstreamNodes;
+
/// Indicates whether this node is always nullable, by construction.
bool get isAlwaysNullable => identical(nullable, ConstraintVariable.always);
+ /// Indicates whether this node is never nullable, by construction.
+ bool get isNeverNullable => nullable == null;
+
/// After constraint solving, this getter can be used to query whether the
/// type associated with this node should be considered nullable.
bool get isNullable => nullable == null ? false : nullable.value;
@@ -153,6 +164,7 @@
Constraints constraints,
bool inConditionalControlFlow) {
var additionalConditions = <ConstraintVariable>[];
+ sourceNode._downstreamNodes.add(destinationNode);
if (sourceNode.nullable != null) {
additionalConditions.add(sourceNode.nullable);
var destinationNonNullIntent = destinationNode.nonNullIntent;
@@ -200,6 +212,26 @@
}
}
+/// Derived class for nullability nodes that arise from the least-upper-bound
+/// implied by a conditional expression.
+class NullabilityNodeForLUB extends NullabilityNode {
+ final NullabilityNode left;
+
+ final NullabilityNode right;
+
+ NullabilityNodeForLUB._(
+ Expression expression,
+ this.left,
+ this.right,
+ ConstraintVariable Function(
+ ConditionalExpression, ConstraintVariable, ConstraintVariable)
+ joinNullabilities)
+ : super._(joinNullabilities(expression, left.nullable, right.nullable)) {
+ left._downstreamNodes.add(this);
+ right._downstreamNodes.add(this);
+ }
+}
+
/// Derived class for nullability nodes that arise from type variable
/// substitution.
class NullabilityNodeForSubstitution extends NullabilityNode {
diff --git a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
index 315cea2..cc45432 100644
--- a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
+++ b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
@@ -31,6 +31,27 @@
@override
final _Constraints constraints = _Constraints();
+ void assertConditional(
+ NullabilityNode node, NullabilityNode left, NullabilityNode right) {
+ var conditionalNode = node as NullabilityNodeForLUB;
+ expect(conditionalNode.left, same(left));
+ expect(conditionalNode.right, same(right));
+ if (left.isNeverNullable) {
+ if (right.isNeverNullable) {
+ expect(conditionalNode.isNeverNullable, true);
+ } else {
+ expect(conditionalNode.nullable, same(right.nullable));
+ }
+ } else {
+ if (right.isNeverNullable) {
+ expect(conditionalNode.nullable, same(left.nullable));
+ } else {
+ assertConstraint([left.nullable], conditionalNode.nullable);
+ assertConstraint([right.nullable], conditionalNode.nullable);
+ }
+ }
+ }
+
/// Checks that a constraint was recorded with a left hand side of
/// [conditions] and a right hand side of [consequence].
void assertConstraint(
@@ -211,14 +232,11 @@
}
''');
- var nullable_i = decoratedTypeAnnotation('int i').node.nullable;
- var nullable_j = decoratedTypeAnnotation('int j').node.nullable;
- var nullable_i_or_nullable_j = _either(nullable_i, nullable_j);
+ var nullable_i = decoratedTypeAnnotation('int i').node;
+ var nullable_j = decoratedTypeAnnotation('int j').node;
var nullable_conditional = decoratedExpressionType('(b ?').node;
+ assertConditional(nullable_conditional, nullable_i, nullable_j);
var nullable_return = decoratedTypeAnnotation('int f').node;
- assertConstraint([nullable_i], nullable_conditional.nullable);
- assertConstraint([nullable_j], nullable_conditional.nullable);
- assertConstraint([nullable_conditional.nullable], nullable_i_or_nullable_j);
assertNullCheck(checkExpression('(b ? i : j)'), nullable_conditional,
contextNode: nullable_return);
}
@@ -230,9 +248,12 @@
}
''');
- var nullable_i = decoratedTypeAnnotation('int i').node.nullable;
- var nullable_conditional = decoratedExpressionType('(b ?').node.nullable;
- expect(nullable_conditional, same(nullable_i));
+ var nullable_i = decoratedTypeAnnotation('int i').node;
+ var nullable_conditional =
+ decoratedExpressionType('(b ?').node as NullabilityNodeForLUB;
+ var nullable_throw = nullable_conditional.left;
+ expect(nullable_throw.isNeverNullable, true);
+ assertConditional(nullable_conditional, nullable_throw, nullable_i);
}
test_conditionalExpression_left_null() async {
@@ -242,8 +263,8 @@
}
''');
- var nullable_conditional = decoratedExpressionType('(b ?').node.nullable;
- expect(nullable_conditional, same(ConstraintVariable.always));
+ var nullable_conditional = decoratedExpressionType('(b ?').node;
+ expect(nullable_conditional.isAlwaysNullable, true);
}
test_conditionalExpression_right_non_null() async {
@@ -253,9 +274,12 @@
}
''');
- var nullable_i = decoratedTypeAnnotation('int i').node.nullable;
- var nullable_conditional = decoratedExpressionType('(b ?').node.nullable;
- expect(nullable_conditional, same(nullable_i));
+ var nullable_i = decoratedTypeAnnotation('int i').node;
+ var nullable_conditional =
+ decoratedExpressionType('(b ?').node as NullabilityNodeForLUB;
+ var nullable_throw = nullable_conditional.right;
+ expect(nullable_throw.isNeverNullable, true);
+ assertConditional(nullable_conditional, nullable_i, nullable_throw);
}
test_conditionalExpression_right_null() async {
@@ -265,8 +289,8 @@
}
''');
- var nullable_conditional = decoratedExpressionType('(b ?').node.nullable;
- expect(nullable_conditional, same(ConstraintVariable.always));
+ var nullable_conditional = decoratedExpressionType('(b ?').node;
+ expect(nullable_conditional.isAlwaysNullable, true);
}
test_functionDeclaration_expression_body() async {
@@ -759,13 +783,6 @@
''');
assertNoConstraints(decoratedTypeAnnotation('Type').node.nullable);
}
-
- /// Creates a variable representing the disjunction of [a] and [b] solely for
- /// the purpose of inspecting constraint equations in unit tests. No
- /// additional constraints will be recorded in [_constraints] as a consequence
- /// of creating this variable.
- ConstraintVariable _either(ConstraintVariable a, ConstraintVariable b) =>
- ConstraintVariable.or(_MockConstraints(), a, b);
}
abstract class ConstraintsTestBase extends MigrationVisitorTestBase {
@@ -988,13 +1005,6 @@
}
}
-/// Mock implementation of [Constraints] that doesn't record any constraints.
-class _MockConstraints implements Constraints {
- @override
- void record(Iterable<ConstraintVariable> conditions,
- ConstraintVariable consequence) {}
-}
-
/// Mock representation of constraint variables.
class _Variables extends Variables {
final _conditionalDiscard = <AstNode, ConditionalDiscard>{};
