Skip to content

Commit 3b15b35

Browse files
jessetrinityjessetrinity
authored
Merge pull request microsoft#38378 from jessetrinity/refactorTriggerReason
Add RefactorTriggerReason
2 parents 51bf887 + d88ea4e commit 3b15b35

26 files changed

+149
-43
lines changed

‎src/harness/fourslashImpl.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3214,8 +3214,8 @@ namespace FourSlash {
32143214
};
32153215
}
32163216

3217-
public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) {
3218-
let refactors = this.getApplicableRefactorsAtSelection();
3217+
public verifyRefactorAvailable(negative: boolean, triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) {
3218+
let refactors = this.getApplicableRefactorsAtSelection(triggerReason);
32193219
refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)));
32203220
const isAvailable = refactors.length > 0;
32213221

@@ -3644,14 +3644,14 @@ namespace FourSlash {
36443644
test(renameKeys(newFileContents, key => pathUpdater(key) || key), "with file moved");
36453645
}
36463646

3647-
private getApplicableRefactorsAtSelection() {
3648-
return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName);
3647+
private getApplicableRefactorsAtSelection(triggerReason: ts.RefactorTriggerReason = "implicit") {
3648+
return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName, ts.emptyOptions, triggerReason);
36493649
}
3650-
private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions): readonly ts.ApplicableRefactorInfo[] {
3651-
return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences); // eslint-disable-line no-in-operator
3650+
private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason = "implicit"): readonly ts.ApplicableRefactorInfo[] {
3651+
return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences, triggerReason); // eslint-disable-line no-in-operator
36523652
}
3653-
private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions): readonly ts.ApplicableRefactorInfo[] {
3654-
return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences) || ts.emptyArray;
3653+
private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason): readonly ts.ApplicableRefactorInfo[] {
3654+
return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences, triggerReason) || ts.emptyArray;
36553655
}
36563656

36573657
public configurePlugin(pluginName: string, configuration: any): void {

‎src/harness/fourslashInterfaceImpl.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,11 @@ namespace FourSlashInterface {
208208
}
209209

210210
public refactorAvailable(name: string, actionName?: string) {
211-
this.state.verifyRefactorAvailable(this.negative, name, actionName);
211+
this.state.verifyRefactorAvailable(this.negative, "implicit", name, actionName);
212+
}
213+
214+
public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) {
215+
this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName);
212216
}
213217
}
214218

‎src/server/protocol.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,11 @@ namespace ts.server.protocol {
546546
command: CommandTypes.GetApplicableRefactors;
547547
arguments: GetApplicableRefactorsRequestArgs;
548548
}
549-
export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs;
549+
export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & {
550+
triggerReason?: RefactorTriggerReason
551+
};
552+
553+
export type RefactorTriggerReason = "implicit" | "invoked";
550554

