Migration: fix futures using .then rather than as

Previously, if the migration tool encountered a Future expression with
a bad type (e.g. a Future<String?> where a Future<String> was needed),
it would "fix" the problem by introducing a cast.  That is nearly
always the wrong thing to do; what we want to do is null check the
value that the future *completes* with.

This CL changes the migration tool so that it fixes this case by
appending `.then((value) => value!)` to the future expression.

Fixes #45472.

Bug: https://github.com/dart-lang/sdk/issues/45472
Change-Id: I7a35b54f673936e2e4b0f8f3a077ba8bf684b4eb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193700
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry
2021-03-31 17:48:19 +00:00
committed by commit-bot@chromium.org
parent 5ca3b45541
commit 0e8a84173e
11 changed files with 529 additions and 267 deletions
@@ -18,6 +18,11 @@ export 'package:nnbd_migration/src/utilities/hint_utils.dart' show HintComment;
/// Description of fixes that might be performed by nullability migration.
class NullabilityFixDescription {
/// A `.then((value) => ...)` suffix was added to an expression.
static const addThen = NullabilityFixDescription._(
appliedMessage: 'Added `.then` to adjust type of Future expression',
kind: NullabilityFixKind.addThen);
/// An import was added to the library.
static const addImport = NullabilityFixDescription._(
appliedMessage: 'Added import for use in migrated code',
@@ -266,6 +271,7 @@ class NullabilityFixDescription {
/// An enumeration of the various kinds of nullability fixes.
enum NullabilityFixKind {
addThen,
addImport,
addLate,
addLateDueToHint,
+19 -9
View File
@@ -281,6 +281,22 @@ class EditPlanner {
]);
}
/// Creates a new edit plan that consists of executing [innerPlan], and then
/// appending the given text with postfix precedence. This could be used, for
/// example, to append a property access or method call.
///
/// Optional argument [info] contains information about why the change was
/// made.
NodeProducingEditPlan addPostfix(NodeProducingEditPlan innerPlan, String text,
{AtomicEditInfo info}) {
assert(innerPlan.sourceNode is Expression);
return surround(innerPlan,
suffix: [AtomicEdit.insert(text, info: info)],
outerPrecedence: Precedence.postfix,
innerPrecedence: Precedence.postfix,
associative: true);
}
/// Creates a new edit plan that consists of executing [innerPlan], and then
/// appending the given postfix [operator]. This could be used, for example,
/// to add a null check.
@@ -288,15 +304,9 @@ class EditPlanner {
/// Optional argument [info] contains information about why the change was
/// made.
NodeProducingEditPlan addUnaryPostfix(
NodeProducingEditPlan innerPlan, TokenType operator,
{AtomicEditInfo info}) {
assert(innerPlan.sourceNode is Expression);
return surround(innerPlan,
suffix: [AtomicEdit.insert(operator.lexeme, info: info)],
outerPrecedence: Precedence.postfix,
innerPrecedence: Precedence.postfix,
associative: true);
}
NodeProducingEditPlan innerPlan, TokenType operator,
{AtomicEditInfo info}) =>
addPostfix(innerPlan, operator.lexeme, info: info);
/// Creates a new edit plan that consists of executing [innerPlan], and then
/// prepending the given prefix [operator].
+148 -74
View File
@@ -18,6 +18,28 @@ import 'package:nnbd_migration/src/edit_plan.dart';
import 'package:nnbd_migration/src/fix_builder.dart';
import 'package:nnbd_migration/src/utilities/hint_utils.dart';
/// Base class representing a change that might need to be made to an
/// expression.
abstract class ExpressionChange {
/// The type of the expression after the change is applied.
final DartType resultType;
ExpressionChange(this.resultType);
/// Description of the change.
NullabilityFixDescription get description;
/// Creates a [NodeProducingEditPlan] that applies the change to [innerPlan].
NodeProducingEditPlan applyExpression(FixAggregator aggregator,
NodeProducingEditPlan innerPlan, AtomicEditInfo info);
/// Creates a string that applies the change to the [inner] text string.
String applyText(FixAggregator aggregator, String inner);
/// Describes the change, for use in debugging.
String describe();
}
/// Visitor that combines together the changes produced by [FixBuilder] into a
/// concrete set of source code edits using the infrastructure of [EditPlan].
class FixAggregator extends UnifyingAstVisitor<void> {
@@ -195,6 +217,64 @@ class FixAggregator extends UnifyingAstVisitor<void> {
}
}
/// [ExpressionChange] describing the addition of an `as` cast to an expression.
class IntroduceAsChange extends ExpressionChange {
/// The type being cast to.
final DartType type;
/// Indicates whether this is a downcast.
final bool isDowncast;
IntroduceAsChange(this.type, {@required this.isDowncast}) : super(type);
@override
NullabilityFixDescription get description => isDowncast
? NullabilityFixDescription.downcastExpression
: NullabilityFixDescription.otherCastExpression;
@override
NodeProducingEditPlan applyExpression(FixAggregator aggregator,
NodeProducingEditPlan innerPlan, AtomicEditInfo info) =>
aggregator.planner.addBinaryPostfix(
innerPlan, TokenType.AS, aggregator.typeToCode(type),
info: info);
@override
String applyText(FixAggregator aggregator, String inner) =>
'$inner as ${aggregator.typeToCode(type)}';
@override
String describe() => 'IntroduceAsChange($type)';
}
/// [ExpressionChange] describing the addition of an `as` cast to an expression
/// having a Future type.
class IntroduceThenChange extends ExpressionChange {
/// The change that should be made to the value the future completes with.
final ExpressionChange innerChange;
IntroduceThenChange(DartType resultType, this.innerChange)
: super(resultType);
@override
NullabilityFixDescription get description =>
NullabilityFixDescription.addThen;
@override
NodeProducingEditPlan applyExpression(FixAggregator aggregator,
NodeProducingEditPlan innerPlan, AtomicEditInfo info) =>
aggregator.planner.addPostfix(innerPlan,
'.then((value) => ${innerChange.applyText(aggregator, 'value')})',
info: info);
@override
String applyText(FixAggregator aggregator, String inner) =>
'$inner.then((value) => ${innerChange.applyText(aggregator, 'value')})';
@override
String describe() => 'IntroduceThenChange($innerChange)';
}
/// Reasons that a variable declaration is to be made late.
enum LateAdditionReason {
/// It was inferred that the associated variable declaration is to be made
@@ -701,69 +781,25 @@ class NodeChangeForDefaultFormalParameter
/// Implementation of [NodeChange] specialized for operating on [Expression]
/// nodes.
class NodeChangeForExpression<N extends Expression> extends NodeChange<N> {
bool _addsNoValidMigration = false;
/// The list of [ExpressionChange] objects that should be applied to the
/// expression, in the order they should be applied.
final List<ExpressionChange> expressionChanges = [];
AtomicEditInfo _addNoValidMigrationInfo;
bool _addsNullCheck = false;
AtomicEditInfo _addNullCheckInfo;
HintComment _addNullCheckHint;
DartType _introducesAsType;
AtomicEditInfo _introduceAsInfo;
/// The list of [AtomicEditInfo] objects corresponding to each change in
/// [expressionChanges].
final List<AtomicEditInfo> expressionChangeInfos = [];
NodeChangeForExpression() : super._();
/// Gets the info for any added "no valid migration" comment.
AtomicEditInfo get addNoValidMigrationInfo => _addNoValidMigrationInfo;
/// Gets the info for any added null check.
AtomicEditInfo get addNullCheckInfo => _addNullCheckInfo;
/// Indicates whether [addNoValidMigration] has been called.
bool get addsNoValidMigration => _addsNoValidMigration;
/// Indicates whether [addNullCheck] has been called.
bool get addsNullCheck => _addsNullCheck;
/// Gets the info for any introduced "as" cast
AtomicEditInfo get introducesAsInfo => _introduceAsInfo;
/// Gets the type for any introduced "as" cast, or `null` if no "as" cast is
/// being introduced.
DartType get introducesAsType => _introducesAsType;
@override
Iterable<String> get _toStringParts => [
if (_addsNoValidMigration) 'addsNoValidMigration',
if (_addsNullCheck) 'addsNullCheck',
if (_introducesAsType != null) 'introducesAsType'
for (var expressionChange in expressionChanges)
expressionChange.describe()
];
void addNoValidMigration(AtomicEditInfo info) {
assert(!_addsNoValidMigration);
_addsNoValidMigration = true;
_addNoValidMigrationInfo = info;
}
/// Causes a null check to be added to this expression, with the given [info].
void addNullCheck(AtomicEditInfo info, {HintComment hint}) {
assert(!_addsNullCheck);
_addsNullCheck = true;
_addNullCheckInfo = info;
_addNullCheckHint = hint;
}
/// Causes a cast to the given [type] to be added to this expression, with
/// the given [info].
void introduceAs(DartType type, AtomicEditInfo info) {
assert(_introducesAsType == null);
assert(type != null);
_introducesAsType = type;
_introduceAsInfo = info;
void addExpressionChange(ExpressionChange change, AtomicEditInfo info) {
expressionChanges.add(change);
expressionChangeInfos.add(info);
}
@override
@@ -778,25 +814,9 @@ class NodeChangeForExpression<N extends Expression> extends NodeChange<N> {
NodeProducingEditPlan _applyExpression(
FixAggregator aggregator, NodeProducingEditPlan innerPlan) {
var plan = innerPlan;
if (_addsNullCheck) {
var hint = _addNullCheckHint;
if (hint != null) {
plan = aggregator.planner.acceptNullabilityOrNullCheckHint(plan, hint,
info: _addNullCheckInfo);
} else {
plan = aggregator.planner
.addUnaryPostfix(plan, TokenType.BANG, info: _addNullCheckInfo);
}
}
if (_addsNoValidMigration) {
plan = aggregator.planner.addCommentPostfix(
plan, '/* no valid migration */',
info: _addNoValidMigrationInfo, isInformative: true);
}
if (_introducesAsType != null) {
plan = aggregator.planner.addBinaryPostfix(
plan, TokenType.AS, aggregator.typeToCode(_introducesAsType),
info: _introduceAsInfo);
for (int i = 0; i < expressionChanges.length; i++) {
plan = expressionChanges[i]
.applyExpression(aggregator, plan, expressionChangeInfos[i]);
}
return plan;
}
@@ -1289,6 +1309,60 @@ class NodeChangeForVariableDeclarationList
}
}
/// [ExpressionChange] describing the addition of a comment explaining that a
/// literal `null` could not be migrated.
class NoValidMigrationChange extends ExpressionChange {
NoValidMigrationChange(DartType resultType) : super(resultType);
@override
NullabilityFixDescription get description =>
NullabilityFixDescription.noValidMigrationForNull;
@override
NodeProducingEditPlan applyExpression(FixAggregator aggregator,
NodeProducingEditPlan innerPlan, AtomicEditInfo info) =>
aggregator.planner.addCommentPostfix(
innerPlan, '/* no valid migration */',
info: info, isInformative: true);
@override
String applyText(FixAggregator aggregator, String inner) =>
'$inner /* no valid migration */';
@override
String describe() => 'NoValidMigrationChange';
}
/// [ExpressionChange] describing the addition of an `!` after an expression.
class NullCheckChange extends ExpressionChange {
/// The hint that is causing this `!` to be added, if any.
final HintComment hint;
NullCheckChange(DartType resultType, {this.hint}) : super(resultType);
@override
NullabilityFixDescription get description =>
NullabilityFixDescription.checkExpression;
@override
NodeProducingEditPlan applyExpression(FixAggregator aggregator,
NodeProducingEditPlan innerPlan, AtomicEditInfo info) {
if (hint != null) {
return aggregator.planner
.acceptNullabilityOrNullCheckHint(innerPlan, hint, info: info);
} else {
return aggregator.planner
.addUnaryPostfix(innerPlan, TokenType.BANG, info: info);
}
}
@override
String applyText(FixAggregator aggregator, String inner) => '$inner!';
@override
String describe() => 'NullCheckChange';
}
/// Visitor that creates an appropriate [NodeChange] object for the node being
/// visited.
class _NodeChangeVisitor extends GeneralizingAstVisitor<NodeChange<AstNode>> {
+53 -32
View File
@@ -577,44 +577,62 @@ class MigrationResolutionHooksImpl
_flowAnalysis = flowAnalysis;
}
DartType _addCast(
DartType _addCastOrNullCheck(
Expression node, DartType expressionType, DartType contextType) {
var isDowncast =
_fixBuilder._typeSystem.isSubtypeOf(contextType, expressionType);
var checks =
_fixBuilder._variables.expressionChecks(_fixBuilder.source, node);
var info = AtomicEditInfo(
isDowncast
? NullabilityFixDescription.downcastExpression
: NullabilityFixDescription.otherCastExpression,
checks != null ? checks.edges : {});
var change = _createExpressionChange(node, expressionType, contextType);
var info = AtomicEditInfo(change.description, checks?.edges ?? {});
(_fixBuilder._getChange(node) as NodeChangeForExpression)
.introduceAs(contextType, info);
_flowAnalysis.asExpression_end(node, contextType);
return contextType;
.addExpressionChange(change, info);
return change.resultType;
}
DartType _addNullCheck(Expression node, DartType type,
{AtomicEditInfo info, HintComment hint}) {
var change = _createNullCheckChange(node, type, hint: hint);
var checks =
_fixBuilder._variables.expressionChecks(_fixBuilder.source, node);
bool noValidMigration = node is NullLiteral && hint == null;
info ??= checks != null
? AtomicEditInfo(
noValidMigration
? NullabilityFixDescription.noValidMigrationForNull
: NullabilityFixDescription.checkExpression,
checks.edges)
: null;
info ??= AtomicEditInfo(change.description, checks?.edges ?? {});
var nodeChangeForExpression =
_fixBuilder._getChange(node) as NodeChangeForExpression;
if (noValidMigration) {
nodeChangeForExpression.addNoValidMigration(info);
} else {
nodeChangeForExpression.addNullCheck(info, hint: hint);
nodeChangeForExpression.addExpressionChange(change, info);
return change.resultType;
}
ExpressionChange _createExpressionChange(
Expression node, DartType expressionType, DartType contextType) {
var expressionFutureTypeArgument = _getFutureTypeArgument(expressionType);
var contextFutureTypeArgument = _getFutureTypeArgument(contextType);
if (expressionFutureTypeArgument != null &&
contextFutureTypeArgument != null) {
return IntroduceThenChange(
contextType,
_createExpressionChange(
node, expressionFutureTypeArgument, contextFutureTypeArgument));
}
// Either a cast or a null check is needed. We prefer to do a null
// check if we can.
var nonNullType = _fixBuilder._typeSystem.promoteToNonNull(expressionType);
if (_fixBuilder._typeSystem.isSubtypeOf(nonNullType, contextType)) {
return _createNullCheckChange(node, expressionType);
} else {
if (node != null) {
_flowAnalysis.asExpression_end(node, contextType);
}
return IntroduceAsChange(contextType,
isDowncast:
_fixBuilder._typeSystem.isSubtypeOf(contextType, expressionType));
}
}
ExpressionChange _createNullCheckChange(Expression node, DartType type,
{HintComment hint}) {
var resultType = _fixBuilder._typeSystem.promoteToNonNull(type as TypeImpl);
_flowAnalysis.nonNullAssert_end(node);
return _fixBuilder._typeSystem.promoteToNonNull(type as TypeImpl);
return node is NullLiteral && hint == null
? NoValidMigrationChange(resultType)
: NullCheckChange(resultType, hint: hint);
}
Expression _findNullabilityContextAncestor(Expression node) {
@@ -638,6 +656,16 @@ class MigrationResolutionHooksImpl
return finalType as InterfaceType;
}
DartType _getFutureTypeArgument(DartType type) {
if (type is InterfaceType && type.isDartAsyncFuture) {
var typeArguments = type.typeArguments;
if (typeArguments.isNotEmpty) {
return typeArguments.first;
}
}
return null;
}
DartType _modifyRValueType(Expression node, DartType type,
{DartType context}) {
if (node is MethodInvocation) {
@@ -689,14 +717,7 @@ class MigrationResolutionHooksImpl
.makeNullable(methodInvocationType as TypeImpl);
return type;
}
// Either a cast or a null check is needed. We prefer to do a null
// check if we can.
var nonNullType = _fixBuilder._typeSystem.promoteToNonNull(type);
if (_fixBuilder._typeSystem.isSubtypeOf(nonNullType, context)) {
return _addNullCheck(node, type);
} else {
return _addCast(node, type, context);
}
return _addCastOrNullCheck(node, type, context);
}
if (!_fixBuilder._typeSystem.isNullable(type)) return type;
if (_needsNullCheckDueToStructure(ancestor)) {
@@ -262,6 +262,9 @@ class InfoBuilder {
// We don't offer any edits around unmigratable `null`s. The user has
// to fix manually.
break;
case NullabilityFixKind.addThen:
// We don't offer any edits around addition of `.then` to a future.
break;
}
return edits;
}
@@ -60,6 +60,8 @@ class MigrationSummary {
String _keyForKind(NullabilityFixKind kind) {
switch (kind) {
case NullabilityFixKind.addThen:
return 'addThen';
case NullabilityFixKind.addImport:
return 'addImport';
case NullabilityFixKind.addLate:
@@ -26,6 +26,7 @@ class UnitRenderer {
NullabilityFixKind.noValidMigrationForNull,
NullabilityFixKind.compoundAssignmentHasBadCombinedType,
NullabilityFixKind.compoundAssignmentHasNullableSource,
NullabilityFixKind.addThen,
NullabilityFixKind.removeDeadCode,
NullabilityFixKind.conditionTrueInStrongMode,
NullabilityFixKind.conditionFalseInStrongMode,
@@ -277,6 +278,8 @@ class UnitRenderer {
var s = count == 1 ? '' : 's';
var es = count == 1 ? '' : 'es';
switch (kind) {
case NullabilityFixKind.addThen:
return '$count invocation$s of `.then` added';
case NullabilityFixKind.addImport:
return '$count import$s added';
case NullabilityFixKind.addLate:
+50
View File
@@ -3357,6 +3357,32 @@ main() {
await _checkSingleFileChanges(content, expected);
}
Future<void> test_future_nullability_mismatch() async {
var content = '''
String foo;
Future<String> getNullableFoo() async {
return foo;
}
Future<String/*!*/> getFoo() {
return getNullableFoo();
}
''';
var expected = '''
String? foo;
Future<String?> getNullableFoo() async {
return foo;
}
Future<String> getFoo() {
return getNullableFoo().then((value) => value!);
}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_future_or_t_downcast_to_t() async {
var content = '''
import 'dart:async';
@@ -3389,6 +3415,30 @@ void f(
await _checkSingleFileChanges(content, expected);
}
Future<void> test_future_type_mismatch() async {
var content = '''
Future<List<int>> getNullableInts() async {
return [null];
}
Future<List<int/*!*/>> getInts() {
return getNullableInts();
}
''';
// TODO(paulberry): this is not a good migration. Really we should produce
// getNullableInts.then((value) => value.cast());
var expected = '''
Future<List<int?>> getNullableInts() async {
return [null];
}
Future<List<int>> getInts() {
return getNullableInts().then((value) => value as List<int>);
}
''';
await _checkSingleFileChanges(content, expected);
}
Future<void> test_generic_exact_propagation() async {
var content = '''
class C<T> {
@@ -315,6 +315,30 @@ class C {
expectedIncludingInformative: 'f() => 0 /* zero */ .isEven;');
}
Future<void> test_addPostfix_inner_precedence_add_parens() async {
await analyze('f(x) => -x;');
checkPlan(
planner.addPostfix(
planner.passThrough(findNode.prefix('-x')), '.abs()'),
'f(x) => (-x).abs();');
}
Future<void> test_addPostfix_inner_precedence_no_parens() async {
await analyze('f(x) => x++;');
checkPlan(
planner.addPostfix(
planner.passThrough(findNode.postfix('x++')), '.abs()'),
'f(x) => x++.abs();');
}
Future<void> test_addPostfix_outer_precedence() async {
await analyze('f(x) => x/*!*/;');
checkPlan(
planner.addPostfix(
planner.passThrough(findNode.simple('x/*!*/')), '.abs()'),
'f(x) => x.abs()/*!*/;');
}
Future<void> test_addUnaryPostfix_inner_precedence_add_parens() async {
await analyze('f(x) => -x;');
checkPlan(
+202 -137
View File
@@ -165,7 +165,8 @@ main() => null;
..addImport('package:collection/collection.dart', 'IterableExtension'),
findNode.import('package:fixnum').combinators[0]:
NodeChangeForShowCombinator()..addName('Int64'),
findNode.expression('null'): NodeChangeForExpression()..addNullCheck(null)
findNode.expression('null'): NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), '''
import 'package:args/args.dart';
@@ -342,10 +343,12 @@ f({@deprecated required int x}) {}
var aRef = findNode.simple('a +');
var bRef = findNode.simple('b;');
var previewInfo = run({
aRef: NodeChangeForExpression()..addNullCheck(_MockInfo()),
bRef: NodeChangeForExpression()..addNullCheck(_MockInfo()),
aRef: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo()),
bRef: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo()),
findNode.binary('a + b'): NodeChangeForExpression()
..addNullCheck(_MockInfo())
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a, b) => (a! + b!)!;');
}
@@ -392,7 +395,8 @@ g(int x, int y) => f(x, y);
var previewInfo = run({
findNode.methodInvocation('f(x').argumentList: NodeChangeForArgumentList()
..dropArgument(findNode.simple('y);'), null),
findNode.simple('x, y'): NodeChangeForExpression()..addNullCheck(null)
findNode.simple('x, y'): NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), '''
f([int x, int y]) => null;
@@ -404,7 +408,8 @@ g(int x, int y) => f(x!);
var content = 'f(int x, int y) => x += y;';
await analyze(content);
var previewInfo = run({
findNode.assignment('+='): NodeChangeForAssignment()..addNullCheck(null)
findNode.assignment('+='): NodeChangeForAssignment()
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(int x, int y) => (x += y)!;');
}
@@ -415,7 +420,7 @@ g(int x, int y) => f(x!);
var previewInfo = run({
findNode.assignment('+='): NodeChangeForAssignment(),
findNode.index('[0]').target: NodeChangeForExpression()
..addNullCheck(null)
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(List<int> x, int y) => x![0] += y;');
}
@@ -426,7 +431,8 @@ g(int x, int y) => f(x!);
var assignment = findNode.assignment('+=');
var previewInfo = run({
assignment: NodeChangeForAssignment(),
assignment.rightHandSide: NodeChangeForExpression()..addNullCheck(null)
assignment.rightHandSide: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(int x, int y) => x += y!;');
}
@@ -468,7 +474,9 @@ g(int x, int y) => f(x!);
await analyze(content);
var previewInfo = run({
findNode.assignment('+='): NodeChangeForAssignment()
..introduceAs(nnbdTypeProvider.intType, null)
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.intType, isDowncast: false),
null)
});
expect(previewInfo.applyTo(code), 'f(int x, int y) => (x += y) as int;');
}
@@ -509,7 +517,7 @@ f(int i, int/*?*/ j) {
findNode.statement('if'): NodeChangeForIfStatement()
..conditionValue = true,
findNode.simple('j.isEven'): NodeChangeForExpression()
..addNullCheck(_MockInfo())
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), '''
f(int i, int/*?*/ j) {
@@ -530,7 +538,7 @@ f(int i, int/*?*/ j) {
findNode.statement('if'): NodeChangeForIfStatement()
..conditionValue = true,
findNode.simple('j.isEven'): NodeChangeForExpression()
..addNullCheck(_MockInfo())
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), '''
f(int i, int/*?*/ j) {
@@ -830,7 +838,9 @@ void f(int i, String callback()) {
var cd = findNode.cascade('c..d');
var previewInfo = run({
cd: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.intType, _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.intType, isDowncast: false),
_MockInfo())
});
expect(
previewInfo.applyTo(code), 'f(a, c) => a..b = (throw (c..d) as int);');
@@ -841,7 +851,9 @@ void f(int i, String callback()) {
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.dynamicType, _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.dynamicType, isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), 'f(Object o) => o as dynamic;');
}
@@ -855,7 +867,10 @@ f(Object o) => o;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.futureNullType, _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.futureNullType,
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), '''
import 'dart:async' as a;
@@ -869,12 +884,14 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), 'f(Object o) => o as bool Function();');
@@ -885,15 +902,17 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = nnbdTypeProvider.numType
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = nnbdTypeProvider.numType
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -905,15 +924,17 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = nnbdTypeProvider.dynamicType
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = nnbdTypeProvider.dynamicType
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(
@@ -925,15 +946,17 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = nnbdTypeProvider.objectType
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = nnbdTypeProvider.objectType
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -946,16 +969,18 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = (nnbdTypeProvider.objectType as TypeImpl)
.withNullability(NullabilitySuffix.question)
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = (nnbdTypeProvider.objectType as TypeImpl)
.withNullability(NullabilitySuffix.question)
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(
@@ -967,16 +992,18 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = (nnbdTypeProvider.numType as TypeImpl)
.withNullability(NullabilitySuffix.question)
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T')
..bound = (nnbdTypeProvider.numType as TypeImpl)
.withNullability(NullabilitySuffix.question)
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -988,15 +1015,17 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T'),
TypeParameterElementImpl.synthetic('U')
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [
TypeParameterElementImpl.synthetic('T'),
TypeParameterElementImpl.synthetic('U')
],
parameters: [],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -1008,17 +1037,19 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [
ParameterElementImpl.synthetic(
'x', nnbdTypeProvider.intType, ParameterKind.REQUIRED),
ParameterElementImpl.synthetic(
'y', nnbdTypeProvider.numType, ParameterKind.REQUIRED)
],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [
ParameterElementImpl.synthetic('x',
nnbdTypeProvider.intType, ParameterKind.REQUIRED),
ParameterElementImpl.synthetic(
'y', nnbdTypeProvider.numType, ParameterKind.REQUIRED)
],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -1030,17 +1061,19 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [
ParameterElementImpl.synthetic(
'x', nnbdTypeProvider.intType, ParameterKind.NAMED),
ParameterElementImpl.synthetic(
'y', nnbdTypeProvider.numType, ParameterKind.NAMED)
],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [
ParameterElementImpl.synthetic(
'x', nnbdTypeProvider.intType, ParameterKind.NAMED),
ParameterElementImpl.synthetic(
'y', nnbdTypeProvider.numType, ParameterKind.NAMED)
],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -1052,17 +1085,19 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [
ParameterElementImpl.synthetic(
'x', nnbdTypeProvider.intType, ParameterKind.POSITIONAL),
ParameterElementImpl.synthetic(
'y', nnbdTypeProvider.numType, ParameterKind.POSITIONAL)
],
nullabilitySuffix: NullabilitySuffix.none),
..addExpressionChange(
IntroduceAsChange(
FunctionTypeImpl(
returnType: nnbdTypeProvider.boolType,
typeFormals: [],
parameters: [
ParameterElementImpl.synthetic('x',
nnbdTypeProvider.intType, ParameterKind.POSITIONAL),
ParameterElementImpl.synthetic('y',
nnbdTypeProvider.numType, ParameterKind.POSITIONAL)
],
nullabilitySuffix: NullabilitySuffix.none),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code),
@@ -1074,9 +1109,11 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(
nnbdTypeProvider.mapType(
nnbdTypeProvider.intType, nnbdTypeProvider.boolType),
..addExpressionChange(
IntroduceAsChange(
nnbdTypeProvider.mapType(
nnbdTypeProvider.intType, nnbdTypeProvider.boolType),
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), 'f(Object o) => o as Map<int, bool>;');
@@ -1087,7 +1124,9 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.binary('a | b');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.intType, _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.intType, isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a, b) => a | b as int;');
}
@@ -1097,7 +1136,9 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.binary('a < b');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.boolType, _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.boolType, isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a, b) => (a < b) as bool;');
}
@@ -1110,7 +1151,10 @@ f(Object o) => o;
var expr = findNode.simple('o;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.futureNullType, _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.futureNullType,
isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), '''
import 'dart:async' as a;
@@ -1123,8 +1167,10 @@ f(Object o) => o as a.Future<Null>;
var expr = findNode.simple('x;');
var previewInfo = run({
expr: NodeChangeForExpression()
..introduceAs(nnbdTypeProvider.intType, _MockInfo())
..addNullCheck(_MockInfo())
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.intType, isDowncast: false),
_MockInfo())
});
expect(previewInfo.applyTo(code), 'f(x) => x! as int;');
}
@@ -1191,8 +1237,11 @@ f(Object o) => o as a.Future<Null>;
Future<void> test_noValidMigration() async {
await analyze('f(a) => null;');
var literal = findNode.nullLiteral('null');
var previewInfo = run(
{literal: NodeChangeForExpression()..addNoValidMigration(_MockInfo())});
var previewInfo = run({
literal: NodeChangeForExpression()
..addExpressionChange(
NoValidMigrationChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), code);
expect(previewInfo.applyTo(code, includeInformative: true),
'f(a) => null /* no valid migration */;');
@@ -1201,40 +1250,50 @@ f(Object o) => o as a.Future<Null>;
Future<void> test_nullCheck_index_cascadeResult() async {
await analyze('f(a) => a..[0].c;');
var index = findNode.index('[0]');
var previewInfo =
run({index: NodeChangeForExpression()..addNullCheck(_MockInfo())});
var previewInfo = run({
index: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a) => a..[0]!.c;');
}
Future<void> test_nullCheck_methodInvocation_cascadeResult() async {
await analyze('f(a) => a..b().c;');
var method = findNode.methodInvocation('b()');
var previewInfo = run(
{method: NodeChangeForMethodInvocation()..addNullCheck(_MockInfo())});
var previewInfo = run({
method: NodeChangeForMethodInvocation()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a) => a..b()!.c;');
}
Future<void> test_nullCheck_no_parens() async {
await analyze('f(a) => a++;');
var expr = findNode.postfix('a++');
var previewInfo =
run({expr: NodeChangeForExpression()..addNullCheck(_MockInfo())});
var previewInfo = run({
expr: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a) => a++!;');
}
Future<void> test_nullCheck_parens() async {
await analyze('f(a) => -a;');
var expr = findNode.prefix('-a');
var previewInfo =
run({expr: NodeChangeForExpression()..addNullCheck(_MockInfo())});
var previewInfo = run({
expr: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a) => (-a)!;');
}
Future<void> test_nullCheck_propertyAccess_cascadeResult() async {
await analyze('f(a) => a..b.c;');
var property = findNode.propertyAccess('b');
var previewInfo = run(
{property: NodeChangeForPropertyAccess()..addNullCheck(_MockInfo())});
var previewInfo = run({
property: NodeChangeForPropertyAccess()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(a) => a..b!.c;');
}
@@ -1409,7 +1468,7 @@ class C {
await analyze(content);
var previewInfo = run({
findNode.postfix('++'): NodeChangeForPostfixExpression()
..addNullCheck(null)
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(int x) => x++!;');
}
@@ -1420,7 +1479,7 @@ class C {
var previewInfo = run({
findNode.postfix('++'): NodeChangeForPostfixExpression(),
findNode.index('[0]').target: NodeChangeForExpression()
..addNullCheck(null)
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(List<int> x) => x![0]++;');
}
@@ -1430,7 +1489,9 @@ class C {
await analyze(content);
var previewInfo = run({
findNode.postfix('++'): NodeChangeForPostfixExpression()
..introduceAs(nnbdTypeProvider.intType, null)
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.intType, isDowncast: false),
null)
});
expect(previewInfo.applyTo(code), 'f(int x) => x++ as int;');
}
@@ -1471,7 +1532,8 @@ class C {
var content = 'f(int x) => ++x;';
await analyze(content);
var previewInfo = run({
findNode.prefix('++'): NodeChangeForPrefixExpression()..addNullCheck(null)
findNode.prefix('++'): NodeChangeForPrefixExpression()
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(int x) => (++x)!;');
}
@@ -1482,7 +1544,7 @@ class C {
var previewInfo = run({
findNode.prefix('++'): NodeChangeForPrefixExpression(),
findNode.index('[0]').target: NodeChangeForExpression()
..addNullCheck(null)
..addExpressionChange(NullCheckChange(MockDartType()), null)
});
expect(previewInfo.applyTo(code), 'f(List<int> x) => ++x![0];');
}
@@ -1492,7 +1554,9 @@ class C {
await analyze(content);
var previewInfo = run({
findNode.prefix('++'): NodeChangeForPrefixExpression()
..introduceAs(nnbdTypeProvider.intType, null)
..addExpressionChange(
IntroduceAsChange(nnbdTypeProvider.intType, isDowncast: false),
null)
});
expect(previewInfo.applyTo(code), 'f(int x) => ++x as int;');
}
@@ -1727,7 +1791,8 @@ int f() => null;
var previewInfo = run({
methodInvocation: NodeChangeForMethodInvocation()
..removeNullAwareness = true,
argument: NodeChangeForExpression()..addNullCheck(_MockInfo())
argument: NodeChangeForExpression()
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'f(x) => x.m(x!);');
}
@@ -1906,7 +1971,7 @@ f({required int x}) {}
NodeChangeForVariableDeclarationList()
..addExplicitType = nnbdTypeProvider.intType,
findNode.integerLiteral('0'): NodeChangeForExpression()
..addNullCheck(_MockInfo())
..addExpressionChange(NullCheckChange(MockDartType()), _MockInfo())
});
expect(previewInfo.applyTo(code), 'int x = 0!;');
}
+19 -15
View File
@@ -3267,7 +3267,7 @@ int/*!*/ f(C/*!*/ c) => c?.i;
var propertyAccess = findNode.propertyAccess('?.');
visitSubexpression(propertyAccess, 'int', changes: {
propertyAccess: TypeMatcher<NodeChangeForPropertyAccess>()
.having((c) => c.addsNullCheck, 'addsNullCheck', true)
.havingNullCheckWithInfo(isNotNull)
.having((c) => c.removeNullAwareness, 'removeNullAwareness', true)
});
}
@@ -3906,21 +3906,25 @@ void _f(bool/*?*/ x, bool/*?*/ y) {
.having((c) => c.conditionValue, 'conditionValue', knownValue);
}
extension on TypeMatcher<NodeChangeForExpression> {
TypeMatcher<NodeChangeForExpression> havingNullCheckWithInfo(
dynamic matcher) =>
having((c) => c.addsNullCheck, 'addsNullCheck', true)
.having((c) => c.addNullCheckInfo, 'addNullCheckInfo', matcher);
extension _NodeChangeForExpressionExtension<T extends NodeChangeForExpression>
on TypeMatcher<T> {
TypeMatcher<T> havingExpressionChange(
dynamic changeMatcher, dynamic infoMatcher) =>
having((c) => c.expressionChanges.single, 'expressionChanges.single',
changeMatcher)
.having((c) => c.expressionChangeInfos.single,
'expressionChangeInfos.single', infoMatcher);
TypeMatcher<NodeChangeForExpression> havingNoValidMigrationWithInfo(
dynamic matcher) =>
having((c) => c.addsNoValidMigration, 'addsNoValidMigration', true)
.having((c) => c.addNoValidMigrationInfo, 'addNoValidMigrationInfo',
matcher);
TypeMatcher<T> havingNullCheckWithInfo(dynamic matcher) =>
havingExpressionChange(TypeMatcher<NullCheckChange>(), matcher);
TypeMatcher<NodeChangeForExpression> havingIndroduceAsWithInfo(
TypeMatcher<T> havingNoValidMigrationWithInfo(dynamic matcher) =>
havingExpressionChange(TypeMatcher<NoValidMigrationChange>(), matcher);
TypeMatcher<T> havingIndroduceAsWithInfo(
dynamic typeStringMatcher, dynamic infoMatcher) =>
having((c) => c.introducesAsType.toString(), 'introducesAsType (string)',
typeStringMatcher)
.having((c) => c.introducesAsInfo, 'introducesAsInfo', infoMatcher);
havingExpressionChange(
TypeMatcher<IntroduceAsChange>().having(
(c) => c.type.toString(), 'type (string)', typeStringMatcher),
infoMatcher);
}