[analysis_server] Insert new arguments before last child/children arg…

…uments

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 <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart

@@ -267,6 +267,22 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
     );
   }
 
+  /// 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 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
         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(),
     );
   }

pkg/analysis_server/test/lsp/edit_argument_test.dart

@@ -273,6 +273,33 @@ class EditArgumentTest extends AbstractLspAnalysisServerTest {
     );
   }
 
+  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 @@ class EditArgumentTest extends AbstractLspAnalysisServerTest {
     );
   }
 
+  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])',