diff --git a/pkg/nnbd_migration/lib/nnbd_migration.dart b/pkg/nnbd_migration/lib/nnbd_migration.dart index a05c09f8fd4..6dd39916247 100644 --- a/pkg/nnbd_migration/lib/nnbd_migration.dart +++ b/pkg/nnbd_migration/lib/nnbd_migration.dart @@ -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, diff --git a/pkg/nnbd_migration/lib/src/edit_plan.dart b/pkg/nnbd_migration/lib/src/edit_plan.dart index c6032255adc..bc70677944e 100644 --- a/pkg/nnbd_migration/lib/src/edit_plan.dart +++ b/pkg/nnbd_migration/lib/src/edit_plan.dart @@ -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]. diff --git a/pkg/nnbd_migration/lib/src/fix_aggregator.dart b/pkg/nnbd_migration/lib/src/fix_aggregator.dart index 6e57d8d35a1..60e779dd504 100644 --- a/pkg/nnbd_migration/lib/src/fix_aggregator.dart +++ b/pkg/nnbd_migration/lib/src/fix_aggregator.dart @@ -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 { @@ -195,6 +217,64 @@ class FixAggregator extends UnifyingAstVisitor { } } +/// [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 extends NodeChange { - bool _addsNoValidMigration = false; + /// The list of [ExpressionChange] objects that should be applied to the + /// expression, in the order they should be applied. + final List 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 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 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 extends NodeChange { 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> { diff --git a/pkg/nnbd_migration/lib/src/fix_builder.dart b/pkg/nnbd_migration/lib/src/fix_builder.dart index 0f2cd7f59f7..5575d97575d 100644 --- a/pkg/nnbd_migration/lib/src/fix_builder.dart +++ b/pkg/nnbd_migration/lib/src/fix_builder.dart @@ -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)) { diff --git a/pkg/nnbd_migration/lib/src/front_end/info_builder.dart b/pkg/nnbd_migration/lib/src/front_end/info_builder.dart index 7852c1d1bad..39c29263b44 100644 --- a/pkg/nnbd_migration/lib/src/front_end/info_builder.dart +++ b/pkg/nnbd_migration/lib/src/front_end/info_builder.dart @@ -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; } diff --git a/pkg/nnbd_migration/lib/src/front_end/migration_summary.dart b/pkg/nnbd_migration/lib/src/front_end/migration_summary.dart index a551df2540c..e8e39e1e13d 100644 --- a/pkg/nnbd_migration/lib/src/front_end/migration_summary.dart +++ b/pkg/nnbd_migration/lib/src/front_end/migration_summary.dart @@ -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: diff --git a/pkg/nnbd_migration/lib/src/front_end/unit_renderer.dart b/pkg/nnbd_migration/lib/src/front_end/unit_renderer.dart index 6f870353d5e..0ede8f92de3 100644 --- a/pkg/nnbd_migration/lib/src/front_end/unit_renderer.dart +++ b/pkg/nnbd_migration/lib/src/front_end/unit_renderer.dart @@ -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: diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart index ff70ada776d..c64786996e3 100644 --- a/pkg/nnbd_migration/test/api_test.dart +++ b/pkg/nnbd_migration/test/api_test.dart @@ -3357,6 +3357,32 @@ main() { await _checkSingleFileChanges(content, expected); } + Future test_future_nullability_mismatch() async { + var content = ''' +String foo; + +Future getNullableFoo() async { + return foo; +} + +Future getFoo() { + return getNullableFoo(); +} +'''; + var expected = ''' +String? foo; + +Future getNullableFoo() async { + return foo; +} + +Future getFoo() { + return getNullableFoo().then((value) => value!); +} +'''; + await _checkSingleFileChanges(content, expected); + } + Future test_future_or_t_downcast_to_t() async { var content = ''' import 'dart:async'; @@ -3389,6 +3415,30 @@ void f( await _checkSingleFileChanges(content, expected); } + Future test_future_type_mismatch() async { + var content = ''' +Future> getNullableInts() async { + return [null]; +} + +Future> getInts() { + return getNullableInts(); +} +'''; + // TODO(paulberry): this is not a good migration. Really we should produce + // getNullableInts.then((value) => value.cast()); + var expected = ''' +Future> getNullableInts() async { + return [null]; +} + +Future> getInts() { + return getNullableInts().then((value) => value as List); +} +'''; + await _checkSingleFileChanges(content, expected); + } + Future test_generic_exact_propagation() async { var content = ''' class C { diff --git a/pkg/nnbd_migration/test/edit_plan_test.dart b/pkg/nnbd_migration/test/edit_plan_test.dart index 64ff882f181..078318e3e34 100644 --- a/pkg/nnbd_migration/test/edit_plan_test.dart +++ b/pkg/nnbd_migration/test/edit_plan_test.dart @@ -315,6 +315,30 @@ class C { expectedIncludingInformative: 'f() => 0 /* zero */ .isEven;'); } + Future 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 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 test_addPostfix_outer_precedence() async { + await analyze('f(x) => x/*!*/;'); + checkPlan( + planner.addPostfix( + planner.passThrough(findNode.simple('x/*!*/')), '.abs()'), + 'f(x) => x.abs()/*!*/;'); + } + Future test_addUnaryPostfix_inner_precedence_add_parens() async { await analyze('f(x) => -x;'); checkPlan( diff --git a/pkg/nnbd_migration/test/fix_aggregator_test.dart b/pkg/nnbd_migration/test/fix_aggregator_test.dart index de9e76a10e3..44301239228 100644 --- a/pkg/nnbd_migration/test/fix_aggregator_test.dart +++ b/pkg/nnbd_migration/test/fix_aggregator_test.dart @@ -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 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; 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; 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; 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; 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; 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; 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; 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; 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; 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; 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; 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;'); @@ -1087,7 +1124,9 @@ f(Object o) => o as a.Future; 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; 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; 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; Future 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; Future 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 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 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 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 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 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 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!;'); } diff --git a/pkg/nnbd_migration/test/fix_builder_test.dart b/pkg/nnbd_migration/test/fix_builder_test.dart index e292a6fdd71..7c6f1f72c37 100644 --- a/pkg/nnbd_migration/test/fix_builder_test.dart +++ b/pkg/nnbd_migration/test/fix_builder_test.dart @@ -3267,7 +3267,7 @@ int/*!*/ f(C/*!*/ c) => c?.i; var propertyAccess = findNode.propertyAccess('?.'); visitSubexpression(propertyAccess, 'int', changes: { propertyAccess: TypeMatcher() - .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 { - TypeMatcher havingNullCheckWithInfo( - dynamic matcher) => - having((c) => c.addsNullCheck, 'addsNullCheck', true) - .having((c) => c.addNullCheckInfo, 'addNullCheckInfo', matcher); +extension _NodeChangeForExpressionExtension + on TypeMatcher { + TypeMatcher havingExpressionChange( + dynamic changeMatcher, dynamic infoMatcher) => + having((c) => c.expressionChanges.single, 'expressionChanges.single', + changeMatcher) + .having((c) => c.expressionChangeInfos.single, + 'expressionChangeInfos.single', infoMatcher); - TypeMatcher havingNoValidMigrationWithInfo( - dynamic matcher) => - having((c) => c.addsNoValidMigration, 'addsNoValidMigration', true) - .having((c) => c.addNoValidMigrationInfo, 'addNoValidMigrationInfo', - matcher); + TypeMatcher havingNullCheckWithInfo(dynamic matcher) => + havingExpressionChange(TypeMatcher(), matcher); - TypeMatcher havingIndroduceAsWithInfo( + TypeMatcher havingNoValidMigrationWithInfo(dynamic matcher) => + havingExpressionChange(TypeMatcher(), matcher); + + TypeMatcher havingIndroduceAsWithInfo( dynamic typeStringMatcher, dynamic infoMatcher) => - having((c) => c.introducesAsType.toString(), 'introducesAsType (string)', - typeStringMatcher) - .having((c) => c.introducesAsInfo, 'introducesAsInfo', infoMatcher); + havingExpressionChange( + TypeMatcher().having( + (c) => c.type.toString(), 'type (string)', typeStringMatcher), + infoMatcher); }