From 062fdfd2b38689cfa8bf33508c3ab82e9af4bbdc Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 23 May 2023 11:30:13 -0700 Subject: [PATCH 1/4] don't use text change's create new file for existing empty file --- src/harness/client.ts | 38 ++++++++++++++++--- src/services/refactors/moveToFile.ts | 15 +++++++- src/services/textChanges.ts | 19 ++++++++++ src/services/tsconfig.json | 2 +- src/services/types.ts | 2 +- .../reference/api/tsserverlibrary.d.ts | 2 +- tests/baselines/reference/api/typescript.d.ts | 2 +- .../fourslash/moveToFile_blankExistingFile.ts | 9 +++-- .../moveToFile_differentDirectories2.ts | 9 +++-- .../server/moveToFile_emptyTargetFile.ts | 27 +++++++++++++ 10 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 tests/cases/fourslash/server/moveToFile_emptyTargetFile.ts diff --git a/src/harness/client.ts b/src/harness/client.ts index a97fde1279ed9..2a9b1e77de9cb 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -1,3 +1,4 @@ +import { GetApplicableRefactorsRequestArgs, GetEditsForRefactorRequestArgs } from "../server/protocol.js"; import { ApplicableRefactorInfo, CallHierarchyIncomingCall, @@ -36,6 +37,7 @@ import { ImplementationLocation, InlayHint, InlayHintKind, + InteractiveRefactorArguments, isString, JSDocTagInfo, LanguageService, @@ -52,6 +54,7 @@ import { Program, QuickInfo, RefactorEditInfo, + RefactorTriggerReason, ReferencedSymbol, ReferenceEntry, RenameInfo, @@ -787,11 +790,25 @@ export class SessionClient implements LanguageService { return { file, line, offset, endLine, endOffset }; } - getApplicableRefactors(fileName: string, positionOrRange: number | TextRange): ApplicableRefactorInfo[] { - const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName); - + getApplicableRefactors( + fileName: string, + positionOrRange: number | TextRange, + preferences: UserPreferences | undefined, + triggerReason?: RefactorTriggerReason, + kind?: string, + includeInteractiveActions?: boolean): ApplicableRefactorInfo[] { + if (preferences) { // Temporarily set preferences + this.configure(preferences); + } + const args: GetApplicableRefactorsRequestArgs = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName); + args.triggerReason = triggerReason; + args.kind = kind; + args.includeInteractiveActions = includeInteractiveActions; const request = this.processRequest(protocol.CommandTypes.GetApplicableRefactors, args); const response = this.processResponse(request); + if (preferences) { // Restore preferences + this.configure(this.preferences || {}); + } return response.body!; // TODO: GH#18217 } @@ -808,11 +825,16 @@ export class SessionClient implements LanguageService { _formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, - actionName: string): RefactorEditInfo { - - const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs; + actionName: string, + preferences: UserPreferences | undefined, + interactiveRefactorArguments?: InteractiveRefactorArguments): RefactorEditInfo { // >> Update here + if (preferences) { // Temporarily set preferences + this.configure(preferences); + } + const args: GetEditsForRefactorRequestArgs = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs; args.refactor = refactorName; args.action = actionName; + args.interactiveRefactorArguments = interactiveRefactorArguments; const request = this.processRequest(protocol.CommandTypes.GetEditsForRefactor, args); const response = this.processResponse(request); @@ -829,6 +851,10 @@ export class SessionClient implements LanguageService { renameLocation = this.lineOffsetToPosition(renameFilename, response.body.renameLocation!); } + if (preferences) { // Restore preferences + this.configure(this.preferences || {}); + } + return { edits, renameFilename, diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index aa566694e8a9b..3699ae8c56dee 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -177,7 +177,7 @@ function doChange(context: RefactorContext, oldFile: SourceFile, targetFile: str const checker = program.getTypeChecker(); const usage = getUsageInfo(oldFile, toMove.all, checker); //For a new file or an existing blank target file - if (!host.fileExists(targetFile) || host.fileExists(targetFile) && program.getSourceFile(targetFile)?.statements.length === 0) { + if (!host.fileExists(targetFile)) { changes.createNewFile(oldFile, targetFile, getNewStatementsAndRemoveFromOldFile(oldFile, targetFile, usage, changes, toMove, program, host, preferences)); addNewFileToTsconfig(program, changes, oldFile.fileName, targetFile, hostGetCanonicalFileName(host)); } @@ -189,7 +189,15 @@ function doChange(context: RefactorContext, oldFile: SourceFile, targetFile: str } function getNewStatementsAndRemoveFromOldFile( - oldFile: SourceFile, targetFile: string | SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, host: LanguageServiceHost, preferences: UserPreferences, importAdder?: codefix.ImportAdder + oldFile: SourceFile, + targetFile: string | SourceFile, + usage: UsageInfo, + changes: textChanges.ChangeTracker, + toMove: ToMove, + program: Program, + host: LanguageServiceHost, + preferences: UserPreferences, + importAdder?: codefix.ImportAdder ) { const checker = program.getTypeChecker(); const prologueDirectives = takeWhile(oldFile.statements, isPrologueDirective); @@ -218,6 +226,9 @@ function getNewStatementsAndRemoveFromOldFile( if (targetFile.statements.length > 0) { changes.insertNodesAfter(targetFile, targetFile.statements[targetFile.statements.length - 1], body); } + else { + changes.insertNodesAtEndOfFile(targetFile, body, /*blankLineBetween*/ false); + } if (imports.length > 0) { insertImports(changes, targetFile, imports, /*blankLineBetween*/ true, preferences); } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 3579b47fbcfde..eb5188250deea 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -631,6 +631,25 @@ export class ChangeTracker { } } + public insertNodesAtEndOfFile( + sourceFile: SourceFile, + newNodes: readonly Statement[], + blankLineBetween: boolean): void { + this.insertAtEndOfFile(sourceFile, newNodes, blankLineBetween); + } + + private insertAtEndOfFile( + sourceFile: SourceFile, + insert: readonly Statement[], + blankLineBetween: boolean): void { + const pos = sourceFile.end + 1; + const options = { + prefix: this.newLineCharacter, + suffix: blankLineBetween ? this.newLineCharacter : "", + }; + this.insertNodesAt(sourceFile, pos, insert, options); + } + private insertStatementsInNewFile(fileName: string, statements: readonly (Statement | SyntaxKind.NewLineTrivia)[], oldFile?: SourceFile): void { if (!this.newFileChanges) { this.newFileChanges = createMultiMap(); diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 42c124fdd8156..529ec2e70b145 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -6,5 +6,5 @@ { "path": "../compiler" }, { "path": "../jsTyping" } ], - "include": ["**/*"] + "include": ["**/*", "refactors/moveToFile.ts"] } diff --git a/src/services/types.ts b/src/services/types.ts index 57daf3dd85e62..ace7789a0efc6 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -652,7 +652,7 @@ export interface LanguageService { * arguments for any interactive action before offering it. */ getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string, includeInteractiveActions?: boolean): ApplicableRefactorInfo[]; - getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined, includeInteractiveActions?: InteractiveRefactorArguments): RefactorEditInfo | undefined; + getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined, interactiveRefactorArguments?: InteractiveRefactorArguments): RefactorEditInfo | undefined; getMoveToRefactoringFileSuggestions(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string): { newFileName: string, files: string[] }; organizeImports(args: OrganizeImportsArgs, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[]; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index af2e3d634e957..56c4b49ff6df2 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -10196,7 +10196,7 @@ declare namespace ts { * arguments for any interactive action before offering it. */ getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string, includeInteractiveActions?: boolean): ApplicableRefactorInfo[]; - getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined, includeInteractiveActions?: InteractiveRefactorArguments): RefactorEditInfo | undefined; + getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined, interactiveRefactorArguments?: InteractiveRefactorArguments): RefactorEditInfo | undefined; getMoveToRefactoringFileSuggestions(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string): { newFileName: string; files: string[]; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index e91614119d1c3..2b642a091ce2e 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -6227,7 +6227,7 @@ declare namespace ts { * arguments for any interactive action before offering it. */ getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string, includeInteractiveActions?: boolean): ApplicableRefactorInfo[]; - getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined, includeInteractiveActions?: InteractiveRefactorArguments): RefactorEditInfo | undefined; + getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined, interactiveRefactorArguments?: InteractiveRefactorArguments): RefactorEditInfo | undefined; getMoveToRefactoringFileSuggestions(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string): { newFileName: string; files: string[]; diff --git a/tests/cases/fourslash/moveToFile_blankExistingFile.ts b/tests/cases/fourslash/moveToFile_blankExistingFile.ts index dda33fca96f55..15788924fe652 100644 --- a/tests/cases/fourslash/moveToFile_blankExistingFile.ts +++ b/tests/cases/fourslash/moveToFile_blankExistingFile.ts @@ -18,11 +18,14 @@ verify.moveToFile({ `, "/bar.ts": -`import { b } from './other'; +`// import { p } from './a'; -const y: Date = p + b; -`, + +import { b } from "./other"; + + +const y: Date = p + b;`, }, interactiveRefactorArguments: {targetFile: "/bar.ts"}, }); diff --git a/tests/cases/fourslash/moveToFile_differentDirectories2.ts b/tests/cases/fourslash/moveToFile_differentDirectories2.ts index 4dba2881f189c..62952b7fc83c5 100644 --- a/tests/cases/fourslash/moveToFile_differentDirectories2.ts +++ b/tests/cases/fourslash/moveToFile_differentDirectories2.ts @@ -23,11 +23,12 @@ export const a = 10; y;`, "/src/dir2/bar.ts": -`import { b } from '../dir1/other'; -import { a } from '../dir1/a'; +`import { a } from '../dir1/a'; -export const y = b + a; -`, +import { b } from "../dir1/other"; + + +export const y = b + a;`, }, interactiveRefactorArguments: { targetFile: "/src/dir2/bar.ts" } }); diff --git a/tests/cases/fourslash/server/moveToFile_emptyTargetFile.ts b/tests/cases/fourslash/server/moveToFile_emptyTargetFile.ts new file mode 100644 index 0000000000000..18429d0672915 --- /dev/null +++ b/tests/cases/fourslash/server/moveToFile_emptyTargetFile.ts @@ -0,0 +1,27 @@ +/// + +// @Filename: /source.ts +////[|export const a = 1;|] +////const b = 2; +////console.log(a, b); + +// @Filename: /target.ts +/////** empty */ + +// @Filename: /tsconfig.json +///// { "compilerOptions": { "newLine": "lf" } } + +verify.moveToFile({ + newFileContents: { + "/source.ts": +`import { a } from "./target"; + +const b = 2; +console.log(a, b);`, + "/target.ts": +`/** empty */ +export const a = 1; +`, + }, + interactiveRefactorArguments: { targetFile: "/target.ts" }, +}); From 9176b4afa1e43c7907d22e661f91f3320474b998 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 23 May 2023 11:36:42 -0700 Subject: [PATCH 2/4] minor fixes --- src/harness/client.ts | 2 +- src/services/refactors/moveToFile.ts | 2 +- src/services/tsconfig.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/harness/client.ts b/src/harness/client.ts index 2a9b1e77de9cb..bac4aa3d566e1 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -827,7 +827,7 @@ export class SessionClient implements LanguageService { refactorName: string, actionName: string, preferences: UserPreferences | undefined, - interactiveRefactorArguments?: InteractiveRefactorArguments): RefactorEditInfo { // >> Update here + interactiveRefactorArguments?: InteractiveRefactorArguments): RefactorEditInfo { if (preferences) { // Temporarily set preferences this.configure(preferences); } diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index 3699ae8c56dee..3b0a0c22ab8e3 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -176,7 +176,7 @@ registerRefactor(refactorNameForMoveToFile, { function doChange(context: RefactorContext, oldFile: SourceFile, targetFile: string, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void { const checker = program.getTypeChecker(); const usage = getUsageInfo(oldFile, toMove.all, checker); - //For a new file or an existing blank target file + //For a new file if (!host.fileExists(targetFile)) { changes.createNewFile(oldFile, targetFile, getNewStatementsAndRemoveFromOldFile(oldFile, targetFile, usage, changes, toMove, program, host, preferences)); addNewFileToTsconfig(program, changes, oldFile.fileName, targetFile, hostGetCanonicalFileName(host)); diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 529ec2e70b145..42c124fdd8156 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -6,5 +6,5 @@ { "path": "../compiler" }, { "path": "../jsTyping" } ], - "include": ["**/*", "refactors/moveToFile.ts"] + "include": ["**/*"] } From d1d9174731008ec90d13777402573aecfc461412 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Tue, 23 May 2023 11:49:28 -0700 Subject: [PATCH 3/4] fix suffix newline --- src/services/textChanges.ts | 2 +- tests/cases/fourslash/moveToFile_blankExistingFile.ts | 3 ++- tests/cases/fourslash/moveToFile_differentDirectories2.ts | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index eb5188250deea..d52ab642a5e5d 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -645,7 +645,7 @@ export class ChangeTracker { const pos = sourceFile.end + 1; const options = { prefix: this.newLineCharacter, - suffix: blankLineBetween ? this.newLineCharacter : "", + suffix: this.newLineCharacter + (blankLineBetween ? this.newLineCharacter : ""), }; this.insertNodesAt(sourceFile, pos, insert, options); } diff --git a/tests/cases/fourslash/moveToFile_blankExistingFile.ts b/tests/cases/fourslash/moveToFile_blankExistingFile.ts index 15788924fe652..7cccbec7c47bc 100644 --- a/tests/cases/fourslash/moveToFile_blankExistingFile.ts +++ b/tests/cases/fourslash/moveToFile_blankExistingFile.ts @@ -25,7 +25,8 @@ import { p } from './a'; import { b } from "./other"; -const y: Date = p + b;`, +const y: Date = p + b; +`, }, interactiveRefactorArguments: {targetFile: "/bar.ts"}, }); diff --git a/tests/cases/fourslash/moveToFile_differentDirectories2.ts b/tests/cases/fourslash/moveToFile_differentDirectories2.ts index 62952b7fc83c5..5c03c2bb6cfe2 100644 --- a/tests/cases/fourslash/moveToFile_differentDirectories2.ts +++ b/tests/cases/fourslash/moveToFile_differentDirectories2.ts @@ -28,7 +28,8 @@ y;`, import { b } from "../dir1/other"; -export const y = b + a;`, +export const y = b + a; +`, }, interactiveRefactorArguments: { targetFile: "/src/dir2/bar.ts" } }); From 3dd77cf885f5df9e58ae5ec74dd039f94cad9b67 Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Wed, 24 May 2023 10:08:43 -0700 Subject: [PATCH 4/4] fix import --- src/harness/client.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/harness/client.ts b/src/harness/client.ts index bac4aa3d566e1..e513f79b3265b 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -1,4 +1,3 @@ -import { GetApplicableRefactorsRequestArgs, GetEditsForRefactorRequestArgs } from "../server/protocol.js"; import { ApplicableRefactorInfo, CallHierarchyIncomingCall, @@ -800,7 +799,7 @@ export class SessionClient implements LanguageService { if (preferences) { // Temporarily set preferences this.configure(preferences); } - const args: GetApplicableRefactorsRequestArgs = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName); + const args: protocol.GetApplicableRefactorsRequestArgs = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName); args.triggerReason = triggerReason; args.kind = kind; args.includeInteractiveActions = includeInteractiveActions; @@ -831,7 +830,8 @@ export class SessionClient implements LanguageService { if (preferences) { // Temporarily set preferences this.configure(preferences); } - const args: GetEditsForRefactorRequestArgs = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs; + const args = + this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs; args.refactor = refactorName; args.action = actionName; args.interactiveRefactorArguments = interactiveRefactorArguments;