(fix) filter out component import if already one present (#466)

#462
Drawback: Also will mark commented out imports as already imported, so there won't be svelte component completions in this case - but this seems like a bigger edge case than having double imports when doing barrel exports

Author: dummdidumm

packages/language-server/src/plugins/typescript/features/CompletionProvider.ts

@@ -17,7 +17,7 @@ import {
     mapRangeToOriginal,
     getNodeIfIsInComponentStartTag,
 } from '../../../lib/documents';
-import { isNotNullOrUndefined, pathToUrl } from '../../../utils';
+import { isNotNullOrUndefined, pathToUrl, getRegExpMatches, flatten } from '../../../utils';
 import { AppCompletionItem, AppCompletionList, CompletionsProvider } from '../../interfaces';
 import { SvelteSnapshotFragment, SvelteDocumentSnapshot } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
@@ -102,9 +102,16 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             return tsDoc.parserError ? CompletionList.create([], true) : null;
         }
 
+        const existingImports = this.getExistingImports(document);
         const completionItems = completions
             .map((comp) =>
-                this.toCompletionItem(fragment, comp, pathToUrl(tsDoc.filePath), position),
+                this.toCompletionItem(
+                    fragment,
+                    comp,
+                    pathToUrl(tsDoc.filePath),
+                    position,
+                    existingImports,
+                ),
             )
             .filter(isNotNullOrUndefined)
             .map((comp) => mapCompletionItemToOriginal(fragment, comp))
@@ -113,6 +120,14 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         return CompletionList.create(completionItems, !!tsDoc.parserError);
     }
 
+    private getExistingImports(document: Document) {
+        const rawImports = getRegExpMatches(scriptImportRegex, document.getText()).map((match) =>
+            (match[1] ?? match[2]).split(','),
+        );
+        const tidiedImports = flatten(rawImports).map((match) => match.trim());
+        return new Set(tidiedImports);
+    }
+
     private getEventCompletions(
         lang: ts.LanguageService,
         doc: Document,
@@ -182,13 +197,21 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         comp: ts.CompletionEntry,
         uri: string,
         position: Position,
+        existingImports: Set<string>,
     ): AppCompletionItem<CompletionEntryWithIdentifer> | null {
         const completionLabelAndInsert = this.getCompletionLabelAndInsert(fragment, comp);
         if (!completionLabelAndInsert) {
             return null;
         }
 
         const { label, insertText, isSvelteComp } = completionLabelAndInsert;
+        // TS may suggest another Svelte component even if there already exists an import
+        // with the same name, because under the hood every Svelte component is postfixed
+        // with `__SvelteComponent`. In this case, filter out this completion by returning null.
+        if (isSvelteComp && existingImports.has(label)) {
+            return null;
+        }
+
         return {
             label,
             insertText,
@@ -418,3 +441,8 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
 }
 
 const beginOfDocumentRange = Range.create(Position.create(0, 0), Position.create(0, 0));
+
+// `import {...} from '..'` or `import ... from '..'`
+// Note: Does not take into account if import is within a comment.
+// eslint-disable-next-line max-len
+const scriptImportRegex = /\bimport\s+{([^}]*?)}\s+?from\s+['"`].+?['"`]|\bimport\s+(\w+?)\s+from\s+['"`].+?['"`]/g;

packages/language-server/src/utils.ts

@@ -95,3 +95,15 @@ export function regexLastIndexOf(text: string, regex: RegExp, endPos?: number) {
     }
     return lastIndexOf;
 }
+
+/**
+ * Get all matches of a regexp.
+ */
+export function getRegExpMatches(regex: RegExp, str: string) {
+    const matches: RegExpExecArray[] = [];
+    let match: RegExpExecArray | null;
+    while ((match = regex.exec(str))) {
+        matches.push(match);
+    }
+    return matches;
+}

packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts

@@ -358,8 +358,9 @@ describe('CompletionProviderImpl', () => {
     async function openFileToBeImported(
         docManager: DocumentManager,
         completionProvider: CompletionsProviderImpl,
+        name = 'imported-file.svelte',
     ) {
-        const filePath = join(testFilesDir, 'imported-file.svelte');
+        const filePath = join(testFilesDir, name);
         const hoverinfoDoc = docManager.openDocument(<any>{
             uri: pathToUrl(filePath),
             text: ts.sys.readFile(filePath) || '',
@@ -458,6 +459,32 @@ describe('CompletionProviderImpl', () => {
 
         assert.strictEqual(additionalTextEdits, undefined);
     });
+
+    it('doesnt suggest svelte auto import when already other import with same name present', async () => {
+        const { completionProvider, document, docManager } = setup(
+            'importcompletions-2nd-import.svelte',
+        );
+        // make sure that the ts language service does know about the imported-file file
+        await openFileToBeImported(docManager, completionProvider, 'ScndImport.svelte');
+
+        const completions = await completionProvider.getCompletions(
+            document,
+            Position.create(2, 13),
+        );
+        document.version++;
+
+        const items = completions?.items.filter((item) => item.label === 'ScndImport');
+        assert.equal(items?.length, 1);
+
+        const item = items?.[0];
+        assert.equal(item?.additionalTextEdits, undefined);
+        assert.equal(item?.detail, undefined);
+        assert.equal(item?.kind, CompletionItemKind.Variable);
+
+        const { additionalTextEdits } = await completionProvider.resolveCompletion(document, item!);
+
+        assert.strictEqual(additionalTextEdits, undefined);
+    });
 });
 
 function harmonizeNewLines(input?: string) {

packages/language-server/test/plugins/typescript/testfiles/completions/importcompletions-2nd-import.svelte

@@ -0,0 +1,4 @@
+<script lang="ts">
+    import { ScndImport } from "./to-import";
+    ScndImpor
+</script>

packages/language-server/test/plugins/typescript/testfiles/completions/to-import.ts

@@ -0,0 +1 @@
+export class ScndImport {}