Skip to content

Commit 2f2f9fc

Browse files
DanTupcommit-bot@chromium.org
authored andcommitted
[Analyzer] Support completeFunctionCalls for LSP SuggestionSet completions
Fixes Dart-Code/Dart-Code#2992. Change-Id: I95ee3735b7f10d9ad06c729c7b5e6aabeee005e0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175251 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent ded707e commit 2f2f9fc

4 files changed

Lines changed: 157 additions & 60 deletions

File tree

‎pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,8 @@ class CompletionHandler
367367
// https://github.com/microsoft/vscode-languageserver-node/issues/673
368368
includeCommitCharacters:
369369
server.clientConfiguration.previewCommitCharacters,
370+
completeFunctionCalls:
371+
server.clientConfiguration.completeFunctionCalls,
370372
));
371373
results.addAll(setResults);
372374
});

‎pkg/analysis_server/lib/src/lsp/handlers/handler_completion_resolve.dart‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,11 @@ class CompletionResolveHandler
7272
);
7373
}
7474

75+
// If filterText is different to the label, it's because label has parens/args
76+
// appended and we should take the basic label. We cannot use insertText as
77+
// it may include snippets, whereas filterText is always just the pure string.
78+
var requestedName = item.filterText ?? item.label;
7579
// The label might be `MyEnum.myValue`, but we import only `MyEnum`.
76-
var requestedName = item.insertText ?? item.label;
7780
if (requestedName.contains('.')) {
7881
requestedName = requestedName.substring(
7982
0,
@@ -173,8 +176,6 @@ class CompletionResolveHandler
173176
insertText: newInsertText,
174177
insertTextFormat: item.insertTextFormat,
175178
textEdit: TextEdit(
176-
// TODO(dantup): If `clientSupportsSnippets == true` then we should map
177-
// `selection` in to a snippet (see how Dart Code does this).
178179
range: toRange(lineInfo, data.rOffset, data.rLength),
179180
newText: newInsertText,
180181
),

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

Lines changed: 114 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import 'package:analyzer/src/generated/source.dart' as server;
3434
import 'package:analyzer/src/services/available_declarations.dart';
3535
import 'package:analyzer/src/services/available_declarations.dart' as dec;
3636
import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
37+
import 'package:analyzer_plugin/utilities/pair.dart';
3738
import 'package:meta/meta.dart';
3839

3940
const diagnosticTagsForErrorCode = <server.ErrorCode, List<lsp.DiagnosticTag>>{
@@ -223,43 +224,67 @@ lsp.CompletionItem declarationToCompletionItem(
223224
int replacementOffset,
224225
int replacementLength, {
225226
@required bool includeCommitCharacters,
227+
@required bool completeFunctionCalls,
226228
}) {
227-
// Build display labels and text to insert. insertText and filterText may
228-
// differ from label (for ex. if the label includes things like (…)). If
229-
// either are missing then label will be used by the client.
230-
String label;
231-
String insertText;
232-
String filterText;
229+
final supportsSnippets =
230+
completionCapabilities?.completionItem?.snippetSupport == true;
231+
232+
String completion;
233233
switch (declaration.kind) {
234234
case DeclarationKind.ENUM_CONSTANT:
235-
label = '${declaration.parent.name}.${declaration.name}';
235+
completion = '${declaration.parent.name}.${declaration.name}';
236236
break;
237237
case DeclarationKind.GETTER:
238238
case DeclarationKind.FIELD:
239-
label = declaration.parent != null &&
239+
completion = declaration.parent != null &&
240240
declaration.parent.name != null &&
241241
declaration.parent.name.isNotEmpty
242242
? '${declaration.parent.name}.${declaration.name}'
243243
: declaration.name;
244244
break;
245245
case DeclarationKind.CONSTRUCTOR:
246-
label = declaration.parent.name;
246+
completion = declaration.parent.name;
247247
if (declaration.name.isNotEmpty) {
248-
label += '.${declaration.name}';
248+
completion += '.${declaration.name}';
249249
}
250-
insertText = label;
251-
filterText = label;
252-
label += declaration.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
253-
break;
254-
case DeclarationKind.FUNCTION:
255-
label = declaration.name;
256-
insertText = label;
257-
filterText = label;
258-
label += declaration.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
259250
break;
260251
default:
261-
label = declaration.name;
252+
completion = declaration.name;
253+
break;
262254
}
255+
// By default, label is the same as the completion text, but may be added to
256+
// later (parens/snippets).
257+
var label = completion;
258+
259+
// isCallable is used to suffix the label with parens so it's clear the item
260+
// is callable.
261+
final declarationKind = declaration.kind;
262+
final isCallable = declarationKind == DeclarationKind.CONSTRUCTOR ||
263+
declarationKind == DeclarationKind.FUNCTION ||
264+
declarationKind == DeclarationKind.METHOD;
265+
266+
if (isCallable) {
267+
label += declaration.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
268+
}
269+
270+
final insertTextInfo = _buildInsertText(
271+
supportsSnippets: supportsSnippets,
272+
includeCommitCharacters: includeCommitCharacters,
273+
completeFunctionCalls: completeFunctionCalls,
274+
isCallable: isCallable,
275+
// For SuggestionSets, we don't have a CompletionKind to check if it's
276+
// an invocation, but since they do not show in show/hide combinators
277+
// we can assume if an item is callable it's probably being used in a context
278+
// that can invoke it.
279+
isInvocation: isCallable,
280+
defaultArgumentListString: declaration.defaultArgumentListString,
281+
defaultArgumentListTextRanges: declaration.defaultArgumentListTextRanges,
282+
completion: completion,
283+
selectionOffset: 0,
284+
selectionLength: 0,
285+
);
286+
final insertText = insertTextInfo.first;
287+
final insertTextFormat = insertTextInfo.last;
263288

264289
final supportsDeprecatedFlag =
265290
completionCapabilities?.completionItem?.deprecatedSupport == true;
@@ -302,12 +327,15 @@ lsp.CompletionItem declarationToCompletionItem(
302327
// 10 -> 999990
303328
// 1 -> 999999
304329
sortText: (1000000 - itemRelevance).toString(),
305-
filterText: filterText != label
306-
? filterText
330+
filterText: completion != label
331+
? completion
307332
: null, // filterText uses label if not set
308333
insertText: insertText != label
309334
? insertText
310335
: null, // insertText uses label if not set
336+
insertTextFormat: insertTextFormat != lsp.InsertTextFormat.PlainText
337+
? insertTextFormat
338+
: null, // Defaults to PlainText if not supplied
311339
// data, used for completionItem/resolve.
312340
data: lsp.DartCompletionItemResolutionInfo(
313341
file: file,
@@ -797,8 +825,6 @@ lsp.CompletionItem toCompletionItem(
797825
// differ from label (for ex. if the label includes things like (…)). If
798826
// either are missing then label will be used by the client.
799827
var label = suggestion.displayText ?? suggestion.completion;
800-
var insertText = suggestion.completion;
801-
var filterText = suggestion.completion;
802828

803829
// Trim any trailing comma from the (displayed) label.
804830
if (label.endsWith(',')) {
@@ -841,37 +867,20 @@ lsp.CompletionItem toCompletionItem(
841867
: suggestionKindToCompletionItemKind(
842868
supportedCompletionItemKinds, suggestion.kind, label);
843869

844-
var insertTextFormat = lsp.InsertTextFormat.PlainText;
845-
846-
// If the client supports snippets, we can support completeFunctionCalls or
847-
// setting a selection.
848-
if (supportsSnippets) {
849-
// completeFunctionCalls should only work if commit characters are disabled
850-
// otherwise the editor may insert parens that we're also inserting.
851-
if (!includeCommitCharacters &&
852-
completeFunctionCalls &&
853-
isCallable &&
854-
isInvocation) {
855-
insertTextFormat = lsp.InsertTextFormat.Snippet;
856-
final hasRequiredParameters =
857-
(suggestion.defaultArgumentListTextRanges?.length ?? 0) > 0;
858-
final functionCallSuffix = hasRequiredParameters
859-
? buildSnippetStringWithTabStops(
860-
suggestion.defaultArgumentListString,
861-
suggestion.defaultArgumentListTextRanges,
862-
)
863-
: '\${0:}'; // No required params still gets a tabstop in the parens.
864-
insertText += '($functionCallSuffix)';
865-
} else if (suggestion.selectionOffset != 0 &&
866-
// We don't need a tabstop if the selection is the end of the string.
867-
suggestion.selectionOffset != suggestion.completion.length) {
868-
insertTextFormat = lsp.InsertTextFormat.Snippet;
869-
insertText = buildSnippetStringWithTabStops(
870-
suggestion.completion,
871-
[suggestion.selectionOffset, suggestion.selectionLength],
872-
);
873-
}
874-
}
870+
final insertTextInfo = _buildInsertText(
871+
supportsSnippets: supportsSnippets,
872+
includeCommitCharacters: includeCommitCharacters,
873+
completeFunctionCalls: completeFunctionCalls,
874+
isCallable: isCallable,
875+
isInvocation: isInvocation,
876+
defaultArgumentListString: suggestion.defaultArgumentListString,
877+
defaultArgumentListTextRanges: suggestion.defaultArgumentListTextRanges,
878+
completion: suggestion.completion,
879+
selectionOffset: suggestion.selectionOffset,
880+
selectionLength: suggestion.selectionLength,
881+
);
882+
final insertText = insertTextInfo.first;
883+
final insertTextFormat = insertTextInfo.last;
875884

876885
// Because we potentially send thousands of these items, we should minimise
877886
// the generated JSON as much as possible - for example using nulls in place
@@ -899,8 +908,8 @@ lsp.CompletionItem toCompletionItem(
899908
// 10 -> 999990
900909
// 1 -> 999999
901910
sortText: (1000000 - suggestion.relevance).toString(),
902-
filterText: filterText != label
903-
? filterText
911+
filterText: suggestion.completion != label
912+
? suggestion.completion
904913
: null, // filterText uses label if not set
905914
insertText: insertText != label
906915
? insertText
@@ -1269,3 +1278,51 @@ lsp.MarkupContent _asMarkup(
12691278

12701279
return lsp.MarkupContent(kind: format, value: content);
12711280
}
1281+
1282+
Pair<String, lsp.InsertTextFormat> _buildInsertText({
1283+
@required bool supportsSnippets,
1284+
@required bool includeCommitCharacters,
1285+
@required bool completeFunctionCalls,
1286+
@required bool isCallable,
1287+
@required bool isInvocation,
1288+
@required String defaultArgumentListString,
1289+
@required List<int> defaultArgumentListTextRanges,
1290+
@required String completion,
1291+
@required int selectionOffset,
1292+
@required int selectionLength,
1293+
}) {
1294+
var insertText = completion;
1295+
var insertTextFormat = lsp.InsertTextFormat.PlainText;
1296+
1297+
// If the client supports snippets, we can support completeFunctionCalls or
1298+
// setting a selection.
1299+
if (supportsSnippets) {
1300+
// completeFunctionCalls should only work if commit characters are disabled
1301+
// otherwise the editor may insert parens that we're also inserting.
1302+
if (!includeCommitCharacters &&
1303+
completeFunctionCalls &&
1304+
isCallable &&
1305+
isInvocation) {
1306+
insertTextFormat = lsp.InsertTextFormat.Snippet;
1307+
final hasRequiredParameters =
1308+
(defaultArgumentListTextRanges?.length ?? 0) > 0;
1309+
final functionCallSuffix = hasRequiredParameters
1310+
? buildSnippetStringWithTabStops(
1311+
defaultArgumentListString,
1312+
defaultArgumentListTextRanges,
1313+
)
1314+
: '\${0:}'; // No required params still gets a tabstop in the parens.
1315+
insertText += '($functionCallSuffix)';
1316+
} else if (selectionOffset != 0 &&
1317+
// We don't need a tabstop if the selection is the end of the string.
1318+
selectionOffset != completion.length) {
1319+
insertTextFormat = lsp.InsertTextFormat.Snippet;
1320+
insertText = buildSnippetStringWithTabStops(
1321+
completion,
1322+
[selectionOffset, selectionLength],
1323+
);
1324+
}
1325+
}
1326+
1327+
return Pair(insertText, insertTextFormat);
1328+
}

‎pkg/analysis_server/test/lsp/completion_dart_test.dart‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,43 @@ main() {
177177
expect(item.textEdit.newText, equals(item.insertText));
178178
}
179179

180+
Future<void> test_completeFunctionCalls_suggestionSets() async {
181+
final content = '''
182+
main() {
183+
[[pri]]^
184+
}
185+
''';
186+
187+
final initialAnalysis = waitForAnalysisComplete();
188+
await provideConfig(
189+
() => initialize(
190+
textDocumentCapabilities: withCompletionItemSnippetSupport(
191+
emptyTextDocumentClientCapabilities),
192+
workspaceCapabilities: withConfigurationSupport(
193+
withApplyEditSupport(emptyWorkspaceClientCapabilities)),
194+
),
195+
{'completeFunctionCalls': true},
196+
);
197+
await openFile(mainFileUri, withoutMarkers(content));
198+
await initialAnalysis;
199+
final res = await getCompletion(mainFileUri, positionFromMarker(content));
200+
final item = res.singleWhere((c) => c.label == 'print(…)');
201+
// Ensure the snippet comes through in the expected format with the expected
202+
// placeholders.
203+
expect(item.insertTextFormat, equals(InsertTextFormat.Snippet));
204+
expect(item.insertText, equals(r'print(${0:object})'));
205+
expect(item.textEdit, isNull);
206+
207+
// Ensure the item can be resolved and gets a proper TextEdit.
208+
final resolved = await resolveCompletion(item);
209+
expect(resolved.textEdit, isNotNull);
210+
expect(resolved.textEdit.newText, equals(item.insertText));
211+
expect(
212+
resolved.textEdit.range,
213+
equals(rangeFromMarkers(content)),
214+
);
215+
}
216+
180217
Future<void> test_completionKinds_default() async {
181218
newFile(join(projectFolderPath, 'file.dart'));
182219
newFolder(join(projectFolderPath, 'folder'));

0 commit comments

Comments
 (0)