(feat) better commit characters handling (#1742)

#1737 - no commit characters in invalid state in template - no commit characters when TS says it's `isNewIdentifierLocation` (matches VS Code behavior)
sveltejs · Nov 29, 2022 · 3a4c838 · 3a4c838
1 parent 8983fa3
commit 3a4c838
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 52 deletions.
diff --git a/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts b/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts
@@ -35,7 +35,6 @@ import { getMarkdownDocumentation } from '../previewer';
 import {
     changeSvelteComponentName,
     convertRange,
-    getCommitCharactersForScriptElement,
     isInScript,
     scriptElementKindToCompletionItemKind
 } from '../utils';
@@ -72,6 +71,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
      * Therefore, only use the characters the typescript compiler treats as valid.
      */
     private readonly validTriggerCharacters = ['.', '"', "'", '`', '/', '@', '<', '#'] as const;
+    private commitCharacters = ['.', ',', ';', '('];
     /**
      * For performance reasons, try to reuse the last completion if possible.
      */
@@ -192,16 +192,20 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             return null;
         }
 
-        let completions =
-            lang.getCompletionsAtPosition(
-                filePath,
-                offset,
-                {
-                    ...userPreferences,
-                    triggerCharacter: validTriggerCharacter
-                },
-                formatSettings
-            )?.entries || [];
+        const response = lang.getCompletionsAtPosition(
+            filePath,
+            offset,
+            {
+                ...userPreferences,
+                triggerCharacter: validTriggerCharacter
+            },
+            formatSettings
+        );
+        const addCommitCharacters =
+            // replicating VS Code behavior https://github.com/microsoft/vscode/blob/main/extensions/typescript-language-features/src/languageFeatures/completions.ts
+            response?.isNewIdentifierLocation !== true &&
+            (!tsDoc.parserError || isInScript(position, tsDoc));
+        let completions = response?.entries || [];
 
         if (!completions.length) {
             completions =
@@ -256,14 +260,16 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
 
         const existingImports = this.getExistingImports(document);
         const wordRangeStartPosition = document.positionAt(wordRange.start);
+        const fileUrl = pathToUrl(tsDoc.filePath); // moved here due to perf reasons
         const completionItems = completions
             .filter(isValidCompletion(document, position, !!tsDoc.parserError))
             .map((comp) =>
                 this.toCompletionItem(
                     tsDoc,
                     comp,
-                    pathToUrl(tsDoc.filePath),
+                    fileUrl,
                     position,
+                    addCommitCharacters,
                     existingImports
                 )
             )
@@ -414,6 +420,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         comp: ts.CompletionEntry,
         uri: string,
         position: Position,
+        addCommitCharacters: boolean,
         existingImports: Set<string>
     ): AppCompletionItem<CompletionEntryWithIdentifier> | null {
         const completionLabelAndInsert = this.getCompletionLabelAndInsert(snapshot, comp);
@@ -436,7 +443,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             label,
             insertText,
             kind: scriptElementKindToCompletionItemKind(comp.kind),
-            commitCharacters: getCommitCharactersForScriptElement(comp.kind),
+            commitCharacters: addCommitCharacters ? this.commitCharacters : undefined,
             // Make sure svelte component takes precedence
             sortText: isSvelteComp ? '-1' : comp.sortText,
             preselect: isSvelteComp ? true : comp.isRecommended,

diff --git a/packages/language-server/src/plugins/typescript/utils.ts b/packages/language-server/src/plugins/typescript/utils.ts
@@ -247,39 +247,6 @@ export function scriptElementKindToCompletionItemKind(
     return CompletionItemKind.Property;
 }
 
-export function getCommitCharactersForScriptElement(
-    kind: ts.ScriptElementKind
-): string[] | undefined {
-    const commitCharacters: string[] = [];
-    switch (kind) {
-        case ts.ScriptElementKind.memberGetAccessorElement:
-        case ts.ScriptElementKind.memberSetAccessorElement:
-        case ts.ScriptElementKind.constructSignatureElement:
-        case ts.ScriptElementKind.callSignatureElement:
-        case ts.ScriptElementKind.indexSignatureElement:
-        case ts.ScriptElementKind.enumElement:
-        case ts.ScriptElementKind.interfaceElement:
-            commitCharacters.push('.');
-            break;
-
-        case ts.ScriptElementKind.moduleElement:
-        case ts.ScriptElementKind.alias:
-        case ts.ScriptElementKind.constElement:
-        case ts.ScriptElementKind.letElement:
-        case ts.ScriptElementKind.variableElement:
-        case ts.ScriptElementKind.localVariableElement:
-        case ts.ScriptElementKind.memberVariableElement:
-        case ts.ScriptElementKind.classElement:
-        case ts.ScriptElementKind.functionElement:
-        case ts.ScriptElementKind.memberFunctionElement:
-            commitCharacters.push('.', ',');
-            commitCharacters.push('(');
-            break;
-    }
-
-    return commitCharacters.length === 0 ? undefined : commitCharacters;
-}
-
 export function mapSeverity(category: ts.DiagnosticCategory): DiagnosticSeverity {
     switch (category) {
         case ts.DiagnosticCategory.Error:

diff --git a/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts
@@ -85,7 +85,7 @@ function test(useNewTransformation: boolean) {
                 insertText: undefined,
                 kind: CompletionItemKind.Method,
                 sortText: '11',
-                commitCharacters: ['.', ',', '('],
+                commitCharacters: ['.', ',', ';', '('],
                 preselect: undefined,
                 textEdit: undefined
             });
@@ -111,7 +111,7 @@ function test(useNewTransformation: boolean) {
                 insertText: undefined,
                 kind: CompletionItemKind.Field,
                 sortText: '11',
-                commitCharacters: ['.', ',', '('],
+                commitCharacters: ['.', ',', ';', '('],
                 preselect: undefined,
                 textEdit: undefined
             });
@@ -512,7 +512,7 @@ function test(useNewTransformation: boolean) {
             const { documentation, detail } = await completionProvider.resolveCompletion(document, {
                 label: 'foo',
                 kind: 6,
-                commitCharacters: ['.', ',', '('],
+                commitCharacters: ['.', ',', ';', '('],
                 data: {
                     name: 'foo',
                     kind: ts.ScriptElementKind.alias,
@@ -1168,7 +1168,7 @@ function test(useNewTransformation: boolean) {
                 insertText: 'import { blubb } from "../definitions";',
                 kind: CompletionItemKind.Function,
                 sortText: '11',
-                commitCharacters: ['.', ',', '('],
+                commitCharacters: undefined,
                 preselect: undefined,
                 textEdit: {
                     newText: '{ blubb } from "../definitions";',
@@ -1226,7 +1226,7 @@ function test(useNewTransformation: boolean) {
                 insertText: '?.toString',
                 kind: CompletionItemKind.Method,
                 sortText: '11',
-                commitCharacters: ['.', ',', '('],
+                commitCharacters: ['.', ',', ';', '('],
                 preselect: undefined,
                 textEdit: {
                     newText: '.toString',
@@ -1268,7 +1268,7 @@ function test(useNewTransformation: boolean) {
                 sortText: '11',
                 preselect: undefined,
                 insertText: undefined,
-                commitCharacters: undefined,
+                commitCharacters: ['.', ',', ';', '('],
                 textEdit: {
                     newText: '@hi',
                     range: {