fix: #708 - getNonWhitespaceStart() should not include the previous…

… node's trailing comment.

This is technically a breaking change, but did a search online and only one other person is using this. This is better behaviour and fixes part of #708.

lib/ts-morph.d.ts

@@ -4445,7 +4445,7 @@ export declare class Node<NodeType extends ts.Node = ts.Node> {
      */
     getFullStart(): number;
     /**
-     * Gets the first source file text position that is not whitespace taking into account comment nodes.
+     * Gets the first source file text position that is not whitespace taking into account comment nodes and a previous node's trailing trivia.
      */
     getNonWhitespaceStart(): number;
     /**

src/compiler/ast/common/Node.ts

@@ -2,15 +2,15 @@ import { CodeBlockWriter } from "../../../codeBlockWriter";
 import * as errors from "../../../errors";
 import { ProjectContext } from "../../../ProjectContext";
 import { getNextMatchingPos, getNextNonWhitespacePos, getPreviousNonWhitespacePos, getPreviousMatchingPos, getTextFromFormattingEdits,
-    insertIntoParentTextRange, replaceSourceFileTextForFormatting, replaceSourceFileTextStraight } from "../../../manipulation";
+    insertIntoParentTextRange, replaceSourceFileTextForFormatting, replaceSourceFileTextStraight, hasNewLineInRange } from "../../../manipulation";
 import { WriterFunction } from "../../../types";
 import { ts, SyntaxKind, SymbolFlags } from "../../../typescript";
 import { ArrayUtils, getParentSyntaxList, getSyntaxKindName, getTextFromStringOrWriter, isStringKind, printNode, PrintNodeOptions, StringUtils, TypeGuards,
     StoredComparer } from "../../../utils";
 import { FormatCodeSettings } from "../../tools";
 import { Symbol } from "../../symbols";
 import { Type } from "../../types";
-import { ExtendedParser, hasParsedTokens, isComment } from "../utils";
+import { ExtendedParser, hasParsedTokens } from "../utils";
 import { CompilerNodeToWrappedType } from "../CompilerNodeToWrappedType";
 import { Expression } from "../expression";
 import { KindToNodeMappings } from "../kindToNodeMappings";
@@ -911,25 +911,36 @@ export class Node<NodeType extends ts.Node = ts.Node> {
     }
 
     /**
-     * Gets the first source file text position that is not whitespace taking into account comment nodes.
+     * Gets the first source file text position that is not whitespace taking into account comment nodes and a previous node's trailing trivia.
      */
     getNonWhitespaceStart(): number {
+        // todo: use forgetNodesCreatedInBlock here
         const parent = this.getParent() as Node | undefined;
+        const pos = this.getPos();
         const parentTakesPrecedence = parent != null
             && !TypeGuards.isSourceFile(parent)
-            && parent.getPos() === this.getPos();
+            && parent.getPos() === pos;
 
         if (parentTakesPrecedence)
             return this.getStart(true);
 
         let startSearchPos: number;
-        const previousSibling = this._getCompilerPreviousSibling();
-        if (previousSibling != null && isComment(previousSibling))
+        const sourceFileFullText = this._sourceFile.getFullText();
+        const previousSibling = this.getPreviousSibling();
+
+        if (previousSibling != null && TypeGuards.isCommentNode(previousSibling))
             startSearchPos = previousSibling.getEnd();
-        else
+        else if (previousSibling != null) {
+            if (hasNewLineInRange(sourceFileFullText, [pos, this.getStart(true)]))
+                startSearchPos = previousSibling.getTrailingTriviaEnd();
+            else
+                startSearchPos = pos;
+        }
+        else {
             startSearchPos = this.getPos();
+        }
 
-        return getNextNonWhitespacePos(this._sourceFile.getFullText(), startSearchPos);
+        return getNextNonWhitespacePos(sourceFileFullText, startSearchPos);
     }
 
     /**

src/manipulation/textChecks/isBlankLineAtPos.ts → src/manipulation/textChecks.ts

@@ -1,4 +1,4 @@
-import { SourceFile } from "../../compiler";
+import { SourceFile } from "../compiler";
 
 export function isBlankLineAtPos(sourceFile: SourceFile, pos: number) {
     const fullText = sourceFile.getFullText();
@@ -24,3 +24,15 @@ export function isBlankLineAtPos(sourceFile: SourceFile, pos: number) {
 
     return false;
 }
+
+export function isNewLineAtPos(fullText: string, pos: number) {
+    return fullText[pos] === "\n" || (fullText[pos] === "\r" && fullText[pos + 1] === "\n");
+}
+
+export function hasNewLineInRange(fullText: string, range: [number, number]) {
+    for (let i = range[0]; i < range[1]; i++) {
+        if (fullText[i] === "\n")
+            return true;
+    }
+    return false;
+}

src/manipulation/textManipulators/RemoveChildrenWithFormattingTextManipulator.ts

@@ -19,12 +19,14 @@ export class RemoveChildrenWithFormattingTextManipulator<TNode extends Node> imp
 
     getNewText(inputText: string) {
         const { children, getSiblingFormatting } = this.opts;
-        const parent = children[0].getParentOrThrow() as TNode;
+        const firstChild = children[0];
+        const lastChild = children[children.length - 1];
+        const parent = firstChild.getParentOrThrow() as TNode;
         const sourceFile = parent.getSourceFile();
         const fullText = sourceFile.getFullText();
         const newLineKind = sourceFile._context.manipulationSettings.getNewLineKindAsString();
-        const previousSibling = children[0].getPreviousSibling();
-        const nextSibling = children[children.length - 1].getNextSibling();
+        const previousSibling = firstChild.getPreviousSibling();
+        const nextSibling = lastChild.getNextSibling();
         const removalPos = getRemovalPos();
         this.removalPos = removalPos;
 
@@ -57,14 +59,15 @@ export class RemoveChildrenWithFormattingTextManipulator<TNode extends Node> imp
                 return isNewLineAtPos(fullText, trailingEnd) ? trailingEnd : previousSibling.getEnd();
             }
 
-            const firstPos = getPreviousNonWhitespacePos(fullText, children[0].getPos());
+            const firstPos = getPreviousNonWhitespacePos(fullText, firstChild.getPos());
             if (parent.getPos() === firstPos)
-                return children[0].getNonWhitespaceStart(); // do not shift the parent
+                return firstChild.getNonWhitespaceStart(); // do not shift the parent
 
-            return children[0].isFirstNodeOnLine() ? firstPos : children[0].getNonWhitespaceStart();
+            return firstChild.isFirstNodeOnLine() ? firstPos : firstChild.getNonWhitespaceStart();
         }
 
         function getRemovalEnd() {
+            const triviaEnd = lastChild.getTrailingTriviaEnd();
             if (previousSibling != null && nextSibling != null) {
                 const nextSiblingFormatting = getSiblingFormatting(parent as TNode, nextSibling);
                 if (nextSiblingFormatting === FormattingKind.Blankline || nextSiblingFormatting === FormattingKind.Newline)
@@ -73,20 +76,19 @@ export class RemoveChildrenWithFormattingTextManipulator<TNode extends Node> imp
                 return nextSibling.getNonWhitespaceStart();
             }
 
-            if (parent.getEnd() === children[children.length - 1].getEnd())
-                return children[children.length - 1].getEnd(); // do not shift the parent
+            if (parent.getEnd() === lastChild.getEnd())
+                return lastChild.getEnd(); // do not shift the parent
 
-            const triviaEnd = children[children.length - 1].getTrailingTriviaEnd();
             if (isNewLineAtPos(fullText, triviaEnd)) {
-                if (previousSibling == null && children[0].getPos() === 0)
+                if (previousSibling == null && firstChild.getPos() === 0)
                     return getPosAtNextNonBlankLine(fullText, triviaEnd);
                 return getPosAtEndOfPreviousLine(fullText, getPosAtNextNonBlankLine(fullText, triviaEnd));
             }
 
             if (previousSibling == null)
                 return triviaEnd;
             else
-                return children[children.length - 1].getEnd();
+                return lastChild.getEnd();
         }
     }
 

src/tests/issues/708tests.ts

@@ -21,7 +21,7 @@ describe("tests for issue #706", () => {
     });
 
 
-    it.skip("should not error", () => {
+    it("should not error", () => {
         const project = new Project({ useVirtualFileSystem: true });
         const file = project.createSourceFile("test.ts", `// comment1