diff --git a/pkg/nnbd_migration/lib/src/edit_plan.dart b/pkg/nnbd_migration/lib/src/edit_plan.dart index bc70677944e..633732fed55 100644 --- a/pkg/nnbd_migration/lib/src/edit_plan.dart +++ b/pkg/nnbd_migration/lib/src/edit_plan.dart @@ -185,8 +185,9 @@ class EditPlanner { EditPlanner(this.lineInfo, this.sourceText, {this.removeViaComments = false}); /// Creates a new edit plan that consists of executing [innerPlan], and then - /// converting the late [hint] into an explicit `late`. - NodeProducingEditPlan acceptLateHint( + /// converting the [hint] (which precedes the node) into text that is not + /// commented out. + NodeProducingEditPlan acceptPrefixHint( NodeProducingEditPlan innerPlan, HintComment hint, {AtomicEditInfo info}) { var affixPlan = innerPlan is _CommentAffixPlan @@ -200,8 +201,9 @@ class EditPlanner { } /// Creates a new edit plan that consists of executing [innerPlan], and then - /// converting the nullability [hint] into an explicit `?` or `!`. - NodeProducingEditPlan acceptNullabilityOrNullCheckHint( + /// converting the [hint] (which follows the node) into text that is not + /// commented out. + NodeProducingEditPlan acceptSuffixHint( NodeProducingEditPlan innerPlan, HintComment hint, {AtomicEditInfo info}) { var affixPlan = innerPlan is _CommentAffixPlan diff --git a/pkg/nnbd_migration/lib/src/fix_aggregator.dart b/pkg/nnbd_migration/lib/src/fix_aggregator.dart index 60e779dd504..2df2b83ebae 100644 --- a/pkg/nnbd_migration/lib/src/fix_aggregator.dart +++ b/pkg/nnbd_migration/lib/src/fix_aggregator.dart @@ -746,6 +746,10 @@ class NodeChangeForDefaultFormalParameter /// contained in the edit. AtomicEditInfo addRequiredKeywordInfo; + /// If [addRequiredKeyword] is `true`, and there is a `/*required*/` hint, + /// the hint comment that should be converted into a simple `required` string. + HintComment requiredHint; + /// If non-null, indicates a `@required` annotation which should be removed /// from this node. Annotation annotationToRemove; @@ -762,8 +766,13 @@ class NodeChangeForDefaultFormalParameter @override EditPlan _apply(DefaultFormalParameter node, FixAggregator aggregator) { - var innerPlan = aggregator.innerPlanForNode(node); - if (!addRequiredKeyword) return innerPlan; + if (!addRequiredKeyword) return aggregator.innerPlanForNode(node); + + if (requiredHint != null) { + var innerPlan = aggregator.innerPlanForNode(node); + return aggregator.planner.acceptPrefixHint(innerPlan, requiredHint, + info: addRequiredKeywordInfo); + } var offset = node.firstTokenAfterCommentAndMetadata.offset; return aggregator.planner.passThrough(node, innerPlans: [ @@ -1205,8 +1214,7 @@ abstract class NodeChangeForType extends NodeChange { if (_makeNullable) { var hint = _nullabilityHint; if (hint != null) { - return aggregator.planner.acceptNullabilityOrNullCheckHint( - innerPlan, hint, + return aggregator.planner.acceptSuffixHint(innerPlan, hint, info: AtomicEditInfo( NullabilityFixDescription.makeTypeNullableDueToHint(typeName), fixReasons, @@ -1302,7 +1310,7 @@ class NodeChangeForVariableDeclarationList var description = lateHint.kind == HintCommentKind.late_ ? NullabilityFixDescription.addLateDueToHint : NullabilityFixDescription.addLateFinalDueToHint; - plan = aggregator.planner.acceptLateHint(plan, lateHint, + plan = aggregator.planner.acceptPrefixHint(plan, lateHint, info: AtomicEditInfo(description, {}, hintComment: lateHint)); } return plan; @@ -1348,8 +1356,7 @@ class NullCheckChange extends ExpressionChange { NodeProducingEditPlan applyExpression(FixAggregator aggregator, NodeProducingEditPlan innerPlan, AtomicEditInfo info) { if (hint != null) { - return aggregator.planner - .acceptNullabilityOrNullCheckHint(innerPlan, hint, info: info); + return aggregator.planner.acceptSuffixHint(innerPlan, hint, info: info); } else { return aggregator.planner .addUnaryPostfix(innerPlan, TokenType.BANG, info: info); diff --git a/pkg/nnbd_migration/lib/src/fix_builder.dart b/pkg/nnbd_migration/lib/src/fix_builder.dart index add21a0402f..1e972be7a16 100644 --- a/pkg/nnbd_migration/lib/src/fix_builder.dart +++ b/pkg/nnbd_migration/lib/src/fix_builder.dart @@ -1113,17 +1113,20 @@ class _FixBuilderPreVisitor extends GeneralizingAstVisitor void visitDefaultFormalParameter(DefaultFormalParameter node) { var element = node.declaredElement; if (node.defaultValue == null) { + var requiredHint = + _fixBuilder._variables.getRequiredHint(_fixBuilder.source, node); var nullabilityNode = _fixBuilder._variables.decoratedElementType(element).node; if (!nullabilityNode.isNullable) { if (element.isNamed) { - _addRequiredKeyword(node, nullabilityNode); + _addRequiredKeyword(node, nullabilityNode, requiredHint); } else { _fixBuilder._addProblem( node, const NonNullableUnnamedOptionalParameter()); } - } else if (element.metadata.any((m) => m.isRequired)) { - _addRequiredKeyword(node, nullabilityNode); + } else if (requiredHint != null || + element.metadata.any((m) => m.isRequired)) { + _addRequiredKeyword(node, nullabilityNode, requiredHint); } } super.visitDefaultFormalParameter(node); @@ -1199,8 +1202,8 @@ class _FixBuilderPreVisitor extends GeneralizingAstVisitor super.visitTypeName(node); } - void _addRequiredKeyword( - DefaultFormalParameter parameter, NullabilityNode node) { + void _addRequiredKeyword(DefaultFormalParameter parameter, + NullabilityNode node, HintComment requiredHint) { // Change an existing `@required` annotation into a `required` keyword if // possible. final element = parameter.declaredElement; @@ -1226,7 +1229,8 @@ class _FixBuilderPreVisitor extends GeneralizingAstVisitor var nodeChange = (_fixBuilder._getChange(parameter) as NodeChangeForDefaultFormalParameter) ..addRequiredKeyword = true - ..addRequiredKeywordInfo = info; + ..addRequiredKeywordInfo = info + ..requiredHint = requiredHint; var requiredAnnotation = metadata?.firstWhere( (annotation) => annotation.elementAnnotation.isRequired, orElse: () => null); diff --git a/pkg/nnbd_migration/test/api_test.dart b/pkg/nnbd_migration/test/api_test.dart index 19248f954f3..b103159e839 100644 --- a/pkg/nnbd_migration/test/api_test.dart +++ b/pkg/nnbd_migration/test/api_test.dart @@ -124,6 +124,32 @@ abstract class _ProvisionalApiTestBase extends AbstractContextTest { /// Mixin containing test cases for the provisional API. mixin _ProvisionalApiTestCases on _ProvisionalApiTestBase { + Future test_accept_required_hint() async { + var content = ''' +f({/*required*/ int i}) {} +'''; + var expected = ''' +f({required int i}) {} +'''; + await _checkSingleFileChanges(content, expected); + } + + Future test_accept_required_hint_nullable() async { + var content = ''' +f({/*required*/ int i}) {} +g() { + f(i: null); +} +'''; + var expected = ''' +f({required int? i}) {} +g() { + f(i: null); +} +'''; + await _checkSingleFileChanges(content, expected); + } + Future test_add_explicit_parameter_type() async { var content = ''' abstract class C { diff --git a/pkg/nnbd_migration/test/edit_plan_test.dart b/pkg/nnbd_migration/test/edit_plan_test.dart index 078318e3e34..d7881a2f96b 100644 --- a/pkg/nnbd_migration/test/edit_plan_test.dart +++ b/pkg/nnbd_migration/test/edit_plan_test.dart @@ -64,7 +64,7 @@ class EditPlanTest extends AbstractSingleUnitTest { await analyze(code); var hint = getPrefixHint(findNode.simple('int').token); var changes = checkPlan( - planner.acceptLateHint( + planner.acceptPrefixHint( planner.passThrough(findNode.simple('int')), hint), 'late int x = 0;'); expect(changes.keys, unorderedEquals([0, 7])); @@ -77,7 +77,7 @@ class EditPlanTest extends AbstractSingleUnitTest { await analyze(code); var hint = getPrefixHint(findNode.simple('int').token); checkPlan( - planner.acceptLateHint( + planner.acceptPrefixHint( planner.passThrough(findNode.simple('int')), hint), 'late int x = 0;'); } @@ -87,7 +87,7 @@ class EditPlanTest extends AbstractSingleUnitTest { await analyze(code); var hint = getPrefixHint(findNode.simple('int').token); checkPlan( - planner.acceptLateHint( + planner.acceptPrefixHint( planner.passThrough(findNode.simple('int')), hint), '@deprecated late int x = 0;'); } @@ -103,7 +103,7 @@ class C { var parameter = findNode.fieldFormalParameter('void this.f(int i)'); var typeName = planner.passThrough(parameter); checkPlan( - planner.acceptNullabilityOrNullCheckHint( + planner.acceptSuffixHint( typeName, getPostfixHint(parameter.parameters.rightParenthesis)), ''' class C { @@ -118,7 +118,7 @@ class C { var parameter = findNode.functionTypedFormalParameter('void g(int i)'); var typeName = planner.passThrough(parameter); checkPlan( - planner.acceptNullabilityOrNullCheckHint( + planner.acceptSuffixHint( typeName, getPostfixHint(parameter.parameters.rightParenthesis)), 'f(void g(int i)?) {}'); } @@ -128,9 +128,7 @@ class C { await analyze(code); var intRef = findNode.simple('int'); var typeName = planner.passThrough(intRef); - checkPlan( - planner.acceptNullabilityOrNullCheckHint( - typeName, getPostfixHint(intRef.token)), + checkPlan(planner.acceptSuffixHint(typeName, getPostfixHint(intRef.token)), 'int? x = 0;'); } @@ -141,7 +139,7 @@ class C { checkPlan( planner.extract( xRef.parent.parent, - planner.acceptNullabilityOrNullCheckHint( + planner.acceptSuffixHint( planner.passThrough(xRef), getPostfixHint(xRef.token))), 'f(x) => x!;'); }