fix: #219 - Removing the last statement in a default or case clause will cause a double newline.

Author: dsherret

src/compiler/base/ModifierableNode.ts

@@ -1,7 +1,7 @@
 import {ts, SyntaxKind} from "../../typescript";
 import {Constructor} from "../../Constructor";
 import * as errors from "../../errors";
-import {insertIntoCreatableSyntaxList, removeChildrenWithFormattingFromCollapsibleSyntaxList, FormattingKind} from "../../manipulation";
+import {insertIntoCreatableSyntaxList, removeChildren, FormattingKind} from "../../manipulation";
 import {ArrayUtils, getSyntaxKindName} from "../../utils";
 import {Node} from "../common";
 import {KindToNodeMappings} from "../kindToNodeMappings";
@@ -160,14 +160,16 @@ export function ModifierableNode<T extends Constructor<ModiferableNodeExtensionT
         }
 
         removeModifier(text: ModifierTexts) {
-            const modifier = ArrayUtils.find(this.getModifiers(), m => m.getText() === text);
+            const modifiers = this.getModifiers();
+            const modifier = ArrayUtils.find(modifiers, m => m.getText() === text);
             if (modifier == null)
                 return false;
 
-            removeChildrenWithFormattingFromCollapsibleSyntaxList({
-                children: [modifier],
-                getSiblingFormatting: () => FormattingKind.Space
+            removeChildren({
+                children: [modifiers.length === 1 ? modifier.getParentSyntaxListOrThrow() : modifier],
+                removeFollowingSpaces: true
             });
+
             return true;
         }
 

src/manipulation/textChecks/index.ts

@@ -1 +1,2 @@
 export * from "./isBlankLineAtPos";
+export * from "./isNewLineAtPos";

src/manipulation/textChecks/isNewLineAtPos.ts

@@ -0,0 +1,3 @@
+export function isNewLineAtPos(fullText: string, pos: number) {
+    return fullText[pos] === "\n" || (fullText[pos] === "\r" && fullText[pos + 1] === "\n");
+}

src/manipulation/textManipulators/RemoveChildrenWithFormattingTextManipulator.ts

@@ -1,7 +1,8 @@
 import {TextManipulator} from "./TextManipulator";
 import {Node} from "../../compiler";
 import {FormattingKind, getFormattingKindText} from "../formatting";
-import {getPosAtNextNonBlankLine, getNextMatchingPos, getPosAfterPreviousNonBlankLine} from "../textSeek";
+import {getPosAtNextNonBlankLine, getNextMatchingPos, getPosAfterPreviousNonBlankLine, getPosAtEndOfPreviousLine} from "../textSeek";
+import {isNewLineAtPos} from "../textChecks";
 import {getSpacingBetweenNodes} from "./getSpacingBetweenNodes";
 import {getTextForError} from "./getTextForError";
 
@@ -27,6 +28,9 @@ export class RemoveChildrenWithFormattingTextManipulator<TNode extends Node> imp
         const removalPos = getRemovalPos();
         this.removalPos = removalPos;
 
+        // console.log(JSON.stringify(fullText.substring(0, removalPos)));
+        // console.log(JSON.stringify(fullText.substring(getRemovalEnd())));
+
         return getPrefix() + getSpacing() + getSuffix();
 
         function getPrefix() {
@@ -48,13 +52,15 @@ export class RemoveChildrenWithFormattingTextManipulator<TNode extends Node> imp
         }
 
         function getRemovalPos() {
-            if (previousSibling != null && nextSibling != null)
-                return previousSibling.getEnd();
+            if (previousSibling != null) {
+                const trailingEnd = previousSibling.getTrailingTriviaEnd();
+                return isNewLineAtPos(fullText, trailingEnd) ? trailingEnd : previousSibling.getEnd();
+            }
 
             if (parent.getPos() === children[0].getPos())
                 return children[0].getNonWhitespaceStart(); // do not shift the parent
 
-            return children[0].isFirstNodeOnLine() ? getPosAfterPreviousNonBlankLine(fullText, children[0].getNonWhitespaceStart()) : children[0].getNonWhitespaceStart();
+            return children[0].isFirstNodeOnLine() ? children[0].getPos() : children[0].getNonWhitespaceStart();
         }
 
         function getRemovalEnd() {
@@ -66,16 +72,23 @@ export class RemoveChildrenWithFormattingTextManipulator<TNode extends Node> imp
                         return nextSiblingStartLinePos;
                 }
 
-                return nextSibling.getStart();
+                return nextSibling.getStart(true);
             }
 
             if (parent.getEnd() === children[children.length - 1].getEnd())
                 return children[children.length - 1].getEnd(); // do not shift the parent
 
-            const nextNonSpaceOrTabChar = getNextMatchingPos(fullText, children[children.length - 1].getEnd(), char => char !== " " && char !== "\t");
-            if (fullText[nextNonSpaceOrTabChar] === "\r" || fullText[nextNonSpaceOrTabChar] === "\n")
-                return getPosAtNextNonBlankLine(fullText, nextNonSpaceOrTabChar);
-            return nextNonSpaceOrTabChar;
+            const triviaEnd = children[children.length - 1].getTrailingTriviaEnd();
+            if (isNewLineAtPos(fullText, triviaEnd)) {
+                if (previousSibling == null && children[0].getPos() === 0)
+                    return getPosAtNextNonBlankLine(fullText, triviaEnd);
+                return getPosAtEndOfPreviousLine(fullText, getPosAtNextNonBlankLine(fullText, triviaEnd));
+            }
+
+            if (previousSibling == null)
+                return triviaEnd;
+            else
+                return children[children.length - 1].getEnd();
         }
     }
 

