[analysis_server] Don't include tabstops in parens for LSP completions with 0 parameters
Fixes https://github.com/Dart-Code/Dart-Code/issues/3873.
Change-Id: I66a5b18dcf0674454cfc820d6cdf0d84e4c4f547
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/238088
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
index 8125d9a..fa6a104 100644
--- a/pkg/analysis_server/lib/src/lsp/mapping.dart
+++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -346,8 +346,9 @@
// we can assume if an item is callable it's probably being used in a context
// that can invoke it.
isInvocation: isCallable,
- defaultArgumentListString: declaration.defaultArgumentListString,
- defaultArgumentListTextRanges: declaration.defaultArgumentListTextRanges,
+ requiredArgumentListString: declaration.defaultArgumentListString,
+ requiredArgumentListTextRanges: declaration.defaultArgumentListTextRanges,
+ hasOptionalParameters: declaration.parameterNames?.isNotEmpty ?? false,
completion: completion,
selectionOffset: 0,
selectionLength: 0,
@@ -1179,8 +1180,9 @@
completeFunctionCalls: completeFunctionCalls,
isCallable: isCallable,
isInvocation: isInvocation,
- defaultArgumentListString: suggestion.defaultArgumentListString,
- defaultArgumentListTextRanges: suggestion.defaultArgumentListTextRanges,
+ requiredArgumentListString: suggestion.defaultArgumentListString,
+ requiredArgumentListTextRanges: suggestion.defaultArgumentListTextRanges,
+ hasOptionalParameters: suggestion.parameterNames?.isNotEmpty ?? false,
completion: suggestion.completion,
selectionOffset: suggestion.selectionOffset,
selectionLength: suggestion.selectionLength,
@@ -1664,8 +1666,9 @@
required bool completeFunctionCalls,
required bool isCallable,
required bool isInvocation,
- required String? defaultArgumentListString,
- required List<int>? defaultArgumentListTextRanges,
+ required String? requiredArgumentListString,
+ required List<int>? requiredArgumentListTextRanges,
+ required bool hasOptionalParameters,
required String completion,
required int selectionOffset,
required int selectionLength,
@@ -1692,13 +1695,16 @@
isInvocation) {
insertTextFormat = lsp.InsertTextFormat.Snippet;
final hasRequiredParameters =
- (defaultArgumentListTextRanges?.length ?? 0) > 0;
+ requiredArgumentListTextRanges?.isNotEmpty ?? false;
final functionCallSuffix =
- hasRequiredParameters && defaultArgumentListString != null
+ hasRequiredParameters && requiredArgumentListString != null
? buildSnippetStringWithTabStops(
- defaultArgumentListString, defaultArgumentListTextRanges)
- // No required params still gets a final tab stop in the parens.
- : SnippetBuilder.finalTabStop;
+ requiredArgumentListString, requiredArgumentListTextRanges)
+ // Optional params still gets a final tab stop in the parens.
+ : hasOptionalParameters
+ ? SnippetBuilder.finalTabStop
+ // And no parameters at all we skip the tabstop in the parens.
+ : '';
insertText =
'${SnippetBuilder.escapeSnippetPlainText(insertText)}($functionCallSuffix)';
} else if (selectionOffset != 0 &&
diff --git a/pkg/analysis_server/test/lsp/completion_dart_test.dart b/pkg/analysis_server/test/lsp/completion_dart_test.dart
index 6ea5f28..66749c1 100644
--- a/pkg/analysis_server/test/lsp/completion_dart_test.dart
+++ b/pkg/analysis_server/test/lsp/completion_dart_test.dart
@@ -33,7 +33,7 @@
with CompletionTestMixin {
Future<void> checkCompleteFunctionCallInsertText(
String content, String completion,
- {required String insertText, InsertTextFormat? insertTextFormat}) async {
+ {required String? insertText, InsertTextFormat? insertTextFormat}) async {
await provideConfig(
() => initialize(
textDocumentCapabilities: withCompletionItemSnippetSupport(
@@ -55,7 +55,9 @@
expect(item.insertText, equals(insertText));
final textEdit = toTextEdit(item.textEdit!);
- expect(textEdit.newText, equals(item.insertText));
+ // newText in the edit will always be set, so if insertText is null we need
+ // fall back to item.label for the expected value.
+ expect(textEdit.newText, equals(item.insertText ?? item.label));
expect(textEdit.range, equals(rangeFromMarkers(content)));
}
@@ -380,7 +382,27 @@
insertText: r'foo(${0:a})',
);
- Future<void> test_completeFunctionCalls_noRequiredParameters() async {
+ Future<void> test_completeFunctionCalls_noParameters() async {
+ final content = '''
+ void myFunction() {}
+
+ void f() {
+ [[myFu^]]
+ }
+ ''';
+
+ await checkCompleteFunctionCallInsertText(
+ content,
+ 'myFunction()',
+ // With no params, we don't put a tab stop inside the parens. This results
+ // in the insertText being the same as the label, which means it will be
+ // set to null so that it falls back without needing to repeat the value.
+ insertText: null,
+ insertTextFormat: InsertTextFormat.Snippet,
+ );
+ }
+
+ Future<void> test_completeFunctionCalls_optionalParameters() async {
final content = '''
void myFunction({int a}) {}
@@ -389,24 +411,13 @@
}
''';
- await provideConfig(
- () => initialize(
- textDocumentCapabilities: withCompletionItemSnippetSupport(
- emptyTextDocumentClientCapabilities),
- workspaceCapabilities:
- withConfigurationSupport(emptyWorkspaceClientCapabilities),
- ),
- {'completeFunctionCalls': true},
+ await checkCompleteFunctionCallInsertText(
+ content,
+ 'myFunction(…)',
+ // With optional params, there should still be parens/tab stop inside.
+ insertText: r'myFunction($0)',
+ insertTextFormat: InsertTextFormat.Snippet,
);
- await openFile(mainFileUri, withoutMarkers(content));
- final res = await getCompletion(mainFileUri, positionFromMarker(content));
- final item = res.singleWhere((c) => c.label == 'myFunction(…)');
- // With no required params, there should still be parens/tab stop inside.
- expect(item.insertTextFormat, equals(InsertTextFormat.Snippet));
- expect(item.insertText, equals(r'myFunction($0)'));
- final textEdit = toTextEdit(item.textEdit!);
- expect(textEdit.newText, equals(item.insertText));
- expect(textEdit.range, equals(rangeFromMarkers(content)));
}
Future<void> test_completeFunctionCalls_show() async {
