Merge pull request #19135 from amcasey/GH18626

Introduce and consume suppressLeadingAndTrailingTrivia

Author: amcasey

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<T extends Node>(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.
      */

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();|]`);

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) {

src/harness/unittests/tsserverProjectSystem.ts

@@ -4519,9 +4519,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 },

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);

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);
+        }
+    }
 }

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

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

tests/baselines/reference/extractFunction/extractFunction13.ts

@@ -20,7 +20,7 @@
         <U2a, U2b>(u2a: U2a, u2b: U2b) => {
             function F2<T2a, T2b>(t2a: T2a, t2b: T2b) {
                 <U3a, U3b>(u3a: U3a, u3b: U3b) => {
-                    /*RENAME*/newFunction<U3a>(u3a);
+                        /*RENAME*/newFunction<U3a>(u3a);
                 }
 
                 function newFunction<U3a>(u3a: U3a) {
@@ -40,7 +40,7 @@
         <U2a, U2b>(u2a: U2a, u2b: U2b) => {
             function F2<T2a, T2b>(t2a: T2a, t2b: T2b) {
                 <U3a, U3b>(u3a: U3a, u3b: U3b) => {
-                    /*RENAME*/newFunction<U2a, T2a, U3a>(t2a, u2a, u3a);
+                        /*RENAME*/newFunction<U2a, T2a, U3a>(t2a, u2a, u3a);
                 }
             }
         }
@@ -60,7 +60,7 @@
         <U2a, U2b>(u2a: U2a, u2b: U2b) => {
             function F2<T2a, T2b>(t2a: T2a, t2b: T2b) {
                 <U3a, U3b>(u3a: U3a, u3b: U3b) => {
-                    /*RENAME*/newFunction<U1a, T1a, U2a, T2a, U3a>(t1a, t2a, u1a, u2a, u3a);
+                        /*RENAME*/newFunction<U1a, T1a, U2a, T2a, U3a>(t1a, t2a, u1a, u2a, u3a);
                 }
             }
         }

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;
+}

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;
+}

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;
 }
 `