diff --git a/flutter-candidate.txt b/flutter-candidate.txt index ef5c9d3396f..281bf477ad8 100644 --- a/flutter-candidate.txt +++ b/flutter-candidate.txt @@ -1 +1 @@ -48f87a5fe76aa65f89f37cc716e50b34933e78e9 +dd671fae53d37eb15e0f8fc94cd52c2f2ff147ee diff --git a/packages/devtools_app/.vscode/launch.json b/packages/devtools_app/.vscode/launch.json index 2ab1290adfa..e0ad61c69dd 100644 --- a/packages/devtools_app/.vscode/launch.json +++ b/packages/devtools_app/.vscode/launch.json @@ -61,11 +61,20 @@ "request": "attach", }, { - "name": "standalone_ui/property_editor_sidebar", + "name": "property_editor_sidebar", "request": "launch", "type": "dart", "program": "test/test_infra/scenes/standalone_ui/property_editor_sidebar.stager_app.g.dart", }, + { + "name": "property_editor_sidebar + experiments", + "request": "launch", + "type": "dart", + "program": "test/test_infra/scenes/standalone_ui/property_editor_sidebar.stager_app.g.dart", + "args": [ + "--dart-define=enable_experiments=true" + ] + }, ] } diff --git a/packages/devtools_app/lib/src/shared/analytics/constants/_property_editor_sidebar_constants.dart b/packages/devtools_app/lib/src/shared/analytics/constants/_property_editor_sidebar_constants.dart index 73a1882aa1b..3931f10c442 100644 --- a/packages/devtools_app/lib/src/shared/analytics/constants/_property_editor_sidebar_constants.dart +++ b/packages/devtools_app/lib/src/shared/analytics/constants/_property_editor_sidebar_constants.dart @@ -9,13 +9,6 @@ extension PropertyEditorSidebar on Never { /// Analytics id to track events that come from the DTD editor sidebar. static String get id => 'propertyEditorSidebar'; - /// Identifier for errors returned from the getEditableArguments API. - static String get getEditableArgumentsIdentifier => - '${id}Error-getEditableArguments'; - - /// Identifier for errors returned from the editArgument API. - static String get editArgumentIdentifier => '${id}Error-editArgument'; - /// Analytics id for opening the documentation. static String get documentationLink => 'propertyEditorDocumentation'; diff --git a/packages/devtools_app/lib/src/shared/editor/api_classes.dart b/packages/devtools_app/lib/src/shared/editor/api_classes.dart index 612fc47f0ba..b30b3a32c20 100644 --- a/packages/devtools_app/lib/src/shared/editor/api_classes.dart +++ b/packages/devtools_app/lib/src/shared/editor/api_classes.dart @@ -31,14 +31,36 @@ enum EditorMethod { /// TODO(https://github.com/flutter/devtools/issues/8824): Add tests that these /// are in-sync with analysis_server. enum LspMethod { + codeAction(methodName: 'textDocument/codeAction'), editableArguments(methodName: 'dart/textDocument/editableArguments'), - editArgument(methodName: 'dart/textDocument/editArgument'); + editArgument(methodName: 'dart/textDocument/editArgument'), + executeCommand(methodName: 'workspace/executeCommand'); const LspMethod({required this.methodName}); + /// Returns the [LspMethod] for the given [methodName]. + /// + /// If the [methodName] does not exist, returns null. + static LspMethod? fromMethodName(String methodName) => + _methodNameToMethodLookup[methodName]; + final String methodName; - String get experimentalMethodName => 'experimental/$methodName'; + static final _methodNameToMethodLookup = { + for (final method in LspMethod.values) method.methodName: method, + }; + + static final _registrationStatus = { + for (final method in LspMethod.values) method: false, + }; + + /// Sets the registration status for this LSP method. + set isRegistered(bool isRegistered) { + _registrationStatus[this] = isRegistered; + } + + /// Gets the current registration status of this LSP method. + bool get isRegistered => _registrationStatus[this] ?? false; } /// Known kinds of events that may come from the editor. @@ -82,12 +104,14 @@ enum EditorEventKind { /// Constants for all fields used in JSON maps to avoid literal strings that /// may have typos sprinkled throughout the API classes. abstract class Field { + static const actions = 'actions'; static const active = 'active'; static const anchor = 'anchor'; static const arguments = 'arguments'; static const backgroundColor = 'backgroundColor'; static const category = 'category'; static const character = 'character'; + static const command = 'command'; static const debuggerType = 'debuggerType'; static const debugSession = 'debugSession'; static const debugSessionId = 'debugSessionId'; @@ -115,6 +139,7 @@ abstract class Field { static const isEditable = 'isEditable'; static const isNullable = 'isNullable'; static const isRequired = 'isRequired'; + static const kind = 'kind'; static const line = 'line'; static const name = 'name'; static const options = 'options'; @@ -133,6 +158,7 @@ abstract class Field { static const supportsForceExternal = 'supportsForceExternal'; static const textDocument = 'textDocument'; static const theme = 'theme'; + static const title = 'title'; static const type = 'type'; static const uri = 'uri'; static const value = 'value'; @@ -501,6 +527,63 @@ class EditableArgumentsResult with Serializable { }; } +/// Constants for [CodeActionCommand] prefixes used to filter the results +/// returned by an [LspMethod.codeAction] request. +abstract class CodeActionPrefixes { + static const flutterWrap = 'refactor.flutter.wrap'; +} + +/// The result of an [LspMethod.codeAction] request to the Analysis Server. +/// +/// Contains a list of [CodeActionCommand]s that can be performed. +class CodeActionResult with Serializable { + CodeActionResult({required this.actions}); + + CodeActionResult.fromJson(List> list) + : this(actions: list.map(CodeActionCommand.fromJson).toList()); + + final List actions; + + @override + Map toJson() => {Field.actions: actions}; +} + +/// A code action (also known as a "Refactor" or "Quick Fix") that can be called +/// via an [LspMethod.executeCommand] request. +/// +/// For example, "Wrap with Center" or "Wrap with Container". +class CodeActionCommand with Serializable { + CodeActionCommand({ + required this.command, + required this.title, + required this.args, + }); + + CodeActionCommand.fromJson(Map map) + : this( + command: map[Field.command] as String, + title: map[Field.title] as String, + args: map[Field.arguments] as List? ?? [], + ); + + /// The command identifier to send to [LspMethod.executeCommand]. + final String command; + + /// The human-readable title of the command, e.g., "Wrap with Center". + final String title; + + /// Arguments that should be passed to [LspMethod.executeCommand] when + /// invoking this action. + final List args; + + @override + Map toJson() => { + Field.command: command, + Field.title: title, + Field.arguments: args, + }; +} + /// Errors that the Analysis Server returns for failed argument edits. /// /// These should be kept in sync with the error coes defined at @@ -554,16 +637,17 @@ enum EditArgumentError { } } -/// Response to an edit argument request. -class EditArgumentResponse { - EditArgumentResponse({required this.success, this.errorMessage, errorCode}) - : _errorCode = errorCode; +/// Generic response representing whether a request was a [success]. +class GenericApiResponse { + GenericApiResponse({ + required this.success, + this.errorMessage, + this.errorCode, + }); final bool success; final String? errorMessage; - final int? _errorCode; - - EditArgumentError? get errorType => EditArgumentError.fromCode(_errorCode); + final int? errorCode; } /// Information about a single editable argument of a widget. diff --git a/packages/devtools_app/lib/src/shared/editor/editor_client.dart b/packages/devtools_app/lib/src/shared/editor/editor_client.dart index b8ebc72264d..bb93305a0ef 100644 --- a/packages/devtools_app/lib/src/shared/editor/editor_client.dart +++ b/packages/devtools_app/lib/src/shared/editor/editor_client.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:devtools_app_shared/utils.dart'; +import 'package:devtools_shared/devtools_shared.dart'; import 'package:dtd/dtd.dart'; import 'package:flutter/foundation.dart'; import 'package:json_rpc_2/json_rpc_2.dart'; @@ -47,8 +48,15 @@ class EditorClient extends DisposableController final isRegistered = kind == 'ServiceRegistered'; final method = data.data['method'] as String; final capabilities = data.data['capabilities'] as Map?; - - if (method == EditorMethod.getDevices.name) { + final lspMethod = LspMethod.fromMethodName(method); + if (lspMethod != null) { + lspMethod.isRegistered = isRegistered; + if (lspMethod == LspMethod.editableArguments) { + // Update the notifier so that the Property Editor is aware that the + // editableArguments API is registered. + _editableArgumentsApiIsRegistered.value = isRegistered; + } + } else if (method == EditorMethod.getDevices.name) { _supportsGetDevices = isRegistered; } else if (method == EditorMethod.getDebugSessions.name) { _supportsGetDebugSessions = isRegistered; @@ -62,18 +70,6 @@ class EditorClient extends DisposableController _supportsOpenDevToolsPage = isRegistered; _supportsOpenDevToolsForceExternal = capabilities?[Field.supportsForceExternal] == true; - } else if (method == LspMethod.editArgument.methodName) { - _editArgumentMethodName.value = LspMethod.editArgument.methodName; - } else if (method == LspMethod.editArgument.experimentalMethodName) { - _editArgumentMethodName.value = - LspMethod.editArgument.experimentalMethodName; - } else if (method == LspMethod.editableArguments.methodName) { - _editableArgumentsMethodName.value = - LspMethod.editableArguments.methodName; - } else if (method == - LspMethod.editableArguments.experimentalMethodName) { - _editableArgumentsMethodName.value = - LspMethod.editableArguments.experimentalMethodName; } else { return; } @@ -165,13 +161,9 @@ class EditorClient extends DisposableController _supportsOpenDevToolsForceExternal; var _supportsOpenDevToolsForceExternal = false; - ValueListenable get editArgumentMethodName => - _editArgumentMethodName; - final _editArgumentMethodName = ValueNotifier(null); - - ValueListenable get editableArgumentsMethodName => - _editableArgumentsMethodName; - final _editableArgumentsMethodName = ValueNotifier(null); + ValueListenable get editableArgumentsApiIsRegistered => + _editableArgumentsApiIsRegistered; + final _editableArgumentsApiIsRegistered = ValueNotifier(false); /// A stream of [ActiveLocationChangedEvent]s from the edtior. Stream get activeLocationChangedStream => @@ -256,26 +248,87 @@ class EditorClient extends DisposableController Future getEditableArguments({ required TextDocument textDocument, required CursorPosition position, + required String screenId, + }) => _callLspApiAndDeserializeResponse( + requestMethod: LspMethod.editableArguments, + requestParams: { + 'textDocument': textDocument.toJson(), + 'position': position.toJson(), + }, + responseDeserializer: (rawJson) => + EditableArgumentsResult.fromJson(rawJson as Map), + screenId: screenId, + ); + + /// Gets the supported refactors from the Analysis Server. + Future getRefactors({ + required TextDocument textDocument, + required EditorRange range, + required String screenId, + }) => _callLspApiAndDeserializeResponse( + requestMethod: LspMethod.codeAction, + requestParams: { + 'textDocument': textDocument.toJson(), + 'range': range.toJson(), + 'context': { + 'diagnostics': [], + 'only': [CodeActionPrefixes.flutterWrap], + }, + }, + responseDeserializer: (rawJson) => CodeActionResult.fromJson( + (rawJson as List).cast>(), + ), + screenId: screenId, + ); + + /// Requests that the Analysis Server makes a code edit for an argument. + Future editArgument({ + required TextDocument textDocument, + required CursorPosition position, + required String name, + required T value, + required String screenId, + }) => _callLspApiAndRespond( + requestMethod: LspMethod.editArgument, + requestParams: { + 'textDocument': textDocument.toJson(), + 'position': position.toJson(), + 'edit': {'name': name, 'newValue': value}, + }, + screenId: screenId, + ); + + /// Requests that the Analysis Server execute the given [commandName]. + Future executeCommand({ + required String commandName, + required List commandArgs, + required String screenId, + }) => _callLspApiAndRespond( + requestMethod: LspMethod.executeCommand, + requestParams: {'command': commandName, 'arguments': commandArgs}, + screenId: screenId, + ); + + Future _callLspApiAndDeserializeResponse({ + required LspMethod requestMethod, + required Map requestParams, + required T? Function(Object? rawJson) responseDeserializer, + required String screenId, }) async { - final method = editableArgumentsMethodName.value; - if (method == null) return null; + if (!requestMethod.isRegistered) { + return null; + } String? errorMessage; StackTrace? stack; - EditableArgumentsResult? result; + T? result; try { final response = await _callLspApi( - method, - params: { - 'type': 'Object', // This is required by DTD. - 'textDocument': textDocument.toJson(), - 'position': position.toJson(), - }, + requestMethod.methodName, + params: _addRequiredDtdParams(requestParams), ); - final rawResult = response.result[Field.result]; - result = rawResult != null - ? EditableArgumentsResult.fromJson(rawResult as Map) - : null; + final rawJson = response.result[Field.result]; + result = rawJson != null ? responseDeserializer(rawJson) : null; } on RpcException catch (e, st) { // We expect content modified errors if a user edits their code before the // request completes. Therefore it is safe to ignore. @@ -290,25 +343,21 @@ class EditorClient extends DisposableController if (errorMessage != null) { reportError( errorMessage, - errorType: PropertyEditorSidebar.getEditableArgumentsIdentifier, + errorType: _lspErrorType(screenId: screenId, method: requestMethod), stack: stack, ); } } - return result; } - /// Requests that the Analysis Server makes a code edit for an argument. - Future editArgument({ - required TextDocument textDocument, - required CursorPosition position, - required String name, - required T value, + Future _callLspApiAndRespond({ + required LspMethod requestMethod, + required Map requestParams, + required String screenId, }) async { - final method = editArgumentMethodName.value; - if (method == null) { - return EditArgumentResponse( + if (!requestMethod.isRegistered) { + return GenericApiResponse( success: false, errorMessage: 'API is unavailable.', ); @@ -318,15 +367,10 @@ class EditorClient extends DisposableController StackTrace? stack; try { await _callLspApi( - method, - params: { - 'type': 'Object', // This is required by DTD. - 'textDocument': textDocument.toJson(), - 'position': position.toJson(), - 'edit': {'name': name, 'newValue': value}, - }, + requestMethod.methodName, + params: _addRequiredDtdParams(requestParams), ); - return EditArgumentResponse(success: true); + return GenericApiResponse(success: true); } on RpcException catch (e, st) { errorMessage = e.message; stack = st; @@ -337,12 +381,21 @@ class EditorClient extends DisposableController if (errorMessage != null) { reportError( errorMessage, - errorType: PropertyEditorSidebar.editArgumentIdentifier, + errorType: _lspErrorType(screenId: screenId, method: requestMethod), stack: stack, ); } } - return EditArgumentResponse(success: false, errorMessage: errorMessage); + return GenericApiResponse(success: false, errorMessage: errorMessage); + } + + String _lspErrorType({required String screenId, required LspMethod method}) => + '${screenId}Error-${method.name}'; + + Map _addRequiredDtdParams(Map params) { + // Specifying type as 'Object' is required by DTD. + params.putIfAbsent('type', () => 'Object'); + return params; } Future _call( diff --git a/packages/devtools_app/lib/src/shared/feature_flags.dart b/packages/devtools_app/lib/src/shared/feature_flags.dart index c15e1298b80..6079842b46f 100644 --- a/packages/devtools_app/lib/src/shared/feature_flags.dart +++ b/packages/devtools_app/lib/src/shared/feature_flags.dart @@ -95,6 +95,11 @@ extension FeatureFlags on Never { /// https://github.com/flutter/devtools/issues/7854 static bool propertyEditor = enableExperiments; + /// Flag to enable refactors in the Flutter Property Editor sidebar. + /// + /// https://github.com/flutter/devtools/issues/8652 + static bool propertyEditorRefactors = enableExperiments; + /// Stores a map of all the feature flags for debugging purposes. /// /// When adding a new flag, you are responsible for adding it to this map as diff --git a/packages/devtools_app/lib/src/shared/primitives/custom_pointer_scroll_view.dart b/packages/devtools_app/lib/src/shared/primitives/custom_pointer_scroll_view.dart index f1199350ec8..eb0e45f5d2a 100644 --- a/packages/devtools_app/lib/src/shared/primitives/custom_pointer_scroll_view.dart +++ b/packages/devtools_app/lib/src/shared/primitives/custom_pointer_scroll_view.dart @@ -821,7 +821,7 @@ class _RenderScrollSemantics extends RenderProxyBox { if (child.isTagged(RenderViewport.excludeFromScrolling)) { excluded.add(child); } else { - if (!child.hasFlag(SemanticsFlag.isHidden)) { + if (!child.flagsCollection.isHidden) { firstVisibleIndex ??= child.indexInParent; } included.add(child); diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart index b36f1c550f4..164a7e5d582 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart @@ -11,12 +11,14 @@ import '../../../shared/analytics/analytics.dart' as ga; import '../../../shared/analytics/constants.dart' as gac; import '../../../shared/editor/api_classes.dart'; import '../../../shared/editor/editor_client.dart'; +import '../../../shared/feature_flags.dart'; import '../../../shared/ui/filter.dart'; import '../../../shared/utils/utils.dart'; import 'property_editor_types.dart'; typedef EditableWidgetData = ({ List properties, + List refactors, String? name, String? documentation, String? fileUri, @@ -24,7 +26,7 @@ typedef EditableWidgetData = ({ }); typedef EditArgumentFunction = - Future Function({ + Future Function({ required String name, required T value, }); @@ -59,11 +61,11 @@ class PropertyEditorController extends DisposableController bool get waitingForFirstEvent => _waitingForFirstEvent; bool _waitingForFirstEvent = true; - late final Debouncer _editableArgsDebouncer; + late final Debouncer _requestDebouncer; late final Timer _checkConnectionTimer; - static const _editableArgsDebounceDuration = Duration(milliseconds: 600); + static const _requestDebounceDuration = Duration(milliseconds: 600); static const _checkConnectionInterval = Duration(minutes: 1); @@ -82,7 +84,7 @@ class PropertyEditorController extends DisposableController @override void init() { super.init(); - _editableArgsDebouncer = Debouncer(duration: _editableArgsDebounceDuration); + _requestDebouncer = Debouncer(duration: _requestDebounceDuration); _checkConnectionTimer = _periodicallyCheckConnection( _checkConnectionInterval, ); @@ -111,6 +113,7 @@ class PropertyEditorController extends DisposableController if (!textDocument.uriAsString.endsWith('.dart')) { _editableWidgetData.value = ( properties: [], + refactors: [], name: null, documentation: null, range: null, @@ -118,8 +121,8 @@ class PropertyEditorController extends DisposableController ); return; } - _editableArgsDebouncer.run( - () => _updateWithEditableArgs( + _requestDebouncer.run( + () => _updateWithEditableWidgetData( textDocument: textDocument, cursorPosition: cursorPosition, ), @@ -130,7 +133,7 @@ class PropertyEditorController extends DisposableController @override void dispose() { - _editableArgsDebouncer.dispose(); + _requestDebouncer.dispose(); _checkConnectionTimer.cancel(); super.dispose(); } @@ -152,7 +155,7 @@ class PropertyEditorController extends DisposableController ..addAll(filtered); } - Future editArgument({ + Future editArgument({ required String name, required T value, }) async { @@ -164,6 +167,7 @@ class PropertyEditorController extends DisposableController position: position, name: name, value: value, + screenId: gac.PropertyEditorSidebar.id, ); } @@ -193,32 +197,41 @@ class PropertyEditorController extends DisposableController ); } - Future _updateWithEditableArgs({ + Future _updateWithEditableWidgetData({ required TextDocument textDocument, required CursorPosition cursorPosition, }) async { _currentDocument = textDocument; _currentCursorPosition = cursorPosition; // Get the editable arguments for the current position. - final result = await editorClient.getEditableArguments( + final editableArgsResult = await editorClient.getEditableArguments( textDocument: textDocument, position: cursorPosition, + screenId: gac.PropertyEditorSidebar.id, ); - final properties = (result?.args ?? []) - .map(argToProperty) - .nonNulls - // Filter out any deprecated properties that aren't set. - .where((property) => !property.isDeprecated || property.hasArgument) - .toList(); - final name = result?.name; - final range = result?.range; - + CodeActionResult? refactorsResult; + // TODO(https://github.com/flutter/devtools/issues/8652): Enable refactors + // in the Property Editor by default. + if (FeatureFlags.propertyEditorRefactors) { + // Get any supported refactors for the current position. + // TODO(elliette): Consider updating the widget data immediately without + // waiting for the refactors result, then updating the refactor buttons + // once the refactors result is available. + refactorsResult = await editorClient.getRefactors( + textDocument: textDocument, + range: EditorRange(start: cursorPosition, end: cursorPosition), + screenId: gac.PropertyEditorSidebar.id, + ); + } + // Update the widget data. + final name = editableArgsResult?.name; _editableWidgetData.value = ( - properties: properties, + properties: _extractProperties(editableArgsResult), + refactors: _extractRefactors(refactorsResult), name: name, - documentation: result?.documentation, + documentation: editableArgsResult?.documentation, fileUri: _currentDocument?.uriAsString, - range: range, + range: editableArgsResult?.range, ); filterData(activeFilter.value); // Register impression. @@ -228,6 +241,17 @@ class PropertyEditorController extends DisposableController ); } + List _extractProperties(EditableArgumentsResult? result) => + (result?.args ?? []) + .map(argToProperty) + .nonNulls + // Filter out any deprecated properties that aren't set. + .where((property) => !property.isDeprecated || property.hasArgument) + .toList(); + + List _extractRefactors(CodeActionResult? result) => + (result?.actions ?? []).toList(); + Timer _periodicallyCheckConnection(Duration interval) { return Timer.periodic(interval, (timer) { final isClosed = editorClient.isDtdClosed; @@ -259,6 +283,9 @@ class PropertyEditorController extends DisposableController .map(argToProperty) .nonNulls .toList(), + // TODO(https://github.com/flutter/devtools/issues/8652): Add tests for + // refactors. + refactors: [], name: editableArgsResult.name, documentation: editableArgsResult.documentation, fileUri: document?.uriAsString, diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart index 1ec55bfe84a..d9b6b2aec09 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_inputs.dart @@ -380,7 +380,7 @@ mixin _PropertyInputMixin on State { } void _handleServerResponse( - EditArgumentResponse? errorResponse, { + GenericApiResponse? errorResponse, { required EditableProperty property, }) { final succeeded = errorResponse == null || errorResponse.success; @@ -401,10 +401,10 @@ mixin _PropertyInputMixin on State { } String _errorMessage( - EditArgumentResponse errorResponse, { + GenericApiResponse errorResponse, { required EditableProperty property, }) { - final errorType = errorResponse.errorType; + final errorType = EditArgumentError.fromCode(errorResponse.errorCode); final messageFromType = errorType?.message; final messageFromResponse = errorResponse.errorMessage; final errorMessage = diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart index 810d55e16f3..0c18a827200 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_view.dart @@ -24,20 +24,16 @@ class PropertyEditorView extends StatelessWidget { Widget build(BuildContext context) { return MultiValueListenableBuilder( listenables: [ - controller.editorClient.editArgumentMethodName, - controller.editorClient.editableArgumentsMethodName, + controller.editorClient.editableArgumentsApiIsRegistered, controller.editableWidgetData, ], builder: (_, values, _) { - final editArgumentMethodName = values.first as String?; - final editableArgumentsMethodName = values.second as String?; - - if (editArgumentMethodName == null || - editableArgumentsMethodName == null) { + final editableArgumentsApiIsRegistered = values.first as bool; + if (!editableArgumentsApiIsRegistered) { return const CenteredCircularProgressIndicator(); } - final editableWidgetData = values.third as EditableWidgetData?; + final editableWidgetData = values.second as EditableWidgetData?; return SelectionArea( child: Column( crossAxisAlignment: CrossAxisAlignment.start, @@ -56,7 +52,7 @@ class PropertyEditorView extends StatelessWidget { return [introSentence, const HowToUseMessage()]; } - final (:properties, :name, :documentation, :fileUri, :range) = + final (:properties, :refactors, :name, :documentation, :fileUri, :range) = editableWidgetData; if (fileUri != null && !fileUri.endsWith('.dart')) { return [const NoDartCodeMessage(), const HowToUseMessage()]; diff --git a/packages/devtools_app/test/shared/editor/editor_client_test.dart b/packages/devtools_app/test/shared/editor/editor_client_test.dart new file mode 100644 index 00000000000..69526c3153b --- /dev/null +++ b/packages/devtools_app/test/shared/editor/editor_client_test.dart @@ -0,0 +1,803 @@ +// Copyright 2025 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. + +import 'dart:convert'; + +import 'package:devtools_app/src/shared/editor/api_classes.dart'; +import 'package:devtools_app/src/shared/editor/editor_client.dart'; +import 'package:devtools_test/devtools_test.dart'; +import 'package:dtd/dtd.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; + +void main() { + late MockDartToolingDaemon mockDtd; + late EditorClient editorClient; + + final methodToResponseJson = { + LspMethod.codeAction: _codeActionResponseJson, + LspMethod.editableArguments: _editableArgumentsResponseJson, + LspMethod.editArgument: 'null', + LspMethod.executeCommand: 'null', + }; + + setUp(() { + mockDtd = MockDartToolingDaemon(); + + for (final MapEntry(key: method, value: responseJson) + in methodToResponseJson.entries) { + when( + mockDtd.call( + lspServiceName, + method.methodName, + params: anyNamed('params'), + ), + ).thenAnswer((_) async => _createDtdResponse(responseJson)); + } + + for (final method in LspMethod.values) { + method.isRegistered = true; + } + + editorClient = EditorClient(mockDtd); + }); + + group('getRefactors', () { + test('deserializes refactors from the CodeActionResult', () async { + final result = await editorClient.getRefactors( + textDocument: _textDocument, + range: _editorRange, + screenId: _fakeScreenId, + ); + + // Verify the expected request was sent. + verify( + mockDtd.call( + lspServiceName, + LspMethod.codeAction.methodName, + params: json.decode(_codeActionRequestJson), + ), + ).called(1); + + // Verify deserialization of the response succeeded. + expect(result, isA()); + const expectedRefactors = [ + 'Wrap with widget...', + 'Wrap with Center', + 'Wrap with Container', + 'Wrap with Expanded', + 'Wrap with Flexible', + 'Wrap with Padding', + 'Wrap with SizedBox', + 'Wrap with Column', + 'Wrap with Row', + 'Wrap with Builder', + 'Wrap with ValueListenableBuilder', + 'Wrap with StreamBuilder', + 'Wrap with FutureBuilder', + ]; + expect( + result!.actions.map((a) => a.title), + containsAll(expectedRefactors), + ); + }); + + test('returns null when API is unavailable', () async { + LspMethod.codeAction.isRegistered = false; + + final result = await editorClient.getRefactors( + textDocument: _textDocument, + range: _editorRange, + screenId: _fakeScreenId, + ); + + // Verify that the request was never sent. + verifyNever(mockDtd.call(any, any, params: anyNamed('params'))); + + // Verify the response is null. + expect(result, isNull); + }); + }); + + group('editableArguments', () { + test('deserializes arguments from the EditableArgumentsResult', () async { + final result = await editorClient.getEditableArguments( + textDocument: _textDocument, + position: _cursorPosition, + screenId: _fakeScreenId, + ); + + // Verify the expected request was sent. + verify( + mockDtd.call( + lspServiceName, + LspMethod.editableArguments.methodName, + params: json.decode(_editableArgumentsRequestJson), + ), + ).called(1); + + // Verify deserialization of the response succeeded. + final expectedArgs = [ + 'mainAxisAlignment - MainAxisAlignment.end', + 'mainAxisSize - null', + 'crossAxisAlignment - null', + 'textDirection - null', + 'verticalDirection - null', + 'textBaseline - null', + 'spacing - null', + ]; + + expect( + result!.args.map((arg) => '${arg.name} - ${arg.value}'), + containsAll(expectedArgs), + ); + }); + + test('returns null when API is not available', () async { + LspMethod.editableArguments.isRegistered = false; + + final result = await editorClient.getEditableArguments( + textDocument: _textDocument, + position: _cursorPosition, + screenId: _fakeScreenId, + ); + + // Verify that the request was never sent. + verifyNever(mockDtd.call(any, any, params: anyNamed('params'))); + + // Verify the response is null. + expect(result, isNull); + }); + }); + + group('executeCommand', () { + test('sends an executeCommand request', () async { + final result = await editorClient.executeCommand( + commandName: 'dart.edit.codeAction.apply', + commandArgs: [json.decode(_executeCommandArg) as Map], + screenId: _fakeScreenId, + ); + + // Verify the expected request was sent. + verify( + mockDtd.call( + lspServiceName, + LspMethod.executeCommand.methodName, + params: json.decode(_executeCommandRequestJson), + ), + ).called(1); + + // Verify the response was successful. + expect(result.success, isTrue); + expect(result.errorMessage, isNull); + }); + + test('returns failure response when API is not available', () async { + LspMethod.executeCommand.isRegistered = false; + + final result = await editorClient.executeCommand( + commandName: 'dart.edit.codeAction.apply', + commandArgs: [json.decode(_executeCommandArg) as Map], + screenId: _fakeScreenId, + ); + + // Verify the request was never sent. + verifyNever(mockDtd.call(any, any, params: anyNamed('params'))); + + // Verify the failure response. + expect(result.success, isFalse); + expect(result.errorMessage, 'API is unavailable.'); + }); + }); + + group('editArgument', () { + test('sends an editArgument request', () async { + final result = await editorClient.editArgument( + textDocument: _textDocument, + position: _cursorPosition, + name: 'mainAxisAlignment', + value: 'MainAxisAlignment.center', + screenId: _fakeScreenId, + ); + + // Verify the expected request was sent. + verify( + mockDtd.call( + lspServiceName, + LspMethod.editArgument.methodName, + params: json.decode(_editArgumentRequestJson), + ), + ).called(1); + + // Verify the response is successful. + expect(result.success, isTrue); + expect(result.errorMessage, isNull); + }); + + test('returns failure response when API is not available', () async { + LspMethod.editArgument.isRegistered = false; + + final result = await editorClient.editArgument( + textDocument: _textDocument, + position: _cursorPosition, + name: 'mainAxisAlignment', + value: 'MainAxisAlignment.center', + screenId: _fakeScreenId, + ); + + // Verify that the request was never sent. + verifyNever(mockDtd.call(any, any, params: anyNamed('params'))); + + // Verify the failure response. + expect(result.success, isFalse); + expect(result.errorMessage, 'API is unavailable.'); + }); + }); +} + +const _fakeScreenId = 'DevToolsScreen'; +const _documentUri = 'file:///Users/me/flutter_app/lib/main.dart'; +const _documentVersion = 1; +const _cursorLine = 10; +const _cursorChar = 20; + +final _textDocument = TextDocument( + uriAsString: _documentUri, + version: _documentVersion, +); +final _cursorPosition = CursorPosition( + line: _cursorLine, + character: _cursorChar, +); +final _editorRange = EditorRange(start: _cursorPosition, end: _cursorPosition); + +DTDResponse _createDtdResponse(String jsonStr) { + final result = json.decode(_wrapJsonInResult(jsonStr)); + return DTDResponse('1', 'type', result); +} + +String _wrapJsonInResult(String jsonStr) => '{"result": $jsonStr}'; + +const _editArgumentRequestJson = + ''' +{ + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "position": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "edit": { + "name": "mainAxisAlignment", + "newValue": "MainAxisAlignment.center" + }, + "type": "Object" +} +'''; + +const _executeCommandArg = + ''' + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.futureBuilder", + "loggedAction": "dart.assist.flutter.wrap.futureBuilder" + } +'''; + +const _executeCommandRequestJson = + ''' +{ + "command": "dart.edit.codeAction.apply", + "arguments": [ + $_executeCommandArg + ], + "type": "Object" +} +'''; + +const _editableArgumentsRequestJson = + ''' +{ + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "position": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "type": "Object" +} +'''; + +const _codeActionRequestJson = + ''' +{ + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "start": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "end": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "context": { + "diagnostics": [], + "only": [ + "refactor.flutter.wrap" + ] + }, + "type": "Object" +} +'''; + +const _editableArgumentsResponseJson = + ''' +{ + "arguments": [ + { + "defaultValue": "MainAxisAlignment.start", + "documentation": "Creates a vertical array of children.", + "hasArgument": true, + "isDeprecated": false, + "isEditable": true, + "isNullable": false, + "isRequired": false, + "name": "mainAxisAlignment", + "options": [ + "MainAxisAlignment.start", + "MainAxisAlignment.end", + "MainAxisAlignment.center", + "MainAxisAlignment.spaceBetween", + "MainAxisAlignment.spaceAround", + "MainAxisAlignment.spaceEvenly" + ], + "type": "enum", + "value": "MainAxisAlignment.end" + }, + { + "defaultValue": "MainAxisSize.max", + "documentation": "Creates a vertical array of children.", + "hasArgument": false, + "isDeprecated": false, + "isEditable": true, + "isNullable": false, + "isRequired": false, + "name": "mainAxisSize", + "options": [ + "MainAxisSize.min", + "MainAxisSize.max" + ], + "type": "enum" + }, + { + "defaultValue": "CrossAxisAlignment.center", + "documentation": "Creates a vertical array of children.", + "hasArgument": false, + "isDeprecated": false, + "isEditable": true, + "isNullable": false, + "isRequired": false, + "name": "crossAxisAlignment", + "options": [ + "CrossAxisAlignment.start", + "CrossAxisAlignment.end", + "CrossAxisAlignment.center", + "CrossAxisAlignment.stretch", + "CrossAxisAlignment.baseline" + ], + "type": "enum" + }, + { + "documentation": "Creates a vertical array of children.", + "hasArgument": false, + "isDeprecated": false, + "isEditable": true, + "isNullable": true, + "isRequired": false, + "name": "textDirection", + "options": [ + "TextDirection.rtl", + "TextDirection.ltr" + ], + "type": "enum" + }, + { + "defaultValue": "VerticalDirection.down", + "documentation": "Creates a vertical array of children.", + "hasArgument": false, + "isDeprecated": false, + "isEditable": true, + "isNullable": false, + "isRequired": false, + "name": "verticalDirection", + "options": [ + "VerticalDirection.up", + "VerticalDirection.down" + ], + "type": "enum" + }, + { + "documentation": "Creates a vertical array of children.", + "hasArgument": false, + "isDeprecated": false, + "isEditable": true, + "isNullable": true, + "isRequired": false, + "name": "textBaseline", + "options": [ + "TextBaseline.alphabetic", + "TextBaseline.ideographic" + ], + "type": "enum" + }, + { + "defaultValue": 0.0, + "documentation": "Creates a vertical array of children.", + "hasArgument": false, + "isDeprecated": false, + "isEditable": true, + "isNullable": false, + "isRequired": false, + "name": "spacing", + "type": "double" + } + ], + "documentation": "Creates a vertical array of children.", + "name": "Column", + "range": { + "end": { + "character": 13, + "line": 64 + }, + "start": { + "character": 19, + "line": 48 + } + }, + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + } +} +'''; + +const _codeActionResponseJson = + ''' +[ + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.generic", + "loggedAction": "dart.assist.flutter.wrap.generic" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with widget..." + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.center", + "loggedAction": "dart.assist.flutter.wrap.center" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Center" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.container", + "loggedAction": "dart.assist.flutter.wrap.container" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Container" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.expanded", + "loggedAction": "dart.assist.flutter.wrap.expanded" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Expanded" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.flexible", + "loggedAction": "dart.assist.flutter.wrap.flexible" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Flexible" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.padding", + "loggedAction": "dart.assist.flutter.wrap.padding" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Padding" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.sizedBox", + "loggedAction": "dart.assist.flutter.wrap.sizedBox" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with SizedBox" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.column", + "loggedAction": "dart.assist.flutter.wrap.column" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Column" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.row", + "loggedAction": "dart.assist.flutter.wrap.row" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Row" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.builder", + "loggedAction": "dart.assist.flutter.wrap.builder" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with Builder" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.valueListenableBuilder", + "loggedAction": "dart.assist.flutter.wrap.valueListenableBuilder" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with ValueListenableBuilder" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.streamBuilder", + "loggedAction": "dart.assist.flutter.wrap.streamBuilder" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with StreamBuilder" + }, + { + "arguments": [ + { + "textDocument": { + "uri": "$_documentUri", + "version": $_documentVersion + }, + "range": { + "end": { + "character": $_cursorChar, + "line": $_cursorLine + }, + "start": { + "character": $_cursorChar, + "line": $_cursorLine + } + }, + "kind": "refactor.flutter.wrap.futureBuilder", + "loggedAction": "dart.assist.flutter.wrap.futureBuilder" + } + ], + "command": "dart.edit.codeAction.apply", + "title": "Wrap with FutureBuilder" + } +] +'''; diff --git a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart index bbc2f72d6b1..3c90ba34531 100644 --- a/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart +++ b/packages/devtools_app/test/standalone_ui/ide_shared/property_editor/property_editor_test.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:ui'; import 'package:devtools_app/devtools_app.dart'; +import 'package:devtools_app/src/shared/analytics/constants.dart' as gac; import 'package:devtools_app/src/shared/editor/api_classes.dart'; import 'package:devtools_app/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart'; import 'package:devtools_app/src/standalone_ui/ide_shared/property_editor/property_editor_types.dart'; @@ -45,11 +46,8 @@ void main() { mockEditorClient = MockEditorClient(); when( - mockEditorClient.editArgumentMethodName, - ).thenReturn(ValueNotifier(LspMethod.editArgument.methodName)); - when( - mockEditorClient.editableArgumentsMethodName, - ).thenReturn(ValueNotifier(LspMethod.editableArguments.methodName)); + mockEditorClient.editableArgumentsApiIsRegistered, + ).thenReturn(ValueNotifier(true)); when( mockEditorClient.activeLocationChangedStream, ).thenAnswer((_) => eventStream); @@ -99,6 +97,7 @@ void main() { mockEditorClient.getEditableArguments( textDocument: location.document, position: location.position, + screenId: gac.PropertyEditorSidebar.id, ), ).thenAnswer((realInvocation) { getEditableArgsCalled?.complete(); @@ -587,6 +586,7 @@ void main() { position: argThat(isNotNull, named: 'position'), name: argThat(isNotNull, named: 'name'), value: argThat(anything, named: 'value'), + screenId: 'propertyEditorSidebar', ), ).thenAnswer((realInvocation) { final calledWithArgs = realInvocation.namedArguments; @@ -598,7 +598,7 @@ void main() { ); return Future.value( - EditArgumentResponse( + GenericApiResponse( success: success, errorCode: success ? null : -32019, ), diff --git a/packages/devtools_app/test/test_infra/goldens/integration_inspector_errors_2_error_selected.png b/packages/devtools_app/test/test_infra/goldens/integration_inspector_errors_2_error_selected.png index 9e909a88f52..80f0bf9c286 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_inspector_errors_2_error_selected.png and b/packages/devtools_app/test/test_infra/goldens/integration_inspector_errors_2_error_selected.png differ diff --git a/packages/devtools_test/lib/src/mocks/generated.dart b/packages/devtools_test/lib/src/mocks/generated.dart index 6bc83a802cf..404f6a2175e 100644 --- a/packages/devtools_test/lib/src/mocks/generated.dart +++ b/packages/devtools_test/lib/src/mocks/generated.dart @@ -4,6 +4,7 @@ import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app_shared/service.dart'; +import 'package:dtd/dtd.dart'; import 'package:mockito/annotations.dart'; import 'package:vm_service/vm_service.dart'; @@ -51,5 +52,6 @@ import 'package:vm_service/vm_service.dart'; MockSpec(), MockSpec(), MockSpec(), + MockSpec(), ]) void main() {} diff --git a/packages/devtools_test/pubspec.yaml b/packages/devtools_test/pubspec.yaml index 7387257835b..b60b5a4a017 100644 --- a/packages/devtools_test/pubspec.yaml +++ b/packages/devtools_test/pubspec.yaml @@ -20,6 +20,7 @@ dependencies: devtools_app: devtools_app_shared: devtools_shared: + dtd: ^2.5.1 flutter: sdk: flutter flutter_test: