Remove the old constraint variable based nullability migration logic.
Change-Id: I4fe2836160acf5adfd805a327616880f857e9b7a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102387
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Paul Berry <paulberry@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 e2f24d9..5a56ba8 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
@@ -9,7 +9,6 @@
import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/ast/visitor.dart';
@@ -36,9 +35,6 @@
final NullabilityMigrationAssumptions assumptions;
- /// Constraints gathered by the visitor are stored here.
- final Constraints _constraints;
-
final NullabilityGraph _graph;
/// The file being analyzed.
@@ -81,14 +77,8 @@
/// or expression.
bool _inConditionalControlFlow = false;
- ConstraintGatherer(
- TypeProvider typeProvider,
- this._variables,
- this._constraints,
- this._graph,
- this._source,
- this._permissive,
- this.assumptions)
+ ConstraintGatherer(TypeProvider typeProvider, this._variables, this._graph,
+ this._source, this._permissive, this.assumptions)
: _notNullType = DecoratedType(
typeProvider.objectType, NullabilityNode.never, _graph),
_nonNullableBoolType =
@@ -132,8 +122,7 @@
} else {
throw element.runtimeType; // TODO(paulberry)
}
- return decoratedBaseType.substitute(
- _constraints, _graph, substitution, elementType);
+ return decoratedBaseType.substitute(_graph, substitution, elementType);
} else {
return decoratedBaseType;
}
@@ -146,7 +135,7 @@
if (!_inConditionalControlFlow &&
_conditionInfo.trueDemonstratesNonNullIntent != null) {
_conditionInfo.trueDemonstratesNonNullIntent
- ?.recordNonNullIntent(_constraints, _guards, _graph);
+ ?.recordNonNullIntent(_guards, _graph);
}
}
node.message?.accept(this);
@@ -215,8 +204,7 @@
assert(_isSimple(elseType)); // TODO(paulberry)
var overallType = DecoratedType(
node.staticType,
- NullabilityNode.forLUB(
- node, thenType.node, elseType.node, _graph, _joinNullabilities),
+ NullabilityNode.forLUB(node, thenType.node, elseType.node, _graph),
_graph);
_variables.recordDecoratedExpressionType(node, overallType);
return overallType;
@@ -235,9 +223,7 @@
NullabilityNode.recordAssignment(
NullabilityNode.always,
getOrComputeElementType(node.declaredElement).node,
- null,
_guards,
- _constraints,
_graph,
false);
} else {
@@ -360,8 +346,7 @@
// Any parameters not supplied must be optional.
for (var entry in calleeType.namedParameters.entries) {
if (suppliedNamedParameters.contains(entry.key)) continue;
- entry.value.node
- .recordNamedParameterNotSupplied(_constraints, _guards, _graph);
+ entry.value.node.recordNamedParameterNotSupplied(_guards, _graph);
}
return calleeType.returnType;
}
@@ -440,9 +425,7 @@
/// where a nullable source is assigned to a non-nullable destination.
void _checkAssignment(DecoratedType destinationType, DecoratedType sourceType,
Expression expression) {
- CheckExpression checkNotNull;
if (expression != null) {
- checkNotNull = CheckExpression(expression);
_variables.recordExpressionChecks(
_source,
expression,
@@ -450,7 +433,7 @@
expression.end, sourceType.node, destinationType.node, _guards));
}
NullabilityNode.recordAssignment(sourceType.node, destinationType.node,
- checkNotNull, _guards, _constraints, _graph, _inConditionalControlFlow);
+ _guards, _graph, _inConditionalControlFlow);
// TODO(paulberry): it's a cheat to pass in expression=null for the
// recursive checks. Really we want to unify all the checks in a single
// ExpressionChecks object.
@@ -510,23 +493,6 @@
if ((type.type as InterfaceType).typeParameters.isNotEmpty) return false;
return true;
}
-
- /// Creates a constraint variable (if necessary) representing the nullability
- /// of [node], which is the disjunction of the nullabilities [a] and [b].
- ConstraintVariable _joinNullabilities(
- Expression node, ConstraintVariable a, ConstraintVariable b) {
- if (a == null) return b;
- if (b == null) return a;
- if (identical(a, ConstraintVariable.always) ||
- identical(b, ConstraintVariable.always)) {
- return ConstraintVariable.always;
- }
- var result = TypeIsNullable(node.offset);
- _constraints.record([a], result);
- _constraints.record([b], result);
- _constraints.record([result], ConstraintVariable.or(_constraints, a, b));
- return result;
- }
}
/// Information about a binary expression whose boolean value could possibly
diff --git a/pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart b/pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart
index c7d0d5f..1db17fa 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart
@@ -8,7 +8,6 @@
import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
@@ -57,7 +56,7 @@
? new DecoratedType(
DynamicTypeImpl.instance,
NullabilityNode.forInferredDynamicType(
- _graph, _variables.constraints, enclosingNode.offset),
+ _graph, enclosingNode.offset),
_graph)
: type.accept(this);
}
@@ -116,7 +115,6 @@
DecoratedType visitSimpleFormalParameter(SimpleFormalParameter node) {
var type = decorateType(node.type, node);
var declaredElement = node.declaredElement;
- type.node.trackNonNullIntent(node.offset);
_variables.recordDecoratedElementType(declaredElement, type);
if (declaredElement.isNamed) {
_currentFunctionType.namedParameters[declaredElement.name] = type;
@@ -193,10 +191,6 @@
/// ([ConstraintVariableGatherer], which finds all the variables that need to be
/// constrained).
abstract class VariableRecorder {
- /// Gets the [Constraints] object currently in use. Note: this will go away
- /// when we stop using constraint variables for migration.
- Constraints get constraints;
-
/// Associates decorated type information with the given [element].
void recordDecoratedElementType(Element element, DecoratedType type);
diff --git a/pkg/analysis_server/lib/src/nullability/decorated_type.dart b/pkg/analysis_server/lib/src/nullability/decorated_type.dart
index a926ba0..4ffa024 100644
--- a/pkg/analysis_server/lib/src/nullability/decorated_type.dart
+++ b/pkg/analysis_server/lib/src/nullability/decorated_type.dart
@@ -5,7 +5,6 @@
import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/src/dart/element/type.dart';
@@ -86,12 +85,11 @@
/// [undecoratedResult] is the result of the substitution, as determined by
/// the normal type system.
DecoratedType substitute(
- Constraints constraints,
NullabilityGraph graph,
Map<TypeParameterElement, DecoratedType> substitution,
DartType undecoratedResult) {
if (substitution.isEmpty) return this;
- return _substitute(constraints, graph, substitution, undecoratedResult);
+ return _substitute(graph, substitution, undecoratedResult);
}
@override
@@ -122,7 +120,6 @@
/// Internal implementation of [_substitute], used as a recursion target.
DecoratedType _substitute(
- Constraints constraints,
NullabilityGraph graph,
Map<TypeParameterElement, DecoratedType> substitution,
DartType undecoratedResult) {
@@ -137,19 +134,17 @@
? undecoratedResult.normalParameterTypes[i]
: undecoratedResult
.optionalParameterTypes[i - numRequiredParameters];
- newPositionalParameters.add(positionalParameters[i]._substitute(
- constraints, graph, substitution, undecoratedParameterType));
+ newPositionalParameters.add(positionalParameters[i]
+ ._substitute(graph, substitution, undecoratedParameterType));
}
return DecoratedType(undecoratedResult, node, graph,
returnType: returnType._substitute(
- constraints, graph, substitution, undecoratedResult.returnType),
+ graph, substitution, undecoratedResult.returnType),
positionalParameters: newPositionalParameters);
} else if (type is TypeParameterType) {
var inner = substitution[type.element];
- return DecoratedType(
- undecoratedResult,
- NullabilityNode.forSubstitution(constraints, inner?.node, node),
- graph);
+ return DecoratedType(undecoratedResult,
+ NullabilityNode.forSubstitution(inner?.node, node), graph);
} else if (type is VoidType) {
return this;
}
@@ -178,25 +173,3 @@
Iterable<SourceEdit> get modifications =>
isEmpty ? [] : [SourceEdit(_offset, 0, '?')];
}
-
-/// Type of a [ConstraintVariable] representing the fact that a type is intended
-/// to be non-null.
-class NonNullIntent extends ConstraintVariable {
- final int _offset;
-
- NonNullIntent(this._offset);
-
- @override
- toString() => 'nonNullIntent($_offset)';
-}
-
-/// Type of a [ConstraintVariable] representing the fact that a type is
-/// nullable.
-class TypeIsNullable extends ConstraintVariable {
- final int _offset;
-
- TypeIsNullable(this._offset);
-
- @override
- toString() => 'nullable($_offset)';
-}
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_graph.dart b/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
index 0541ed0..fa7d16c 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
@@ -29,22 +29,6 @@
final _nonNullIntentNodes = Set<NullabilityNode>.identity();
- /// Verifies that the conclusions reached by [propagate] match the conclusions
- /// reached by the old nullability propagation algorithm (using constraint
- /// variables).
- void check() {
- try {
- var allNodes = _downstream.keys.toSet();
- allNodes.addAll(_upstream.keys);
- for (var node in allNodes) {
- node.check();
- }
- } catch (_) {
- debugDump();
- rethrow;
- }
- }
-
/// Records that [sourceNode] is immediately upstream from [destinationNode].
void connect(NullabilityNode sourceNode, NullabilityNode destinationNode,
{bool unconditional: false, List<NullabilityNode> guards: const []}) {
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_node.dart b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
index a489dd4..70c0c89 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_node.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
@@ -2,10 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-import 'package:analysis_server/src/nullability/decorated_type.dart';
import 'package:analysis_server/src/nullability/nullability_graph.dart';
-import 'package:analysis_server/src/nullability/transitional_api.dart';
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:meta/meta.dart';
@@ -19,24 +16,15 @@
/// [NullabilityNode] used for types that are known a priori to be nullable
/// (e.g. the type of the `null` literal).
static final NullabilityNode always =
- _NullabilityNodeImmutable(ConstraintVariable.always, 'always', true);
+ _NullabilityNodeImmutable('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 =
- _NullabilityNodeImmutable(null, 'never', false);
+ _NullabilityNodeImmutable('never', false);
static final _debugNamesInUse = Set<String>();
- /// [ConstraintVariable] whose value will be set to `true` if this type needs
- /// to be nullable.
- ///
- /// If `null`, that means that an external constraint (outside the code being
- /// migrated) forces this type to be non-nullable.
- final ConstraintVariable _nullable;
-
- ConstraintVariable _nonNullIntent;
-
bool _isPossiblyOptional = false;
String _debugName;
@@ -49,10 +37,8 @@
/// TODO(paulberry): this should go away; we should decorate the actual
/// inferred type rather than assuming `dynamic`.
factory NullabilityNode.forInferredDynamicType(
- NullabilityGraph graph, Constraints constraints, int offset) {
- var node = _NullabilityNodeSimple(
- TypeIsNullable(null), 'inferredDynamic($offset)');
- constraints.record([], node._nullable);
+ NullabilityGraph graph, int offset) {
+ var node = _NullabilityNodeSimple('inferredDynamic($offset)');
graph.connect(NullabilityNode.always, node);
return node;
}
@@ -68,10 +54,7 @@
Expression conditionalExpression,
NullabilityNode a,
NullabilityNode b,
- NullabilityGraph graph,
- ConstraintVariable Function(
- Expression, ConstraintVariable, ConstraintVariable)
- joinNullabilities) = NullabilityNodeForLUB._;
+ NullabilityGraph graph) = NullabilityNodeForLUB._;
/// Creates a [NullabilityNode] representing the nullability of a type
/// substitution where [outerNode] is the nullability node for the type
@@ -85,19 +68,16 @@
/// TODO(paulberry): this should become unnecessary once constraint solving is
/// performed directly using [NullabilityNode] objects.
factory NullabilityNode.forSubstitution(
- Constraints constraints,
- NullabilityNode innerNode,
- NullabilityNode outerNode) = NullabilityNodeForSubstitution._;
+ NullabilityNode innerNode, NullabilityNode outerNode) =
+ NullabilityNodeForSubstitution._;
/// Creates a [NullabilityNode] representing the nullability of a type
/// annotation appearing explicitly in the user's program.
factory NullabilityNode.forTypeAnnotation(int endOffset,
{@required bool always}) =>
- _NullabilityNodeSimple(
- always ? ConstraintVariable.always : TypeIsNullable(endOffset),
- 'type($endOffset)');
+ _NullabilityNodeSimple('type($endOffset)');
- NullabilityNode._(this._nullable, {bool initiallyNullable: false})
+ NullabilityNode._({bool initiallyNullable: false})
: _isNullable = initiallyNullable;
/// Gets a string that can be appended to a type name during debugging to help
@@ -113,18 +93,8 @@
/// nullability migration needs to decide whether it is optional or required.
bool get isPossiblyOptional => _isPossiblyOptional;
- /// [ConstraintVariable] whose value will be set to `true` if the usage of
- /// this type suggests that it is intended to be non-null (because of the
- /// presence of a statement or expression that would unconditionally lead to
- /// an exception being thrown in the case of a `null` value at runtime).
- ConstraintVariable get nonNullIntent => _nonNullIntent;
-
String get _debugPrefix;
- /// 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.
///
@@ -135,29 +105,18 @@
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 $_oldIsNullable');
- }
- }
-
/// Records the fact that an invocation was made to a function with named
/// parameters, and the named parameter associated with this node was not
/// supplied.
- void recordNamedParameterNotSupplied(Constraints constraints,
+ void recordNamedParameterNotSupplied(
List<NullabilityNode> guards, NullabilityGraph graph) {
if (isPossiblyOptional) {
- _recordConstraints(constraints, guards, const [], _nullable);
graph.connect(NullabilityNode.always, this, guards: guards);
}
}
- void recordNonNullIntent(Constraints constraints,
+ void recordNonNullIntent(
List<NullabilityNode> guards, NullabilityGraph graph) {
- _recordConstraints(constraints, guards, const [], nonNullIntent);
graph.connect(this, NullabilityNode.never, unconditional: true);
}
@@ -179,17 +138,6 @@
return _debugName;
}
- /// Tracks that the possibility that this nullability node might demonstrate
- /// non-null intent, based on the fact that it corresponds to a formal
- /// parameter declaration at location [offset].
- ///
- /// TODO(paulberry): consider eliminating this method altogether, and simply
- /// allowing all nullability nodes to track non-null intent if necessary.
- void trackNonNullIntent(int offset) {
- assert(_nonNullIntent == null);
- _nonNullIntent = NonNullIntent(offset);
- }
-
/// Tracks the possibility that this node is associated with a named parameter
/// for which nullability migration needs to decide whether it is optional or
/// required.
@@ -210,58 +158,11 @@
static void recordAssignment(
NullabilityNode sourceNode,
NullabilityNode destinationNode,
- CheckExpression checkNotNull,
List<NullabilityNode> guards,
- Constraints constraints,
NullabilityGraph graph,
bool inConditionalControlFlow) {
- var additionalConditions = <ConstraintVariable>[];
graph.connect(sourceNode, destinationNode,
guards: guards, unconditional: !inConditionalControlFlow);
- if (sourceNode != NullabilityNode.never) {
- additionalConditions.add(sourceNode._nullable);
- var destinationNonNullIntent = destinationNode.nonNullIntent;
- // nullable_src => nullable_dst | check_expr
- _recordConstraints(
- constraints,
- guards,
- additionalConditions,
- ConstraintVariable.or(
- constraints, destinationNode._nullable, checkNotNull));
- if (checkNotNull != null) {
- // nullable_src & nonNullIntent_dst => check_expr
- if (destinationNonNullIntent != null) {
- additionalConditions.add(destinationNonNullIntent);
- _recordConstraints(
- constraints, guards, additionalConditions, checkNotNull);
- }
- }
- additionalConditions.clear();
- var sourceNonNullIntent = sourceNode.nonNullIntent;
- if (!inConditionalControlFlow && sourceNonNullIntent != null) {
- if (destinationNode._nullable == null) {
- // The destination type can never be nullable so this demonstrates
- // non-null intent.
- _recordConstraints(
- constraints, guards, additionalConditions, sourceNonNullIntent);
- } else if (destinationNonNullIntent != null) {
- // Propagate non-null intent from the destination to the source.
- additionalConditions.add(destinationNonNullIntent);
- _recordConstraints(
- constraints, guards, additionalConditions, sourceNonNullIntent);
- }
- }
- }
- }
-
- static void _recordConstraints(
- Constraints constraints,
- List<NullabilityNode> guards,
- List<ConstraintVariable> additionalConditions,
- ConstraintVariable consequence) {
- var conditions = guards.map((node) => node._nullable).toList();
- conditions.addAll(additionalConditions);
- constraints.record(conditions, consequence);
}
}
@@ -273,15 +174,8 @@
final NullabilityNode right;
NullabilityNodeForLUB._(
- Expression expression,
- this.left,
- this.right,
- NullabilityGraph graph,
- ConstraintVariable Function(
- ConditionalExpression, ConstraintVariable, ConstraintVariable)
- joinNullabilities)
- : super._(
- joinNullabilities(expression, left._nullable, right._nullable)) {
+ Expression expression, this.left, this.right, NullabilityGraph graph)
+ : super._() {
graph.connect(left, this);
graph.connect(right, this);
}
@@ -307,19 +201,15 @@
/// `*` in `T*`.
final NullabilityNode outerNode;
- NullabilityNodeForSubstitution._(
- Constraints constraints, this.innerNode, this.outerNode)
- : super._(ConstraintVariable.or(
- constraints, innerNode?._nullable, outerNode._nullable));
+ NullabilityNodeForSubstitution._(this.innerNode, this.outerNode) : super._();
@override
String get _debugPrefix => 'Substituted($innerNode, $outerNode)';
}
class _NullabilityNodeImmutable extends _NullabilityNodeSimple {
- _NullabilityNodeImmutable(
- ConstraintVariable nullable, String debugPrefix, bool isNullable)
- : super(nullable, debugPrefix, initiallyNullable: isNullable);
+ _NullabilityNodeImmutable(String debugPrefix, bool isNullable)
+ : super(debugPrefix, initiallyNullable: isNullable);
@override
bool becomeNullable() {
@@ -332,7 +222,6 @@
@override
final String _debugPrefix;
- _NullabilityNodeSimple(ConstraintVariable nullable, this._debugPrefix,
- {bool initiallyNullable: false})
- : super._(nullable, initiallyNullable: initiallyNullable);
+ _NullabilityNodeSimple(this._debugPrefix, {bool initiallyNullable: false})
+ : super._(initiallyNullable: initiallyNullable);
}
diff --git a/pkg/analysis_server/lib/src/nullability/transitional_api.dart b/pkg/analysis_server/lib/src/nullability/transitional_api.dart
index ab436bd..261a03d 100644
--- a/pkg/analysis_server/lib/src/nullability/transitional_api.dart
+++ b/pkg/analysis_server/lib/src/nullability/transitional_api.dart
@@ -9,23 +9,12 @@
import 'package:analysis_server/src/nullability/expression_checks.dart';
import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/generated/resolver.dart';
import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer_plugin/protocol/protocol_common.dart' show SourceEdit;
-/// Type of a [ConstraintVariable] representing the addition of a null check.
-class CheckExpression extends ConstraintVariable {
- final int offset;
-
- CheckExpression(Expression expression) : offset = expression.end;
-
- @override
- toString() => 'checkNotNull($offset)';
-}
-
/// Records information about how a conditional expression or statement might
/// need to be modified.
class ConditionalModification extends PotentialModification {
@@ -148,8 +137,6 @@
final Variables _variables;
- final Solver _constraints;
-
final NullabilityGraph _graph;
/// Prepares to perform nullability migration.
@@ -162,16 +149,13 @@
{bool permissive: false,
NullabilityMigrationAssumptions assumptions:
const NullabilityMigrationAssumptions()})
- : this._(permissive, assumptions, NullabilityGraph(), Solver());
+ : this._(permissive, assumptions, NullabilityGraph());
- NullabilityMigration._(
- this._permissive, this.assumptions, this._graph, this._constraints)
- : _variables = Variables(_graph, _constraints);
+ NullabilityMigration._(this._permissive, this.assumptions, this._graph)
+ : _variables = Variables(_graph);
Map<Source, List<PotentialModification>> finish() {
- _constraints.applyHeuristics();
_graph.propagate();
- _graph.check();
return _variables.getPotentialModifications();
}
@@ -181,8 +165,8 @@
}
void processInput(CompilationUnit unit, TypeProvider typeProvider) {
- unit.accept(ConstraintGatherer(typeProvider, _variables, _constraints,
- _graph, unit.declaredElement.source, _permissive, assumptions));
+ unit.accept(ConstraintGatherer(typeProvider, _variables, _graph,
+ unit.declaredElement.source, _permissive, assumptions));
}
}
@@ -278,10 +262,7 @@
final NullabilityGraph _graph;
- @override
- final Constraints constraints;
-
- Variables(this._graph, this.constraints);
+ Variables(this._graph);
@override
DecoratedType decoratedElementType(Element element, {bool create: false}) =>
diff --git a/pkg/analysis_server/lib/src/nullability/unit_propagation.dart b/pkg/analysis_server/lib/src/nullability/unit_propagation.dart
deleted file mode 100644
index b587e33..0000000
--- a/pkg/analysis_server/lib/src/nullability/unit_propagation.dart
+++ /dev/null
@@ -1,187 +0,0 @@
-// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-/// Repository of constraints corresponding to the code being migrated.
-///
-/// This data structure carries information from the second pass of migration
-/// ([ConstraintGatherer]) to the third (which creates individual code
-/// modifications from each constraint).
-abstract class Constraints {
- /// Records a new constraint equation.
- void record(
- Iterable<ConstraintVariable> conditions, ConstraintVariable consequence);
-}
-
-/// Representation of a single boolean variable in the constraint solver.
-class ConstraintVariable {
- /// A special boolean variable whose value is known to be `true`.
- static final ConstraintVariable always = _Always();
-
- /// A list of all constraints containing this variable on their left hand side
- /// that may not have been satisfied yet.
- final _dependencies = <_Clause>[];
-
- /// The value assigned to this constraint variable by the solution currently
- /// being computed.
- bool _value = false;
-
- /// If this variable represents a disjunction ("or") of several other
- /// variables, the variables in the disjunction. Otherwise a singleton list
- /// containing `this`.
- final List<ConstraintVariable> _disjunctionParts;
-
- ConstraintVariable() : _disjunctionParts = List.filled(1, null) {
- _disjunctionParts[0] = this;
- }
-
- /// Creates a [ConstraintVariable] representing a disjunction ("or") of
- /// several other variables.
- ///
- /// Additional constraints will be recorded in [constraints] to ensure that
- /// the solution will be consistent.
- factory ConstraintVariable.or(
- Constraints constraints, ConstraintVariable a, ConstraintVariable b) {
- if (a == null) return b;
- if (b == null) return a;
- var parts = a.disjunctionParts.toList();
- parts.addAll(b.disjunctionParts);
- assert(parts.length > 1);
- var result = ConstraintVariable._(parts);
- constraints.record([a], result);
- constraints.record([b], result);
- return result;
- }
-
- ConstraintVariable._(this._disjunctionParts);
-
- /// If this variable represents a disjunction ("or") of several other
- /// variables, the variables in the disjunction. Otherwise a singleton list
- /// containing `this`.
- Iterable<ConstraintVariable> get disjunctionParts => _disjunctionParts;
-
- /// Indicates whether this variable represents a disjunction ("or") of several
- /// other variables.
- bool get isDisjunction => _disjunctionParts.length > 1;
-
- /// The value assigned to this constraint variable by the solution currently
- /// being computed.
- bool get value => _value;
-
- @override
- String toString() =>
- isDisjunction ? '(${_disjunctionParts.join(' | ')})' : super.toString();
-}
-
-/// The core of the migration tool's constraint solver. This class implements
-/// unit propagation (see https://en.wikipedia.org/wiki/Unit_propagation),
-/// extended to support disjunctions.
-///
-/// The extension works approximately as follows: first we perform ordinary unit
-/// propagation, accumulating a list of any disjunction variables that need to
-/// be assigned a value of `true`. Once this finishes, we heuristically choose
-/// one of these disjunction variables and ensure that it is assigned a value of
-/// `true` by setting one of its constituent variables to `true` and propagating
-/// again. Once all disjunctions have been resolved, we have a final solution.
-class Solver extends Constraints {
- /// Clauses that should be evaluated as part of ordinary unit propagation.
- final _pending = <_Clause>[];
-
- /// Disjunction variables that have been determined by unit propagation to be
- /// `true`, but for which we have not yet propagated the `true` value to one
- /// of the constituent variables.
- final _pendingDisjunctions = <ConstraintVariable>[];
-
- /// Heuristically resolves any pending disjunctions.
- void applyHeuristics() {
- while (_pendingDisjunctions.isNotEmpty) {
- var disjunction = _pendingDisjunctions.removeLast();
- if (disjunction.disjunctionParts.any((v) => v.value)) continue;
- // TODO(paulberry): smarter heuristics
- var choice = disjunction.disjunctionParts.first;
- record([], choice);
- }
- }
-
- @override
- void record(Iterable<ConstraintVariable> conditions,
- covariant ConstraintVariable consequence) {
- var _conditions = List<ConstraintVariable>.from(conditions);
- var clause = _Clause(_conditions, consequence);
- int i = 0;
- while (i < _conditions.length) {
- ConstraintVariable variable = _conditions[i];
- if (variable._value) {
- int j = _conditions.length - 1;
- _conditions[i] = _conditions[j];
- _conditions.removeLast();
- continue;
- }
- variable._dependencies.add(clause);
- i++;
- }
- if (i == 0) {
- if (!consequence._value) {
- consequence._value = true;
- if (consequence.isDisjunction) {
- _pendingDisjunctions.add(consequence);
- }
- _pending.addAll(consequence._dependencies);
- _propagate();
- }
- } else {
- consequence._dependencies.add(clause);
- }
- }
-
- /// Performs ordinary unit propagation, recording any disjunctions encountered
- /// in [_pendingDisjunctions].
- void _propagate() {
- while (_pending.isNotEmpty) {
- var clause = _pending.removeLast();
- var conditions = clause.conditions;
- int i = 0;
- while (i < conditions.length) {
- ConstraintVariable variable = conditions[i];
- if (variable._value) {
- int j = conditions.length - 1;
- conditions[i] = conditions[j];
- conditions.removeLast();
- continue;
- }
- i++;
- }
- if (i == 0) {
- var consequence = clause.consequence;
- if (!consequence._value) {
- consequence._value = true;
- if (consequence.isDisjunction) {
- _pendingDisjunctions.add(consequence);
- }
- _pending.addAll(consequence._dependencies);
- }
- }
- }
- }
-}
-
-/// The special singleton [ConstraintVariable] whose value is always `true`.
-class _Always extends ConstraintVariable {
- _Always() {
- _value = true;
- }
-
- @override
- String toString() => 'always';
-}
-
-/// A single equation in a system of constraints.
-class _Clause {
- /// The conditions on the left hand side of the equation.
- final List<ConstraintVariable> conditions;
-
- /// The single variable on the right hand side of the equation.
- final ConstraintVariable consequence;
-
- _Clause(this.conditions, this.consequence);
-}
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 51ae0e6..1255b40 100644
--- a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
+++ b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
@@ -10,7 +10,6 @@
import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/src/generated/resolver.dart';
import 'package:analyzer/src/generated/source.dart';
@@ -42,32 +41,13 @@
expect(graph.getDownstreamNodes(sourceNode), contains(destinationNode));
}
- /// Checks that a constraint was recorded with a left hand side of
- /// [conditions] and a right hand side of [consequence].
- void assertConstraint(
- Iterable<ConstraintVariable> conditions, ConstraintVariable consequence) {
- expect(constraints._clauses,
- contains(_Clause(conditions.toSet(), consequence)));
- }
-
- /// Checks that no constraint was recorded with a right hand side of
- /// [consequence].
- void assertNoConstraints(ConstraintVariable consequence) {
- expect(
- constraints._clauses,
- isNot(contains(
- predicate((_Clause clause) => clause.consequence == consequence))));
- }
-
void assertNonNullIntent(NullabilityNode node, bool expected) {
if (expected) {
expect(graph.getUnconditionalUpstreamNodes(NullabilityNode.never),
contains(node));
- assertConstraint([], node.nonNullIntent);
} else {
expect(graph.getUnconditionalUpstreamNodes(NullabilityNode.never),
isNot(contains(node)));
- assertNoConstraints(node.nonNullIntent);
}
}
@@ -122,10 +102,7 @@
test_always() async {
await analyze('');
- // No clause is needed for `always`; it is assigned the value `true` before
- // solving begins.
- assertNoConstraints(ConstraintVariable.always);
- assert(ConstraintVariable.always.value, isTrue);
+ expect(NullabilityNode.always.isNullable, isTrue);
}
test_assert_demonstrates_non_null_intent() async {
@@ -445,7 +422,8 @@
var int_2 = decoratedTypeAnnotation('int/*2*/');
var i_3 = checkExpression('i/*3*/');
assertNullCheck(i_3, int_2.node, contextNode: int_1.node);
- assertConstraint([int_1.node.nonNullIntent], int_2.node.nonNullIntent);
+ expect(
+ graph.getUnconditionalUpstreamNodes(int_1.node), contains(int_2.node));
}
test_functionInvocation_parameter_named() async {
@@ -720,6 +698,12 @@
assertNonNullIntent(decoratedTypeAnnotation('C c').node, true);
}
+ test_never() async {
+ await analyze('');
+
+ expect(NullabilityNode.never.isNullable, isFalse);
+ }
+
test_parenthesizedExpression() async {
await analyze('''
int f() {
@@ -801,8 +785,8 @@
{NullabilityMigrationAssumptions assumptions:
const NullabilityMigrationAssumptions()}) async {
var unit = await super.analyze(code);
- unit.accept(ConstraintGatherer(typeProvider, _variables, constraints, graph,
- testSource, false, assumptions));
+ unit.accept(ConstraintGatherer(
+ typeProvider, _variables, graph, testSource, false, assumptions));
return unit;
}
}
@@ -903,7 +887,6 @@
same(decoratedType));
expect(decoratedType.node, isNotNull);
expect(decoratedType.node, isNot(NullabilityNode.never));
- expect(decoratedType.node.nonNullIntent, isNotNull);
}
test_topLevelFunction_returnType_implicit_dynamic() async {
@@ -934,12 +917,9 @@
final NullabilityGraph graph;
- final _Constraints constraints;
+ MigrationVisitorTestBase() : this._(NullabilityGraph());
- MigrationVisitorTestBase() : this._(NullabilityGraph(), _Constraints());
-
- MigrationVisitorTestBase._(this.graph, this.constraints)
- : _variables = _Variables(graph, constraints);
+ MigrationVisitorTestBase._(this.graph) : _variables = _Variables(graph);
TypeProvider get typeProvider => testAnalysisResult.typeProvider;
@@ -971,52 +951,6 @@
}
}
-/// Mock representation of a constraint equation that is not connected to a
-/// constraint solver. We use this to confirm that analysis produces the
-/// correct constraint equations.
-///
-/// [hashCode] and equality are implemented using [toString] for simplicity.
-class _Clause {
- final Set<ConstraintVariable> conditions;
- final ConstraintVariable consequence;
-
- _Clause(this.conditions, this.consequence);
-
- @override
- int get hashCode => toString().hashCode;
-
- @override
- bool operator ==(Object other) =>
- other is _Clause && toString() == other.toString();
-
- @override
- String toString() {
- String lhs;
- if (conditions.isNotEmpty) {
- var sortedConditionStrings = conditions.map((v) => v.toString()).toList()
- ..sort();
- lhs = sortedConditionStrings.join(' & ') + ' => ';
- } else {
- lhs = '';
- }
- String rhs = consequence.toString();
- return lhs + rhs;
- }
-}
-
-/// Mock representation of a constraint solver that does not actually do any
-/// solving. We use this to confirm that analysis produced the correct
-/// constraint equations.
-class _Constraints extends Constraints {
- final _clauses = <_Clause>[];
-
- @override
- void record(
- Iterable<ConstraintVariable> conditions, ConstraintVariable consequence) {
- _clauses.add(_Clause(conditions.toSet(), consequence));
- }
-}
-
/// Mock representation of constraint variables.
class _Variables extends Variables {
final _conditionalDiscard = <AstNode, ConditionalDiscard>{};
@@ -1029,8 +963,7 @@
final _possiblyOptional = <DefaultFormalParameter, NullabilityNode>{};
- _Variables(NullabilityGraph graph, Constraints constraints)
- : super(graph, constraints);
+ _Variables(NullabilityGraph graph) : super(graph);
/// Gets the [ExpressionChecks] associated with the given [expression].
ExpressionChecks checkExpression(Expression expression) =>
diff --git a/pkg/analysis_server/test/src/nullability/test_all.dart b/pkg/analysis_server/test/src/nullability/test_all.dart
index 77b5187..c43a34f 100644
--- a/pkg/analysis_server/test/src/nullability/test_all.dart
+++ b/pkg/analysis_server/test/src/nullability/test_all.dart
@@ -6,12 +6,10 @@
import 'migration_visitor_test.dart' as migration_visitor_test;
import 'provisional_api_test.dart' as provisional_api_test;
-import 'unit_propagation_test.dart' as unit_propagation_test;
main() {
defineReflectiveSuite(() {
migration_visitor_test.main();
provisional_api_test.main();
- unit_propagation_test.main();
});
}
diff --git a/pkg/analysis_server/test/src/nullability/unit_propagation_test.dart b/pkg/analysis_server/test/src/nullability/unit_propagation_test.dart
deleted file mode 100644
index d2fcf89..0000000
--- a/pkg/analysis_server/test/src/nullability/unit_propagation_test.dart
+++ /dev/null
@@ -1,57 +0,0 @@
-// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-import 'package:analysis_server/src/nullability/unit_propagation.dart';
-import 'package:test/test.dart';
-import 'package:test_reflective_loader/test_reflective_loader.dart';
-
-main() {
- defineReflectiveSuite(() {
- defineReflectiveTests(UnitPropagationTest);
- });
-}
-
-/// TODO(paulberry): write more tests
-@reflectiveTest
-class UnitPropagationTest {
- var solver = Solver();
-
- ConstraintVariable newVar(String name) => _NamedConstraintVariable(name);
-
- test_record_copies_conditions() {
- var a = newVar('a');
- var b = newVar('b');
- var conditions = [a];
- solver.record(conditions, b);
- conditions.removeLast();
- expect(a.value, false);
- expect(b.value, false);
- solver.record([], a);
- expect(a.value, true);
- expect(b.value, true);
- }
-
- test_record_propagates_true_variables_immediately() {
- var a = newVar('a');
- expect(a.value, false);
- solver.record([], a);
- expect(a.value, true);
- var b = newVar('b');
- expect(b.value, false);
- solver.record([a], b);
- expect(b.value, true);
- }
-}
-
-/// Representation of a constraint variable with a specified name.
-///
-/// This makes test failures easier to comprehend.
-class _NamedConstraintVariable extends ConstraintVariable {
- final String _name;
-
- _NamedConstraintVariable(this._name);
-
- @override
- String toString() => _name;
-}
