Migration: add support for null-aware accesses.
Change-Id: Ie034c0410e667f2e55fd2b9d9e9b2b3b3726de30
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105465
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/nnbd_migration/lib/src/decorated_type.dart b/pkg/nnbd_migration/lib/src/decorated_type.dart
index 580164e..7dd5b69 100644
--- a/pkg/nnbd_migration/lib/src/decorated_type.dart
+++ b/pkg/nnbd_migration/lib/src/decorated_type.dart
@@ -117,6 +117,13 @@
}
}
+ /// Creates a shallow copy of `this`, replacing the nullability node.
+ DecoratedType withNode(NullabilityNode node) => DecoratedType(type, node,
+ returnType: returnType,
+ positionalParameters: positionalParameters,
+ namedParameters: namedParameters,
+ typeArguments: typeArguments);
+
/// Internal implementation of [_substitute], used as a recursion target.
DecoratedType _substitute(
Map<TypeParameterElement, DecoratedType> substitution,
diff --git a/pkg/nnbd_migration/lib/src/graph_builder.dart b/pkg/nnbd_migration/lib/src/graph_builder.dart
index 85ef317..0bd0aa8 100644
--- a/pkg/nnbd_migration/lib/src/graph_builder.dart
+++ b/pkg/nnbd_migration/lib/src/graph_builder.dart
@@ -74,6 +74,8 @@
/// or expression.
bool _inConditionalControlFlow = false;
+ NullabilityNode _lastConditionalNode;
+
GraphBuilder(TypeProvider typeProvider, this._variables, this._graph,
this._source, this.listener)
: _notNullType =
@@ -164,7 +166,15 @@
throw UnimplementedError('TODO(paulberry)');
}
var leftType = node.leftHandSide.accept(this);
- return _handleAssignment(leftType, node.rightHandSide);
+ var conditionalNode = _lastConditionalNode;
+ _lastConditionalNode = null;
+ var expressionType = _handleAssignment(leftType, node.rightHandSide);
+ if (_isConditionalExpression(node.leftHandSide)) {
+ expressionType = expressionType.withNode(
+ NullabilityNode.forLUB(conditionalNode, expressionType.node));
+ _variables.recordDecoratedExpressionType(node, expressionType);
+ }
+ return expressionType;
}
@override
@@ -363,15 +373,13 @@
DecoratedType visitMethodInvocation(MethodInvocation node) {
DecoratedType targetType;
var target = node.realTarget;
+ bool isConditional = _isConditionalExpression(node);
if (target != null) {
- switch (node.operator.type) {
- case TokenType.PERIOD:
- case TokenType.PERIOD_PERIOD:
- _checkNonObjectMember(node.methodName.name); // TODO(paulberry)
- targetType = _handleAssignment(_notNullType, target);
- break;
- default:
- throw new UnimplementedError('TODO(paulberry)');
+ if (isConditional) {
+ targetType = target.accept(this);
+ } else {
+ _checkNonObjectMember(node.methodName.name); // TODO(paulberry)
+ targetType = _handleAssignment(_notNullType, target);
}
}
var callee = node.methodName.staticElement;
@@ -400,7 +408,13 @@
if (suppliedNamedParameters.contains(entry.key)) continue;
entry.value.node.recordNamedParameterNotSupplied(_guards, _graph);
}
- return calleeType.returnType;
+ var expressionType = calleeType.returnType;
+ if (isConditional) {
+ expressionType = expressionType.withNode(
+ NullabilityNode.forLUB(targetType.node, expressionType.node));
+ _variables.recordDecoratedExpressionType(node, expressionType);
+ }
+ return expressionType;
}
@override
@@ -435,14 +449,13 @@
if (node.prefix.staticElement is ImportElement) {
throw new UnimplementedError('TODO(paulberry)');
} else {
- return _handlePropertyAccess(node.prefix, node.period, node.identifier);
+ return _handlePropertyAccess(node, node.prefix, node.identifier);
}
}
@override
DecoratedType visitPropertyAccess(PropertyAccess node) {
- return _handlePropertyAccess(
- node.realTarget, node.operator, node.propertyName);
+ return _handlePropertyAccess(node, node.realTarget, node.propertyName);
}
@override
@@ -586,23 +599,53 @@
}
DecoratedType _handlePropertyAccess(
- Expression target, Token operator, SimpleIdentifier propertyName) {
- switch (operator.type) {
+ Expression node, Expression target, SimpleIdentifier propertyName) {
+ DecoratedType targetType;
+ bool isConditional = _isConditionalExpression(node);
+ if (isConditional) {
+ targetType = target.accept(this);
+ } else {
+ _checkNonObjectMember(propertyName.name); // TODO(paulberry)
+ targetType = _handleAssignment(_notNullType, target);
+ }
+ var callee = propertyName.staticElement;
+ if (callee == null) {
+ throw new UnimplementedError('TODO(paulberry)');
+ }
+ var calleeType = getOrComputeElementType(callee, targetType: targetType);
+ // TODO(paulberry): substitute if necessary
+ if (propertyName.inSetterContext()) {
+ if (isConditional) {
+ _lastConditionalNode = targetType.node;
+ }
+ return calleeType.positionalParameters[0];
+ } else {
+ var expressionType = calleeType.returnType;
+ if (isConditional) {
+ expressionType = expressionType.withNode(
+ NullabilityNode.forLUB(targetType.node, expressionType.node));
+ _variables.recordDecoratedExpressionType(node, expressionType);
+ }
+ return expressionType;
+ }
+ }
+
+ bool _isConditionalExpression(Expression expression) {
+ Token token;
+ if (expression is MethodInvocation) {
+ token = expression.operator;
+ if (token == null) return false;
+ } else if (expression is PropertyAccess) {
+ token = expression.operator;
+ } else {
+ return false;
+ }
+ switch (token.type) {
case TokenType.PERIOD:
case TokenType.PERIOD_PERIOD:
- _checkNonObjectMember(propertyName.name); // TODO(paulberry)
- var targetType = _handleAssignment(_notNullType, target);
- var callee = propertyName.staticElement;
- if (callee == null) {
- throw new UnimplementedError('TODO(paulberry)');
- }
- var calleeType =
- getOrComputeElementType(callee, targetType: targetType);
- // TODO(paulberry): substitute if necessary
- if (propertyName.inSetterContext()) {
- return calleeType.positionalParameters[0];
- }
- return calleeType.returnType;
+ return false;
+ case TokenType.QUESTION_PERIOD:
+ return true;
default:
throw new UnimplementedError('TODO(paulberry)');
}
diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart
index cf2d0d1..8a15fbf 100644
--- a/pkg/nnbd_migration/test/api_test.dart
+++ b/pkg/nnbd_migration/test/api_test.dart
@@ -844,6 +844,82 @@
await _checkSingleFileChanges(content, expected);
}
+ test_null_aware_getter_invocation() async {
+ var content = '''
+bool f(int i) => i?.isEven;
+main() {
+ f(null);
+}
+''';
+ var expected = '''
+bool? f(int? i) => i?.isEven;
+main() {
+ f(null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ test_null_aware_method_invocation() async {
+ var content = '''
+int f(int i) => i?.abs();
+main() {
+ f(null);
+}
+''';
+ var expected = '''
+int? f(int? i) => i?.abs();
+main() {
+ f(null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ test_null_aware_setter_invocation_null_target() async {
+ var content = '''
+class C {
+ void set x(int value);
+}
+int f(C c) => c?.x = 1;
+main() {
+ f(null);
+}
+''';
+ var expected = '''
+class C {
+ void set x(int value);
+}
+int? f(C? c) => c?.x = 1;
+main() {
+ f(null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
+ test_null_aware_setter_invocation_null_value() async {
+ var content = '''
+class C {
+ void set x(int value);
+}
+int f(C c) => c?.x = 1;
+main() {
+ f(null);
+}
+''';
+ var expected = '''
+class C {
+ void set x(int value);
+}
+int? f(C? c) => c?.x = 1;
+main() {
+ f(null);
+}
+''';
+ await _checkSingleFileChanges(content, expected);
+ }
+
test_single_file_multiple_changes() async {
var content = '''
int f() => null;
diff --git a/pkg/nnbd_migration/test/migration_visitor_test.dart b/pkg/nnbd_migration/test/migration_visitor_test.dart
index 8ff7c98..ab5c5e3 100644
--- a/pkg/nnbd_migration/test/migration_visitor_test.dart
+++ b/pkg/nnbd_migration/test/migration_visitor_test.dart
@@ -251,6 +251,20 @@
hard: true);
}
+ test_assignmentExpression_setter_null_aware() async {
+ await analyze('''
+class C {
+ void set s(int value) {}
+}
+int f(C c, int i) => (c?.s = i);
+''');
+ var lubNode =
+ decoratedExpressionType('(c?.s = i)').node as NullabilityNodeForLUB;
+ expect(lubNode.left, same(decoratedTypeAnnotation('C c').node));
+ expect(lubNode.right, same(decoratedTypeAnnotation('int i').node));
+ assertEdge(lubNode, decoratedTypeAnnotation('int f').node, hard: false);
+ }
+
test_assignmentExpression_setter_target_check() async {
await analyze('''
class C {
@@ -877,6 +891,20 @@
hard: false);
}
+ test_methodInvocation_return_type_null_aware() async {
+ await analyze('''
+class C {
+ bool m() => true;
+}
+bool f(C c) => (c?.m());
+''');
+ var lubNode =
+ decoratedExpressionType('(c?.m())').node as NullabilityNodeForLUB;
+ expect(lubNode.left, same(decoratedTypeAnnotation('C c').node));
+ expect(lubNode.right, same(decoratedTypeAnnotation('bool m').node));
+ assertEdge(lubNode, decoratedTypeAnnotation('bool f').node, hard: false);
+ }
+
test_methodInvocation_target_check() async {
await analyze('''
class C {
@@ -1011,6 +1039,20 @@
hard: false);
}
+ test_propertyAccess_return_type_null_aware() async {
+ await analyze('''
+class C {
+ bool get b => true;
+}
+bool f(C c) => (c?.b);
+''');
+ var lubNode =
+ decoratedExpressionType('(c?.b)').node as NullabilityNodeForLUB;
+ expect(lubNode.left, same(decoratedTypeAnnotation('C c').node));
+ expect(lubNode.right, same(decoratedTypeAnnotation('bool get b').node));
+ assertEdge(lubNode, decoratedTypeAnnotation('bool f').node, hard: false);
+ }
+
test_propertyAccess_target_check() async {
await analyze('''
class C {