src/manipulation/textSeek/getPosAfterNewLine.ts

@@ -0,0 +1,8 @@
+export function getPosAfterNewLine(text: string, pos: number) {
+    while (pos < text.length) {
+        if (text[pos] === "\n")
+            return pos + 1;
+        pos++;
+    }
+    return pos;
+}

src/manipulation/textSeek/getPosAtEndOfPreviousLine.ts

@@ -0,0 +1,12 @@
+export function getPosAtEndOfPreviousLine(fullText: string, pos: number) {
+    while (pos > 0) {
+        pos--;
+        if (fullText[pos] === "\n") {
+            if (fullText[pos - 1] === "\r")
+                return pos - 1;
+            return pos;
+        }
+    }
+
+    return pos;
+}

src/manipulation/textSeek/index.ts

@@ -1,5 +1,7 @@
 export * from "./getNextMatchingPos";
 export * from "./getNextNonWhitespacePos";
+export * from "./getPosAfterNewLine";
 export * from "./getPosAfterPreviousNonBlankLine";
 export * from "./getPosAtNextNonBlankLine";
+export * from "./getPosAtEndOfPreviousLine";
 export * from "./getPreviousMatchingPos";

src/tests/compiler/file/sourceFileTests.ts

@@ -790,13 +790,13 @@ describe(nameof(SourceFile), () => {
             expect(sourceFile.getFullText()).to.equal("");
         });
 
-        it("should return the default export symbol when one exists", () => {
+        it("should remove the default export symbol when one exists", () => {
             const {sourceFile} = getInfoFromText("export default class Identifier {}");
             sourceFile.removeDefaultExport();
             expect(sourceFile.getFullText()).to.equal("class Identifier {}");
         });
 
-        it("should return the default export symbol when default exported on a separate statement", () => {
+        it("should remove the default export symbol when default exported on a separate statement", () => {
             const {sourceFile} = getInfoFromText("namespace Identifier {}\nclass Identifier {}\nexport default Identifier;\n");
             sourceFile.removeDefaultExport();
             expect(sourceFile.getFullText()).to.equal("namespace Identifier {}\nclass Identifier {}\n");

src/tests/compiler/statement/statementedNodeTests.ts

@@ -270,7 +270,7 @@ describe(nameof(StatementedNode), () => {
         });
 
         it("should remove statements at the end case clause", () => {
-            doFirstChildTest<CaseClause>(caseClause, [1, 2], "switch (x) {\n    case 1:\n        x = 0;\n\n}", SyntaxKind.CaseClause);
+            doFirstChildTest<CaseClause>(caseClause, [1, 2], "switch (x) {\n    case 1:\n        x = 0;\n}", SyntaxKind.CaseClause);
         });
 
         const defaultClause = "switch (x) {\n    default:\n        x = 0;\n        y = 1;\n        break;\n}";
@@ -283,7 +283,7 @@ describe(nameof(StatementedNode), () => {
         });
 
         it("should remove statements at the end default clause", () => {
-            doFirstChildTest<DefaultClause>(defaultClause, [1, 2], "switch (x) {\n    default:\n        x = 0;\n\n}", SyntaxKind.DefaultClause);
+            doFirstChildTest<DefaultClause>(defaultClause, [1, 2], "switch (x) {\n    default:\n        x = 0;\n}", SyntaxKind.DefaultClause);
         });
     });