Handle private named parameters in ConvertAllFormalParametersToNamed.

If all of the positional parameters in the parameter list are public,
then it happily converts them to public named parameters.

If any of them have private names and they can be private named
parameters, it does that too. For that to be true:

- The library containing the declaration needs to support private named
  parameters.

- The parameter needs to be an initializing formal (or a declaring
  parameter when those are supported). That implies it must also be in
  a constructor.

If there are any positional parameters that are private and don't fit
that, then it doesn't allow the refactoring.

Also, in cases where it does convert a private positional parameter to
a private named parameter, this CL makes sure that the added argument
name is the corresponding public name.

This also fixes #62284 by not applying the refactoring unless the
result yields valid private named parameters.

Fix #62284.

Change-Id: Id00831c2631ff3bf6587ac63d90865f7b0fb367c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/475267
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

Author: munificent

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

@@ -12,6 +12,7 @@ import 'package:analysis_server/src/services/search/hierarchy.dart';
 import 'package:analysis_server/src/services/search/search_engine.dart';
 import 'package:analysis_server_plugin/edit/correction_utils.dart';
 import 'package:analysis_server_plugin/src/utilities/selection.dart';
+import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ -63,11 +64,48 @@ sealed class Available extends Availability {
 
   Available({required this.refactoringContext});
 
-  bool get hasPositionalParameters;
+  /// Whether there are any positional parameters and, if so, if all of them
+  /// can be converted to named parameters.
+  ///
+  /// Even if a parameter is positional, it may not be convertible if it has a
+  /// private name and isn't in a context where it could become a private named
+  /// parameter. If that case, this returns `false`.
+  bool get hasPositionalParametersToConvertToNamed {
+    var supportsPrivateNamedParameters = refactoringContext
+        .resolvedLibraryResult
+        .element
+        .featureSet
+        .isEnabled(Feature.private_named_parameters);
+
+    var hasPositional = false;
+    for (var parameter in _formalParameters) {
+      if (parameter.isNamed) continue;
+
+      hasPositional = true;
+
+      // If the parameter has a private name, we must be able to convert it to
+      // a private named parameter.
+      if (Identifier.isPrivateName(parameter.name!)) {
+        if (!supportsPrivateNamedParameters) {
+          return false;
+        }
+
+        // TODO(rnystrom): Check for primary constructor declaring parameter
+        // here once those are implemented.
+        if (parameter is! FieldFormalParameterElement) {
+          return false;
+        }
+      }
+    }
+
+    return hasPositional;
+  }
 
   bool get hasSelectedFormalParametersToConvertToNamed => false;
 
   bool get hasSelectedFormalParametersToMoveLeft => false;
+
+  List<FormalParameterElement> get _formalParameters;
 }
 
 /// The supertype return types from [computeSourceChange].
@@ -408,11 +446,6 @@ final class _AvailableWithDeclaration extends Available {
     required this.declaration,
   });
 
-  @override
-  bool get hasPositionalParameters {
-    return declaration.element.formalParameters.any((e) => e.isPositional);
-  }
-
   @override
   bool get hasSelectedFormalParametersToConvertToNamed {
     var selected = declaration.selected;
@@ -472,6 +505,10 @@ final class _AvailableWithDeclaration extends Available {
 
     return true;
   }
+
+  @override
+  List<FormalParameterElement> get _formalParameters =>
+      declaration.element.formalParameters;
 }
 
 final class _AvailableWithExecutableElement extends Available {
@@ -483,9 +520,8 @@ final class _AvailableWithExecutableElement extends Available {
   });
 
   @override
-  bool get hasPositionalParameters {
-    return element.formalParameters.any((e) => e.isPositional);
-  }
+  List<FormalParameterElement> get _formalParameters =>
+      element.formalParameters;
 }
 
 /// The target method declaration.

pkg/analysis_server/lib/src/services/refactoring/convert_all_formal_parameters_to_named.dart

@@ -87,6 +87,7 @@ class ConvertAllFormalParametersToNamed extends RefactoringProducer {
     if (availability is! Available) {
       return false;
     }
-    return availability.hasPositionalParameters;
+
+    return availability.hasPositionalParametersToConvertToNamed;
   }
 }

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

@@ -2,6 +2,7 @@
 // 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_impl.dart';
 import 'package:analysis_server/src/services/refactoring/framework/formal_parameter.dart';
 import 'package:analysis_server_plugin/edit/correction_utils.dart';
 import 'package:analyzer/dart/analysis/results.dart';
@@ -49,8 +50,11 @@ Future<WriteArgumentsStatus> writeArguments({
               return WriteArgumentsStatusFailure();
             }
             if (update is FormalParameterUpdateExistingNamed) {
+              // If the parameter's name is private, then the argument uses the
+              // corresponding public name.
+              var name = correspondingPublicName(update.name) ?? update.name;
               newArguments.add(
-                _ArgumentAddName(name: update.name, argument: argument),
+                _ArgumentAddName(name: name, argument: argument),
               );
             } else {
               newArguments.add(_ArgumentAsIs(argument: argument));

pkg/analysis_server/test/src/services/refactoring/convert_all_formal_parameters_to_named_test.dart

@@ -155,6 +155,82 @@ void f() {
     await _assertNoRefactoring();
   }
 
+  Future<void> test_private_constructor() async {
+    addTestSource(r'''
+class C {
+  int? _a;
+  int? _b;
+
+  ^C(this._a, {this._b});
+}
+
+void f() {
+  C(0, b: 1);
+}
+''');
+
+    await verifyRefactoring(r'''
+>>>>>>>>>> lib/main.dart
+class C {
+  int? _a;
+  int? _b;
+
+  C({required this._a, this._b});
+}
+
+void f() {
+  C(a: 0, b: 1);
+}
+''');
+  }
+
+  Future<void> test_private_constructor_unsupported() async {
+    addTestSource(r'''
+// @dart=3.10
+class C {
+  int? _a;
+  int? _b;
+
+  ^C(this._a, {int? b}) : _b = b;
+}
+
+void f() {
+  C(0, b: 1);
+}
+''');
+
+    await _assertNoRefactoring();
+  }
+
+  Future<void> test_private_function() async {
+    // Functions can't have private named parameters, so the refactoring can't
+    // be applied.
+    addTestSource(r'''
+^test(int _a, int _b) {}
+
+void f() {
+  test(0, 1);
+}
+''');
+
+    await _assertNoRefactoring();
+  }
+
+  Future<void> test_private_function_unsupported() async {
+    // Functions can't have private named parameters, so the refactoring can't
+    // be applied.
+    addTestSource(r'''
+// @dart=3.10
+^test(int _a, int _b) {}
+
+void f() {
+  test(0, 1);
+}
+''');
+
+    await _assertNoRefactoring();
+  }
+
   Future<void> test_target_methodInvocation_name() async {
     addTestSource(r'''
 void test(int a) {}