Add support for CompletionList "applyKind" to control how defaults and per-item commitCharacters/data are combined (#1558)

* Add support for CompletionList "applyKind" to control how defaults and per-item commitCharacters/data are combined

Implements the changes in the LSP spec PR at microsoft/language-server-protocol#2018.

(Also see microsoft/language-server-protocol#1802)

* Update meta model

* Add non-null falsy test

* Change ApplyKind to ints

* Tweaks + typos

---------

Co-authored-by: Dirk Bäumer <dirkb@microsoft.com>

Author: DanTup

client-node-tests/src/converter.test.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { strictEqual, deepEqual, ok } from 'assert';
+import { strictEqual, deepEqual, ok, deepStrictEqual } from 'assert';
 
 import * as proto from 'vscode-languageclient';
 import * as codeConverter from 'vscode-languageclient/$test/common/codeConverter';
@@ -859,6 +859,145 @@ suite('Protocol Converter', () => {
 		ok(result.items[0].insertText instanceof vscode.SnippetString);
 	});
 
+	test('Completion Result - applyKind:default - commitCharacters', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults:  { commitCharacters: ['d'] },
+			items: [{ label: 'item', commitCharacters: ['i'] }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		deepStrictEqual(result.items[0].commitCharacters, ['i']);
+	});
+
+	test('Completion Result - applyKind:replace - commitCharacters', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { commitCharacters: ['1'] },
+			// Set other fields to "merge" to ensure the correct field was used.
+			applyKind: { commitCharacters: proto.ApplyKind.Replace, data: proto.ApplyKind.Merge },
+			items: [{ label: 'item', commitCharacters: ['2'] }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		deepStrictEqual(result.items[0].commitCharacters, ['2']);
+	});
+
+	test('Completion Result - applyKind:replace - commitCharacters - item empty', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { commitCharacters: ['1'] },
+			// Set other fields to "merge" to ensure the correct field was used.
+			applyKind: { commitCharacters: proto.ApplyKind.Replace, data: proto.ApplyKind.Merge },
+			items: [{ label: 'item', commitCharacters: [] }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		deepStrictEqual(result.items[0].commitCharacters, []);
+	});
+
+	test('Completion Result - applyKind:merge - commitCharacters - both supplied with overlaps', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { commitCharacters: ['d', 'b'] },
+			applyKind: { commitCharacters: proto.ApplyKind.Merge },
+			items: [{ label: 'item', commitCharacters: ['b', 'i'] }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		deepStrictEqual(result.items[0].commitCharacters, ['d', 'b', 'i']);
+	});
+
+	test('Completion Result - applyKind:merge - commitCharacters - only default supplied', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { commitCharacters: ['d'] },
+			applyKind: { commitCharacters: proto.ApplyKind.Merge },
+			items: [{ label: 'item' }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		deepStrictEqual(result.items[0].commitCharacters, ['d']);
+	});
+
+	test('Completion Result - applyKind:merge - commitCharacters - only item supplied', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { },
+			applyKind: { commitCharacters: proto.ApplyKind.Merge },
+			items: [{ label: 'item', commitCharacters: ['i'] }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		deepStrictEqual(result.items[0].commitCharacters, ['i']);
+	});
+
+	test('Completion Result - applyKind:default - data', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { data: { 'd': 'd' } },
+			items: [{ label: 'item', data: { 'i': 'i' } }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		const protoResult = await c2p.asCompletionItem(result.items[0]);
+		deepStrictEqual(protoResult.data, {'i': 'i'});
+	});
+
+	test('Completion Result - applyKind:replace - data', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { data: { 'd': 'd' } },
+			// Set other fields to "merge" to ensure the correct field was used.
+			applyKind: { data: proto.ApplyKind.Replace, commitCharacters: proto.ApplyKind.Merge },
+			items: [{ label: 'item', data: { 'i': 'i' } }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		const protoResult = await c2p.asCompletionItem(result.items[0]);
+		deepStrictEqual(protoResult.data, {'i': 'i'});
+	});
+
+	test('Completion Result - applyKind:merge - data - both supplied', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { data: { 'd': 'd' } },
+			applyKind: { data: proto.ApplyKind.Merge },
+			items: [{ label: 'item', data: { 'i': 'i' } }]
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		const protoResult = await c2p.asCompletionItem(result.items[0]);
+		deepStrictEqual(protoResult.data, {'d': 'd', 'i': 'i'});
+	});
+
+	test('Completion Result - applyKind:merge - data - default supplied, item null', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { data: { 'd': 'd' } },
+			applyKind: { data: proto.ApplyKind.Merge },
+			items: [{ label: 'item', data: null }] // null treated like undefined
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		const protoResult = await c2p.asCompletionItem(result.items[0]);
+		deepStrictEqual(protoResult.data, {'d': 'd'}); // gets default
+	});
+
+	test('Completion Result - applyKind:merge - data - both supplied, item has null fields', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { data: { 'd': 'd' } },
+			applyKind: { data: proto.ApplyKind.Merge },
+			items: [{ label: 'item', data: { 'd': null, 'i': 'i'} }] // null treated like undefined
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		const protoResult = await c2p.asCompletionItem(result.items[0]);
+		deepStrictEqual(protoResult.data, {'d': 'd', 'i': 'i'}); // gets default for 'd'
+	});
+
+	test('Completion Result - applyKind:merge - data - both supplied, item has non-null falsy fields', async () => {
+		const completionResult: proto.CompletionList = {
+			isIncomplete: false,
+			itemDefaults: { data: { 'd1': 'd1', 'd2': 'd2' } },
+			applyKind: { data: proto.ApplyKind.Merge },
+			items: [{ label: 'item', data: { 'd1': 0, 'd2': ''} }] // Both falsy, but should be used.
+		};
+		const result = await p2c.asCompletionResult(completionResult);
+		const protoResult = await c2p.asCompletionItem(result.items[0]);
+		deepStrictEqual(protoResult.data, {'d1': 0, 'd2': ''});
+	});
+
 	test('Parameter Information', async () => {
 		const parameterInfo: proto.ParameterInformation = {
 			label: 'label'

client/src/common/completion.ts

@@ -92,7 +92,8 @@ export class CompletionItemFeature extends TextDocumentLanguageFeature<Completio
 		completion.completionList = {
 			itemDefaults: [
 				'commitCharacters', 'editRange', 'insertTextFormat', 'insertTextMode', 'data'
-			]
+			],
+			applyKindSupport: true,
 		};
 	}
 
@@ -153,4 +154,4 @@ export class CompletionItemFeature extends TextDocumentLanguageFeature<Completio
 		};
 		return [Languages.registerCompletionItemProvider(this._client.protocol2CodeConverter.asDocumentSelector(selector), provider, ...triggerCharacters), provider];
 	}
