Switch nullability migration over to the new algorithm based on the new nullability graph.
Change-Id: I1cbfba8d3d46020d8e943109e582d61315781561
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102386
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_graph.dart b/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
index 598dff1..0541ed0 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
@@ -27,8 +27,6 @@
final _unconditionalUpstream =
Map<NullabilityNode, List<NullabilityNode>>.identity();
- final _nullableNodes = Set<NullabilityNode>.identity();
-
final _nonNullIntentNodes = Set<NullabilityNode>.identity();
/// Verifies that the conclusions reached by [propagate] match the conclusions
@@ -39,7 +37,7 @@
var allNodes = _downstream.keys.toSet();
allNodes.addAll(_upstream.keys);
for (var node in allNodes) {
- node.check(_nullableNodes.contains(node));
+ node.check();
}
} catch (_) {
debugDump();
@@ -76,7 +74,7 @@
return '${edge.destinationNode}$suffix';
});
var suffixes = <String>[];
- if (_nullableNodes.contains(entry.key)) {
+ if (entry.key.isNullable) {
suffixes.add('nullable');
}
if (_nonNullIntentNodes.contains(entry.key)) {
@@ -122,7 +120,8 @@
/// Propagates nullability downstream.
void _propagateDownstream() {
- var pendingEdges = [_NullabilityEdge(NullabilityNode.always, const [])];
+ var pendingEdges = <_NullabilityEdge>[]
+ ..addAll(_downstream[NullabilityNode.always] ?? const []);
var pendingSubstitutions = <NullabilityNodeForSubstitution>[];
while (true) {
nextEdge:
@@ -135,12 +134,12 @@
continue;
}
for (var source in edge.sources) {
- if (!_nullableNodes.contains(source)) {
+ if (!source.isNullable) {
// Note all sources are nullable, so this edge doesn't apply yet.
continue nextEdge;
}
}
- if (_nullableNodes.add(node)) {
+ if (node.becomeNullable()) {
// Was not previously nullable, so we need to propagate.
pendingEdges.addAll(_downstream[node] ?? const []);
if (node is NullabilityNodeForSubstitution) {
@@ -150,8 +149,7 @@
}
if (pendingSubstitutions.isEmpty) break;
var node = pendingSubstitutions.removeLast();
- if (_nullableNodes.contains(node.innerNode) ||
- _nullableNodes.contains(node.outerNode)) {
+ if (node.innerNode.isNullable || node.outerNode.isNullable) {
// No further propagation is needed, since some other connection already
// propagated nullability to either the inner or outer node.
continue;
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_node.dart b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
index d9ed674..a489dd4 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_node.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
@@ -19,11 +19,12 @@
/// [NullabilityNode] used for types that are known a priori to be nullable
/// (e.g. the type of the `null` literal).
static final NullabilityNode always =
- _NullabilityNodeSimple(ConstraintVariable.always, 'always');
+ _NullabilityNodeImmutable(ConstraintVariable.always, 'always', true);
/// [NullabilityNode] used for types that are known a priori to be
/// non-nullable (e.g. the type of an integer literal).
- static final NullabilityNode never = _NullabilityNodeSimple(null, 'never');
+ static final NullabilityNode never =
+ _NullabilityNodeImmutable(null, 'never', false);
static final _debugNamesInUse = Set<String>();
@@ -40,6 +41,8 @@
String _debugName;
+ bool _isNullable;
+
/// Creates a [NullabilityNode] representing the nullability of a variable
/// whose type is `dynamic` due to type inference.
///
@@ -94,16 +97,17 @@
always ? ConstraintVariable.always : TypeIsNullable(endOffset),
'type($endOffset)');
- NullabilityNode._(this._nullable);
+ NullabilityNode._(this._nullable, {bool initiallyNullable: false})
+ : _isNullable = initiallyNullable;
/// Gets a string that can be appended to a type name during debugging to help
/// annotate the nullability of that type.
String get debugSuffix =>
this == always ? '?' : this == never ? '' : '?($this)';
- /// 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;
+ /// After nullability propagation, this getter can be used to query whether
+ /// the type associated with this node should be considered nullable.
+ bool get isNullable => _isNullable;
/// Indicates whether this node is associated with a named parameter for which
/// nullability migration needs to decide whether it is optional or required.
@@ -117,12 +121,26 @@
String get _debugPrefix;
- /// Verifies that the nullability of this node matches [isNullable].
- void check(bool isNullable) {
- if (isNullable != this.isNullable) {
+ /// After constraint solving, this getter can be used to query whether the
+ /// type associated with this node should be considered nullable.
+ bool get _oldIsNullable => _nullable == null ? false : _nullable.value;
+
+ /// During constraint solving, this method marks the type as nullable, or does
+ /// nothing if the type was already nullable.
+ ///
+ /// Return value indicates whether a change was made.
+ bool becomeNullable() {
+ if (_isNullable) return false;
+ _isNullable = true;
+ return true;
+ }
+
+ /// Verifies that the old and new nullability propagation algorithms match.
+ void check() {
+ if (_oldIsNullable != this.isNullable) {
throw new StateError(
'For $this, new algorithm gives nullability $isNullable; '
- 'old algorithm gives ${this.isNullable}');
+ 'old algorithm gives $_oldIsNullable');
}
}
@@ -298,10 +316,23 @@
String get _debugPrefix => 'Substituted($innerNode, $outerNode)';
}
+class _NullabilityNodeImmutable extends _NullabilityNodeSimple {
+ _NullabilityNodeImmutable(
+ ConstraintVariable nullable, String debugPrefix, bool isNullable)
+ : super(nullable, debugPrefix, initiallyNullable: isNullable);
+
+ @override
+ bool becomeNullable() {
+ if (_isNullable) return false;
+ throw new StateError('Tried to change the nullability of $this');
+ }
+}
+
class _NullabilityNodeSimple extends NullabilityNode {
@override
final String _debugPrefix;
- _NullabilityNodeSimple(ConstraintVariable nullable, this._debugPrefix)
- : super._(nullable);
+ _NullabilityNodeSimple(ConstraintVariable nullable, this._debugPrefix,
+ {bool initiallyNullable: false})
+ : super._(nullable, initiallyNullable: initiallyNullable);
}
