Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making Move To File Action appear less often #57080

Merged
merged 26 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b8ea147
wip
aiday-mar Dec 20, 2023
1adf357
Merge branch 'microsoft:main' into main
aiday-mar Jan 17, 2024
9e8359e
adding changes
aiday-mar Jan 17, 2024
0547a88
formatting the code correctly
aiday-mar Jan 17, 2024
2472259
adding a missing comma
aiday-mar Jan 17, 2024
afc1fd2
adding baseline change
aiday-mar Jan 17, 2024
f0b00aa
review changes
aiday-mar Jan 18, 2024
d7eedd8
adding code from review
aiday-mar Feb 13, 2024
7d21d2d
code review changes
aiday-mar Feb 13, 2024
766a690
Merge branch 'main' of github.com:aiday-mar/TypeScript into pr/aiday-…
aiday-mar Feb 13, 2024
32fce2a
add back template file
aiday-mar Feb 13, 2024
bbff752
fixing formatting and linting errors
aiday-mar Feb 13, 2024
dea141a
making the preferences object also parameter for the tests
aiday-mar Feb 13, 2024
3b78378
fixing formatting errors
aiday-mar Feb 13, 2024
1cad4fe
adding changes
aiday-mar Feb 13, 2024
be3e49c
polishing
aiday-mar Feb 13, 2024
4e60710
removing duplicate type
aiday-mar Feb 13, 2024
e59f006
polishing tsts
aiday-mar Feb 13, 2024
1e74fc3
adding baseline change
aiday-mar Feb 13, 2024
db1078e
adding review changes
aiday-mar Feb 14, 2024
fa0a7d6
removing the rest of the rootNode code
aiday-mar Feb 14, 2024
89d5a88
adding review changes
aiday-mar Feb 16, 2024
154fcc5
adding a reason also into the comment
aiday-mar Feb 16, 2024
c6b461c
adding a semi colon
aiday-mar Feb 16, 2024
22c8fb3
changing to fetching the token at the end position
aiday-mar Feb 16, 2024
dcf2ffb
correcting the test
aiday-mar Feb 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3305,6 +3305,10 @@ export type BlockLike =
| ModuleBlock
| CaseOrDefaultClause;

export type RootNode =
| SourceFile
| Bundle;

export interface Block extends Statement, LocalsContainer {
readonly kind: SyntaxKind.Block;
readonly statements: NodeArray<Statement>;
Expand Down
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
27 changes: 27 additions & 0 deletions src/services/refactors/moveToFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
BinaryExpression,
BindingElement,
BindingName,
BlockLike,
CallExpression,
canHaveDecorators,
canHaveModifiers,
Expand Down Expand Up @@ -60,6 +61,7 @@ import {
getRelativePathFromFile,
getSourceFileOfNode,
getSynthesizedDeepClone,
getTokenAtPosition,
getUniqueName,
hasJSFileExtension,
hasSyntacticModifier,
Expand All @@ -75,6 +77,7 @@ import {
isArrayLiteralExpression,
isBinaryExpression,
isBindingElement,
isBlockLike,
isDeclarationName,
isExportDeclaration,
isExportSpecifier,
Expand All @@ -93,6 +96,7 @@ import {
isPropertyAccessExpression,
isPropertyAssignment,
isRequireCall,
isRootNode,
isSourceFile,
isStatement,
isStringLiteral,
Expand Down Expand Up @@ -126,6 +130,7 @@ import {
RequireOrImportCall,
RequireVariableStatement,
resolvePath,
RootNode,
ScriptTarget,
skipAlias,
some,
Expand Down Expand Up @@ -167,6 +172,17 @@ registerRefactor(refactorNameForMoveToFile, {
if (!interactiveRefactorArguments) {
return emptyArray;
}
const endPosition = context.endPosition;
if (endPosition !== undefined) {
aiday-mar marked this conversation as resolved.
Show resolved Hide resolved
const file = context.file;
const startNode = getTokenAtPosition(file, context.startPosition);
const endNode = getTokenAtPosition(file, endPosition);
const startNodeAncestor = getBlockLikeAncestorOrRootNode(startNode);
const endNodeAncestor = getBlockLikeAncestorOrRootNode(endNode);
aiday-mar marked this conversation as resolved.
Show resolved Hide resolved
if (!isRootNode(startNodeAncestor) && !isRootNode(endNodeAncestor)) {
aiday-mar marked this conversation as resolved.
Show resolved Hide resolved
return emptyArray;
aiday-mar marked this conversation as resolved.
Show resolved Hide resolved
}
}
if (context.preferences.allowTextChangesInNewFiles && statements) {
return [{ name: refactorNameForMoveToFile, description, actions: [moveToFileAction] }];
}
Expand Down Expand Up @@ -274,6 +290,17 @@ function getNewStatementsAndRemoveFromOldFile(
];
}

function getBlockLikeAncestorOrRootNode(node: Node): BlockLike | RootNode {
let currentNode = node;
while (!isRootNode(currentNode)) {
aiday-mar marked this conversation as resolved.
Show resolved Hide resolved
if (isBlockLike(currentNode)) {
break;
}
currentNode = currentNode.parent;
}
return currentNode;
}

function getTargetFileImportsAndAddExportInOldFile(
oldFile: SourceFile,
targetFile: string,
Expand Down
26 changes: 26 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 @@ -310,6 +311,7 @@ import {
pseudoBigIntToString,
QualifiedName,
RefactorContext,
RootNode,
Scanner,
ScriptElementKind,
ScriptElementKindModifier,
Expand Down Expand Up @@ -4274,3 +4276,27 @@ 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;
}
}

/** @internal */
export function isRootNode(node: Node): node is RootNode {
switch (node.kind) {
case SyntaxKind.SourceFile:
case SyntaxKind.Bundle:
return true;
default:
return false;
}
}
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5789,6 +5789,7 @@ declare namespace ts {
readonly name?: Identifier;
}
type BlockLike = SourceFile | Block | ModuleBlock | CaseOrDefaultClause;
type RootNode = SourceFile | Bundle;
interface Block extends Statement, LocalsContainer {
readonly kind: SyntaxKind.Block;
readonly statements: NodeArray<Statement>;
Expand Down
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/refactorMoveToFile1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />
aiday-mar marked this conversation as resolved.
Show resolved Hide resolved

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

goTo.select("a", "b");
verify.not.refactorAvailable("Move to file",
undefined,
undefined,
undefined,
{
allowTextChangesInNewFiles : true
},
true);
20 changes: 20 additions & 0 deletions tests/cases/fourslash/refactorMoveToFile2.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",
undefined,
undefined,
undefined,
{
allowTextChangesInNewFiles : true
},
true);

Loading