[analysis_server] Support prompting for name for "Add Import Prefix" refactor

If the client supports Interactive Forms, this will allow prompting for a name for the import prefix instead of using "prefix" (or "prefix1", etc.). Includes moving some boilerplate out of each refactor into `ParameterizedRefactoringProducer` and support for a custom validation function for form fields (to validate the import prefix name in this case). Change-Id: I7150664a18944c723eeeac0a309341af2860201f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/509340 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Samuel Rawlins <srawlins@google.com>
2026-06-04 10:41:15 -07:00
parent 43f3a50349
commit 8f30cb7e43
7 changed files with 411 additions and 62 deletions
@@ -45,6 +45,12 @@ class CommandResolveHandler
    MessageInfo message,
    CancellationToken token,
  ) async {
+    // If the experiment is not enabled, never do any work, just pass the
+    // command back as-is.
+    if (!server.lspClientConfiguration.global.experimentalInteractiveForms) {
+      return success(command);
+    }
+
    if (RefactoringProcessor.generators[command.command] case var generator?) {
      return await _handleRefactorCommand(command, generator, message, token);
    }
@@ -53,7 +59,7 @@ class CommandResolveHandler
  }

  /// Handles resolving a command that relates to a refactor by using
-  /// [RefactorCommandResolver].
+  /// [RefactorCommandResolver] to delegate to the [RefactoringProducer].
  Future<ErrorOr<InteractiveExecuteCommandParams>> _handleRefactorCommand(
    InteractiveExecuteCommandParams command,
    RefactoringProducerGenerator generator,
@@ -182,12 +182,19 @@ class InteractiveForm {
      return null;
    }

-    return switch (field.type) {
+    var errorMessage = switch (field.type) {
      FormFieldTypeFile() => _validateFile(answerValue),
      FormFieldTypeBool() => _validateBool(answerValue),
      FormFieldTypeNumber() => _validateNumber(answerValue),
      FormFieldTypeString() => _validateString(answerValue),
    };
+
+    // Handle fields with custom validation.
+    if (field is ValidatableFormField) {
+      errorMessage ??= field._validate(answerValue);
+    }
+
+    return errorMessage;
  }

  /// Validates this value is a valid boolean, returning a user-facing error
@@ -217,6 +224,25 @@ class InteractiveForm {
  }
 }

