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

Add 'move to new file' refactor #23726

Merged
15 commits merged into from
May 10, 2018
18 changes: 11 additions & 7 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ namespace ts {
}

/** Works like Array.prototype.findIndex, returning `-1` if no element satisfying the predicate is found. */
export function findIndex<T>(array: ReadonlyArray<T>, predicate: (element: T, index: number) => boolean): number {
for (let i = 0; i < array.length; i++) {
export function findIndex<T>(array: ReadonlyArray<T>, predicate: (element: T, index: number) => boolean, startIndex?: number): number {
for (let i = startIndex || 0; i < array.length; i++) {
if (predicate(array[i], i)) {
return i;
}
Expand Down Expand Up @@ -2289,20 +2289,24 @@ namespace ts {
return ["", ...relative, ...components];
}

export function getRelativePathFromFile(from: string, to: string, getCanonicalFileName: GetCanonicalFileName) {
return ensurePathIsNonModuleName(getRelativePathFromDirectory(getDirectoryPath(from), to, getCanonicalFileName));
}

/**
* Gets a relative path that can be used to traverse between `from` and `to`.
*/
export function getRelativePath(from: string, to: string, ignoreCase: boolean): string;
export function getRelativePathFromDirectory(from: string, to: string, ignoreCase: boolean): string;
/**
* Gets a relative path that can be used to traverse between `from` and `to`.
*/
// tslint:disable-next-line:unified-signatures
export function getRelativePath(from: string, to: string, getCanonicalFileName: GetCanonicalFileName): string;
export function getRelativePath(from: string, to: string, getCanonicalFileNameOrIgnoreCase: GetCanonicalFileName | boolean) {
Debug.assert((getRootLength(from) > 0) === (getRootLength(to) > 0), "Paths must either both be absolute or both be relative");
export function getRelativePathFromDirectory(fromDirectory: string, to: string, getCanonicalFileName: GetCanonicalFileName): string;
export function getRelativePathFromDirectory(fromDirectory: string, to: string, getCanonicalFileNameOrIgnoreCase: GetCanonicalFileName | boolean) {
Debug.assert((getRootLength(fromDirectory) > 0) === (getRootLength(to) > 0), "Paths must either both be absolute or both be relative");
const getCanonicalFileName = typeof getCanonicalFileNameOrIgnoreCase === "function" ? getCanonicalFileNameOrIgnoreCase : identity;
const ignoreCase = typeof getCanonicalFileNameOrIgnoreCase === "boolean" ? getCanonicalFileNameOrIgnoreCase : false;
const pathComponents = getPathComponentsRelativeTo(from, to, ignoreCase ? equateStringsCaseInsensitive : equateStringsCaseSensitive, getCanonicalFileName);
const pathComponents = getPathComponentsRelativeTo(fromDirectory, to, ignoreCase ? equateStringsCaseInsensitive : equateStringsCaseSensitive, getCanonicalFileName);
return getPathFromPathComponents(pathComponents);
}

Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4190,5 +4190,9 @@
"Convert all 'require' to 'import'": {
"category": "Message",
"code": 95048
},
"Move to a new file": {
"category": "Message",
"code": 95049
}
}
11 changes: 8 additions & 3 deletions src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,13 @@ namespace ts {
return block;
}

/* @internal */
export function createExpressionStatement(expression: Expression): ExpressionStatement {
const node = <ExpressionStatement>createSynthesizedNode(SyntaxKind.ExpressionStatement);
node.expression = expression;
return node;
}

export function updateBlock(node: Block, statements: ReadonlyArray<Statement>) {
return node.statements !== statements
? updateNode(createBlock(statements, node.multiLine), node)
Expand All @@ -1525,9 +1532,7 @@ namespace ts {
}

export function createStatement(expression: Expression) {
const node = <ExpressionStatement>createSynthesizedNode(SyntaxKind.ExpressionStatement);
node.expression = parenthesizeExpressionForExpressionStatement(expression);
return node;
return createExpressionStatement(parenthesizeExpressionForExpressionStatement(expression));
}

export function updateStatement(node: ExpressionStatement, expression: Expression) {
Expand Down
74 changes: 61 additions & 13 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3049,9 +3049,7 @@ Actual: ${stringify(fullActual)}`);
}

public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) {
const marker = this.getMarkerByName(markerName);
const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, marker.position, ts.defaultPreferences);
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
const isAvailable = this.getApplicableRefactors(this.getMarkerByName(markerName).position).length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableAtMarker failed - expected no refactor at marker ${markerName} but found some.`);
}
Expand All @@ -3068,9 +3066,7 @@ Actual: ${stringify(fullActual)}`);
}

public verifyRefactorAvailable(negative: boolean, name: string, actionName?: string) {
const selection = this.getSelection();

let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection, ts.defaultPreferences) || [];
let refactors = this.getApplicableRefactors(this.getSelection());
refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)));
const isAvailable = refactors.length > 0;

Expand All @@ -3090,10 +3086,7 @@ Actual: ${stringify(fullActual)}`);
}

public verifyRefactor({ name, actionName, refactors }: FourSlashInterface.VerifyRefactorOptions) {
const selection = this.getSelection();

const actualRefactors = (this.languageService.getApplicableRefactors(this.activeFile.fileName, selection, ts.defaultPreferences) || ts.emptyArray)
.filter(r => r.name === name && r.actions.some(a => a.name === actionName));
const actualRefactors = this.getApplicableRefactors(this.getSelection()).filter(r => r.name === name && r.actions.some(a => a.name === actionName));
this.assertObjectsEqual(actualRefactors, refactors);
}

Expand All @@ -3103,8 +3096,7 @@ Actual: ${stringify(fullActual)}`);
throw new Error("Exactly one refactor range is allowed per test.");
}

const applicableRefactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, ts.first(ranges), ts.defaultPreferences);
const isAvailable = applicableRefactors && applicableRefactors.length > 0;
const isAvailable = this.getApplicableRefactors(ts.first(ranges)).length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`);
}
Expand All @@ -3115,7 +3107,7 @@ Actual: ${stringify(fullActual)}`);

public applyRefactor({ refactorName, actionName, actionDescription, newContent: newContentWithRenameMarker }: FourSlashInterface.ApplyRefactorOptions) {
const range = this.getSelection();
const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range, ts.defaultPreferences);
const refactors = this.getApplicableRefactors(range);
const refactorsWithName = refactors.filter(r => r.name === refactorName);
if (refactorsWithName.length === 0) {
this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.\nAvailable refactors: ${refactors.map(r => r.name)}`);
Expand Down Expand Up @@ -3161,7 +3153,48 @@ Actual: ${stringify(fullActual)}`);
return { renamePosition, newContent };
}
}
}

public noMoveToNewFile() {
for (const range of this.getRanges()) {
for (const refactor of this.getApplicableRefactors(range, { allowTextChangesInNewFiles: true })) {
if (refactor.name === "Move to a new file") {
ts.Debug.fail("Did not expect to get 'move to a new file' refactor");
}
}
}
}

public moveToNewFile(options: FourSlashInterface.MoveToNewFileOptions): void {
assert(this.getRanges().length === 1);
const range = this.getRanges()[0];
const refactor = ts.find(this.getApplicableRefactors(range, { allowTextChangesInNewFiles: true }), r => r.name === "Move to a new file");
assert(refactor.actions.length === 1);
const action = ts.first(refactor.actions);
assert(action.name === "Move to a new file" && action.description === "Move to a new file");

const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactor.name, action.name, ts.defaultPreferences);
for (const edit of editInfo.edits) {
const newContent = options.newFileContents[edit.fileName];
if (newContent === undefined) {
this.raiseError(`There was an edit in ${edit.fileName} but new content was not specified.`);
}
if (this.testData.files.some(f => f.fileName === edit.fileName)) {
this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false);
this.openFile(edit.fileName);
this.verifyCurrentFileContent(newContent);
}
else {
assert(edit.textChanges.length === 1);
const change = ts.first(edit.textChanges);
assert.deepEqual(change.span, ts.createTextSpan(0, 0));
assert.equal(change.newText, newContent, `Content for ${edit.fileName}`);
}
}

for (const fileName in options.newFileContents) {
assert(editInfo.edits.some(e => e.fileName === fileName));
}
}

public verifyFileAfterApplyingRefactorAtMarker(
Expand Down Expand Up @@ -3377,6 +3410,10 @@ Actual: ${stringify(fullActual)}`);
this.verifyCurrentFileContent(options.newFileContents[fileName]);
}
}

