[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 <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Felipe Morschel <git@fmorschel.dev>
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 6704592..6622984 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 @@
}
var diagnostic = this.diagnostic;
- if (diagnostic is! Diagnostic) {
+ if (diagnostic == null) {
return;
}
@@ -80,6 +80,7 @@
// 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 @@
_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<String, DefaultArgument?> 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 048536a..2808fd1 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 @@
''');
}
+ Future<void> test_flutter_closure_children() async {
+ await resolveTestCode('''
+import 'package:flutter/widgets.dart';
+
+class MyWidget extends Widget {
+ MyWidget({required List<Widget> children, required void Function()? fn});
+}
+
+build() {
+ return new MyWidget();
+}
+''');
+ await assertHasFix('''
+import 'package:flutter/widgets.dart';
+
+class MyWidget extends Widget {
+ MyWidget({required List<Widget> children, required void Function()? fn});
+}
+
+build() {
+ return new MyWidget(fn: () { }, children: [],);
+}
+''', errorFilter: (diagnostic) => diagnostic.message.contains("'children'"));
+ }
+
Future<void> test_functionType_noParameterName() async {
await resolveTestCode('''
void foo({required void Function(int) f}) {}
