From 443e8ccb422f5229c19bb005e67a454502bac79a Mon Sep 17 00:00:00 2001 From: FMorschel Date: Thu, 25 Sep 2025 15:18:54 -0700 Subject: [PATCH] [DAS] Fixes multiple commas by `Add missing required arguments` This also refactors the correction producer so that it is a bit easier to read and maintain. Fixes: https://github.com/dart-lang/sdk/issues/61531 Change-Id: I54656f3e917764881bc4817f7b9692bb9fd973b5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450360 Commit-Queue: Brian Wilkerson Reviewed-by: Brian Wilkerson Reviewed-by: Samuel Rawlins Auto-Submit: Felipe Morschel --- .../dart/add_missing_required_argument.dart | 210 +++++++++++------- .../add_missing_required_argument_test.dart | 25 +++ 2 files changed, 152 insertions(+), 83 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_required_argument.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_required_argument.dart index 6704592e297..6622984dd76 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_missing_required_argument.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_missing_required_argument.dart @@ -2,12 +2,12 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'package:_fe_analyzer_shared/src/scanner/token.dart'; import 'package:analysis_server/src/services/completion/dart/utilities.dart'; import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analysis_server_plugin/src/correction/fix_generators.dart'; import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/diagnostic/diagnostic.dart'; import 'package:analyzer/error/error.dart'; @@ -68,7 +68,7 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer { } var diagnostic = this.diagnostic; - if (diagnostic is! Diagnostic) { + if (diagnostic == null) { return; } @@ -80,6 +80,7 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer { // Should not happen since the current diagnostic is in the list of errors // where this fix is valid. if (diagnostics.isEmpty) { + assert(false, 'No valid diagnostics found, but expected at least one.'); diagnostics = [diagnostic]; } @@ -91,92 +92,135 @@ class AddMissingRequiredArgument extends ResolvedCorrectionProducer { _missingParameters = diagnostics.length; - for (var (index, diagnostic) in diagnostics.indexed) { - if (targetElement is ExecutableElement && argumentList != null) { - // Format: "Missing required argument 'foo'." - var messageParts = diagnostic.problemMessage - .messageText(includeUrl: false) - .split("'"); - if (messageParts.length < 2) { - return; - } + if (argumentList == null || targetElement is! ExecutableElement) { + return; + } + var arguments = argumentList.arguments; - var parameterName = messageParts[1]; - - var missingParameter = targetElement.formalParameters.firstWhereOrNull( - (p) => p.name == parameterName, - ); - if (missingParameter == null) { - return; - } - - int offset; - var hasTrailingComma = false; - var insertBetweenParams = false; - var arguments = argumentList.arguments; - if (arguments.isEmpty) { - offset = argumentList.leftParenthesis.end; - } else { - var lastArgument = arguments.last; - offset = lastArgument.end; - hasTrailingComma = - lastArgument.endToken.next!.type == TokenType.COMMA; - - if (lastArgument is NamedExpression && creation.isWidgetExpression) { - if (lastArgument.isChildArgument || - lastArgument.isChildrenArgument) { - offset = lastArgument.offset; - hasTrailingComma = true; - insertBetweenParams = true; - } + int offset; + var insertLeadingComma = false; + var insertFlutterTrailingComma = true; + var insertBetweenFlutterParams = false; + if (arguments.isEmpty) { + offset = argumentList.leftParenthesis.end; + } else { + var lastArgument = arguments.last; + offset = lastArgument.end; + insertLeadingComma = true; + if (creation.isWidgetExpression) { + // If the last argument is a `child:` or `children:` parameter in a + // Flutter widget, insert the new parameter(s) before it. + if (lastArgument is NamedExpression) { + if (lastArgument.isChildArgument || lastArgument.isChildrenArgument) { + offset = lastArgument.offset; + insertBetweenFlutterParams = true; + // Insert a leading comma if we are have a case like: + // ```dart + // MyWidget(other: 1, child: ...,) + // ``` + // so that we get: + // ```dart + // MyWidget(other: 1, newParam: ..., child: ...,) + // ``` + // since the offset is at the end of `other: 1`. + insertLeadingComma = arguments.length > 1; } } - - var codeStyleOptions = getCodeStyleOptions(unitResult.file); - var defaultValue = getDefaultStringParameterValue( - missingParameter, - codeStyleOptions.preferredQuoteForStrings, - ); - - await builder.addDartFileEdit(file, (builder) { - builder.addInsertion(offset, (builder) { - if ((arguments.isNotEmpty || index > 0) && !insertBetweenParams) { - builder.write(', '); - } - - builder.write('$parameterName: '); - - // Use defaultValue.cursorPosition if it's not null. - if (defaultValue != null) { - var text = defaultValue.text; - var cursorPosition = defaultValue.cursorPosition; - if (cursorPosition != null) { - builder.write(text.substring(0, cursorPosition)); - builder.selectHere(); - builder.write(text.substring(cursorPosition)); - } else { - builder.addSimpleLinkedEdit('VALUE', text); - } - } else { - builder.addSimpleLinkedEdit('VALUE', 'null'); - } - - if (creation.isWidgetExpression) { - // Insert a trailing comma after Flutter instance creation params. - if (!hasTrailingComma) { - builder.write(','); - } else if (insertBetweenParams) { - builder.writeln(','); - - // Insert indent before the child: or children: param. - var indent = utils.getLinePrefix(offset); - builder.write(indent); - } - } - }); - }); + if (lastArgument.endToken.next case Token( + type: TokenType.COMMA, + ) when !insertBetweenFlutterParams) { + // If there is a trailing comma after the last argument, don't add + // another one. + insertFlutterTrailingComma = false; + } } } + + Map parameters = {}; + for (var diagnostic in diagnostics) { + // Format: "Missing required argument 'foo'." + var messageParts = diagnostic.problemMessage + .messageText(includeUrl: false) + .split("'"); + if (messageParts.length < 2) { + return; + } + + var parameterName = messageParts[1]; + + var missingParameter = targetElement.formalParameters.firstWhereOrNull( + (p) => p.name == parameterName, + ); + if (missingParameter == null) { + assert( + false, + 'Could not find parameter $parameterName, but expected since it was ' + 'reported as missing.', + ); + continue; + } + + var codeStyleOptions = getCodeStyleOptions(unitResult.file); + var defaultValue = getDefaultStringParameterValue( + missingParameter, + codeStyleOptions.preferredQuoteForStrings, + ); + + parameters[parameterName] = defaultValue; + } + + var parametersLength = parameters.length; + + await builder.addDartFileEdit(file, (builder) { + builder.addInsertion(offset, (builder) { + if (insertLeadingComma) { + builder.write(', '); + } + var entries = parameters.entries.toList(); + var childOrChildren = entries.firstWhereOrNull( + (e) => const {'child', 'children'}.contains(e.key), + ); + if (childOrChildren != null) { + // If there is a child: or children: parameter, insert it last. + entries.remove(childOrChildren); + entries.add(childOrChildren); + } + entries.forEachIndexed((index, entry) { + var MapEntry(key: parameterName, value: defaultValue) = entry; + + builder.write('$parameterName: '); + + // Use defaultValue.cursorPosition if it's not null. + if (defaultValue != null) { + var text = defaultValue.text; + var cursorPosition = defaultValue.cursorPosition; + if (cursorPosition != null) { + builder.write(text.substring(0, cursorPosition)); + builder.selectHere(); + builder.write(text.substring(cursorPosition)); + } else { + builder.addSimpleLinkedEdit('VALUE', text); + } + } else { + builder.addSimpleLinkedEdit('VALUE', 'null'); + } + + if (index < parametersLength - 1) { + builder.write(', '); + } else if (insertBetweenFlutterParams) { + // Insert a trailing comma after Flutter instance creation params. + builder.writeln(','); + // Insert indent before the child: or children: param. + var indent = utils.getLinePrefix(offset); + builder.write(indent); + } else if (creation.isWidgetExpression && + insertFlutterTrailingComma) { + // If this is a Flutter widget, format with a trailing comma. + builder.write(','); + } + }); + }); + }); } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_missing_required_argument_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_missing_required_argument_test.dart index 048536a7edf..2808fd1fd7c 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/add_missing_required_argument_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/add_missing_required_argument_test.dart @@ -319,6 +319,31 @@ void f() { '''); } + Future test_flutter_closure_children() async { + await resolveTestCode(''' +import 'package:flutter/widgets.dart'; + +class MyWidget extends Widget { + MyWidget({required List children, required void Function()? fn}); +} + +build() { + return new MyWidget(); +} +'''); + await assertHasFix(''' +import 'package:flutter/widgets.dart'; + +class MyWidget extends Widget { + MyWidget({required List children, required void Function()? fn}); +} + +build() { + return new MyWidget(fn: () { }, children: [],); +} +''', errorFilter: (diagnostic) => diagnostic.message.contains("'children'")); + } + Future test_functionType_noParameterName() async { await resolveTestCode(''' void foo({required void Function(int) f}) {}