From c8f7940428be4ccc5f45023293b6e650d2bc0058 Mon Sep 17 00:00:00 2001 From: Lyu Jason Date: Thu, 13 May 2021 11:19:12 +0800 Subject: [PATCH 1/2] fix code-action for ts-checked files --- .../features/getCodeActions/getQuickfixes.ts | 4 +- .../features/CodeActionsProvider.ts | 194 +++++++++++------- packages/language-server/src/utils.ts | 4 + .../features/CodeActionsProvider.test.ts | 99 ++++++++- .../code-actions/codeaction-checkJs.svelte | 4 + .../{ => code-actions}/codeactions.svelte | 0 .../organize-imports-module-store.svelte | 0 .../organize-imports-with-module.svelte | 0 8 files changed, 231 insertions(+), 74 deletions(-) create mode 100644 packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-checkJs.svelte rename packages/language-server/test/plugins/typescript/testfiles/{ => code-actions}/codeactions.svelte (100%) rename packages/language-server/test/plugins/typescript/testfiles/{ => code-actions}/organize-imports-module-store.svelte (100%) rename packages/language-server/test/plugins/typescript/testfiles/{ => code-actions}/organize-imports-with-module.svelte (100%) diff --git a/packages/language-server/src/plugins/svelte/features/getCodeActions/getQuickfixes.ts b/packages/language-server/src/plugins/svelte/features/getCodeActions/getQuickfixes.ts index 2d9809280..e2cd43f56 100644 --- a/packages/language-server/src/plugins/svelte/features/getCodeActions/getQuickfixes.ts +++ b/packages/language-server/src/plugins/svelte/features/getCodeActions/getQuickfixes.ts @@ -12,7 +12,7 @@ import { TextEdit } from 'vscode-languageserver'; import { mapObjWithRangeToOriginal, offsetAt, positionAt } from '../../../../lib/documents'; -import { pathToUrl } from '../../../../utils'; +import { getIndent, pathToUrl } from '../../../../utils'; import { SvelteDocument } from '../../SvelteDocument'; import ts from 'typescript'; // estree does not have start/end in their public Node interface, @@ -111,7 +111,7 @@ async function getSvelteIgnoreEdit(svelteDoc: SvelteDocument, ast: Ast, diagnost transpiled.getText() ); const afterStartLineStart = content.slice(nodeLineStart); - const indent = /^[ |\t]+/.exec(afterStartLineStart)?.[0] ?? ''; + const indent = getIndent(afterStartLineStart); // TODO: Make all code action's new line consistent const ignore = `${indent}${EOL}`; diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index 3e9b5513d..da57c5bd7 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -15,7 +15,7 @@ import { isInTag, getLineAtPosition } from '../../../lib/documents'; -import { pathToUrl, flatten, isNotNullOrUndefined, modifyLines } from '../../../utils'; +import { pathToUrl, flatten, isNotNullOrUndefined, modifyLines, getIndent } from '../../../utils'; import { CodeActionsProvider } from '../../interfaces'; import { SnapshotFragment, SvelteSnapshotFragment } from '../DocumentSnapshot'; import { LSAndTSDocResolver } from '../LSAndTSDocResolver'; @@ -163,76 +163,89 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { const docs = new SnapshotFragmentMap(this.lsAndTsDocResolver); docs.set(tsDoc.filePath, { fragment, snapshot: tsDoc }); - return await Promise.all( - codeFixes.map(async (fix) => { - const documentChanges = await Promise.all( - fix.changes.map(async (change) => { - const { snapshot, fragment } = await docs.retrieve(change.fileName); - return TextDocumentEdit.create( - OptionalVersionedTextDocumentIdentifier.create( - pathToUrl(change.fileName), - null - ), - change.textChanges - .map((edit) => { - if ( - fix.fixName === 'import' && - fragment instanceof SvelteSnapshotFragment - ) { - return this.completionProvider.codeActionChangeToTextEdit( - document, - fragment, - edit, - true, - isInTag(range.start, document.scriptInfo) || - isInTag(range.start, document.moduleScriptInfo) - ); - } - - if ( - !isNoTextSpanInGeneratedCode( - snapshot.getFullText(), - edit.span - ) - ) { - return undefined; - } - - let originalRange = mapRangeToOriginal( - fragment, - convertRange(fragment, edit.span) - ); - - if (fix.fixName === 'unusedIdentifier') { - originalRange = this.checkRemoveImportCodeActionRange( - edit, - fragment, - originalRange - ); - } - - if (fix.fixName === 'fixMissingFunctionDeclaration') { - originalRange = this.checkEndOfFileCodeInsert( - originalRange, - range, - document - ); - } - - return TextEdit.replace(originalRange, edit.newText); - }) - .filter(isNotNullOrUndefined) - ); - }) - ); - return CodeAction.create( - fix.description, - { - documentChanges - }, - CodeActionKind.QuickFix + const codeActionsPromises = codeFixes.map(async (fix) => { + const documentChangesPromises = fix.changes.map(async (change) => { + const { snapshot, fragment } = await docs.retrieve(change.fileName); + return TextDocumentEdit.create( + OptionalVersionedTextDocumentIdentifier.create( + pathToUrl(change.fileName), + null + ), + change.textChanges + .map((edit) => { + if ( + fix.fixName === 'import' && + fragment instanceof SvelteSnapshotFragment + ) { + return this.completionProvider.codeActionChangeToTextEdit( + document, + fragment, + edit, + true, + isInTag(range.start, document.scriptInfo) || + isInTag(range.start, document.moduleScriptInfo) + ); + } + + if (!isNoTextSpanInGeneratedCode(snapshot.getFullText(), edit.span)) { + return undefined; + } + + let originalRange = mapRangeToOriginal( + fragment, + convertRange(fragment, edit.span) + ); + + if (fix.fixName === 'unusedIdentifier') { + originalRange = this.checkRemoveImportCodeActionRange( + edit, + fragment, + originalRange + ); + } + + if (fix.fixName === 'fixMissingFunctionDeclaration') { + originalRange = this.checkEndOfFileCodeInsert( + originalRange, + range, + document + ); + } + + if (fix.fixName === 'disableJsDiagnostics') { + if (edit.newText.includes('ts-nocheck')) { + return this.checkTsNoCheckCodeInsert(document, edit); + } + + return this.checkDisableJsDiagnosticsCodeInsert( + originalRange, + document, + edit + ); + } + + return TextEdit.replace(originalRange, edit.newText); + }) + .filter(isNotNullOrUndefined) ); - }) + }); + const documentChanges = await Promise.all(documentChangesPromises); + return CodeAction.create( + fix.description, + { + documentChanges + }, + CodeActionKind.QuickFix + ); + }); + + const codeActions = await Promise.all(codeActionsPromises); + + // filter out empty code action + return codeActions.filter((codeAction) => + codeAction.edit?.documentChanges?.every( + (change) => (change).edits.length > 0 + ) ); } @@ -398,6 +411,47 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { return resultRange; } + private checkTsNoCheckCodeInsert( + document: Document, + edit: ts.TextChange + ): TextEdit | undefined { + if (!document.scriptInfo) { + return undefined; + } + + const newText = ts.sys.newLine + edit.newText; + + return TextEdit.insert(document.scriptInfo.startPos, newText); + } + + private checkDisableJsDiagnosticsCodeInsert( + originalRange: Range, + document: Document, + edit: ts.TextChange + ): TextEdit { + const startOffset = document.offsetAt(originalRange.start); + const text = document.getText(); + + // svetlte2tsx removes export in instance script + const insertedAfterExport = text.slice(0, startOffset).trim().endsWith('export'); + + if (!insertedAfterExport) { + return TextEdit.replace(originalRange, edit.newText); + } + + const position = document.positionAt(text.lastIndexOf('export', startOffset)); + + // fix the length of trailing indent + const linesOfNewText = edit.newText.split('\n'); + if (/^[ \t]*$/.test(linesOfNewText[linesOfNewText.length - 1])) { + const line = getLineAtPosition(originalRange.start, document.getText()); + const indent = getIndent(line); + linesOfNewText[linesOfNewText.length - 1] = indent; + } + + return TextEdit.insert(position, linesOfNewText.join('\n')); + } + private async getLSAndTSDoc(document: Document) { return this.lsAndTsDocResolver.getLSAndTSDoc(document); } diff --git a/packages/language-server/src/utils.ts b/packages/language-server/src/utils.ts index ef0ebbebd..90fad5ec8 100644 --- a/packages/language-server/src/utils.ts +++ b/packages/language-server/src/utils.ts @@ -141,3 +141,7 @@ export async function filterAsync( ) ).filter((i) => i !== fail) as T[]; } + +export function getIndent(text: string) { + return /^[ |\t]+/.exec(text)?.[0] ?? ''; +} diff --git a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts index 553f246b5..c1d9632ab 100644 --- a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts @@ -5,7 +5,13 @@ import { pathToUrl } from '../../../../src/utils'; import ts from 'typescript'; import * as path from 'path'; import * as assert from 'assert'; -import { Range, Position, CodeActionKind, TextDocumentEdit } from 'vscode-languageserver'; +import { + Range, + Position, + CodeActionKind, + TextDocumentEdit, + CodeAction +} from 'vscode-languageserver'; import { CompletionsProviderImpl } from '../../../../src/plugins/typescript/features/CompletionProvider'; import { LSConfigManager } from '../../../../src/ls-config'; @@ -13,7 +19,7 @@ const testDir = path.join(__dirname, '..'); describe('CodeActionsProvider', () => { function getFullPath(filename: string) { - return path.join(testDir, 'testfiles', filename); + return path.join(testDir, 'testfiles', 'code-actions', filename); } function getUri(filename: string) { @@ -152,6 +158,95 @@ describe('CodeActionsProvider', () => { ]); }); + it('provides quickfix for ts-checked-js', async () => { + const { provider, document } = setup('codeaction-checkJs.svelte'); + const errorRange = Range.create(Position.create(2, 21), Position.create(2, 26)); + + const codeActions = await provider.getCodeActions(document, errorRange, { + diagnostics: [ + { + code: 2304, + message: "Cannot find name 'blubb'.", + range: errorRange + } + ] + }); + + for (const codeAction of codeActions) { + (codeAction.edit?.documentChanges?.[0])?.edits.forEach( + (edit) => (edit.newText = harmonizeNewLines(edit.newText)) + ); + } + + const textDocument = { + uri: getUri('codeaction-checkJs.svelte'), + version: null + }; + assert.deepStrictEqual(codeActions, [ + { + edit: { + documentChanges: [ + { + edits: [ + { + newText: '\nimport { blubb } from "../definitions";\n\n', + range: Range.create( + Position.create(0, 8), + Position.create(0, 8) + ) + } + ], + textDocument + } + ] + }, + kind: 'quickfix', + title: 'Import \'blubb\' from module "../definitions"' + }, + { + edit: { + documentChanges: [ + { + edits: [ + { + newText: '// @ts-ignore\n ', + range: Range.create( + Position.create(2, 4), + Position.create(2, 4) + ) + } + ], + textDocument + } + ] + }, + kind: 'quickfix', + title: 'Ignore this error message' + }, + { + edit: { + documentChanges: [ + { + edits: [ + { + newText: '\n// @ts-nocheck', + range: Range.create( + Position.create(0, 8), + Position.create(0, 8) + ) + } + ], + textDocument + } + ] + }, + + kind: 'quickfix', + title: 'Disable checking for this file' + } + ]); + }); + it('organizes imports', async () => { const { provider, document } = setup('codeactions.svelte'); diff --git a/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-checkJs.svelte b/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-checkJs.svelte new file mode 100644 index 000000000..5872c1562 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeaction-checkJs.svelte @@ -0,0 +1,4 @@ + diff --git a/packages/language-server/test/plugins/typescript/testfiles/codeactions.svelte b/packages/language-server/test/plugins/typescript/testfiles/code-actions/codeactions.svelte similarity index 100% rename from packages/language-server/test/plugins/typescript/testfiles/codeactions.svelte rename to packages/language-server/test/plugins/typescript/testfiles/code-actions/codeactions.svelte diff --git a/packages/language-server/test/plugins/typescript/testfiles/organize-imports-module-store.svelte b/packages/language-server/test/plugins/typescript/testfiles/code-actions/organize-imports-module-store.svelte similarity index 100% rename from packages/language-server/test/plugins/typescript/testfiles/organize-imports-module-store.svelte rename to packages/language-server/test/plugins/typescript/testfiles/code-actions/organize-imports-module-store.svelte diff --git a/packages/language-server/test/plugins/typescript/testfiles/organize-imports-with-module.svelte b/packages/language-server/test/plugins/typescript/testfiles/code-actions/organize-imports-with-module.svelte similarity index 100% rename from packages/language-server/test/plugins/typescript/testfiles/organize-imports-with-module.svelte rename to packages/language-server/test/plugins/typescript/testfiles/code-actions/organize-imports-with-module.svelte From 850f2c96c4d2360882ee4e5de8a2fc61dd3514e9 Mon Sep 17 00:00:00 2001 From: Lyu Jason Date: Thu, 13 May 2021 11:25:26 +0800 Subject: [PATCH 2/2] filter out invalid text edit --- .../src/plugins/typescript/features/CodeActionsProvider.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index da57c5bd7..8f1647631 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -224,6 +224,10 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { ); } + if (originalRange.start.line < 0 || originalRange.end.line < 0) { + return undefined; + } + return TextEdit.replace(originalRange, edit.newText); }) .filter(isNotNullOrUndefined)