From bac046573f822486f5218eb970d289bced09295f Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 14 Apr 2020 13:17:19 -0700 Subject: [PATCH 01/36] add trigger reason to protocol --- src/server/protocol.ts | 10 +++++++++- src/server/session.ts | 2 +- .../refactors/addOrRemoveBracesToArrowFunction.ts | 6 +++--- src/services/services.ts | 7 ++++--- src/services/types.ts | 9 ++++++++- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index b6cc2e4d8fdab..62c81fca2ac8f 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -546,7 +546,15 @@ namespace ts.server.protocol { command: CommandTypes.GetApplicableRefactors; arguments: GetApplicableRefactorsRequestArgs; } - export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs; + export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & { + triggerReason?: RefactorTriggerReason; + }; + + export type RefactorTriggerReason = RefactorInvokedReason; + + export interface RefactorInvokedReason { + kind: 'invoked'; + } /** * Response is a list of available refactorings. diff --git a/src/server/session.ts b/src/server/session.ts index 3f0321074a994..780c7ab5085e2 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1970,7 +1970,7 @@ namespace ts.server { private getApplicableRefactors(args: protocol.GetApplicableRefactorsRequestArgs): protocol.ApplicableRefactorInfo[] { const { file, project } = this.getFileAndProject(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file)!; - return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file)); + return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason); } private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo { diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index c7c3a04e8d391..1256de9cfbed1 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -16,8 +16,8 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { } function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const { file, startPosition } = context; - const info = getConvertibleArrowFunctionAtPosition(file, startPosition); + const { file, startPosition, triggerReason } = context; + const info = getConvertibleArrowFunctionAtPosition(file, startPosition, triggerReason); if (!info) return emptyArray; return [{ @@ -70,7 +70,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return { renameFilename: undefined, renameLocation: undefined, edits }; } - function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined { + function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, triggerReason?: RefactorTriggerReason): Info | undefined { const node = getTokenAtPosition(file, startPosition); const func = getContainingFunction(node); if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) return undefined; diff --git a/src/services/services.ts b/src/services/services.ts index 28cf22ec473d1..5cfeb2b754211 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2140,7 +2140,7 @@ namespace ts { return Rename.getRenameInfo(program, getValidSourceFile(fileName), position, options); } - function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, preferences: UserPreferences, formatOptions?: FormatCodeSettings): RefactorContext { + function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, preferences: UserPreferences, formatOptions?: FormatCodeSettings, triggerReason?: RefactorTriggerReason): RefactorContext { const [startPosition, endPosition] = typeof positionOrRange === "number" ? [positionOrRange, undefined] : [positionOrRange.pos, positionOrRange.end]; return { file, @@ -2151,6 +2151,7 @@ namespace ts { formatContext: formatting.getFormatContext(formatOptions!, host), // TODO: GH#18217 cancellationToken, preferences, + triggerReason, }; } @@ -2158,10 +2159,10 @@ namespace ts { return SmartSelectionRange.getSmartSelectionRange(position, syntaxTreeCache.getCurrentSourceFile(fileName)); } - function getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences = emptyOptions): ApplicableRefactorInfo[] { + function getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences = emptyOptions, triggerReason: RefactorTriggerReason): ApplicableRefactorInfo[] { synchronizeHostData(); const file = getValidSourceFile(fileName); - return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange, preferences)); + return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange, preferences, undefined, triggerReason)); } function getEditsForRefactor( diff --git a/src/services/types.ts b/src/services/types.ts index fd8ade8f8e6e1..ff85050def74e 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -475,7 +475,7 @@ namespace ts { /** @deprecated `fileName` will be ignored */ applyCodeActionCommand(fileName: string, action: CodeActionCommand | CodeActionCommand[]): Promise; - getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined): ApplicableRefactorInfo[]; + getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason): ApplicableRefactorInfo[]; getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined): RefactorEditInfo | undefined; organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; @@ -741,6 +741,12 @@ namespace ts { commands?: CodeActionCommand[]; } + export type RefactorTriggerReason = RefactorInvokedReason; + + export interface RefactorInvokedReason { + kind: 'invoked'; + } + export interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ @@ -1404,5 +1410,6 @@ namespace ts { program: Program; cancellationToken?: CancellationToken; preferences: UserPreferences; + triggerReason?: RefactorTriggerReason; } } From 02fa39a6a99acc9de59ae12e81d078cca9f311fe Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 15 Apr 2020 13:49:50 -0700 Subject: [PATCH 02/36] enable explicit requests for addOrRemoveBracesToArrowFunction --- src/services/refactors/addOrRemoveBracesToArrowFunction.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 1256de9cfbed1..de048b7dbd3f7 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -38,7 +38,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { const { file, startPosition } = context; - const info = getConvertibleArrowFunctionAtPosition(file, startPosition); + const info = getConvertibleArrowFunctionAtPosition(file, startPosition, /*triggerReason*/ { kind: "invoked" }); if (!info) return undefined; const { expression, returnStatement, func } = info; @@ -73,7 +73,9 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, triggerReason?: RefactorTriggerReason): Info | undefined { const node = getTokenAtPosition(file, startPosition); const func = getContainingFunction(node); - if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) return undefined; + // Only offer a refactor in the function body on explicit refactor requests. + if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) + || (rangeContainsRange(func.body, node) && triggerReason?.kind !== "invoked"))) return undefined; if (isExpression(func.body)) { return { From c3828ed08697f91fae0490fd5eb246bff4266d4e Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 22 Apr 2020 11:01:37 -0700 Subject: [PATCH 03/36] loosen convertExport conditions --- src/services/refactors/convertExport.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index 18de048ac6c3e..a3fd2bc114026 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -31,7 +31,8 @@ namespace ts.refactor { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); - const exportNode = getParentNodeInSpan(token, file, span); + // If the span is entirely contained in an export node, check that node. + const exportNode = !!(getModifierFlags(token.parent) & ModifierFlags.Export) ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { return undefined; } From 1efcffc9150b9d5e7d8467974377dce7a259d0fc Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 28 Apr 2020 12:24:07 -0700 Subject: [PATCH 04/36] convertImport --- src/services/refactors/convertImport.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 29cefdc3210e3..43120591e6b31 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -23,8 +23,8 @@ namespace ts.refactor { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); - const importDecl = getParentNodeInSpan(token, file, span); - if (!importDecl || !isImportDeclaration(importDecl)) return undefined; + const importDecl = findAncestor(token, isImportDeclaration); + if (!importDecl || !isImportDeclaration(importDecl) || (importDecl.getEnd() < span.start + span.length)) return undefined; const { importClause } = importDecl; return importClause && importClause.namedBindings; } From 0296254a60cd5b58b7b220fe0efe837b0da16125 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 28 Apr 2020 13:13:50 -0700 Subject: [PATCH 05/36] extractType --- src/services/refactors/extractType.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 560fc5deb684a..544124a7fc970 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -64,7 +64,7 @@ namespace ts.refactor { const current = getTokenAtPosition(file, startPosition); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); - const selection = findAncestor(current, (node => node.parent && rangeContainsSkipTrivia(range, node, file) && !rangeContainsSkipTrivia(range, node.parent, file))); + const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && nodeOverlapsWithStartEnd(current, file, range.pos, range.end) && !rangeContainsSkipTrivia(range, node.parent, file))); if (!selection || !isTypeNode(selection)) return undefined; const checker = context.program.getTypeChecker(); From 55880ee5e04507a9db6622c5af1770111c520ddd Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 29 Apr 2020 12:19:15 -0700 Subject: [PATCH 06/36] extractType explicitCursorRequest --- src/services/refactors/extractType.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 544124a7fc970..1a22a8065c704 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -6,7 +6,7 @@ namespace ts.refactor { const extractToTypeDef = "Extract to typedef"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { - const info = getRangeToExtract(context); + const info = getRangeToExtract(context, context.triggerReason); if (!info) return emptyArray; return [{ @@ -22,8 +22,8 @@ namespace ts.refactor { }]; }, getEditsForAction(context, actionName): RefactorEditInfo { - const { file } = context; - const info = Debug.checkDefined(getRangeToExtract(context), "Expected to find a range to extract"); + const { file, } = context; + const info = Debug.checkDefined(getRangeToExtract(context, /*triggerReason*/ { kind: "invoked" }), "Expected to find a range to extract"); const name = getUniqueName("NewType", file); const edits = textChanges.ChangeTracker.with(context, changes => { @@ -58,13 +58,15 @@ namespace ts.refactor { type Info = TypeAliasInfo | InterfaceInfo; - function getRangeToExtract(context: RefactorContext): Info | undefined { + function getRangeToExtract(context: RefactorContext, triggerReason?: RefactorTriggerReason): Info | undefined { const { file, startPosition } = context; const isJS = isSourceFileJS(file); const current = getTokenAtPosition(file, startPosition); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); + const explicitCursorRequest = range.pos === range.end && triggerReason?.kind === "invoked"; - const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && nodeOverlapsWithStartEnd(current, file, range.pos, range.end) && !rangeContainsSkipTrivia(range, node.parent, file))); + const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && + (explicitCursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); if (!selection || !isTypeNode(selection)) return undefined; const checker = context.program.getTypeChecker(); From 602ee995d0349e3f260d11827c5b122b4774415b Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 5 May 2020 14:32:47 -0700 Subject: [PATCH 07/36] fix lint errors --- src/server/protocol.ts | 2 +- src/services/services.ts | 2 +- src/services/types.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 62c81fca2ac8f..a975b0c9b2d11 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -553,7 +553,7 @@ namespace ts.server.protocol { export type RefactorTriggerReason = RefactorInvokedReason; export interface RefactorInvokedReason { - kind: 'invoked'; + kind: "invoked"; } /** diff --git a/src/services/services.ts b/src/services/services.ts index 5cfeb2b754211..2002af73df9c2 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2162,7 +2162,7 @@ namespace ts { function getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences = emptyOptions, triggerReason: RefactorTriggerReason): ApplicableRefactorInfo[] { synchronizeHostData(); const file = getValidSourceFile(fileName); - return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange, preferences, undefined, triggerReason)); + return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange, preferences, emptyOptions, triggerReason)); } function getEditsForRefactor( diff --git a/src/services/types.ts b/src/services/types.ts index ff85050def74e..d72060bf51698 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -744,7 +744,7 @@ namespace ts { export type RefactorTriggerReason = RefactorInvokedReason; export interface RefactorInvokedReason { - kind: 'invoked'; + kind: "invoked"; } export interface TextInsertion { From f1127e6e9471bc289fc233a6be240f9dba1cb69c Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 6 May 2020 14:59:21 -0700 Subject: [PATCH 08/36] extract symbol --- src/services/refactors/extractSymbol.ts | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 8c6c8f6fc698b..8b36c3afb62dd 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -8,7 +8,7 @@ namespace ts.refactor.extractSymbol { * Exported for tests. */ export function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context)); + const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), context.triggerReason); const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { @@ -87,7 +87,7 @@ namespace ts.refactor.extractSymbol { /* Exported for tests */ export function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { - const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context)); + const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), /* triggerReason*/ { kind: "invoked" }); const targetRange = rangeToExtract.targetRange!; // TODO:GH#18217 const parsedFunctionIndexMatch = /^function_scope_(\d+)$/.exec(actionName); @@ -186,18 +186,20 @@ namespace ts.refactor.extractSymbol { * not shown to the user, but can be used by us diagnostically) */ // exported only for tests - export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan): RangeToExtract { + export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, triggerReason?: RefactorTriggerReason): RangeToExtract { const { length } = span; - - if (length === 0) { + if (length === 0 && triggerReason?.kind !== "invoked") { return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractEmpty)] }; } + const explicitCursorRequest = length === 0 && triggerReason?.kind === "invoked"; // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span. // This may fail (e.g. you select two statements in the root of a source file) - const start = getParentNodeInSpan(getTokenAtPosition(sourceFile, span.start), sourceFile, span); + const startToken = getTokenAtPosition(sourceFile, span.start); + const start = explicitCursorRequest ? getExtractableParent(startToken): getParentNodeInSpan(startToken, sourceFile, span); // Do the same for the ending position - const end = getParentNodeInSpan(findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)), sourceFile, span); + const endToken = findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)); + const end = explicitCursorRequest ? start : getParentNodeInSpan(endToken, sourceFile, span); const declarations: Symbol[] = []; @@ -1832,6 +1834,10 @@ namespace ts.refactor.extractSymbol { } } + function getExtractableParent(node: Node | undefined): Node | undefined { + return findAncestor(node, node => node.parent && isExtractableExpression(node) && !isBinaryExpression(node.parent)); + } + /** * Computes whether or not a node represents an expression in a position where it could * be extracted. From c5343f1d4e9c23906e615f7daeb71c23f20d622c Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 6 May 2020 15:10:18 -0700 Subject: [PATCH 09/36] update refactorConvertImport_partialSelection --- ...DefaultName.ts => refactorConvertImport_partialSelection.ts} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/cases/fourslash/{refactorConvertImport_notAtDefaultName.ts => refactorConvertImport_partialSelection.ts} (65%) diff --git a/tests/cases/fourslash/refactorConvertImport_notAtDefaultName.ts b/tests/cases/fourslash/refactorConvertImport_partialSelection.ts similarity index 65% rename from tests/cases/fourslash/refactorConvertImport_notAtDefaultName.ts rename to tests/cases/fourslash/refactorConvertImport_partialSelection.ts index 5f62bbb439446..0f703d7436f8d 100644 --- a/tests/cases/fourslash/refactorConvertImport_notAtDefaultName.ts +++ b/tests/cases/fourslash/refactorConvertImport_partialSelection.ts @@ -3,4 +3,4 @@ ////import /*a*/d/*b*/, * as n from "m"; goTo.select("a", "b"); -verify.not.refactorAvailable("Convert import"); +verify.refactorAvailable("Convert import"); From 53e8ed0089774c28e927e623c86e82e8630a67db Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 6 May 2020 15:21:09 -0700 Subject: [PATCH 10/36] accept new baseline --- tests/baselines/reference/api/tsserverlibrary.d.ts | 14 ++++++++++++-- tests/baselines/reference/api/typescript.d.ts | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 70e563b7e1cf7..13492ab6e1313 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5309,7 +5309,7 @@ declare namespace ts { applyCodeActionCommand(fileName: string, action: CodeActionCommand[]): Promise; /** @deprecated `fileName` will be ignored */ applyCodeActionCommand(fileName: string, action: CodeActionCommand | CodeActionCommand[]): Promise; - getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined): ApplicableRefactorInfo[]; + getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason): ApplicableRefactorInfo[]; getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined): RefactorEditInfo | undefined; organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; @@ -5531,6 +5531,10 @@ declare namespace ts { renameLocation?: number; commands?: CodeActionCommand[]; } + type RefactorTriggerReason = RefactorInvokedReason; + interface RefactorInvokedReason { + kind: "invoked"; + } interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ @@ -6629,7 +6633,13 @@ declare namespace ts.server.protocol { command: CommandTypes.GetApplicableRefactors; arguments: GetApplicableRefactorsRequestArgs; } - type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs; + type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & { + triggerReason?: RefactorTriggerReason; + }; + type RefactorTriggerReason = RefactorInvokedReason; + interface RefactorInvokedReason { + kind: "invoked"; + } /** * Response is a list of available refactorings. * Each refactoring exposes one or more "Actions"; a user selects one action to invoke a refactoring diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index b6397f88523d9..001f4d618ee74 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5309,7 +5309,7 @@ declare namespace ts { applyCodeActionCommand(fileName: string, action: CodeActionCommand[]): Promise; /** @deprecated `fileName` will be ignored */ applyCodeActionCommand(fileName: string, action: CodeActionCommand | CodeActionCommand[]): Promise; - getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined): ApplicableRefactorInfo[]; + getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason): ApplicableRefactorInfo[]; getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined): RefactorEditInfo | undefined; organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; @@ -5531,6 +5531,10 @@ declare namespace ts { renameLocation?: number; commands?: CodeActionCommand[]; } + type RefactorTriggerReason = RefactorInvokedReason; + interface RefactorInvokedReason { + kind: "invoked"; + } interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ From 23f4dc9bd9761e3d2e01d6fab3871e374ef6a981 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Wed, 13 May 2020 16:53:31 -0700 Subject: [PATCH 11/36] use enum for RefactorTriggerReason --- src/server/protocol.ts | 6 ++---- .../refactors/addOrRemoveBracesToArrowFunction.ts | 9 +++++---- src/services/refactors/extractSymbol.ts | 11 ++++++----- src/services/refactors/extractType.ts | 9 +++++---- src/services/types.ts | 7 +++---- src/testRunner/unittests/services/extract/ranges.ts | 2 +- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index a975b0c9b2d11..58a5988a4bfaa 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -550,10 +550,8 @@ namespace ts.server.protocol { triggerReason?: RefactorTriggerReason; }; - export type RefactorTriggerReason = RefactorInvokedReason; - - export interface RefactorInvokedReason { - kind: "invoked"; + export enum RefactorTriggerReason { + Invoked = "invoked" } /** diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index de048b7dbd3f7..3f499a06e7f26 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -17,7 +17,8 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, startPosition, triggerReason } = context; - const info = getConvertibleArrowFunctionAtPosition(file, startPosition, triggerReason); + const forImplicitRequest = triggerReason ? triggerReason === RefactorTriggerReason.Implicit : true; + const info = getConvertibleArrowFunctionAtPosition(file, startPosition, forImplicitRequest); if (!info) return emptyArray; return [{ @@ -38,7 +39,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { const { file, startPosition } = context; - const info = getConvertibleArrowFunctionAtPosition(file, startPosition, /*triggerReason*/ { kind: "invoked" }); + const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return undefined; const { expression, returnStatement, func } = info; @@ -70,12 +71,12 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return { renameFilename: undefined, renameLocation: undefined, edits }; } - function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, triggerReason?: RefactorTriggerReason): Info | undefined { + function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, forImplicitRequest = false): Info | undefined { const node = getTokenAtPosition(file, startPosition); const func = getContainingFunction(node); // Only offer a refactor in the function body on explicit refactor requests. if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) - || (rangeContainsRange(func.body, node) && triggerReason?.kind !== "invoked"))) return undefined; + || (rangeContainsRange(func.body, node) && forImplicitRequest))) return undefined; if (isExpression(func.body)) { return { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 8b36c3afb62dd..b7c535fc44fff 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -8,7 +8,8 @@ namespace ts.refactor.extractSymbol { * Exported for tests. */ export function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), context.triggerReason); + const forImplicitRequest = context.triggerReason ? context.triggerReason === RefactorTriggerReason.Implicit : true; + const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), forImplicitRequest); const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { @@ -87,7 +88,7 @@ namespace ts.refactor.extractSymbol { /* Exported for tests */ export function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { - const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), /* triggerReason*/ { kind: "invoked" }); + const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context)); const targetRange = rangeToExtract.targetRange!; // TODO:GH#18217 const parsedFunctionIndexMatch = /^function_scope_(\d+)$/.exec(actionName); @@ -186,12 +187,12 @@ namespace ts.refactor.extractSymbol { * not shown to the user, but can be used by us diagnostically) */ // exported only for tests - export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, triggerReason?: RefactorTriggerReason): RangeToExtract { + export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, forImplicitRequest = false): RangeToExtract { const { length } = span; - if (length === 0 && triggerReason?.kind !== "invoked") { + if (length === 0 && forImplicitRequest) { return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractEmpty)] }; } - const explicitCursorRequest = length === 0 && triggerReason?.kind === "invoked"; + const explicitCursorRequest = length === 0 && !forImplicitRequest; // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span. // This may fail (e.g. you select two statements in the root of a source file) diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 1a22a8065c704..f442c5e0c0d61 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -6,7 +6,8 @@ namespace ts.refactor { const extractToTypeDef = "Extract to typedef"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { - const info = getRangeToExtract(context, context.triggerReason); + const forImplicitRequest = context.triggerReason ? context.triggerReason === RefactorTriggerReason.Implicit : true; + const info = getRangeToExtract(context, forImplicitRequest); if (!info) return emptyArray; return [{ @@ -23,7 +24,7 @@ namespace ts.refactor { }, getEditsForAction(context, actionName): RefactorEditInfo { const { file, } = context; - const info = Debug.checkDefined(getRangeToExtract(context, /*triggerReason*/ { kind: "invoked" }), "Expected to find a range to extract"); + const info = Debug.checkDefined(getRangeToExtract(context), "Expected to find a range to extract"); const name = getUniqueName("NewType", file); const edits = textChanges.ChangeTracker.with(context, changes => { @@ -58,12 +59,12 @@ namespace ts.refactor { type Info = TypeAliasInfo | InterfaceInfo; - function getRangeToExtract(context: RefactorContext, triggerReason?: RefactorTriggerReason): Info | undefined { + function getRangeToExtract(context: RefactorContext, forImplicitRequest = false): Info | undefined { const { file, startPosition } = context; const isJS = isSourceFileJS(file); const current = getTokenAtPosition(file, startPosition); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); - const explicitCursorRequest = range.pos === range.end && triggerReason?.kind === "invoked"; + const explicitCursorRequest = range.pos === range.end && !forImplicitRequest; const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && (explicitCursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); diff --git a/src/services/types.ts b/src/services/types.ts index d72060bf51698..e4878471e6849 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -741,10 +741,9 @@ namespace ts { commands?: CodeActionCommand[]; } - export type RefactorTriggerReason = RefactorInvokedReason; - - export interface RefactorInvokedReason { - kind: "invoked"; + export enum RefactorTriggerReason { + Implicit = "implicit", + Invoked = "invoked", } export interface TextInsertion { diff --git a/src/testRunner/unittests/services/extract/ranges.ts b/src/testRunner/unittests/services/extract/ranges.ts index ab205288b330c..e10ed6cbb914c 100644 --- a/src/testRunner/unittests/services/extract/ranges.ts +++ b/src/testRunner/unittests/services/extract/ranges.ts @@ -7,7 +7,7 @@ namespace ts { if (!selectionRange) { throw new Error(`Test ${s} does not specify selection range`); } - const result = refactor.extractSymbol.getRangeToExtract(file, createTextSpanFromRange(selectionRange)); + const result = refactor.extractSymbol.getRangeToExtract(file, createTextSpanFromRange(selectionRange), /*forImplicitRequest*/ true); assert(result.targetRange === undefined, "failure expected"); const sortedErrors = result.errors!.map(e => e.messageText).sort(); assert.deepEqual(sortedErrors, expectedErrors.sort(), "unexpected errors"); From 57e856b0b42f9e4aa6a3d999e9cbdad7458115d3 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Thu, 14 May 2020 11:31:50 -0700 Subject: [PATCH 12/36] convert export tests --- .../refactorConvertExport_defaultToNamedValidSpans.ts | 9 +++++++++ .../refactorConvertExport_namedToDefaultValidSpans.ts | 9 +++++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts create mode 100644 tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts diff --git a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts new file mode 100644 index 0000000000000..6b06450f93dbf --- /dev/null +++ b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts @@ -0,0 +1,9 @@ +/// + +/////*a1*/ex/*a2*/port def/*a3*//*b3*/ault functio/*b2*/n f() {}/*b1*/ + +// verify that the refactor is offered for full, partial, and empty spans. +for (const m of ["1", "2", "3"]) { + goTo.select("a" + m, "b" + m); + verify.refactorAvailable("Convert export", "Convert default export to named export"); +} diff --git a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts new file mode 100644 index 0000000000000..f32d1514e9d68 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts @@ -0,0 +1,9 @@ +/// + +/////*a1*/ex/*a2*/port f/*a3*//*b3*/unctio/*b2*/n f() {}/*b1*/ + +// verify that the refactor is offered for full, partial, and empty spans. +for (const m of ["1", "2", "3"]) { + goTo.select("a" + m, "b" + m); + verify.refactorAvailable("Convert export", "Convert named export to default export"); +} From 665e434832f652c76c34e093a439adbb55cafab0 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Thu, 14 May 2020 11:53:43 -0700 Subject: [PATCH 13/36] convert import tests --- ...refactorConvertImport_namedToNamespaceValidSpans.ts | 9 +++++++++ ...refactorConvertImport_namespaceToNamedValidSpans.ts | 10 ++++++++++ 2 files changed, 19 insertions(+) create mode 100644 tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts create mode 100644 tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts new file mode 100644 index 0000000000000..0847e5d5744b8 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts @@ -0,0 +1,9 @@ +/// + +/////*a1*/im/*a2*/port { /*a3*//*b3*/x, y as z, T } fro/*b2*/m "m";/*b1*/ + +// verify that the refactor is offered for full, partial, and empty spans. +for (const m of ["1", "2", "3"]) { + goTo.select("a" + m, "b" + m); + verify.refactorAvailable("Convert import", "Convert named imports to namespace import"); +} diff --git a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts new file mode 100644 index 0000000000000..768d05282c017 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts @@ -0,0 +1,10 @@ +/// + +/////*a1*/im/*a2*/port * as /*a3*//*b3*/m from "m/*b2*/";/*b1*/ + +// verify that the refactor is offered for full, partial, and empty spans. +for (const m of ["1", "2", "3"]) { + goTo.select("a" + m, "b" + m); + verify.refactorAvailable("Convert import", "Convert namespace import to named imports"); +} + From 23e00644f466e322d2058d24feb296b8e5bee877 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Thu, 14 May 2020 14:49:35 -0700 Subject: [PATCH 14/36] accept new baseline --- tests/baselines/reference/api/tsserverlibrary.d.ts | 11 +++++------ tests/baselines/reference/api/typescript.d.ts | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 13492ab6e1313..14f27cab8e50d 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5531,9 +5531,9 @@ declare namespace ts { renameLocation?: number; commands?: CodeActionCommand[]; } - type RefactorTriggerReason = RefactorInvokedReason; - interface RefactorInvokedReason { - kind: "invoked"; + enum RefactorTriggerReason { + Implicit = "implicit", + Invoked = "invoked" } interface TextInsertion { newText: string; @@ -6636,9 +6636,8 @@ declare namespace ts.server.protocol { type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & { triggerReason?: RefactorTriggerReason; }; - type RefactorTriggerReason = RefactorInvokedReason; - interface RefactorInvokedReason { - kind: "invoked"; + enum RefactorTriggerReason { + Invoked = "invoked" } /** * Response is a list of available refactorings. diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 001f4d618ee74..91dad9f4fc178 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5531,9 +5531,9 @@ declare namespace ts { renameLocation?: number; commands?: CodeActionCommand[]; } - type RefactorTriggerReason = RefactorInvokedReason; - interface RefactorInvokedReason { - kind: "invoked"; + enum RefactorTriggerReason { + Implicit = "implicit", + Invoked = "invoked" } interface TextInsertion { newText: string; From f3751fbdf3034d4ccc760753e689b401c2fdbce6 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Thu, 21 May 2020 16:08:19 -0700 Subject: [PATCH 15/36] change type of RefactorTriggerReason --- src/harness/fourslashImpl.ts | 16 ++++++++-------- src/harness/fourslashInterfaceImpl.ts | 6 +++++- src/server/protocol.ts | 6 ++---- .../addOrRemoveBracesToArrowFunction.ts | 2 +- src/services/refactors/extractSymbol.ts | 2 +- src/services/refactors/extractType.ts | 2 +- src/services/types.ts | 5 +---- tests/cases/fourslash/fourslash.ts | 3 +++ 8 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index dd4135ca40c15..d3cdcb1655856 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -3214,8 +3214,8 @@ namespace FourSlash { }; } - public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) { - let refactors = this.getApplicableRefactorsAtSelection(); + public verifyRefactorAvailable(negative: boolean, triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { + let refactors = this.getApplicableRefactorsAtSelection(triggerReason); refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName))); const isAvailable = refactors.length > 0; @@ -3644,14 +3644,14 @@ namespace FourSlash { test(renameKeys(newFileContents, key => pathUpdater(key) || key), "with file moved"); } - private getApplicableRefactorsAtSelection() { - return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName); + private getApplicableRefactorsAtSelection(triggerReason: ts.RefactorTriggerReason = "implicit") { + return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName, ts.emptyOptions, triggerReason); } - private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions): readonly ts.ApplicableRefactorInfo[] { - return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences); // eslint-disable-line no-in-operator + private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason = "implicit"): readonly ts.ApplicableRefactorInfo[] { + return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences, triggerReason); // eslint-disable-line no-in-operator } - private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions): readonly ts.ApplicableRefactorInfo[] { - return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences) || ts.emptyArray; + private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason): readonly ts.ApplicableRefactorInfo[] { + return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences, triggerReason) || ts.emptyArray; } public configurePlugin(pluginName: string, configuration: any): void { diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts index f4905c00b84b5..07eea0c906655 100644 --- a/src/harness/fourslashInterfaceImpl.ts +++ b/src/harness/fourslashInterfaceImpl.ts @@ -208,7 +208,11 @@ namespace FourSlashInterface { } public refactorAvailable(name: string, actionName?: string) { - this.state.verifyRefactorAvailable(this.negative, name, actionName); + this.state.verifyRefactorAvailable(this.negative, "implicit", name, actionName); + } + + public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { + this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName); } } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 58a5988a4bfaa..db34b0838d96b 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -547,12 +547,10 @@ namespace ts.server.protocol { arguments: GetApplicableRefactorsRequestArgs; } export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & { - triggerReason?: RefactorTriggerReason; + triggerReason?: RefactorTriggerReason }; - export enum RefactorTriggerReason { - Invoked = "invoked" - } + export type RefactorTriggerReason = "implicit" | "invoked"; /** * Response is a list of available refactorings. diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 3f499a06e7f26..3f1324e2266fd 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -17,7 +17,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, startPosition, triggerReason } = context; - const forImplicitRequest = triggerReason ? triggerReason === RefactorTriggerReason.Implicit : true; + const forImplicitRequest = triggerReason ? triggerReason === "implicit" : true; const info = getConvertibleArrowFunctionAtPosition(file, startPosition, forImplicitRequest); if (!info) return emptyArray; diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index b7c535fc44fff..8425ff9d45783 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -8,7 +8,7 @@ namespace ts.refactor.extractSymbol { * Exported for tests. */ export function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const forImplicitRequest = context.triggerReason ? context.triggerReason === RefactorTriggerReason.Implicit : true; + const forImplicitRequest = context.triggerReason ? context.triggerReason === "implicit" : true; const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), forImplicitRequest); const targetRange = rangeToExtract.targetRange; diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index f442c5e0c0d61..114df56e8d96b 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -6,7 +6,7 @@ namespace ts.refactor { const extractToTypeDef = "Extract to typedef"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { - const forImplicitRequest = context.triggerReason ? context.triggerReason === RefactorTriggerReason.Implicit : true; + const forImplicitRequest = context.triggerReason ? context.triggerReason === "implicit" : true; const info = getRangeToExtract(context, forImplicitRequest); if (!info) return emptyArray; diff --git a/src/services/types.ts b/src/services/types.ts index e4878471e6849..d970e5a69e2b8 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -741,10 +741,7 @@ namespace ts { commands?: CodeActionCommand[]; } - export enum RefactorTriggerReason { - Implicit = "implicit", - Invoked = "invoked", - } + export type RefactorTriggerReason = "implicit" | "invoked"; export interface TextInsertion { newText: string; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index d7d4935118d0f..b8b6540605571 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -243,6 +243,7 @@ declare namespace FourSlashInterface { applicableRefactorAvailableForRange(): void; refactorAvailable(name: string, actionName?: string): void; + refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void } class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; @@ -683,6 +684,8 @@ declare namespace FourSlashInterface { triggerCharacter?: string, } + export type RefactorTriggerReason = "implicit" | "invoked"; + export interface VerifyCodeFixAvailableOptions { readonly description: string; readonly actions?: ReadonlyArray<{ readonly type: string, readonly data: {} }>; From 96f210c1c87222aa50f85871da49d39f055fdfaa Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 10:37:34 -0700 Subject: [PATCH 16/36] arrow function refactor test --- .../refactorAddOrRemoveBracesToArrowFunctionBody.ts | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts new file mode 100644 index 0000000000000..587f26ded43c9 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts @@ -0,0 +1,7 @@ +/// + +//// const a = (a: number) => { return/*a*//*b*/ a; }; + +// an invoked refactor request for a cursor in the body should return a refactor +goTo.select("a", "b"); +verify.refactorAvailableForTriggerReason("invoked","Add or remove braces in an arrow function", "Remove braces from arrow function"); \ No newline at end of file From fbf6737ae29577bdc27b2b0752eaf87590a9339d Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 12:41:20 -0700 Subject: [PATCH 17/36] use verify trigger reason for import export --- .../refactorConvertExport_defaultToNamedValidSpans.ts | 2 +- .../refactorConvertExport_namedToDefaultValidSpans.ts | 2 +- .../refactorConvertImport_namedToNamespaceValidSpans.ts | 2 +- .../refactorConvertImport_namespaceToNamedValidSpans.ts | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts index 6b06450f93dbf..c6e86c6c01b2e 100644 --- a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts @@ -5,5 +5,5 @@ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { goTo.select("a" + m, "b" + m); - verify.refactorAvailable("Convert export", "Convert default export to named export"); + verify.refactorAvailableForTriggerReason("invoked", "Convert export", "Convert default export to named export"); } diff --git a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts index f32d1514e9d68..bb8483dde6296 100644 --- a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts @@ -5,5 +5,5 @@ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { goTo.select("a" + m, "b" + m); - verify.refactorAvailable("Convert export", "Convert named export to default export"); + verify.refactorAvailableForTriggerReason("invoked", "Convert export", "Convert named export to default export"); } diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts index 0847e5d5744b8..e8cbbf80090da 100644 --- a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts @@ -5,5 +5,5 @@ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { goTo.select("a" + m, "b" + m); - verify.refactorAvailable("Convert import", "Convert named imports to namespace import"); + verify.refactorAvailableForTriggerReason("invoked", "Convert import", "Convert named imports to namespace import"); } diff --git a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts index 768d05282c017..1a53cea217285 100644 --- a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts @@ -5,6 +5,5 @@ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { goTo.select("a" + m, "b" + m); - verify.refactorAvailable("Convert import", "Convert namespace import to named imports"); + verify.refactorAvailableForTriggerReason("invoked","Convert import", "Convert namespace import to named imports"); } - From dc363f10fc14b15e7a07d32ad4cd48e8c9ac360b Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 12:56:36 -0700 Subject: [PATCH 18/36] fix some indices --- .../refactorConvertExport_defaultToNamedValidSpans.ts | 4 ++-- .../refactorConvertExport_namedToDefaultValidSpans.ts | 4 ++-- .../refactorConvertImport_namedToNamespaceValidSpans.ts | 4 ++-- .../refactorConvertImport_namespaceToNamedValidSpans.ts | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts index c6e86c6c01b2e..e4322853da34b 100644 --- a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts @@ -1,9 +1,9 @@ /// -/////*a1*/ex/*a2*/port def/*a3*//*b3*/ault functio/*b2*/n f() {}/*b1*/ +/////*1a*/ex/*2a*/port def/*3a*//*3b*/ault functio/*2b*/n f() {}/*1b*/ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { - goTo.select("a" + m, "b" + m); + goTo.select(m + "a", m + "b"); verify.refactorAvailableForTriggerReason("invoked", "Convert export", "Convert default export to named export"); } diff --git a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts index bb8483dde6296..f9955962437e4 100644 --- a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts @@ -1,9 +1,9 @@ /// -/////*a1*/ex/*a2*/port f/*a3*//*b3*/unctio/*b2*/n f() {}/*b1*/ +/////*1a*/ex/*2a*/port f/*3a*//*3b*/unctio/*2b*/n f() {}/*1b*/ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { - goTo.select("a" + m, "b" + m); + goTo.select(m + "a", m + "b"); verify.refactorAvailableForTriggerReason("invoked", "Convert export", "Convert named export to default export"); } diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts index e8cbbf80090da..4c36c84929c9c 100644 --- a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts @@ -1,9 +1,9 @@ /// -/////*a1*/im/*a2*/port { /*a3*//*b3*/x, y as z, T } fro/*b2*/m "m";/*b1*/ +/////*1a*/im/*2a*/port { /*3a*//*3b*/x, y as z, T } fro/*2b*/m "m";/*1b*/ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { - goTo.select("a" + m, "b" + m); + goTo.select(m + "a", m + "b"); verify.refactorAvailableForTriggerReason("invoked", "Convert import", "Convert named imports to namespace import"); } diff --git a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts index 1a53cea217285..9b5bf353cb39e 100644 --- a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts +++ b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts @@ -1,9 +1,9 @@ /// -/////*a1*/im/*a2*/port * as /*a3*//*b3*/m from "m/*b2*/";/*b1*/ +/////*1a*/im/*2a*/port * as /*3a*//*3b*/m from "m/*2b*/";/*1b*/ // verify that the refactor is offered for full, partial, and empty spans. for (const m of ["1", "2", "3"]) { - goTo.select("a" + m, "b" + m); + goTo.select(m + "a", m + "b"); verify.refactorAvailableForTriggerReason("invoked","Convert import", "Convert namespace import to named imports"); } From 85e0d8b14af90528af5de46edd185d890d583e2a Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 13:11:38 -0700 Subject: [PATCH 19/36] add refactorNotAvailableForTriggerReason --- src/harness/fourslashInterfaceImpl.ts | 4 ++++ tests/cases/fourslash/fourslash.ts | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts index 07eea0c906655..606dec937ff8f 100644 --- a/src/harness/fourslashInterfaceImpl.ts +++ b/src/harness/fourslashInterfaceImpl.ts @@ -214,6 +214,10 @@ namespace FourSlashInterface { public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName); } + + public refactorNotAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { + this.state.verifyRefactorAvailable(!this.negative, triggerReason, name, actionName); + } } export class Verify extends VerifyNegatable { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index b8b6540605571..460942de4c309 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -243,7 +243,8 @@ declare namespace FourSlashInterface { applicableRefactorAvailableForRange(): void; refactorAvailable(name: string, actionName?: string): void; - refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void + refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void; + refactorNotAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void; } class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; From df8ff659c5dbcdd00e718ac6e2f3954fe8448548 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 13:12:02 -0700 Subject: [PATCH 20/36] extract type test --- .../cases/fourslash/refactorExtractTypeValidSpans.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/cases/fourslash/refactorExtractTypeValidSpans.ts diff --git a/tests/cases/fourslash/refactorExtractTypeValidSpans.ts b/tests/cases/fourslash/refactorExtractTypeValidSpans.ts new file mode 100644 index 0000000000000..cf0319876be90 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractTypeValidSpans.ts @@ -0,0 +1,12 @@ +/// + +//// var x: /*1a*/{ a?:/*2a*/ number, b?: string/*2b*//*3a*//*3b*/ }/*1b*/ = { }; + +// Only offer refactor for cursor position if explicitly requested +goTo.select("3a", "3b"); +verify.refactorNotAvailableForTriggerReason("implicit", "Extract type"); + +for (const m of ["1", "2", "3"]) { + goTo.select(m + "a", m + "b"); + verify.refactorAvailableForTriggerReason("invoked", "Extract type"); +} From 3825d193f6a41b60e559757e5794eba772156f4c Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 17:41:23 -0700 Subject: [PATCH 21/36] extract symbol test --- tests/cases/fourslash/extractSymbolForTriggerReason.ts | 10 ++++++++++ tests/cases/fourslash/refactorExtractTypeValidSpans.ts | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/extractSymbolForTriggerReason.ts diff --git a/tests/cases/fourslash/extractSymbolForTriggerReason.ts b/tests/cases/fourslash/extractSymbolForTriggerReason.ts new file mode 100644 index 0000000000000..be946f5fc1545 --- /dev/null +++ b/tests/cases/fourslash/extractSymbolForTriggerReason.ts @@ -0,0 +1,10 @@ +/// + +////function foo() { +//// return 1/*a*//*b*/00; +////} + +// Only offer refactor for empty span if explicity requested +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Extract Symbol"); +verify.refactorAvailableForTriggerReason("invoked", "Extract Symbol", "constant_scope_0"); diff --git a/tests/cases/fourslash/refactorExtractTypeValidSpans.ts b/tests/cases/fourslash/refactorExtractTypeValidSpans.ts index cf0319876be90..a1030a44c43bc 100644 --- a/tests/cases/fourslash/refactorExtractTypeValidSpans.ts +++ b/tests/cases/fourslash/refactorExtractTypeValidSpans.ts @@ -2,7 +2,7 @@ //// var x: /*1a*/{ a?:/*2a*/ number, b?: string/*2b*//*3a*//*3b*/ }/*1b*/ = { }; -// Only offer refactor for cursor position if explicitly requested +// Only offer refactor for empty span if explicity requested goTo.select("3a", "3b"); verify.refactorNotAvailableForTriggerReason("implicit", "Extract type"); From 673a86833aa070f94f6ec4c245f3ccb8309d36f7 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 17:51:00 -0700 Subject: [PATCH 22/36] update test names --- .../refactorAddOrRemoveBracesToArrowFunctionBody.ts | 7 ------- ...rAddOrRemoveBracesToArrowFunctionTriggerReason.ts | 8 ++++++++ .../fourslash/refactorExtractTypeTriggerReason.ts | 8 ++++++++ .../cases/fourslash/refactorExtractTypeValidSpans.ts | 12 ------------ 4 files changed, 16 insertions(+), 19 deletions(-) delete mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts create mode 100644 tests/cases/fourslash/refactorExtractTypeTriggerReason.ts delete mode 100644 tests/cases/fourslash/refactorExtractTypeValidSpans.ts diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts deleted file mode 100644 index 587f26ded43c9..0000000000000 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionBody.ts +++ /dev/null @@ -1,7 +0,0 @@ -/// - -//// const a = (a: number) => { return/*a*//*b*/ a; }; - -// an invoked refactor request for a cursor in the body should return a refactor -goTo.select("a", "b"); -verify.refactorAvailableForTriggerReason("invoked","Add or remove braces in an arrow function", "Remove braces from arrow function"); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts new file mode 100644 index 0000000000000..a1e940fa7b1ee --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts @@ -0,0 +1,8 @@ +/// + +//// const a = (a: number) => { return/*a*//*b*/ a; }; + +// Only offer refactor for empty span if explicity requested +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Add or remove braces in an arrow function"); +verify.refactorAvailableForTriggerReason("invoked", "Add or remove braces in an arrow function", "Remove braces from arrow function"); diff --git a/tests/cases/fourslash/refactorExtractTypeTriggerReason.ts b/tests/cases/fourslash/refactorExtractTypeTriggerReason.ts new file mode 100644 index 0000000000000..019996fbbd000 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractTypeTriggerReason.ts @@ -0,0 +1,8 @@ +/// + +//// var x: str/*a*//*b*/ing; + +// Only offer refactor for empty span if explicity requested +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Extract type"); +verify.refactorAvailableForTriggerReason("invoked", "Extract type"); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorExtractTypeValidSpans.ts b/tests/cases/fourslash/refactorExtractTypeValidSpans.ts deleted file mode 100644 index a1030a44c43bc..0000000000000 --- a/tests/cases/fourslash/refactorExtractTypeValidSpans.ts +++ /dev/null @@ -1,12 +0,0 @@ -/// - -//// var x: /*1a*/{ a?:/*2a*/ number, b?: string/*2b*//*3a*//*3b*/ }/*1b*/ = { }; - -// Only offer refactor for empty span if explicity requested -goTo.select("3a", "3b"); -verify.refactorNotAvailableForTriggerReason("implicit", "Extract type"); - -for (const m of ["1", "2", "3"]) { - goTo.select(m + "a", m + "b"); - verify.refactorAvailableForTriggerReason("invoked", "Extract type"); -} From 86122c412f701606415ab69b592c446a858231b5 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 18:00:38 -0700 Subject: [PATCH 23/36] convert get set test --- .../refactorConvertToGetAndSetAccessTriggerReason.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts diff --git a/tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts b/tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts new file mode 100644 index 0000000000000..f77de717dbf80 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts @@ -0,0 +1,9 @@ +/// + +//// class A { +//// public /*a*//*b*/a: string; +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Generate 'get' and 'set' accessors"); +verify.refactorAvailableForTriggerReason("invoked", "Generate 'get' and 'set' accessors"); \ No newline at end of file From d37f4c33cc563eff03c5e31449a09780467f207f Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 18:36:29 -0700 Subject: [PATCH 24/36] accept new baseline --- tests/baselines/reference/api/tsserverlibrary.d.ts | 9 ++------- tests/baselines/reference/api/typescript.d.ts | 5 +---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 14f27cab8e50d..37ec6e753ab22 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5531,10 +5531,7 @@ declare namespace ts { renameLocation?: number; commands?: CodeActionCommand[]; } - enum RefactorTriggerReason { - Implicit = "implicit", - Invoked = "invoked" - } + type RefactorTriggerReason = "implicit" | "invoked"; interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ @@ -6636,9 +6633,7 @@ declare namespace ts.server.protocol { type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & { triggerReason?: RefactorTriggerReason; }; - enum RefactorTriggerReason { - Invoked = "invoked" - } + type RefactorTriggerReason = "implicit" | "invoked"; /** * Response is a list of available refactorings. * Each refactoring exposes one or more "Actions"; a user selects one action to invoke a refactoring diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 91dad9f4fc178..b20cc1a215a26 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5531,10 +5531,7 @@ declare namespace ts { renameLocation?: number; commands?: CodeActionCommand[]; } - enum RefactorTriggerReason { - Implicit = "implicit", - Invoked = "invoked" - } + type RefactorTriggerReason = "implicit" | "invoked"; interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ From e56593139267cb796bc7d8fe1ed13cb93f4bfc04 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 26 May 2020 21:07:33 -0700 Subject: [PATCH 25/36] fix up some bools --- .../refactors/addOrRemoveBracesToArrowFunction.ts | 7 +++---- src/services/refactors/extractSymbol.ts | 13 ++++++------- src/services/refactors/extractType.ts | 9 ++++----- src/testRunner/unittests/services/extract/ranges.ts | 2 +- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 3f1324e2266fd..84e596c5e4116 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -17,8 +17,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, startPosition, triggerReason } = context; - const forImplicitRequest = triggerReason ? triggerReason === "implicit" : true; - const info = getConvertibleArrowFunctionAtPosition(file, startPosition, forImplicitRequest); + const info = getConvertibleArrowFunctionAtPosition(file, startPosition, triggerReason === "invoked"); if (!info) return emptyArray; return [{ @@ -71,12 +70,12 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return { renameFilename: undefined, renameLocation: undefined, edits }; } - function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, forImplicitRequest = false): Info | undefined { + function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, userRequested = true): Info | undefined { const node = getTokenAtPosition(file, startPosition); const func = getContainingFunction(node); // Only offer a refactor in the function body on explicit refactor requests. if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) - || (rangeContainsRange(func.body, node) && forImplicitRequest))) return undefined; + || (rangeContainsRange(func.body, node) && !userRequested))) return undefined; if (isExpression(func.body)) { return { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 8425ff9d45783..3da484a9ce630 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -8,8 +8,7 @@ namespace ts.refactor.extractSymbol { * Exported for tests. */ export function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const forImplicitRequest = context.triggerReason ? context.triggerReason === "implicit" : true; - const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), forImplicitRequest); + const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), context.triggerReason === "invoked"); const targetRange = rangeToExtract.targetRange; if (targetRange === undefined) { @@ -187,20 +186,20 @@ namespace ts.refactor.extractSymbol { * not shown to the user, but can be used by us diagnostically) */ // exported only for tests - export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, forImplicitRequest = false): RangeToExtract { + export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, userRequested = true): RangeToExtract { const { length } = span; - if (length === 0 && forImplicitRequest) { + if (length === 0 && !userRequested) { return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractEmpty)] }; } - const explicitCursorRequest = length === 0 && !forImplicitRequest; + const cursorRequest = length === 0 && userRequested; // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span. // This may fail (e.g. you select two statements in the root of a source file) const startToken = getTokenAtPosition(sourceFile, span.start); - const start = explicitCursorRequest ? getExtractableParent(startToken): getParentNodeInSpan(startToken, sourceFile, span); + const start = cursorRequest ? getExtractableParent(startToken): getParentNodeInSpan(startToken, sourceFile, span); // Do the same for the ending position const endToken = findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)); - const end = explicitCursorRequest ? start : getParentNodeInSpan(endToken, sourceFile, span); + const end = cursorRequest ? start : getParentNodeInSpan(endToken, sourceFile, span); const declarations: Symbol[] = []; diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 114df56e8d96b..4941c4dffd6ac 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -6,8 +6,7 @@ namespace ts.refactor { const extractToTypeDef = "Extract to typedef"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { - const forImplicitRequest = context.triggerReason ? context.triggerReason === "implicit" : true; - const info = getRangeToExtract(context, forImplicitRequest); + const info = getRangeToExtract(context, context.triggerReason === "invoked"); if (!info) return emptyArray; return [{ @@ -59,15 +58,15 @@ namespace ts.refactor { type Info = TypeAliasInfo | InterfaceInfo; - function getRangeToExtract(context: RefactorContext, forImplicitRequest = false): Info | undefined { + function getRangeToExtract(context: RefactorContext, userRequested = true): Info | undefined { const { file, startPosition } = context; const isJS = isSourceFileJS(file); const current = getTokenAtPosition(file, startPosition); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); - const explicitCursorRequest = range.pos === range.end && !forImplicitRequest; + const cursorRequest = range.pos === range.end && userRequested; const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && - (explicitCursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); + (cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); if (!selection || !isTypeNode(selection)) return undefined; const checker = context.program.getTypeChecker(); diff --git a/src/testRunner/unittests/services/extract/ranges.ts b/src/testRunner/unittests/services/extract/ranges.ts index e10ed6cbb914c..8fe5aeb3c131f 100644 --- a/src/testRunner/unittests/services/extract/ranges.ts +++ b/src/testRunner/unittests/services/extract/ranges.ts @@ -7,7 +7,7 @@ namespace ts { if (!selectionRange) { throw new Error(`Test ${s} does not specify selection range`); } - const result = refactor.extractSymbol.getRangeToExtract(file, createTextSpanFromRange(selectionRange), /*forImplicitRequest*/ true); + const result = refactor.extractSymbol.getRangeToExtract(file, createTextSpanFromRange(selectionRange), /*userRequested*/ false); assert(result.targetRange === undefined, "failure expected"); const sortedErrors = result.errors!.map(e => e.messageText).sort(); assert.deepEqual(sortedErrors, expectedErrors.sort(), "unexpected errors"); From a07a79b772ca5edbdff0e0d7f00aae523c6f654b Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 10:19:55 -0700 Subject: [PATCH 26/36] remove unused test method --- src/harness/fourslashInterfaceImpl.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts index 606dec937ff8f..07eea0c906655 100644 --- a/src/harness/fourslashInterfaceImpl.ts +++ b/src/harness/fourslashInterfaceImpl.ts @@ -214,10 +214,6 @@ namespace FourSlashInterface { public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName); } - - public refactorNotAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { - this.state.verifyRefactorAvailable(!this.negative, triggerReason, name, actionName); - } } export class Verify extends VerifyNegatable { From 3e1e61470bfcaea49cd83aa8abfbb2fb79d77453 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 10:37:49 -0700 Subject: [PATCH 27/36] Revert "update refactorConvertImport_partialSelection" This reverts commit e28d8a0895f118321c37016a6dad4751a7c13673. --- ...alSelection.ts => refactorConvertImport_notAtDefaultName.ts} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/cases/fourslash/{refactorConvertImport_partialSelection.ts => refactorConvertImport_notAtDefaultName.ts} (65%) diff --git a/tests/cases/fourslash/refactorConvertImport_partialSelection.ts b/tests/cases/fourslash/refactorConvertImport_notAtDefaultName.ts similarity index 65% rename from tests/cases/fourslash/refactorConvertImport_partialSelection.ts rename to tests/cases/fourslash/refactorConvertImport_notAtDefaultName.ts index 0f703d7436f8d..5f62bbb439446 100644 --- a/tests/cases/fourslash/refactorConvertImport_partialSelection.ts +++ b/tests/cases/fourslash/refactorConvertImport_notAtDefaultName.ts @@ -3,4 +3,4 @@ ////import /*a*/d/*b*/, * as n from "m"; goTo.select("a", "b"); -verify.refactorAvailable("Convert import"); +verify.not.refactorAvailable("Convert import"); From 4971c7d546a27c54693ac192894585da34557d6d Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:09:10 -0700 Subject: [PATCH 28/36] remove declaration --- tests/cases/fourslash/fourslash.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 460942de4c309..ce3eefcb4ac84 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -244,7 +244,6 @@ declare namespace FourSlashInterface { refactorAvailable(name: string, actionName?: string): void; refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void; - refactorNotAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void; } class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; From a7c07d67d8fbe1ed235db18ee979698565ae3994 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:10:12 -0700 Subject: [PATCH 29/36] convert export cursor only changes --- src/services/refactors/convertExport.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index a3fd2bc114026..cbcc4c13871aa 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -5,7 +5,7 @@ namespace ts.refactor { const actionNameNamedToDefault = "Convert named export to default export"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { - const info = getInfo(context); + const info = getInfo(context, context.triggerReason === "invoked"); if (!info) return emptyArray; const description = info.wasDefault ? Diagnostics.Convert_default_export_to_named_export.message : Diagnostics.Convert_named_export_to_default_export.message; const actionName = info.wasDefault ? actionNameDefaultToNamed : actionNameNamedToDefault; @@ -27,12 +27,12 @@ namespace ts.refactor { readonly exportingModuleSymbol: Symbol; } - function getInfo(context: RefactorContext): Info | undefined { + function getInfo(context: RefactorContext, userRequested = true): Info | undefined { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); - // If the span is entirely contained in an export node, check that node. - const exportNode = !!(getModifierFlags(token.parent) & ModifierFlags.Export) ? token.parent : getParentNodeInSpan(token, file, span); + const cursorRequest = userRequested && span; + const exportNode = !!(getModifierFlags(token.parent) & ModifierFlags.Export) && cursorRequest ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { return undefined; } From c20908a3e74757fcfe4eef01c5991080e74069f0 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:10:44 -0700 Subject: [PATCH 30/36] convert export trigger reason test --- .../fourslash/refactorConvertExportForTriggerReason.ts | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tests/cases/fourslash/refactorConvertExportForTriggerReason.ts diff --git a/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts b/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts new file mode 100644 index 0000000000000..c3cffbeec06eb --- /dev/null +++ b/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts @@ -0,0 +1,7 @@ +/// + +////export /*a*//*b*/function f() {} + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Convert export"); +verify.refactorAvailableForTriggerReason("invoked", "Convert export"); \ No newline at end of file From 06d2461cd0013b1a046c84c577698547a3693398 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:11:09 -0700 Subject: [PATCH 31/36] convert import trigger reason only --- src/services/refactors/convertImport.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 43120591e6b31..88c501925c15f 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -5,7 +5,7 @@ namespace ts.refactor { const actionNameNamedToNamespace = "Convert named imports to namespace import"; registerRefactor(refactorName, { getAvailableActions(context): readonly ApplicableRefactorInfo[] { - const i = getImportToConvert(context); + const i = getImportToConvert(context, context.triggerReason === "invoked"); if (!i) return emptyArray; const description = i.kind === SyntaxKind.NamespaceImport ? Diagnostics.Convert_namespace_import_to_named_imports.message : Diagnostics.Convert_named_imports_to_namespace_import.message; const actionName = i.kind === SyntaxKind.NamespaceImport ? actionNameNamespaceToNamed : actionNameNamedToNamespace; @@ -19,11 +19,12 @@ namespace ts.refactor { }); // Can convert imports of the form `import * as m from "m";` or `import d, { x, y } from "m";`. - function getImportToConvert(context: RefactorContext): NamedImportBindings | undefined { + function getImportToConvert(context: RefactorContext, userRequested = true): NamedImportBindings | undefined { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); - const importDecl = findAncestor(token, isImportDeclaration); + const cursorRequest = userRequested && span.length === 0; + const importDecl = cursorRequest ? findAncestor(token, isImportDeclaration) : getParentNodeInSpan(token, file, span); if (!importDecl || !isImportDeclaration(importDecl) || (importDecl.getEnd() < span.start + span.length)) return undefined; const { importClause } = importDecl; return importClause && importClause.namedBindings; From f4792792176498a0551f689ae50c27cc5c6f9d1c Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:11:33 -0700 Subject: [PATCH 32/36] convert import trigger reason test --- .../fourslash/refactorConvertImportForTriggerReason.ts | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tests/cases/fourslash/refactorConvertImportForTriggerReason.ts diff --git a/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts b/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts new file mode 100644 index 0000000000000..b192b114c1a70 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts @@ -0,0 +1,7 @@ +/// + +////import /*a*//*b*/d, * as n from "m"; + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Convert import"); +verify.refactorAvailableForTriggerReason("invoked", "Convert import"); \ No newline at end of file From 2db00544445e04b8ef384c9209bec402b57e17db Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:16:47 -0700 Subject: [PATCH 33/36] remove outdated tests --- .../refactorConvertExport_defaultToNamedValidSpans.ts | 9 --------- .../refactorConvertExport_namedToDefaultValidSpans.ts | 9 --------- .../refactorConvertImport_namedToNamespaceValidSpans.ts | 9 --------- .../refactorConvertImport_namespaceToNamedValidSpans.ts | 9 --------- 4 files changed, 36 deletions(-) delete mode 100644 tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts delete mode 100644 tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts delete mode 100644 tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts delete mode 100644 tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts diff --git a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts deleted file mode 100644 index e4322853da34b..0000000000000 --- a/tests/cases/fourslash/refactorConvertExport_defaultToNamedValidSpans.ts +++ /dev/null @@ -1,9 +0,0 @@ -/// - -/////*1a*/ex/*2a*/port def/*3a*//*3b*/ault functio/*2b*/n f() {}/*1b*/ - -// verify that the refactor is offered for full, partial, and empty spans. -for (const m of ["1", "2", "3"]) { - goTo.select(m + "a", m + "b"); - verify.refactorAvailableForTriggerReason("invoked", "Convert export", "Convert default export to named export"); -} diff --git a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts b/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts deleted file mode 100644 index f9955962437e4..0000000000000 --- a/tests/cases/fourslash/refactorConvertExport_namedToDefaultValidSpans.ts +++ /dev/null @@ -1,9 +0,0 @@ -/// - -/////*1a*/ex/*2a*/port f/*3a*//*3b*/unctio/*2b*/n f() {}/*1b*/ - -// verify that the refactor is offered for full, partial, and empty spans. -for (const m of ["1", "2", "3"]) { - goTo.select(m + "a", m + "b"); - verify.refactorAvailableForTriggerReason("invoked", "Convert export", "Convert named export to default export"); -} diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts deleted file mode 100644 index 4c36c84929c9c..0000000000000 --- a/tests/cases/fourslash/refactorConvertImport_namedToNamespaceValidSpans.ts +++ /dev/null @@ -1,9 +0,0 @@ -/// - -/////*1a*/im/*2a*/port { /*3a*//*3b*/x, y as z, T } fro/*2b*/m "m";/*1b*/ - -// verify that the refactor is offered for full, partial, and empty spans. -for (const m of ["1", "2", "3"]) { - goTo.select(m + "a", m + "b"); - verify.refactorAvailableForTriggerReason("invoked", "Convert import", "Convert named imports to namespace import"); -} diff --git a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts b/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts deleted file mode 100644 index 9b5bf353cb39e..0000000000000 --- a/tests/cases/fourslash/refactorConvertImport_namespaceToNamedValidSpans.ts +++ /dev/null @@ -1,9 +0,0 @@ -/// - -/////*1a*/im/*2a*/port * as /*3a*//*3b*/m from "m/*2b*/";/*1b*/ - -// verify that the refactor is offered for full, partial, and empty spans. -for (const m of ["1", "2", "3"]) { - goTo.select(m + "a", m + "b"); - verify.refactorAvailableForTriggerReason("invoked","Convert import", "Convert namespace import to named imports"); -} From a86a2fa7038337ff17b702772e18ef5a7f5e048b Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 11:57:19 -0700 Subject: [PATCH 34/36] polish tests --- ...efactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts} | 0 tests/cases/fourslash/refactorConvertExportForTriggerReason.ts | 3 ++- tests/cases/fourslash/refactorConvertImportForTriggerReason.ts | 3 ++- ....ts => refactorConvertToGetAndSetAccessForTriggerReason.ts} | 3 ++- ...TriggerReason.ts => refactorExtractTypeForTriggerReason.ts} | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) rename tests/cases/fourslash/{refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts => refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts} (100%) rename tests/cases/fourslash/{refactorConvertToGetAndSetAccessTriggerReason.ts => refactorConvertToGetAndSetAccessForTriggerReason.ts} (74%) rename tests/cases/fourslash/{refactorExtractTypeTriggerReason.ts => refactorExtractTypeForTriggerReason.ts} (95%) diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts similarity index 100% rename from tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionTriggerReason.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts diff --git a/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts b/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts index c3cffbeec06eb..9735acb5132b8 100644 --- a/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts +++ b/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts @@ -2,6 +2,7 @@ ////export /*a*//*b*/function f() {} +// Only offer refactor for empty span if explicity requested goTo.select("a", "b"); verify.not.refactorAvailableForTriggerReason("implicit", "Convert export"); -verify.refactorAvailableForTriggerReason("invoked", "Convert export"); \ No newline at end of file +verify.refactorAvailableForTriggerReason("invoked", "Convert export"); diff --git a/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts b/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts index b192b114c1a70..72e8a6f8e1b76 100644 --- a/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts +++ b/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts @@ -2,6 +2,7 @@ ////import /*a*//*b*/d, * as n from "m"; +// Only offer refactor for empty span if explicity requested goTo.select("a", "b"); verify.not.refactorAvailableForTriggerReason("implicit", "Convert import"); -verify.refactorAvailableForTriggerReason("invoked", "Convert import"); \ No newline at end of file +verify.refactorAvailableForTriggerReason("invoked", "Convert import"); diff --git a/tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts b/tests/cases/fourslash/refactorConvertToGetAndSetAccessForTriggerReason.ts similarity index 74% rename from tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts rename to tests/cases/fourslash/refactorConvertToGetAndSetAccessForTriggerReason.ts index f77de717dbf80..a6edb03c95f57 100644 --- a/tests/cases/fourslash/refactorConvertToGetAndSetAccessTriggerReason.ts +++ b/tests/cases/fourslash/refactorConvertToGetAndSetAccessForTriggerReason.ts @@ -4,6 +4,7 @@ //// public /*a*//*b*/a: string; //// } +// Only offer refactor for empty span if explicity requested goTo.select("a", "b"); verify.not.refactorAvailableForTriggerReason("implicit", "Generate 'get' and 'set' accessors"); -verify.refactorAvailableForTriggerReason("invoked", "Generate 'get' and 'set' accessors"); \ No newline at end of file +verify.refactorAvailableForTriggerReason("invoked", "Generate 'get' and 'set' accessors"); diff --git a/tests/cases/fourslash/refactorExtractTypeTriggerReason.ts b/tests/cases/fourslash/refactorExtractTypeForTriggerReason.ts similarity index 95% rename from tests/cases/fourslash/refactorExtractTypeTriggerReason.ts rename to tests/cases/fourslash/refactorExtractTypeForTriggerReason.ts index 019996fbbd000..4bc28336657d3 100644 --- a/tests/cases/fourslash/refactorExtractTypeTriggerReason.ts +++ b/tests/cases/fourslash/refactorExtractTypeForTriggerReason.ts @@ -5,4 +5,4 @@ // Only offer refactor for empty span if explicity requested goTo.select("a", "b"); verify.not.refactorAvailableForTriggerReason("implicit", "Extract type"); -verify.refactorAvailableForTriggerReason("invoked", "Extract type"); \ No newline at end of file +verify.refactorAvailableForTriggerReason("invoked", "Extract type"); From ca58c0e03cb6c5cc7530669507edce79673c0c42 Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Fri, 29 May 2020 14:04:07 -0700 Subject: [PATCH 35/36] fix merge conflicts --- src/services/codefixes/generateAccessors.ts | 5 +++-- src/services/refactors/convertExport.ts | 2 +- src/services/refactors/generateGetAccessorAndSetAccessor.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index 7fed793a2f1ac..904e52d29c586 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -104,12 +104,13 @@ namespace ts.codefix { return modifierFlags; } - export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number): Info | undefined { + export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, userRequested = true): Info | undefined { const node = getTokenAtPosition(file, start); + const cursorRequest = start === end && userRequested; const declaration = findAncestor(node.parent, isAcceptedDeclaration); // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; - if (!declaration || !nodeOverlapsWithStartEnd(declaration.name, file, start, end) + if (!declaration || !(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest) || !isConvertibleName(declaration.name) || (getEffectiveModifierFlags(declaration) | meaning) !== meaning) return undefined; const name = declaration.name.text; diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index cbcc4c13871aa..b6440de228444 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -32,7 +32,7 @@ namespace ts.refactor { const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); const cursorRequest = userRequested && span; - const exportNode = !!(getModifierFlags(token.parent) & ModifierFlags.Export) && cursorRequest ? token.parent : getParentNodeInSpan(token, file, span); + const exportNode = !!(getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && cursorRequest ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { return undefined; } diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index e942305f6b92c..53bdf728fbe94 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -19,7 +19,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { }, getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { if (!context.endPosition) return emptyArray; - if (!codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition)) return emptyArray; + if (!codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition, context.triggerReason === "invoked")) return emptyArray; return [{ name: actionName, From d88ea4e1f8eb6ebc90e79d90030488175485109d Mon Sep 17 00:00:00 2001 From: Jesse Trinity Date: Tue, 2 Jun 2020 19:06:12 -0700 Subject: [PATCH 36/36] address PR comments --- src/services/codefixes/generateAccessors.ts | 4 ++-- .../refactors/addOrRemoveBracesToArrowFunction.ts | 4 ++-- src/services/refactors/convertExport.ts | 5 ++--- src/services/refactors/convertImport.ts | 5 ++--- src/services/refactors/extractSymbol.ts | 6 +++--- src/services/refactors/extractType.ts | 4 ++-- ...rAddOrRemoveBracesToArrowFunctionForTriggerReason1.ts} | 2 +- ...orAddOrRemoveBracesToArrowFunctionForTriggerReason2.ts | 8 ++++++++ ...eason.ts => refactorConvertImportForTriggerReason1.ts} | 0 .../fourslash/refactorConvertImportForTriggerReason2.ts | 8 ++++++++ ...rReason.ts => refactorExtractTypeForTriggerReason1.ts} | 0 .../fourslash/refactorExtractTypeForTriggerReason2.ts | 7 +++++++ 12 files changed, 37 insertions(+), 16 deletions(-) rename tests/cases/fourslash/{refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts => refactorAddOrRemoveBracesToArrowFunctionForTriggerReason1.ts} (82%) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason2.ts rename tests/cases/fourslash/{refactorConvertImportForTriggerReason.ts => refactorConvertImportForTriggerReason1.ts} (100%) create mode 100644 tests/cases/fourslash/refactorConvertImportForTriggerReason2.ts rename tests/cases/fourslash/{refactorExtractTypeForTriggerReason.ts => refactorExtractTypeForTriggerReason1.ts} (100%) create mode 100644 tests/cases/fourslash/refactorExtractTypeForTriggerReason2.ts diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index 904e52d29c586..3c9e82e157cbb 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -104,9 +104,9 @@ namespace ts.codefix { return modifierFlags; } - export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, userRequested = true): Info | undefined { + export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, considerEmptySpans = true): Info | undefined { const node = getTokenAtPosition(file, start); - const cursorRequest = start === end && userRequested; + const cursorRequest = start === end && considerEmptySpans; const declaration = findAncestor(node.parent, isAcceptedDeclaration); // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 84e596c5e4116..5a11b862dd8d9 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -70,12 +70,12 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return { renameFilename: undefined, renameLocation: undefined, edits }; } - function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, userRequested = true): Info | undefined { + function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, considerFunctionBodies = true): Info | undefined { const node = getTokenAtPosition(file, startPosition); const func = getContainingFunction(node); // Only offer a refactor in the function body on explicit refactor requests. if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) - || (rangeContainsRange(func.body, node) && !userRequested))) return undefined; + || (rangeContainsRange(func.body, node) && !considerFunctionBodies))) return undefined; if (isExpression(func.body)) { return { diff --git a/src/services/refactors/convertExport.ts b/src/services/refactors/convertExport.ts index b6440de228444..c79833db1ea6b 100644 --- a/src/services/refactors/convertExport.ts +++ b/src/services/refactors/convertExport.ts @@ -27,12 +27,11 @@ namespace ts.refactor { readonly exportingModuleSymbol: Symbol; } - function getInfo(context: RefactorContext, userRequested = true): Info | undefined { + function getInfo(context: RefactorContext, considerPartialSpans = true): Info | undefined { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); - const cursorRequest = userRequested && span; - const exportNode = !!(getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && cursorRequest ? token.parent : getParentNodeInSpan(token, file, span); + const exportNode = !!(token.parent && getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && considerPartialSpans ? token.parent : getParentNodeInSpan(token, file, span); if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) { return undefined; } diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index 88c501925c15f..1383aa18ddbd2 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -19,12 +19,11 @@ namespace ts.refactor { }); // Can convert imports of the form `import * as m from "m";` or `import d, { x, y } from "m";`. - function getImportToConvert(context: RefactorContext, userRequested = true): NamedImportBindings | undefined { + function getImportToConvert(context: RefactorContext, considerPartialSpans = true): NamedImportBindings | undefined { const { file } = context; const span = getRefactorContextSpan(context); const token = getTokenAtPosition(file, span.start); - const cursorRequest = userRequested && span.length === 0; - const importDecl = cursorRequest ? findAncestor(token, isImportDeclaration) : getParentNodeInSpan(token, file, span); + const importDecl = considerPartialSpans ? findAncestor(token, isImportDeclaration) : getParentNodeInSpan(token, file, span); if (!importDecl || !isImportDeclaration(importDecl) || (importDecl.getEnd() < span.start + span.length)) return undefined; const { importClause } = importDecl; return importClause && importClause.namedBindings; diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 3da484a9ce630..5c274ffbe1817 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -186,12 +186,12 @@ namespace ts.refactor.extractSymbol { * not shown to the user, but can be used by us diagnostically) */ // exported only for tests - export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, userRequested = true): RangeToExtract { + export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, considerEmptySpans = true): RangeToExtract { const { length } = span; - if (length === 0 && !userRequested) { + if (length === 0 && !considerEmptySpans) { return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractEmpty)] }; } - const cursorRequest = length === 0 && userRequested; + const cursorRequest = length === 0 && considerEmptySpans; // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span. // This may fail (e.g. you select two statements in the root of a source file) diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 4941c4dffd6ac..491fec63bd016 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -58,12 +58,12 @@ namespace ts.refactor { type Info = TypeAliasInfo | InterfaceInfo; - function getRangeToExtract(context: RefactorContext, userRequested = true): Info | undefined { + function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): Info | undefined { const { file, startPosition } = context; const isJS = isSourceFileJS(file); const current = getTokenAtPosition(file, startPosition); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); - const cursorRequest = range.pos === range.end && userRequested; + const cursorRequest = range.pos === range.end && considerEmptySpans; const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) && (cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)))); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason1.ts similarity index 82% rename from tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason1.ts index a1e940fa7b1ee..4f5e41a8e3ae4 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason1.ts @@ -2,7 +2,7 @@ //// const a = (a: number) => { return/*a*//*b*/ a; }; -// Only offer refactor for empty span if explicity requested +// Only offer refactor for empty span in body if explicity requested goTo.select("a", "b"); verify.not.refactorAvailableForTriggerReason("implicit", "Add or remove braces in an arrow function"); verify.refactorAvailableForTriggerReason("invoked", "Add or remove braces in an arrow function", "Remove braces from arrow function"); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason2.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason2.ts new file mode 100644 index 0000000000000..5677e2c714450 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunctionForTriggerReason2.ts @@ -0,0 +1,8 @@ +/// + +//// const a = (a: number) => { re/*a*/tur/*b*/n a; }; + +// Only offer refactor in body if explicity requested +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Add or remove braces in an arrow function"); +verify.refactorAvailableForTriggerReason("invoked", "Add or remove braces in an arrow function", "Remove braces from arrow function"); diff --git a/tests/cases/fourslash/refactorConvertImportForTriggerReason.ts b/tests/cases/fourslash/refactorConvertImportForTriggerReason1.ts similarity index 100% rename from tests/cases/fourslash/refactorConvertImportForTriggerReason.ts rename to tests/cases/fourslash/refactorConvertImportForTriggerReason1.ts diff --git a/tests/cases/fourslash/refactorConvertImportForTriggerReason2.ts b/tests/cases/fourslash/refactorConvertImportForTriggerReason2.ts new file mode 100644 index 0000000000000..1404ade7348e2 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImportForTriggerReason2.ts @@ -0,0 +1,8 @@ +/// + +////import d, * as /*a*/n/*b*/ from "m"; + +// Only offer refactor for sub span if explicity requested +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Convert import"); +verify.refactorAvailableForTriggerReason("invoked", "Convert import"); diff --git a/tests/cases/fourslash/refactorExtractTypeForTriggerReason.ts b/tests/cases/fourslash/refactorExtractTypeForTriggerReason1.ts similarity index 100% rename from tests/cases/fourslash/refactorExtractTypeForTriggerReason.ts rename to tests/cases/fourslash/refactorExtractTypeForTriggerReason1.ts diff --git a/tests/cases/fourslash/refactorExtractTypeForTriggerReason2.ts b/tests/cases/fourslash/refactorExtractTypeForTriggerReason2.ts new file mode 100644 index 0000000000000..0f81c3cc88b84 --- /dev/null +++ b/tests/cases/fourslash/refactorExtractTypeForTriggerReason2.ts @@ -0,0 +1,7 @@ +/// + +//// var x: s/*a*/tr/*b*/ing; + +goTo.select("a", "b"); +verify.refactorAvailableForTriggerReason("implicit", "Extract type"); +verify.refactorAvailableForTriggerReason("invoked", "Extract type");