[Analyzer] Add an LSP setting for including parameters in completions
Fixes https://github.com/Dart-Code/Dart-Code/issues/2823.
Change-Id: I477aba5f3563c01a85bb867a1e01f294eed3636e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171585
Commit-Queue: Danny Tuppeny <danny@tuppeny.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/client_configuration.dart b/pkg/analysis_server/lib/src/lsp/client_configuration.dart
index 55bfaf5..8af5a85 100644
--- a/pkg/analysis_server/lib/src/lsp/client_configuration.dart
+++ b/pkg/analysis_server/lib/src/lsp/client_configuration.dart
@@ -20,6 +20,7 @@
}
}
+ bool get completeFunctionCalls => _settings['completeFunctionCalls'] ?? false;
bool get enableSdkFormatter => _settings['enableSdkFormatter'] ?? true;
int get lineLength => _settings['lineLength'];
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
index 430489a..e9d0fcd 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart
@@ -239,6 +239,8 @@
// https://github.com/microsoft/vscode-languageserver-node/issues/673
includeCommitCharacters:
server.clientConfiguration.previewCommitCharacters,
+ completeFunctionCalls:
+ server.clientConfiguration.completeFunctionCalls,
),
)
.toList();
@@ -372,6 +374,7 @@
// not assume that the Dart ones would be correct for all of their
// completions.
includeCommitCharacters: false,
+ completeFunctionCalls: false,
),
);
});
diff --git a/pkg/analysis_server/lib/src/lsp/mapping.dart b/pkg/analysis_server/lib/src/lsp/mapping.dart
index 039b503..e425b2d 100644
--- a/pkg/analysis_server/lib/src/lsp/mapping.dart
+++ b/pkg/analysis_server/lib/src/lsp/mapping.dart
@@ -62,13 +62,14 @@
_asMarkup(preferredFormats, content));
}
-/// Builds an LSP snippet string that uses a $1 tabstop to set the selected text
-/// after insertion.
-String buildSnippetStringWithSelection(
+/// Builds an LSP snippet string with supplied ranges as tabstops.
+String buildSnippetStringWithTabStops(
String text,
- int selectionOffset,
- int selectionLength,
+ List<int> offsetLengthPairs,
) {
+ text ??= '';
+ offsetLengthPairs ??= const [];
+
String escape(String input) => input.replaceAllMapped(
RegExp(r'[$}\\]'), // Replace any of $ } \
(c) => '\\${c[0]}', // Prefix with a backslash
@@ -77,17 +78,29 @@
// https://microsoft.github.io/language-server-protocol/specifications/specification-3-14/#snippet-syntax
//
// $1, $2, etc. are used for tab stops and ${1:foo} inserts a placeholder of foo.
- // Since we only need to support a single tab stop, our string is constructed of three parts:
- // - Anything before the selection
- // - The selection (which may or may not include text, depending on selectionLength)
- // - Anything after the selection
- final prefix = escape(text.substring(0, selectionOffset));
- final selectionText = escape(
- text.substring(selectionOffset, selectionOffset + selectionLength));
- final selection = '\${1:$selectionText}';
- final suffix = escape(text.substring(selectionOffset + selectionLength));
- return '$prefix$selection$suffix';
+ final output = [];
+ var offset = 0;
+ var tabStopNumber = 1;
+ for (var i = 0; i < offsetLengthPairs.length; i += 2) {
+ final pairOffset = offsetLengthPairs[i];
+ final pairLength = offsetLengthPairs[i + 1];
+
+ // Add any text that came before this tabstop to the result.
+ output.add(escape(text.substring(offset, pairOffset)));
+
+ // Add this tabstop
+ final tabStopText =
+ escape(text.substring(pairOffset, pairOffset + pairLength));
+ output.add('\${${tabStopNumber++}:$tabStopText}');
+
+ offset = pairOffset + pairLength;
+ }
+
+ // Add any remaining text that was after the last tabstop.
+ output.add(escape(text.substring(offset)));
+
+ return output.join('');
}
/// Note: This code will fetch the version of each document being modified so
@@ -769,6 +782,7 @@
int replacementOffset,
int replacementLength, {
@required bool includeCommitCharacters,
+ @required bool completeFunctionCalls,
}) {
// Build display labels and text to insert. insertText and filterText may
// differ from label (for ex. if the label includes things like (…)). If
@@ -782,14 +796,24 @@
label = label.substring(0, label.length - 1);
}
- if (suggestion.displayText == null) {
- switch (suggestion.element?.kind) {
- case server.ElementKind.CONSTRUCTOR:
- case server.ElementKind.FUNCTION:
- case server.ElementKind.METHOD:
- label += suggestion.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
- break;
- }
+ // isCallable is used to suffix the label with parens so it's clear the item
+ // is callable.
+ //
+ // isInvocation means the location at which it's used is an invocation (and
+ // therefore it is appropriate to include the parens/parameters in the
+ // inserted text).
+ //
+ // In the case of show combinators, the parens will still be shown to indicate
+ // functions but they should not be included in the completions.
+ final elementKind = suggestion.element?.kind;
+ final isCallable = elementKind == server.ElementKind.CONSTRUCTOR ||
+ elementKind == server.ElementKind.FUNCTION ||
+ elementKind == server.ElementKind.METHOD;
+ final isInvocation =
+ suggestion.kind == server.CompletionSuggestionKind.INVOCATION;
+
+ if (suggestion.displayText == null && isCallable) {
+ label += suggestion.parameterNames?.isNotEmpty ?? false ? '(…)' : '()';
}
final supportsDeprecatedFlag =
@@ -809,13 +833,33 @@
supportedCompletionItemKinds, suggestion.kind, label);
var insertTextFormat = lsp.InsertTextFormat.PlainText;
- if (supportsSnippets && suggestion.selectionOffset != 0) {
- insertTextFormat = lsp.InsertTextFormat.Snippet;
- insertText = buildSnippetStringWithSelection(
- suggestion.completion,
- suggestion.selectionOffset,
- suggestion.selectionLength,
- );
+
+ // If the client supports snippets, we can support completeFunctionCalls or
+ // setting a selection.
+ if (supportsSnippets) {
+ // completeFunctionCalls should only work if commit characters are disabled
+ // otherwise the editor may insert parens that we're also inserting.
+ if (!includeCommitCharacters &&
+ completeFunctionCalls &&
+ isCallable &&
+ isInvocation) {
+ insertTextFormat = lsp.InsertTextFormat.Snippet;
+ final hasRequiredParameters =
+ (suggestion.defaultArgumentListTextRanges?.length ?? 0) > 0;
+ final functionCallSuffix = hasRequiredParameters
+ ? buildSnippetStringWithTabStops(
+ suggestion.defaultArgumentListString,
+ suggestion.defaultArgumentListTextRanges,
+ )
+ : '\${1:}'; // No required params still gets a tabstop in the parens.
+ insertText += '($functionCallSuffix)';
+ } else if (suggestion.selectionOffset != 0) {
+ insertTextFormat = lsp.InsertTextFormat.Snippet;
+ insertText = buildSnippetStringWithTabStops(
+ suggestion.completion,
+ [suggestion.selectionOffset, suggestion.selectionLength],
+ );
+ }
}
// Because we potentially send thousands of these items, we should minimise
diff --git a/pkg/analysis_server/test/lsp/completion_test.dart b/pkg/analysis_server/test/lsp/completion_test.dart
index cdecc6f..94ff72b 100644
--- a/pkg/analysis_server/test/lsp/completion_test.dart
+++ b/pkg/analysis_server/test/lsp/completion_test.dart
@@ -15,6 +15,7 @@
void main() {
defineReflectiveSuite(() {
defineReflectiveTests(CompletionTest);
+ defineReflectiveTests(CompletionTestWithNullSafetyTest);
});
}
@@ -89,6 +90,93 @@
expect(options.allCommitCharacters, equals(dartCompletionCommitCharacters));
}
+ Future<void> test_completeFunctionCalls() async {
+ final content = '''
+ void myFunction(String a, int b, {String c}) {}
+
+ main() {
+ [[myFu^]]
+ }
+ ''';
+
+ await provideConfig(
+ () => initialize(
+ textDocumentCapabilities: withCompletionItemSnippetSupport(
+ emptyTextDocumentClientCapabilities),
+ workspaceCapabilities:
+ withConfigurationSupport(emptyWorkspaceClientCapabilities),
+ ),
+ {'completeFunctionCalls': true},
+ );
+ await openFile(mainFileUri, withoutMarkers(content));
+ final res = await getCompletion(mainFileUri, positionFromMarker(content));
+ final item = res.singleWhere((c) => c.label == 'myFunction(…)');
+ // Ensure the snippet comes through in the expected format with the expected
+ // placeholders.
+ expect(item.insertTextFormat, equals(InsertTextFormat.Snippet));
+ expect(item.insertText, equals(r'myFunction(${1:a}, ${2:b})'));
+ expect(item.textEdit.newText, equals(item.insertText));
+ expect(
+ item.textEdit.range,
+ equals(rangeFromMarkers(content)),
+ );
+ }
+
+ Future<void> test_completeFunctionCalls_noRequiredParameters() async {
+ final content = '''
+ void myFunction({int a}) {}
+
+ main() {
+ [[myFu^]]
+ }
+ ''';
+
+ await provideConfig(
+ () => initialize(
+ textDocumentCapabilities: withCompletionItemSnippetSupport(
+ emptyTextDocumentClientCapabilities),
+ workspaceCapabilities:
+ withConfigurationSupport(emptyWorkspaceClientCapabilities),
+ ),
+ {'completeFunctionCalls': true},
+ );
+ 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 and a tabstop inside.
+ expect(item.insertTextFormat, equals(InsertTextFormat.Snippet));
+ expect(item.insertText, equals(r'myFunction(${1:})'));
+ expect(item.textEdit.newText, equals(item.insertText));
+ expect(
+ item.textEdit.range,
+ equals(rangeFromMarkers(content)),
+ );
+ }
+
+ Future<void> test_completeFunctionCalls_show() async {
+ final content = '''
+ import 'dart:math' show mi^
+ ''';
+
+ await provideConfig(
+ () => initialize(
+ textDocumentCapabilities: withCompletionItemSnippetSupport(
+ emptyTextDocumentClientCapabilities),
+ workspaceCapabilities:
+ withConfigurationSupport(emptyWorkspaceClientCapabilities),
+ ),
+ {'completeFunctionCalls': true},
+ );
+ await openFile(mainFileUri, withoutMarkers(content));
+ final res = await getCompletion(mainFileUri, positionFromMarker(content));
+ final item = res.singleWhere((c) => c.label == 'min(…)');
+ // Ensure the snippet does not include the parens/args as it doesn't
+ // make sense in the show clause. There will still be a trailing tabstop.
+ expect(item.insertTextFormat, equals(InsertTextFormat.Snippet));
+ expect(item.insertText, equals(r'min${1:}'));
+ expect(item.textEdit.newText, equals(item.insertText));
+ }
+
Future<void> test_completionKinds_default() async {
newFile(join(projectFolderPath, 'file.dart'));
newFolder(join(projectFolderPath, 'folder'));
@@ -1237,3 +1325,44 @@
expect(updated, contains('a.abcdefghij'));
}
}
+
+@reflectiveTest
+class CompletionTestWithNullSafetyTest extends AbstractLspAnalysisServerTest {
+ @override
+ String get testPackageLanguageVersion => latestLanguageVersion;
+
+ @failingTest
+ Future<void> test_completeFunctionCalls_requiredNamed() async {
+ // TODO(dantup): Find out how we can tell this parameter is required
+ // (in the completion mapping).
+ final content = '''
+ void myFunction(String a, int b, {required String c, String d = ''}) {}
+
+ main() {
+ [[myFu^]]
+ }
+ ''';
+
+ await provideConfig(
+ () => initialize(
+ textDocumentCapabilities: withCompletionItemSnippetSupport(
+ emptyTextDocumentClientCapabilities),
+ workspaceCapabilities:
+ withConfigurationSupport(emptyWorkspaceClientCapabilities),
+ ),
+ {'completeFunctionCalls': true},
+ );
+ await openFile(mainFileUri, withoutMarkers(content));
+ final res = await getCompletion(mainFileUri, positionFromMarker(content));
+ final item = res.singleWhere((c) => c.label == 'myFunction(…)');
+ // Ensure the snippet comes through in the expected format with the expected
+ // placeholders.
+ expect(item.insertTextFormat, equals(InsertTextFormat.Snippet));
+ expect(item.insertText, equals(r'myFunction(${1:a}, ${2:b}, ${2:c})'));
+ expect(item.textEdit.newText, equals(item.insertText));
+ expect(
+ item.textEdit.range,
+ equals(rangeFromMarkers(content)),
+ );
+ }
+}
diff --git a/pkg/analysis_server/test/lsp/mapping_test.dart b/pkg/analysis_server/test/lsp/mapping_test.dart
index 2ae7ac1..72bf3f6 100644
--- a/pkg/analysis_server/test/lsp/mapping_test.dart
+++ b/pkg/analysis_server/test/lsp/mapping_test.dart
@@ -74,18 +74,35 @@
expect(result, isNull);
}
- Future<void> test_selectionsInSnippets_empty() async {
- var result = lsp.buildSnippetStringWithSelection('teststring', 4, 0);
- expect(result, equals(r'test${1:}string'));
+ Future<void> test_tabStopsInSnippets_contains() async {
+ var result = lsp.buildSnippetStringWithTabStops('a, b, c', [3, 1]);
+ expect(result, equals(r'a, ${1:b}, c'));
}
- Future<void> test_selectionsInSnippets_escaping() async {
- var result = lsp.buildSnippetStringWithSelection(r'te$tstri}ng', 4, 3);
- expect(result, equals(r'te\$t${1:str}i\}ng'));
+ Future<void> test_tabStopsInSnippets_empty() async {
+ var result = lsp.buildSnippetStringWithTabStops('a, b', []);
+ expect(result, equals(r'a, b'));
}
- Future<void> test_selectionsInSnippets_selection() async {
- var result = lsp.buildSnippetStringWithSelection('teststring', 4, 3);
- expect(result, equals(r'test${1:str}ing'));
+ Future<void> test_tabStopsInSnippets_endsWith() async {
+ var result = lsp.buildSnippetStringWithTabStops('a, b', [3, 1]);
+ expect(result, equals(r'a, ${1:b}'));
+ }
+
+ Future<void> test_tabStopsInSnippets_escape() async {
+ var result = lsp.buildSnippetStringWithTabStops(
+ r'te$tstri}ng, te$tstri}ng, te$tstri}ng', [13, 11]);
+ expect(result, equals(r'te\$tstri\}ng, ${1:te\$tstri\}ng}, te\$tstri\}ng'));
+ }
+
+ Future<void> test_tabStopsInSnippets_multiple() async {
+ var result =
+ lsp.buildSnippetStringWithTabStops('a, b, c', [0, 1, 3, 1, 6, 1]);
+ expect(result, equals(r'${1:a}, ${2:b}, ${3:c}'));
+ }
+
+ Future<void> test_tabStopsInSnippets_startsWith() async {
+ var result = lsp.buildSnippetStringWithTabStops('a, b', [0, 1]);
+ expect(result, equals(r'${1:a}, b'));
}
}
diff --git a/pkg/analysis_server/tool/lsp_spec/README.md b/pkg/analysis_server/tool/lsp_spec/README.md
index 051abb3..e95c70a 100644
--- a/pkg/analysis_server/tool/lsp_spec/README.md
+++ b/pkg/analysis_server/tool/lsp_spec/README.md
@@ -31,6 +31,7 @@
- `dart.enableSdkFormatter`: When set to `false`, prevents registration (or unregisters) the SDK formatter. When set to `true` or not supplied, will register/reregister the SDK formatter.
- `dart.lineLength`: The number of characters the formatter should wrap code at. If unspecified, code will be wrapped at `80` characters.
+- `dart.completeFunctionCalls`: Completes functions/methods with their required parameters.
## Method Status
