Skip to content

Commit d84b178

Browse files
jasonlyu123jasonlyu123
authored
fix: prevent organize imports from removing snippet (#2809)
* wip * test, fix leftover indent
1 parent 23db5a4 commit d84b178

File tree

6 files changed

+277
-21
lines changed

6 files changed

+277
-21
lines changed

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

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
flatten,
3030
getIndent,
3131
isNotNullOrUndefined,
32+
isPositionEqual,
3233
memoize,
3334
modifyLines,
3435
normalizePath,
@@ -42,6 +43,7 @@ import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
4243
import { LanguageServiceContainer } from '../service';
4344
import {
4445
changeSvelteComponentName,
46+
cloneRange,
4547
convertRange,
4648
isInScript,
4749
toGeneratedSvelteComponentName
@@ -469,6 +471,10 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
469471
})
470472
);
471473

474+
for (const change of documentChanges) {
475+
this.checkIndentLeftover(change, document);
476+
}
477+
472478
return [
473479
CodeAction.create(
474480
skipDestructiveCodeActions ? 'Sort Imports' : 'Organize Imports',
@@ -521,21 +527,20 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
521527
snapshot: DocumentSnapshot,
522528
range: Range
523529
) {
530+
if (!(snapshot instanceof SvelteDocumentSnapshot)) {
531+
return range;
532+
}
524533
// Handle svelte2tsx wrong import mapping:
525534
// The character after the last import maps to the start of the script
526535
// TODO find a way to fix this in svelte2tsx and then remove this
527536
if (
528537
(range.end.line === 0 && range.end.character === 1) ||
529-
range.end.line < range.start.line
538+
range.end.line < range.start.line ||
539+
(isInScript(range.start, snapshot) && !isInScript(range.end, snapshot))
530540
) {
531541
edit.span.length -= 1;
532542
range = mapRangeToOriginal(snapshot, convertRange(snapshot, edit.span));
533543

534-
if (!(snapshot instanceof SvelteDocumentSnapshot)) {
535-
range.end.character += 1;
536-
return range;
537-
}
538-
539544
const line = getLineAtPosition(range.end, snapshot.getOriginalText());
540545
// remove-import code action will removes the
541546
// line break generated by svelte2tsx,
@@ -554,6 +559,67 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
554559
return range;
555560
}
556561

562+
private checkIndentLeftover(change: TextDocumentEdit, document: Document) {
563+
if (!change.edits.length) {
564+
return;
565+
}
566+
const orderedByStart = change.edits.sort((a, b) => {
567+
if (a.range.start.line !== b.range.start.line) {
568+
return a.range.start.line - b.range.start.line;
569+
}
570+
return a.range.start.character - b.range.start.character;
571+
});
572+
573+
let current: TextEdit | undefined;
574+
let groups: TextEdit[] = [];
575+
for (let i = 0; i < orderedByStart.length; i++) {
576+
const edit = orderedByStart[i];
577+
if (!current) {
578+
current = { range: cloneRange(edit.range), newText: edit.newText };
579+
continue;
580+
}
581+
if (isPositionEqual(current.range.end, edit.range.start)) {
582+
current.range.end = edit.range.end;
583+
current.newText += edit.newText;
584+
} else {
585+
groups.push(current);
586+
current = { range: cloneRange(edit.range), newText: edit.newText };
587+
}
588+
}
589+
if (current) {
590+
groups.push(current);
591+
}
592+
593+
for (const edit of groups) {
594+
if (edit.newText) {
595+
continue;
596+
}
597+
const range = edit.range;
598+
const lineContentBeforeRemove = document.getText({
599+
start: { line: range.start.line, character: 0 },
600+
end: range.start
601+
});
602+
603+
const onlyIndentLeft = !lineContentBeforeRemove.trim();
604+
if (!onlyIndentLeft) {
605+
continue;
606+
}
607+
608+
const lineContentAfterRemove = document.getText({
609+
start: range.end,
610+
end: { line: range.end.line, character: Number.MAX_VALUE }
611+
});
612+
const emptyAfterRemove =
613+
!lineContentAfterRemove.trim() || lineContentAfterRemove.startsWith('</script>');
614+
if (emptyAfterRemove) {
615+
change.edits.push({
616+
range: { start: { line: range.start.line, character: 0 }, end: range.start },
617+
newText: ''
618+
});
619+
}
620+
}
621+
}
622+
557623
private async applyQuickfix(
558624
document: Document,
559625
range: Range,

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

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ import {
3838
convertRange,
3939
isInScript,
4040
isGeneratedSvelteComponentName,
41-
scriptElementKindToCompletionItemKind
41+
scriptElementKindToCompletionItemKind,
42+
cloneRange
4243
} from '../utils';
4344
import { getJsDocTemplateCompletion } from './getJsDocTemplateCompletion';
4445
import {
@@ -595,7 +596,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
595596
return tagNames.map((name) => ({
596597
label: name,
597598
kind: CompletionItemKind.Property,
598-
textEdit: TextEdit.replace(this.cloneRange(replacementRange), name),
599+
textEdit: TextEdit.replace(cloneRange(replacementRange), name),
599600
commitCharacters: []
600601
}));
601602
}
@@ -641,18 +642,11 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
641642
documentation: info.doc && { kind: MarkupKind.Markdown, value: info.doc },
642643
commitCharacters: [],
643644
textEdit: defaultTextEditRange
644-
? TextEdit.replace(this.cloneRange(defaultTextEditRange), name)
645+
? TextEdit.replace(cloneRange(defaultTextEditRange), name)
645646
: undefined
646647
};
647648
}
648649

649-
private cloneRange(range: Range) {
650-
return Range.create(
651-
Position.create(range.start.line, range.start.character),
652-
Position.create(range.end.line, range.end.character)
653-
);
654-
}
655-
656650
private getCompletionListForDirectiveOrProps(
657651
attributeContext: AttributeContext | null,
658652
componentInfo: ComponentInfoProvider | null,
@@ -885,10 +879,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionRe
885879
tsDoc: SvelteDocumentSnapshot
886880
) {
887881
if (isHandlerCompletion && completionItem.label.startsWith('on:')) {
888-
completionItem.textEdit = TextEdit.replace(
889-
this.cloneRange(wordRange),
890-
completionItem.label
891-
);
882+
completionItem.textEdit = TextEdit.replace(cloneRange(wordRange), completionItem.label);
892883

893884
return completionItem;
894885
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,3 +377,10 @@ export function hasTsExtensions(fileName: string) {
377377
export function isSvelte2tsxShimFile(fileName: string | undefined) {
378378
return fileName?.endsWith('svelte-shims.d.ts') || fileName?.endsWith('svelte-shims-v4.d.ts');
379379
}
380+
381+
export function cloneRange(range: Range): Range {
382+
return Range.create(
383+
Position.create(range.start.line, range.start.character),
384+
Position.create(range.end.line, range.end.character)
385+
);
386+
}

‎packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts

Lines changed: 177 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('CodeActionsProvider', function () {
6767
const filePath = getFullPath(filename);
6868
const document = docManager.openClientDocument(<any>{
6969
uri: pathToUrl(filePath),
70-
text: harmonizeNewLines(ts.sys.readFile(filePath) || '')
70+
text: ts.sys.readFile(filePath) || ''
7171
});
7272
return { provider, document, docManager, lsAndTsDocResolver };
7373
}
@@ -2086,6 +2086,116 @@ describe('CodeActionsProvider', function () {
20862086
]);
20872087
});
20882088

2089+
it('organize imports without leftover indentation', async () => {
2090+
const { provider, document } = setup('organize-import-all-remove.svelte');
2091+
2092+
const codeActions = await provider.getCodeActions(
2093+
document,
2094+
Range.create(Position.create(1, 4), Position.create(1, 5)),
2095+
{
2096+
diagnostics: [],
2097+
only: [CodeActionKind.SourceOrganizeImports]
2098+
}
2099+
);
2100+
2101+
assert.deepStrictEqual(codeActions, [
2102+
{
2103+
title: 'Organize Imports',
2104+
edit: {
2105+
documentChanges: [
2106+
{
2107+
textDocument: {
2108+
uri: getUri('organize-import-all-remove.svelte'),
2109+
version: null
2110+
},
2111+
edits: [
2112+
{
2113+
range: {
2114+
start: {
2115+
line: 1,
2116+
character: 4
2117+
},
2118+
end: {
2119+
line: 2,
2120+
character: 4
2121+
}
2122+
},
2123+
newText: ''
2124+
},
2125+
{
2126+
range: {
2127+
start: {
2128+
line: 2,
2129+
character: 4
2130+
},
2131+
end: {
2132+
line: 3,
2133+
character: 0
2134+
}
2135+
},
2136+
newText: ''
2137+
},
2138+
{
2139+
range: {
2140+
start: {
2141+
line: 4,
2142+
character: 4
2143+
},
2144+
end: {
2145+
line: 5,
2146+
character: 4
2147+
}
2148+
},
2149+
newText: ''
2150+
},
2151+
{
2152+
range: {
2153+
start: {
2154+
line: 5,
2155+
character: 4
2156+
},
2157+
end: {
2158+
line: 6,
2159+
character: 0
2160+
}
2161+
},
2162+
newText: ''
2163+
},
2164+
{
2165+
range: {
2166+
start: {
2167+
line: 1,
2168+
character: 0
2169+
},
2170+
end: {
2171+
line: 1,
2172+
character: 4
2173+
}
2174+
},
2175+
newText: ''
2176+
},
2177+
{
2178+
range: {
2179+
start: {
2180+
line: 4,
2181+
character: 0
2182+
},
2183+
end: {
2184+
line: 4,
2185+
character: 4
2186+
}
2187+
},
2188+
newText: ''
2189+
}
2190+
]
2191+
}
2192+
]
2193+
},
2194+
kind: 'source.organizeImports'
2195+
}
2196+
]);
2197+
});
2198+
20892199
it('should do extract into function refactor', async () => {
20902200
const { provider, document } = setup('codeactions.svelte');
20912201

@@ -2310,4 +2420,70 @@ describe('CodeActionsProvider', function () {
23102420

23112421
assert.deepStrictEqual(codeActions, []);
23122422
});
2423+
2424+
if (!isSvelte5Plus) {
2425+
return;
2426+
}
2427+
2428+
it('organizes imports with top-level snippets', async () => {
2429+
const { provider, document } = setup('organize-imports-snippet.svelte');
2430+
2431+
const codeActions = await provider.getCodeActions(
2432+
document,
2433+
Range.create(Position.create(4, 15), Position.create(4, 15)),
2434+
{
2435+
diagnostics: [],
2436+
only: [CodeActionKind.SourceOrganizeImports]
2437+
}
2438+
);
2439+
2440+
(<TextDocumentEdit>codeActions[0]?.edit?.documentChanges?.[0])?.edits.forEach(
2441+
(edit) => (edit.newText = harmonizeNewLines(edit.newText))
2442+
);
2443+
2444+
assert.deepStrictEqual(codeActions, [
2445+
{
2446+
edit: {
2447+
documentChanges: [
2448+
{
2449+
edits: [
2450+
{
2451+
newText: '',
2452+
range: {
2453+
end: {
2454+
character: 0,
2455+
line: 5
2456+
},
2457+
start: {
2458+
character: 4,
2459+
line: 4
2460+
}
2461+
}
2462+
},
2463+
{
2464+
newText: '',
2465+
range: {
2466+
end: {
2467+
character: 4,
2468+
line: 4
2469+
},
2470+
start: {
2471+
character: 0,
2472+
line: 4
2473+
}
2474+
}
2475+
}
2476+
],
2477+
textDocument: {
2478+
uri: getUri('organize-imports-snippet.svelte'),
2479+
version: null
2480+
}
2481+
}
2482+
]
2483+
},
2484+
kind: 'source.organizeImports',
2485+
title: 'Organize Imports'
2486+
}
2487+
]);
2488+
});
23132489
});

‎packages/language-server/test/plugins/typescript/testfiles/code-actions/organize-import-all-remove.svelte

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<script lang="ts">
2+
import { someStore } from './importing/a';
3+
import { someStore as someOtherStore } from './importing/a';
4+
5+
import { someStore as someOtherStore2 } from './importing/a';
6+
import { someStore as someOtherStore3 } from './importing/a';
7+
</script>

‎packages/language-server/test/plugins/typescript/testfiles/code-actions/organize-imports-snippet.svelte

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script lang="ts" module>
2+
</script>
3+
4+
<script lang="ts">
5+
import { Foo } from "./importing/a";
6+
</script>
7+
8+
{#snippet baz()}
9+
{/snippet}

0 commit comments

Comments
 (0)