From 1674a3025f3a9d7e20643f1d42ba3139336eb68b Mon Sep 17 00:00:00 2001 From: ivanwonder Date: Sat, 9 Jul 2022 22:15:09 +0800 Subject: [PATCH] feat(server): support code action In ts service, the `codeFixes`([getCodeFixesAtPosition](https://github.com/microsoft/TypeScript/blob/7584e6aad6b21f7334562bfd9d8c3c80aafed064/src/services/services.ts#L2689)) and refactors([getApplicableRefactors](https://github.com/microsoft/TypeScript/blob/7584e6aad6b21f7334562bfd9d8c3c80aafed064/src/services/services.ts#L2699)) are resolved in different request. But in LSP they are resolved in the `CodeAction` request. Now, this PR only handles the `codeFixes` because the `@angular/language-service` only supports it. --- integration/lsp/ivy_spec.ts | 125 ++++++++++++++++++++++++++++++++++++ package.json | 2 +- server/package.json | 2 +- server/src/session.ts | 110 ++++++++++++++++++++++++++++++- server/src/utils.ts | 29 +++++++++ yarn.lock | 8 +-- 6 files changed, 269 insertions(+), 7 deletions(-) diff --git a/integration/lsp/ivy_spec.ts b/integration/lsp/ivy_spec.ts index c4525ffa61..910cb41c51 100644 --- a/integration/lsp/ivy_spec.ts +++ b/integration/lsp/ivy_spec.ts @@ -647,6 +647,131 @@ describe('insert snippet text', () => { }); }); +describe('code fixes', () => { + jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; /* 10 seconds */ + + let client: MessageConnection; + beforeEach(async () => { + client = createConnection({ + ivy: true, + includeCompletionsWithSnippetText: true, + }); + // If debugging, set to + // - lsp.Trace.Messages to inspect request/response/notification, or + // - lsp.Trace.Verbose to inspect payload + client.trace(lsp.Trace.Off, createTracer()); + client.listen(); + await initializeServer(client); + }); + + afterEach(() => { + client.dispose(); + }); + + it('should fix error when property does not exist on type', async () => { + openTextDocument(client, FOO_TEMPLATE, `{{titl}}`); + const languageServiceEnabled = await waitForNgcc(client); + expect(languageServiceEnabled).toBeTrue(); + const diags = await getDiagnosticsForFile(client, FOO_TEMPLATE); + const codeActions = await client.sendRequest(lsp.CodeActionRequest.type, { + textDocument: { + uri: FOO_TEMPLATE_URI, + }, + range: lsp.Range.create(lsp.Position.create(0, 3), lsp.Position.create(0, 3)), + context: lsp.CodeActionContext.create(diags), + }) as lsp.CodeAction[]; + const expectedCodeActionInTemplate = { + 'edit': { + 'changes': { + [FOO_TEMPLATE_URI]: [{ + 'newText': 'title', + 'range': {'start': {'line': 0, 'character': 2}, 'end': {'line': 0, 'character': 6}} + }] + } + } + }; + expect(codeActions).toContain(jasmine.objectContaining(expectedCodeActionInTemplate)); + }); + + describe('should work', () => { + beforeEach(async () => { + openTextDocument(client, FOO_COMPONENT, ` + import {Component, NgModule} from '@angular/core'; + @Component({ + template: '{{tite}}{{bannr}}', + }) + export class AppComponent { + title = ''; + banner = ''; + } + `); + const languageServiceEnabled = await waitForNgcc(client); + expect(languageServiceEnabled).toBeTrue(); + }); + + it('for "fixSpelling"', async () => { + const fixSpellingCodeAction = await client.sendRequest(lsp.CodeActionResolveRequest.type, { + title: '', + data: { + fixId: 'fixSpelling', + document: lsp.TextDocumentIdentifier.create(FOO_COMPONENT_URI), + }, + }); + const expectedFixSpellingInTemplate = { + 'edit': { + 'changes': { + [FOO_COMPONENT_URI]: [ + { + 'newText': 'title', + 'range': { + 'start': {'line': 3, 'character': 21}, + 'end': {'line': 3, 'character': 25}, + }, + }, + { + 'newText': 'banner', + 'range': + {'start': {'line': 3, 'character': 29}, 'end': {'line': 3, 'character': 34}} + } + ] + } + } + }; + expect(fixSpellingCodeAction) + .toEqual(jasmine.objectContaining(expectedFixSpellingInTemplate)); + }); + + it('for "fixMissingMember"', async () => { + const fixMissingMemberCodeAction = + await client.sendRequest(lsp.CodeActionResolveRequest.type, { + title: '', + data: { + fixId: 'fixMissingMember', + document: lsp.TextDocumentIdentifier.create(FOO_COMPONENT_URI), + }, + }); + const expectedFixMissingMemberInComponent = { + 'edit': { + 'changes': { + [FOO_COMPONENT_URI]: [ + { + 'newText': 'tite: any;\n', + 'range': {'start': {'line': 8, 'character': 0}, 'end': {'line': 8, 'character': 0}} + }, + { + 'newText': 'bannr: any;\n', + 'range': {'start': {'line': 8, 'character': 0}, 'end': {'line': 8, 'character': 0}} + } + ] + } + } + }; + expect(fixMissingMemberCodeAction) + .toEqual(jasmine.objectContaining(expectedFixMissingMemberInComponent)); + }); + }); +}); + function onNgccProgress(client: MessageConnection): Promise { return new Promise(resolve => { client.onNotification(NgccProgressEnd, (params) => { diff --git a/package.json b/package.json index 03f023931c..a65b50e69f 100644 --- a/package.json +++ b/package.json @@ -216,7 +216,7 @@ "test:syntaxes": "yarn compile:syntaxes-test && yarn build:syntaxes && jasmine dist/syntaxes/test/driver.js" }, "dependencies": { - "@angular/language-service": "14.1.0", + "@angular/language-service": "14.2.0-next.0", "typescript": "4.7.4", "vscode-jsonrpc": "6.0.0", "vscode-languageclient": "7.0.0", diff --git a/server/package.json b/server/package.json index 1034bc611d..e63056c44f 100644 --- a/server/package.json +++ b/server/package.json @@ -15,7 +15,7 @@ "ngserver": "./bin/ngserver" }, "dependencies": { - "@angular/language-service": "14.1.0", + "@angular/language-service": "14.2.0-next.0", "vscode-jsonrpc": "6.0.0", "vscode-languageserver": "7.0.0", "vscode-uri": "3.0.3" diff --git a/server/src/session.ts b/server/src/session.ts index 56c37edf60..01116b1fa3 100644 --- a/server/src/session.ts +++ b/server/src/session.ts @@ -19,7 +19,7 @@ import {readNgCompletionData, tsCompletionEntryToLspCompletionItem} from './comp import {tsDiagnosticToLspDiagnostic} from './diagnostic'; import {resolveAndRunNgcc} from './ngcc'; import {ServerHost} from './server_host'; -import {filePathToUri, getMappedDefinitionInfo, isConfiguredProject, isDebugMode, lspPositionToTsPosition, lspRangeToTsPositions, MruTracker, tsDisplayPartsToText, tsTextSpanToLspRange, uriToFilePath} from './utils'; +import {filePathToUri, getMappedDefinitionInfo, isConfiguredProject, isDebugMode, lspPositionToTsPosition, lspRangeToTsPositions, MruTracker, tsDisplayPartsToText, tsFileTextChangesToLspWorkspaceEdit, tsTextSpanToLspRange, uriToFilePath} from './utils'; export interface SessionOptions { host: ServerHost; @@ -48,6 +48,9 @@ enum NgccErrorMessageAction { showOutput, } +const defaultFormatOptions: ts.FormatCodeSettings = {}; +const defaultPreferences: ts.UserPreferences = {}; + /** * Session is a wrapper around lsp.IConnection, with all the necessary protocol * handlers installed for Angular language service. @@ -197,6 +200,67 @@ export class Session { conn.onCodeLens(p => this.onCodeLens(p)); conn.onCodeLensResolve(p => this.onCodeLensResolve(p)); conn.onSignatureHelp(p => this.onSignatureHelp(p)); + conn.onCodeAction(p => this.onCodeAction(p)); + conn.onCodeActionResolve(p => this.onCodeActionResolve(p)); + } + + private onCodeAction(params: lsp.CodeActionParams): lsp.CodeAction[]|null { + const filePath = uriToFilePath(params.textDocument.uri); + const lsInfo = this.getLSAndScriptInfo(params.textDocument); + if (!lsInfo) { + return null; + } + const start = lspPositionToTsPosition(lsInfo.scriptInfo, params.range.start); + const end = lspPositionToTsPosition(lsInfo.scriptInfo, params.range.end); + const errorCodes = params.context.diagnostics.map(diag => diag.code) + .filter((code): code is number => typeof code === 'number'); + + const codeActions = lsInfo.languageService.getCodeFixesAtPosition( + filePath, start, end, errorCodes, defaultFormatOptions, defaultPreferences); + const individualCodeFixes = codeActions.map(codeAction => { + return { + title: codeAction.description, + kind: lsp.CodeActionKind.QuickFix, + diagnostics: params.context.diagnostics, + edit: tsFileTextChangesToLspWorkspaceEdit( + codeAction.changes, (path: string) => this.projectService.getScriptInfo(path)), + }; + }); + const codeFixesAll = getCodeFixesAll(codeActions, params.textDocument); + return [...individualCodeFixes, ...codeFixesAll]; + } + + private onCodeActionResolve(param: lsp.CodeAction): lsp.CodeAction { + const codeActionResolve = param.data as unknown as CodeActionResolveData; + /** + * Now `@angular/language-service` only support quick fix, so the `onCodeAction` will return the + * `edit` of the `lsp.CodeAction` for the diagnostics in the range that the user selects except + * the fix all code actions. + * + * And the function `getCombinedCodeFix` only cares about the `fixId` and the `document`. + * https://github.com/microsoft/vscode/blob/8ba9963c2edb08d54f2b7221137d6f1de79ecc09/extensions/typescript-language-features/src/languageFeatures/quickFix.ts#L258 + */ + const isCodeFixesAll = codeActionResolve.fixId !== undefined; + if (!isCodeFixesAll) { + return param; + } + const filePath = uriToFilePath(codeActionResolve.document.uri); + const lsInfo = this.getLSAndScriptInfo(codeActionResolve.document); + if (!lsInfo) { + return param; + } + const fixesAllChanges = lsInfo.languageService.getCombinedCodeFix( + { + type: 'file', + fileName: filePath, + }, + codeActionResolve.fixId as {}, defaultFormatOptions, defaultPreferences); + + return { + title: param.title, + edit: tsFileTextChangesToLspWorkspaceEdit( + fixesAllChanges.changes, (path) => this.projectService.getScriptInfo(path)), + }; } private isInAngularProject(params: IsInAngularProjectParams): boolean|null { @@ -663,6 +727,10 @@ export class Session { workspace: { workspaceFolders: {supported: true}, }, + codeActionProvider: this.ivy ? { + resolveProvider: true, + } : + undefined, }, serverOptions, }; @@ -1239,3 +1307,43 @@ function isTypeScriptFile(path: string): boolean { function isExternalTemplate(path: string): boolean { return !isTypeScriptFile(path); } + +interface CodeActionResolveData { + fixId?: string; + document: lsp.TextDocumentIdentifier; +} + +/** + * Extract the fixAll action from `codeActions` + * + * When getting code fixes at the specified cursor position, the LS will return the code actions + * that tell the editor how to fix it. For each code action, if the document includes multi + * same-type errors, the `fixId` will append to it, because they are not `complete`. This function + * will extract them, and they will be resolved lazily in the `onCodeActionResolve` function. + * + * Now the client can only resolve the `edit` property. + * https://github.com/microsoft/vscode-languageserver-node/blob/f97bb73dbfb920af4bc8c13ecdcdc16359cdeda6/client/src/common/codeAction.ts#L45 + */ +function getCodeFixesAll( + codeActions: readonly ts.CodeFixAction[], + document: lsp.TextDocumentIdentifier): lsp.CodeAction[] { + const seenFixId = new Set(); + const lspCodeActions: lsp.CodeAction[] = []; + for (const codeAction of codeActions) { + const fixId = codeAction.fixId as string | undefined; + if (fixId === undefined || codeAction.fixAllDescription === undefined || seenFixId.has(fixId)) { + continue; + } + seenFixId.add(fixId); + const codeActionResolveData: CodeActionResolveData = { + fixId, + document, + }; + lspCodeActions.push({ + title: codeAction.fixAllDescription, + kind: lsp.CodeActionKind.QuickFix, + data: codeActionResolveData, + }); + } + return lspCodeActions; +} diff --git a/server/src/utils.ts b/server/src/utils.ts index 658068b63a..cf0b5c7ea6 100644 --- a/server/src/utils.ts +++ b/server/src/utils.ts @@ -39,6 +39,35 @@ export function filePathToUri(filePath: string): lsp.DocumentUri { return URI.file(filePath).toString(); } +/** + * Converts ts.FileTextChanges to lsp.WorkspaceEdit. + */ +export function tsFileTextChangesToLspWorkspaceEdit( + changes: readonly ts.FileTextChanges[], + getScriptInfo: (path: string) => ts.server.ScriptInfo | undefined): lsp.WorkspaceEdit { + const workspaceChanges: {[uri: string]: lsp.TextEdit[]} = {}; + for (const change of changes) { + const scriptInfo = getScriptInfo(change.fileName); + const uri = filePathToUri(change.fileName); + if (scriptInfo === undefined) { + continue; + } + if (!workspaceChanges[uri]) { + workspaceChanges[uri] = []; + } + for (const textChange of change.textChanges) { + const textEdit: lsp.TextEdit = { + newText: textChange.newText, + range: tsTextSpanToLspRange(scriptInfo, textChange.span), + }; + workspaceChanges[uri].push(textEdit); + } + } + return { + changes: workspaceChanges, + }; +} + /** * Convert ts.TextSpan to lsp.TextSpan. TypeScript keeps track of offset using * 1-based index whereas LSP uses 0-based index. diff --git a/yarn.lock b/yarn.lock index 3a7122528e..133edd5fe8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -158,10 +158,10 @@ uuid "^8.3.2" yargs "^17.0.0" -"@angular/language-service@14.1.0": - version "14.1.0" - resolved "https://registry.yarnpkg.com/@angular/language-service/-/language-service-14.1.0.tgz#e5ca5f948d1930ebd5791795c983dd0887d0fd63" - integrity sha512-ldL4xMDjXYZ93FCEIBVGipx9Qfgr7NuBNO+e25d+nWikXrUOnLfvF4UOL/TSUwSwqN4jxDI2KMNQIF6SecZfvQ== +"@angular/language-service@14.2.0-next.0": + version "14.2.0-next.0" + resolved "https://registry.yarnpkg.com/@angular/language-service/-/language-service-14.2.0-next.0.tgz#f1e60f00c5cbcd0b88b563837dfd01332b7d611f" + integrity sha512-Uj2XGSS6Z7wb6zuZ39uaXxwlXNTN+ByhUr5qJ8Ya4GkxjKJLN1uQDvFhjGIwMyG75q2Wv4GOIGeGo+0Gts2b6A== "@assemblyscript/loader@^0.10.1": version "0.10.1"