551555
/**
552556
* Response is a list of available refactorings.

‎src/server/session.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1970,7 +1970,7 @@ namespace ts.server {
19701970
private getApplicableRefactors(args: protocol.GetApplicableRefactorsRequestArgs): protocol.ApplicableRefactorInfo[] {
19711971
const { file, project } = this.getFileAndProject(args);
19721972
const scriptInfo = project.getScriptInfoForNormalizedPath(file)!;
1973-
return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file));
1973+
return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason);
19741974
}
19751975

19761976
private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo {

‎src/services/codefixes/generateAccessors.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,13 @@ namespace ts.codefix {
104104
return modifierFlags;
105105
}
106106

107-
export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number): Info | undefined {
107+
export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, considerEmptySpans = true): Info | undefined {
108108
const node = getTokenAtPosition(file, start);
109+
const cursorRequest = start === end && considerEmptySpans;
109110
const declaration = findAncestor(node.parent, isAcceptedDeclaration);
110111
// make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier
111112
const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly;
112-
if (!declaration || !nodeOverlapsWithStartEnd(declaration.name, file, start, end)
113+
if (!declaration || !(nodeOverlapsWithStartEnd(declaration.name, file, start, end) || cursorRequest)
113114
|| !isConvertibleName(declaration.name) || (getEffectiveModifierFlags(declaration) | meaning) !== meaning) return undefined;
114115

115116
const name = declaration.name.text;

‎src/services/refactors/addOrRemoveBracesToArrowFunction.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
1616
}
1717

1818
function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] {
19-
const { file, startPosition } = context;
20-
const info = getConvertibleArrowFunctionAtPosition(file, startPosition);
19+
const { file, startPosition, triggerReason } = context;
20+
const info = getConvertibleArrowFunctionAtPosition(file, startPosition, triggerReason === "invoked");
2121
if (!info) return emptyArray;
2222

2323
return [{
@@ -70,10 +70,12 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
7070
return { renameFilename: undefined, renameLocation: undefined, edits };
7171
}
7272

73-
function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined {
73+
function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, considerFunctionBodies = true): Info | undefined {
7474
const node = getTokenAtPosition(file, startPosition);
7575
const func = getContainingFunction(node);
76-
if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) return undefined;
76+
// Only offer a refactor in the function body on explicit refactor requests.
77+
if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node)
78+
|| (rangeContainsRange(func.body, node) && !considerFunctionBodies))) return undefined;
7779

7880
if (isExpression(func.body)) {
7981
return {

‎src/services/refactors/convertExport.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace ts.refactor {
55
const actionNameNamedToDefault = "Convert named export to default export";
66
registerRefactor(refactorName, {
77
getAvailableActions(context): readonly ApplicableRefactorInfo[] {
8-
const info = getInfo(context);
8+
const info = getInfo(context, context.triggerReason === "invoked");
99
if (!info) return emptyArray;
1010
const description = info.wasDefault ? Diagnostics.Convert_default_export_to_named_export.message : Diagnostics.Convert_named_export_to_default_export.message;
1111
const actionName = info.wasDefault ? actionNameDefaultToNamed : actionNameNamedToDefault;
@@ -27,11 +27,11 @@ namespace ts.refactor {
2727
readonly exportingModuleSymbol: Symbol;
2828
}
2929

30-
function getInfo(context: RefactorContext): Info | undefined {
30+
function getInfo(context: RefactorContext, considerPartialSpans = true): Info | undefined {
3131
const { file } = context;
3232
const span = getRefactorContextSpan(context);
3333
const token = getTokenAtPosition(file, span.start);
34-
const exportNode = getParentNodeInSpan(token, file, span);
34+
const exportNode = !!(token.parent && getSyntacticModifierFlags(token.parent) & ModifierFlags.Export) && considerPartialSpans ? token.parent : getParentNodeInSpan(token, file, span);
3535
if (!exportNode || (!isSourceFile(exportNode.parent) && !(isModuleBlock(exportNode.parent) && isAmbientModule(exportNode.parent.parent)))) {
3636
return undefined;
3737
}

‎src/services/refactors/convertImport.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace ts.refactor {
55
const actionNameNamedToNamespace = "Convert named imports to namespace import";
66
registerRefactor(refactorName, {
77
getAvailableActions(context): readonly ApplicableRefactorInfo[] {
8-
const i = getImportToConvert(context);
8+
const i = getImportToConvert(context, context.triggerReason === "invoked");
99
if (!i) return emptyArray;
1010
const description = i.kind === SyntaxKind.NamespaceImport ? Diagnostics.Convert_namespace_import_to_named_imports.message : Diagnostics.Convert_named_imports_to_namespace_import.message;
1111
const actionName = i.kind === SyntaxKind.NamespaceImport ? actionNameNamespaceToNamed : actionNameNamedToNamespace;
@@ -19,12 +19,12 @@ namespace ts.refactor {
1919
});
2020

2121
// Can convert imports of the form `import * as m from "m";` or `import d, { x, y } from "m";`.
22-
function getImportToConvert(context: RefactorContext): NamedImportBindings | undefined {
22+
function getImportToConvert(context: RefactorContext, considerPartialSpans = true): NamedImportBindings | undefined {
2323
const { file } = context;
2424
const span = getRefactorContextSpan(context);
2525
const token = getTokenAtPosition(file, span.start);
26-
const importDecl = getParentNodeInSpan(token, file, span);
27-
if (!importDecl || !isImportDeclaration(importDecl)) return undefined;
26+
const importDecl = considerPartialSpans ? findAncestor(token, isImportDeclaration) : getParentNodeInSpan(token, file, span);
27+
if (!importDecl || !isImportDeclaration(importDecl) || (importDecl.getEnd() < span.start + span.length)) return undefined;
2828
const { importClause } = importDecl;
2929
return importClause && importClause.namedBindings;
3030
}

‎src/services/refactors/extractSymbol.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace ts.refactor.extractSymbol {
88
* Exported for tests.
99
*/
1010
export function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] {
11-
const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context));
11+
const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context), context.triggerReason === "invoked");
1212

