From e38b98abe6b46d77694737874c7c41a2f08bb8e2 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Mon, 13 Mar 2017 00:24:29 -0700 Subject: [PATCH 01/12] Add infrastructure for refactors --- src/harness/fourslash.ts | 71 +++++++++++ src/harness/harnessLanguageService.ts | 6 + .../unittests/tsserverProjectSystem.ts | 16 ++- src/server/client.ts | 44 +++++++ src/server/protocol.ts | 62 +++++++++- src/server/session.ts | 115 ++++++++++++++---- src/services/refactorProvider.ts | 92 ++++++++++++++ src/services/services.ts | 46 +++++++ src/services/tsconfig.json | 1 + src/services/types.ts | 10 ++ tests/cases/fourslash/fourslash.ts | 2 + 11 files changed, 432 insertions(+), 33 deletions(-) create mode 100644 src/services/refactorProvider.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 35788ebca8034..1e6fff32e0282 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -485,6 +485,10 @@ namespace FourSlash { return diagnostics; } + private getRefactorDiagnostics(fileName: string, range?: ts.TextRange): ts.RefactorDiagnostic[] { + return this.languageService.getRefactorDiagnostics(fileName, range); + } + private getAllDiagnostics(): ts.Diagnostic[] { const diagnostics: ts.Diagnostic[] = []; @@ -2193,6 +2197,22 @@ namespace FourSlash { return actions; } + private getRefactorActions(fileName: string, range?: ts.TextRange, formattingOptions?: ts.FormatCodeSettings): ts.CodeAction[] { + const diagnostics = this.getRefactorDiagnostics(fileName, range); + const actions: ts.CodeAction[] = []; + formattingOptions = formattingOptions || this.formatCodeSettings; + + for (const diagnostic of diagnostics) { + const diagnosticRange: ts.TextRange = { + pos: diagnostic.start, + end: diagnostic.end + }; + const newActions = this.languageService.getCodeActionsForRefactorAtPosition(fileName, diagnosticRange, diagnostic.code, formattingOptions); + actions.push.apply(actions, newActions); + } + return actions; + } + private applyCodeAction(fileName: string, actions: ts.CodeAction[], index?: number): void { if (index === undefined) { if (!(actions && actions.length === 1)) { @@ -2551,6 +2571,49 @@ namespace FourSlash { } } + public verifyRefactorAvailable(negative: boolean) { + // The ranges are used only when the refactors require a range as input information. For example the "extractMethod" refactor + // onlye one range is allowed per test + const ranges = this.getRanges(); + if (ranges.length > 1) { + throw new Error("only one refactor range is allowed per test"); + } + + const range = ranges[0] ? { pos: ranges[0].start, end: ranges[0].end } : undefined; + const refactorDiagnostics = this.getRefactorDiagnostics(this.activeFile.fileName, range); + if (negative && refactorDiagnostics.length > 0) { + this.raiseError(`verifyRefactorAvailable failed - expected no refactors but found some.`); + } + if (!negative && refactorDiagnostics.length === 0) { + this.raiseError(`verifyRefactorAvailable failed: expected refactor diagnostics but none found.`); + } + } + + public verifyFileAfterApplyingRefactors(expectedContent: string, formattingOptions?: ts.FormatCodeSettings) { + const ranges = this.getRanges(); + if (ranges.length > 1) { + throw new Error("only one refactor range is allowed per test"); + } + + const range = ranges[0] ? { pos: ranges[0].start, end: ranges[0].end } : undefined; + const actions = this.getRefactorActions(this.activeFile.fileName, range, formattingOptions); + + // Each refactor diagnostic will return one code action, but multiple refactor diagnostics can point to the same + // code action. For example in the case of "convert function to es6 class": + // + // function foo() { } + // ^^^ + // foo.prototype.getName = function () { return "name"; }; + // ^^^ + // These two diagnostics result in the same code action, so we only apply the first one. + this.applyCodeAction(this.activeFile.fileName, actions, /*index*/ 0); + const actualContent = this.getFileContent(this.activeFile.fileName); + + if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) { + this.raiseError(`verifyFileAfterApplyingRefactors failed: expected:\n${expectedContent}\nactual:\n${actualContent}`); + } + } + // Get the text of the entire line the caret is currently at private getCurrentLineContent() { const text = this.getFileContent(this.activeFile.fileName); @@ -3342,6 +3405,10 @@ namespace FourSlashInterface { public codeFixAvailable() { this.state.verifyCodeFixAvailable(this.negative); } + + public refactorAvailable() { + this.state.verifyRefactorAvailable(this.negative); + } } export class Verify extends VerifyNegatable { @@ -3548,6 +3615,10 @@ namespace FourSlashInterface { this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index); } + public fileAfterApplyingRefactors(expectedContent: string, formattingOptions: ts.FormatCodeSettings): void { + this.state.verifyFileAfterApplyingRefactors(expectedContent, formattingOptions); + } + public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void { this.state.verifyImportFixAtPosition(expectedTextArray, errorCode); } diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index e7f3274857063..8f35f0c33fa52 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -489,6 +489,12 @@ namespace Harness.LanguageService { getCodeFixesAtPosition(): ts.CodeAction[] { throw new Error("Not supported on the shim."); } + getRefactorDiagnostics(): ts.RefactorDiagnostic[] { + throw new Error("Not supported on the shim."); + } + getCodeActionsForRefactorAtPosition(): ts.CodeAction[] { + throw new Error("Not supported on the shim."); + } getEmitOutput(fileName: string): ts.EmitOutput { return unwrapJSONCallResult(this.shim.getEmitOutput(fileName)); } diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 4d1b2a0b10ccf..ff6e5b0309d41 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -313,6 +313,7 @@ namespace ts.projectSystem { this.map[timeoutId] = cb.bind(undefined, ...args); return timeoutId; } + unregister(id: any) { if (typeof id === "number") { delete this.map[id]; @@ -328,10 +329,13 @@ namespace ts.projectSystem { } invoke() { + // Note: invoking a callback may result in new callbacks been queued, + // so do not clear the entire callback list regardless. Only remove the + // ones we have invoked. for (const key in this.map) { this.map[key](); + delete this.map[key]; } - this.map = []; } } @@ -3446,10 +3450,18 @@ namespace ts.projectSystem { assert.equal(e1.event, "syntaxDiag"); host.clearOutput(); + // the semanticDiag message host.runQueuedImmediateCallbacks(); - assert.equal(host.getOutput().length, 2, "expect 2 messages"); + assert.equal(host.getOutput().length, 1, "expect 1 messages"); const e2 = getMessage(0); assert.equal(e2.event, "semanticDiag"); + host.clearOutput(); + + // the refactor diagnostics check + host.runQueuedImmediateCallbacks(); + assert.equal(host.getOutput().length, 2, "expect 2 messages"); + const e3 = getMessage(0); + assert.equal(e3.event, "refactorDiag"); verifyRequestCompleted(getErrId, 1); requestToCancel = -1; diff --git a/src/server/client.ts b/src/server/client.ts index 6f784db1682d2..856a734d597b3 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -692,6 +692,50 @@ namespace ts.server { return response.body.map(entry => this.convertCodeActions(entry, fileName)); } + getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[] { + const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos); + const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end); + + const args: protocol.GetRefactorsForRangeRequestArgs = { + file: fileName, + startLine: startLineOffset.line, + startOffset: startLineOffset.offset, + endLine: endLineOffset.line, + endOffset: endLineOffset.offset, + }; + + const request = this.processRequest(CommandNames.GetRefactorsForRange, args); + const response = this.processResponse(request); + + return response.body.map(entry => { + return { + code: entry.code, + end: this.lineOffsetToPosition(fileName, entry.end), + start: this.lineOffsetToPosition(fileName, entry.start), + text: entry.text + }; + }); + } + + getCodeActionsForRefactorAtPosition(fileName: string, range: TextRange, refactorCode: number): CodeAction[] { + const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos); + const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end); + + const args: protocol.GetCodeActionsForRefactorRequestArgs = { + file: fileName, + startLine: startLineOffset.line, + startOffset: startLineOffset.offset, + endLine: endLineOffset.line, + endOffset: endLineOffset.offset, + refactorCode + }; + + const request = this.processRequest(CommandNames.GetCodeActionsForRefactor, args); + const response = this.processResponse(request); + + return response.body.map(entry => this.convertCodeActions(entry, fileName)); + } + convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction { return { description: entry.description, diff --git a/src/server/protocol.ts b/src/server/protocol.ts index d12aa2e0a48b6..c188f07c9ad32 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -95,6 +95,9 @@ namespace ts.server.protocol { /* @internal */ export type GetCodeFixesFull = "getCodeFixes-full"; export type GetSupportedCodeFixes = "getSupportedCodeFixes"; + + export type GetRefactorsForRange = "getRefactorsForRange"; + export type GetCodeActionsForRefactor = "getCodeActionsForRefactor"; } /** @@ -394,6 +397,55 @@ namespace ts.server.protocol { position?: number; } + /** + * An diagnostic information suggesting refactors at applicable positions without + * clients asking. + */ + export interface RefactorDiagnostic { + text: string; + code: number; + start: Location; + end: Location; + } + + export interface RefactorDiagnosticEventBody { + file: string; + diagnostics: RefactorDiagnostic[]; + } + + /** + * Returns a list of applicable refactors at a given position. This request does not actually + * compute the refactors; instead it goes through faster checks to determine what is possible. + */ + export interface GetRefactorsForRangeRequest extends Request { + command: CommandTypes.GetRefactorsForRange; + arguments: GetRefactorsForRangeRequestArgs; + } + + export interface GetRefactorsForRangeRequestArgs extends TextRangeRequestArgs { + } + + export interface GetRefactorsForRangeResponse extends Response { + body?: RefactorDiagnostic[]; + } + + /** + * Computes the code actions for a given refactor. This is normally called after the user commited one + * of the applicable refactors provided by the "GetApplicableRefactors" API. + */ + export interface GetCodeActionsForRefactorRequest extends Request { + command: CommandTypes.GetCodeActionsForRefactor; + arguments: GetCodeActionsForRefactorRequestArgs; + } + + export interface GetCodeActionsForRefactorRequestArgs extends GetRefactorsForRangeRequestArgs { + refactorCode: number; + } + + export interface GetCodeActionsForRefactorResponse extends Response { + body?: CodeAction[]; + } + /** * Request for the available codefixes at a specific position. */ @@ -402,10 +454,7 @@ namespace ts.server.protocol { arguments: CodeFixRequestArgs; } - /** - * Instances of this interface specify errorcodes on a specific location in a sourcefile. - */ - export interface CodeFixRequestArgs extends FileRequestArgs { + export interface TextRangeRequestArgs extends FileRequestArgs { /** * The line number for the request (1-based). */ @@ -437,7 +486,12 @@ namespace ts.server.protocol { */ /* @internal */ endPosition?: number; + } + /** + * Instances of this interface specify errorcodes on a specific location in a sourcefile. + */ + export interface CodeFixRequestArgs extends TextRangeRequestArgs { /** * Errorcodes we want to get the fixes for. */ diff --git a/src/server/session.ts b/src/server/session.ts index b5e7e44970d4a..2921080190e3f 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -85,7 +85,7 @@ namespace ts.server { } export interface EventSender { - event(payload: any, eventName: string): void; + event(payload: T, eventName: string): void; } function allEditsBeforePos(edits: ts.TextChange[], pos: number) { @@ -190,6 +190,9 @@ namespace ts.server { /* @internal */ export const GetCodeFixesFull: protocol.CommandTypes.GetCodeFixesFull = "getCodeFixes-full"; export const GetSupportedCodeFixes: protocol.CommandTypes.GetSupportedCodeFixes = "getSupportedCodeFixes"; + + export const GetRefactorsForRange: protocol.CommandTypes.GetRefactorsForRange = "getRefactorsForRange"; + export const GetCodeActionsForRefactor: protocol.CommandTypes.GetCodeActionsForRefactor = "getCodeActionsForRefactor"; } export function formatMessage(msg: T, logger: server.Logger, byteLength: (s: string, encoding: string) => number, newLine: string): string { @@ -219,7 +222,7 @@ namespace ts.server { getCurrentRequestId(): number; sendRequestCompletedEvent(requestId: number): void; getServerHost(): ServerHost; - isCancellationRequested(): boolean; + isCancellationRequested(): boolean; executeWithRequestId(requestId: number, action: () => void): void; logError(error: Error, message: string): void; } @@ -300,7 +303,7 @@ namespace ts.server { } } - private setTimerHandle(timerHandle: any) {; + private setTimerHandle(timerHandle: any) { if (this.timerHandle !== undefined) { this.operationHost.getServerHost().clearTimeout(this.timerHandle); } @@ -380,7 +383,7 @@ namespace ts.server { break; case ProjectLanguageServiceStateEvent: const eventName: protocol.ProjectLanguageServiceStateEventName = "projectLanguageServiceState"; - this.event({ + this.event({ projectName: event.data.project.getProjectName(), languageServiceEnabled: event.data.languageServiceEnabled }, eventName); @@ -424,7 +427,7 @@ namespace ts.server { this.send(ev); } - public event(info: any, eventName: string) { + public event(info: T, eventName: string) { const ev: protocol.Event = { seq: 0, type: "event", @@ -459,7 +462,7 @@ namespace ts.server { } const bakedDiags = diags.map((diag) => formatDiag(file, project, diag)); - this.event({ file: file, diagnostics: bakedDiags }, "semanticDiag"); + this.event({ file: file, diagnostics: bakedDiags }, "semanticDiag"); } catch (err) { this.logError(err, "semantic check"); @@ -471,7 +474,7 @@ namespace ts.server { const diags = project.getLanguageService().getSyntacticDiagnostics(file); if (diags) { const bakedDiags = diags.map((diag) => formatDiag(file, project, diag)); - this.event({ file: file, diagnostics: bakedDiags }, "syntaxDiag"); + this.event({ file: file, diagnostics: bakedDiags }, "syntaxDiag"); } } catch (err) { @@ -501,9 +504,12 @@ namespace ts.server { this.syntacticCheck(checkSpec.fileName, checkSpec.project); next.immediate(() => { this.semanticCheck(checkSpec.fileName, checkSpec.project); - if (checkList.length > index) { - next.delay(followMs, checkOne); - } + next.immediate(() => { + this.checkRefactorDiagnostics(checkSpec.fileName, checkSpec.project); + if (checkList.length > index) { + next.delay(followMs, checkOne); + } + }) }); } } @@ -1296,8 +1302,8 @@ namespace ts.server { return !items ? undefined : simplifiedResult - ? this.decorateNavigationBarItems(items, project.getScriptInfoForNormalizedPath(file)) - : items; + ? this.decorateNavigationBarItems(items, project.getScriptInfoForNormalizedPath(file)) + : items; } private decorateNavigationTree(tree: ts.NavigationTree, scriptInfo: ScriptInfo): protocol.NavigationTree { @@ -1323,8 +1329,8 @@ namespace ts.server { return !tree ? undefined : simplifiedResult - ? this.decorateNavigationTree(tree, project.getScriptInfoForNormalizedPath(file)) - : tree; + ? this.decorateNavigationTree(tree, project.getScriptInfoForNormalizedPath(file)) + : tree; } private getNavigateToItems(args: protocol.NavtoRequestArgs, simplifiedResult: boolean): protocol.NavtoItem[] | NavigateToItem[] { @@ -1411,6 +1417,64 @@ namespace ts.server { return ts.getSupportedCodeFixes(); } + private extractStartAndEndPositionFromTextRangeRequestArgs(args: protocol.TextRangeRequestArgs, scriptInfo: ScriptInfo): { startPosition: number, endPosition: number } { + const startPosition = getStartPosition(); + const endPosition = getEndPosition(); + return { startPosition, endPosition }; + + function getStartPosition() { + return args.startPosition !== undefined ? args.startPosition : scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); + } + + function getEndPosition() { + return args.endPosition !== undefined ? args.endPosition : scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); + } + } + + private normalizeRefactorDiagnostics(refactorDiags: RefactorDiagnostic[], scriptInfo: ScriptInfo): protocol.RefactorDiagnostic[] { + return map(refactorDiags, refactorDiag => { + return { + code: refactorDiag.code, + text: refactorDiag.text, + start: scriptInfo.positionToLineOffset(refactorDiag.start), + end: scriptInfo.positionToLineOffset(refactorDiag.end) + }; + }) + } + + private checkRefactorDiagnostics(file: NormalizedPath, project: Project): void { + const scriptInfo = project.getScriptInfoForNormalizedPath(file); + const refactorDiags = project.getLanguageService().getRefactorDiagnostics(file); + const normalizedDiags = this.normalizeRefactorDiagnostics(refactorDiags, scriptInfo); + + this.event({ file, diagnostics: normalizedDiags }, "refactorDiag"); + } + + private getRefactorDiagnosticsForRange(args: protocol.GetRefactorsForRangeRequestArgs): protocol.RefactorDiagnostic[] { + const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); + const scriptInfo = project.getScriptInfoForNormalizedPath(file); + const { startPosition, endPosition } = this.extractStartAndEndPositionFromTextRangeRequestArgs(args, scriptInfo); + + const range = { pos: startPosition, end: endPosition }; + const refactorDiags = project.getLanguageService().getRefactorDiagnostics(file, range); + return this.normalizeRefactorDiagnostics(refactorDiags, scriptInfo); + } + + private getCodeActionsForRefactorDiagnostic(args: protocol.GetCodeActionsForRefactorRequestArgs): protocol.CodeAction[] { + const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); + const scriptInfo = project.getScriptInfoForNormalizedPath(file); + const { startPosition, endPosition } = this.extractStartAndEndPositionFromTextRangeRequestArgs(args, scriptInfo); + + const actions = project.getLanguageService().getCodeActionsForRefactorAtPosition( + file, + { pos: startPosition, end: endPosition }, + args.refactorCode, + this.projectService.getFormatCodeOptions() + ); + + return map(actions, action => this.mapCodeAction(action, scriptInfo)); + } + private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): protocol.CodeAction[] | CodeAction[] { if (args.errorCodes.length === 0) { return undefined; @@ -1418,8 +1482,7 @@ namespace ts.server { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const startPosition = getStartPosition(); - const endPosition = getEndPosition(); + const { startPosition, endPosition } = this.extractStartAndEndPositionFromTextRangeRequestArgs(args, scriptInfo); const codeActions = project.getLanguageService().getCodeFixesAtPosition(file, startPosition, endPosition, args.errorCodes); if (!codeActions) { @@ -1431,14 +1494,6 @@ namespace ts.server { else { return codeActions; } - - function getStartPosition() { - return args.startPosition !== undefined ? args.startPosition : scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); - } - - function getEndPosition() { - return args.endPosition !== undefined ? args.endPosition : scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); - } } private mapCodeAction(codeAction: CodeAction, scriptInfo: ScriptInfo): protocol.CodeAction { @@ -1469,8 +1524,8 @@ namespace ts.server { return !spans ? undefined : simplifiedResult - ? spans.map(span => this.decorateSpan(span, scriptInfo)) - : spans; + ? spans.map(span => this.decorateSpan(span, scriptInfo)) + : spans; } private getDiagnosticsForProject(next: NextStep, delay: number, fileName: string): void { @@ -1771,6 +1826,12 @@ namespace ts.server { }, [CommandNames.GetSupportedCodeFixes]: () => { return this.requiredResponse(this.getSupportedCodeFixes()); + }, + [CommandNames.GetRefactorsForRange]: (request: protocol.GetRefactorsForRangeRequest) => { + return this.requiredResponse(this.getRefactorDiagnosticsForRange(request.arguments)); + }, + [CommandNames.GetCodeActionsForRefactor]: (request: protocol.GetCodeActionsForRefactorRequest) => { + return this.requiredResponse(this.getCodeActionsForRefactorDiagnostic(request.arguments)); } }); @@ -1828,7 +1889,7 @@ namespace ts.server { let request: protocol.Request; try { request = JSON.parse(message); - const {response, responseRequired} = this.executeCommand(request); + const { response, responseRequired } = this.executeCommand(request); if (this.logger.hasLevel(LogLevel.requestTime)) { const elapsedTime = hrTimeToMilliseconds(this.hrtime(start)).toFixed(4); diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts new file mode 100644 index 0000000000000..d33ccb6d5dcec --- /dev/null +++ b/src/services/refactorProvider.ts @@ -0,0 +1,92 @@ +/* @internal */ +namespace ts { + interface BaseRefactor { + /** Description of the refactor to display in the UI of the editor */ + description: string; + + /** An unique code associated with each refactor */ + refactorCode: number; + + /** Compute the associated code actions */ + getCodeActions(range: TextRange, context: RefactorContext): CodeAction[]; + } + + export interface SuggestableRefactor extends BaseRefactor { + canBeSuggested: true; + isApplicableForNode(node: Node, context: RefactorContext): boolean; + } + + export interface NonSuggestableRefactor extends BaseRefactor { + canBeSuggested: false; + + /** A fast syntactic check to see if the refactor is applicable at given position. */ + isApplicableForRange(range: TextRange, context: LightRefactorContext): boolean; + } + + export type Refactor = SuggestableRefactor | NonSuggestableRefactor; + + export interface LightRefactorContext { + nonBoundSourceFile: SourceFile; + newLineCharacter: string; + } + + export interface RefactorContext { + boundSourceFile: SourceFile; + program: Program; + newLineCharacter: string; + rulesProvider: formatting.RulesProvider; + } + + export namespace refactor { + // A map with the refactor code as key, the refactor itself as value + const suggestableRefactors: SuggestableRefactor[] = []; + const nonSuggestableRefactors: NonSuggestableRefactor[] = []; + + export function registerRefactor(refactor: Refactor) { + switch (refactor.canBeSuggested) { + case true: + suggestableRefactors[refactor.refactorCode] = refactor; + break; + case false: + nonSuggestableRefactors[refactor.refactorCode] = refactor; + break; + } + } + + export function getRefactorDiagnosticsForRange(range: TextRange, context: LightRefactorContext) { + const results: RefactorDiagnostic[] = []; + for (const code in nonSuggestableRefactors) { + const refactor = nonSuggestableRefactors[code]; + if (refactor && refactor.isApplicableForRange(range, context)) { + results.push(createRefactorDiagnostic(refactor, range)); + } + } + return results; + } + + export function getSuggestedRefactorDiagnosticsForNode(node: Node, context: RefactorContext) { + const result: RefactorDiagnostic[] = []; + for (const code in suggestableRefactors) { + const refactor = suggestableRefactors[code]; + if (refactor.isApplicableForNode(node, context)) { + result.push(createRefactorDiagnostic(refactor, node)); + } + } + return result; + } + + function createRefactorDiagnostic(refactor: Refactor, range: TextRange) { + return { + code: refactor.refactorCode, + start: range.pos, + end: range.end, + text: refactor.description + }; + } + + export function getCodeActionsForRefactor(refactorCode: number, range: TextRange, context: RefactorContext): CodeAction[] { + const refactor = suggestableRefactors[refactorCode] || nonSuggestableRefactors[refactorCode]; + return refactor && refactor.getCodeActions(range, context); + } + } +} diff --git a/src/services/services.ts b/src/services/services.ts index df5330ebaed9f..f7585e532fce6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -25,6 +25,7 @@ /// /// /// +/// /// namespace ts { @@ -1898,11 +1899,56 @@ namespace ts { return Rename.getRenameInfo(program.getTypeChecker(), defaultLibFileName, getCanonicalFileName, getValidSourceFile(fileName), position); } + function getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[] { + synchronizeHostData(); + const newLineCharacter = host.getNewLine(); + if (range) { + const nonBoundSourceFile = getNonBoundSourceFile(fileName); + const context: LightRefactorContext = { newLineCharacter, nonBoundSourceFile }; + return refactor.getRefactorDiagnosticsForRange(range, context); + } + + const boundSourceFile = getValidSourceFile(fileName); + const program = getProgram(); + const context: RefactorContext = { boundSourceFile, newLineCharacter, program, rulesProvider: ruleProvider }; + const result: RefactorDiagnostic[] = []; + + forEachChild(boundSourceFile, visitor); + return result; + + function visitor(node: Node): void { + const diags = refactor.getSuggestedRefactorDiagnosticsForNode(node, context); + if (diags.length > 0) { + addRange(result, diags); + } + + forEachChild(node, visitor); + } + } + + function getCodeActionsForRefactorAtPosition( + fileName: string, + range: TextRange, + refactorCode: number, + formatOptions: FormatCodeSettings): CodeAction[] { + + const context: RefactorContext = { + boundSourceFile: getValidSourceFile(fileName), + newLineCharacter: host.getNewLine(), + program: getProgram(), + rulesProvider: getRuleProvider(formatOptions) + }; + + return refactor.getCodeActionsForRefactor(refactorCode, range, context); + } + return { dispose, cleanupSemanticCache, getSyntacticDiagnostics, getSemanticDiagnostics, + getRefactorDiagnostics, + getCodeActionsForRefactorAtPosition, getCompilerOptionsDiagnostics, getSyntacticClassifications, getSemanticClassifications, diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 93f861e80aa58..13355c537aac8 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -79,6 +79,7 @@ "formatting/rulesProvider.ts", "formatting/smartIndenter.ts", "formatting/tokenRange.ts", + "refactorProvider.ts", "codeFixProvider.ts", "codefixes/fixAddMissingMember.ts", "codefixes/fixExtendsInterfaceBecomesImplements.ts", diff --git a/src/services/types.ts b/src/services/types.ts index c7852db6d6eac..5ee5b27c286d5 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -250,6 +250,9 @@ namespace ts { getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[]): CodeAction[]; + getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[]; + getCodeActionsForRefactorAtPosition(fileName: string, range: TextRange, refactorCode: number, formatOptions: FormatCodeSettings): CodeAction[]; + getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput; getProgram(): Program; @@ -340,6 +343,13 @@ namespace ts { changes: FileTextChanges[]; } + export interface RefactorDiagnostic { + start: number; + end: number; + text: string; + code: number; + } + export interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index ab83752d93bd0..e0efef2cb1c3f 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -148,6 +148,7 @@ declare namespace FourSlashInterface { implementationListIsEmpty(): void; isValidBraceCompletionAtPosition(openingBrace?: string): void; codeFixAvailable(): void; + refactorAvailable(): void; } class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; @@ -227,6 +228,7 @@ declare namespace FourSlashInterface { DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean): void; noDocCommentTemplate(): void; rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void; + fileAfterApplyingRefactors(expectedText: string, formattingOptions?: FormatCodeOptions): void; importFixAtPosition(expectedTextArray: string[], errorCode?: number): void; navigationBar(json: any): void; From 3011d165b8a943485bae192be9bc7279373dca7f Mon Sep 17 00:00:00 2001 From: zhengbli Date: Wed, 15 Mar 2017 16:26:28 -0700 Subject: [PATCH 02/12] Grammar --- src/harness/unittests/tsserverProjectSystem.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index ff6e5b0309d41..417af6ab96a13 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3423,7 +3423,7 @@ namespace ts.projectSystem { // run first step host.runQueuedTimeoutCallbacks(); - assert.equal(host.getOutput().length, 1, "expect 1 messages"); + assert.equal(host.getOutput().length, 1, "expect 1 message"); const e1 = getMessage(0); assert.equal(e1.event, "syntaxDiag"); host.clearOutput(); @@ -3445,14 +3445,14 @@ namespace ts.projectSystem { // run first step host.runQueuedTimeoutCallbacks(); - assert.equal(host.getOutput().length, 1, "expect 1 messages"); + assert.equal(host.getOutput().length, 1, "expect 1 message"); const e1 = getMessage(0); assert.equal(e1.event, "syntaxDiag"); host.clearOutput(); // the semanticDiag message host.runQueuedImmediateCallbacks(); - assert.equal(host.getOutput().length, 1, "expect 1 messages"); + assert.equal(host.getOutput().length, 1, "expect 1 message"); const e2 = getMessage(0); assert.equal(e2.event, "semanticDiag"); host.clearOutput(); @@ -3475,7 +3475,7 @@ namespace ts.projectSystem { assert.equal(host.getOutput().length, 0, "expect 0 messages"); // run first step host.runQueuedTimeoutCallbacks(); - assert.equal(host.getOutput().length, 1, "expect 1 messages"); + assert.equal(host.getOutput().length, 1, "expect 1 message"); const e1 = getMessage(0); assert.equal(e1.event, "syntaxDiag"); host.clearOutput(); From fa5a483219bb77872388626148b7598ad406e2b6 Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Mon, 20 Mar 2017 16:14:24 -0700 Subject: [PATCH 03/12] cr feedbacks --- src/server/client.ts | 2 +- src/server/protocol.ts | 6 +++--- src/server/session.ts | 10 +++++++--- src/services/refactorProvider.ts | 9 ++++++++- src/services/services.ts | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/server/client.ts b/src/server/client.ts index 856a734d597b3..1f5724a0b9501 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -692,7 +692,7 @@ namespace ts.server { return response.body.map(entry => this.convertCodeActions(entry, fileName)); } - getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[] { + getRefactorDiagnostics(fileName: string, range: TextRange): RefactorDiagnostic[] { const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos); const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end); diff --git a/src/server/protocol.ts b/src/server/protocol.ts index c188f07c9ad32..8d8bfb3fe5edd 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -422,7 +422,7 @@ namespace ts.server.protocol { arguments: GetRefactorsForRangeRequestArgs; } - export interface GetRefactorsForRangeRequestArgs extends TextRangeRequestArgs { + export interface GetRefactorsForRangeRequestArgs extends FileRangeRequestArgs { } export interface GetRefactorsForRangeResponse extends Response { @@ -454,7 +454,7 @@ namespace ts.server.protocol { arguments: CodeFixRequestArgs; } - export interface TextRangeRequestArgs extends FileRequestArgs { + export interface FileRangeRequestArgs extends FileRequestArgs { /** * The line number for the request (1-based). */ @@ -491,7 +491,7 @@ namespace ts.server.protocol { /** * Instances of this interface specify errorcodes on a specific location in a sourcefile. */ - export interface CodeFixRequestArgs extends TextRangeRequestArgs { + export interface CodeFixRequestArgs extends FileRangeRequestArgs { /** * Errorcodes we want to get the fixes for. */ diff --git a/src/server/session.ts b/src/server/session.ts index 8e3dd0b29be9e..439cbeb016ca4 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1417,17 +1417,21 @@ namespace ts.server { return ts.getSupportedCodeFixes(); } - private extractStartAndEndPositionFromTextRangeRequestArgs(args: protocol.TextRangeRequestArgs, scriptInfo: ScriptInfo): { startPosition: number, endPosition: number } { + private extractStartAndEndPositionFromTextRangeRequestArgs(args: protocol.FileRangeRequestArgs, scriptInfo: ScriptInfo): { startPosition: number, endPosition: number } { const startPosition = getStartPosition(); const endPosition = getEndPosition(); return { startPosition, endPosition }; function getStartPosition() { - return args.startPosition !== undefined ? args.startPosition : scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); + return args.startPosition !== undefined + ? args.startPosition + : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); } function getEndPosition() { - return args.endPosition !== undefined ? args.endPosition : scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); + return args.endPosition !== undefined + ? args.endPosition + : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); } } diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index d33ccb6d5dcec..c12121c34d68e 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -7,6 +7,9 @@ namespace ts { /** An unique code associated with each refactor */ refactorCode: number; + /** Indicates whether the refactor can be suggested without the editor asking for it */ + canBeSuggested: boolean; + /** Compute the associated code actions */ getCodeActions(range: TextRange, context: RefactorContext): CodeAction[]; } @@ -26,6 +29,10 @@ namespace ts { export type Refactor = SuggestableRefactor | NonSuggestableRefactor; export interface LightRefactorContext { + /** + * The AST that was not bound, so the symbols associated with the nodes are not accessible. + * Such a source file should be cheap to get. + */ nonBoundSourceFile: SourceFile; newLineCharacter: string; } @@ -64,7 +71,7 @@ namespace ts { return results; } - export function getSuggestedRefactorDiagnosticsForNode(node: Node, context: RefactorContext) { + export function getSuggestableRefactorDiagnosticsForNode(node: Node, context: RefactorContext) { const result: RefactorDiagnostic[] = []; for (const code in suggestableRefactors) { const refactor = suggestableRefactors[code]; diff --git a/src/services/services.ts b/src/services/services.ts index e837d85e6830b..a756608eabc6b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1919,7 +1919,7 @@ namespace ts { return result; function visitor(node: Node): void { - const diags = refactor.getSuggestedRefactorDiagnosticsForNode(node, context); + const diags = refactor.getSuggestableRefactorDiagnosticsForNode(node, context); if (diags.length > 0) { addRange(result, diags); } From b357841fad9ed99cb75affb7091140ba1fd7e2a3 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Thu, 23 Mar 2017 23:39:06 -0700 Subject: [PATCH 04/12] Redesign APIs --- src/compiler/types.ts | 1 + src/harness/fourslash.ts | 134 +++++++++++++++++--------- src/harness/harnessLanguageService.ts | 7 +- src/server/client.ts | 65 +++++++------ src/server/protocol.ts | 89 ++++++++--------- src/server/session.ts | 105 ++++++++++---------- src/services/refactorProvider.ts | 99 +++++++++++-------- src/services/services.ts | 37 +++---- src/services/types.ts | 17 ++-- tests/cases/fourslash/fourslash.ts | 6 +- 10 files changed, 322 insertions(+), 238 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 86895f3db6e31..a779569439458 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3301,6 +3301,7 @@ namespace ts { Warning, Error, Message, + Refactor } export enum ModuleResolutionKind { diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index dac7bb2d39848..cee58b82aadc1 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -489,8 +489,8 @@ namespace FourSlash { return diagnostics; } - private getRefactorDiagnostics(fileName: string, range?: ts.TextRange): ts.RefactorDiagnostic[] { - return this.languageService.getRefactorDiagnostics(fileName, range); + private getRefactorDiagnostics(fileName: string): ts.Diagnostic[] { + return this.languageService.getRefactorDiagnostics(fileName); } private getAllDiagnostics(): ts.Diagnostic[] { @@ -2200,20 +2200,15 @@ namespace FourSlash { return actions; } - private getRefactorActions(fileName: string, range?: ts.TextRange, formattingOptions?: ts.FormatCodeSettings): ts.CodeAction[] { - const diagnostics = this.getRefactorDiagnostics(fileName, range); - const actions: ts.CodeAction[] = []; - formattingOptions = formattingOptions || this.formatCodeSettings; + private applyAllCodeActions(fileName: string, codeActions: ts.CodeAction[]): void { + if (!codeActions) { + return; + } - for (const diagnostic of diagnostics) { - const diagnosticRange: ts.TextRange = { - pos: diagnostic.start, - end: diagnostic.end - }; - const newActions = this.languageService.getCodeActionsForRefactorAtPosition(fileName, diagnosticRange, diagnostic.code, formattingOptions); - actions.push.apply(actions, newActions); + for (const codeAction of codeActions) { + const fileChanges = ts.find(codeAction.changes, change => change.fileName === fileName); + this.applyEdits(fileChanges.fileName, fileChanges.textChanges, /*isFormattingEdit*/ false); } - return actions; } private applyCodeAction(fileName: string, actions: ts.CodeAction[], index?: number): void { @@ -2574,42 +2569,79 @@ namespace FourSlash { } } - public verifyRefactorAvailable(negative: boolean) { - // The ranges are used only when the refactors require a range as input information. For example the "extractMethod" refactor - // onlye one range is allowed per test - const ranges = this.getRanges(); - if (ranges.length > 1) { - throw new Error("only one refactor range is allowed per test"); + public verifyRefactorDiagnosticsAvailableAtMarker(negative: boolean, markerName: string, diagnosticCode?: number) { + const marker = this.getMarkerByName(markerName); + const markerPos = marker.position; + let foundDiagnostic = false; + + const refactorDiagnostics = this.getRefactorDiagnostics(this.activeFile.fileName); + for (const diag of refactorDiagnostics) { + if (diag.start <= markerPos && diag.start + diag.length >= markerPos) { + foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code; + } + } + + if (negative && foundDiagnostic) { + this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected no refactor diagnostic at marker ${markerName} but found some.`); } + if (!negative && !foundDiagnostic) { + this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected a refactor diagnostic at marker ${markerName} but found none.`); + } + } - const range = ranges[0] ? { pos: ranges[0].start, end: ranges[0].end } : undefined; - const refactorDiagnostics = this.getRefactorDiagnostics(this.activeFile.fileName, range); - if (negative && refactorDiagnostics.length > 0) { - this.raiseError(`verifyRefactorAvailable failed - expected no refactors but found some.`); + public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) { + const marker = this.getMarkerByName(markerName); + const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position); + const isAvailable = applicableRefactors && applicableRefactors.length > 0; + if (negative && isAvailable) { + this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected no refactor at marker ${markerName} but found some.`); } - if (!negative && refactorDiagnostics.length === 0) { - this.raiseError(`verifyRefactorAvailable failed: expected refactor diagnostics but none found.`); + if (!negative && !isAvailable) { + this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected a refactor at marker ${markerName} but found none.`); } } - public verifyFileAfterApplyingRefactors(expectedContent: string, formattingOptions?: ts.FormatCodeSettings) { + public verifyApplicableRefactorAvailableForRange(negative: boolean) { const ranges = this.getRanges(); - if (ranges.length > 1) { - throw new Error("only one refactor range is allowed per test"); - } - - const range = ranges[0] ? { pos: ranges[0].start, end: ranges[0].end } : undefined; - const actions = this.getRefactorActions(this.activeFile.fileName, range, formattingOptions); - - // Each refactor diagnostic will return one code action, but multiple refactor diagnostics can point to the same - // code action. For example in the case of "convert function to es6 class": - // - // function foo() { } - // ^^^ - // foo.prototype.getName = function () { return "name"; }; - // ^^^ - // These two diagnostics result in the same code action, so we only apply the first one. - this.applyCodeAction(this.activeFile.fileName, actions, /*index*/ 0); + if (!(ranges && ranges.length === 1)) { + throw new Error("Exactly one refactor range is allowed per test."); + } + + const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, { pos: ranges[0].start, end: ranges[0].end }); + const isAvailable = applicableRefactors && applicableRefactors.length > 0; + if (negative && isAvailable) { + this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`); + } + if (!negative && !isAvailable) { + this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`); + } + } + + public verifyFileAfterApplyingRefactorAtMarker( + markerName: string, + expectedContent: string, + refactorKindToApply?: ts.RefactorKind, + formattingOptions?: ts.FormatCodeSettings) { + + formattingOptions = formattingOptions || this.formatCodeSettings; + const markerPos = this.getMarkerByName(markerName).position; + const diagnostics = this.getRefactorDiagnostics(this.activeFile.fileName); + + const diagnosticCodesAtMarker: number[] = []; + for (const diagnostic of diagnostics) { + if (diagnostic.start <= markerPos && diagnostic.start + diagnostic.length >= markerPos) { + diagnosticCodesAtMarker.push(diagnostic.code); + } + } + + const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos); + const applicableRefactorKinds = + ts.map(ts.filter(applicableRefactors, ar => refactorKindToApply === undefined || refactorKindToApply === ar.refactorKind), + refactorInfo => refactorInfo.refactorKind); + const codeActions = this.languageService.getRefactorCodeActions( + this.activeFile.fileName, formattingOptions, markerPos, applicableRefactorKinds, diagnosticCodesAtMarker); + + this.applyAllCodeActions(this.activeFile.fileName, codeActions); const actualContent = this.getFileContent(this.activeFile.fileName); if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) { @@ -3409,8 +3441,16 @@ namespace FourSlashInterface { this.state.verifyCodeFixAvailable(this.negative); } - public refactorAvailable() { - this.state.verifyRefactorAvailable(this.negative); + public refactorDiagnosticsAvailableAtMarker(markerName: string, refactorCode?: number) { + this.state.verifyRefactorDiagnosticsAvailableAtMarker(this.negative, markerName, refactorCode); + } + + public applicableRefactorAvailableAtMarker(markerName: string) { + this.state.verifyApplicableRefactorAvailableAtMarker(this.negative, markerName); + } + + public applicableRefactorAvailableForRange(){ + this.state.verifyApplicableRefactorAvailableForRange(this.negative); } } @@ -3618,8 +3658,8 @@ namespace FourSlashInterface { this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index); } - public fileAfterApplyingRefactors(expectedContent: string, formattingOptions: ts.FormatCodeSettings): void { - this.state.verifyFileAfterApplyingRefactors(expectedContent, formattingOptions); + public fileAfterApplyingRefactorsAtMarker(markerName: string, expectedContent: string, refactorKindToApply?: ts.RefactorKind, formattingOptions?: ts.FormatCodeSettings): void { + this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorKindToApply, formattingOptions); } public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void { diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 8f35f0c33fa52..6e466f2efd5a5 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -489,10 +489,13 @@ namespace Harness.LanguageService { getCodeFixesAtPosition(): ts.CodeAction[] { throw new Error("Not supported on the shim."); } - getRefactorDiagnostics(): ts.RefactorDiagnostic[] { + getRefactorDiagnostics(): ts.Diagnostic[] { throw new Error("Not supported on the shim."); } - getCodeActionsForRefactorAtPosition(): ts.CodeAction[] { + getRefactorCodeActions(): ts.CodeAction[] { + throw new Error("Not supported on the shim."); + } + getApplicableRefactors(): ts.ApplicableRefactorInfo[] { throw new Error("Not supported on the shim."); } getEmitOutput(fileName: string): ts.EmitOutput { diff --git a/src/server/client.ts b/src/server/client.ts index 1f5724a0b9501..40b27a4953764 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -692,48 +692,53 @@ namespace ts.server { return response.body.map(entry => this.convertCodeActions(entry, fileName)); } - getRefactorDiagnostics(fileName: string, range: TextRange): RefactorDiagnostic[] { - const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos); - const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end); + getRefactorDiagnostics(_fileName: string): Diagnostic[] { + return notImplemented(); + } - const args: protocol.GetRefactorsForRangeRequestArgs = { + private positionOrRangeToLocationOrSpan(positionOrRange: number | TextRange, fileName: string) { + let locationOrSpan: protocol.LocationOrSpanWithPosition; + if (typeof positionOrRange === "number") { + locationOrSpan = this.positionToOneBasedLineOffset(fileName, positionOrRange); + } + else { + locationOrSpan = positionOrRange + ? { start: this.positionToOneBasedLineOffset(fileName, positionOrRange.pos), end: this.positionToOneBasedLineOffset(fileName, positionOrRange.end) } + : undefined; + } + return locationOrSpan; + } + + getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] { + const args: protocol.Refactor.GetApplicableRefactorsRequestArgs = { file: fileName, - startLine: startLineOffset.line, - startOffset: startLineOffset.offset, - endLine: endLineOffset.line, - endOffset: endLineOffset.offset, + locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName) }; - const request = this.processRequest(CommandNames.GetRefactorsForRange, args); - const response = this.processResponse(request); + const request = this.processRequest(CommandNames.GetApplicableRefactors, args); + const response = this.processResponse(request); - return response.body.map(entry => { - return { - code: entry.code, - end: this.lineOffsetToPosition(fileName, entry.end), - start: this.lineOffsetToPosition(fileName, entry.start), - text: entry.text - }; - }); + return response.body; } - getCodeActionsForRefactorAtPosition(fileName: string, range: TextRange, refactorCode: number): CodeAction[] { - const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos); - const endLineOffset = this.positionToOneBasedLineOffset(fileName, range.end); + getRefactorCodeActions( + fileName: string, + _formatOptions: FormatCodeSettings, + positionOrRange: number | TextRange, + refactorKinds?: RefactorKind[], + diagnosticCodes?: number[]) { - const args: protocol.GetCodeActionsForRefactorRequestArgs = { + const args: protocol.Refactor.GetRefactorCodeActionsRequestArgs = { file: fileName, - startLine: startLineOffset.line, - startOffset: startLineOffset.offset, - endLine: endLineOffset.line, - endOffset: endLineOffset.offset, - refactorCode + locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName), + refactorKinds, + diagnosticCodes }; - const request = this.processRequest(CommandNames.GetCodeActionsForRefactor, args); - const response = this.processResponse(request); + const request = this.processRequest(CommandNames.GetRefactorCodeActions, args); + const codeActions = this.processResponse(request).body; - return response.body.map(entry => this.convertCodeActions(entry, fileName)); + return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName)); } convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction { diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 8d8bfb3fe5edd..b03033d88c73b 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -96,8 +96,8 @@ namespace ts.server.protocol { export type GetCodeFixesFull = "getCodeFixes-full"; export type GetSupportedCodeFixes = "getSupportedCodeFixes"; - export type GetRefactorsForRange = "getRefactorsForRange"; - export type GetCodeActionsForRefactor = "getCodeActionsForRefactor"; + export type GetApplicableRefactors = "getApplicableRefactors"; + export type GetRefactorCodeActions = "getRefactorCodeActions"; } /** @@ -397,53 +397,54 @@ namespace ts.server.protocol { position?: number; } - /** - * An diagnostic information suggesting refactors at applicable positions without - * clients asking. - */ - export interface RefactorDiagnostic { - text: string; - code: number; - start: Location; - end: Location; - } - - export interface RefactorDiagnosticEventBody { - file: string; - diagnostics: RefactorDiagnostic[]; - } - - /** - * Returns a list of applicable refactors at a given position. This request does not actually - * compute the refactors; instead it goes through faster checks to determine what is possible. - */ - export interface GetRefactorsForRangeRequest extends Request { - command: CommandTypes.GetRefactorsForRange; - arguments: GetRefactorsForRangeRequestArgs; - } + export type LocationOrSpanWithPosition = LocationWithPosition | { start: LocationWithPosition, end: LocationWithPosition }; - export interface GetRefactorsForRangeRequestArgs extends FileRangeRequestArgs { + export interface FileLocationOrSpanWithPositionRequestArgs extends FileRequestArgs { + locationOrSpan: LocationOrSpanWithPosition; } - export interface GetRefactorsForRangeResponse extends Response { - body?: RefactorDiagnostic[]; - } - - /** - * Computes the code actions for a given refactor. This is normally called after the user commited one - * of the applicable refactors provided by the "GetApplicableRefactors" API. - */ - export interface GetCodeActionsForRefactorRequest extends Request { - command: CommandTypes.GetCodeActionsForRefactor; - arguments: GetCodeActionsForRefactorRequestArgs; - } - - export interface GetCodeActionsForRefactorRequestArgs extends GetRefactorsForRangeRequestArgs { - refactorCode: number; + export interface LocationWithPosition extends Location { + position?: number; } - export interface GetCodeActionsForRefactorResponse extends Response { - body?: CodeAction[]; + export namespace Refactor { + export interface GetApplicableRefactorsRequest extends Request { + command: CommandTypes.GetApplicableRefactors; + arguments: GetApplicableRefactorsRequestArgs; + } + + export interface GetApplicableRefactorsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { + } + + export interface ApplicableRefactorInfo { + refactorKind: number; + description: string; + } + + export interface GetApplicableRefactorsResponse extends Response { + body?: ApplicableRefactorInfo[]; + } + + export interface GetRefactorCodeActionsRequest extends Request { + command: CommandTypes.GetRefactorCodeActions; + arguments: GetRefactorCodeActionsRequestArgs; + } + + export interface GetRefactorCodeActionsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { + /* The kind of the applicable refactor */ + refactorKinds?: number[]; + /* The diagnostic code of a refactor diagnostic */ + diagnosticCodes?: number[]; + } + + export interface GetRefactorCodeActionsResponse extends Response { + body?: CodeAction[]; + } + + export interface RefactorDiagnosticEventBody { + file: string; + diagnostics: Diagnostic[]; + } } /** diff --git a/src/server/session.ts b/src/server/session.ts index 439cbeb016ca4..24d8da964982a 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -191,8 +191,8 @@ namespace ts.server { export const GetCodeFixesFull: protocol.CommandTypes.GetCodeFixesFull = "getCodeFixes-full"; export const GetSupportedCodeFixes: protocol.CommandTypes.GetSupportedCodeFixes = "getSupportedCodeFixes"; - export const GetRefactorsForRange: protocol.CommandTypes.GetRefactorsForRange = "getRefactorsForRange"; - export const GetCodeActionsForRefactor: protocol.CommandTypes.GetCodeActionsForRefactor = "getCodeActionsForRefactor"; + export const GetApplicableRefactors: protocol.CommandTypes.GetApplicableRefactors = "getApplicableRefactors"; + export const GetRefactorCodeActions: protocol.CommandTypes.GetRefactorCodeActions = "getRefactorCodeActions"; } export function formatMessage(msg: T, logger: server.Logger, byteLength: (s: string, encoding: string) => number, newLine: string): string { @@ -505,7 +505,7 @@ namespace ts.server { next.immediate(() => { this.semanticCheck(checkSpec.fileName, checkSpec.project); next.immediate(() => { - this.checkRefactorDiagnostics(checkSpec.fileName, checkSpec.project); + this.refactorDiagnosticsCheck(checkSpec.fileName, checkSpec.project); if (checkList.length > index) { next.delay(followMs, checkOne); } @@ -1417,66 +1417,56 @@ namespace ts.server { return ts.getSupportedCodeFixes(); } - private extractStartAndEndPositionFromTextRangeRequestArgs(args: protocol.FileRangeRequestArgs, scriptInfo: ScriptInfo): { startPosition: number, endPosition: number } { - const startPosition = getStartPosition(); - const endPosition = getEndPosition(); - return { startPosition, endPosition }; - - function getStartPosition() { - return args.startPosition !== undefined - ? args.startPosition - : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); - } + private refactorDiagnosticsCheck(file: NormalizedPath, project: Project): void { + const refactorDiags = project.getLanguageService().getRefactorDiagnostics(file); + const diagnostics = refactorDiags.map(d => formatDiag(file, project, d)); - function getEndPosition() { - return args.endPosition !== undefined - ? args.endPosition - : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); - } + this.event({ file, diagnostics }, "refactorDiag"); } - private normalizeRefactorDiagnostics(refactorDiags: RefactorDiagnostic[], scriptInfo: ScriptInfo): protocol.RefactorDiagnostic[] { - return map(refactorDiags, refactorDiag => { - return { - code: refactorDiag.code, - text: refactorDiag.text, - start: scriptInfo.positionToLineOffset(refactorDiag.start), - end: scriptInfo.positionToLineOffset(refactorDiag.end) - }; - }) + private isLocation(locationOrSpan: protocol.LocationOrSpanWithPosition): locationOrSpan is protocol.LocationWithPosition { + return (locationOrSpan).start === undefined; } - private checkRefactorDiagnostics(file: NormalizedPath, project: Project): void { - const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const refactorDiags = project.getLanguageService().getRefactorDiagnostics(file); - const normalizedDiags = this.normalizeRefactorDiagnostics(refactorDiags, scriptInfo); + private extractPositionAndRange(args: protocol.FileLocationOrSpanWithPositionRequestArgs, scriptInfo: ScriptInfo): { position: number, textRange: TextRange } { + let position: number; + let textRange: TextRange; + if (this.isLocation(args.locationOrSpan)) { + position = getPosition(args.locationOrSpan); + } + else { + textRange = { + pos: getPosition(args.locationOrSpan.start), + end: getPosition(args.locationOrSpan.end) + }; + } + return { position, textRange }; - this.event({ file, diagnostics: normalizedDiags }, "refactorDiag"); + function getPosition(loc: protocol.LocationWithPosition) { + return loc.position !== undefined ? loc.position : scriptInfo.lineOffsetToPosition(loc.line, loc.offset); + } } - private getRefactorDiagnosticsForRange(args: protocol.GetRefactorsForRangeRequestArgs): protocol.RefactorDiagnostic[] { + private getApplicableRefactors(args: protocol.Refactor.GetApplicableRefactorsRequestArgs): protocol.Refactor.ApplicableRefactorInfo[] { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const { startPosition, endPosition } = this.extractStartAndEndPositionFromTextRangeRequestArgs(args, scriptInfo); - - const range = { pos: startPosition, end: endPosition }; - const refactorDiags = project.getLanguageService().getRefactorDiagnostics(file, range); - return this.normalizeRefactorDiagnostics(refactorDiags, scriptInfo); + const { position, textRange } = this.extractPositionAndRange(args, scriptInfo); + return project.getLanguageService().getApplicableRefactors(file, position || textRange); } - private getCodeActionsForRefactorDiagnostic(args: protocol.GetCodeActionsForRefactorRequestArgs): protocol.CodeAction[] { + private getRefactorCodeActions(args: protocol.Refactor.GetRefactorCodeActionsRequestArgs): protocol.CodeAction[] { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const { startPosition, endPosition } = this.extractStartAndEndPositionFromTextRangeRequestArgs(args, scriptInfo); + const { position, textRange } = this.extractPositionAndRange(args, scriptInfo); - const actions = project.getLanguageService().getCodeActionsForRefactorAtPosition( + const result = project.getLanguageService().getRefactorCodeActions( file, - { pos: startPosition, end: endPosition }, - args.refactorCode, - this.projectService.getFormatCodeOptions() + this.projectService.getFormatCodeOptions(), + position || textRange, + args.refactorKinds, + args.diagnosticCodes ); - - return map(actions, action => this.mapCodeAction(action, scriptInfo)); + return map(result, action => this.mapCodeAction(action, scriptInfo)); } private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): protocol.CodeAction[] | CodeAction[] { @@ -1486,7 +1476,8 @@ namespace ts.server { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const { startPosition, endPosition } = this.extractStartAndEndPositionFromTextRangeRequestArgs(args, scriptInfo); + const startPosition = getStartPosition(); + const endPosition = getEndPosition(); const formatOptions = this.projectService.getFormatCodeOptions(file); const codeActions = project.getLanguageService().getCodeFixesAtPosition(file, startPosition, endPosition, args.errorCodes, formatOptions); @@ -1499,6 +1490,18 @@ namespace ts.server { else { return codeActions; } + + function getStartPosition() { + return args.startPosition !== undefined + ? args.startPosition + : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); + } + + function getEndPosition() { + return args.endPosition !== undefined + ? args.endPosition + : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); + } } private mapCodeAction(codeAction: CodeAction, scriptInfo: ScriptInfo): protocol.CodeAction { @@ -1832,11 +1835,11 @@ namespace ts.server { [CommandNames.GetSupportedCodeFixes]: () => { return this.requiredResponse(this.getSupportedCodeFixes()); }, - [CommandNames.GetRefactorsForRange]: (request: protocol.GetRefactorsForRangeRequest) => { - return this.requiredResponse(this.getRefactorDiagnosticsForRange(request.arguments)); + [CommandNames.GetApplicableRefactors]: (request: protocol.Refactor.GetApplicableRefactorsRequest) => { + return this.requiredResponse(this.getApplicableRefactors(request.arguments)); }, - [CommandNames.GetCodeActionsForRefactor]: (request: protocol.GetCodeActionsForRefactorRequest) => { - return this.requiredResponse(this.getCodeActionsForRefactorDiagnostic(request.arguments)); + [CommandNames.GetRefactorCodeActions]: (request: protocol.Refactor.GetRefactorCodeActionsRequest) => { + return this.requiredResponse(this.getRefactorCodeActions(request.arguments)); } }); diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index c12121c34d68e..d1f4640d9f3eb 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -1,29 +1,30 @@ /* @internal */ namespace ts { interface BaseRefactor { - /** Description of the refactor to display in the UI of the editor */ - description: string; - /** An unique code associated with each refactor */ - refactorCode: number; - + refactorKind: RefactorKind; /** Indicates whether the refactor can be suggested without the editor asking for it */ - canBeSuggested: boolean; - + suggestable: boolean; /** Compute the associated code actions */ - getCodeActions(range: TextRange, context: RefactorContext): CodeAction[]; + getCodeActions(context: RefactorContext, positionOrRange: number | TextRange): CodeAction[]; } export interface SuggestableRefactor extends BaseRefactor { - canBeSuggested: true; - isApplicableForNode(node: Node, context: RefactorContext): boolean; + suggestable: true; + /** + * The corresponding refactor diagnostic code this refactor generates. All suggestableRefactors + * generate one refactor diagnostic and handle it accordingly. + */ + diagnosticCode: number; + createRefactorDiagnosticIfApplicable(node: Node, context: RefactorContext): Diagnostic | undefined; } export interface NonSuggestableRefactor extends BaseRefactor { - canBeSuggested: false; - + suggestable: false; + /** Description of the refactor to display in the UI of the editor */ + description: string; /** A fast syntactic check to see if the refactor is applicable at given position. */ - isApplicableForRange(range: TextRange, context: LightRefactorContext): boolean; + isApplicableForPositionOrRange(positionOrRange: number | TextRange, context: LightRefactorContext): boolean; } export type Refactor = SuggestableRefactor | NonSuggestableRefactor; @@ -45,55 +46,77 @@ namespace ts { } export namespace refactor { - // A map with the refactor code as key, the refactor itself as value + // A map with the diagnostic code as key, the refactors themselves as value + // e.g. suggestableRefactors[diagnosticCode] -> the refactor you want const suggestableRefactors: SuggestableRefactor[] = []; + // A map with the refactor code as key, the refactor itself as value + // e.g. nonSuggestableRefactors[refactorCode] -> the refactor you want const nonSuggestableRefactors: NonSuggestableRefactor[] = []; export function registerRefactor(refactor: Refactor) { - switch (refactor.canBeSuggested) { + switch (refactor.suggestable) { case true: - suggestableRefactors[refactor.refactorCode] = refactor; + suggestableRefactors[refactor.diagnosticCode] = refactor; break; case false: - nonSuggestableRefactors[refactor.refactorCode] = refactor; + nonSuggestableRefactors[refactor.refactorKind] = refactor; break; } } - export function getRefactorDiagnosticsForRange(range: TextRange, context: LightRefactorContext) { - const results: RefactorDiagnostic[] = []; + export function getApplicableRefactors( + context: LightRefactorContext, + positionOrRange: number | TextRange): ApplicableRefactorInfo[] | undefined { + + let results: ApplicableRefactorInfo[]; for (const code in nonSuggestableRefactors) { const refactor = nonSuggestableRefactors[code]; - if (refactor && refactor.isApplicableForRange(range, context)) { - results.push(createRefactorDiagnostic(refactor, range)); + if (refactor.isApplicableForPositionOrRange(positionOrRange, context)) { + (results || (results = [])).push({ refactorKind: refactor.refactorKind, description: refactor.description }); } } return results; } - export function getSuggestableRefactorDiagnosticsForNode(node: Node, context: RefactorContext) { - const result: RefactorDiagnostic[] = []; - for (const code in suggestableRefactors) { - const refactor = suggestableRefactors[code]; - if (refactor.isApplicableForNode(node, context)) { - result.push(createRefactorDiagnostic(refactor, node)); + export function getRefactorDiagnosticsForNode(context: RefactorContext, node: Node): Diagnostic[] | undefined { + let result: Diagnostic[]; + for (const key in suggestableRefactors) { + const refactor = suggestableRefactors[key]; + const newDiag = refactor.createRefactorDiagnosticIfApplicable(node, context); + if (newDiag) { + (result || (result = [])).push(newDiag); } } return result; } - function createRefactorDiagnostic(refactor: Refactor, range: TextRange) { - return { - code: refactor.refactorCode, - start: range.pos, - end: range.end, - text: refactor.description - }; - } + export function getRefactorCodeActions( + context: RefactorContext, + positionOrRange: number | TextRange, + refactorKinds?: RefactorKind[], + diagnosticCodes?: number[]) { - export function getCodeActionsForRefactor(refactorCode: number, range: TextRange, context: RefactorContext): CodeAction[] { - const refactor = suggestableRefactors[refactorCode] || nonSuggestableRefactors[refactorCode]; - return refactor && refactor.getCodeActions(range, context); + let result: CodeAction[] = []; + if (refactorKinds !== undefined) { + for (const refactorKind of refactorKinds) { + const refactor = nonSuggestableRefactors[refactorKind]; + const codeActions = refactor.getCodeActions(context, positionOrRange); + if (codeActions) { + addRange(result, codeActions); + } + } + } + if (diagnosticCodes !== undefined) { + for (const diagnosticCode of diagnosticCodes) { + const refactor = suggestableRefactors[diagnosticCode]; + const codeActions = refactor && refactor.getCodeActions(context, positionOrRange); + if (codeActions) { + addRange(result, codeActions); + } + } + } + + return result; } } } diff --git a/src/services/services.ts b/src/services/services.ts index a756608eabc6b..7fe64dafe32c6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1901,26 +1901,19 @@ namespace ts { return Rename.getRenameInfo(program.getTypeChecker(), defaultLibFileName, getCanonicalFileName, getValidSourceFile(fileName), position); } - function getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[] { - synchronizeHostData(); + function getRefactorDiagnostics(fileName: string): Diagnostic[] { const newLineCharacter = host.getNewLine(); - if (range) { - const nonBoundSourceFile = getNonBoundSourceFile(fileName); - const context: LightRefactorContext = { newLineCharacter, nonBoundSourceFile }; - return refactor.getRefactorDiagnosticsForRange(range, context); - } - const boundSourceFile = getValidSourceFile(fileName); const program = getProgram(); const context: RefactorContext = { boundSourceFile, newLineCharacter, program, rulesProvider: ruleProvider }; - const result: RefactorDiagnostic[] = []; + const result: Diagnostic[] = []; forEachChild(boundSourceFile, visitor); return result; function visitor(node: Node): void { - const diags = refactor.getSuggestableRefactorDiagnosticsForNode(node, context); - if (diags.length > 0) { + const diags = refactor.getRefactorDiagnosticsForNode(context, node); + if (diags) { addRange(result, diags); } @@ -1928,11 +1921,20 @@ namespace ts { } } - function getCodeActionsForRefactorAtPosition( + function getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] { + synchronizeHostData(); + const newLineCharacter = host.getNewLine(); + const nonBoundSourceFile = getNonBoundSourceFile(fileName); + const context: LightRefactorContext = { newLineCharacter, nonBoundSourceFile }; + return refactor.getApplicableRefactors(context, positionOrRange); + } + + function getRefactorCodeActions( fileName: string, - range: TextRange, - refactorCode: number, - formatOptions: FormatCodeSettings): CodeAction[] { + formatOptions: FormatCodeSettings, + positionOrRange: number | TextRange, + refactorKinds?: RefactorKind[], + diagnosticCodes?: number[]): CodeAction[] { const context: RefactorContext = { boundSourceFile: getValidSourceFile(fileName), @@ -1941,7 +1943,7 @@ namespace ts { rulesProvider: getRuleProvider(formatOptions) }; - return refactor.getCodeActionsForRefactor(refactorCode, range, context); + return refactor.getRefactorCodeActions(context, positionOrRange, refactorKinds, diagnosticCodes); } return { @@ -1950,7 +1952,8 @@ namespace ts { getSyntacticDiagnostics, getSemanticDiagnostics, getRefactorDiagnostics, - getCodeActionsForRefactorAtPosition, + getApplicableRefactors, + getRefactorCodeActions, getCompilerOptionsDiagnostics, getSyntacticClassifications, getSemanticClassifications, diff --git a/src/services/types.ts b/src/services/types.ts index a8915d4eb6e8b..1b64ab366efd2 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -194,6 +194,7 @@ namespace ts { getSyntacticDiagnostics(fileName: string): Diagnostic[]; getSemanticDiagnostics(fileName: string): Diagnostic[]; + getRefactorDiagnostics(fileName: string): Diagnostic[]; // TODO: Rename this to getProgramDiagnostics to better indicate that these are any // diagnostics present for the program level, and not just 'options' diagnostics. @@ -258,8 +259,9 @@ namespace ts { getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; - getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[]; - getCodeActionsForRefactorAtPosition(fileName: string, range: TextRange, refactorCode: number, formatOptions: FormatCodeSettings): CodeAction[]; + getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; + getRefactorCodeActions( + fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorKinds?: RefactorKind[], diagnosticCodes?: number[]): CodeAction[]; getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput; @@ -351,11 +353,12 @@ namespace ts { changes: FileTextChanges[]; } - export interface RefactorDiagnostic { - start: number; - end: number; - text: string; - code: number; + export interface ApplicableRefactorInfo { + refactorKind: RefactorKind; + description: string; + } + + export enum RefactorKind { } export interface TextInsertion { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index e0efef2cb1c3f..3b5636f0e6bd0 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -148,7 +148,9 @@ declare namespace FourSlashInterface { implementationListIsEmpty(): void; isValidBraceCompletionAtPosition(openingBrace?: string): void; codeFixAvailable(): void; - refactorAvailable(): void; + applicableRefactorAvailableAtMarker(markerName: string): void; + refactorDiagnosticsAvailableAtMarker(markerName: string, diagnosticCode?: number): void; + applicableRefactorAvailableForRange(): void; } class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; @@ -228,7 +230,7 @@ declare namespace FourSlashInterface { DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean): void; noDocCommentTemplate(): void; rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void; - fileAfterApplyingRefactors(expectedText: string, formattingOptions?: FormatCodeOptions): void; + fileAfterApplyingRefactorAtMarker(markerName: string, expectedText: string, refactorKindToApply?: number, formattingOptions?: FormatCodeOptions): void; importFixAtPosition(expectedTextArray: string[], errorCode?: number): void; navigationBar(json: any): void; From 675cf0323078077557e43f2052c3fa4e87aa4294 Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Mon, 27 Mar 2017 00:53:50 -0700 Subject: [PATCH 05/12] fix linting issues --- src/harness/fourslash.ts | 6 +++--- src/server/session.ts | 2 +- src/services/refactorProvider.ts | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index d41d73bc439e4..18e53626d5534 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2610,7 +2610,7 @@ namespace FourSlash { foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code; } } - + if (negative && foundDiagnostic) { this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected no refactor diagnostic at marker ${markerName} but found some.`); } @@ -2665,7 +2665,7 @@ namespace FourSlash { } const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos); - const applicableRefactorKinds = + const applicableRefactorKinds = ts.map(ts.filter(applicableRefactors, ar => refactorKindToApply === undefined || refactorKindToApply === ar.refactorKind), refactorInfo => refactorInfo.refactorKind); const codeActions = this.languageService.getRefactorCodeActions( @@ -3479,7 +3479,7 @@ namespace FourSlashInterface { this.state.verifyApplicableRefactorAvailableAtMarker(this.negative, markerName); } - public applicableRefactorAvailableForRange(){ + public applicableRefactorAvailableForRange() { this.state.verifyApplicableRefactorAvailableForRange(this.negative); } } diff --git a/src/server/session.ts b/src/server/session.ts index 6b8b95a87d911..442b0b86b28cb 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -508,7 +508,7 @@ namespace ts.server { if (checkList.length > index) { next.delay(followMs, checkOne); } - }) + }); }); } } diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index d1f4640d9f3eb..47c449aca1c4a 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -1,4 +1,4 @@ -/* @internal */ +/* @internal */ namespace ts { interface BaseRefactor { /** An unique code associated with each refactor */ @@ -30,7 +30,7 @@ namespace ts { export type Refactor = SuggestableRefactor | NonSuggestableRefactor; export interface LightRefactorContext { - /** + /** * The AST that was not bound, so the symbols associated with the nodes are not accessible. * Such a source file should be cheap to get. */ @@ -96,7 +96,7 @@ namespace ts { refactorKinds?: RefactorKind[], diagnosticCodes?: number[]) { - let result: CodeAction[] = []; + const result: CodeAction[] = []; if (refactorKinds !== undefined) { for (const refactorKind of refactorKinds) { const refactor = nonSuggestableRefactors[refactorKind]; From f108fd77d24412fe7df76fc4719ac91f93a24830 Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Tue, 28 Mar 2017 16:25:44 -0700 Subject: [PATCH 06/12] replace the 'suggestable' refactors with code fixes --- src/harness/fourslash.ts | 47 +++++--------- src/harness/harnessLanguageService.ts | 2 +- src/server/client.ts | 12 ++-- src/server/protocol.ts | 10 ++- src/server/session.ts | 5 +- src/services/codeFixProvider.ts | 30 ++++++++- src/services/refactorProvider.ts | 91 +++++++-------------------- src/services/services.ts | 13 ++-- src/services/types.ts | 10 +-- 9 files changed, 84 insertions(+), 136 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 18e53626d5534..a527f0dea99ad 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -489,8 +489,8 @@ namespace FourSlash { return diagnostics; } - private getRefactorDiagnostics(fileName: string): ts.Diagnostic[] { - return this.languageService.getRefactorDiagnostics(fileName); + private getCodeFixDiagnostics(fileName: string): ts.Diagnostic[] { + return this.languageService.getCodeFixDiagnostics(fileName); } private getAllDiagnostics(): ts.Diagnostic[] { @@ -2231,17 +2231,6 @@ namespace FourSlash { return actions; } - private applyAllCodeActions(fileName: string, codeActions: ts.CodeAction[]): void { - if (!codeActions) { - return; - } - - for (const codeAction of codeActions) { - const fileChanges = ts.find(codeAction.changes, change => change.fileName === fileName); - this.applyEdits(fileChanges.fileName, fileChanges.textChanges, /*isFormattingEdit*/ false); - } - } - private applyCodeAction(fileName: string, actions: ts.CodeAction[], index?: number): void { if (index === undefined) { if (!(actions && actions.length === 1)) { @@ -2599,12 +2588,12 @@ namespace FourSlash { } } - public verifyRefactorDiagnosticsAvailableAtMarker(negative: boolean, markerName: string, diagnosticCode?: number) { + public verifyCodeFixDiagnosticsAvailableAtMarker(negative: boolean, markerName: string, diagnosticCode?: number) { const marker = this.getMarkerByName(markerName); const markerPos = marker.position; let foundDiagnostic = false; - const refactorDiagnostics = this.getRefactorDiagnostics(this.activeFile.fileName); + const refactorDiagnostics = this.getCodeFixDiagnostics(this.activeFile.fileName); for (const diag of refactorDiagnostics) { if (diag.start <= markerPos && diag.start + diag.length >= markerPos) { foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code; @@ -2650,28 +2639,22 @@ namespace FourSlash { public verifyFileAfterApplyingRefactorAtMarker( markerName: string, expectedContent: string, - refactorKindToApply?: ts.RefactorKind, + refactorNameToApply: string, formattingOptions?: ts.FormatCodeSettings) { formattingOptions = formattingOptions || this.formatCodeSettings; const markerPos = this.getMarkerByName(markerName).position; - const diagnostics = this.getRefactorDiagnostics(this.activeFile.fileName); - const diagnosticCodesAtMarker: number[] = []; - for (const diagnostic of diagnostics) { - if (diagnostic.start <= markerPos && diagnostic.start + diagnostic.length >= markerPos) { - diagnosticCodesAtMarker.push(diagnostic.code); - } + const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos); + const applicableRefactorToApply = ts.find(applicableRefactors, refactor => refactor.refactorName === refactorNameToApply); + + if (!applicableRefactorToApply) { + this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`); } - const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, markerPos); - const applicableRefactorKinds = - ts.map(ts.filter(applicableRefactors, ar => refactorKindToApply === undefined || refactorKindToApply === ar.refactorKind), - refactorInfo => refactorInfo.refactorKind); - const codeActions = this.languageService.getRefactorCodeActions( - this.activeFile.fileName, formattingOptions, markerPos, applicableRefactorKinds, diagnosticCodesAtMarker); + const codeActions = this.languageService.getRefactorCodeActions(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply); - this.applyAllCodeActions(this.activeFile.fileName, codeActions); + this.applyCodeAction(this.activeFile.fileName, codeActions); const actualContent = this.getFileContent(this.activeFile.fileName); if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) { @@ -3472,7 +3455,7 @@ namespace FourSlashInterface { } public refactorDiagnosticsAvailableAtMarker(markerName: string, refactorCode?: number) { - this.state.verifyRefactorDiagnosticsAvailableAtMarker(this.negative, markerName, refactorCode); + this.state.verifyCodeFixDiagnosticsAvailableAtMarker(this.negative, markerName, refactorCode); } public applicableRefactorAvailableAtMarker(markerName: string) { @@ -3688,8 +3671,8 @@ namespace FourSlashInterface { this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index); } - public fileAfterApplyingRefactorsAtMarker(markerName: string, expectedContent: string, refactorKindToApply?: ts.RefactorKind, formattingOptions?: ts.FormatCodeSettings): void { - this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorKindToApply, formattingOptions); + public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: ts.FormatCodeSettings): void { + this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, formattingOptions); } public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void { diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 6e466f2efd5a5..7e7fe2d73cfd9 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -489,7 +489,7 @@ namespace Harness.LanguageService { getCodeFixesAtPosition(): ts.CodeAction[] { throw new Error("Not supported on the shim."); } - getRefactorDiagnostics(): ts.Diagnostic[] { + getCodeFixDiagnostics(): ts.Diagnostic[] { throw new Error("Not supported on the shim."); } getRefactorCodeActions(): ts.CodeAction[] { diff --git a/src/server/client.ts b/src/server/client.ts index 0aa7036a8a836..deadf52f3fbf1 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -694,7 +694,7 @@ namespace ts.server { return response.body.map(entry => this.convertCodeActions(entry, fileName)); } - getRefactorDiagnostics(_fileName: string): Diagnostic[] { + getCodeFixDiagnostics(_fileName: string): Diagnostic[] { return notImplemented(); } @@ -720,25 +720,23 @@ namespace ts.server { const request = this.processRequest(CommandNames.GetApplicableRefactors, args); const response = this.processResponse(request); - return response.body; + return response.body.refactors; } getRefactorCodeActions( fileName: string, _formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, - refactorKinds?: RefactorKind[], - diagnosticCodes?: number[]) { + refactorName: string) { const args: protocol.Refactor.GetRefactorCodeActionsRequestArgs = { file: fileName, locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName), - refactorKinds, - diagnosticCodes + refactorName }; const request = this.processRequest(CommandNames.GetRefactorCodeActions, args); - const codeActions = this.processResponse(request).body; + const codeActions = this.processResponse(request).body.actions; return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName)); } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index c8aac79e56c51..bf0c956ae2282 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -417,12 +417,12 @@ namespace ts.server.protocol { } export interface ApplicableRefactorInfo { - refactorKind: number; + refactorName: string; description: string; } export interface GetApplicableRefactorsResponse extends Response { - body?: ApplicableRefactorInfo[]; + body?: { refactors: ApplicableRefactorInfo[] }; } export interface GetRefactorCodeActionsRequest extends Request { @@ -432,13 +432,11 @@ namespace ts.server.protocol { export interface GetRefactorCodeActionsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { /* The kind of the applicable refactor */ - refactorKinds?: number[]; - /* The diagnostic code of a refactor diagnostic */ - diagnosticCodes?: number[]; + refactorName: string; } export interface GetRefactorCodeActionsResponse extends Response { - body?: CodeAction[]; + body?: { actions: CodeAction[] }; } export interface RefactorDiagnosticEventBody { diff --git a/src/server/session.ts b/src/server/session.ts index 442b0b86b28cb..ca39356e9a090 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1422,7 +1422,7 @@ namespace ts.server { } private refactorDiagnosticsCheck(file: NormalizedPath, project: Project): void { - const refactorDiags = project.getLanguageService().getRefactorDiagnostics(file); + const refactorDiags = project.getLanguageService().getCodeFixDiagnostics(file); const diagnostics = refactorDiags.map(d => formatDiag(file, project, d)); this.event({ file, diagnostics }, "refactorDiag"); @@ -1467,8 +1467,7 @@ namespace ts.server { file, this.projectService.getFormatCodeOptions(), position || textRange, - args.refactorKinds, - args.diagnosticCodes + args.refactorName ); return map(result, action => this.mapCodeAction(action, scriptInfo)); } diff --git a/src/services/codeFixProvider.ts b/src/services/codeFixProvider.ts index bab5356e99c82..a04754ba28b92 100644 --- a/src/services/codeFixProvider.ts +++ b/src/services/codeFixProvider.ts @@ -3,6 +3,14 @@ namespace ts { export interface CodeFix { errorCodes: number[]; getCodeActions(context: CodeFixContext): CodeAction[] | undefined; + createCodeFixDiagnosticIfApplicable?(node: Node, context: CodeFixDiagnoseContext): Diagnostic | undefined; + } + + export interface CodeFixDiagnoseContext { + boundSourceFile: SourceFile; + program: Program; + newLineCharacter: string; + rulesProvider: formatting.RulesProvider; } export interface CodeFixContext { @@ -18,22 +26,38 @@ namespace ts { export namespace codefix { const codeFixes: CodeFix[][] = []; + const diagnosticGeneratingCodeFixes: CodeFix[] = []; - export function registerCodeFix(action: CodeFix) { - forEach(action.errorCodes, error => { + export function registerCodeFix(codeFix: CodeFix) { + forEach(codeFix.errorCodes, error => { let fixes = codeFixes[error]; if (!fixes) { fixes = []; codeFixes[error] = fixes; } - fixes.push(action); + fixes.push(codeFix); }); + + if (codeFix.createCodeFixDiagnosticIfApplicable) { + diagnosticGeneratingCodeFixes.push(codeFix); + } } export function getSupportedErrorCodes() { return Object.keys(codeFixes); } + export function getCodeFixDiagnosticsForNode(context: CodeFixDiagnoseContext, node: Node): Diagnostic[] | undefined { + let result: Diagnostic[]; + for (const codeFix of diagnosticGeneratingCodeFixes) { + const newDiag = codeFix.createCodeFixDiagnosticIfApplicable(node, context); + if (newDiag) { + (result || (result = [])).push(newDiag); + } + } + return result; + } + export function getFixes(context: CodeFixContext): CodeAction[] { const fixes = codeFixes[context.errorCode]; let allActions: CodeAction[] = []; diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index 47c449aca1c4a..6a7f92bce78c8 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -1,34 +1,19 @@ /* @internal */ namespace ts { - interface BaseRefactor { + interface Refactor { /** An unique code associated with each refactor */ - refactorKind: RefactorKind; - /** Indicates whether the refactor can be suggested without the editor asking for it */ - suggestable: boolean; - /** Compute the associated code actions */ - getCodeActions(context: RefactorContext, positionOrRange: number | TextRange): CodeAction[]; - } - - export interface SuggestableRefactor extends BaseRefactor { - suggestable: true; - /** - * The corresponding refactor diagnostic code this refactor generates. All suggestableRefactors - * generate one refactor diagnostic and handle it accordingly. - */ - diagnosticCode: number; - createRefactorDiagnosticIfApplicable(node: Node, context: RefactorContext): Diagnostic | undefined; - } + name: string; - export interface NonSuggestableRefactor extends BaseRefactor { - suggestable: false; /** Description of the refactor to display in the UI of the editor */ description: string; + + /** Compute the associated code actions */ + getCodeActions(context: RefactorContext, positionOrRange: number | TextRange): CodeAction[]; + /** A fast syntactic check to see if the refactor is applicable at given position. */ - isApplicableForPositionOrRange(positionOrRange: number | TextRange, context: LightRefactorContext): boolean; + isApplicableForPositionOrRange(context: LightRefactorContext, positionOrRange: number | TextRange): boolean; } - export type Refactor = SuggestableRefactor | NonSuggestableRefactor; - export interface LightRefactorContext { /** * The AST that was not bound, so the symbols associated with the nodes are not accessible. @@ -46,22 +31,12 @@ namespace ts { } export namespace refactor { - // A map with the diagnostic code as key, the refactors themselves as value - // e.g. suggestableRefactors[diagnosticCode] -> the refactor you want - const suggestableRefactors: SuggestableRefactor[] = []; // A map with the refactor code as key, the refactor itself as value // e.g. nonSuggestableRefactors[refactorCode] -> the refactor you want - const nonSuggestableRefactors: NonSuggestableRefactor[] = []; + const refactors: Map = createMap(); export function registerRefactor(refactor: Refactor) { - switch (refactor.suggestable) { - case true: - suggestableRefactors[refactor.diagnosticCode] = refactor; - break; - case false: - nonSuggestableRefactors[refactor.refactorKind] = refactor; - break; - } + refactors.set(refactor.name, refactor); } export function getApplicableRefactors( @@ -69,53 +44,29 @@ namespace ts { positionOrRange: number | TextRange): ApplicableRefactorInfo[] | undefined { let results: ApplicableRefactorInfo[]; - for (const code in nonSuggestableRefactors) { - const refactor = nonSuggestableRefactors[code]; - if (refactor.isApplicableForPositionOrRange(positionOrRange, context)) { - (results || (results = [])).push({ refactorKind: refactor.refactorKind, description: refactor.description }); + refactors.forEach(refactor => { + if (refactor.isApplicableForPositionOrRange(context, positionOrRange)) { + (results || (results = [])).push({ refactorName: refactor.name, description: refactor.description }); } - } + }); return results; } - export function getRefactorDiagnosticsForNode(context: RefactorContext, node: Node): Diagnostic[] | undefined { - let result: Diagnostic[]; - for (const key in suggestableRefactors) { - const refactor = suggestableRefactors[key]; - const newDiag = refactor.createRefactorDiagnosticIfApplicable(node, context); - if (newDiag) { - (result || (result = [])).push(newDiag); - } - } - return result; - } - export function getRefactorCodeActions( context: RefactorContext, positionOrRange: number | TextRange, - refactorKinds?: RefactorKind[], - diagnosticCodes?: number[]) { + refactorName: string) { const result: CodeAction[] = []; - if (refactorKinds !== undefined) { - for (const refactorKind of refactorKinds) { - const refactor = nonSuggestableRefactors[refactorKind]; - const codeActions = refactor.getCodeActions(context, positionOrRange); - if (codeActions) { - addRange(result, codeActions); - } - } - } - if (diagnosticCodes !== undefined) { - for (const diagnosticCode of diagnosticCodes) { - const refactor = suggestableRefactors[diagnosticCode]; - const codeActions = refactor && refactor.getCodeActions(context, positionOrRange); - if (codeActions) { - addRange(result, codeActions); - } - } + const refactor = refactors.get(refactorName); + if (!refactor) { + return undefined; } + const codeActions = refactor.getCodeActions(context, positionOrRange); + if (codeActions) { + addRange(result, codeActions); + } return result; } } diff --git a/src/services/services.ts b/src/services/services.ts index 7f0732418432c..a19365ffe8fb3 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1910,18 +1910,18 @@ namespace ts { return Rename.getRenameInfo(program.getTypeChecker(), defaultLibFileName, getCanonicalFileName, getValidSourceFile(fileName), position); } - function getRefactorDiagnostics(fileName: string): Diagnostic[] { + function getCodeFixDiagnostics(fileName: string): Diagnostic[] { const newLineCharacter = host.getNewLine(); const boundSourceFile = getValidSourceFile(fileName); const program = getProgram(); - const context: RefactorContext = { boundSourceFile, newLineCharacter, program, rulesProvider: ruleProvider }; + const context: CodeFixDiagnoseContext = { boundSourceFile, newLineCharacter, program, rulesProvider: ruleProvider }; const result: Diagnostic[] = []; forEachChild(boundSourceFile, visitor); return result; function visitor(node: Node): void { - const diags = refactor.getRefactorDiagnosticsForNode(context, node); + const diags = codefix.getCodeFixDiagnosticsForNode(context, node); if (diags) { addRange(result, diags); } @@ -1942,8 +1942,7 @@ namespace ts { fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, - refactorKinds?: RefactorKind[], - diagnosticCodes?: number[]): CodeAction[] { + refactorName: string): CodeAction[] { const context: RefactorContext = { boundSourceFile: getValidSourceFile(fileName), @@ -1952,7 +1951,7 @@ namespace ts { rulesProvider: getRuleProvider(formatOptions) }; - return refactor.getRefactorCodeActions(context, positionOrRange, refactorKinds, diagnosticCodes); + return refactor.getRefactorCodeActions(context, positionOrRange, refactorName); } return { @@ -1960,7 +1959,7 @@ namespace ts { cleanupSemanticCache, getSyntacticDiagnostics, getSemanticDiagnostics, - getRefactorDiagnostics, + getCodeFixDiagnostics, getApplicableRefactors, getRefactorCodeActions, getCompilerOptionsDiagnostics, diff --git a/src/services/types.ts b/src/services/types.ts index 91ba94080cb3c..04f934b3e2265 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -196,7 +196,7 @@ namespace ts { getSyntacticDiagnostics(fileName: string): Diagnostic[]; getSemanticDiagnostics(fileName: string): Diagnostic[]; - getRefactorDiagnostics(fileName: string): Diagnostic[]; + getCodeFixDiagnostics(fileName: string): Diagnostic[]; // TODO: Rename this to getProgramDiagnostics to better indicate that these are any // diagnostics present for the program level, and not just 'options' diagnostics. @@ -262,8 +262,7 @@ namespace ts { getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; - getRefactorCodeActions( - fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorKinds?: RefactorKind[], diagnosticCodes?: number[]): CodeAction[]; + getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[]; getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput; @@ -356,13 +355,10 @@ namespace ts { } export interface ApplicableRefactorInfo { - refactorKind: RefactorKind; + refactorName: string; description: string; } - export enum RefactorKind { - } - export interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ From b86d40a403e514b72385c5a52b387ee636ac38f0 Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Tue, 28 Mar 2017 17:05:28 -0700 Subject: [PATCH 07/12] Remove namespace in protocol --- src/server/client.ts | 12 ++++---- src/server/protocol.ts | 70 ++++++++++++++++++++---------------------- src/server/session.ts | 8 ++--- 3 files changed, 44 insertions(+), 46 deletions(-) diff --git a/src/server/client.ts b/src/server/client.ts index deadf52f3fbf1..5ce37b09546d8 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -712,13 +712,13 @@ namespace ts.server { } getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] { - const args: protocol.Refactor.GetApplicableRefactorsRequestArgs = { + const args: protocol.GetApplicableRefactorsRequestArgs = { file: fileName, locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName) }; - const request = this.processRequest(CommandNames.GetApplicableRefactors, args); - const response = this.processResponse(request); + const request = this.processRequest(CommandNames.GetApplicableRefactors, args); + const response = this.processResponse(request); return response.body.refactors; } @@ -729,14 +729,14 @@ namespace ts.server { positionOrRange: number | TextRange, refactorName: string) { - const args: protocol.Refactor.GetRefactorCodeActionsRequestArgs = { + const args: protocol.GetRefactorCodeActionsRequestArgs = { file: fileName, locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName), refactorName }; - const request = this.processRequest(CommandNames.GetRefactorCodeActions, args); - const codeActions = this.processResponse(request).body.actions; + const request = this.processRequest(CommandNames.GetRefactorCodeActions, args); + const codeActions = this.processResponse(request).body.actions; return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName)); } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index bf0c956ae2282..9018b42a9b1fe 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -407,42 +407,40 @@ namespace ts.server.protocol { position?: number; } - export namespace Refactor { - export interface GetApplicableRefactorsRequest extends Request { - command: CommandTypes.GetApplicableRefactors; - arguments: GetApplicableRefactorsRequestArgs; - } - - export interface GetApplicableRefactorsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { - } - - export interface ApplicableRefactorInfo { - refactorName: string; - description: string; - } - - export interface GetApplicableRefactorsResponse extends Response { - body?: { refactors: ApplicableRefactorInfo[] }; - } - - export interface GetRefactorCodeActionsRequest extends Request { - command: CommandTypes.GetRefactorCodeActions; - arguments: GetRefactorCodeActionsRequestArgs; - } - - export interface GetRefactorCodeActionsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { - /* The kind of the applicable refactor */ - refactorName: string; - } - - export interface GetRefactorCodeActionsResponse extends Response { - body?: { actions: CodeAction[] }; - } - - export interface RefactorDiagnosticEventBody { - file: string; - diagnostics: Diagnostic[]; - } + export interface GetApplicableRefactorsRequest extends Request { + command: CommandTypes.GetApplicableRefactors; + arguments: GetApplicableRefactorsRequestArgs; + } + + export interface GetApplicableRefactorsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { + } + + export interface ApplicableRefactorInfo { + refactorName: string; + description: string; + } + + export interface GetApplicableRefactorsResponse extends Response { + body?: { refactors: ApplicableRefactorInfo[] }; + } + + export interface GetRefactorCodeActionsRequest extends Request { + command: CommandTypes.GetRefactorCodeActions; + arguments: GetRefactorCodeActionsRequestArgs; + } + + export interface GetRefactorCodeActionsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { + /* The kind of the applicable refactor */ + refactorName: string; + } + + export interface GetRefactorCodeActionsResponse extends Response { + body?: { actions: CodeAction[] }; + } + + export interface RefactorDiagnosticEventBody { + file: string; + diagnostics: Diagnostic[]; } /** diff --git a/src/server/session.ts b/src/server/session.ts index ca39356e9a090..332b2bf8c90b5 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1451,14 +1451,14 @@ namespace ts.server { } } - private getApplicableRefactors(args: protocol.Refactor.GetApplicableRefactorsRequestArgs): protocol.Refactor.ApplicableRefactorInfo[] { + private getApplicableRefactors(args: protocol.GetApplicableRefactorsRequestArgs): protocol.ApplicableRefactorInfo[] { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const { position, textRange } = this.extractPositionAndRange(args, scriptInfo); return project.getLanguageService().getApplicableRefactors(file, position || textRange); } - private getRefactorCodeActions(args: protocol.Refactor.GetRefactorCodeActionsRequestArgs): protocol.CodeAction[] { + private getRefactorCodeActions(args: protocol.GetRefactorCodeActionsRequestArgs): protocol.CodeAction[] { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const { position, textRange } = this.extractPositionAndRange(args, scriptInfo); @@ -1838,10 +1838,10 @@ namespace ts.server { [CommandNames.GetSupportedCodeFixes]: () => { return this.requiredResponse(this.getSupportedCodeFixes()); }, - [CommandNames.GetApplicableRefactors]: (request: protocol.Refactor.GetApplicableRefactorsRequest) => { + [CommandNames.GetApplicableRefactors]: (request: protocol.GetApplicableRefactorsRequest) => { return this.requiredResponse(this.getApplicableRefactors(request.arguments)); }, - [CommandNames.GetRefactorCodeActions]: (request: protocol.Refactor.GetRefactorCodeActionsRequest) => { + [CommandNames.GetRefactorCodeActions]: (request: protocol.GetRefactorCodeActionsRequest) => { return this.requiredResponse(this.getRefactorCodeActions(request.arguments)); } }); From 09652232ede793d81a84abce8059596e7295196a Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Tue, 28 Mar 2017 17:46:29 -0700 Subject: [PATCH 08/12] Change the args shape to be more compatible --- src/server/client.ts | 33 +++++++++++++++------------------ src/server/protocol.ts | 19 +++++-------------- src/server/session.ts | 41 ++++++++++++++++++----------------------- 3 files changed, 38 insertions(+), 55 deletions(-) diff --git a/src/server/client.ts b/src/server/client.ts index 5ce37b09546d8..f393c0f5e1e3d 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -698,24 +698,24 @@ namespace ts.server { return notImplemented(); } - private positionOrRangeToLocationOrSpan(positionOrRange: number | TextRange, fileName: string) { - let locationOrSpan: protocol.LocationOrSpanWithPosition; + private createFileLocationOrRangeRequestArgs(positionOrRange: number | TextRange, fileName: string): protocol.FileLocationOrRangeRequestArgs { if (typeof positionOrRange === "number") { - locationOrSpan = this.positionToOneBasedLineOffset(fileName, positionOrRange); + const { line, offset } = this.positionToOneBasedLineOffset(fileName, positionOrRange); + return { file: fileName, line, offset }; } - else { - locationOrSpan = positionOrRange - ? { start: this.positionToOneBasedLineOffset(fileName, positionOrRange.pos), end: this.positionToOneBasedLineOffset(fileName, positionOrRange.end) } - : undefined; - } - return locationOrSpan; + const { line: startLine, offset: startOffset } = this.positionToOneBasedLineOffset(fileName, positionOrRange.pos); + const { line: endLine, offset: endOffset } = this.positionToOneBasedLineOffset(fileName, positionOrRange.end); + return { + file: fileName, + startLine, + startOffset, + endLine, + endOffset + }; } getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] { - const args: protocol.GetApplicableRefactorsRequestArgs = { - file: fileName, - locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName) - }; + const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName); const request = this.processRequest(CommandNames.GetApplicableRefactors, args); const response = this.processResponse(request); @@ -729,11 +729,8 @@ namespace ts.server { positionOrRange: number | TextRange, refactorName: string) { - const args: protocol.GetRefactorCodeActionsRequestArgs = { - file: fileName, - locationOrSpan: this.positionOrRangeToLocationOrSpan(positionOrRange, fileName), - refactorName - }; + const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetRefactorCodeActionsRequestArgs; + args.refactorName = refactorName; const request = this.processRequest(CommandNames.GetRefactorCodeActions, args); const codeActions = this.processResponse(request).body.actions; diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 9018b42a9b1fe..6e3988a61e898 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -397,23 +397,14 @@ namespace ts.server.protocol { position?: number; } - export type LocationOrSpanWithPosition = LocationWithPosition | { start: LocationWithPosition, end: LocationWithPosition }; - - export interface FileLocationOrSpanWithPositionRequestArgs extends FileRequestArgs { - locationOrSpan: LocationOrSpanWithPosition; - } - - export interface LocationWithPosition extends Location { - position?: number; - } + export type FileLocationOrRangeRequestArgs = FileLocationRequestArgs | FileRangeRequestArgs; export interface GetApplicableRefactorsRequest extends Request { command: CommandTypes.GetApplicableRefactors; arguments: GetApplicableRefactorsRequestArgs; } - export interface GetApplicableRefactorsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { - } + export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs; export interface ApplicableRefactorInfo { refactorName: string; @@ -429,16 +420,16 @@ namespace ts.server.protocol { arguments: GetRefactorCodeActionsRequestArgs; } - export interface GetRefactorCodeActionsRequestArgs extends FileLocationOrSpanWithPositionRequestArgs { + export type GetRefactorCodeActionsRequestArgs = FileLocationOrRangeRequestArgs & { /* The kind of the applicable refactor */ refactorName: string; - } + }; export interface GetRefactorCodeActionsResponse extends Response { body?: { actions: CodeAction[] }; } - export interface RefactorDiagnosticEventBody { + export interface CodeFixDiagnosticEventBody { file: string; diagnostics: Diagnostic[]; } diff --git a/src/server/session.ts b/src/server/session.ts index 37f3fac6d0b04..3637c4877798c 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1430,25 +1430,23 @@ namespace ts.server { this.event({ file, diagnostics }, "refactorDiag"); } - private isLocation(locationOrSpan: protocol.LocationOrSpanWithPosition): locationOrSpan is protocol.LocationWithPosition { - return (locationOrSpan).start === undefined; + private isLocation(locationOrSpan: protocol.FileLocationOrRangeRequestArgs): locationOrSpan is protocol.FileLocationRequestArgs { + return (locationOrSpan).line !== undefined; } - private extractPositionAndRange(args: protocol.FileLocationOrSpanWithPositionRequestArgs, scriptInfo: ScriptInfo): { position: number, textRange: TextRange } { + private extractPositionAndRange(args: protocol.FileLocationOrRangeRequestArgs, scriptInfo: ScriptInfo): { position: number, textRange: TextRange } { let position: number; let textRange: TextRange; - if (this.isLocation(args.locationOrSpan)) { - position = getPosition(args.locationOrSpan); + if (this.isLocation(args)) { + position = getPosition(args); } else { - textRange = { - pos: getPosition(args.locationOrSpan.start), - end: getPosition(args.locationOrSpan.end) - }; + const { startPosition, endPosition } = this.getStartAndEndPosition(args, scriptInfo); + textRange = { pos: startPosition, end: endPosition }; } return { position, textRange }; - function getPosition(loc: protocol.LocationWithPosition) { + function getPosition(loc: protocol.FileLocationRequestArgs) { return loc.position !== undefined ? loc.position : scriptInfo.lineOffsetToPosition(loc.line, loc.offset); } } @@ -1481,8 +1479,7 @@ namespace ts.server { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); - const startPosition = getStartPosition(); - const endPosition = getEndPosition(); + const { startPosition, endPosition } = this.getStartAndEndPosition(args, scriptInfo); const formatOptions = this.projectService.getFormatCodeOptions(file); const codeActions = project.getLanguageService().getCodeFixesAtPosition(file, startPosition, endPosition, args.errorCodes, formatOptions); @@ -1495,18 +1492,16 @@ namespace ts.server { else { return codeActions; } + } - function getStartPosition() { - return args.startPosition !== undefined - ? args.startPosition - : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); - } - - function getEndPosition() { - return args.endPosition !== undefined - ? args.endPosition - : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); - } + private getStartAndEndPosition(args: protocol.FileRangeRequestArgs, scriptInfo: ScriptInfo) { + const startPosition = args.startPosition !== undefined + ? args.startPosition + : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); + const endPosition = args.endPosition !== undefined + ? args.endPosition + : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); + return { startPosition, endPosition }; } private mapCodeAction(codeAction: CodeAction, scriptInfo: ScriptInfo): protocol.CodeAction { From 683a0a03316f8a572532fce4335e9bed1d623cb1 Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Tue, 18 Apr 2017 14:38:02 -0400 Subject: [PATCH 09/12] address CR feedback --- src/server/session.ts | 28 ++++++++++++++++++++-------- src/services/refactorProvider.ts | 11 ++++++++--- src/services/services.ts | 2 +- src/services/types.ts | 2 +- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 25cf8afa93f5c..9180ffb46d57c 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1447,7 +1447,7 @@ namespace ts.server { } private extractPositionAndRange(args: protocol.FileLocationOrRangeRequestArgs, scriptInfo: ScriptInfo): { position: number, textRange: TextRange } { - let position: number; + let position: number = undefined; let textRange: TextRange; if (this.isLocation(args)) { position = getPosition(args); @@ -1481,7 +1481,7 @@ namespace ts.server { position || textRange, args.refactorName ); - return map(result, action => this.mapCodeAction(action, scriptInfo)); + return result ? map(result, action => this.mapCodeAction(action, scriptInfo)) : undefined; } private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): protocol.CodeAction[] | CodeAction[] { @@ -1507,12 +1507,24 @@ namespace ts.server { } private getStartAndEndPosition(args: protocol.FileRangeRequestArgs, scriptInfo: ScriptInfo) { - const startPosition = args.startPosition !== undefined - ? args.startPosition - : args.startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); - const endPosition = args.endPosition !== undefined - ? args.endPosition - : args.startPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); + let startPosition: number = undefined, endPosition: number = undefined; + if (args.startPosition !== undefined ) { + startPosition = args.startPosition; + } + else { + startPosition = scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset); + // save the result so we don't always recompute + args.startPosition = startPosition; + } + + if (args.endPosition !== undefined) { + endPosition = args.endPosition; + } + else { + endPosition = scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset); + args.endPosition = endPosition; + } + return { startPosition, endPosition }; } diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index 6a7f92bce78c8..8926a431a25b8 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -14,6 +14,11 @@ namespace ts { isApplicableForPositionOrRange(context: LightRefactorContext, positionOrRange: number | TextRange): boolean; } + /** + * The `GetApplicableRefactor` API call is supposed to be fast, therefore only syntactic checks should be conducted + * to see if a refactor is applicable. The `LightRefactorContent` limits the context information accesable to the + * refactor to enforce such design. Such context should not provide a bound source file with symbols. + */ export interface LightRefactorContext { /** * The AST that was not bound, so the symbols associated with the nodes are not accessible. @@ -55,9 +60,9 @@ namespace ts { export function getRefactorCodeActions( context: RefactorContext, positionOrRange: number | TextRange, - refactorName: string) { + refactorName: string): CodeAction[] | undefined { - const result: CodeAction[] = []; + let result: CodeAction[]; const refactor = refactors.get(refactorName); if (!refactor) { return undefined; @@ -65,7 +70,7 @@ namespace ts { const codeActions = refactor.getCodeActions(context, positionOrRange); if (codeActions) { - addRange(result, codeActions); + addRange((result || (result = [])), codeActions); } return result; } diff --git a/src/services/services.ts b/src/services/services.ts index 5af3a188bc059..311e99e354a6d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1999,7 +1999,7 @@ namespace ts { fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, - refactorName: string): CodeAction[] { + refactorName: string): CodeAction[] | undefined { const context: RefactorContext = { boundSourceFile: getValidSourceFile(fileName), diff --git a/src/services/types.ts b/src/services/types.ts index fa9bd27077389..2e2eb547152f2 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -264,7 +264,7 @@ namespace ts { getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; - getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[]; + getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[] | undefined; getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput; From dfa25068d6854cae3d54d9bd47d843d3061d191c Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Tue, 18 Apr 2017 21:19:34 -0400 Subject: [PATCH 10/12] add codefix to convert function to es6 class --- src/compiler/diagnosticMessages.json | 6 +- src/compiler/types.ts | 2 +- src/harness/fourslash.ts | 12 +- .../unittests/tsserverProjectSystem.ts | 2 +- src/server/session.ts | 10 +- .../codefixes/convertFunctionToEs6Class.ts | 191 ++++++++++++++++++ src/services/codefixes/fixes.ts | 1 + src/services/tsconfig.json | 3 +- .../fourslash/convertFunctionToEs6Class1.ts | 27 +++ .../fourslash/convertFunctionToEs6Class2.ts | 27 +++ .../fourslash/convertFunctionToEs6Class3.ts | 28 +++ 11 files changed, 298 insertions(+), 11 deletions(-) create mode 100644 src/services/codefixes/convertFunctionToEs6Class.ts create mode 100644 tests/cases/fourslash/convertFunctionToEs6Class1.ts create mode 100644 tests/cases/fourslash/convertFunctionToEs6Class2.ts create mode 100644 tests/cases/fourslash/convertFunctionToEs6Class3.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 075d4247460e9..17a54328b378f 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3500,8 +3500,6 @@ "category": "Message", "code": 90021 }, - - "Octal literal types must use ES2015 syntax. Use the syntax '{0}'.": { "category": "Error", "code": 8017 @@ -3513,5 +3511,9 @@ "Report errors in .js files.": { "category": "Message", "code": 8019 + }, + "Convert function '{0}' to ES6 class.": { + "category": "CodeFix", + "code": 100000 } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index e79c4cea5d547..e2bcf54aa6d1e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3349,7 +3349,7 @@ namespace ts { Warning, Error, Message, - Refactor + CodeFix } export enum ModuleResolutionKind { diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 551b22325fb3f..4e5de9e0eb3ed 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -481,16 +481,26 @@ namespace FourSlash { private getDiagnostics(fileName: string): ts.Diagnostic[] { const syntacticErrors = this.languageService.getSyntacticDiagnostics(fileName); const semanticErrors = this.languageService.getSemanticDiagnostics(fileName); + const codeFixDiagnostics = this.getCodeFixDiagnostics(fileName); const diagnostics: ts.Diagnostic[] = []; diagnostics.push.apply(diagnostics, syntacticErrors); diagnostics.push.apply(diagnostics, semanticErrors); + diagnostics.push.apply(diagnostics, codeFixDiagnostics); return diagnostics; } private getCodeFixDiagnostics(fileName: string): ts.Diagnostic[] { - return this.languageService.getCodeFixDiagnostics(fileName); + let result: ts.Diagnostic[]; + + try { + result = this.languageService.getCodeFixDiagnostics(fileName); + } + catch(e) { + result = []; + } + return result; } private getAllDiagnostics(): ts.Diagnostic[] { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 84fcbe437a80e..956514c7dac22 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3631,7 +3631,7 @@ namespace ts.projectSystem { host.runQueuedImmediateCallbacks(); assert.equal(host.getOutput().length, 2, "expect 2 messages"); const e3 = getMessage(0); - assert.equal(e3.event, "refactorDiag"); + assert.equal(e3.event, "codeFixDiag"); verifyRequestCompleted(getErrId, 1); cancellationToken.resetToken(); diff --git a/src/server/session.ts b/src/server/session.ts index 9180ffb46d57c..34cb6ca78caae 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -506,7 +506,7 @@ namespace ts.server { next.immediate(() => { this.semanticCheck(checkSpec.fileName, checkSpec.project); next.immediate(() => { - this.refactorDiagnosticsCheck(checkSpec.fileName, checkSpec.project); + this.codeFixDiagnosticsCheck(checkSpec.fileName, checkSpec.project); if (checkList.length > index) { next.delay(followMs, checkOne); } @@ -1435,11 +1435,11 @@ namespace ts.server { return ts.getSupportedCodeFixes(); } - private refactorDiagnosticsCheck(file: NormalizedPath, project: Project): void { - const refactorDiags = project.getLanguageService().getCodeFixDiagnostics(file); - const diagnostics = refactorDiags.map(d => formatDiag(file, project, d)); + private codeFixDiagnosticsCheck(file: NormalizedPath, project: Project): void { + const codeFixDiags = project.getLanguageService().getCodeFixDiagnostics(file); + const diagnostics = codeFixDiags.map(d => formatDiag(file, project, d)); - this.event({ file, diagnostics }, "refactorDiag"); + this.event({ file, diagnostics }, "codeFixDiag"); } private isLocation(locationOrSpan: protocol.FileLocationOrRangeRequestArgs): locationOrSpan is protocol.FileLocationRequestArgs { diff --git a/src/services/codefixes/convertFunctionToEs6Class.ts b/src/services/codefixes/convertFunctionToEs6Class.ts new file mode 100644 index 0000000000000..3e3f7ac62bf6e --- /dev/null +++ b/src/services/codefixes/convertFunctionToEs6Class.ts @@ -0,0 +1,191 @@ +/* @internal */ +namespace ts.codefix { + registerCodeFix({ + errorCodes: [Diagnostics.Convert_function_0_to_ES6_class.code], + getCodeActions, + createCodeFixDiagnosticIfApplicable + }); + + function createCodeFixDiagnosticIfApplicable(node: Node, context: CodeFixDiagnoseContext): Diagnostic | undefined { + if (!isSourceFileJavaScript(context.boundSourceFile)) { + return undefined; + } + + const checker = context.program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (isClassLikeSymbol(symbol)) { + return createDiagnosticForNode(node, Diagnostics.Convert_function_0_to_ES6_class, symbol.name); + } + + function isClassLikeSymbol(symbol: Symbol) { + if (!symbol || !symbol.valueDeclaration) { + return false; + } + + let targetSymbol: Symbol; + if (symbol.valueDeclaration.kind === SyntaxKind.FunctionDeclaration) { + targetSymbol = symbol; + } + else if (isDeclarationOfFunctionOrClassExpression(symbol)) { + targetSymbol = (symbol.valueDeclaration as VariableDeclaration).initializer.symbol; + } + + // if there is a prototype property assignment like: + // foo.prototype.method = function () { } + // then the symbol for "foo" will have a member + return targetSymbol && targetSymbol.members && targetSymbol.members.size > 0; + } + } + + function getCodeActions(context: CodeFixContext): CodeAction[] { + const sourceFile = context.sourceFile; + const checker = context.program.getTypeChecker(); + const token = getTokenAtPosition(sourceFile, context.span.start); + const ctorSymbol = checker.getSymbolAtLocation(token); + + const deletes: (() => any)[] = []; + + if (!(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) { + return []; + } + + const ctorDeclaration = ctorSymbol.valueDeclaration; + const changeTracker = textChanges.ChangeTracker.fromCodeFixContext(context); + + let precedingNode: Node; + let newClassDeclaration: ClassDeclaration; + switch (ctorDeclaration.kind) { + case SyntaxKind.FunctionDeclaration: + precedingNode = ctorDeclaration; + deletes.push(() => changeTracker.deleteNode(sourceFile, ctorDeclaration)); + newClassDeclaration = createClassFromFunctionDeclaration(ctorDeclaration as FunctionDeclaration); + break; + + case SyntaxKind.VariableDeclaration: + precedingNode = ctorDeclaration.parent.parent; + if ((ctorDeclaration.parent).declarations.length === 1) { + deletes.push(() => changeTracker.deleteNode(sourceFile, precedingNode)); + } + else { + deletes.push(() => changeTracker.deleteNodeInList(sourceFile, ctorDeclaration)); + } + newClassDeclaration = createClassFromVariableDeclaration(ctorDeclaration as VariableDeclaration); + break; + } + + if (!newClassDeclaration) { + return []; + } + + // Because the preceding node could be touched, we need to insert nodes before delete nodes. + changeTracker.insertNodeAfter(sourceFile, precedingNode, newClassDeclaration, { suffix: "\n" }); + for (const deleteCallback of deletes) { + deleteCallback(); + } + + return [{ + description: `Convert function ${ctorSymbol.name} to ES6 class`, + changes: changeTracker.getChanges() + }]; + + function createClassElementsFromSymbol(symbol: Symbol) { + const memberElements: ClassElement[] = []; + // all instance members are stored in the "member" array of symbol + if (symbol.members) { + symbol.members.forEach(member => { + const memberElement = createClassElement(member, /*modifiers*/ undefined); + if (memberElement) { + memberElements.push(memberElement); + } + }); + } + + // all static members are stored in the "exports" array of symbol + if (symbol.exports) { + symbol.exports.forEach(member => { + const memberElement = createClassElement(member, [createToken(SyntaxKind.StaticKeyword)]); + if (memberElement) { + memberElements.push(memberElement); + } + }); + } + + return memberElements; + + function createClassElement(symbol: Symbol, modifiers: Modifier[]): ClassElement { + // both properties and methods are bound as property symbols + if (!(symbol.flags & SymbolFlags.Property)) { + return; + } + + const memberDeclaration = symbol.valueDeclaration as PropertyAccessExpression; + const assignmentBinaryExpression = memberDeclaration.parent as BinaryExpression; + + // delete the entire statement if this expression is the sole expression to take care of the semicolon at the end + const nodeToDelete = assignmentBinaryExpression.parent && assignmentBinaryExpression.parent.kind === SyntaxKind.ExpressionStatement + ? assignmentBinaryExpression.parent : assignmentBinaryExpression; + deletes.push(() => changeTracker.deleteNode(sourceFile, nodeToDelete)); + + if (!assignmentBinaryExpression.right) { + return createProperty([], modifiers, symbol.name, /*questionToken*/ undefined, + /*type*/ undefined, /*initializer*/ undefined); + } + + switch (assignmentBinaryExpression.right.kind) { + case SyntaxKind.FunctionExpression: + const functionExpression = assignmentBinaryExpression.right as FunctionExpression; + return createMethodDeclaration(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, + /*typeParameters*/ undefined, functionExpression.parameters, /*type*/ undefined, functionExpression.body); + + case SyntaxKind.ArrowFunction: + const arrowFunction = assignmentBinaryExpression.right as ArrowFunction; + const arrowFunctionBody = arrowFunction.body; + let bodyBlock: Block; + + // case 1: () => { return [1,2,3] } + if (arrowFunctionBody.kind === SyntaxKind.Block) { + bodyBlock = arrowFunctionBody as Block; + } + // case 2: () => [1,2,3] + else { + const expression = arrowFunctionBody as Expression; + bodyBlock = createBlock([createReturn(expression)]); + } + return createMethodDeclaration(/*decorators*/ undefined, modifiers, /*asteriskToken*/ undefined, memberDeclaration.name, /*questionToken*/ undefined, + /*typeParameters*/ undefined, arrowFunction.parameters, /*type*/ undefined, bodyBlock); + default: + return createProperty(/*decorators*/ undefined, modifiers, memberDeclaration.name, /*questionToken*/ undefined, + /*type*/ undefined, assignmentBinaryExpression.right); + } + } + } + + function createClassFromVariableDeclaration(node: VariableDeclaration): ClassDeclaration { + const initializer = node.initializer as FunctionExpression; + if (!initializer || initializer.kind !== SyntaxKind.FunctionExpression) { + return undefined; + } + + if (node.name.kind !== SyntaxKind.Identifier) { + return undefined; + } + + const memberElements = createClassElementsFromSymbol(initializer.symbol); + if (initializer.body) { + memberElements.unshift(createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, initializer.parameters, initializer.body)); + } + + return createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, node.name, + /*typeParameters*/ undefined, /*heritageClauses*/ undefined, memberElements); + } + + function createClassFromFunctionDeclaration(node: FunctionDeclaration): ClassDeclaration { + const memberElements = createClassElementsFromSymbol(ctorSymbol); + if (node.body) { + memberElements.unshift(createConstructor(/*decorators*/ undefined, /*modifiers*/ undefined, node.parameters, node.body)); + } + return createClassDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, node.name, + /*typeParameters*/ undefined, /*heritageClauses*/ undefined, memberElements); + } + } +} \ No newline at end of file diff --git a/src/services/codefixes/fixes.ts b/src/services/codefixes/fixes.ts index ae1643dfa3baa..6f58f16ea0e73 100644 --- a/src/services/codefixes/fixes.ts +++ b/src/services/codefixes/fixes.ts @@ -9,3 +9,4 @@ /// /// /// +/// diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 372e51b20de00..72a88a5c63b36 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -93,6 +93,7 @@ "codefixes/helpers.ts", "codefixes/importFixes.ts", "codefixes/unusedIdentifierFixes.ts", - "codefixes/disableJsDiagnostics.ts" + "codefixes/disableJsDiagnostics.ts", + "codefixes/convertFunctionToEs6Class.ts" ] } \ No newline at end of file diff --git a/tests/cases/fourslash/convertFunctionToEs6Class1.ts b/tests/cases/fourslash/convertFunctionToEs6Class1.ts new file mode 100644 index 0000000000000..1ffc536731453 --- /dev/null +++ b/tests/cases/fourslash/convertFunctionToEs6Class1.ts @@ -0,0 +1,27 @@ +/// + +// @allowNonTsExtensions: true +// @Filename: test123.js +//// [|function foo() { } +//// foo.prototype.instanceMethod1 = function() { return "this is name"; }; +//// foo.prototype.instanceMethod2 = () => { return "this is name"; }; +//// foo.prototype.instanceProp1 = "hello"; +//// foo.prototype.instanceProp2 = undefined; +//// foo.staticProp = "world"; +//// foo.staticMethod1 = function() { return "this is static name"; }; +//// foo.staticMethod2 = () => "this is static name";|] + + +verify.codeFixAvailable(); +verify.rangeAfterCodeFix( +`class foo { + constructor() { } + instanceMethod1() { return "this is name"; } + instanceMethod2() { return "this is name"; } + instanceProp1 = "hello"; + instanceProp2 = undefined; + static staticProp = "world"; + static staticMethod1() { return "this is static name"; } + static staticMethod2() { return "this is static name"; } +} +`, /*includeWhiteSpace*/ true, /*errorCode*/ undefined, /*index*/ 0); \ No newline at end of file diff --git a/tests/cases/fourslash/convertFunctionToEs6Class2.ts b/tests/cases/fourslash/convertFunctionToEs6Class2.ts new file mode 100644 index 0000000000000..ceda60a87667f --- /dev/null +++ b/tests/cases/fourslash/convertFunctionToEs6Class2.ts @@ -0,0 +1,27 @@ +/// + +// @allowNonTsExtensions: true +// @Filename: test123.js +//// [|var foo = function() { }; +//// foo.prototype.instanceMethod1 = function() { return "this is name"; }; +//// foo.prototype.instanceMethod2 = () => { return "this is name"; }; +//// foo.prototype.instanceProp1 = "hello"; +//// foo.prototype.instanceProp2 = undefined; +//// foo.staticProp = "world"; +//// foo.staticMethod1 = function() { return "this is static name"; }; +//// foo.staticMethod2 = () => "this is static name";|] + + +verify.codeFixAvailable(); +verify.rangeAfterCodeFix( +`class foo { + constructor() { } + instanceMethod1() { return "this is name"; } + instanceMethod2() { return "this is name"; } + instanceProp1 = "hello"; + instanceProp2 = undefined; + static staticProp = "world"; + static staticMethod1() { return "this is static name"; } + static staticMethod2() { return "this is static name"; } +} +`, /*includeWhiteSpace*/ true, /*errorCode*/ undefined, /*index*/ 0); \ No newline at end of file diff --git a/tests/cases/fourslash/convertFunctionToEs6Class3.ts b/tests/cases/fourslash/convertFunctionToEs6Class3.ts new file mode 100644 index 0000000000000..cdbc0ccd05e1b --- /dev/null +++ b/tests/cases/fourslash/convertFunctionToEs6Class3.ts @@ -0,0 +1,28 @@ +/// + +// @allowNonTsExtensions: true +// @Filename: test123.js +//// [|var bar = 10, foo = function() { }; +//// foo.prototype.instanceMethod1 = function() { return "this is name"; }; +//// foo.prototype.instanceMethod2 = () => { return "this is name"; }; +//// foo.prototype.instanceProp1 = "hello"; +//// foo.prototype.instanceProp2 = undefined; +//// foo.staticProp = "world"; +//// foo.staticMethod1 = function() { return "this is static name"; }; +//// foo.staticMethod2 = () => "this is static name";|] + + +verify.codeFixAvailable(); +verify.rangeAfterCodeFix( +`var bar = 10; +class foo { + constructor() { } + instanceMethod1() { return "this is name"; } + instanceMethod2() { return "this is name"; } + instanceProp1 = "hello"; + instanceProp2 = undefined; + static staticProp = "world"; + static staticMethod1() { return "this is static name"; } + static staticMethod2() { return "this is static name"; } +} +`, /*includeWhiteSpace*/ true, /*errorCode*/ undefined, /*index*/ 0); \ No newline at end of file From 0aeff7241a87b22a251d19d043a491189286a917 Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Tue, 18 Apr 2017 22:24:30 -0400 Subject: [PATCH 11/12] improve tests and force synchronizeHosts before codeFix diag --- src/harness/fourslash.ts | 36 ++++++++++--------- src/services/services.ts | 2 ++ .../fourslash/convertFunctionToEs6Class1.ts | 18 +++++----- .../fourslash/convertFunctionToEs6Class2.ts | 18 +++++----- .../fourslash/convertFunctionToEs6Class3.ts | 18 +++++----- tests/cases/fourslash/fourslash.ts | 2 +- 6 files changed, 49 insertions(+), 45 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 4e5de9e0eb3ed..1d7de2b1d5e25 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -494,10 +494,11 @@ namespace FourSlash { private getCodeFixDiagnostics(fileName: string): ts.Diagnostic[] { let result: ts.Diagnostic[]; + // In some language service implementation the `getCodeFixDiagnostics` is not implemented try { result = this.languageService.getCodeFixDiagnostics(fileName); } - catch(e) { + catch (e) { result = []; } return result; @@ -2623,23 +2624,24 @@ namespace FourSlash { } } - public verifyCodeFixDiagnosticsAvailableAtMarker(negative: boolean, markerName: string, diagnosticCode?: number) { - const marker = this.getMarkerByName(markerName); - const markerPos = marker.position; - let foundDiagnostic = false; - + public verifyCodeFixDiagnosticsAvailableAtMarkers(negative: boolean, markerNames: string[], diagnosticCode?: number) { const refactorDiagnostics = this.getCodeFixDiagnostics(this.activeFile.fileName); - for (const diag of refactorDiagnostics) { - if (diag.start <= markerPos && diag.start + diag.length >= markerPos) { - foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code; + + for (const markerName of markerNames) { + const marker = this.getMarkerByName(markerName); + let foundDiagnostic = false; + for (const diag of refactorDiagnostics) { + if (diag.start <= marker.position && diag.start + diag.length >= marker.position) { + foundDiagnostic = diagnosticCode === undefined || diagnosticCode === diag.code; + } } - } - if (negative && foundDiagnostic) { - this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected no refactor diagnostic at marker ${markerName} but found some.`); - } - if (!negative && !foundDiagnostic) { - this.raiseError(`verifyRefactorDiagnosticsAvailableAtMarker failed - expected a refactor diagnostic at marker ${markerName} but found none.`); + if (negative && foundDiagnostic) { + this.raiseError(`verifyCodeFixDiagnosticsAvailableAtMarkers failed - expected no codeFix diagnostic at marker ${markerName} but found some.`); + } + if (!negative && !foundDiagnostic) { + this.raiseError(`verifyCodeFixDiagnosticsAvailableAtMarkers failed - expected a codeFix diagnostic at marker ${markerName} but found none.`); + } } } @@ -3494,8 +3496,8 @@ namespace FourSlashInterface { this.state.verifyCodeFixAvailable(this.negative); } - public refactorDiagnosticsAvailableAtMarker(markerName: string, refactorCode?: number) { - this.state.verifyCodeFixDiagnosticsAvailableAtMarker(this.negative, markerName, refactorCode); + public codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], refactorCode?: number) { + this.state.verifyCodeFixDiagnosticsAvailableAtMarkers(this.negative, markerNames, refactorCode); } public applicableRefactorAvailableAtMarker(markerName: string) { diff --git a/src/services/services.ts b/src/services/services.ts index 311e99e354a6d..1f7630f2ad91f 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1968,6 +1968,8 @@ namespace ts { } function getCodeFixDiagnostics(fileName: string): Diagnostic[] { + synchronizeHostData(); + const newLineCharacter = host.getNewLine(); const boundSourceFile = getValidSourceFile(fileName); const program = getProgram(); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class1.ts b/tests/cases/fourslash/convertFunctionToEs6Class1.ts index 1ffc536731453..2d5df7eb6265d 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class1.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class1.ts @@ -2,17 +2,17 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// [|function foo() { } -//// foo.prototype.instanceMethod1 = function() { return "this is name"; }; -//// foo.prototype.instanceMethod2 = () => { return "this is name"; }; -//// foo.prototype.instanceProp1 = "hello"; -//// foo.prototype.instanceProp2 = undefined; -//// foo.staticProp = "world"; -//// foo.staticMethod1 = function() { return "this is static name"; }; -//// foo.staticMethod2 = () => "this is static name";|] +//// [|function /*1*/foo() { } +//// /*2*/foo.prototype.instanceMethod1 = function() { return "this is name"; }; +//// /*3*/foo.prototype.instanceMethod2 = () => { return "this is name"; }; +//// /*4*/foo.prototype.instanceProp1 = "hello"; +//// /*5*/foo.prototype.instanceProp2 = undefined; +//// /*6*/foo.staticProp = "world"; +//// /*7*/foo.staticMethod1 = function() { return "this is static name"; }; +//// /*8*/foo.staticMethod2 = () => "this is static name";|] -verify.codeFixAvailable(); +verify.codeFixDiagnosticsAvailableAtMarkers(['1', '2', '3', '4', '5', '6', '7', '8']); verify.rangeAfterCodeFix( `class foo { constructor() { } diff --git a/tests/cases/fourslash/convertFunctionToEs6Class2.ts b/tests/cases/fourslash/convertFunctionToEs6Class2.ts index ceda60a87667f..1c216c9e2280f 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class2.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class2.ts @@ -2,17 +2,17 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// [|var foo = function() { }; -//// foo.prototype.instanceMethod1 = function() { return "this is name"; }; -//// foo.prototype.instanceMethod2 = () => { return "this is name"; }; -//// foo.prototype.instanceProp1 = "hello"; -//// foo.prototype.instanceProp2 = undefined; -//// foo.staticProp = "world"; -//// foo.staticMethod1 = function() { return "this is static name"; }; -//// foo.staticMethod2 = () => "this is static name";|] +//// [|var /*1*/foo = function() { }; +//// /*2*/foo.prototype.instanceMethod1 = function() { return "this is name"; }; +//// /*3*/foo.prototype.instanceMethod2 = () => { return "this is name"; }; +//// /*4*/foo.prototype.instanceProp1 = "hello"; +//// /*5*/foo.prototype.instanceProp2 = undefined; +//// /*6*/foo.staticProp = "world"; +//// /*7*/foo.staticMethod1 = function() { return "this is static name"; }; +//// /*8*/foo.staticMethod2 = () => "this is static name";|] -verify.codeFixAvailable(); +verify.codeFixDiagnosticsAvailableAtMarkers(['1', '2', '3', '4', '5', '6', '7', '8']); verify.rangeAfterCodeFix( `class foo { constructor() { } diff --git a/tests/cases/fourslash/convertFunctionToEs6Class3.ts b/tests/cases/fourslash/convertFunctionToEs6Class3.ts index cdbc0ccd05e1b..ae81aba31f43b 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class3.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class3.ts @@ -2,17 +2,17 @@ // @allowNonTsExtensions: true // @Filename: test123.js -//// [|var bar = 10, foo = function() { }; -//// foo.prototype.instanceMethod1 = function() { return "this is name"; }; -//// foo.prototype.instanceMethod2 = () => { return "this is name"; }; -//// foo.prototype.instanceProp1 = "hello"; -//// foo.prototype.instanceProp2 = undefined; -//// foo.staticProp = "world"; -//// foo.staticMethod1 = function() { return "this is static name"; }; -//// foo.staticMethod2 = () => "this is static name";|] +//// [|var bar = 10, /*1*/foo = function() { }; +//// /*2*/foo.prototype.instanceMethod1 = function() { return "this is name"; }; +//// /*3*/foo.prototype.instanceMethod2 = () => { return "this is name"; }; +//// /*4*/foo.prototype.instanceProp1 = "hello"; +//// /*5*/foo.prototype.instanceProp2 = undefined; +//// /*6*/foo.staticProp = "world"; +//// /*7*/foo.staticMethod1 = function() { return "this is static name"; }; +//// /*8*/foo.staticMethod2 = () => "this is static name";|] -verify.codeFixAvailable(); +verify.codeFixDiagnosticsAvailableAtMarkers(['1', '2', '3', '4', '5', '6', '7', '8']); verify.rangeAfterCodeFix( `var bar = 10; class foo { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 155182f4519a8..ecbfa64a3eea5 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -149,7 +149,7 @@ declare namespace FourSlashInterface { isValidBraceCompletionAtPosition(openingBrace?: string): void; codeFixAvailable(): void; applicableRefactorAvailableAtMarker(markerName: string): void; - refactorDiagnosticsAvailableAtMarker(markerName: string, diagnosticCode?: number): void; + codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; applicableRefactorAvailableForRange(): void; } class verify extends verifyNegatable { From 2e8f6c6db54dae756367e2ada21e1630291b928d Mon Sep 17 00:00:00 2001 From: Zhengbo Li Date: Wed, 19 Apr 2017 01:04:59 -0400 Subject: [PATCH 12/12] Add a simple refactor as a sample --- src/services/refactorProvider.ts | 2 +- src/services/refactors/addAsyncSuffix.ts | 51 +++++++++++++++++++ src/services/refactors/refactors.ts | 1 + src/services/services.ts | 1 + src/services/textChanges.ts | 2 +- src/services/tsconfig.json | 36 +++---------- .../cases/fourslash/addAsyncSuffixRefactor.ts | 16 ++++++ tests/cases/fourslash/fourslash.ts | 2 +- 8 files changed, 78 insertions(+), 33 deletions(-) create mode 100644 src/services/refactors/addAsyncSuffix.ts create mode 100644 src/services/refactors/refactors.ts create mode 100644 tests/cases/fourslash/addAsyncSuffixRefactor.ts diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index 8926a431a25b8..6a13bc3c4e4d5 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -1,6 +1,6 @@ /* @internal */ namespace ts { - interface Refactor { + export interface Refactor { /** An unique code associated with each refactor */ name: string; diff --git a/src/services/refactors/addAsyncSuffix.ts b/src/services/refactors/addAsyncSuffix.ts new file mode 100644 index 0000000000000..470dbb0837bfa --- /dev/null +++ b/src/services/refactors/addAsyncSuffix.ts @@ -0,0 +1,51 @@ +/* @internal */ +namespace ts.refactor { + + const asyncSuffixRefactor: Refactor = { + name: "Add Async suffix", + description: "Add an 'Async' suffix to async function declarations", + getCodeActions, + isApplicableForPositionOrRange + }; + + registerRefactor(asyncSuffixRefactor); + + function getCodeActions(context: RefactorContext, positionOrRange: number | TextRange): CodeAction[] | undefined { + const { boundSourceFile, program } = context; + const tokenPos = typeof positionOrRange === "number" ? positionOrRange : positionOrRange.pos; + const token = getTokenAtPosition(boundSourceFile, tokenPos); + + const functionSymbol = program.getTypeChecker().getSymbolAtLocation(token); + if (!(functionSymbol.flags & SymbolFlags.Function)) { + return undefined; + } + + const oldNameNode = functionSymbol.valueDeclaration.name as Identifier; + const changeTracker = textChanges.ChangeTracker.fromCodeFixContext(context); + + changeTracker.replaceNode(boundSourceFile, oldNameNode, createIdentifier(functionSymbol.name + "Async")); + + return [{ + changes: changeTracker.getChanges(), + description: asyncSuffixRefactor.description + }]; + } + + function isApplicableForPositionOrRange(context: LightRefactorContext, positionOrRange: number | TextRange): boolean { + const { nonBoundSourceFile } = context; + const tokenPos = typeof positionOrRange === "number" ? positionOrRange : positionOrRange.pos; + const token = getTokenAtPosition(nonBoundSourceFile, tokenPos); + + let node = token; + while (node) { + if (node.kind === SyntaxKind.FunctionDeclaration) { + const nameNode = (node).name; + const modifiers = (node).modifiers; + const hasAsyncModifier = modifiers && forEach(modifiers, modifier => modifier.kind === SyntaxKind.AsyncKeyword); + return hasAsyncModifier && nameNode === token && !nameNode.text.match(/.*[aA]sync$/g); + } + node = node.parent; + } + return false; + } +} \ No newline at end of file diff --git a/src/services/refactors/refactors.ts b/src/services/refactors/refactors.ts new file mode 100644 index 0000000000000..585e4f2daf6c3 --- /dev/null +++ b/src/services/refactors/refactors.ts @@ -0,0 +1 @@ +/// diff --git a/src/services/services.ts b/src/services/services.ts index 1f7630f2ad91f..a117fbf4ec486 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -28,6 +28,7 @@ /// /// /// +/// namespace ts { /** The version of the language service API */ diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index d7056cfdca241..e16ae05a5c2de 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -157,7 +157,7 @@ namespace ts.textChanges { private changes: Change[] = []; private readonly newLineCharacter: string; - public static fromCodeFixContext(context: CodeFixContext) { + public static fromCodeFixContext(context: { newLineCharacter: string, rulesProvider: formatting.RulesProvider }) { return new ChangeTracker(context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed, context.rulesProvider); } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 72a88a5c63b36..af8fac33b9bb1 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -64,36 +64,12 @@ "signatureHelp.ts", "symbolDisplay.ts", "textChanges.ts", - "formatting/formatting.ts", - "formatting/formattingContext.ts", - "formatting/formattingRequestKind.ts", - "formatting/formattingScanner.ts", - "formatting/references.ts", - "formatting/rule.ts", - "formatting/ruleAction.ts", - "formatting/ruleDescriptor.ts", - "formatting/ruleFlag.ts", - "formatting/ruleOperation.ts", - "formatting/ruleOperationContext.ts", - "formatting/rules.ts", - "formatting/rulesMap.ts", - "formatting/rulesProvider.ts", - "formatting/smartIndenter.ts", - "formatting/tokenRange.ts", "refactorProvider.ts", - "codeFixProvider.ts", - "codefixes/fixAddMissingMember.ts", - "codefixes/fixExtendsInterfaceBecomesImplements.ts", - "codefixes/fixClassIncorrectlyImplementsInterface.ts", - "codefixes/fixClassDoesntImplementInheritedAbstractMember.ts", - "codefixes/fixClassSuperMustPrecedeThisAccess.ts", - "codefixes/fixConstructorForDerivedNeedSuperCall.ts", - "codefixes/fixForgottenThisPropertyAccess.ts", - "codefixes/fixes.ts", - "codefixes/helpers.ts", - "codefixes/importFixes.ts", - "codefixes/unusedIdentifierFixes.ts", - "codefixes/disableJsDiagnostics.ts", - "codefixes/convertFunctionToEs6Class.ts" + "codeFixProvider.ts" + ], + "include": [ + "formatting/*", + "codefixes/*", + "refactors/*" ] } \ No newline at end of file diff --git a/tests/cases/fourslash/addAsyncSuffixRefactor.ts b/tests/cases/fourslash/addAsyncSuffixRefactor.ts new file mode 100644 index 0000000000000..28c4da83c66e1 --- /dev/null +++ b/tests/cases/fourslash/addAsyncSuffixRefactor.ts @@ -0,0 +1,16 @@ +/// + +////function /*1*/test() { } +////async function /*2*/test2async() { } +////async function /*3*/test3Async() { } +////async function /*4*/test4() { } + +verify.not.applicableRefactorAvailableAtMarker('1'); +verify.not.applicableRefactorAvailableAtMarker('2'); +verify.not.applicableRefactorAvailableAtMarker('3'); +verify.applicableRefactorAvailableAtMarker('4'); +verify.fileAfterApplyingRefactorAtMarker('4', + `function test() { } +async function test2async() { } +async function test3Async() { } +async function test4Async() { }`, "Add Async suffix"); \ No newline at end of file diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index ecbfa64a3eea5..02dbbc41a5d5e 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -231,7 +231,7 @@ declare namespace FourSlashInterface { DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean): void; noDocCommentTemplate(): void; rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void; - fileAfterApplyingRefactorAtMarker(markerName: string, expectedText: string, refactorKindToApply?: number, formattingOptions?: FormatCodeOptions): void; + fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: FormatCodeOptions): void; importFixAtPosition(expectedTextArray: string[], errorCode?: number): void; navigationBar(json: any): void;