diff --git a/pkg/analysis_server/lib/src/services/interactive_forms/interactive_forms.dart b/pkg/analysis_server/lib/src/services/interactive_forms/interactive_forms.dart index fb01e4f6fa1..4802bf14a05 100644 --- a/pkg/analysis_server/lib/src/services/interactive_forms/interactive_forms.dart +++ b/pkg/analysis_server/lib/src/services/interactive_forms/interactive_forms.dart @@ -2,7 +2,6 @@ // 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:collection/collection.dart'; import 'package:language_server_protocol/protocol_custom_generated.dart'; /// A class for processing interactive forms. @@ -22,100 +21,144 @@ class InteractiveForm { /// use them in place of user values). final Set supportedInteractiveFormInputTypes; - /// The complete set of all fields for this form regardless of whether they - /// are supported by the client or previously answered. - final List _masterFields; + /// The fields for this form, indexed by [FormField.id]. + final Map _fieldMap = {}; - /// The set of existing answers. + /// The master list of all fields for this form in the order to be shown to + /// the user. /// - /// The items in this list correspond go the fields in [_masterFields]. - /// Initially this list will contain nulls or default values but should become - /// populated after round trips to the client. - /// - /// It is up to the caller to determine how to carry answers from - /// [existingAnswers] through the client and back into this field. For - /// commands this is usually in the real set of arguments against the command. - /// - /// These answers will be updated by [processResponse] so they can be - /// sent back to the client. - final List existingAnswers; + /// The [answers] getter will return all answers with matching order/indexes. + final List fields; - /// The current outstanding fields that need to go back to the client. - final List outstandingFields = []; - - /// The current answers for [outstandingFields]. + /// The current answers for the form and whether they are valid, indexed by + /// [FormAnswer.id]. /// - /// Values may be `null` if the user has not provided a value, but the list - /// always contains the same number of items as [outstandingFields]. - final List outstandingFieldAnswers = []; + /// It is not guaranteed that answers for all fields are present (for example + /// some fields may be optional and unanswered). + final Map _answerMap = {}; + + /// The set of fields to go back to the client. + /// + /// If the form is complete, this list will be empty. If there are validation + /// errors, the errors will be attached to the fields. + /// + /// The order of these fields is not guaranteed. Answers should always be + /// looked up by ID. + final List clientFields = []; + + /// The set of answers provided by, and to go back to, the client. + /// + /// The server never modifies these answers, they are just retained by + /// [processResponse] for convenience. + final List clientAnswers = []; + + bool _isComplete = false; new({ required this.supportedInteractiveFormInputTypes, - required this._masterFields, - required this.existingAnswers, + required this.fields, }) { - if (_masterFields.length != existingAnswers.length) { - throw ArgumentError( - 'masterFields and existingAnswers must have the same length', - ); + // Validate input and build a map. + for (var field in fields) { + if (_fieldMap.containsKey(field.id)) { + throw ArgumentError( + 'Multiple fields were given with the ID "${field.id}"', + 'fields', + ); + } else if (!_isSupported(field) && field.defaultValue == null) { + throw ArgumentError( + 'Field "${field.id}" is not supported by the client and does not ', + 'have a default value', + ); + } + _fieldMap[field.id] = field; } - // Process the full set of fields initially, so we can handle defaults for - // unsupported fields by reading their answers and removing them from the - // outstanding field list. - processResponse(_masterFields, List.filled(_masterFields.length, null)); + // Pre-populate the fields to be sent to the client. + clientFields.addAll(_fieldMap.values.where(_isSupported)); } - /// Processes the set of answers from the client, updating - /// [outstandingFields], [outstandingFieldAnswers] and [existingAnswers]. + /// A list of all answers matching the order of [fields]. /// - /// [clientFields] is the set of fields that returned from the client (the - /// previous turns [outstandingFields]), and [clientAnswers] are the responses - /// (matched by index). - void processResponse( - List clientFields, - List clientAnswers, - ) { - if (clientFields.length != clientAnswers.length) { - throw ArgumentError( - 'clientFields and clientAnswers must have the same length', - ); - } + /// This list is computed on-demand and always matches the length of [fields] + /// with unanswered questions (or those with invalid answers) having their + /// default answers (or `null`). + /// + /// This getter is for convenience for callers that flatten answers into an + /// arguments array such as for LSP command execution. + List get answers { + return fields.map((field) { + var answer = _answerMap[field.id]; + var isValid = answer?.isValid ?? !field.required; + // Only use the users answer if valid, otherwise use the default value. + return isValid ? answer?.value ?? field.defaultValue : field.defaultValue; + }).toList(); + } - // Validate all responses from the client. - var responses = clientFields - .mapIndexed((i, field) => _validate(field, clientAnswers[i])) - .toList(); + /// Whether the form is complete. + /// + /// `true` if all required fields are present and pass validation. + /// `false` if there are missing required fields, or validation errors. + bool get isComplete => _isComplete; - // Rebuild the outstanding fields from only those that did not contain - // valid answers. - outstandingFields.clear(); - outstandingFieldAnswers.clear(); + /// Replaces the current set of answers with a new set from the client and + /// updates [clientAnswers] and [isComplete]. + void processResponse(List answers) { + clientAnswers + ..clear() + ..addAll(answers); + _answerMap.clear(); - // Process any responses and update the existing answers. - for (var response in responses) { - var field = response.field; - var isValid = response.isValid; - var value = response.value; - - if (!isValid) { - // Field is not valid and must go back to the client. - outstandingFields.add(field); - outstandingFieldAnswers.add(value); - } else { - // Field was valid, so update the existing answers. - var masterIndex = _masterFields.indexWhere(field.matches); - if (masterIndex == -1) { - throw StateError( - "Field '${field.description}' provided by client is not recognised by the server", - ); - } - existingAnswers[masterIndex] = value; + // Validate input and build a map. + Map answerById = {}; + for (var answer in answers) { + if (!_fieldMap.containsKey(answer.id)) { + throw ArgumentError( + 'Answer references non-existent field "${answer.id}"', + 'answers', + ); + } else if (answerById.containsKey(answer.id)) { + throw ArgumentError( + 'Multiple answers were given for field "${answer.id}"', + 'answers', + ); } + + answerById[answer.id] = answer; } - assert(outstandingFields.length == outstandingFieldAnswers.length); - assert(_masterFields.length == existingAnswers.length); + // Validate the answers for all fields and rebuild the fields that go back + // to the client with any validation errors. + _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 isValid = errorMessage == null; + + // Record the current answer and validation state so it can be used by + // [answers] later. + _answerMap[field.id] = (value: answerValue, isValid: isValid); + + // Record any error message. + field = field.withError(errorMessage); + + // Only supported fields go back to the client. + if (_isSupported(field)) { + clientFields.add(field); + } + + // Update form completion state. + _isComplete = _isComplete && isValid; + } + + // If the form is complete, no fields go back to the client. + if (_isComplete) { + clientFields.clear(); + } } /// Returns whether [field] is a type of field that the client supports @@ -124,39 +167,17 @@ class InteractiveForm { return supportedInteractiveFormInputTypes.contains(field.type.kind); } - ValidatedResponse _validate(FormField field, Object? answer) { - // If a field is not supported, it must have a default and we will use - // that value and consider it valid. It is up to the caller (for example the - // refactor) to ensure that if there are required fields with no defaults - // that they do not present themselves to the user (for example by checking - // in isAvailable). - if (answer == null && !_isSupported(field)) { - if (field.defaultValue == null) { - throw ArgumentError( - "The form field '${field.description}' is not supported by " - 'the client and has no default value', - ); - } - answer = field.defaultValue; + String? _validateAnswer(FormField field, Object? answerValue) { + // Optional fields with no answer are valid. + if (!field.required && answerValue == null) { + return null; } - var errorMessage = _validateAnswer(field, answer); - var isValid = answer != null && errorMessage == null; - - // Only attach error messages if the user had provided a value. - if (answer != null) { - field = field.withError(errorMessage); - } - - return ValidatedResponse(field, answer, isValid: isValid); - } - - String? _validateAnswer(FormField field, Object? answer) { return switch (field.type) { - FormFieldTypeFile() => _validateFile(answer), - FormFieldTypeBool() => _validateBool(answer), - FormFieldTypeNumber() => _validateNumber(answer), - FormFieldTypeString() => _validateString(answer), + FormFieldTypeFile() => _validateFile(answerValue), + FormFieldTypeBool() => _validateBool(answerValue), + FormFieldTypeNumber() => _validateNumber(answerValue), + FormFieldTypeString() => _validateString(answerValue), }; } @@ -187,51 +208,19 @@ class InteractiveForm { } } -class ValidatedResponse { - /// The [FormField] this response relates to. - final FormField field; - - /// The current value provided by the user. - /// - /// The value might be null (if the user has not provided an answer), or an - /// invalid value (or even an incorrect type). - final Object? value; - - /// Whether the valid provided in [value] validated correctly and the user - /// does not need to be re-prompted for this field. - final bool isValid; - - new(this.field, this.value, {required this.isValid}); -} - extension FormFieldExtension on FormField { - /// Returns whether this field is the same as [other]. - /// - /// Fields are round-tripped to the client so we cannot check references for - /// equality. The fields sent to the client may also include error messages so - /// we cannot use value equality. - /// - /// We assume that the type + description are a unique pair. - bool matches(FormField other) { - // TODO(dantup): Determine if we can do something better here. - return description == other.description && type == other.type; - } - FormField withError(String? error) { if (this.error == error) { return this; } else { return FormField( + id: id, description: description, type: type, + required: required, defaultValue: defaultValue, error: error, ); } } } - -extension ListFormField on List { - /// The default values for these fields. - List get defaults => map((field) => field.defaultValue).toList(); -} diff --git a/pkg/analysis_server/test/src/services/interactive_forms/interactive_forms_test.dart b/pkg/analysis_server/test/src/services/interactive_forms/interactive_forms_test.dart index dc0a0066f04..833e022350e 100644 --- a/pkg/analysis_server/test/src/services/interactive_forms/interactive_forms_test.dart +++ b/pkg/analysis_server/test/src/services/interactive_forms/interactive_forms_test.dart @@ -16,147 +16,201 @@ void main() { @reflectiveTest class InteractiveFormsTest { test_initialState() { - var fieldA = _stringField('a', 'aDefault'); + var fieldA = _stringField('a', defaultValue: 'aDefault'); var fieldB = _stringField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'string'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); - expect(form.outstandingFields, [fieldA, fieldB]); - expect(form.outstandingFieldAnswers, [null, null]); // No client answers - expect(form.existingAnswers, ['aDefault', null]); // But defaults here + expect(form.clientFields, [fieldA, fieldB]); + expect(form.clientAnswers, isEmpty); // No client answers + expect(form.answers, ['aDefault', null]); // But defaults here } test_initialState_defaultsForUnsupportedFields() { - var fieldA = _stringField('a', 'aDefault'); - var fieldB = _stringField('b', 'bDefault'); - var masterFields = [fieldA, fieldB]; + var fieldA = _stringField('a', defaultValue: 'aDefault'); + var fieldB = _stringField('b', defaultValue: 'bDefault'); + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'int'}, // We don't support strings! - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // No outstanding fields, because we don't support strings and used the // defaults instead. - expect(form.outstandingFields, isEmpty); - expect(form.outstandingFieldAnswers, isEmpty); - expect(form.existingAnswers, ['aDefault', 'bDefault']); + expect(form.clientFields, isEmpty); + expect(form.clientAnswers, isEmpty); + expect(form.answers, ['aDefault', 'bDefault']); } test_processResponse_invalidAnswers() { - var fieldA = _stringField('a', 'aDefault'); + var fieldA = _stringField('a', defaultValue: 'aDefault'); var fieldB = _stringField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'string'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process a response with invalid answers. - form.processResponse(masterFields, [1, 2]); + var clientAnswers = [fieldA.answer(1), fieldB.answer(2)]; + form.processResponse(clientAnswers); // Expect error messages on the fields. - expect(form.outstandingFields, [ + expect(form.clientFields, [ fieldA.withError('Must be a valid string'), fieldB.withError('Must be a valid string'), ]); - expect(form.outstandingFieldAnswers, [1, 2]); // Previous user input - expect(form.existingAnswers, ['aDefault', null]); // Still defaults here + expect(form.clientAnswers, clientAnswers); // Previous user input + expect(form.answers, ['aDefault', null]); // Still defaults here } test_processResponse_mixedValidInvalidAnswers() { - var fieldA = _stringField('a', 'aDefault'); + var fieldA = _stringField('a', defaultValue: 'aDefault'); var fieldB = _stringField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'string'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process a response with some valid and some invalid answers. - form.processResponse(masterFields, ['valid', 2]); + var clientAnswers = [fieldA.answer('valid'), fieldB.answer(2)]; + form.processResponse(clientAnswers); - // Expect only the invalid field as outstanding. - expect(form.outstandingFields, [ + // Expect only the invalid field to have a validation error. + expect(form.clientFields, [ + fieldA, fieldB.withError('Must be a valid string'), ]); - expect(form.outstandingFieldAnswers, [2]); // Previous user input - expect(form.existingAnswers, ['valid', null]); // Updated with valid answer + expect(form.clientAnswers, clientAnswers); // Previous user input + expect(form.answers, ['valid', null]); // Updated with valid answer } test_processResponse_multipleRounds() { - var fieldA = _stringField('a', 'aDefault'); + var fieldA = _stringField('a', defaultValue: 'aDefault'); var fieldB = _stringField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'string'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process response with one valid answer. - form.processResponse(masterFields, ['valid', 2]); + form.processResponse([fieldA.answer('valid'), fieldB.answer(2)]); - // One remaining oustanding field. - expect(form.outstandingFields, [ + // Expect the invalid field to have a validation error. + expect(form.clientFields, [ + fieldA, fieldB.withError('Must be a valid string'), ]); - // Process that one field. - form.processResponse([fieldB], ['alsoValid']); + // Process with both valid answers. + form.processResponse([fieldA.answer('valid'), fieldB.answer('alsoValid')]); // Now we have no outstanding fields, but both answers populated. - expect(form.outstandingFields, isEmpty); - expect(form.existingAnswers, ['valid', 'alsoValid']); + expect(form.clientFields, isEmpty); + expect(form.clientAnswers, hasLength(2)); + expect(form.answers, ['valid', 'alsoValid']); } test_processResponse_noAnswers() { - var fieldA = _stringField('a', 'aDefault'); + var fieldA = _stringField('a', defaultValue: 'aDefault'); var fieldB = _stringField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'string'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); - // This should not change anything, because the client didn't provide any - // answers. - form.processResponse(masterFields, List.filled(masterFields.length, null)); + // Provide no answers. + form.processResponse([]); - expect(form.outstandingFields, [fieldA, fieldB]); - expect(form.outstandingFieldAnswers, [null, null]); // No client answers - expect(form.existingAnswers, ['aDefault', null]); // But defaults here + // Fields now show validation messages. + expect(form.clientFields, [ + fieldA, + fieldB.withError('Must be a valid string'), + ]); + expect(form.clientAnswers, isEmpty); // No client answers + expect(form.answers, ['aDefault', null]); // But defaults here + } + + test_throws_duplicateAnswerIDs() { + var fieldA = _fileField('a'); + + var form = InteractiveForm( + supportedInteractiveFormInputTypes: {'file'}, + fields: [fieldA], + ); + + expect( + () => form.processResponse([fieldA.answer(1), fieldA.answer(2)]), + throwsArgumentError, + ); + } + + test_throws_duplicateFieldIDs() { + var fieldA = _fileField('a'); + + expect( + () => InteractiveForm( + supportedInteractiveFormInputTypes: {'file'}, + fields: [fieldA, fieldA], + ), + throwsArgumentError, + ); + } + + test_throws_invalidAnswerId() { + var fieldA = _fileField('a'); + + var form = InteractiveForm( + supportedInteractiveFormInputTypes: {'file'}, + fields: [fieldA], + ); + + expect( + () => form.processResponse([FormAnswer(id: 'fake')]), + throwsArgumentError, + ); + } + + test_throws_unsupportedWithNoDefault() { + var fieldA = _fileField('a'); + + expect( + () => InteractiveForm( + supportedInteractiveFormInputTypes: {'string'}, // no 'file' + fields: [fieldA], + ), + throwsArgumentError, + ); } test_validation_bool() { var fieldA = _boolField('a'); var fieldB = _boolField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'bool'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process a response with some valid and some invalid answers. - form.processResponse(masterFields, [true, 'invalid']); + form.processResponse([fieldA.answer(true), fieldB.answer('invalid')]); - // Expect only the invalid field as outstanding. - expect(form.outstandingFields, [ + // Expect all fields, with a validation message on the invalid one. + expect(form.clientFields, [ + fieldA, fieldB.withError('Must be a valid boolean'), ]); } @@ -164,19 +218,22 @@ class InteractiveFormsTest { test_validation_file() { var fieldA = _fileField('a'); var fieldB = _fileField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'file'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process a response with some valid and some invalid answers. - form.processResponse(masterFields, ['file:///a/b', 'invalid']); + form.processResponse([ + fieldA.answer('file:///a/b'), + fieldB.answer('invalid'), + ]); - // Expect only the invalid field as outstanding. - expect(form.outstandingFields, [ + // Expect all fields, with a validation message on the invalid one. + expect(form.clientFields, [ + fieldA, fieldB.withError('Must be a valid file:// URI'), ]); } @@ -184,72 +241,161 @@ class InteractiveFormsTest { test_validation_number() { var fieldA = _numberField('a'); var fieldB = _numberField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'number'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process a response with some valid and some invalid answers. - form.processResponse(masterFields, [1, 'invalid']); + form.processResponse([fieldA.answer(1), fieldB.answer('invalid')]); - // Expect only the invalid field as outstanding. - expect(form.outstandingFields, [ + // Expect all fields, with a validation message on the invalid one. + expect(form.clientFields, [ + fieldA, fieldB.withError('Must be a valid number'), ]); } + test_validation_optional() { + var fieldA = _boolField('a', required: false); + var fieldB = _boolField('b'); + var fields = [fieldA, fieldB]; + + var form = InteractiveForm( + supportedInteractiveFormInputTypes: {'bool'}, + fields: fields, + ); + + // No answers. + form.processResponse([]); + + expect(form.clientFields, [ + fieldA, // Valid because optional. + fieldB.withError('Must be a valid boolean'), + ]); + } + + test_validation_optionalButWrongType() { + var fieldA = _boolField('a', required: false); + var fieldB = _boolField('b'); + var fields = [fieldA, fieldB]; + + var form = InteractiveForm( + supportedInteractiveFormInputTypes: {'bool'}, + fields: fields, + ); + + // No answers. + form.processResponse([fieldA.answer('invalid')]); + + expect(form.clientFields, [ + fieldA.withError('Must be a valid boolean'), // Invalid type + fieldB.withError('Must be a valid boolean'), // Not answered + ]); + } + test_validation_string() { var fieldA = _stringField('a'); var fieldB = _stringField('b'); - var masterFields = [fieldA, fieldB]; + var fields = [fieldA, fieldB]; var form = InteractiveForm( supportedInteractiveFormInputTypes: {'string'}, - masterFields: masterFields, - existingAnswers: masterFields.defaults, + fields: fields, ); // Process a response with some valid and some invalid answers. - form.processResponse(masterFields, ['valid', 2]); + form.processResponse([fieldA.answer('valid'), fieldB.answer(2)]); - // Expect only the invalid field as outstanding. - expect(form.outstandingFields, [ + // Expect all fields, with a validation message on the invalid one. + expect(form.clientFields, [ + fieldA, fieldB.withError('Must be a valid string'), ]); } - FormField _boolField(String description, [String? defaultValue]) { - return _field(FormFieldTypeBool(), description, defaultValue); - } - - FormField _field( - FormFieldType type, - String description, [ + FormField _boolField( + String id, { + bool? required, + String? description, String? defaultValue, - ]) { - return FormField( + }) { + return _field( + FormFieldTypeBool(), + id, description: description, - type: type, + required: required, defaultValue: defaultValue, ); } - FormField _fileField(String description, [String? defaultValue]) { - return _field( - FormFieldTypeFile(existence: FileExistence.New, type: FileType.Regular), - description, - defaultValue, + FormField _field( + FormFieldType type, + String id, { + String? description, + bool? required, + String? defaultValue, + }) { + return FormField( + id: id, + description: description ?? 'Field for $id', + type: type, + required: required ?? true, + defaultValue: defaultValue, ); } - FormField _numberField(String description, [String? defaultValue]) { - return _field(FormFieldTypeNumber(), description, defaultValue); + FormField _fileField( + String id, { + bool? required, + String? description, + String? defaultValue, + }) { + return _field( + FormFieldTypeFile(existence: FileExistence.New, type: FileType.Regular), + id, + description: description, + required: required, + defaultValue: defaultValue, + ); } - FormField _stringField(String description, [String? defaultValue]) { - return _field(FormFieldTypeString(), description, defaultValue); + FormField _numberField( + String id, { + bool? required, + String? description, + String? defaultValue, + }) { + return _field( + FormFieldTypeNumber(), + id, + description: description, + required: required, + defaultValue: defaultValue, + ); + } + + FormField _stringField( + String id, { + bool? required, + String? description, + String? defaultValue, + }) { + return _field( + FormFieldTypeString(), + id, + description: description, + required: required, + defaultValue: defaultValue, + ); + } +} + +extension on FormField { + /// Returns a [FormAnswer] for this field with the answer [value]. + FormAnswer answer(Object? value) { + return FormAnswer(id: id, value: value); } } diff --git a/pkg/analysis_server/test/tool/lsp_spec/generated_classes_test.dart b/pkg/analysis_server/test/tool/lsp_spec/generated_classes_test.dart index 80f37741755..f9fb7967eba 100644 --- a/pkg/analysis_server/test/tool/lsp_spec/generated_classes_test.dart +++ b/pkg/analysis_server/test/tool/lsp_spec/generated_classes_test.dart @@ -138,14 +138,18 @@ class GeneratedClassesTest { void test_interactiveForms_deserialize_formFieldsIntoSubclasses() { var stringField = FormField.fromJson({ + 'id': 'a', 'type': {'kind': 'string'}, 'description': '', + 'required': true, }); expect(stringField.type, isA()); var boolField = FormField.fromJson({ + 'id': 'b', 'type': {'kind': 'bool'}, 'description': '', + 'required': false, }); expect(boolField.type, isA()); } diff --git a/pkg/analysis_server/tool/lsp_spec/custom/interactive_forms.dart b/pkg/analysis_server/tool/lsp_spec/custom/interactive_forms.dart index 795cd7f51ee..001cf6b59ca 100644 --- a/pkg/analysis_server/tool/lsp_spec/custom/interactive_forms.dart +++ b/pkg/analysis_server/tool/lsp_spec/custom/interactive_forms.dart @@ -7,9 +7,8 @@ import '../utils.dart'; /// Classes that support for the new (Go-specified) interactive-refactors. final interactiveFormClasses = [ - // TODO(dantup): Try to generate this from the form.ts file once it has a - // stable location. - // https://github.com/golang/vscode-go/blob/fecc31339bc33de4b1db2a2242ba46ea552d0f39/extension/src/language/form.ts + // TODO(dantup): Generate this from a JSON metadata file if one is made in the + // same format as the LSP metaModel file. interface('InteractiveParams', [ field( 'formFields', @@ -17,7 +16,7 @@ final interactiveFormClasses = [ type: 'FormField', canBeUndefined: true, comment: - 'FormFields defines the questions and validation errors in previous ' + 'The questions and validation errors in previous ' 'answers to the same questions.\n\n' 'This is a server-to-client field. The language server defines ' 'these, and the client uses them to render the form.\n\n' @@ -26,23 +25,32 @@ final interactiveFormClasses = [ ), field( 'formAnswers', + type: 'FormAnswer', array: true, - type: 'LSPAny', canBeUndefined: true, comment: - 'FormAnswers contains the values for the form questions.\n\n' - 'When sent by the language server, this field is optional but ' - 'recommended to support editing previous values.\n\n' - 'When sent by the language client as part of the ResolveXXX request, ' - 'this field is required. The slice must have the same length as ' - 'FormFields (one answer per question), where the answer at index i ' - 'corresponds to the field at index i.', + 'The answers for the form questions.\n\n' + 'When sent by the language server, this field is optional and ' + 'contains the current or default answers to the questions to support ' + 'editing previous values.\n\n' + "When sent by the language client, this field contains the user's " + 'answers.\n\n' + "Answers are linked to their respective questions using the field's " + 'unique `id` rather than their array index. The list must not ' + "contain duplicate IDs, and each answer's ID must correspond to a " + 'field ID defined in `formFields`.\n\n' + 'The client must include answers for all required fields (where ' + '`required` is true). Answers for optional fields (where `required` ' + 'is false) may be omitted if no answer was provided, or included if ' + 'an answer is available.', ), field( 'data', type: 'LSPAny', canBeUndefined: true, - comment: 'Context preserved for the server.', + comment: + 'Additional data that the client preserves for the server. This ' + 'data is for server use only and the client should not inspect it.', ), ]), interface( @@ -69,6 +77,14 @@ final interactiveFormClasses = [ ), interface('FormField', [ + field( + 'id', + type: 'String', + comment: + 'A unique identifier for this field. This key is used as the ' + "property name in FormAnswers to map the user's input back to this " + 'specific field.', + ), field( 'description', type: 'String', @@ -81,6 +97,11 @@ final interactiveFormClasses = [ type: 'FormFieldType', comment: 'The data type and validation constraints for the answer.', ), + field( + 'required', + type: 'boolean', + comment: 'Whether an answer is absolutely required for this field.', + ), field( 'default', type: 'LSPAny', @@ -99,6 +120,14 @@ final interactiveFormClasses = [ 'null, the current answer is considered valid.', ), ], comment: 'A single question in a form and its validation state.'), + interface('FormAnswer', [ + field( + 'id', + type: 'String', + comment: 'The ID of the FormField being answered.', + ), + field('value', type: 'LSPAny', comment: "The user's answer value."), + ], comment: 'A single answer to a FormField, identified by its unique ID.'), // Field kinds interface('FormFieldType', sealed: true, [field('kind', type: 'String')]), @@ -113,16 +142,29 @@ final interactiveFormClasses = [ field( 'existence', type: 'FileExistence', + canBeUndefined: true, comment: 'Existence constraint.', ), field( 'type', type: 'FileType', + canBeUndefined: true, comment: 'Type specifies the set of allowed file types (regular file, ' 'directory, etc).\n\n' 'Only applicable against existing file.', ), + field( + 'filters', + type: 'string', + array: true, + canBeUndefined: true, + comment: + 'Filters specifies the allowed file extensions without the leading ' + 'dot. A file is valid if it matches any of the extensions ' + '(OR logic). e.g. ["png", "jpg"].\n\n' + 'If omitted or empty, no extension filter is applied.', + ), ], comment: 'FormFieldTypeFile defines an input for a file or directory URI.\n\n' diff --git a/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart b/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart index 9c05de462be..90cc88f4bd8 100644 --- a/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart +++ b/third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart @@ -445,6 +445,33 @@ bool _canParseListFlutterWidgetPreviewDetails( return true; } +bool _canParseListFormAnswer( + Map map, LspJsonReporter reporter, String fieldName, + {required bool allowsUndefined, required bool allowsNull}) { + reporter.push(fieldName); + try { + if (!allowsUndefined && !map.containsKey(fieldName)) { + reporter.reportError('must not be undefined'); + return false; + } + final value = map[fieldName]; + final nullCheck = allowsNull || allowsUndefined; + if (!nullCheck && value == null) { + reporter.reportError('must not be null'); + return false; + } + if ((!nullCheck || value != null) && + (value is! List || + value.any((item) => !FormAnswer.canParse(item, reporter)))) { + reporter.reportError('must be of type List'); + return false; + } + } finally { + reporter.pop(); + } + return true; +} + bool _canParseListFormField( Map map, LspJsonReporter reporter, String fieldName, {required bool allowsUndefined, required bool allowsNull}) { @@ -2799,6 +2826,70 @@ class FlutterWidgetPreviews implements ToJsonable { } } +/// A single answer to a FormField, identified by its unique ID. +class FormAnswer implements ToJsonable { + static const jsonHandler = LspJsonHandler( + FormAnswer.canParse, + FormAnswer.fromJson, + ); + + /// The ID of the FormField being answered. + final String id; + + /// The user's answer value. + final LSPAny value; + + FormAnswer({ + required this.id, + this.value, + }); + @override + int get hashCode => Object.hash( + id, + value, + ); + + @override + bool operator ==(Object other) { + return other is FormAnswer && + other.runtimeType == FormAnswer && + id == other.id && + value == other.value; + } + + @override + Map toJson() { + var result = {}; + result['id'] = id; + result['value'] = value; + return result; + } + + @override + String toString() => jsonEncoder.convert(toJson()); + + static bool canParse(Object? obj, LspJsonReporter reporter) { + if (obj is Map) { + return _canParseString(obj, reporter, 'id', + allowsUndefined: false, allowsNull: false); + } else { + reporter.reportError('must be of type FormAnswer'); + return false; + } + } + + static FormAnswer fromJson(Map json) { + final idJson = json['id']; + final id = idJson as String; + final valueJson = json['value']; + final value = valueJson; + return FormAnswer( + id: id, + value: value, + ); + } +} + /// A single question in a form and its validation state. class FormField implements ToJsonable { static const jsonHandler = LspJsonHandler( @@ -2817,12 +2908,21 @@ class FormField implements ToJsonable { /// current answer is considered valid. final String? error; + /// A unique identifier for this field. This key is used as the property name + /// in FormAnswers to map the user's input back to this specific field. + final String id; + + /// Whether an answer is absolutely required for this field. + final bool required; + /// The data type and validation constraints for the answer. final FormFieldType type; FormField({ this.defaultValue, required this.description, this.error, + required this.id, + required this.required, required this.type, }); @override @@ -2830,6 +2930,8 @@ class FormField implements ToJsonable { defaultValue, description, error, + id, + required, type, ); @@ -2840,6 +2942,8 @@ class FormField implements ToJsonable { defaultValue == other.defaultValue && description == other.description && error == other.error && + id == other.id && + required == other.required && type == other.type; } @@ -2853,6 +2957,8 @@ class FormField implements ToJsonable { if (error != null) { result['error'] = error; } + result['id'] = id; + result['required'] = required; result['type'] = type.toJson(); return result; } @@ -2870,6 +2976,14 @@ class FormField implements ToJsonable { allowsUndefined: true, allowsNull: false)) { return false; } + if (!_canParseString(obj, reporter, 'id', + allowsUndefined: false, allowsNull: false)) { + return false; + } + if (!_canParseBool(obj, reporter, 'required', + allowsUndefined: false, allowsNull: false)) { + return false; + } return _canParseFormFieldType(obj, reporter, 'type', allowsUndefined: false, allowsNull: false); } else { @@ -2885,12 +2999,18 @@ class FormField implements ToJsonable { final description = descriptionJson as String; final errorJson = json['error']; final error = errorJson as String?; + final idJson = json['id']; + final id = idJson as String; + final requiredJson = json['required']; + final required = requiredJson as bool; final typeJson = json['type']; final type = FormFieldType.fromJson(typeJson as Map); return FormField( defaultValue: defaultValue, description: description, error: error, + id: id, + required: required, type: type, ); } @@ -3029,7 +3149,14 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { ); /// Existence constraint. - final FileExistence existence; + final FileExistence? existence; + + /// Filters specifies the allowed file extensions without the leading dot. A + /// file is valid if it matches any of the extensions (OR logic). e.g. ["png", + /// "jpg"]. + /// + /// If omitted or empty, no extension filter is applied. + final List? filters; @override final String kind; @@ -3038,11 +3165,12 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { /// etc). /// /// Only applicable against existing file. - final FileType type; + final FileType? type; FormFieldTypeFile({ - required this.existence, + this.existence, + this.filters, this.kind = 'file', - required this.type, + this.type, }) { if (kind != 'file') { throw 'kind may only be the literal \'file\''; @@ -3051,6 +3179,7 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { @override int get hashCode => Object.hash( existence, + lspHashCode(filters), kind, type, ); @@ -3060,6 +3189,7 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { return other is FormFieldTypeFile && other.runtimeType == FormFieldTypeFile && existence == other.existence && + const DeepCollectionEquality().equals(filters, other.filters) && kind == other.kind && type == other.type; } @@ -3067,9 +3197,16 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { @override Map toJson() { var result = {}; - result['existence'] = existence.toJson(); + if (existence != null) { + result['existence'] = existence?.toJson(); + } + if (filters != null) { + result['filters'] = filters; + } result['kind'] = kind; - result['type'] = type.toJson(); + if (type != null) { + result['type'] = type?.toJson(); + } return result; } @@ -3079,7 +3216,11 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { static bool canParse(Object? obj, LspJsonReporter reporter) { if (obj is Map) { if (!_canParseFileExistence(obj, reporter, 'existence', - allowsUndefined: false, allowsNull: false)) { + allowsUndefined: true, allowsNull: false)) { + return false; + } + if (!_canParseListString(obj, reporter, 'filters', + allowsUndefined: true, allowsNull: false)) { return false; } if (!_canParseLiteral(obj, reporter, 'kind', @@ -3087,7 +3228,7 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { return false; } return _canParseFileType(obj, reporter, 'type', - allowsUndefined: false, allowsNull: false); + allowsUndefined: true, allowsNull: false); } else { reporter.reportError('must be of type FormFieldTypeFile'); return false; @@ -3096,13 +3237,19 @@ class FormFieldTypeFile implements FormFieldType, ToJsonable { static FormFieldTypeFile fromJson(Map json) { final existenceJson = json['existence']; - final existence = FileExistence.fromJson(existenceJson as int); + final existence = existenceJson != null + ? FileExistence.fromJson(existenceJson as int) + : null; + final filtersJson = json['filters']; + final filters = + (filtersJson as List?)?.map((item) => item as String).toList(); final kindJson = json['kind']; final kind = kindJson as String; final typeJson = json['type']; - final type = FileType.fromJson(typeJson as int); + final type = typeJson != null ? FileType.fromJson(typeJson as int) : null; return FormFieldTypeFile( existence: existence, + filters: filters, kind: kind, type: type, ); @@ -3337,24 +3484,32 @@ class InteractiveExecuteCommandParams @override final String command; - /// Context preserved for the server. + /// Additional data that the client preserves for the server. This data is for + /// server use only and the client should not inspect it. @override final LSPAny data; - /// FormAnswers contains the values for the form questions. + /// The answers for the form questions. /// - /// When sent by the language server, this field is optional but recommended - /// to support editing previous values. + /// When sent by the language server, this field is optional and contains the + /// current or default answers to the questions to support editing previous + /// values. /// - /// When sent by the language client as part of the ResolveXXX request, this - /// field is required. The slice must have the same length as FormFields (one - /// answer per question), where the answer at index i corresponds to the field - /// at index i. + /// When sent by the language client, this field contains the user's answers. + /// + /// Answers are linked to their respective questions using the field's unique + /// `id` rather than their array index. The list must not contain duplicate + /// IDs, and each answer's ID must correspond to a field ID defined in + /// `formFields`. + /// + /// The client must include answers for all required fields (where `required` + /// is true). Answers for optional fields (where `required` is false) may be + /// omitted if no answer was provided, or included if an answer is available. @override - final List? formAnswers; + final List? formAnswers; - /// FormFields defines the questions and validation errors in previous answers - /// to the same questions. + /// The questions and validation errors in previous answers to the same + /// questions. /// /// This is a server-to-client field. The language server defines these, and /// the client uses them to render the form. @@ -3408,7 +3563,8 @@ class InteractiveExecuteCommandParams result['data'] = data; } if (formAnswers != null) { - result['formAnswers'] = formAnswers; + result['formAnswers'] = + formAnswers?.map((item) => item.toJson()).toList(); } if (formFields != null) { result['formFields'] = formFields?.map((item) => item.toJson()).toList(); @@ -3432,7 +3588,7 @@ class InteractiveExecuteCommandParams allowsUndefined: false, allowsNull: false)) { return false; } - if (!_canParseListObjectNullable(obj, reporter, 'formAnswers', + if (!_canParseListFormAnswer(obj, reporter, 'formAnswers', allowsUndefined: true, allowsNull: false)) { return false; } @@ -3457,8 +3613,9 @@ class InteractiveExecuteCommandParams final dataJson = json['data']; final data = dataJson; final formAnswersJson = json['formAnswers']; - final formAnswers = - (formAnswersJson as List?)?.map((item) => item).toList(); + final formAnswers = (formAnswersJson as List?) + ?.map((item) => FormAnswer.fromJson(item as Map)) + .toList(); final formFieldsJson = json['formFields']; final formFields = (formFieldsJson as List?) ?.map((item) => FormField.fromJson(item as Map)) @@ -3483,22 +3640,30 @@ class InteractiveParams implements ToJsonable { InteractiveParams.fromJson, ); - /// Context preserved for the server. + /// Additional data that the client preserves for the server. This data is for + /// server use only and the client should not inspect it. final LSPAny data; - /// FormAnswers contains the values for the form questions. + /// The answers for the form questions. /// - /// When sent by the language server, this field is optional but recommended - /// to support editing previous values. + /// When sent by the language server, this field is optional and contains the + /// current or default answers to the questions to support editing previous + /// values. /// - /// When sent by the language client as part of the ResolveXXX request, this - /// field is required. The slice must have the same length as FormFields (one - /// answer per question), where the answer at index i corresponds to the field - /// at index i. - final List? formAnswers; + /// When sent by the language client, this field contains the user's answers. + /// + /// Answers are linked to their respective questions using the field's unique + /// `id` rather than their array index. The list must not contain duplicate + /// IDs, and each answer's ID must correspond to a field ID defined in + /// `formFields`. + /// + /// The client must include answers for all required fields (where `required` + /// is true). Answers for optional fields (where `required` is false) may be + /// omitted if no answer was provided, or included if an answer is available. + final List? formAnswers; - /// FormFields defines the questions and validation errors in previous answers - /// to the same questions. + /// The questions and validation errors in previous answers to the same + /// questions. /// /// This is a server-to-client field. The language server defines these, and /// the client uses them to render the form. @@ -3534,7 +3699,8 @@ class InteractiveParams implements ToJsonable { result['data'] = data; } if (formAnswers != null) { - result['formAnswers'] = formAnswers; + result['formAnswers'] = + formAnswers?.map((item) => item.toJson()).toList(); } if (formFields != null) { result['formFields'] = formFields?.map((item) => item.toJson()).toList(); @@ -3547,7 +3713,7 @@ class InteractiveParams implements ToJsonable { static bool canParse(Object? obj, LspJsonReporter reporter) { if (obj is Map) { - if (!_canParseListObjectNullable(obj, reporter, 'formAnswers', + if (!_canParseListFormAnswer(obj, reporter, 'formAnswers', allowsUndefined: true, allowsNull: false)) { return false; } @@ -3566,8 +3732,9 @@ class InteractiveParams implements ToJsonable { final dataJson = json['data']; final data = dataJson; final formAnswersJson = json['formAnswers']; - final formAnswers = - (formAnswersJson as List?)?.map((item) => item).toList(); + final formAnswers = (formAnswersJson as List?) + ?.map((item) => FormAnswer.fromJson(item as Map)) + .toList(); final formFieldsJson = json['formFields']; final formFields = (formFieldsJson as List?) ?.map((item) => FormField.fromJson(item as Map))