[analysis_server] Support multi-group snippets in LSP CodeActions
Fixes https://github.com/Dart-Code/Dart-Code/issues/3869.
Change-Id: Ib1925cc090fe62815c9257c72823c0e84d93efd0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/236640
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
index a1e8437..6ae02e1 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
@@ -169,12 +169,13 @@
/// version of each document being modified so it's important to call this
/// immediately after computing edits to ensure the document is not modified
/// before the version number is read.
- CodeAction _createAssistAction(SourceChange change) {
+ CodeAction _createAssistAction(SourceChange change, ResolvedUnitResult unit) {
return CodeAction(
title: change.message,
kind: toCodeActionKind(change.id, CodeActionKind.Refactor),
diagnostics: const [],
- edit: createWorkspaceEdit(server, change),
+ edit: createWorkspaceEdit(server, change,
+ allowSnippets: true, filePath: unit.path, lineInfo: unit.lineInfo),
);
}
@@ -182,12 +183,14 @@
/// version of each document being modified so it's important to call this
/// immediately after computing edits to ensure the document is not modified
/// before the version number is read.
- CodeAction _createFixAction(SourceChange change, Diagnostic diagnostic) {
+ CodeAction _createFixAction(
+ SourceChange change, Diagnostic diagnostic, ResolvedUnitResult unit) {
return CodeAction(
title: change.message,
kind: toCodeActionKind(change.id, CodeActionKind.QuickFix),
diagnostics: [diagnostic],
- edit: createWorkspaceEdit(server, change),
+ edit: createWorkspaceEdit(server, change,
+ allowSnippets: true, filePath: unit.path, lineInfo: unit.lineInfo),
);
}
@@ -269,12 +272,12 @@
final codeActions = <CodeAction>[];
codeActions.addAll(assists.map((assist) {
- final action = _createAssistAction(assist.change);
+ final action = _createAssistAction(assist.change, unit);
codeActionPriorities[action] = assist.kind.priority;
return action;
}));
codeActions.addAll(pluginChanges.map((change) {
- final action = _createAssistAction(change.change);
+ final action = _createAssistAction(change.change, unit);
codeActionPriorities[action] = change.priority;
return action;
}));
@@ -361,7 +364,7 @@
);
codeActions.addAll(
fixes.map((fix) {
- final action = _createFixAction(fix.change, diagnostic);
+ final action = _createFixAction(fix.change, diagnostic, unit);
codeActionPriorities[action] = fix.kind.priority;
return action;
}),
@@ -382,7 +385,7 @@
final pluginFixActions = pluginFixes.expand(
(fix) => fix.fixes.map((fixChange) {
final action = _createFixAction(
- fixChange.change, pluginErrorToDiagnostic(fix.error));
+ fixChange.change, pluginErrorToDiagnostic(fix.error), unit);
codeActionPriorities[action] = fixChange.priority;
return action;
}),
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
index 9026e05..8125d9a 100644
--- a/pkg/analysis_server/lib/src/lsp/mapping.dart
+++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -119,51 +119,74 @@
return edit;
}
-/// Creates a [lsp.WorkspaceEdit] from a [server.SourceChange] that can include
-/// experimental [server.SnippetTextEdit]s if the client has indicated support
-/// for these in the experimental section of their client capabilities.
+/// Creates a [lsp.WorkspaceEdit] from a [server.SourceChange].
+///
+/// Can return experimental [server.SnippetTextEdit]s if the following are true:
+/// - the client has indicated support for in the experimental section of their
+/// client capabilities, and
+/// - [allowSnippets] is true, and
+/// - [change] contains only a single edit to the single file [filePath]
+/// - [lineInfo] is provided (which should be for the single edited file)
///
/// Note: This code will fetch the version of each document being modified so
/// it's important to call this immediately after computing edits to ensure
/// the document is not modified before the version number is read.
lsp.WorkspaceEdit createWorkspaceEdit(
- lsp.LspAnalysisServer server, server.SourceChange change) {
+ lsp.LspAnalysisServer server,
+ server.SourceChange change, {
+ // The caller must specify whether snippets are valid here for where they're
+ // sending this edit. Right now, support is limited to CodeActions.
+ bool allowSnippets = false,
+ String? filePath,
+ LineInfo? lineInfo,
+}) {
// In order to return snippets, we must ensure we are only modifying a single
- // existing file with a single edit and that there is a linked edit group with
- // only one position and no suggestions.
- if (!server.clientCapabilities!.experimentalSnippetTextEdit ||
+ // existing file with a single edit and that there is either a selection or a
+ // linked edit group (otherwise there's no value in snippets).
+ if (!allowSnippets ||
+ !server.clientCapabilities!.experimentalSnippetTextEdit ||
+ !server.clientCapabilities!.documentChanges ||
+ filePath == null ||
+ lineInfo == null ||
change.edits.length != 1 ||
- change.edits.first.fileStamp == -1 || // new file
- change.edits.first.edits.length != 1 ||
- change.linkedEditGroups.isEmpty ||
- change.linkedEditGroups.first.positions.length != 1 ||
- change.linkedEditGroups.first.suggestions.isNotEmpty) {
+ change.edits.single.fileStamp == -1 || // new file
+ change.edits.single.file != filePath ||
+ change.edits.single.edits.length != 1 ||
+ (change.selection == null && change.linkedEditGroups.isEmpty)) {
return createPlainWorkspaceEdit(server, change.edits);
}
- // Additionally, the selection must fall within the edit offset.
- final edit = change.edits.first.edits.first;
- final selectionOffset = change.linkedEditGroups.first.positions.first.offset;
- final selectionLength = change.linkedEditGroups.first.length;
+ final fileEdit = change.edits.single;
+ final snippetEdits = toSnippetTextEdits(
+ fileEdit.file,
+ fileEdit,
+ change.linkedEditGroups,
+ lineInfo,
+ selectionOffset: change.selection?.offset,
+ );
- if (selectionOffset < edit.offset ||
- selectionOffset + selectionLength > edit.offset + edit.length) {
- return createPlainWorkspaceEdit(server, change.edits);
- }
+ // Compile the edits into a TextDocumentEdit for this file.
+ final textDocumentEdit = lsp.TextDocumentEdit(
+ textDocument: server.getVersionedDocumentIdentifier(fileEdit.file),
+ edits: snippetEdits
+ .map((e) => Either3<lsp.SnippetTextEdit, lsp.AnnotatedTextEdit,
+ lsp.TextEdit>.t1(e))
+ .toList(),
+ );
- return toWorkspaceEdit(
- server.clientCapabilities!,
- change.edits
- .map((e) => FileEditInformation(
- server.getVersionedDocumentIdentifier(e.file),
- // We should never produce edits for a file with no LineInfo.
- server.getLineInfo(e.file)!,
- e.edits,
- selectionOffsetRelative: selectionOffset - edit.offset,
- selectionLength: selectionLength,
- newFile: e.fileStamp == -1,
- ))
- .toList());
+ // Convert to the union that documentChanges require.
+ final textDocumentEditsAsUnion = Either4<lsp.TextDocumentEdit, lsp.CreateFile,
+ lsp.RenameFile, lsp.DeleteFile>.t1(textDocumentEdit);
+
+ // Convert to the union that documentChanges is.
+ final documentChanges = Either2<
+ List<lsp.TextDocumentEdit>,
+ List<
+ Either4<lsp.TextDocumentEdit, lsp.CreateFile, lsp.RenameFile,
+ lsp.DeleteFile>>>.t2([textDocumentEditsAsUnion]);
+
+ /// Add the textDocumentEdit to a WorkspaceEdit.
+ return lsp.WorkspaceEdit(documentChanges: documentChanges);
}
lsp.CompletionItemKind? declarationKindToCompletionItemKind(
diff --git a/pkg/analysis_server/lib/src/lsp/snippets.dart b/pkg/analysis_server/lib/src/lsp/snippets.dart
index e88dcaa..0c9d5c6 100644
--- a/pkg/analysis_server/lib/src/lsp/snippets.dart
+++ b/pkg/analysis_server/lib/src/lsp/snippets.dart
@@ -85,6 +85,11 @@
// Use the index as an ID to keep all related positions together (so
// the remain "linked").
linkedGroupId: index,
+ // If there is no selection, no tabstops, and only a single edit group
+ // allow it to be the final tabstop.
+ isFinal: selectionOffset == null &&
+ (tabStopOffsetLengthPairs?.isEmpty ?? false) &&
+ editGroups?.length == 1,
),
);
}
diff --git a/pkg/analysis_server/test/lsp/code_actions_assists_test.dart b/pkg/analysis_server/test/lsp/code_actions_assists_test.dart
index cf67637f..68652df 100644
--- a/pkg/analysis_server/test/lsp/code_actions_assists_test.dart
+++ b/pkg/analysis_server/test/lsp/code_actions_assists_test.dart
@@ -249,7 +249,70 @@
);
}
- Future<void> test_snippetTextEdits_supported() async {
+ Future<void> test_snippetTextEdits_multiEditGroup() async {
+ // As test_snippetTextEdits_singleEditGroup, but uses an assist that
+ // produces multiple linked edit groups.
+
+ const content = '''
+ import 'package:flutter/widgets.dart';
+ build() {
+ return Container(
+ child: Ro^w(
+ children: [
+ Text('111'),
+ Text('222'),
+ Container(),
+ ],
+ ),
+ );
+ }
+ ''';
+
+ const expectedContent = r'''
+ import 'package:flutter/widgets.dart';
+ build() {
+ return Container(
+ child: ${1:widget}(
+ ${2:child}: Row(
+ children: [
+ Text('111'),
+ Text('222'),
+ Container(),
+ ],
+ ),
+ ),
+ );
+ }
+ ''';
+
+ newFile(mainFilePath, content: withoutMarkers(content));
+ await initialize(
+ textDocumentCapabilities: withCodeActionKinds(
+ emptyTextDocumentClientCapabilities, [CodeActionKind.Refactor]),
+ workspaceCapabilities:
+ withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
+ experimentalCapabilities: {
+ 'snippetTextEdit': true,
+ },
+ );
+
+ final codeActions = await getCodeActions(mainFileUri.toString(),
+ position: positionFromMarker(content));
+ final assist = findEditAction(
+ codeActions,
+ CodeActionKind('refactor.flutter.wrap.generic'),
+ 'Wrap with widget...')!;
+
+ // Ensure applying the changes will give us the expected content.
+ final edit = assist.edit!;
+ final contents = {
+ mainFilePath: withoutMarkers(content),
+ };
+ applyDocumentChanges(contents, edit.documentChanges!);
+ expect(contents[mainFilePath], equals(expectedContent));
+ }
+
+ Future<void> test_snippetTextEdits_singleEditGroup() async {
// This tests experimental support for including Snippets in TextEdits.
// https://github.com/rust-analyzer/rust-analyzer/blob/b35559a2460e7f0b2b79a7029db0c5d4e0acdb44/docs/dev/lsp-extensions.md#snippet-textedit
//
@@ -273,8 +336,8 @@
''';
// For testing, the snippet will be inserted literally into the text, as
- // this requires some magic on the client. The expected text should therefore
- // contain the snippets in the standard format.
+ // this requires some magic on the client. The expected text should
+ // therefore contain the snippets in the standard format.
const expectedContent = r'''
import 'package:flutter/widgets.dart';
build() {
@@ -426,6 +489,63 @@
);
}
+ Future<void> test_surround_editGroupsAndSelection() async {
+ const content = '''
+void f() {
+ [[print(0);]]
+}
+''';
+
+ const expectedContent = r'''
+void f() {
+ if (${1:condition}) {
+ print(0);
+ }$0
+}
+''';
+
+ newFile(mainFilePath, content: withoutMarkers(content));
+ await initialize(
+ textDocumentCapabilities: withCodeActionKinds(
+ emptyTextDocumentClientCapabilities, [CodeActionKind.Refactor]),
+ workspaceCapabilities:
+ withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
+ experimentalCapabilities: {
+ 'snippetTextEdit': true,
+ },
+ );
+
+ final codeActions = await getCodeActions(mainFileUri.toString(),
+ range: rangeFromMarkers(content));
+ final assist = findEditAction(codeActions,
+ CodeActionKind('refactor.surround.if'), "Surround with 'if'")!;
+
+ // Ensure the edit came back, and using documentChanges.
+ final edit = assist.edit!;
+ expect(edit.documentChanges, isNotNull);
+ expect(edit.changes, isNull);
+
+ // Ensure applying the changes will give us the expected content.
+ final contents = {
+ mainFilePath: withoutMarkers(content),
+ };
+ applyDocumentChanges(contents, edit.documentChanges!);
+ expect(contents[mainFilePath], equals(expectedContent));
+
+ // Also ensure there was a single edit that was correctly marked
+ // as a SnippetTextEdit.
+ final textEdits = _extractTextDocumentEdits(edit.documentChanges!)
+ .expand((tde) => tde.edits)
+ .map((edit) => edit.map(
+ (e) => e,
+ (e) => throw 'Expected SnippetTextEdit, got AnnotatedTextEdit',
+ (e) => throw 'Expected SnippetTextEdit, got TextEdit',
+ ))
+ .toList();
+ expect(textEdits, hasLength(1));
+ expect(textEdits.first.insertTextFormat, equals(InsertTextFormat.Snippet));
+ }
+
List<TextDocumentEdit> _extractTextDocumentEdits(
Either2<
List<TextDocumentEdit>,
diff --git a/pkg/analysis_server/test/lsp/snippets_test.dart b/pkg/analysis_server/test/lsp/snippets_test.dart
index a126c9b..48cefb0 100644
--- a/pkg/analysis_server/test/lsp/snippets_test.dart
+++ b/pkg/analysis_server/test/lsp/snippets_test.dart
@@ -155,7 +155,7 @@
],
);
expect(result, equals(r'''
-var ${1|a,aaa,bbb\${\}\,\|,ccc|} = 1;
+var ${0|a,aaa,bbb\${\}\,\|,ccc|} = 1;
'''));
}
@@ -180,8 +180,8 @@
],
);
expect(result, equals(r'''
-class $1 {
- $1();
+class $0 {
+ $0();
}
'''));
}
@@ -208,8 +208,8 @@
],
);
expect(result, equals(r'''
-class ${1:A} {
- ${1:A}();
+class ${0:A} {
+ ${0:A}();
}
'''));
}
@@ -235,8 +235,8 @@
],
);
expect(result, equals(r'''
-class ${1:A} {
- ${1:A}();
+class ${0:A} {
+ ${0:A}();
}
'''));
}
@@ -264,8 +264,8 @@
],
);
expect(result, equals(r'''
-class ${1:A} {
- ${1:A}();
+class ${0:A} {
+ ${0:A}();
}
'''));
}
