Skip to content

Commit 886a937

Browse files
author
Andy Hanson
committed
Code review
1 parent 7051e32 commit 886a937

File tree

7 files changed

+41
-11
lines changed

7 files changed

+41
-11
lines changed

src/compiler/core.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2290,7 +2290,7 @@ namespace ts {
22902290
}
22912291

22922292
export function getRelativePathFromFile(from: string, to: string, getCanonicalFileName: GetCanonicalFileName) {
2293-
return ensurePathIsNonModuleName(getRelativePathFromDirectory(ts.getDirectoryPath(from), to, getCanonicalFileName));
2293+
return ensurePathIsNonModuleName(getRelativePathFromDirectory(getDirectoryPath(from), to, getCanonicalFileName));
22942294
}
22952295

22962296
/**

src/harness/fourslash.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3155,6 +3155,16 @@ Actual: ${stringify(fullActual)}`);
31553155
}
31563156
}
31573157

3158+
public noMoveToNewFile() {
3159+
for (const range of this.getRanges()) {
3160+
for (const refactor of this.getApplicableRefactors(range, { allowTextChangesInNewFiles: true })) {
3161+
if (refactor.name === "Move to a new file") {
3162+
ts.Debug.fail("Did not expect to get 'move to a new file' refactor");
3163+
}
3164+
}
3165+
}
3166+
}
3167+
31583168
public moveToNewFile(options: FourSlashInterface.MoveToNewFileOptions): void {
31593169
assert(this.getRanges().length === 1);
31603170
const range = this.getRanges()[0];
@@ -4509,6 +4519,9 @@ namespace FourSlashInterface {
45094519
public moveToNewFile(options: MoveToNewFileOptions): void {
45104520
this.state.moveToNewFile(options);
45114521
}
4522+
public noMoveToNewFile(): void {
4523+
this.state.noMoveToNewFile();
4524+
}
45124525
}
45134526

45144527
export class Edit {

src/services/refactors/moveToNewFile.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ namespace ts.refactor {
33
const refactorName = "Move to a new file";
44
registerRefactor(refactorName, {
55
getAvailableActions(context): ApplicableRefactorInfo[] {
6-
if (getStatementsToMove(context) === undefined || !context.preferences.allowTextChangesInNewFiles) return undefined;
6+
if (!context.preferences.allowTextChangesInNewFiles || getStatementsToMove(context) === undefined) return undefined;
77
const description = getLocaleSpecificMessage(Diagnostics.Move_to_a_new_file);
88
return [{ name: refactorName, description, actions: [{ name: refactorName, description }] }];
99
},
@@ -23,11 +23,12 @@ namespace ts.refactor {
2323
const startNodeIndex = findIndex(statements, s => s.end > range.pos);
2424
if (startNodeIndex === -1) return undefined;
2525
// Can't only partially include the start node or be partially into the next node
26-
const okStart = range.pos <= statements[startNodeIndex].getStart(file);
26+
if (range.pos > statements[startNodeIndex].getStart(file)) return undefined;
2727
const afterEndNodeIndex = findIndex(statements, s => s.end > range.end, startNodeIndex);
2828
// Can't be partially into the next node
29-
const okEnd = afterEndNodeIndex === -1 || afterEndNodeIndex !== 0 && statements[afterEndNodeIndex].getStart(file) >= range.end;
30-
return okStart && okEnd ? statements.slice(startNodeIndex, afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex) : undefined;
29+
if (afterEndNodeIndex !== -1 && (afterEndNodeIndex === 0 || statements[afterEndNodeIndex].getStart(file) < range.end)) return undefined;
30+
31+
return statements.slice(startNodeIndex, afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex);
3132
}
3233

3334
function doChange(oldFile: SourceFile, program: Program, toMove: ReadonlyArray<Statement>, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void {

tests/cases/fourslash/fourslash.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@ declare namespace FourSlashInterface {
362362
moveToNewFile(options: {
363363
readonly newFileContents: { readonly [fileName: string]: string };
364364
}): void;
365+
noMoveToNewFile(): void;
365366
}
366367
class edit {
367368
backspace(count?: number): void;

tests/cases/fourslash/moveToNewFile.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/// <reference path='fourslash.ts' />
22

33
// @Filename: /a.ts
4-
////import { a, b } from "./other";
4+
////import { a, b, alreadyUnused } from "./other";
55
////const p = 0;
66
////[|const y = p + b;|]
77
////a; y;
@@ -11,7 +11,7 @@ verify.moveToNewFile({
1111
"/a.ts":
1212
`import { y } from "./y";
1313
14-
import { a } from "./other";
14+
import { a, alreadyUnused } from "./other";
1515
export const p = 0;
1616
a; y;`,
1717

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @Filename: /a.ts
4+
////[|import { a, b } from "m";
5+
////a;|]
6+
////b;
7+
8+
//verify.noMoveToNewFile();
9+
verify.moveToNewFile({
10+
newFileContents: {
11+
"/a.ts":
12+
`b;`,
13+
"/newFile.ts":
14+
`import { a } from "m";
15+
import { a, b } from "m";
16+
a;`,
17+
}
18+
});

tests/cases/fourslash/moveToNewFile_rangeInvalid.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,4 @@
77
//// [|function inner() {}|]
88
////}
99

10-
for (const range of test.ranges()) {
11-
goTo.selectRange(range);
12-
verify.not.refactorAvailable("Move to a new file")
13-
}
10+
verify.noMoveToNewFile();

0 commit comments

Comments
 (0)