-}
+}

client/src/common/protocolConverter.ts

@@ -515,7 +515,7 @@ export function createConverter(
 		const list = <ls.CompletionList>value;
 		const { defaultRange, commitCharacters } = getCompletionItemDefaults(list, allCommitCharacters);
 		const converted = await async.map(list.items, (item) => {
-			return asCompletionItem(item, commitCharacters, defaultRange, list.itemDefaults?.insertTextMode, list.itemDefaults?.insertTextFormat, list.itemDefaults?.data);
+			return asCompletionItem(item, commitCharacters, list.applyKind?.commitCharacters, defaultRange, list.itemDefaults?.insertTextMode, list.itemDefaults?.insertTextFormat, list.itemDefaults?.data, list.applyKind?.data);
 		}, token);
 		return new code.CompletionList(converted, list.isIncomplete);
 	}
@@ -561,7 +561,16 @@ export function createConverter(
 		return result;
 	}
 
-	function asCompletionItem(item: ls.CompletionItem, defaultCommitCharacters?: string[], defaultRange?: code.Range | InsertReplaceRange,  defaultInsertTextMode?: ls.InsertTextMode, defaultInsertTextFormat?: ls.InsertTextFormat, defaultData?: ls.LSPAny): ProtocolCompletionItem {
+	function asCompletionItem(
+		item: ls.CompletionItem,
+		defaultCommitCharacters?: string[],
+		commitCharactersApplyKind?: ls.ApplyKind,
+		defaultRange?: code.Range | InsertReplaceRange,
+		defaultInsertTextMode?: ls.InsertTextMode,
+		defaultInsertTextFormat?: ls.InsertTextFormat,
+		defaultData?: ls.LSPAny,
+		dataApplyKind?: ls.ApplyKind,
+	): ProtocolCompletionItem {
 		const tags: code.CompletionItemTag[] = asCompletionItemTags(item.tags);
 		const label = asCompletionItemLabel(item);
 		const result = new ProtocolCompletionItem(label);
@@ -587,9 +596,7 @@ export function createConverter(
 		}
 		if (item.sortText) { result.sortText = item.sortText; }
 		if (item.additionalTextEdits) { result.additionalTextEdits = asTextEditsSync(item.additionalTextEdits); }
-		const commitCharacters = item.commitCharacters !== undefined
-			? Is.stringArray(item.commitCharacters) ? item.commitCharacters : undefined
-			: defaultCommitCharacters;
+		const commitCharacters = applyCommitCharacters(item, defaultCommitCharacters, commitCharactersApplyKind);
 		if (commitCharacters) { result.commitCharacters = commitCharacters.slice(); }
 		if (item.command) { result.command = asCommand(item.command); }
 		if (item.deprecated === true || item.deprecated === false) {
@@ -599,7 +606,7 @@ export function createConverter(
 			}
 		}
 		if (item.preselect === true || item.preselect === false) { result.preselect = item.preselect; }
-		const data = item.data ?? defaultData;
+		const data = applyData(item, defaultData, dataApplyKind);
 		if (data !== undefined) { result.data = data; }
 		if (tags.length > 0) {
 			result.tags = tags;
@@ -614,6 +621,50 @@ export function createConverter(
 		return result;
 	}
 
+	function applyCommitCharacters(item: ls.CompletionItem, defaultCommitCharacters: string[] | undefined, applyKind: ls.ApplyKind | undefined): string[] | undefined {
+		if (applyKind === ls.ApplyKind.Merge) {
+			if (!defaultCommitCharacters && !item.commitCharacters) {
+				return undefined;
+			}
+			const set = new Set<string>();
+			if (defaultCommitCharacters) {
+				for (const char of defaultCommitCharacters) {
+					set.add(char);
+				}
+			}
+			if (Is.stringArray(item.commitCharacters)) {
+				for (const char of item.commitCharacters) {
+					set.add(char);
+				}
+			}
+			return Array.from(set);
+		}
+
+		return item.commitCharacters !== undefined
+			? Is.stringArray(item.commitCharacters) ? item.commitCharacters : undefined
+			: defaultCommitCharacters;
+	}
+
+	function applyData(item: ls.CompletionItem, defaultData: any, applyKind: ls.ApplyKind | undefined): string[] | undefined {
+		if (applyKind === ls.ApplyKind.Merge) {
+			const data = {
+				...defaultData,
+			};
+
+			if (item.data) {
+				Object.entries(item.data).forEach(([key, value]) => {
+					if (value !== undefined && value !== null) {
+						data[key] = value;
+					}
+				});
+			}
+
+			return data;
+		}
+
+		return item.data ?? defaultData;
+	}
+
 	function asCompletionItemLabel(item: ls.CompletionItem): code.CompletionItemLabel | string {
 		if (ls.CompletionItemLabelDetails.is(item.labelDetails)) {
 			return {

protocol/metaModel.json

@@ -5168,9 +5168,19 @@
 						"name": "CompletionItemDefaults"
 					},
 					"optional": true,
-					"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value the one from the item is used.\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
+					"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value, the rules for combining these are\ndefined by `applyKinds` (if the client supports it), defaulting to\n\"replace\".\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
 					"since": "3.17.0"
 				},
+				{
+					"name": "applyKind",
+					"type": {
+						"kind": "reference",
+						"name": "CompletionItemApplyKinds"
+					},
+					"optional": true,
+					"documentation": "Specifies how fields from a completion item should be combined with those\nfrom `completionList.itemDefaults`.\n\nIf unspecified, all fields will be treated as \"replace\".\n\nIf a field's value is \"replace\", the value from a completion item (if\nprovided and not `null`) will always be used instead of the value from\n`completionItem.itemDefaults`.\n\nIf a field's value is \"merge\", the values will be merged using the rules\ndefined against each field below.\n\nServers are only allowed to return `applyKind` if the client\nsignals support for this via the `completionList.applyKindSupport`\ncapability.\n\n@since 3.18.0",
+					"since": "3.18.0"
+				},
 				{
 					"name": "items",
 					"type": {
@@ -9195,9 +9205,36 @@
 					"since": "3.17.0"
 				}
 			],
-			"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value the one from the item is used.\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
+			"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value, the rules for combining these are\ndefined by `applyKinds` (if the client supports it), defaulting to\n\"replace\".\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
 			"since": "3.17.0"
 		},
+		{
+			"name": "CompletionItemApplyKinds",
+			"properties": [
+				{
+					"name": "commitCharacters",
+					"type": {
+						"kind": "reference",
+						"name": "ApplyKind"
+					},
+					"optional": true,
+					"documentation": "Specifies whether commitCharacters on a completion will replace or be\nmerged with those in `completionList.itemDefaults.commitCharacters`.\n\nIf \"replace\", the commit characters from the completion item will\nalways be used unless not provided, in which case those from\n`completionList.itemDefaults.commitCharacters` will be used. An\nempty list can be used if a completion item does not have any commit\ncharacters and also should not use those from\n`completionList.itemDefaults.commitCharacters`.\n\nIf \"merge\" the commitCharacters for the completion will be the union\nof all values in both `completionList.itemDefaults.commitCharacters`\nand the completion's own `commitCharacters`.\n\n@since 3.18.0",
+					"since": "3.18.0"
+				},
+				{
+					"name": "data",
+					"type": {
+						"kind": "reference",
+						"name": "ApplyKind"
+					},
+					"optional": true,
+					"documentation": "Specifies whether the `data` field on a completion will replace or\nbe merged with data from `completionList.itemDefaults.data`.\n\nIf \"replace\", the data from the completion item will be used if\nprovided (and not `null`), otherwise\n`completionList.itemDefaults.data` will be used. An empty object can\nbe used if a completion item does not have any data but also should\nnot use the value from `completionList.itemDefaults.data`.\n\nIf \"merge\", a shallow merge will be performed between\n`completionList.itemDefaults.data` and the completion's own data\nusing the following rules:\n\n- If a completion's `data` field is not provided (or `null`), the\n  entire `data` field from `completionList.itemDefaults.data` will be\n  used as-is.\n- If a completion's `data` field is provided, each field will\n  overwrite the field of the same name in\n  `completionList.itemDefaults.data` but no merging of nested fields\n  within that value will occur.\n\n@since 3.18.0",
+					"since": "3.18.0"
+				}
+			],
+			"documentation": "Specifies how fields from a completion item should be combined with those\nfrom `completionList.itemDefaults`.\n\nIf unspecified, all fields will be treated as \"replace\".\n\nIf a field's value is \"replace\", the value from a completion item (if\nprovided and not `null`) will always be used instead of the value from\n`completionItem.itemDefaults`.\n\nIf a field's value is \"merge\", the values will be merged using the rules\ndefined against each field below.\n\nServers are only allowed to return `applyKind` if the client\nsignals support for this via the `completionList.applyKindSupport`\ncapability.\n\n@since 3.18.0",
+			"since": "3.18.0"
+		},
 		{
 			"name": "CompletionOptions",
 			"properties": [
@@ -13560,6 +13597,16 @@
 					"optional": true,
 					"documentation": "The client supports the following itemDefaults on\na completion list.\n\nThe value lists the supported property names of the\n`CompletionList.itemDefaults` object. If omitted\nno properties are supported.\n\n@since 3.17.0",
 					"since": "3.17.0"
+				},
+				{
+					"name": "applyKindSupport",
+					"type": {
+						"kind": "base",
+						"name": "boolean"
+					},
+					"optional": true,
+					"documentation": "Specifies whether the client supports `CompletionList.applyKind` to\nindicate how supported values from `completionList.itemDefaults`\nand `completion` will be combined.\n\nIf a client supports `applyKind` it must support it for all fields\nthat it supports that are listed in `CompletionList.applyKind`. This\nmeans when clients add support for new/future fields in completion\nitems the MUST also support merge for them if those fields are\ndefined in `CompletionList.applyKind`.\n\n@since 3.18.0",
+					"since": "3.18.0"
 				}
 			],
 			"documentation": "The client supports the following `CompletionList` specific\ncapabilities.\n\n@since 3.17.0",
@@ -15314,6 +15361,27 @@
 			],
 			"documentation": "How a completion was triggered"
 		},
+		{
+			"name": "ApplyKind",
+			"type": {
+				"kind": "base",
+				"name": "string"
+			},
+			"values": [
+				{
+					"name": "Replace",
+					"value": "replace",
+					"documentation": "The value from the individual item (if provided and not `null`) will be\nused instead of the default."
+				},
+				{
+					"name": "Merge",
+					"value": "merge",
+					"documentation": "The value from the item will be merged with the default.\n\nThe specific rules for mergeing values are defined against each field\nthat supports merging."
+				}
+			],
+			"documentation": "Defines how values from a set of defaults and an individual item will be\nmerged.\n\n@since 3.18.0",
+			"since": "3.18.0"
+		},
 		{
 			"name": "SignatureHelpTriggerKind",
 			"type": {