(fix) better import completion locations (#1011)

Add import to module script if trigger in there, same for instance script
#927

Author: dummdidumm

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

@@ -1,3 +1,4 @@
+import ts from 'typescript';
 import {
     CodeAction,
     CodeActionContext,
@@ -10,18 +11,15 @@ import {
 } from 'vscode-languageserver';
 import {
     Document,
-    mapRangeToOriginal,
+    getLineAtPosition,
     isRangeInTag,
-    isInTag,
-    getLineAtPosition
+    mapRangeToOriginal
 } from '../../../lib/documents';
-import { pathToUrl, flatten, isNotNullOrUndefined, modifyLines, getIndent } from '../../../utils';
+import { flatten, getIndent, isNotNullOrUndefined, modifyLines, pathToUrl } from '../../../utils';
 import { CodeActionsProvider } from '../../interfaces';
 import { SnapshotFragment, SvelteSnapshotFragment } from '../DocumentSnapshot';
 import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
 import { convertRange } from '../utils';
-
-import ts from 'typescript';
 import { CompletionsProviderImpl } from './CompletionProvider';
 import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './utils';
 
@@ -189,8 +187,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
                                     fragment,
                                     edit,
                                     true,
-                                    isInTag(range.start, document.scriptInfo) ||
-                                        isInTag(range.start, document.moduleScriptInfo)
+                                    range.start
                                 );
                             }
 

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

@@ -317,8 +317,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
                             fragment,
                             change,
                             isImport,
-                            isInTag(comp.position, document.scriptInfo) ||
-                                isInTag(comp.position, document.moduleScriptInfo)
+                            comp.position
                         )
                     );
                 }
@@ -355,15 +354,15 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         fragment: SvelteSnapshotFragment,
         changes: ts.FileTextChanges,
         isImport: boolean,
-        actionTriggeredInScript: boolean
+        originalTriggerPosition: Position
     ): TextEdit[] {
         return changes.textChanges.map((change) =>
             this.codeActionChangeToTextEdit(
                 doc,
                 fragment,
                 change,
                 isImport,
-                actionTriggeredInScript
+                originalTriggerPosition
             )
         );
     }
@@ -373,9 +372,12 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         fragment: SvelteSnapshotFragment,
         change: ts.TextChange,
         isImport: boolean,
-        actionTriggeredInScript: boolean
+        originalTriggerPosition: Position
     ): TextEdit {
-        change.newText = this.changeComponentImport(change.newText, actionTriggeredInScript);
+        change.newText = this.changeComponentImport(
+            change.newText,
+            isInScript(originalTriggerPosition, doc)
+        );
 
         const scriptTagInfo = fragment.scriptInfo || fragment.moduleScriptInfo;
         if (!scriptTagInfo) {
@@ -410,7 +412,11 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             !isInScript(range.start, fragment)
         ) {
             range = convertRange(doc, {
-                start: scriptTagInfo.start,
+                start: isInTag(originalTriggerPosition, doc.scriptInfo)
+                    ? fragment.scriptInfo?.start || scriptTagInfo.start
+                    : isInTag(originalTriggerPosition, doc.moduleScriptInfo)
+                    ? fragment.moduleScriptInfo?.start || scriptTagInfo.start
+                    : scriptTagInfo.start,
                 length: span.length
             });
         }

packages/language-server/src/plugins/typescript/utils.ts

@@ -7,7 +7,7 @@ import {
     Range,
     SymbolKind
 } from 'vscode-languageserver';
-import { isInTag, mapRangeToOriginal } from '../../lib/documents';
+import { Document, isInTag, mapRangeToOriginal } from '../../lib/documents';
 import { pathToUrl } from '../../utils';
 import { SnapshotFragment, SvelteSnapshotFragment } from './DocumentSnapshot';
 
@@ -298,6 +298,6 @@ export function convertToTextSpan(range: Range, fragment: SnapshotFragment): ts.
     };
 }
 
-export function isInScript(position: Position, fragment: SvelteSnapshotFragment) {
+export function isInScript(position: Position, fragment: SvelteSnapshotFragment | Document) {
     return isInTag(position, fragment.scriptInfo) || isInTag(position, fragment.moduleScriptInfo);
 }

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

@@ -30,7 +30,7 @@ const fileNameToAbsoluteUri = (file: string) => {
     return pathToUrl(join(testFilesDir, file));
 };
 
-describe('CompletionProviderImpl', () => {
+describe.only('CompletionProviderImpl', () => {
     function setup(filename: string) {
         const docManager = new DocumentManager(
             (textDocument) => new Document(textDocument.uri, textDocument.text)
@@ -524,6 +524,54 @@ describe('CompletionProviderImpl', () => {
         );
     });
 
+    it('resolve auto import completion in instance script (instance and module script present)', async () => {
+        const { completionProvider, document } = setup('importcompletions9.svelte');
+
+        const completions = await completionProvider.getCompletions(
+            document,
+            Position.create(5, 7)
+        );
+        document.version++;
+
+        const item = completions?.items.find((item) => item.label === 'onMount');
+        const { additionalTextEdits } = await completionProvider.resolveCompletion(document, item!);
+
+        assert.strictEqual(
+            harmonizeNewLines(additionalTextEdits![0]?.newText),
+            // " instead of ' because VSCode uses " by default when there are no other imports indicating otherwise
+            `${newLine}import { onMount } from "svelte";${newLine}${newLine}`
+        );
+
+        assert.deepEqual(
+            additionalTextEdits![0]?.range,
+            Range.create(Position.create(4, 8), Position.create(4, 8))
+        );
+    });
+
+    it('resolve auto import completion in module script (instance and module script present)', async () => {
+        const { completionProvider, document } = setup('importcompletions9.svelte');
+
+        const completions = await completionProvider.getCompletions(
+            document,
+            Position.create(1, 7)
+        );
+        document.version++;
+
+        const item = completions?.items.find((item) => item.label === 'onMount');
+        const { additionalTextEdits } = await completionProvider.resolveCompletion(document, item!);
+
+        assert.strictEqual(
+            harmonizeNewLines(additionalTextEdits![0]?.newText),
+            // " instead of ' because VSCode uses " by default when there are no other imports indicating otherwise
+            `${newLine}import { onMount } from "svelte";${newLine}${newLine}`
+        );
+
+        assert.deepEqual(
+            additionalTextEdits![0]?.range,
+            Range.create(Position.create(0, 25), Position.create(0, 25))
+        );
+    });
+
     async function openFileToBeImported(
         docManager: DocumentManager,
         completionProvider: CompletionsProviderImpl,
@@ -656,7 +704,7 @@ describe('CompletionProviderImpl', () => {
     });
 
     it('resolve auto completion in correct place when already imported in module script', async () => {
-        const { completionProvider, document } = setup('importCompletion8.svelte');
+        const { completionProvider, document } = setup('importcompletions8.svelte');
 
         const completions = await completionProvider.getCompletions(
             document,

packages/language-server/test/plugins/typescript/testfiles/completions/importCompletion8.svelte renamed to packages/language-server/test/plugins/typescript/testfiles/completions/importcompletions8.svelte

@@ -0,0 +1,7 @@
+<script context="module">
+	onMoun
+</script>
+
+<script>
+	onMoun
+</script>