Stop using ConstraintVariable.always to represent the nullability of `dynamic`.
This is a necessary part of reworking nullability migration to stop
using constraint variables entirely.
Change-Id: If6c347c16882edfc2e8bdbeac761e09d8256c9bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101622
Reviewed-by: Brian Wilkerson <brianwilkerson@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 c6d9e80..c30ef23 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
@@ -89,14 +89,14 @@
this._source,
this._permissive,
this.assumptions)
- : _notNullType =
- DecoratedType(typeProvider.objectType, NullabilityNode.never),
+ : _notNullType = DecoratedType(
+ typeProvider.objectType, NullabilityNode.never, _graph),
_nonNullableBoolType =
- DecoratedType(typeProvider.boolType, NullabilityNode.never),
+ DecoratedType(typeProvider.boolType, NullabilityNode.never, _graph),
_nonNullableTypeType =
- DecoratedType(typeProvider.typeType, NullabilityNode.never),
- _nullType =
- DecoratedType(typeProvider.nullType, NullabilityNode.always);
+ DecoratedType(typeProvider.typeType, NullabilityNode.never, _graph),
+ _nullType = DecoratedType(
+ typeProvider.nullType, NullabilityNode.always, _graph);
/// Gets the decorated type of [element] from [_variables], performing any
/// necessary substitutions.
@@ -133,7 +133,7 @@
throw element.runtimeType; // TODO(paulberry)
}
return decoratedBaseType.substitute(
- _constraints, substitution, elementType);
+ _constraints, _graph, substitution, elementType);
} else {
return decoratedBaseType;
}
@@ -196,7 +196,7 @@
@override
DecoratedType visitBooleanLiteral(BooleanLiteral node) {
- return DecoratedType(node.staticType, NullabilityNode.never);
+ return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
@@ -216,7 +216,8 @@
var overallType = DecoratedType(
node.staticType,
NullabilityNode.forLUB(
- node, thenType.node, elseType.node, _graph, _joinNullabilities));
+ node, thenType.node, elseType.node, _graph, _joinNullabilities),
+ _graph);
_variables.recordDecoratedExpressionType(node, overallType);
return overallType;
}
@@ -311,7 +312,7 @@
@override
DecoratedType visitIntegerLiteral(IntegerLiteral node) {
- return DecoratedType(node.staticType, NullabilityNode.never);
+ return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
@@ -412,24 +413,24 @@
@override
DecoratedType visitStringLiteral(StringLiteral node) {
- return DecoratedType(node.staticType, NullabilityNode.never);
+ return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
DecoratedType visitThisExpression(ThisExpression node) {
- return DecoratedType(node.staticType, NullabilityNode.never);
+ return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
DecoratedType visitThrowExpression(ThrowExpression node) {
node.expression.accept(this);
// TODO(paulberry): do we need to check the expression type? I think not.
- return DecoratedType(node.staticType, NullabilityNode.never);
+ return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
DecoratedType visitTypeName(TypeName typeName) {
- return DecoratedType(typeName.type, NullabilityNode.never);
+ return DecoratedType(typeName.type, NullabilityNode.never, _graph);
}
/// Creates the necessary constraint(s) for an assignment from [sourceType] to
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 adbae5e..51ae58c 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart
@@ -5,6 +5,7 @@
import 'package:analysis_server/src/nullability/conditional_discard.dart';
import 'package:analysis_server/src/nullability/decorated_type.dart';
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/transitional_api.dart';
import 'package:analyzer/dart/ast/ast.dart';
@@ -40,8 +41,10 @@
final NullabilityMigrationAssumptions assumptions;
- ConstraintVariableGatherer(
- this._variables, this._source, this._permissive, this.assumptions);
+ final NullabilityGraph _graph;
+
+ ConstraintVariableGatherer(this._variables, this._source, this._permissive,
+ this.assumptions, this._graph);
/// Creates and stores a [DecoratedType] object corresponding to the given
/// [type] AST, and returns it.
@@ -50,8 +53,8 @@
// TODO(danrubel): Return something other than this
// to indicate that we should insert a type for the declaration
// that is missing a type reference.
- ? new DecoratedType(
- DynamicTypeImpl.instance, NullabilityNode.forInferredDynamicType())
+ ? new DecoratedType(DynamicTypeImpl.instance,
+ NullabilityNode.forInferredDynamicType(_graph), _graph)
: type.accept(this);
}
@@ -125,8 +128,8 @@
assert(node is NamedType); // TODO(paulberry)
var type = node.type;
if (type.isVoid) {
- return DecoratedType(
- type, NullabilityNode.forTypeAnnotation(node.end, always: true));
+ return DecoratedType(type,
+ NullabilityNode.forTypeAnnotation(node.end, always: true), _graph);
}
assert(
type is InterfaceType || type is TypeParameterType); // TODO(paulberry)
@@ -145,6 +148,7 @@
NullabilityNode.forTypeAnnotation(node.end,
always: node.question != null),
node.end,
+ _graph,
typeArguments: typeArguments);
_variables.recordDecoratedTypeAnnotation(_source, node, decoratedType);
return decoratedType;
@@ -161,7 +165,7 @@
// TODO(paulberry): test that it's correct to use `null` for the nullability
// of the function type
var functionType = DecoratedType(
- declaredElement.type, NullabilityNode.never,
+ declaredElement.type, NullabilityNode.never, _graph,
returnType: decoratedReturnType,
positionalParameters: [],
namedParameters: {});
diff --git a/pkg/analysis_server/lib/src/nullability/decorated_type.dart b/pkg/analysis_server/lib/src/nullability/decorated_type.dart
index a1ae73c..a926ba0 100644
--- a/pkg/analysis_server/lib/src/nullability/decorated_type.dart
+++ b/pkg/analysis_server/lib/src/nullability/decorated_type.dart
@@ -2,6 +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/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';
@@ -36,25 +37,27 @@
/// TODO(paulberry): how should we handle generic typedefs?
final List<DecoratedType> typeArguments;
- DecoratedType(this.type, this.node,
+ DecoratedType(this.type, this.node, NullabilityGraph graph,
{this.returnType,
this.positionalParameters = const [],
this.namedParameters = const {},
this.typeArguments = const []}) {
assert(node != null);
// The type system doesn't have a non-nullable version of `dynamic`. So if
- // the type is `dynamic`, verify that `nullable` is `always`.
- assert(!type.isDynamic || node.isAlwaysNullable);
+ // the type is `dynamic`, verify that the node is directly downstream from
+ // Nullability.always.
+ assert(!type.isDynamic ||
+ graph.getUpstreamNodes(node).contains(NullabilityNode.always));
}
/// Creates a [DecoratedType] corresponding to the given [element], which is
/// presumed to have come from code that is already migrated.
- factory DecoratedType.forElement(Element element) {
+ factory DecoratedType.forElement(Element element, NullabilityGraph graph) {
DecoratedType decorate(DartType type) {
assert((type as TypeImpl).nullabilitySuffix ==
NullabilitySuffix.star); // TODO(paulberry)
if (type is FunctionType) {
- var decoratedType = DecoratedType(type, NullabilityNode.never,
+ var decoratedType = DecoratedType(type, NullabilityNode.never, graph,
returnType: decorate(type.returnType), positionalParameters: []);
for (var parameter in type.parameters) {
assert(parameter.isPositional); // TODO(paulberry)
@@ -63,7 +66,7 @@
return decoratedType;
} else if (type is InterfaceType) {
assert(type.typeParameters.isEmpty); // TODO(paulberry)
- return DecoratedType(type, NullabilityNode.never);
+ return DecoratedType(type, NullabilityNode.never, graph);
} else {
throw type.runtimeType; // TODO(paulberry)
}
@@ -84,10 +87,11 @@
/// the normal type system.
DecoratedType substitute(
Constraints constraints,
+ NullabilityGraph graph,
Map<TypeParameterElement, DecoratedType> substitution,
DartType undecoratedResult) {
if (substitution.isEmpty) return this;
- return _substitute(constraints, substitution, undecoratedResult);
+ return _substitute(constraints, graph, substitution, undecoratedResult);
}
@override
@@ -119,6 +123,7 @@
/// Internal implementation of [_substitute], used as a recursion target.
DecoratedType _substitute(
Constraints constraints,
+ NullabilityGraph graph,
Map<TypeParameterElement, DecoratedType> substitution,
DartType undecoratedResult) {
var type = this.type;
@@ -132,17 +137,19 @@
? undecoratedResult.normalParameterTypes[i]
: undecoratedResult
.optionalParameterTypes[i - numRequiredParameters];
- newPositionalParameters.add(positionalParameters[i]
- ._substitute(constraints, substitution, undecoratedParameterType));
+ newPositionalParameters.add(positionalParameters[i]._substitute(
+ constraints, graph, substitution, undecoratedParameterType));
}
- return DecoratedType(undecoratedResult, node,
+ return DecoratedType(undecoratedResult, node, graph,
returnType: returnType._substitute(
- constraints, substitution, undecoratedResult.returnType),
+ constraints, 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));
+ return DecoratedType(
+ undecoratedResult,
+ NullabilityNode.forSubstitution(constraints, inner?.node, node),
+ graph);
} else if (type is VoidType) {
return this;
}
@@ -159,10 +166,10 @@
implements PotentialModification {
final int _offset;
- DecoratedTypeAnnotation(
- DartType type, NullabilityNode nullabilityNode, this._offset,
+ DecoratedTypeAnnotation(DartType type, NullabilityNode nullabilityNode,
+ this._offset, NullabilityGraph graph,
{List<DecoratedType> typeArguments = const []})
- : super(type, nullabilityNode, typeArguments: typeArguments);
+ : super(type, nullabilityNode, graph, typeArguments: typeArguments);
@override
bool get isEmpty => !node.isNullable;
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_node.dart b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
index e871c28..03e1b43 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_node.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
@@ -40,7 +40,11 @@
///
/// TODO(paulberry): this should go away; we should decorate the actual
/// inferred type rather than assuming `dynamic`.
- NullabilityNode.forInferredDynamicType() : this._(ConstraintVariable.always);
+ factory NullabilityNode.forInferredDynamicType(NullabilityGraph graph) {
+ var node = NullabilityNode._(TypeIsNullable(null));
+ graph.connect(NullabilityNode.always, node);
+ return node;
+ }
/// Creates a [NullabilityNode] representing the nullability of an
/// expression which is nullable iff both [a] and [b] are nullable.
diff --git a/pkg/analysis_server/lib/src/nullability/transitional_api.dart b/pkg/analysis_server/lib/src/nullability/transitional_api.dart
index ac1d8df..ffbad45 100644
--- a/pkg/analysis_server/lib/src/nullability/transitional_api.dart
+++ b/pkg/analysis_server/lib/src/nullability/transitional_api.dart
@@ -146,11 +146,11 @@
final NullabilityMigrationAssumptions assumptions;
- final _variables = Variables();
+ final Variables _variables;
final _constraints = Solver();
- final _graph = NullabilityGraph();
+ final NullabilityGraph _graph;
/// Prepares to perform nullability migration.
///
@@ -160,8 +160,12 @@
/// is fully implemented.
NullabilityMigration(
{bool permissive: false,
- this.assumptions: const NullabilityMigrationAssumptions()})
- : _permissive = permissive;
+ NullabilityMigrationAssumptions assumptions:
+ const NullabilityMigrationAssumptions()})
+ : this._(permissive, assumptions, NullabilityGraph());
+
+ NullabilityMigration._(this._permissive, this.assumptions, this._graph)
+ : _variables = Variables(_graph);
Map<Source, List<PotentialModification>> finish() {
_constraints.applyHeuristics();
@@ -169,8 +173,8 @@
}
void prepareInput(CompilationUnit unit) {
- unit.accept(ConstraintVariableGatherer(
- _variables, unit.declaredElement.source, _permissive, assumptions));
+ unit.accept(ConstraintVariableGatherer(_variables,
+ unit.declaredElement.source, _permissive, assumptions, _graph));
}
void processInput(CompilationUnit unit, TypeProvider typeProvider) {
@@ -269,10 +273,14 @@
final _potentialModifications = <Source, List<PotentialModification>>{};
+ final NullabilityGraph _graph;
+
+ Variables(this._graph);
+
@override
DecoratedType decoratedElementType(Element element, {bool create: false}) =>
_decoratedElementTypes[element] ??= create
- ? DecoratedType.forElement(element)
+ ? DecoratedType.forElement(element, _graph)
: throw StateError('No element found');
Map<Source, List<PotentialModification>> getPotentialModifications() =>
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 764f29e..160a53a 100644
--- a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
+++ b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
@@ -32,9 +32,6 @@
@override
final _Constraints constraints = _Constraints();
- @override
- final graph = NullabilityGraph();
-
void assertConditional(
NullabilityNode node, NullabilityNode left, NullabilityNode right) {
var conditionalNode = node as NullabilityNodeForLUB;
@@ -806,8 +803,6 @@
abstract class ConstraintsTestBase extends MigrationVisitorTestBase {
Constraints get constraints;
- NullabilityGraph get graph;
-
/// Analyzes the given source code, producing constraint variables and
/// constraints for it.
@override
@@ -851,7 +846,8 @@
expect(decoratedFunctionType('f').positionalParameters[0],
same(decoratedType));
expect(decoratedType.type.isDynamic, isTrue);
- expect(decoratedType.node.nullable, same(ConstraintVariable.always));
+ expect(graph.getUpstreamNodes(decoratedType.node),
+ contains(NullabilityNode.always));
}
test_topLevelFunction_parameterType_named_no_default() async {
@@ -918,7 +914,8 @@
''');
var decoratedType = decoratedFunctionType('f').returnType;
expect(decoratedType.type.isDynamic, isTrue);
- expect(decoratedType.node.nullable, same(ConstraintVariable.always));
+ expect(graph.getUpstreamNodes(decoratedType.node),
+ contains(NullabilityNode.always));
}
test_topLevelFunction_returnType_simple() async {
@@ -932,18 +929,24 @@
}
class MigrationVisitorTestBase extends AbstractSingleUnitTest {
- final _variables = _Variables();
+ final _Variables _variables;
FindNode findNode;
+ final NullabilityGraph graph;
+
+ MigrationVisitorTestBase() : this._(NullabilityGraph());
+
+ MigrationVisitorTestBase._(this.graph) : _variables = _Variables(graph);
+
TypeProvider get typeProvider => testAnalysisResult.typeProvider;
Future<CompilationUnit> analyze(String code,
{NullabilityMigrationAssumptions assumptions:
const NullabilityMigrationAssumptions()}) async {
await resolveTestUnit(code);
- testUnit.accept(
- ConstraintVariableGatherer(_variables, testSource, false, assumptions));
+ testUnit.accept(ConstraintVariableGatherer(
+ _variables, testSource, false, assumptions, graph));
findNode = FindNode(code, testUnit);
return testUnit;
}
@@ -1024,6 +1027,8 @@
final _possiblyOptional = <DefaultFormalParameter, NullabilityNode>{};
+ _Variables(NullabilityGraph graph) : super(graph);
+
/// Gets the [ExpressionChecks] associated with the given [expression].
ExpressionChecks checkExpression(Expression expression) =>
_expressionChecks[_normalizeExpression(expression)];
