Improve insertion position of extracted methods

Old: End of target scope
New: Before the first non-constructor function following the extracted
range in the target scope
This commit is contained in:
Andrew Casey
2017-09-08 13:07:32 -07:00
parent deefb01c9d
commit 26903552fe
3 changed files with 56 additions and 7 deletions

View File

@@ -505,7 +505,7 @@ namespace ts {
}
}
function staticAssertNever(_: never): void {}
export function staticAssertNever(_: never): void {}
// Gets the nearest enclosing block scope container that has the provided node
// as a descendant, that is not the provided node.

View File

@@ -697,8 +697,14 @@ namespace ts.refactor.extractMethod {
}
const changeTracker = textChanges.ChangeTracker.fromContext(context);
// insert function at the end of the scope
changeTracker.insertNodeBefore(context.file, scope.getLastToken(), newFunction, { prefix: context.newLineCharacter, suffix: context.newLineCharacter });
const minInsertionPos = (isReadonlyArray(range.range) ? lastOrUndefined(range.range) : range.range).end;
const nodeToInsertBefore = getNodeToInsertBefore(minInsertionPos, scope);
if (nodeToInsertBefore) {
changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newFunction, { suffix: context.newLineCharacter + context.newLineCharacter });
}
else {
changeTracker.insertNodeBefore(context.file, scope.getLastToken(), newFunction, { prefix: context.newLineCharacter, suffix: context.newLineCharacter });
}
const newNodes: Node[] = [];
// replace range with function call
@@ -831,6 +837,39 @@ namespace ts.refactor.extractMethod {
return "__return";
}
function getStatementsOrClassElements(scope: Scope): ReadonlyArray<Statement> | ReadonlyArray<ClassElement> {
if (isFunctionLike(scope)) {
const body = scope.body;
if (isBlock(body)) {
return body.statements;
}
}
else if (isModuleBlock(scope) || isSourceFile(scope)) {
return scope.statements;
}
else if (isClassLike(scope)) {
return scope.members;
}
else {
staticAssertNever(scope);
}
return emptyArray;
}
/**
* If `scope` contains a function after `minPos`, then return the first such function.
* Otherwise, return `undefined`.
*/
function getNodeToInsertBefore(minPos: number, scope: Scope): Node | undefined {
const children = getStatementsOrClassElements(scope);
for (const child of children) {
if (child.pos >= minPos && isFunctionLike(child) && !isConstructorDeclaration(child)) {
return child;
}
}
}
function transformFunctionBody(body: Node) {
if (isBlock(body) && !writes && substitutions.size === 0) {
// already block, no writes to propagate back, no substitutions - can use node as is

View File

@@ -16,6 +16,16 @@ edit.applyRefactor({
actionDescription: "Extract function into class 'C'",
});
verify.currentFileContentIs(`class C {
static j = 1 + 1;
constructor(q: string = C.newFunction()) {
}
private static newFunction(): string {
return "a" + "b";
}
}`);
goTo.select('c', 'd');
edit.applyRefactor({
refactorName: "Extract Method",
@@ -28,11 +38,11 @@ verify.currentFileContentIs(`class C {
constructor(q: string = C.newFunction()) {
}
private static newFunction(): string {
return "a" + "b";
}
private static newFunction_1() {
return 1 + 1;
}
private static newFunction(): string {
return "a" + "b";
}
}`);