Retains trailing cmments in a logical manner in extract type (#40245)

src/services/refactors/extractType.ts

@@ -190,7 +190,7 @@ namespace ts.refactor {
             selection
         );
         changes.insertNodeBefore(file, firstStatement, ignoreSourceNewlines(newTypeNode), /* blankLineBetween */ true);
-        changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined))));
+        changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined))), { leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude, trailingTriviaOption: textChanges.TrailingTriviaOption.ExcludeWhitespace });
     }
 
     function doInterfaceChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: InterfaceInfo) {
@@ -204,8 +204,9 @@ namespace ts.refactor {
             /* heritageClauses */ undefined,
             typeElements
         );
+        setTextRange(newTypeNode, typeElements[0]?.parent);
         changes.insertNodeBefore(file, firstStatement, ignoreSourceNewlines(newTypeNode), /* blankLineBetween */ true);
-        changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined))));
+        changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /* typeArguments */ undefined))), { leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude, trailingTriviaOption: textChanges.TrailingTriviaOption.ExcludeWhitespace });
     }
 
     function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: Info) {

src/services/textChanges.ts

@@ -56,6 +56,8 @@ namespace ts.textChanges {
     export enum TrailingTriviaOption {
         /** Exclude all trailing trivia (use getEnd()) */
         Exclude,
+        /** Doesn't include whitespace, but does strip comments */
+        ExcludeWhitespace,
         /** Include trailing trivia */
         Include,
     }
@@ -209,10 +211,19 @@ namespace ts.textChanges {
     function getAdjustedEndPosition(sourceFile: SourceFile, node: Node, options: ConfigurableEnd) {
         const { end } = node;
         const { trailingTriviaOption } = options;
-        if (trailingTriviaOption === TrailingTriviaOption.Exclude || (isExpression(node) && trailingTriviaOption !== TrailingTriviaOption.Include)) {
+        if (trailingTriviaOption === TrailingTriviaOption.Exclude) {
+            return end;
+        }
+        if (trailingTriviaOption === TrailingTriviaOption.ExcludeWhitespace) {
+            const comments = concatenate(getTrailingCommentRanges(sourceFile.text, end), getLeadingCommentRanges(sourceFile.text, end));
+            const realEnd = comments?.[comments.length - 1]?.end;
+            if (realEnd) {
+                return realEnd;
+            }
             return end;
         }
         const newEnd = skipTrivia(sourceFile.text, end, /*stopAfterLineBreak*/ true);
+
         return newEnd !== end && (trailingTriviaOption === TrailingTriviaOption.Include || isLineBreak(sourceFile.text.charCodeAt(newEnd - 1)))
             ? newEnd
             : end;

tests/cases/fourslash/typeExtractionDoesNotDuplicateTrailingComment.ts

@@ -0,0 +1,67 @@
+/// <reference path='fourslash.ts' />
+/// Doesn't duplicate comments - #31629
+
+//// type a = /*a*/{ x: string } /* foo */ | string /* bar *//*b*/;
+//// type b = /*c*//* leading */{ x: string } /* trailing *//*d*/;
+//// type c = /*e*/{ x: string } /* inner */ | string// trailing/*f*/
+////
+
+goTo.select("a", "b");
+edit.applyRefactor({
+  refactorName: "Extract type",
+  actionName: "Extract to type alias",
+  actionDescription: "Extract to type alias",
+  newContent: `type /*RENAME*/NewType = {
+    x: string;
+} /* foo */ | string /* bar */;
+
+type a = NewType;
+type b = /* leading */{ x: string } /* trailing */;
+type c = { x: string } /* inner */ | string// trailing
+`,
+});
+
+// Extract to interface
+goTo.select("c", "d");
+edit.applyRefactor({
+  refactorName: "Extract type",
+  actionName: "Extract to interface",
+  actionDescription: "Extract to interface",
+  newContent: `type NewType = {
+    x: string;
+} /* foo */ | string /* bar */;
+
+type a = NewType;
+interface /*RENAME*/NewType_1 {
+    x: string;
+} /* trailing */
+
+type b = /* leading */NewType_1;
+type c = { x: string } /* inner */ | string// trailing
+`,
+});
+
+// Trailing comment using '//'
+goTo.select("e", "f");
+edit.applyRefactor({
+  refactorName: "Extract type",
+  actionName: "Extract to type alias",
+  actionDescription: "Extract to type alias",
+  newContent: `type NewType = {
+    x: string;
+} /* foo */ | string /* bar */;
+
+type a = NewType;
+interface NewType_1 {
+    x: string;
+} /* trailing */
+
+type b = /* leading */NewType_1;
+type /*RENAME*/NewType_2 = {
+    x: string;
+} /* inner */ | string // trailing
+    ;
+
+type c = NewType_2
+`,
+});