dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Stop using ConstraintVariable.always to represent the nullability of dynamic.

Browse Source 
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>
This commit is contained in:
Paul Berry
2019-05-07 20:15:18 +00:00
committed by commit-bot@chromium.org
parent d06c7b7c89
commit df7db795dc
6 changed files with 84 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
+15 -14
View File
@@ -89,14 +89,14 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
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 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
throw element.runtimeType; // TODO(paulberry)
}
return decoratedBaseType.substitute(
_constraints, substitution, elementType);
_constraints, _graph, substitution, elementType);
} else {
return decoratedBaseType;
}
@@ -196,7 +196,7 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
@override
DecoratedType visitBooleanLiteral(BooleanLiteral node) {
return DecoratedType(node.staticType, NullabilityNode.never);
return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
@@ -216,7 +216,8 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
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 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
@override
DecoratedType visitIntegerLiteral(IntegerLiteral node) {
return DecoratedType(node.staticType, NullabilityNode.never);
return DecoratedType(node.staticType, NullabilityNode.never, _graph);
}
@override
@@ -412,24 +413,24 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
@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
pkg/analysis_server/lib/src/nullability/constraint_variable_gatherer.dart
+11 -7
View File
@@ -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 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor<DecoratedType> {
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 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor<DecoratedType> {
// 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 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor<DecoratedType> {
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 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor<DecoratedType> {
NullabilityNode.forTypeAnnotation(node.end,
always: node.question != null),
node.end,
_graph,
typeArguments: typeArguments);
_variables.recordDecoratedTypeAnnotation(_source, node, decoratedType);
return decoratedType;
@@ -161,7 +165,7 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor<DecoratedType> {
// 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: {});
pkg/analysis_server/lib/src/nullability/decorated_type.dart
+23 -16
View File
@@ -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 @@ class DecoratedType {
/// 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 @@ class DecoratedType {
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 @@ class DecoratedType {
/// 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 @@ class DecoratedType {
/// 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 @@ class DecoratedType {
? 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 @@ class DecoratedTypeAnnotation extends DecoratedType
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;
pkg/analysis_server/lib/src/nullability/nullability_node.dart
+5 -1
View File
@@ -40,7 +40,11 @@ class NullabilityNode {
///
/// 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.
pkg/analysis_server/lib/src/nullability/transitional_api.dart
+15 -7
View File
@@ -146,11 +146,11 @@ class NullabilityMigration {
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 @@ class NullabilityMigration {
/// 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 @@ class NullabilityMigration {
}
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 @@ class Variables implements VariableRecorder, VariableRepository {
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() =>
pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
+15 -10
View File
@@ -32,9 +32,6 @@ class ConstraintGathererTest extends ConstraintsTestBase {
@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 @@ Type f() {
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 @@ void f(x) {}
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 @@ f() {}
''');
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 @@ int f() => 0;
}
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 @@ class _Variables extends Variables {
final _possiblyOptional = <DefaultFormalParameter, NullabilityNode>{};
_Variables(NullabilityGraph graph) : super(graph);
/// Gets the [ExpressionChecks] associated with the given [expression].
ExpressionChecks checkExpression(Expression expression) =>
_expressionChecks[_normalizeExpression(expression)];