Skip to content

Commit

Permalink
fix(51301): Fixing an unused import at the end of a line removes the …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
a-tarasyuk committed Oct 28, 2022
1 parent 754eeb2 commit 64d0d5a
Show file tree
Hide file tree
Showing 23 changed files with 489 additions and 1 deletion.
24 changes: 23 additions & 1 deletion src/services/textChanges.ts
Expand Up @@ -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);
Expand Down Expand Up @@ -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]),
});
}
}
@@ -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)`
});
@@ -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)`
});
@@ -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)`
});
@@ -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)`
});
@@ -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)`
});
23 changes: 23 additions & 0 deletions 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";`
});
31 changes: 31 additions & 0 deletions 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";`
});
23 changes: 23 additions & 0 deletions 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";`
});
21 changes: 21 additions & 0 deletions 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";`
});
25 changes: 25 additions & 0 deletions 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";`
});
22 changes: 22 additions & 0 deletions 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,
)`
});
26 changes: 26 additions & 0 deletions 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,
)`
});
22 changes: 22 additions & 0 deletions 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,
)`
});
23 changes: 23 additions & 0 deletions 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,
)`
});

0 comments on commit 64d0d5a

Please sign in to comment.