[CMSR] Support for removing formal parameters.

Change-Id: Id04f6c7854dec3d5059009a0a579110690d3eb62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310963
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>

Author: scheglov

pkg/analysis_server/lib/src/services/refactoring/agnostic/change_method_signature.dart

@@ -171,10 +171,14 @@ class MethodSignatureUpdate {
   /// class hierarchy, will be updated. The new formal parameters will be
   /// written in the order [formalParameters] field, with new kinds.
   ///
-  /// TODO(scheglov) Test removing.
   /// TODO(scheglov) Consider adding.
   final List<FormalParameterUpdate> formalParameters;
 
+  /// Normally, after writing formal parameters in the order specified by
+  /// [formalParameters], we write any remaining named formal parameters.
+  /// So, here we record names that are explicitly removed.
+  final Set<String> removedNamedFormalParameters;
+
   /// Specifies whether to add the trailing comma after formal parameters.
   ///
   /// TODO(scheglov) Test for empty formal parameter lists.
@@ -185,6 +189,7 @@ class MethodSignatureUpdate {
 
   MethodSignatureUpdate({
     required this.formalParameters,
+    this.removedNamedFormalParameters = const {},
     required this.formalParametersTrailingComma,
     required this.argumentsTrailingComma,
   });
@@ -684,6 +689,8 @@ class _SignatureUpdater {
             );
         }
       }).toList(),
+      removedNamedFormalParameters:
+          signatureUpdate.removedNamedFormalParameters,
       trailingComma: signatureUpdate.argumentsTrailingComma,
       resolvedUnit: resolvedUnit,
       argumentList: argumentList,
