diff --git a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart index c6d9e809e91..c30ef232365 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 @@ class ConstraintGatherer extends GeneralizingAstVisitor { 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 { 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 { @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 { 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 { @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 { @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 adbae5ee57e..51ae58cf708 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 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor { 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 { // 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 { 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 { 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 { // 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 a1ae73ca0d2..a926ba07bf6 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 @@ class DecoratedType { /// TODO(paulberry): how should we handle generic typedefs? final List 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 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 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 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 e871c289ccd..03e1b4387a5 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 @@ 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. diff --git a/pkg/analysis_server/lib/src/nullability/transitional_api.dart b/pkg/analysis_server/lib/src/nullability/transitional_api.dart index ac1d8df7a3e..ffbad45802a 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 @@ 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> 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 = >{}; + 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> 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 764f29e5d48..160a53a9ddd 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 @@ 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 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 = {}; + _Variables(NullabilityGraph graph) : super(graph); + /// Gets the [ExpressionChecks] associated with the given [expression]. ExpressionChecks checkExpression(Expression expression) => _expressionChecks[_normalizeExpression(expression)];