[analysis_server] Fix handling of default values in unanswered Interactive Forms

This fixes a bug where an interactive form where all fields have defaults would be considered complete immediately, so we'd never present the fields to the client.

It also includes some minor refactoring extracted from a future CL that implements command/resolve and supports Interactive Forms in refactors in an attempt to reduce the size of that change to aid reviewing.

Change-Id: I176fe25dbb0b610d69617fa04562b0d3ce571642
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/508220
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>

pkg/analysis_server/benchmark/perf/memory_tests.dart

@@ -216,8 +216,9 @@ class LspAnalysisServerMemoryUsageTest
   Map<String, List<lsp.Diagnostic>> currentAnalysisErrors = {};
 
   @override
-  void expect(Object? actual, Matcher matcher, {String? reason}) =>
-      outOfTestExpect(actual, matcher, reason: reason);
+  void expect(Object? actual, Object? matcher, {String? reason}) {
+    outOfTestExpect(actual, matcher, reason: reason);
+  }
 
   /// The server is automatically started before every test.
   @override

pkg/analysis_server/integration_test/support/integration_tests.dart

@@ -56,11 +56,15 @@ Matcher isOneOf(List<Matcher> choiceMatchers) => _OneOf(choiceMatchers);
 /// Assert that [actual] matches [matcher].
 void outOfTestExpect(
   Object? actual,
-  Matcher matcher, {
+  Object? matcherOrValue, {
   String? reason,
   skip,
   bool verbose = false,
 }) {
+  var matcher = matcherOrValue is Matcher
+      ? matcherOrValue
+      : equals(matcherOrValue);
+
   var matchState = {};
   try {
     if (matcher.matches(actual, matchState)) return;
@@ -95,12 +99,15 @@ String _defaultFailFormatter(
 typedef MatcherCreator = Matcher Function();
 
 /// Type of closures used by MatchesJsonObject to record field mismatches.
-typedef MismatchDescriber =
-    Description Function(Description mismatchDescription);
+typedef MismatchDescriber = Description Function(
+  Description mismatchDescription,
+);
 
 /// Type of callbacks used to process notifications.
-typedef NotificationProcessor =
-    void Function(String event, Map<Object?, Object?> params);
+typedef NotificationProcessor = void Function(
+  String event,
+  Map<Object?, Object?> params,
+);
 
 /// Type of callbacks used to process reverse-requests.
 typedef ReverseRequestProcessor = void Function(Request request);

pkg/analysis_server/lib/src/lsp/mapping.dart

@@ -22,6 +22,7 @@ import 'package:analysis_server/src/protocol_server.dart'
 import 'package:analysis_server/src/services/completion/dart/dart_completion_suggestion.dart';
 import 'package:analysis_server/src/services/completion/dart/feature_computer.dart';
 import 'package:analysis_server/src/services/snippets/snippet.dart';
+import 'package:analysis_server/src/utilities/extensions/list.dart';
 import 'package:analysis_server/src/utilities/extensions/string.dart';
 import 'package:analyzer/dart/analysis/results.dart' as server;
 import 'package:analyzer/dart/element/element.dart';
@@ -1899,8 +1900,3 @@ extension CompletionLabelExtension on CompletionItemLabelDetails {
   CompletionItemLabelDetails? get nullIfEmpty =>
       detail != null || description != null ? this : null;
 }
-
-extension _ListExtensions<T> on List<T> {
-  /// Returns `null` if this list is empty, otherwise `this`.
-  List<T>? get nullIfEmpty => isEmpty ? null : this;
-}

pkg/analysis_server/lib/src/services/interactive_forms/interactive_forms.dart

@@ -132,11 +132,13 @@ class InteractiveForm {
     _isComplete = true; // Default until we see validation errors.
     clientFields.clear();
     for (var field in _fieldMap.values) {
-      // Use the default value if no answer was supplied by the client, since
-      // this allows us to have unsupported form fields as long as they have
-      // defaults.
-      var answerValue = answerById[field.id]?.value ?? field.defaultValue;
-      var errorMessage = _validateAnswer(field, answerValue);
+      var answerValue = answerById[field.id]?.value;
+      var errorMessage = _validateAnswer(
+        field,
+        // For validation, we can use the default value if none was provided.
+        // This allows unsupported fields with defaults to pass validation.
+        answerValue ?? field.defaultValue,
+      );
       var isValid = errorMessage == null;
 
       // Record the current answer and validation state so it can be used by
@@ -152,7 +154,14 @@ class InteractiveForm {
       }
 
       // Update form completion state.
-      _isComplete = _isComplete && isValid;
+      if (!isValid) {
+        // User has given an invalid answer and must be shown an error.
+        _isComplete = false;
+      } else if (_isSupported(field) && field.required && answerValue == null) {
+        // A supported, required field does not have an answer so the form must
+        // still be presented again.
+        _isComplete = false;
+      }
     }
 
     // If the form is complete, no fields go back to the client.

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

@@ -18,8 +18,9 @@ import 'package:language_server_protocol/protocol_custom_generated.dart';
 import 'package:language_server_protocol/protocol_generated.dart';
 
 /// A function that can be executed to create a refactoring producer.
-typedef RefactoringProducerGenerator =
-    RefactoringProducer Function(RefactoringContext);
+typedef RefactoringProducerGenerator = RefactoringProducer Function(
+  RefactoringContext,
+);
 
 class RefactoringProcessor {
   /// A list of the generators used to produce refactorings.
@@ -101,16 +102,10 @@ class RefactoringProcessor {
             command: Command(
               command: command,
               title: producer.title,
-              arguments: [
-                {
-                  'filePath': context.resolvedUnitResult.path,
-                  'selectionOffset': context.selectionOffset,
-                  'selectionLength': context.selectionLength,
-                  'arguments': parameters
-                      .map((param) => param.defaultValue)
-                      .toList(),
-                },
-              ],
+              arguments: buildCommandArguments(
+                context,
+                parameters.map((param) => param.defaultValue).toList(),
+              ),
             ),
             data: {'parameters': parameters},
           ),
@@ -134,4 +129,28 @@ class RefactoringProcessor {
     _performance?.computeTime = _timer.elapsed;
     return refactorings;
   }
+
+  /// Builds the command arguments that go to the client, which include the
+  /// values required to rebuild the refactoring context, and the arguments
+  /// specific to the refactor.
+  ///
+  /// We always use a single argument that is a map so all values are named,
+  /// with the refactor-specific arguments being in the `arguments` field of
+  /// that map.
+  ///
+  /// This is the opposite of [extractRefactorArguments] which extracts the
+  /// refactor arguments back out of the command.
+  static List<Object?> buildCommandArguments(
+    RefactoringContext context,
+    List<Object?> refactorAguments,
+  ) {
+    return [
+      {
+        'filePath': context.resolvedUnitResult.path,
+        'selectionOffset': context.selectionOffset,
+        'selectionLength': context.selectionLength,
+        'arguments': refactorAguments,
+      },
+    ];
+  }
 }

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 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_plugin/edit/correction_utils.dart';
 import 'package:analysis_server_plugin/src/utilities/selection.dart';
@@ -34,6 +35,11 @@ abstract class ParameterizedRefactoringProducer extends RefactoringProducer {
 
   /// Return a list of the parameters to send to the client.
   List<CommandParameter> get parameters;
+
+  /// A convenience wrapper around [RefactoringProcessor.buildCommandArguments].
+  List<Object?> buildCommandArguments(List<Object?> args) {
+    return RefactoringProcessor.buildCommandArguments(refactoringContext, args);
+  }
 }
 
 /// An object that can compute a refactoring in a Dart file.

pkg/analysis_server/lib/src/utilities/extensions/list.dart

@@ -0,0 +1,8 @@
+// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
+// 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.
+
+extension ListExtensions<T> on List<T> {
+  /// Returns `null` if this list is empty, otherwise `this`.
+  List<T>? get nullIfEmpty => isEmpty ? null : this;
+}

pkg/analysis_server/test/lsp/request_helpers_mixin.dart

@@ -301,8 +301,9 @@ mixin LspRequestHelpersMixin {
     );
   }
 
-  void expect(Object? actual, Matcher matcher, {String? reason}) =>
-      test.expect(actual, matcher, reason: reason);
+  void expect(Object? actual, Object? matcher, {String? reason}) {
+    test.expect(actual, matcher, reason: reason);
+  }
 
   Future<T> expectSuccessfulResponseTo<T, R>(
     RequestMessage request,

pkg/analysis_server/test/src/services/interactive_forms/interactive_forms_test.dart

@@ -7,6 +7,8 @@ import 'package:analysis_server/src/services/interactive_forms/interactive_forms
 import 'package:matcher/expect.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
+import '../../../support/interactive_forms.dart';
+
 void main() {
   defineReflectiveSuite(() {
     defineReflectiveTests(InteractiveFormsTest);
@@ -15,6 +17,49 @@ void main() {
 
 @reflectiveTest
 class InteractiveFormsTest {
+  /// Default values are not treated the same as user answers. A form will not
+  /// be considered complete even if unanswered fields have defaults (as long
+  /// as they are supported).
+  test_defaults_doNotCompleteForm_answered() {
+    var fieldA = _stringField('a', defaultValue: 'aDefault');
+    var fieldB = _stringField('b', defaultValue: 'bDefault');
+    var fields = [fieldA, fieldB];
+
+    var form = InteractiveForm(
+      supportedInteractiveFormInputTypes: {'string'},
+      fields: fields,
+    );
+
+    // Process empty answers. This makes no difference to the unanswered case
+    // above.
+    form.processResponse([]);
+
+    // Because we never provided answers, we still have fields to complete.
+    expect(form.clientFields, [fieldA, fieldB]);
+    expect(form.clientAnswers, isEmpty);
+    expect(form.answers, ['aDefault', 'bDefault']);
+  }
+
+  /// Default values are not treated the same as user answers. A form will not
+  /// be considered complete even if unanswered fields have defaults (as long
+  /// as they are supported).
+  test_defaults_doNotCompleteForm_unanswered() {
+    var fieldA = _stringField('a', defaultValue: 'aDefault');
+    var fieldB = _stringField('b', defaultValue: 'bDefault');
+    var fields = [fieldA, fieldB];
+
+    var form = InteractiveForm(
+      supportedInteractiveFormInputTypes: {'string'},
+      fields: fields,
+    );
+
+    // Because we have never responded to the form, we still have fields to
+    // complete.
+    expect(form.clientFields, [fieldA, fieldB]);
+    expect(form.clientAnswers, isEmpty);
+    expect(form.answers, ['aDefault', 'bDefault']);
+  }
+
   test_initialState() {
     var fieldA = _stringField('a', defaultValue: 'aDefault');
     var fieldB = _stringField('b');
@@ -392,10 +437,3 @@ class InteractiveFormsTest {
     );
   }
 }
-
-extension on FormField {
-  /// Returns a [FormAnswer] for this field with the answer [value].
-  FormAnswer answer(Object? value) {
-    return FormAnswer(id: id, value: value);
-  }
-}

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

@@ -32,7 +32,7 @@ class ^A {}
 
   /// Replaces the "Save URI" argument in [action].
   void replaceSaveUriArgument(CodeAction action, Uri newFileUri) {
-    var arguments = getRefactorCommandArguments(action);
+    var arguments = getRefactorCommandArguments(action.command?.arguments);
     // The filename is the first item we prompt for so is first in the
     // arguments.
     arguments[0] = newFileUri.toString();

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

@@ -115,16 +115,12 @@ abstract class RefactoringTest extends AbstractLspAnalysisServerTest
 
   /// Unwraps the 'arguments' field from the arguments object (which is the
   /// single argument for the command).
-  List<Object?> getRefactorCommandArguments(CodeAction action) {
-    var command = action.command!;
-    var commandArguments = command.arguments as List<Object?>;
+  List<Object?> getRefactorCommandArguments(List<Object?>? commandArguments) {
+    // Our refactor commands use a single object in their arguments so we can
+    // have named fields instead of positional arguments.
+    var argsObject = commandArguments!.single as Map<String, Object?>;
 
-    // Our refactor command uses a single object in its arguments so we can have
-    // named fields instead of having the client have to know which index
-    // corresponds to the parameters.
-    var argsObject = commandArguments.single as Map<String, Object?>;
-
-    // Within that object, the 'arguments' field is the List<Object?> that
+    // Within the object, the 'arguments' field is the List<Object?> that
     // contains the values for the parameters.
     var arguments = argsObject['arguments'] as List<Object?>;
 
@@ -136,8 +132,14 @@ abstract class RefactoringTest extends AbstractLspAnalysisServerTest
   /// Enables all required client capabilities for new refactors unless the
   /// corresponding flags are set to `false`.
   @override
-  Future<void> initializeServer({bool experimentalOptInFlag = true}) async {
-    var config = {if (experimentalOptInFlag) 'experimentalRefactors': true};
+  Future<void> initializeServer({
+    bool experimentalOptInFlag = true,
+    bool experimentalInteractiveForms = false,
+  }) async {
+    var config = {
+      if (experimentalOptInFlag) 'experimentalRefactors': true,
+      if (experimentalInteractiveForms) 'experimentalInteractiveForms': true,
+    };
 
     await provideConfig(super.initializeServer, config);
   }

pkg/analysis_server/test/support/interactive_forms.dart

@@ -0,0 +1,12 @@
+// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
+// 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:language_server_protocol/protocol_custom_generated.dart';
+
+extension FormFieldExtension on FormField {
+  /// Returns a [FormAnswer] for this field with the answer [value].
+  FormAnswer answer(Object? value) {
+    return FormAnswer(id: id, value: value);
+  }
+}

pkg/analysis_server/tool/lsp_spec/README.md

@@ -60,17 +60,14 @@ Client workspace settings are requested with `workspace/configuration` during in
 Below is a list of LSP methods and their implementation status.
 
 - Method: The LSP method name
-- Basic Impl: This method has an implementation but may assume some client capabilities
-- Capabilities: Only types from the original spec or as advertised in client capabilities are returned
-- Plugins: This functionality works with server plugins
-- Tests: Has automated tests
-- Tested Client: Has been manually tested in at least one LSP client editor
+- Server: The method is supported by the Dart server.
+- Plugins: This functionality can be extended by third party analyzer plugins.
 
 | Method | Server | Plugins | Notes |
 | - | - | - | - |
-| initialize | ✅ | N/A | trace and other options NYI|
+| initialize | ✅ | N/A | trace and other options NYI |
 | initialized | ✅ | N/A | |
-| shutdown | ✅ | N/A | supported but does nothing|
+| shutdown | ✅ | N/A | supported but does nothing |
 | exit | ✅ | N/A | |
 | $/cancelRequest | ✅ | | |
 | $/logTrace | | | |
@@ -80,9 +77,9 @@ Below is a list of LSP methods and their implementation status.
 | client/unregisterCapability | ✅ | ✅ | |
 | notebookDocument/* | | | |
 | telemetry/event | | | |
-| textDocument/codeAction (assists) | ✅ | ✅ | Only if the client advertises `codeActionLiteralSupport` with `Refactor`|
+| textDocument/codeAction (assists) | ✅ | ✅ | Only if the client advertises `codeActionLiteralSupport` with `Refactor` |
 | textDocument/codeAction (fixAll) | ✅ | | |
-| textDocument/codeAction (fixes) | ✅ | ✅ | Only if the client advertises `codeActionLiteralSupport` with `QuickFix`|
+| textDocument/codeAction (fixes) | ✅ | ✅ | Only if the client advertises `codeActionLiteralSupport` with `QuickFix` |
 | textDocument/codeAction (organiseImports) | ✅ | | |
 | textDocument/codeAction (refactors) | ✅ | | |
 | textDocument/codeAction (sortMembers) | ✅ | | |
@@ -146,7 +143,7 @@ Below is a list of LSP methods and their implementation status.
 | workspace/diagnostic | | | |
 | workspace/diagnostic/refresh | | | |
 | workspace/didChangeConfiguration | ✅ | | |
-| workspace/didChangeWatchedFiles | | | unused, server does own watching|
+| workspace/didChangeWatchedFiles | | | unused, server does own watching |
 | workspace/didChangeWorkspaceFolders | ✅ | ✅ | |
 | workspace/didCreateFiles | | | |
 | workspace/didDeleteFiles | | | |