private getApplicableRefactors(positionOrRange: number | ts.TextRange, preferences = ts.defaultPreferences): ReadonlyArray<ts.ApplicableRefactorInfo> {
return this.languageService.getApplicableRefactors(this.activeFile.fileName, positionOrRange, preferences) || ts.emptyArray;
}
}

export function runFourSlashTest(basePath: string, testType: FourSlashTestType, fileName: string) {
Expand Down Expand Up @@ -4478,6 +4515,13 @@ namespace FourSlashInterface {
public getEditsForFileRename(options: GetEditsForFileRenameOptions) {
this.state.getEditsForFileRename(options);
}

public moveToNewFile(options: MoveToNewFileOptions): void {
this.state.moveToNewFile(options);
}
public noMoveToNewFile(): void {
this.state.noMoveToNewFile();
}
}

export class Edit {
Expand Down Expand Up @@ -4851,4 +4895,8 @@ namespace FourSlashInterface {
readonly newPath: string;
readonly newFileContents: { readonly [fileName: string]: string };
}

export interface MoveToNewFileOptions {
readonly newFileContents: { readonly [fileName: string]: string };
}
}
1 change: 1 addition & 0 deletions src/harness/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
"../services/codefixes/useDefaultImport.ts",
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
"../services/sourcemaps.ts",
"../services/services.ts",
"../services/breakpoints.ts",
Expand Down
40 changes: 20 additions & 20 deletions src/harness/unittests/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,25 +268,25 @@ describe("core paths", () => {
assert.strictEqual(ts.resolvePath("a", "b", "../c"), "a/c");
});
it("getPathRelativeTo", () => {
assert.strictEqual(ts.getRelativePath("/", "/", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePath("/a", "/a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePath("/a/", "/a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePath("/a", "/", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePath("/a", "/b", /*ignoreCase*/ false), "../b");
assert.strictEqual(ts.getRelativePath("/a/b", "/b", /*ignoreCase*/ false), "../../b");
assert.strictEqual(ts.getRelativePath("/a/b/c", "/b", /*ignoreCase*/ false), "../../../b");
assert.strictEqual(ts.getRelativePath("/a/b/c", "/b/c", /*ignoreCase*/ false), "../../../b/c");
assert.strictEqual(ts.getRelativePath("/a/b/c", "/a/b", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePath("c:", "d:", /*ignoreCase*/ false), "d:/");
assert.strictEqual(ts.getRelativePath("file:///", "file:///", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePath("file:///a", "file:///a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePath("file:///a/", "file:///a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePath("file:///a", "file:///", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePath("file:///a", "file:///b", /*ignoreCase*/ false), "../b");
assert.strictEqual(ts.getRelativePath("file:///a/b", "file:///b", /*ignoreCase*/ false), "../../b");
assert.strictEqual(ts.getRelativePath("file:///a/b/c", "file:///b", /*ignoreCase*/ false), "../../../b");
assert.strictEqual(ts.getRelativePath("file:///a/b/c", "file:///b/c", /*ignoreCase*/ false), "../../../b/c");
assert.strictEqual(ts.getRelativePath("file:///a/b/c", "file:///a/b", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePath("file:///c:", "file:///d:", /*ignoreCase*/ false), "file:///d:/");
assert.strictEqual(ts.getRelativePathFromDirectory("/", "/", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePathFromDirectory("/a", "/a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePathFromDirectory("/a/", "/a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePathFromDirectory("/a", "/", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePathFromDirectory("/a", "/b", /*ignoreCase*/ false), "../b");
assert.strictEqual(ts.getRelativePathFromDirectory("/a/b", "/b", /*ignoreCase*/ false), "../../b");
assert.strictEqual(ts.getRelativePathFromDirectory("/a/b/c", "/b", /*ignoreCase*/ false), "../../../b");
assert.strictEqual(ts.getRelativePathFromDirectory("/a/b/c", "/b/c", /*ignoreCase*/ false), "../../../b/c");
assert.strictEqual(ts.getRelativePathFromDirectory("/a/b/c", "/a/b", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePathFromDirectory("c:", "d:", /*ignoreCase*/ false), "d:/");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///", "file:///", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a", "file:///a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a/", "file:///a", /*ignoreCase*/ false), "");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a", "file:///", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a", "file:///b", /*ignoreCase*/ false), "../b");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a/b", "file:///b", /*ignoreCase*/ false), "../../b");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a/b/c", "file:///b", /*ignoreCase*/ false), "../../../b");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a/b/c", "file:///b/c", /*ignoreCase*/ false), "../../../b/c");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///a/b/c", "file:///a/b", /*ignoreCase*/ false), "..");
assert.strictEqual(ts.getRelativePathFromDirectory("file:///c:", "file:///d:", /*ignoreCase*/ false), "file:///d:/");
});
});
2 changes: 1 addition & 1 deletion src/harness/vpath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace vpath {
export import dirname = ts.getDirectoryPath;
export import basename = ts.getBaseFileName;
export import extname = ts.getAnyExtensionFromPath;
export import relative = ts.getRelativePath;
export import relative = ts.getRelativePathFromDirectory;
export import beneath = ts.containsPath;
export import changeExtension = ts.changeAnyExtension;
export import isTypeScript = ts.hasTypeScriptFileExtension;
Expand Down
1 change: 1 addition & 0 deletions src/server/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
"../services/codefixes/useDefaultImport.ts",
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
"../services/sourcemaps.ts",
"../services/services.ts",
"../services/breakpoints.ts",
Expand Down
1 change: 1 addition & 0 deletions src/server/tsconfig.library.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
"../services/codefixes/useDefaultImport.ts",
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
"../services/sourcemaps.ts",
"../services/services.ts",
"../services/breakpoints.ts",
Expand Down
9 changes: 0 additions & 9 deletions src/services/codefixes/convertToEs6Module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,15 +507,6 @@ namespace ts.codefix {
: makeImport(/*name*/ undefined, [makeImportSpecifier(propertyName, localName)], moduleSpecifier);
}

function makeImport(name: Identifier | undefined, namedImports: ReadonlyArray<ImportSpecifier> | undefined, moduleSpecifier: StringLiteralLike): ImportDeclaration {
return makeImportDeclaration(name, namedImports, moduleSpecifier);
}

export function makeImportDeclaration(name: Identifier, namedImports: ReadonlyArray<ImportSpecifier> | undefined, moduleSpecifier: Expression) {
const importClause = (name || namedImports) && createImportClause(name, namedImports && createNamedImports(namedImports));
return createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, moduleSpecifier);
}

function makeImportSpecifier(propertyName: string | undefined, name: string): ImportSpecifier {
return createImportSpecifier(propertyName !== undefined && propertyName !== name ? createIdentifier(propertyName) : undefined, createIdentifier(name));
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/fixInvalidImportSyntax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace ts.codefix {
const variations: CodeFixAction[] = [];

// import Bluebird from "bluebird";
variations.push(createAction(context, sourceFile, node, makeImportDeclaration(namespace.name, /*namedImports*/ undefined, node.moduleSpecifier)));
variations.push(createAction(context, sourceFile, node, makeImport(namespace.name, /*namedImports*/ undefined, node.moduleSpecifier)));

if (getEmitModuleKind(opts) === ModuleKind.CommonJS) {
// import Bluebird = require("bluebird");
Expand Down