Skip to content

Commit

Permalink
fix: #484 - organizeImports() would sometimes throw.
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret committed Oct 28, 2018
1 parent 23ae0a8 commit 447bcfc
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 5 deletions.
11 changes: 9 additions & 2 deletions src/compiler/ast/common/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,17 @@ export class Node<NodeType extends ts.Node = ts.Node> {
if (this.wasForgotten())
return;

this.forgetChildren();
this.forgetOnlyThis();
}

/**
* Forgets the children of this node.
* @internal
*/
forgetChildren() {
for (const child of this.getChildrenInCacheIterator())
child.forget();

this.forgetOnlyThis();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/ast/module/SourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
* @param textChanges - Text changes.
*/
applyTextChanges(textChanges: ReadonlyArray<TextChange>) {
this.getChildSyntaxListOrThrow().forget();
this.forgetChildren();
replaceNodeText({
sourceFile: this.sourceFile,
start: 0,
Expand Down
13 changes: 11 additions & 2 deletions src/manipulation/nodeHandlers/ForgetChangedNodeHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,17 @@ export class ForgetChangedNodeHandler implements NodeHandler {
const [currentNodeChildren, newNodeChildrenArray] = this.helper.getChildrenFast(currentNode, newNode, newSourceFile);
const newNodeChildren = ArrayUtils.toIterator(newNodeChildrenArray);

for (const currentNodeChild of currentNodeChildren)
this.helper.handleForValues(this, currentNodeChild, newNodeChildren.next().value, newSourceFile);
for (const currentNodeChild of currentNodeChildren) {
const nextNodeChildResult = newNodeChildren.next();
if (nextNodeChildResult.done && nextNodeChildResult.value == null) {
const existingNode = this.compilerFactory.getExistingCompilerNode(currentNodeChild);
if (existingNode != null)
existingNode.forget();
}
else {
this.helper.handleForValues(this, currentNodeChild, nextNodeChildResult.value, newSourceFile);
}
}

this.compilerFactory.replaceCompilerNode(currentNode, newNode);
}
Expand Down
35 changes: 35 additions & 0 deletions src/tests/issues/484tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { expect } from "chai";
import { getInfoFromText } from "../compiler/testHelpers";

describe("tests for issue #484", () => {
it("should not error when organizing imports", () => {
const { project } = getInfoFromText("");

const lionFile = project.createSourceFile("src/animal/lion/Lion.ts", `
import {Food} from '../../food/Food';
import {Animal} from '../Animal';
export class Lion {
eat(meat: Food) {}
}`);

const foodFile = project.createSourceFile("src/food/Food.ts", `
export class Food {
energy: number;
canEatBy: number[];
}`);
const foodClass = foodFile.getClassOrThrow("Food");
foodClass.findReferences();

lionFile.addImportDeclaration({
moduleSpecifier: "../foo",
namedImports: [{ name: "Foo" }]
});
lionFile.organizeImports();
expect(lionFile.getText()).to.equal(`import { Food } from '../../food/Food';
export class Lion {
eat(meat: Food) {}
}`);
});
});

0 comments on commit 447bcfc

Please sign in to comment.