@@ -778,8 +785,8 @@ class _SignatureUpdater {
           return ChangeStatusFailure();
         }
       } else {
-        existing =
-            existingFormalParameters.named.remove(formalParameterState.name);
+        final name = formalParameterState.name;
+        existing = existingFormalParameters.named.remove(name);
         if (existing == null) {
           continue;
         }
@@ -808,10 +815,14 @@ class _SignatureUpdater {
     }
 
     // Add back remaining named formal parameters.
-    for (final existing in existingFormalParameters.named.values) {
-      final text = utils.getNodeText(existing);
+    final removedNamed = signatureUpdate.removedNamedFormalParameters;
+    existingFormalParameters.named.forEach((name, node) {
+      if (removedNamed.contains(name)) {
+        return;
+      }
+      final text = utils.getNodeText(node);
       namedWrites.add(text);
-    }
+    });
 
     await builder.addDartFileEdit(path, (builder) {
       builder.addReplacement(

pkg/analysis_server/lib/src/services/refactoring/framework/write_invocation_arguments.dart

@@ -15,6 +15,7 @@ import 'package:collection/collection.dart';
 /// by the formal parameter updates, reordering, changing kind, etc.
 Future<WriteArgumentsStatus> writeArguments({
   required List<FormalParameterUpdate> formalParameterUpdates,
+  required Set<String> removedNamedFormalParameters,
   required ArgumentsTrailingComma trailingComma,
   required ResolvedUnitResult resolvedUnit,
   required ArgumentList argumentList,
@@ -31,7 +32,8 @@ Future<WriteArgumentsStatus> writeArguments({
       case FormalParameterUpdateExisting(:final reference):
         switch (reference) {
           case NamedFormalParameterReference():
-            final argument = namedArguments.remove(reference.name);
+            final name = reference.name;
+            final argument = namedArguments.remove(name);
             if (argument == null) {
               continue;
             }
@@ -86,13 +88,16 @@ Future<WriteArgumentsStatus> writeArguments({
   }
 
   // Add remaining named arguments.
-  for (final argument in namedArguments.values) {
+  namedArguments.forEach((name, argument) {
+    if (removedNamedFormalParameters.contains(name)) {
+      return;
+    }
     newArguments.add(
       _ArgumentAsIs(
         argument: argument,
       ),
     );
-  }
+  });
 
   await builder.addDartFileEdit(resolvedUnit.path, (builder) {
     builder.addReplacement(range.node(argumentList), (builder) {

pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart

@@ -1393,6 +1393,69 @@ void f(A a, B b) {
 ''');
   }
 
+  Future<void> test_classMethod_requiredNamed_remove() async {
+    await _analyzeValidSelection(r'''
+class A {
+  void ^test({
+    required int a,
+    required int b,
+    required int c,
+  }) {}
+}
+
+class B extends A {
+  void test({
+    required int a,
+    required int b,
+    required int c,
+  }) {}
+}
+
+void f(A a, B b) {
+  a.test(a: 0, b: 1, c: 2);
+  b.test(a: 3, b: 4, c: 5);
+}
+''');
+
+    final signatureUpdate = MethodSignatureUpdate(
+      formalParameters: [
+        FormalParameterUpdate(
+          id: 0,
+          kind: FormalParameterKind.requiredNamed,
+        ),
+        FormalParameterUpdate(
+          id: 2,
+          kind: FormalParameterKind.requiredNamed,
+        ),
+      ],
+      removedNamedFormalParameters: {'b'},
+      formalParametersTrailingComma: TrailingComma.ifPresent,
+      argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
+    );
+
+    await _assertUpdate(signatureUpdate, r'''
+>>>>>>> /home/test/lib/test.dart
+class A {
+  void test({
+    required int a,
+    required int c,
+  }) {}
+}
+
+class B extends A {
+  void test({
+    required int a,
+    required int c,
+  }) {}
+}
+
+void f(A a, B b) {
+  a.test(a: 0, c: 2);
+  b.test(a: 3, c: 5);
+}
+''');
+  }
+
   Future<void> test_classMethod_requiredPositional_reorder() async {
     await _analyzeValidSelection(r'''
 class A {
@@ -1769,6 +1832,41 @@ void f() {
 ''');
   }
 
+  Future<void> test_topFunction_optionalNamed_remove() async {
+    await _analyzeValidSelection(r'''
+void ^test({int a, int b, int c}) {}
+
+void f() {
+  test(a: 0, b: 1, c: 2);
+}
+''');
+
+    final signatureUpdate = MethodSignatureUpdate(
+      formalParameters: [
+        FormalParameterUpdate(
+          id: 0,
+          kind: FormalParameterKind.optionalNamed,
+        ),
+        FormalParameterUpdate(
+          id: 2,
+          kind: FormalParameterKind.optionalNamed,
+        ),
+      ],
+      removedNamedFormalParameters: {'b'},
+      formalParametersTrailingComma: TrailingComma.ifPresent,
+      argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
+    );
+
+    await _assertUpdate(signatureUpdate, r'''
+>>>>>>> /home/test/lib/test.dart
+void test({int a, int c}) {}
+
+void f() {
+  test(a: 0, c: 2);
+}
+''');
+  }
+
   Future<void> test_topFunction_optionalNamed_reorder() async {
     await _analyzeValidSelection(r'''
 void ^test({
@@ -1994,6 +2092,40 @@ void f() {
 ''');
   }
 
+  Future<void> test_topFunction_optionalPositional_remove() async {
+    await _analyzeValidSelection(r'''
+void ^test([int? a, int? b, int? c]) {}
+
+void f() {
+  test(0, 1, 2);
+}
+''');
+
+    final signatureUpdate = MethodSignatureUpdate(
+      formalParameters: [
+        FormalParameterUpdate(
+          id: 0,
+          kind: FormalParameterKind.optionalPositional,
+        ),
+        FormalParameterUpdate(
+          id: 2,
+          kind: FormalParameterKind.optionalPositional,
+        ),
+      ],
+      formalParametersTrailingComma: TrailingComma.ifPresent,
+      argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
+    );
+
+    await _assertUpdate(signatureUpdate, r'''
+>>>>>>> /home/test/lib/test.dart
+void test([int? a, int? c]) {}
+
+void f() {
+  test(0, 2);
+}
+''');
+  }
+
   Future<void> test_topFunction_optionalPositional_reorder() async {
     await _analyzeValidSelection(r'''
 void ^test([int a, double b]) {}
@@ -2193,6 +2325,48 @@ ChangeStatusFailure
 ''');
   }
 
+  Future<void> test_topFunction_requiredNamed_remove() async {
+    await _analyzeValidSelection(r'''
+void ^test({
+  required int a,
+  required int b,
+  required int c,
+}) {}
+
+void f() {
+  test(a: 0, b: 1, c: 2);
+}
+''');
+
+    final signatureUpdate = MethodSignatureUpdate(
+      formalParameters: [
+        FormalParameterUpdate(
+          id: 0,
+          kind: FormalParameterKind.requiredNamed,
+        ),
+        FormalParameterUpdate(
+          id: 2,
+          kind: FormalParameterKind.requiredNamed,
+        ),
+      ],
+      removedNamedFormalParameters: {'b'},
+      formalParametersTrailingComma: TrailingComma.ifPresent,
+      argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
+    );
+
+    await _assertUpdate(signatureUpdate, r'''
+>>>>>>> /home/test/lib/test.dart
+void test({
+  required int a,
+  required int c,
+}) {}
+
+void f() {
+  test(a: 0, c: 2);
+}
+''');
+  }
+
   Future<void> test_topFunction_requiredNamed_reorder() async {
     await _analyzeValidSelection(r'''
 void ^test({
@@ -2387,6 +2561,40 @@ void f() {
 ''');
   }
 
+  Future<void> test_topFunction_requiredPositional_remove() async {
+    await _analyzeValidSelection(r'''
+void ^test(int a, int b, int c) {}
+
+void f() {
+  test(0, 1, 2);
+}
+''');
+
+    final signatureUpdate = MethodSignatureUpdate(
+      formalParameters: [
+        FormalParameterUpdate(
+          id: 0,
+          kind: FormalParameterKind.requiredPositional,
+        ),
+        FormalParameterUpdate(
+          id: 2,
+          kind: FormalParameterKind.requiredPositional,
+        ),
+      ],
+      formalParametersTrailingComma: TrailingComma.ifPresent,
+      argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
+    );
+
+    await _assertUpdate(signatureUpdate, r'''
+>>>>>>> /home/test/lib/test.dart
+void test(int a, int c) {}
+
+void f() {
+  test(0, 2);
+}
+''');
+  }
+
   Future<void> test_topFunction_requiredPositional_reorder() async {
     await _analyzeValidSelection(r'''
 void ^test(int a, double b) {}