1313
const targetRange = rangeToExtract.targetRange;
1414
if (targetRange === undefined) {
@@ -186,18 +186,20 @@ namespace ts.refactor.extractSymbol {
186186
* not shown to the user, but can be used by us diagnostically)
187187
*/
188188
// exported only for tests
189-
export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan): RangeToExtract {
189+
export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, considerEmptySpans = true): RangeToExtract {
190190
const { length } = span;
191-
192-
if (length === 0) {
191+
if (length === 0 && !considerEmptySpans) {
193192
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractEmpty)] };
194193
}
194+
const cursorRequest = length === 0 && considerEmptySpans;
195195

196196
// Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span.
197197
// This may fail (e.g. you select two statements in the root of a source file)
198-
const start = getParentNodeInSpan(getTokenAtPosition(sourceFile, span.start), sourceFile, span);
198+
const startToken = getTokenAtPosition(sourceFile, span.start);
199+
const start = cursorRequest ? getExtractableParent(startToken): getParentNodeInSpan(startToken, sourceFile, span);
199200
// Do the same for the ending position
200-
const end = getParentNodeInSpan(findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)), sourceFile, span);
201+
const endToken = findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span));
202+
const end = cursorRequest ? start : getParentNodeInSpan(endToken, sourceFile, span);
201203

202204
const declarations: Symbol[] = [];
203205

@@ -1846,6 +1848,10 @@ namespace ts.refactor.extractSymbol {
18461848
}
18471849
}
18481850

1851+
function getExtractableParent(node: Node | undefined): Node | undefined {
1852+
return findAncestor(node, node => node.parent && isExtractableExpression(node) && !isBinaryExpression(node.parent));
1853+
}
1854+
18491855
/**
18501856
* Computes whether or not a node represents an expression in a position where it could
18511857
* be extracted.

‎src/services/refactors/extractType.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace ts.refactor {
66
const extractToTypeDef = "Extract to typedef";
77
registerRefactor(refactorName, {
88
getAvailableActions(context): readonly ApplicableRefactorInfo[] {
9-
const info = getRangeToExtract(context);
9+
const info = getRangeToExtract(context, context.triggerReason === "invoked");
1010
if (!info) return emptyArray;
1111

1212
return [{
@@ -22,7 +22,7 @@ namespace ts.refactor {
2222
}];
2323
},
2424
getEditsForAction(context, actionName): RefactorEditInfo {
25-
const { file } = context;
25+
const { file, } = context;
2626
const info = Debug.checkDefined(getRangeToExtract(context), "Expected to find a range to extract");
2727

2828
const name = getUniqueName("NewType", file);
@@ -58,13 +58,15 @@ namespace ts.refactor {
5858

5959
type Info = TypeAliasInfo | InterfaceInfo;
6060

61-
function getRangeToExtract(context: RefactorContext): Info | undefined {
61+
function getRangeToExtract(context: RefactorContext, considerEmptySpans = true): Info | undefined {
6262
const { file, startPosition } = context;
6363
const isJS = isSourceFileJS(file);
6464
const current = getTokenAtPosition(file, startPosition);
6565
const range = createTextRangeFromSpan(getRefactorContextSpan(context));
66+
const cursorRequest = range.pos === range.end && considerEmptySpans;
6667

67-
const selection = findAncestor(current, (node => node.parent && rangeContainsSkipTrivia(range, node, file) && !rangeContainsSkipTrivia(range, node.parent, file)));
68+
const selection = findAncestor(current, (node => node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) &&
69+
(cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end))));
6870
if (!selection || !isTypeNode(selection)) return undefined;
6971

7072
const checker = context.program.getTypeChecker();

0 commit comments

Comments
 (0)