[analysis_server] Improve the formatting of inserted arguments

This improves the formatting of inserted arguments by including newlines and indentation if an invocation spans multiple lines.

Change-Id: Idef4602ee9ac9f145879a4628190ec44f87a0018
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405122
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Elliott Brooks <[email protected]>

Author: DanTup

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

@@ -12,11 +12,14 @@ import 'package:analysis_server/src/lsp/handlers/custom/editable_arguments/edita
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/lsp/source_edits.dart';
+import 'package:analysis_server_plugin/edit/correction_utils.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/element/element2.dart';
 import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/source/source_range.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
@@ -234,10 +237,13 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
     );
 
     var changeBuilder = ChangeBuilder(session: result.session);
+    var utils = CorrectionUtils(result);
     await changeBuilder.addDartFileEdit(result.path, (builder) {
       if (argument == null) {
         _writeNewArgument(
           builder,
+          utils,
+          result.lineInfo,
           parameters,
           argumentList,
           parameter,
@@ -352,6 +358,8 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
   /// earlier missing positionals.
   void _writeNewArgument(
     DartFileEditBuilder builder,
+    CorrectionUtils utils,
+    LineInfo lineInfo,
     List<FormalParameterElement> parameters,
     ArgumentList argumentList,
     FormalParameterElement parameter,
@@ -377,28 +385,64 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
     }
 
     var parameterName = parameter.name3;
-    var prefix =
+    var argumentNamePrefix =
         parameter.isNamed && parameterName != null ? '$parameterName: ' : '';
-    var argumentCodeToInsert = '$prefix$newValueCode';
+    var argumentCode = '$argumentNamePrefix$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,
     );
 
+    var hasArgumentsBefore = argumentToInsertAfter != null;
+    var hasArgumentsAfter =
+        (argumentToInsertAfter == null && argumentList.arguments.isNotEmpty) ||
+        (argumentToInsertAfter != null &&
+            argumentToInsertAfter != argumentList.arguments.last);
+    var commaFollowsInsertion =
+        argumentToInsertAfter?.endToken.next?.type == TokenType.COMMA;
+
+    // If the invocation is already across more than one line, we will format
+    // accordingly. If it is already on one line, keep it that way.
+    var isMultiline =
+        lineInfo.getLocation(argumentList.leftParenthesis.offset).lineNumber !=
+        lineInfo.getLocation(argumentList.rightParenthesis.offset).lineNumber;
+
+    // If we are multiline, indent one level more than the invocation.
+    var indent =
+        isMultiline
+            ? '${utils.getLinePrefix(argumentList.leftParenthesis.offset)}  '
+            : '';
+
+    // The prefix we need depends on whether there is an argument before us
+    // and whether we are multiline.
+    var codePrefix = switch ((isMultiline, hasArgumentsBefore)) {
+      (true, true) => ',${utils.endOfLine}$indent',
+      (true, false) => '${utils.endOfLine}$indent',
+      (false, true) => ', ',
+      (false, false) => '',
+    };
+
+    // The suffix depends on whether there is an argument after us and whether
+    // we are multiline. If there is an argument after us, there is already
+    // the correct whitespace and comma after our insertion point.
+    var codeSuffix = switch ((
+      isMultiline,
+      hasArgumentsAfter,
+      commaFollowsInsertion,
+    )) {
+      (_, true, _) => '',
+      (true, false, false) => ',',
+      (true, false, true) => '',
+      (false, false, _) => '',
+    };
+
     // Build the final code to insert.
     var newCode = StringBuffer();
-    if (argumentToInsertAfter != null) {
-      // If we're being inserted after an argument, put a new comma between us.
-      newCode.write(', ');
-    }
-    newCode.write(argumentCodeToInsert);
-    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(', ');
-    }
+    newCode.write(codePrefix);
+    newCode.write(argumentCode);
+    newCode.write(codeSuffix);
 
     builder.addSimpleInsertion(
       argumentToInsertAfter?.end ?? argumentList.leftParenthesis.end,

pkg/analysis_server/test/shared/shared_edit_argument_tests.dart

@@ -66,6 +66,54 @@ mixin SharedEditArgumentTests
     );
   }
 
+  test_format_multiline_insert_between() async {
+    await _expectSimpleArgumentEdit(
+      params: '({ int? x, int? y, int? children })',
+      originalArgs: '''\n
+      (
+        x: 1,
+        children: 3,
+      )''',
+      edit: ArgumentEdit(name: 'y', newValue: 2),
+      expectedArgs: '''\n
+      (
+        x: 1,
+        y: 2,
+        children: 3,
+      )''',
+    );
+  }
+
+  test_format_multiline_insert_last() async {
+    await _expectSimpleArgumentEdit(
+      params: '({ int? x, int? y })',
+      originalArgs: '''\n
+      (
+        x: 1,
+      )''',
+      edit: ArgumentEdit(name: 'y', newValue: 2),
+      expectedArgs: '''\n
+      (
+        x: 1,
+        y: 2,
+      )''',
+    );
+  }
+
+  test_format_multiline_insert_solo() async {
+    await _expectSimpleArgumentEdit(
+      params: '({ int? x })',
+      originalArgs: '''\n
+      (
+      )''',
+      edit: ArgumentEdit(name: 'x', newValue: 1),
+      expectedArgs: '''\n
+      (
+        x: 1,
+      )''',
+    );
+  }
+
   test_named_addAfterNamed() async {
     await _expectSimpleArgumentEdit(
       params: '({ int? x, int? y })',