diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index fe183c5e8060c..65c1c92f36652 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -2612,6 +2612,16 @@ namespace ts { return node; } + /** + * Sets flags that control emit behavior of a node. + */ + /* @internal */ + export function addEmitFlags(node: T, emitFlags: EmitFlags) { + const emitNode = getOrCreateEmitNode(node); + emitNode.flags = emitNode.flags | emitFlags; + return node; + } + /** * Gets a custom text range to use when emitting source maps. */ diff --git a/src/harness/unittests/extractConstants.ts b/src/harness/unittests/extractConstants.ts index 09f8db34f31fe..61ffbd01a639b 100644 --- a/src/harness/unittests/extractConstants.ts +++ b/src/harness/unittests/extractConstants.ts @@ -223,6 +223,14 @@ const f = () => { testExtractConstant("extractConstant_ArrowFunction_Expression", `const f = () => [#|2 + 1|];`); + testExtractConstant("extractConstant_PreserveTrivia", ` +// a +var q = /*b*/ //c + /*d*/ [#|1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2|] /*k*/ //l + /*m*/; /*n*/ //o`); + testExtractConstantFailed("extractConstant_Void", ` function f(): void { } [#|f();|]`); diff --git a/src/harness/unittests/extractFunctions.ts b/src/harness/unittests/extractFunctions.ts index c69026c0be8af..789882ffd5091 100644 --- a/src/harness/unittests/extractFunctions.ts +++ b/src/harness/unittests/extractFunctions.ts @@ -532,6 +532,14 @@ function f() { [#|let x;|] return { x }; }`); + + testExtractFunction("extractFunction_PreserveTrivia", ` +// a +var q = /*b*/ //c + /*d*/ [#|1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2|] /*k*/ //l + /*m*/; /*n*/ //o`); }); function testExtractFunction(caption: string, text: string) { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 716c36d1e5d2a..7dce256e38831 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -4412,9 +4412,9 @@ namespace ts.projectSystem { fileName: "/a.ts", textChanges: [ { - start: { line: 2, offset: 1 }, - end: { line: 3, offset: 1 }, - newText: " newFunction();\n", + start: { line: 2, offset: 3 }, + end: { line: 2, offset: 5 }, + newText: "newFunction();", }, { start: { line: 3, offset: 2 }, diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 9dd148e5420fa..715b42b6bc89a 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -740,6 +740,8 @@ namespace ts.refactor.extractSymbol { } const { body, returnValueProperty } = transformFunctionBody(node, exposedVariableDeclarations, writes, substitutions, !!(range.facts & RangeFacts.HasReturn)); + suppressLeadingAndTrailingTrivia(body); + let newFunction: MethodDeclaration | FunctionDeclaration; if (isClassLike(scope)) { @@ -926,15 +928,10 @@ namespace ts.refactor.extractSymbol { } } - if (isReadonlyArray(range.range)) { - changeTracker.replaceNodesWithNodes(context.file, range.range, newNodes, { - nodeSeparator: context.newLineCharacter, - suffix: context.newLineCharacter // insert newline only when replacing statements - }); - } - else { - changeTracker.replaceNodeWithNodes(context.file, range.range, newNodes, { nodeSeparator: context.newLineCharacter }); - } + const replacementRange = isReadonlyArray(range.range) + ? { pos: first(range.range).getStart(), end: last(range.range).end } + : { pos: range.range.getStart(), end: range.range.end }; + changeTracker.replaceRangeWithNodes(context.file, replacementRange, newNodes, { nodeSeparator: context.newLineCharacter }); const edits = changeTracker.getChanges(); const renameRange = isReadonlyArray(range.range) ? first(range.range) : range.range; @@ -982,6 +979,7 @@ namespace ts.refactor.extractSymbol { : checker.typeToTypeNode(checker.getContextualType(node), scope, NodeBuilderFlags.NoTruncation); const initializer = transformConstantInitializer(node, substitutions); + suppressLeadingAndTrailingTrivia(initializer); const changeTracker = textChanges.ChangeTracker.fromContext(context); @@ -1014,7 +1012,7 @@ namespace ts.refactor.extractSymbol { changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newVariable, { suffix: context.newLineCharacter + context.newLineCharacter }); // Consume - changeTracker.replaceNodeWithNodes(context.file, node, [localReference], { nodeSeparator: context.newLineCharacter }); + changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference); } else { const newVariableDeclaration = createVariableDeclaration(localNameText, variableType, initializer); diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 0eb3f88cc9a77..df3f3c5c6b489 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1369,4 +1369,39 @@ namespace ts { return visited; } + + /** + * Sets EmitFlags to suppress leading and trailing trivia on the node. + */ + /* @internal */ + export function suppressLeadingAndTrailingTrivia(node: Node) { + Debug.assert(node !== undefined); + + suppressLeading(node); + suppressTrailing(node); + + function suppressLeading(node: Node) { + addEmitFlags(node, EmitFlags.NoLeadingComments); + + const firstChild = forEachChild(node, child => child); + firstChild && suppressLeading(firstChild); + } + + function suppressTrailing(node: Node) { + addEmitFlags(node, EmitFlags.NoTrailingComments); + + let lastChild: Node = undefined; + forEachChild( + node, + child => (lastChild = child, undefined), + children => { + // As an optimization, jump straight to the end of the list. + if (children.length) { + lastChild = last(children); + } + return undefined; + }); + lastChild && suppressTrailing(lastChild); + } + } } diff --git a/tests/baselines/reference/extractConstant/extractConstant_PreserveTrivia.js b/tests/baselines/reference/extractConstant/extractConstant_PreserveTrivia.js new file mode 100644 index 0000000000000..22abb77901d68 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_PreserveTrivia.js @@ -0,0 +1,17 @@ +// ==ORIGINAL== + +// a +var q = /*b*/ //c + /*d*/ /*[#|*/1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2/*|]*/ /*k*/ //l + /*m*/; /*n*/ //o +// ==SCOPE::Extract to constant in enclosing scope== +const newLocal = 1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2; + +// a +var q = /*b*/ //c + /*d*/ /*RENAME*/newLocal /*k*/ //l + /*m*/; /*n*/ //o \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_PreserveTrivia.ts b/tests/baselines/reference/extractConstant/extractConstant_PreserveTrivia.ts new file mode 100644 index 0000000000000..22abb77901d68 --- /dev/null +++ b/tests/baselines/reference/extractConstant/extractConstant_PreserveTrivia.ts @@ -0,0 +1,17 @@ +// ==ORIGINAL== + +// a +var q = /*b*/ //c + /*d*/ /*[#|*/1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2/*|]*/ /*k*/ //l + /*m*/; /*n*/ //o +// ==SCOPE::Extract to constant in enclosing scope== +const newLocal = 1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2; + +// a +var q = /*b*/ //c + /*d*/ /*RENAME*/newLocal /*k*/ //l + /*m*/; /*n*/ //o \ No newline at end of file diff --git a/tests/baselines/reference/extractFunction/extractFunction13.ts b/tests/baselines/reference/extractFunction/extractFunction13.ts index 4987039d5dac3..662701ebf415a 100644 --- a/tests/baselines/reference/extractFunction/extractFunction13.ts +++ b/tests/baselines/reference/extractFunction/extractFunction13.ts @@ -20,7 +20,7 @@ (u2a: U2a, u2b: U2b) => { function F2(t2a: T2a, t2b: T2b) { (u3a: U3a, u3b: U3b) => { - /*RENAME*/newFunction(u3a); + /*RENAME*/newFunction(u3a); } function newFunction(u3a: U3a) { @@ -40,7 +40,7 @@ (u2a: U2a, u2b: U2b) => { function F2(t2a: T2a, t2b: T2b) { (u3a: U3a, u3b: U3b) => { - /*RENAME*/newFunction(t2a, u2a, u3a); + /*RENAME*/newFunction(t2a, u2a, u3a); } } } @@ -60,7 +60,7 @@ (u2a: U2a, u2b: U2b) => { function F2(t2a: T2a, t2b: T2b) { (u3a: U3a, u3b: U3b) => { - /*RENAME*/newFunction(t1a, t2a, u1a, u2a, u3a); + /*RENAME*/newFunction(t1a, t2a, u1a, u2a, u3a); } } } diff --git a/tests/baselines/reference/extractFunction/extractFunction_PreserveTrivia.js b/tests/baselines/reference/extractFunction/extractFunction_PreserveTrivia.js new file mode 100644 index 0000000000000..b5e4bc76c6e2f --- /dev/null +++ b/tests/baselines/reference/extractFunction/extractFunction_PreserveTrivia.js @@ -0,0 +1,19 @@ +// ==ORIGINAL== + +// a +var q = /*b*/ //c + /*d*/ /*[#|*/1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2/*|]*/ /*k*/ //l + /*m*/; /*n*/ //o +// ==SCOPE::Extract to function in global scope== + +// a +var q = /*b*/ //c + /*d*/ /*RENAME*/newFunction() /*k*/ //l + /*m*/; /*n*/ //o +function newFunction() { + return 1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2; +} diff --git a/tests/baselines/reference/extractFunction/extractFunction_PreserveTrivia.ts b/tests/baselines/reference/extractFunction/extractFunction_PreserveTrivia.ts new file mode 100644 index 0000000000000..b5e4bc76c6e2f --- /dev/null +++ b/tests/baselines/reference/extractFunction/extractFunction_PreserveTrivia.ts @@ -0,0 +1,19 @@ +// ==ORIGINAL== + +// a +var q = /*b*/ //c + /*d*/ /*[#|*/1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2/*|]*/ /*k*/ //l + /*m*/; /*n*/ //o +// ==SCOPE::Extract to function in global scope== + +// a +var q = /*b*/ //c + /*d*/ /*RENAME*/newFunction() /*k*/ //l + /*m*/; /*n*/ //o +function newFunction() { + return 1 /*e*/ //f + /*g*/ + /*h*/ //i + /*j*/ 2; +} diff --git a/tests/cases/fourslash/extract-method-uniqueName.ts b/tests/cases/fourslash/extract-method-uniqueName.ts index 4271d9e84d2f2..da4c68cfb7e1d 100644 --- a/tests/cases/fourslash/extract-method-uniqueName.ts +++ b/tests/cases/fourslash/extract-method-uniqueName.ts @@ -11,10 +11,9 @@ edit.applyRefactor({ actionName: "function_scope_0", actionDescription: "Extract to function in global scope", newContent: -`/*RENAME*/newFunction_1(); - +`// newFunction +/*RENAME*/newFunction_1(); function newFunction_1() { - // newFunction 1 + 1; } `