+/// A [FormField] with custom validation.
+class ValidatableFormField extends FormField {
+  /// A custom validation function for this field.
+  ///
+  /// Returns `null` if the value is valid, otherwise a validation error
+  /// message.
+  final String? Function(Object? value) _validate;
+
+  new({
+    super.defaultValue,
+    required super.description,
+    super.error,
+    required super.id,
+    required super.required,
+    required super.type,
+    required this._validate,
+  });
+}
+
 extension FormFieldExtension on FormField {
  FormField withError(String? error) {
    if (this.error == error) {
@@ -4,7 +4,10 @@

 import 'package:_fe_analyzer_shared/src/scanner/token.dart' show Keyword;
 import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:analysis_server/src/lsp/error_or.dart';
+import 'package:analysis_server/src/services/interactive_forms/interactive_forms.dart';
 import 'package:analysis_server/src/services/refactoring/framework/refactoring_producer.dart';
+import 'package:analysis_server/src/services/refactoring/legacy/naming_conventions.dart';
 import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart';
 import 'package:analysis_server/src/services/search/search_engine_internal.dart';
 import 'package:analysis_server/src/utilities/extensions/selection.dart';
@@ -13,11 +16,12 @@ import 'package:analyzer/source/source.dart' show Source;
 import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/utilities/extensions/element.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
+import 'package:language_server_protocol/protocol_custom_generated.dart';
 import 'package:language_server_protocol/protocol_generated.dart';
 import 'package:path/path.dart' as path;

 /// The refactoring that adds a prefix to an import directive.
-class AddImportPrefix extends RefactoringProducer {
+class AddImportPrefix extends ParameterizedRefactoringProducer {
  static const String commandName = 'dart.refactor_add.import_prefix';

  static const String constTitle = 'Add a prefix to the import';
@@ -32,14 +36,51 @@ class AddImportPrefix extends RefactoringProducer {
  @override
  CodeActionKind get kind => DartCodeActionKind.refactorAdd;

+  @override
+  /// This refactor supports input using the new system (see
+  /// [buildInteractiveForm]) but not using the old one, so there are no
+  /// parameters.
+  List<CommandParameter> get parameters => [];
+
  @override
  String get title => constTitle;

+  /// Builds the [InteractiveForm] to collect input for this refactor.
+  @override
+  ErrorOr<InteractiveForm> buildInteractiveForm() {
+    var element = selection?.importDirective(mustNotHavePrefix: true);
+    if (element == null) {
+      // We shouldn't have gotten here if the selection was not valid for this
+      // refactor, but return a useful error to aid debugging if so.
+      return error(
+        ErrorCodes.InvalidParams,
+        'The selection is not valid for adding an import prefix',
+      );
+    }
+
+    var nameField = ValidatableFormField(
+      id: 'name',
+      description: 'Import Prefix',
+      required: true,
+      defaultValue: _computeName(element),
+      type: FormFieldTypeString(),
+      validate: wrapRefactorValidationFunction(validateImportPrefixName),
+    );
+
+    return success(createForm([nameField]));
+  }
+
  @override
  Future<ComputeStatus> compute(
    List<Object?> commandArguments,
    ChangeBuilder builder,
  ) async {
+    // Handle optional name in the arguments (if Interactive Forms were used).
+    var prefixName = switch (commandArguments) {
+      [String name] => name,
+      _ => null,
+    };
+
    var element = selection?.importDirective(mustNotHavePrefix: true);
    if (element == null) {
      // This should never happen because `isAvailable` would have returned
@@ -51,7 +92,8 @@ class AddImportPrefix extends RefactoringProducer {
    if (refactoring == null) {
      return ComputeStatusFailure();
    }
-    refactoring.newName = _computeName(element);
+    prefixName ??= _computeName(element);
+    refactoring.newName = prefixName;
    var status = await refactoring.checkAllConditions();
    if (status.hasError) {
      return ComputeStatusFailure();
@@ -3,9 +3,12 @@
 // BSD-style license that can be found in the LICENSE file.

 import 'package:analysis_server/src/lsp/error_or.dart';
+import 'package:analysis_server/src/services/correction/status.dart';
+import 'package:analysis_server/src/services/interactive_forms/interactive_forms.dart';
 import 'package:analysis_server/src/services/refactoring/framework/refactoring_context.dart';
 import 'package:analysis_server/src/services/refactoring/framework/refactoring_processor.dart';
 import 'package:analysis_server/src/services/search/search_engine.dart';
+import 'package:analysis_server/src/utilities/extensions/list.dart';
 import 'package:analysis_server_plugin/edit/correction_utils.dart';
 import 'package:analysis_server_plugin/src/utilities/selection.dart';
 import 'package:analyzer/dart/analysis/results.dart';
@@ -42,10 +45,65 @@ abstract class ParameterizedRefactoringProducer extends RefactoringProducer {
    return RefactoringProcessor.buildCommandArguments(refactoringContext, args);
  }

-  /// Resolves command arguments using the interactive forms functionality.
+  /// Builds the interative form for this refactor.
+  ErrorOr<InteractiveForm> buildInteractiveForm();
+
+  /// A helper to create an [InteractiveForm] with client capabilities.
+  InteractiveForm createForm(List<FormField> fields) {
+    var supportedInteractiveFormInputTypes =
+        refactoringContext
+            .clientCapabilities
+            ?.supportedInteractiveFormInputTypes ??
+        {};
+
+    return InteractiveForm(
+      supportedInteractiveFormInputTypes: supportedInteractiveFormInputTypes,
+      fields: fields,
+    );
+  }
+
+  /// Resolves the command used for invoking this refactor by calling
+  /// [buildInteractiveForm] to build the interactive form.
  Future<ErrorOr<InteractiveExecuteCommandParams>> resolve(
    InteractiveExecuteCommandParams command,
-  );
+  ) async {
+    var commandArguments = command.arguments;
+    if (commandArguments == null) {
+      return error(
+        ErrorCodes.InvalidParams,
+        'Refactor commands must have arguments',
+      );
+    }
+
+    return buildInteractiveForm().mapResultSync((form) {
+      form.processResponse(command.formAnswers ?? []);
+
+      return success(
+        InteractiveExecuteCommandParams(
+          command: command.command,
+          arguments: buildCommandArguments(form.answers),
+          data: command.data,
+          formFields: form.clientFields.nullIfEmpty,
+          formAnswers: form.clientAnswers.nullIfEmpty,
+        ),
+      );
+    });
+  }
+
+  /// Wraps a refactor validation function that returns [RefactoringStatus]
+  /// to return a String error message if there are fatal errors, matching the
+  /// validation of Interactive Forms.
+  String? Function(Object? value) wrapRefactorValidationFunction<T>(
+    RefactoringStatus Function(T name) validator,
+  ) {
+    return (Object? value) {
+      var result = validator(value as T);
+      if (result.hasFatalError) {
+        return result.message ?? 'invalid value';
+      }
+      return null;
+    };
+  }
 }

 /// An object that can compute a refactoring in a Dart file.
@@ -7,7 +7,6 @@ import 'package:analysis_server/src/lsp/error_or.dart';
 import 'package:analysis_server/src/services/interactive_forms/interactive_forms.dart';
 import 'package:analysis_server/src/services/refactoring/framework/refactoring_producer.dart';
 import 'package:analysis_server/src/utilities/extensions/ast.dart';
-import 'package:analysis_server/src/utilities/extensions/list.dart';
 import 'package:analysis_server/src/utilities/extensions/string.dart';
 import 'package:analysis_server/src/utilities/import_analyzer.dart';
 import 'package:analyzer/dart/ast/ast.dart';
@@ -62,6 +61,24 @@ class MoveTopLevelToFile extends ParameterizedRefactoringProducer {
    ),
  ];

+  /// Builds the [InteractiveForm] to collect input for this refactor.
+  @override
+  ErrorOr<InteractiveForm> buildInteractiveForm() {
+    var destinationUriField = FormField(
+      id: 'destinationUri',
+      description: 'Move to file',
+      required: true,
+      defaultValue: refactoringContext.server.pathContext
+          .toUri(defaultFilePath)
+          .toString(),
+      type: FormFieldTypeFile(type: .Regular, filters: ['dart']),
+    );
+
+    var form = createForm([destinationUriField]);
+
+    return success(form);
+  }
+
  @override
  Future<ComputeStatus> compute(
    List<Object?> commandArguments,
@@ -210,34 +227,6 @@ class MoveTopLevelToFile extends ParameterizedRefactoringProducer {
    return false;
  }

-  /// Handles resolving the command to execute this refactor using Interactive
-  /// Forms.
-  @override
-  Future<ErrorOr<InteractiveExecuteCommandParams>> resolve(
-    InteractiveExecuteCommandParams command,
-  ) async {
-    var commandArguments = command.arguments;
-    if (commandArguments == null) {
-      return error(
-        ErrorCodes.InvalidParams,
-        'Refactor commands must have arguments',
-      );
-    }
-
-    var form = _createInteractiveForm()
-      ..processResponse(command.formAnswers ?? []);
-
-    return success(
-      InteractiveExecuteCommandParams(
-        command: command.command,
-        arguments: buildCommandArguments(form.answers),
-        data: command.data,
-        formFields: form.clientFields.nullIfEmpty,
-        formAnswers: form.clientAnswers.nullIfEmpty,
-      ),
-    );
-  }
-
  /// Use the [builder] to add the imports that need to be added to the library
  /// to which the code is being moved based on the information in the import
  /// [analyzer].
@@ -266,30 +255,6 @@ class MoveTopLevelToFile extends ParameterizedRefactoringProducer {
    }
  }

-  /// Builds the [InteractiveForm] to collect input for this refactor.
-  InteractiveForm _createInteractiveForm() {
-    var destinationUriField = FormField(
-      id: 'destinationUri',
-      description: 'Move to file',
-      required: true,
-      defaultValue: refactoringContext.server.pathContext
-          .toUri(defaultFilePath)
-          .toString(),
-      type: FormFieldTypeFile(type: .Regular, filters: ['dart']),
-    );
-
-    var supportedInteractiveFormInputTypes =
-        refactoringContext
-            .clientCapabilities
-            ?.supportedInteractiveFormInputTypes ??
-        {};
-
-    return InteractiveForm(
-      supportedInteractiveFormInputTypes: supportedInteractiveFormInputTypes,
-      fields: [destinationUriField],
-    );
-  }
-
  /// Initialize the [title] and [defaultFilePath] based on the [members] being
  /// moved.
  void _initializeFromMembers(_MembersToMove members) {
@@ -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:analysis_server/src/services/refactoring/add_import_prefix.dart';
 import 'package:analysis_server/src/services/refactoring/move_top_level_to_file.dart';
 import 'package:language_server_protocol/protocol_custom_generated.dart';
 import 'package:language_server_protocol/protocol_generated.dart';
@@ -17,12 +18,14 @@ void main() {
  defineReflectiveSuite(() {
    defineReflectiveTests(CommandResolveTest);
    defineReflectiveTests(CommandResolveFileInputTest);
+    defineReflectiveTests(CommandResolveStringInputTest);
  });
 }

 /// Tests file inputs in `command/resolve` using the MoveToFile refactor.
@reflectiveTest
-class CommandResolveFileInputTest extends RefactoringTest {
+class CommandResolveFileInputTest extends RefactoringTest
+    with InteractiveFormsExperimentMixin {
  /// Simple file content with a single class named 'A'.
  final simpleClassContent = '''
 class ^A {}
@@ -85,7 +88,7 @@ class ^A {}

  Future<void> test_formFields_notSupported() async {
    // If we don't support the 'file' kind, then we shouldn't get back any
-    // form fields, only the default.
+    // form fields, only the default value in the arguments.
    setSupportedInteractiveFormInputKinds({'number'});

    addTestSource(simpleClassContent);
@@ -263,6 +266,183 @@ class ^A {}
  }
 }

+/// Tests string inputs in `command/resolve` using the AddImportPrefix refactor.
+@reflectiveTest
+class CommandResolveStringInputTest extends RefactoringTest
+    with InteractiveFormsExperimentMixin {
+  final source = '''
+^import 'package:test/main.dart';
+''';
+
+  @override
+  String get refactoringCommandId => AddImportPrefix.commandName;
+
+  String get refactoringTitle => AddImportPrefix.constTitle;
+
+  @override
+  void setUp() {
+    super.setUp();
+
+    // Most of the tests here assume we support string. Tests that do not will
+    // explicitly unset this.
+    setSupportedInteractiveFormInputKinds({'string'});
+  }
+
+  Future<void> test_acceptsValidAnswers() async {
+    addTestSource(source);
+
+    await initializeServer();
+    var action = await expectCodeActionWithTitle(refactoringTitle);
+    var command = action.asCommand;
+
+    // Resolve the command, which should add the outstanding form fields.
+    var resolvedCommand = await resolveCommand(
+      ExecuteCommandParams(
+        command: command.command,
+        arguments: command.arguments,
+      ),
+    );
+
+    // Resolve again, with a valid answer.
+    expect(resolvedCommand.formFields, hasLength(1));
+    var field = resolvedCommand.formFields!.single;
+    resolvedCommand = await resolveCommand(
+      InteractiveExecuteCommandParams(
+        command: resolvedCommand.command,
+        arguments: resolvedCommand.arguments,
+        formFields: resolvedCommand.formFields,
+        formAnswers: [field.answer('custom_prefix')],
+      ),
+    );
+
+    // Check that the form field has gone, and that the valid answer was moved
+    // into the command arguments.
+    expect(resolvedCommand.formFields, isNull);
+    var arguments = getRefactorCommandArguments(resolvedCommand.arguments);
+    expect(arguments, hasLength(1));
+    expect(arguments.single, 'custom_prefix');
+  }
+
+  Future<void> test_formFields_notSupported() async {
+    // If we don't support the 'string' kind, then we shouldn't get back any
+    // form fields, only the default value in the arguments.
+    setSupportedInteractiveFormInputKinds({'file'});
+
+    addTestSource(source);
+
+    await initializeServer();
+    var action = await expectCodeActionWithTitle(refactoringTitle);
+    var command = action.asCommand;
+
+    // Resolve the command, which would normally add the form fields, but the
+    // only one used here is not supported by us.
+    var resolvedCommand = await resolveCommand(
+      ExecuteCommandParams(
+        command: command.command,
+        arguments: command.arguments,
+      ),
+    );
+
+    // Check basics.
+    expect(resolvedCommand.command, command.command);
+    expect(resolvedCommand.formAnswers, isNull);
+    expect(resolvedCommand.formFields, isNull);
+
+    // Check the arguments to the command contain the default value so the
+    // command will still work without the user prompt.
+    var refactorArguments = getRefactorCommandArguments(
+      resolvedCommand.arguments,
+    );
+    expect(refactorArguments, hasLength(1));
+    expect(refactorArguments.single, 'main');
+  }
+
+  Future<void> test_formFields_supported() async {
+    addTestSource(source);
+
+    await initializeServer();
+    var action = await expectCodeActionWithTitle(refactoringTitle);
+    var command = action.asCommand;
+
+    // Resolve the command, which should add the outstanding form fields.
+    var resolvedCommand = await resolveCommand(
+      ExecuteCommandParams(
+        command: command.command,
+        arguments: command.arguments,
+      ),
+    );
+
+    // Check basics.
+    expect(resolvedCommand.command, command.command);
+    expect(resolvedCommand.formAnswers, isNull);
+    expect(resolvedCommand.formFields, hasLength(1));
+
+    // Because this argument is optional (because we only expect it when using
+    // Interactive Forms but the refactor can be used without), args are
+    // populated with the default during resolve.
+    var arguments = getRefactorCommandArguments(resolvedCommand.arguments);
+    expect(arguments, hasLength(1));
+    expect(arguments.single, 'main');
+
+    // Check the form field is what we'd expect.
+    var field = resolvedCommand.formFields!.single;
+    expect(field.type.kind, 'string');
+    expect(field.description, 'Import Prefix');
+    expect(field.defaultValue, 'main');
+    expect(field.error, isNull);
+  }
+
+  Future<void> test_validates_invalidValue() async {
+    addTestSource(source);
+
+    await initializeServer();
+    var action = await expectCodeActionWithTitle(refactoringTitle);
+    var command = action.asCommand;
+
+    // Resolve the command, which should add the outstanding form fields.
+    var resolvedCommand = await resolveCommand(
+      ExecuteCommandParams(
+        command: command.command,
+        arguments: command.arguments,
+      ),
+    );
+
+    // Resolve again, with an invalid prefix name.
+    expect(resolvedCommand.formFields, hasLength(1));
+    var field = resolvedCommand.formFields!.single;
+    resolvedCommand = await resolveCommand(
+      InteractiveExecuteCommandParams(
+        command: resolvedCommand.command,
+        arguments: resolvedCommand.arguments,
+        formFields: resolvedCommand.formFields,
+        formAnswers: [field.answer('Invalid Prefix')],
+      ),
+    );
+
+    // Check the field has a validation error and the current value is
+    // preserved.
+    expect(resolvedCommand.formFields, hasLength(1));
+    field = resolvedCommand.formFields!.single;
+    expect(field.error, "Import prefix name must not contain ' '.");
+    expect(resolvedCommand.formAnswers!.single, field.answer('Invalid Prefix'));
+
+    // Resolve again with a valid value and ensure the field is completed.
+    resolvedCommand = await resolveCommand(
+      InteractiveExecuteCommandParams(
+        command: resolvedCommand.command,
+        arguments: resolvedCommand.arguments,
+        formFields: resolvedCommand.formFields,
+        formAnswers: [field.answer('valid_prefix')],
+      ),
+    );
+
+    expect(resolvedCommand.formFields, isNull);
+    var arguments = getRefactorCommandArguments(resolvedCommand.arguments);
+    expect(arguments, hasLength(1));
+    expect(arguments.single, 'valid_prefix');
+  }
+}
+
@reflectiveTest
 class CommandResolveTest extends AbstractLspAnalysisServerTest {
  Future<void> test_returnsInputForUnknownCommand() async {
@@ -281,3 +461,20 @@ class CommandResolveTest extends AbstractLspAnalysisServerTest {
    expect(resolvedCommand, command);
  }
 }
+
+/// A temporary mixin that sets the flag to enable the Interactive Forms
+/// experiment setting.
+mixin InteractiveFormsExperimentMixin on RefactoringTest {
+  @override
+  Future<void> initializeServer({
+    bool experimentalOptInFlag = true,
+    // We default this to true for these tests, though it's false in the
+    // the super implementation.
+    bool experimentalInteractiveForms = true,
+  }) {
+    return super.initializeServer(
+      experimentalOptInFlag: experimentalOptInFlag,
+      experimentalInteractiveForms: experimentalInteractiveForms,
+    );
+  }
+}
@@ -3,9 +3,13 @@
 // BSD-style license that can be found in the LICENSE file.

 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/lsp/extensions/code_action.dart';
 import 'package:analysis_server/src/services/refactoring/add_import_prefix.dart';
+import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';

+import '../../../support/interactive_forms.dart';
+import '../../../utils/lsp_protocol_extensions.dart';
 import 'refactoring_test_support.dart';

 void main() {
@@ -21,7 +25,8 @@ void main() {
 /// references to imported symbols) are in
 /// `test\services\refactoring\legacy\rename_import_test.dart`.
@reflectiveTest
-class AddImportPrefixTest extends RefactoringTest {
+class AddImportPrefixTest extends RefactoringTest
+    with InteractiveFormsTestMixin {
  @override
  String get refactoringCommandId => AddImportPrefix.commandName;

@@ -216,6 +221,56 @@ import 'foo - bar.dart' as foo_bar;
    );
  }

+  Future<void> test_interactiveForm_clientModifiedValues() async {
+    setSupportedInteractiveFormInputKinds({'string'});
+
+    var originalSource = '''
+^import 'package:test/main.dart';
+''';
+    var expected = '''
+>>>>>>>>>> lib/main.dart
+import 'package:test/main.dart' as custom_prefix;
+''';
+
+    addTestSource(originalSource);
+
+    await initializeServer(experimentalInteractiveForms: true);
+    var action = await expectCodeActionWithTitle(refactoringTitle);
+    var completedCommand = await completeInteractiveForm(action.command!, {
+      'name': 'custom_prefix',
+    });
+
+    await verifyCommandEdits(completedCommand, expected);
+  }
+
+  Future<void> test_interactiveForm_expectedFields() async {
+    setSupportedInteractiveFormInputKinds({'string'});
+
+    var originalSource = '''
+^import 'package:test/main.dart';
+''';
+
+    addTestSource(originalSource);
+
+    await initializeServer(experimentalInteractiveForms: true);
+    var action = await expectCodeActionWithTitle(refactoringTitle);
+    var command = action.asCommand;
+    var interactiveCommand = await resolveCommand(
+      ExecuteCommandParams(
+        command: command.command,
+        arguments: command.arguments,
+      ),
+    );
+
+    expect(interactiveCommand.formFields, hasLength(1));
+    var field = interactiveCommand.formFields!.single;
+    expect(field.id, 'name');
+    expect(field.description, 'Import Prefix');
+    expect(field.defaultValue, 'main');
+    expect(field.error, isNull);
+    expect(field.type, isA<FormFieldTypeString>());
+  }
+
  Future<void> _assertNoRefactoring({required String originalSource}) async {
    await assertNoRefactoring(
      originalSource: originalSource,