Skip to content

Commit

Permalink
Add 'move to new file' refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy Hanson committed Apr 26, 2018
1 parent bcb5599 commit 8f62d5c
Show file tree
Hide file tree
Showing 28 changed files with 722 additions and 45 deletions.
4 changes: 2 additions & 2 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 = 0): number {
for (let i = startIndex; i < array.length; i++) {
if (predicate(array[i], i)) {
return i;
}
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4174,5 +4174,9 @@
"Generate 'get' and 'set' accessors": {
"category": "Message",
"code": 95046
},
"Move to new file": {
"category": "Message",
"code": 95047
}
}
61 changes: 48 additions & 13 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2950,9 +2950,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 @@ -2969,9 +2967,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 @@ -2991,10 +2987,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 @@ -3004,8 +2997,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 @@ -3016,7 +3008,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 @@ -3062,7 +3054,38 @@ Actual: ${stringify(fullActual)}`);
return { renamePosition, newContent };
}
}
}

public moveToNewFile(options: FourSlashInterface.MoveToNewFileOptions): void {
assert(this.getRanges().length === 1);
const range = this.getRanges()[0];
const refactor = ts.find(this.getApplicableRefactors(range), r => r.name === "Move to new file");
assert(refactor.actions.length === 1);
const action = ts.first(refactor.actions);
assert(action.name === "Move to new file" && action.description === "Move to 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 @@ -3278,6 +3301,10 @@ Actual: ${stringify(fullActual)}`);
this.verifyCurrentFileContent(options.newFileContents[fileName]);
}
}

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

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

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

export class Edit {
Expand Down Expand Up @@ -4721,4 +4752,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 @@ -115,6 +115,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.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,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 @@ -117,6 +117,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
2 changes: 1 addition & 1 deletion src/services/codefixes/useDefaultImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@ namespace ts.codefix {
}

function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, info: Info): void {
changes.replaceNode(sourceFile, info.importNode, makeImportDeclaration(info.name, /*namedImports*/ undefined, info.moduleSpecifier));
changes.replaceNode(sourceFile, info.importNode, makeImport(info.name, /*namedImports*/ undefined, info.moduleSpecifier));
}
}
15 changes: 2 additions & 13 deletions src/services/importTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ namespace ts.FindAllReferences {
const { name } = importClause;
// If a default import has the same name as the default export, allow to rename it.
// Given `import f` and `export default function f`, we will rename both, but for `import g` we will rename just that.
if (name && (!isForRename || name.escapedText === symbolName(exportSymbol))) {
if (name && (!isForRename || name.escapedText === symbolEscapedNameNoDefault(exportSymbol))) {
const defaultImportAlias = checker.getSymbolAtLocation(name);
addSearch(name, defaultImportAlias);
}
Expand Down Expand Up @@ -534,7 +534,7 @@ namespace ts.FindAllReferences {
// If the import has a different name than the export, do not continue searching.
// If `importedName` is undefined, do continue searching as the export is anonymous.
// (All imports returned from this function will be ignored anyway if we are in rename and this is a not a named export.)
const importedName = symbolName(importedSymbol);
const importedName = symbolEscapedNameNoDefault(importedSymbol);
if (importedName === undefined || importedName === InternalSymbolName.Default || importedName === symbol.escapedName) {
return { kind: ImportExport.Import, symbol: importedSymbol, ...isImport };
}
Expand Down Expand Up @@ -606,17 +606,6 @@ namespace ts.FindAllReferences {
return isExternalModuleSymbol(exportingModuleSymbol) ? { exportingModuleSymbol, exportKind } : undefined;
}

function symbolName(symbol: Symbol): __String | undefined {
if (symbol.escapedName !== InternalSymbolName.Default) {
return symbol.escapedName;
}

return forEach(symbol.declarations, decl => {
const name = getNameOfDeclaration(decl);
return name && name.kind === SyntaxKind.Identifier && name.escapedText;
});
}

/** If at an export specifier, go to the symbol it refers to. */
function skipExportSpecifierSymbol(symbol: Symbol, checker: TypeChecker): Symbol {
// For `export { foo } from './bar", there's nothing to skip, because it does not create a new alias. But `export { foo } does.
Expand Down
4 changes: 2 additions & 2 deletions src/services/refactorProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace ts {
}
}

export function getRefactorContextLength(context: RefactorContext): number {
return context.endPosition === undefined ? 0 : context.endPosition - context.startPosition;
export function getRefactorContextSpan({ startPosition, endPosition }: RefactorContext): TextSpan {
return createTextSpanFromBounds(startPosition, endPosition === undefined ? startPosition : endPosition);
}
}
4 changes: 2 additions & 2 deletions src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace ts.refactor.extractSymbol {
* Exported for tests.
*/
export function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
const rangeToExtract = getRangeToExtract(context.file, { start: context.startPosition, length: getRefactorContextLength(context) });
const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context));

const targetRange: TargetRange = rangeToExtract.targetRange;
if (targetRange === undefined) {
Expand Down Expand Up @@ -87,7 +87,7 @@ namespace ts.refactor.extractSymbol {

/* Exported for tests */
export function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined {
const rangeToExtract = getRangeToExtract(context.file, { start: context.startPosition, length: getRefactorContextLength(context) });
const rangeToExtract = getRangeToExtract(context.file, getRefactorContextSpan(context));
const targetRange: TargetRange = rangeToExtract.targetRange;

const parsedFunctionIndexMatch = /^function_scope_(\d+)$/.exec(actionName);
Expand Down
Loading

0 comments on commit 8f62d5c

Please sign in to comment.