Skip to content

Commit

Permalink
fix: Removing last modifier should not remove preceding comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored and David Sherret committed Oct 10, 2018
1 parent 49ddccb commit 3aa9390
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lib/ts-simple-ast.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4271,7 +4271,7 @@ export declare class Node<NodeType extends ts.Node = ts.Node> {
*/
getFullStart(): number;
/**
* Gets the first source file text position from the result of .getPos() that is not whitespace.
* Gets the first source file text position that is not whitespace.
*/
getNonWhitespaceStart(): number;
/**
Expand Down
11 changes: 8 additions & 3 deletions src/compiler/ast/common/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,10 +780,15 @@ export class Node<NodeType extends ts.Node = ts.Node> {
}

/**
* Gets the first source file text position from the result of .getPos() that is not whitespace.
* Gets the first source file text position that is not whitespace.
*/
getNonWhitespaceStart() {
return getNextNonWhitespacePos(this.sourceFile.getFullText(), this.getPos());
getNonWhitespaceStart(): number {
const parent = this.getParent() as Node | undefined;
const parentTakesPrecedence = parent != null
&& !TypeGuards.isSourceFile(parent)
&& parent.getPos() === this.getPos();

return getNextNonWhitespacePos(this.sourceFile.getFullText(), parentTakesPrecedence ? this.getStart(true) : this.getPos());
}

/**
Expand Down
12 changes: 12 additions & 0 deletions src/tests/compiler/base/modifierableNodeTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,17 @@ describe(nameof(ModifierableNode), () => {
firstChild.toggleModifier("export", false);
expect(firstChild.getText()).to.equal("class Identifier {}");
});

it("should not remove jsdoc comment", () => {
const { sourceFile, firstChild } = getInfoFromText<ClassDeclaration>("/** test */declare function Test {}");
firstChild.toggleModifier("declare", false);
expect(sourceFile.getFullText()).to.equal("/** test */function Test {}");
});

it("should not remove comment", () => {
const { sourceFile, firstChild } = getInfoFromText<ClassDeclaration>("// test\ndeclare function Test {}");
firstChild.toggleModifier("declare", false);
expect(sourceFile.getFullText()).to.equal("// test\nfunction Test {}");
});
});
});
26 changes: 26 additions & 0 deletions src/tests/compiler/common/nodeTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1498,4 +1498,30 @@ class MyClass {
expect(syntaxList.getTrailingTriviaEnd()).to.equal(expectedTriviaEnd);
});
});

describe(nameof<Node>(n => n.getNonWhitespaceStart), () => {
function doTest(text: string, selectNode: (sourceFile: SourceFile) => Node, expected: number) {
const { sourceFile } = getInfoFromText(text);
const node = selectNode(sourceFile);
expect(node.getNonWhitespaceStart()).to.equal(expected);
}

// todo: more tests

it("should include the jsdocs if a node with jsdocs", () => {
doTest("/** test */declare function test() {}", sourceFile => sourceFile.getFunctions()[0], 0);
});

it("should not include jsdocs when the pos is before and the start is after", () => {
doTest("/** test */declare function test() {}", sourceFile => sourceFile.getFunctions()[0].getModifiers()[0], 11);
});

it("should not include the comment when for a child with the same pos as the parent", () => {
doTest("// testing\ndeclare function test() {}", sourceFile => sourceFile.getFunctions()[0].getModifiers()[0], 11);
});

it("should include the comment for the node that handles it", () => {
doTest("// testing\ndeclare function test() {}", sourceFile => sourceFile.getFunctions()[0], 0);
});
});
});

0 comments on commit 3aa9390

Please sign in to comment.