Skip to content

Commit

Permalink
fix: sourceFile.isFromExternalLibrary() would become out of date afte…
Browse files Browse the repository at this point in the history
…r a manipulation.
  • Loading branch information
dsherret committed Nov 12, 2018
1 parent 9dbab4c commit 43c6149
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 7 deletions.
22 changes: 20 additions & 2 deletions src/compiler/ast/module/SourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { getNextMatchingPos, getPreviousMatchingPos } from "../../../manipulatio
import { SourceFileStructure } from "../../../structures";
import { Constructor } from "../../../types";
import { LanguageVariant, ScriptTarget, ts } from "../../../typescript";
import { ArrayUtils, EventContainer, FileUtils, ModuleUtils, SourceFileReferenceContainer, StringUtils, TypeGuards } from "../../../utils";
import { ArrayUtils, EventContainer, FileUtils, ModuleUtils, SourceFileReferenceContainer, StringUtils, Memoize } from "../../../utils";
import { getBodyTextWithoutLeadingIndentation } from "../base/helpers";
import { TextInsertableNode, ModuledNode } from "../base";
import { callBaseSet } from "../callBaseSet";
Expand Down Expand Up @@ -68,6 +68,13 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
super(context, node, undefined as any);
this._sourceFile = this;
// end hack

// store this before a modification happens to the file
const onPreModified = () => {
this.isFromExternalLibrary(); // memoize
this._preModifiedEventContainer.unsubscribe(onPreModified);
};
this._preModifiedEventContainer.subscribe(onPreModified);
}

/**
Expand Down Expand Up @@ -494,8 +501,19 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
/**
* Gets if the source file is from an external library.
*/
@Memoize
isFromExternalLibrary() {
return this._context.program.isSourceFileFromExternalLibrary(this);
// This needs to be memoized and stored before modification because the TypeScript
// compiler does the following code:
//
// function isSourceFileFromExternalLibrary(file: SourceFile): boolean {
// return !!sourceFilesFoundSearchingNodeModules.get(file.path);
// }
//
// So the compiler node will become out of date after a manipulation occurs and
// this will return false.
const compilerProgram = this._context.program.compilerObject;
return compilerProgram.isSourceFileFromExternalLibrary(this.compilerNode);
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/tools/Program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ export class Program {
* @param sourceFile - Source file.
*/
isSourceFileFromExternalLibrary(sourceFile: SourceFile) {
return this.compilerObject.isSourceFileFromExternalLibrary(sourceFile.compilerNode);
// Do not use compilerObject.isSourceFileFromExternalLibrary because that method
// will become out of date after a manipulation has happened to a source file.
// Read more in sourceFile.isFromExternalLibrary()'s method body.
return sourceFile.isFromExternalLibrary();
}
}
15 changes: 13 additions & 2 deletions src/tests/compiler/module/sourceFileTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,17 @@ describe(nameof(SourceFile), () => {
});

it("should be when is", () => {
const { librarySourceFile } = trueSetup();
expect(librarySourceFile.isFromExternalLibrary()).to.be.true;
});

it("should be after manipulating the file", () => {
const { librarySourceFile } = trueSetup();
librarySourceFile.addStatements("console;");
expect(librarySourceFile.isFromExternalLibrary()).to.be.true;
});

function trueSetup() {
const fileSystem = getFileSystemHostWithFiles([
{ filePath: "package.json", text: `{ "name": "testing", "version": "0.0.1" }` },
{
Expand All @@ -523,8 +534,8 @@ describe(nameof(SourceFile), () => {
], ["node_modules", "node_modules/library"]);
const { sourceFile } = getInfoFromText("import { Test } from 'library';", { host: fileSystem });
const librarySourceFile = sourceFile.getImportDeclarations()[0].getModuleSpecifierSourceFileOrThrow();
expect(librarySourceFile.isFromExternalLibrary()).to.be.true;
});
return { librarySourceFile };
}
});

describe(nameof<SourceFile>(n => n.getLanguageVariant), () => {
Expand Down
38 changes: 36 additions & 2 deletions src/tests/compiler/tools/programTests.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,51 @@
import { expect } from "chai";
import { Program } from "../../../compiler";
import { getFileSystemHostWithFiles } from "../../testHelpers";
import { getInfoFromText } from "../testHelpers";

describe(nameof(Program), () => {
describe(nameof<Program>(p => p.getGlobalDiagnostics), () => {
it("should get the global diagnostics when not including a the lib.d.ts files", () => {
const { sourceFile, project } = getInfoFromText("const t: string;");
const { project } = getInfoFromText("const t: string;");
expect(project.getProgram().getGlobalDiagnostics().length).to.equal(8);
});

it("should have no global compile errors when including the lib.d.ts files", () => {
const { sourceFile, project } = getInfoFromText("const t: string;", { includeLibDts: true });
const { project } = getInfoFromText("const t: string;", { includeLibDts: true });
expect(project.getProgram().getGlobalDiagnostics().length).to.equal(0);
});
});

describe(nameof<Program>(p => p.isSourceFileFromExternalLibrary), () => {
it("should not be when not", () => {
const { project, sourceFile } = getInfoFromText("");
expect(project.getProgram().isSourceFileFromExternalLibrary(sourceFile)).to.be.false;
});

it("should be when is", () => {
const { program, librarySourceFile } = trueSetup();
expect(program.isSourceFileFromExternalLibrary(librarySourceFile)).to.be.true;
});

it("should be after manipulating the file", () => {
const { program, librarySourceFile } = trueSetup();
librarySourceFile.addStatements("console;");
expect(program.isSourceFileFromExternalLibrary(librarySourceFile)).to.be.true;
});

function trueSetup() {
const fileSystem = getFileSystemHostWithFiles([
{ filePath: "package.json", text: `{ "name": "testing", "version": "0.0.1" }` },
{
filePath: "node_modules/library/package.json",
text: `{ "name": "library", "version": "0.0.1", "main": "index.js", "typings": "index.d.ts", "typescript": { "definition": "index.d.ts" } }`
},
{ filePath: "node_modules/library/index.js", text: "export class Test {}" },
{ filePath: "node_modules/library/index.d.ts", text: "export class Test {}" }
], ["node_modules", "node_modules/library"]);
const { sourceFile, project } = getInfoFromText("import { Test } from 'library';", { host: fileSystem });
const librarySourceFile = sourceFile.getImportDeclarations()[0].getModuleSpecifierSourceFileOrThrow();
return { program: project.getProgram(), librarySourceFile };
}
});
});

0 comments on commit 43c6149

Please sign in to comment.