Skip to content

Commit

Permalink
Making Move To File Action appear less often (#57080)
Browse files Browse the repository at this point in the history
  • Loading branch information
aiday-mar authored Feb 20, 2024
1 parent 363abe8 commit 60f93aa
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 23 deletions.
8 changes: 4 additions & 4 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3979,8 +3979,8 @@ export class TestState {
};
}

public verifyRefactorAvailable(negative: boolean, triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string, actionDescription?: string) {
let refactors = this.getApplicableRefactorsAtSelection(triggerReason);
public verifyRefactorAvailable(negative: boolean, triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string, actionDescription?: string, kind?: string, preferences = ts.emptyOptions, includeInteractiveActions?: boolean) {
let refactors = this.getApplicableRefactorsAtSelection(triggerReason, kind, preferences, includeInteractiveActions);
refactors = refactors.filter(r => r.name === name);

if (actionName !== undefined) {
Expand Down Expand Up @@ -4445,8 +4445,8 @@ export class TestState {
test(renameKeys(newFileContents, key => pathUpdater(key) || key), "with file moved");
}

private getApplicableRefactorsAtSelection(triggerReason: ts.RefactorTriggerReason = "implicit", kind?: string, preferences = ts.emptyOptions) {
return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName, preferences, triggerReason, kind);
private getApplicableRefactorsAtSelection(triggerReason: ts.RefactorTriggerReason = "implicit", kind?: string, preferences = ts.emptyOptions, includeInteractiveActions?: boolean) {
return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName, preferences, triggerReason, kind, includeInteractiveActions);
}
private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason = "implicit", kind?: string, includeInteractiveActions?: boolean): readonly ts.ApplicableRefactorInfo[] {
return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences, triggerReason, kind, includeInteractiveActions); // eslint-disable-line local/no-in-operator
Expand Down
8 changes: 4 additions & 4 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,12 @@ export class VerifyNegatable {
this.state.verifyRefactorsAvailable(names);
}

public refactorAvailable(name: string, actionName?: string, actionDescription?: string) {
this.state.verifyRefactorAvailable(this.negative, "implicit", name, actionName, actionDescription);
public refactorAvailable(name: string, actionName?: string, actionDescription?: string, kind?: string, preferences = ts.emptyOptions, includeInteractiveActions?: boolean) {
this.state.verifyRefactorAvailable(this.negative, "implicit", name, actionName, actionDescription, kind, preferences, includeInteractiveActions);
}

public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) {
this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName);
public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string, actionDescription?: string, kind?: string, preferences = ts.emptyOptions, includeInteractiveActions?: boolean) {
this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName, actionDescription, kind, preferences, includeInteractiveActions);
}

