[analysis_server] Insert new arguments before last child/children arguments
Usually when adding arguments, we'd always put them at the end. However there is convention (and a lint) for putting child/children last - so if the arguments in the last position are child/children, we should insert before them.
Change-Id: Ia0e0cd6b10f16cef30aa0cfe4fdedecc223f08c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403584
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart
index 70160c0..3c55664 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart
@@ -267,6 +267,22 @@
);
}
+ /// Returns whether [argument] is a [NamedExpression] with a name of
+ /// 'child' or 'children'.
+ bool _isNamedChildOrChildren(Expression argument) {
+ if (argument is! NamedExpression) {
+ return false;
+ }
+
+ return argument.name.label.name == 'child' ||
+ argument.name.label.name == 'children';
+ }
+
+ /// Returns whether [argument] is _not_ a [NamedExpression] with a name of
+ /// 'child' or 'children'.
+ bool _isNotNamedChildOrChildren(Expression argument) =>
+ !_isNamedChildOrChildren(argument);
+
/// Sends [workspaceEdit] to the client and returns `null` if applied
/// successfully or an error otherwise.
Future<ErrorOr<Null>> _sendEditToClient(WorkspaceEdit workspaceEdit) async {
@@ -367,22 +383,27 @@
parameter.isNamed && parameterName != null ? '$parameterName: ' : '';
var argumentCodeToInsert = '$prefix$newValueCode';
+ // Usually we insert at the end (after the last argument), but if the last
+ // argument is child/children we should go before it.
+ var argumentToInsertAfter = argumentList.arguments.lastWhereOrNull(
+ _isNotNamedChildOrChildren,
+ );
+
// Build the final code to insert.
var newCode = StringBuffer();
- var hasTrailingComma =
- argumentList.arguments.lastOrNull?.endToken.next?.lexeme == ',';
- if (!hasTrailingComma && argumentList.arguments.isNotEmpty) {
+ if (argumentToInsertAfter != null) {
+ // If we're being inserted after an argument, put a new comma between us.
newCode.write(', ');
- } else if (hasTrailingComma) {
- newCode.write(' ');
}
newCode.write(argumentCodeToInsert);
- if (hasTrailingComma) {
- newCode.write(',');
+ if (argumentToInsertAfter == null && argumentList.arguments.isNotEmpty) {
+ // If we're not inserted after an existing argument but there are future
+ // arguments, add a comma in between us.
+ newCode.write(', ');
}
builder.addSimpleInsertion(
- argumentList.rightParenthesis.offset,
+ argumentToInsertAfter?.end ?? argumentList.leftParenthesis.end,
newCode.toString(),
);
}
diff --git a/pkg/analysis_server/test/lsp/edit_argument_test.dart b/pkg/analysis_server/test/lsp/edit_argument_test.dart
index beef9af..5e2a997 100644
--- a/pkg/analysis_server/test/lsp/edit_argument_test.dart
+++ b/pkg/analysis_server/test/lsp/edit_argument_test.dart
@@ -273,6 +273,33 @@
);
}
+ test_named_addAfterNamed_afterChildNotAtEnd() async {
+ await _expectSimpleArgumentEdit(
+ params: '({ int? x, int? y, Widget? child })',
+ originalArgs: '(child: null, x: 1)',
+ edit: ArgumentEdit(name: 'y', newValue: 2),
+ expectedArgs: '(child: null, x: 1, y: 2)',
+ );
+ }
+
+ test_named_addAfterNamed_beforeChildAtEnd() async {
+ await _expectSimpleArgumentEdit(
+ params: '({ int? x, int? y, Widget? child })',
+ originalArgs: '(x: 1, child: null)',
+ edit: ArgumentEdit(name: 'y', newValue: 2),
+ expectedArgs: '(x: 1, y: 2, child: null)',
+ );
+ }
+
+ test_named_addAfterNamed_beforeChildrenAtEnd() async {
+ await _expectSimpleArgumentEdit(
+ params: '({ int? x, int? y, List<Widget>? children })',
+ originalArgs: '(x: 1, children: [])',
+ edit: ArgumentEdit(name: 'y', newValue: 2),
+ expectedArgs: '(x: 1, y: 2, children: [])',
+ );
+ }
+
test_named_addAfterPositional() async {
await _expectSimpleArgumentEdit(
params: '(int? x, { int? y })',
@@ -282,6 +309,33 @@
);
}
+ test_named_addAfterPositional_afterChildNotAtEnd() async {
+ await _expectSimpleArgumentEdit(
+ params: '(int? x, { int? y, Widget? child })',
+ originalArgs: '(child: null, 1)',
+ edit: ArgumentEdit(name: 'y', newValue: 2),
+ expectedArgs: '(child: null, 1, y: 2)',
+ );
+ }
+
+ test_named_addAfterPositional_beforeChildAtEnd() async {
+ await _expectSimpleArgumentEdit(
+ params: '(int? x, { int? y, Widget? child })',
+ originalArgs: '(1, child: null)',
+ edit: ArgumentEdit(name: 'y', newValue: 2),
+ expectedArgs: '(1, y: 2, child: null)',
+ );
+ }
+
+ test_named_addAfterPositional_beforeChildrenAtEnd() async {
+ await _expectSimpleArgumentEdit(
+ params: '(int? x, { int? y, List<Widget>? children })',
+ originalArgs: '(1, children: [])',
+ edit: ArgumentEdit(name: 'y', newValue: 2),
+ expectedArgs: '(1, y: 2, children: [])',
+ );
+ }
+
test_optionalPositional_addAfterPositional() async {
await _expectSimpleArgumentEdit(
params: '([int? x, int? y])',
