fix(51301): Fixing an unused import at the end of a line removes the newline (#51320)

* fix(51301): keep the line break after removing the unused identifier

* preserve line breaks in import specifiers

* preserve line breaks in parameters and destructuring elements

* remove preserveLineBreak option

src/services/textChanges.ts

@@ -993,6 +993,27 @@ namespace ts.textChanges {
         return skipTrivia(sourceFile.text, getAdjustedStartPosition(sourceFile, node, { leadingTriviaOption: LeadingTriviaOption.IncludeAll }), /*stopAfterLineBreak*/ false, /*stopAtComments*/ true);
     }
 
+    function endPositionToDeleteNodeInList(sourceFile: SourceFile, node: Node, prevNode: Node | undefined, nextNode: Node): number {
+        const end = startPositionToDeleteNodeInList(sourceFile, nextNode);
+        if (prevNode === undefined || positionsAreOnSameLine(getAdjustedEndPosition(sourceFile, node, {}), end, sourceFile)) {
+            return end;
+        }
+        const token = findPrecedingToken(nextNode.getStart(sourceFile), sourceFile);
+        if (isSeparator(node, token)) {
+            const prevToken = findPrecedingToken(node.getStart(sourceFile), sourceFile);
+            if (isSeparator(prevNode, prevToken)) {
+                const pos = skipTrivia(sourceFile.text, token.getEnd(), /*stopAfterLineBreak*/ true, /*stopAtComments*/ true);
+                if (positionsAreOnSameLine(prevToken.getStart(sourceFile), token.getStart(sourceFile), sourceFile)) {
+                    return isLineBreak(sourceFile.text.charCodeAt(pos - 1)) ? pos - 1 : pos;
+                }
+                if (isLineBreak(sourceFile.text.charCodeAt(pos))) {
+                    return pos;
+                }
+            }
+        }
+        return end;
+    }
+
     function getClassOrObjectBraceEnds(cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression, sourceFile: SourceFile): [number | undefined, number | undefined] {
         const open = findChildOfKind(cls, SyntaxKind.OpenBraceToken, sourceFile);
         const close = findChildOfKind(cls, SyntaxKind.CloseBraceToken, sourceFile);
@@ -1589,9 +1610,10 @@ namespace ts.textChanges {
         // That's handled in the end by `finishTrailingCommaAfterDeletingNodesInList`.
         Debug.assert(!deletedNodesInLists.has(node), "Deleting a node twice");
         deletedNodesInLists.add(node);
+
         changes.deleteRange(sourceFile, {
             pos: startPositionToDeleteNodeInList(sourceFile, node),
-            end: index === containingList.length - 1 ? getAdjustedEndPosition(sourceFile, node, {}) : startPositionToDeleteNodeInList(sourceFile, containingList[index + 1]),
+            end: index === containingList.length - 1 ? getAdjustedEndPosition(sourceFile, node, {}) : endPositionToDeleteNodeInList(sourceFile, node, containingList[index - 1], containingList[index + 1]),
         });
     }
 }

tests/cases/fourslash/codeFixUnusedIdentifier_destructuring_elements2.ts

@@ -0,0 +1,22 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f({
+////    a, b,
+////    c,
+////}: any)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f({
+    a, 
+    c,
+}: any)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_destructuring_elements3.ts

@@ -0,0 +1,26 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f({
+////    a, b,
+////
+////
+////    c,
+////}: any)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f({
+    a, 
+
+
+    c,
+}: any)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_destructuring_elements4.ts

@@ -0,0 +1,22 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f({
+////    a, b, /* comment related to c */
+////    c,
+////}: any)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f({
+    a, /* comment related to c */
+    c,
+}: any)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_destructuring_elements5.ts

@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f({
+////    a,
+////    b,
+////    c,
+////}: any)|] {
+////    b;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'a'",
+    newRangeContent:
+`function f({
+    b,
+    c,
+}: any)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_destructuring_elements6.ts

@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f({
+////    a,
+////    b,
+////
+////    c,
+////}: any)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f({
+    a,
+    
+    c,
+}: any)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_importSpecifier1.ts

@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+////[|import {
+////    a, b, c, d,
+////    e, f,
+////} from "fs";|]
+////
+////a;
+////b;
+////c;
+////e;
+////f;
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'd'",
+    newRangeContent:
+`import {
+    a, b, c, 
+    e, f,
+} from "fs";`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_importSpecifier2.ts

@@ -0,0 +1,31 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+////[|import {
+////    a, b, c, d,
+////
+////
+////
+////
+////    e, f,
+////} from "fs";|]
+////
+////a;
+////b;
+////c;
+////e;
+////f;
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'd'",
+    newRangeContent:
+`import {
+    a, b, c, 
+
+
+
+
+    e, f,
+} from "fs";`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_importSpecifier3.ts

@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+////[|import {
+////    a, b, c, d, /** comment related to e */
+////    e, f,
+////} from "fs";|]
+////
+////a;
+////b;
+////c;
+////e;
+////f;
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'd'",
+    newRangeContent:
+`import {
+    a, b, c, /** comment related to e */
+    e, f,
+} from "fs";`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_importSpecifier4.ts

@@ -0,0 +1,21 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+////[|import {
+////    a,
+////    b,
+////    c,
+////} from "fs";|]
+////
+////a;
+////c;
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`import {
+    a,
+    c,
+} from "fs";`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_importSpecifier5.ts

@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+////[|import {
+////    a,
+////    b,
+////
+////
+////    c,
+////} from "fs";|]
+////
+////a;
+////c;
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`import {
+    a,
+    
+
+    c,
+} from "fs";`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_parameter2.ts

@@ -0,0 +1,22 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f(
+////    a: number, b: number,
+////    c: number,
+////)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f(
+    a: number, 
+    c: number,
+)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_parameter3.ts

@@ -0,0 +1,26 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f(
+////    a: number, b: number,
+////
+////    // comment
+////    c: number,
+////)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f(
+    a: number, 
+
+    // comment
+    c: number,
+)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_parameter4.ts

@@ -0,0 +1,22 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f(
+////    a: number, b: number, /* comment related to c */
+////    c: number,
+////)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f(
+    a: number, /* comment related to c */
+    c: number,
+)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_parameter5.ts

@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f(
+////    a: number,
+////    b: number,
+////    c: number,
+////)|] {
+////    b;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'a'",
+    newRangeContent:
+`function f(
+    b: number,
+    c: number,
+)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_parameter6.ts

@@ -0,0 +1,27 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////[|function f(
+////    a: number,
+////    b: number,
+////
+////
+////    c: number,
+////)|] {
+////    a;
+////    c;
+////}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'b'",
+    newRangeContent:
+`function f(
+    a: number,
+    
+
+    c: number,
+)`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_typeParameter1.ts

@@ -0,0 +1,19 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////function f<
+////    T1, T2,
+////    T3,
+////>(a: T1, b: T3) {}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'T2'",
+    newFileContent:
+`function f<
+    T1, 
+    T3,
+>(a: T1, b: T3) {}`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_typeParameter2.ts

@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////function f<
+////    T1, T2,
+////
+////
+////    T3,
+////>(a: T1, b: T3) {}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'T2'",
+    newFileContent:
+`function f<
+    T1, 
+
+
+    T3,
+>(a: T1, b: T3) {}`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_typeParameter3.ts

@@ -0,0 +1,19 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////function f<
+////    T1, T2, /** comment related to T3 */
+////    T3,
+////>(a: T1, b: T3) {}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'T2'",
+    newFileContent:
+`function f<
+    T1, /** comment related to T3 */
+    T3,
+>(a: T1, b: T3) {}`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_typeParameter4.ts

@@ -0,0 +1,20 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////function f<
+////    T1,
+////    T2,
+////    T3,
+////>(a: T1, b: T3) {}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'T2'",
+    newFileContent:
+`function f<
+    T1,
+    T3,
+>(a: T1, b: T3) {}`
+});

tests/cases/fourslash/codeFixUnusedIdentifier_typeParameter5.ts

@@ -0,0 +1,24 @@
+/// <reference path="fourslash.ts" />
+
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+////function f<
+////    T1,
+////    T2,
+////
+////
+////    T3,
+////>(a: T1, b: T3) {}
+
+verify.codeFix({
+    index: 0,
+    description: "Remove unused declaration for: 'T2'",
+    newFileContent:
+`function f<
+    T1,
+    
+
+    T3,
+>(a: T1, b: T3) {}`
+});