public refactorKindAvailable(kind: string, expected: string[], preferences = ts.emptyOptions) {
Expand Down
14 changes: 1 addition & 13 deletions src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
assertType,
BindingElement,
Block,
BlockLike,
BreakStatement,
CancellationToken,
canHaveModifiers,
Expand Down Expand Up @@ -67,6 +66,7 @@ import {
isAssignmentExpression,
isBinaryExpression,
isBlock,
isBlockLike,
isBlockScope,
isCaseClause,
isClassLike,
Expand Down Expand Up @@ -2221,18 +2221,6 @@ function isExtractableExpression(node: Node): boolean {
return true;
}

function isBlockLike(node: Node): node is BlockLike {
switch (node.kind) {
case SyntaxKind.Block:
case SyntaxKind.SourceFile:
case SyntaxKind.ModuleBlock:
case SyntaxKind.CaseClause:
return true;
default:
return false;
}
}

function isInJSXContent(node: Node) {
return isStringLiteralJsxAttribute(node) ||
(isJsxElement(node) || isJsxSelfClosingElement(node) || isJsxFragment(node)) && (isJsxElement(node.parent) || isJsxFragment(node.parent));
Expand Down
13 changes: 13 additions & 0 deletions src/services/refactors/moveToFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
filter,
find,
FindAllReferences,
findAncestor,
findIndex,
findLast,
firstDefined,
Expand All @@ -58,6 +59,7 @@ import {
getRelativePathFromFile,
getSourceFileOfNode,
getSynthesizedDeepClone,
getTokenAtPosition,
getUniqueName,
hasJSFileExtension,
hasSyntacticModifier,
Expand All @@ -73,6 +75,7 @@ import {
isArrayLiteralExpression,
isBinaryExpression,
isBindingElement,
isBlockLike,
isDeclarationName,
isExportDeclaration,
isExportSpecifier,
Expand Down Expand Up @@ -165,6 +168,16 @@ registerRefactor(refactorNameForMoveToFile, {
if (!interactiveRefactorArguments) {
return emptyArray;
}
/** If the start/end nodes of the selection are inside a block like node do not show the `Move to file` code action
* This condition is used in order to show less often the `Move to file` code action */
if (context.endPosition !== undefined) {
const file = context.file;
const startNodeAncestor = findAncestor(getTokenAtPosition(file, context.startPosition), isBlockLike);
const endNodeAncestor = findAncestor(getTokenAtPosition(file, context.endPosition), isBlockLike);
if (startNodeAncestor && !isSourceFile(startNodeAncestor) && endNodeAncestor && !isSourceFile(endNodeAncestor)) {
return emptyArray;
}
}
if (context.preferences.allowTextChangesInNewFiles && statements) {
return [{ name: refactorNameForMoveToFile, description, actions: [moveToFileAction] }];
}
Expand Down
14 changes: 14 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
BinaryExpression,
binarySearchKey,
BindingElement,
BlockLike,
BreakOrContinueStatement,
CallExpression,
canHaveModifiers,
Expand Down Expand Up @@ -4274,3 +4275,16 @@ export function fileShouldUseJavaScriptRequire(file: SourceFile | string, progra
}
return preferRequire;
}

/** @internal */
export function isBlockLike(node: Node): node is BlockLike {
switch (node.kind) {
case SyntaxKind.Block:
case SyntaxKind.SourceFile:
case SyntaxKind.ModuleBlock:
case SyntaxKind.CaseClause:
return true;
default:
return false;
}
}
4 changes: 2 additions & 2 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ declare namespace FourSlashInterface {
codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void;
applicableRefactorAvailableForRange(): void;

refactorAvailable(name: string, actionName?: string, actionDescription?: string): void;
refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void;
refactorAvailable(name: string, actionName?: string, actionDescription?: string, kind?: string, preferences?: {}, includeInteractiveActions?: boolean): void;
refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string, actionDescription?: string, kind?: string, preferences?: {}, includeInteractiveActions?: boolean): void;
refactorKindAvailable(refactorKind: string, expected: string[], preferences?: {}): void;
}
class verify extends verifyNegatable {
Expand Down
18 changes: 18 additions & 0 deletions tests/cases/fourslash/moveToFile_refactorAvailable1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

////namespace ns {
//// /*a*/export function fn() {
//// }
//// fn();
//// /*b*/
////}

goTo.select("a", "b");
verify.not.refactorAvailable("Move to file",
/*actionName*/ undefined,
/*actionDescription*/ undefined,
/*kind*/ undefined,
{
allowTextChangesInNewFiles : true
},
/*includeInteractiveActions*/ true);
20 changes: 20 additions & 0 deletions tests/cases/fourslash/moveToFile_refactorAvailable2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />

/////*a*/
////namespace ns {
//// export function fn() {
//// }
//// fn();
////}
/////*b*/

goTo.select("a", "b");
verify.refactorAvailable("Move to file",
/*actionName*/ undefined,
/*actionDescription*/ undefined,
/*kind*/ undefined,
{
allowTextChangesInNewFiles : true
},
/*includeInteractiveActions*/ true);

0 comments on commit 60f93aa

Please sign in to comment.