Skip to content

Commit

Permalink
fix: createDirectory should throw if the directory exists.
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret committed Dec 17, 2017
1 parent d9d8a54 commit 93a9da2
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 28 deletions.
11 changes: 5 additions & 6 deletions src/TsSimpleAst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,23 @@ export class TsSimpleAst {
dirPath = FileUtils.getStandardizedAbsolutePath(dirPath);
if (!this.fileSystem.directoryExistsSync(dirPath))
throw new errors.DirectoryNotFoundError(dirPath);
return this.global.compilerFactory.createDirectoryIfNotExists(dirPath);
return this.global.compilerFactory.addDirectoryIfNotExists(dirPath);
}

/**
* Creates a directory at the specified path.
*
* Will return the existing directory if it was already created.
* Note: Will not save the directory to disk until one of its source files is saved.
* @param dirPath
* @param dirPath - Path to create the directory at.
* @throws - InvalidOperationError if a directory already exists at the provided file path.
*/
createDirectory(dirPath: string): Directory {
dirPath = FileUtils.getStandardizedAbsolutePath(dirPath);
return this.global.compilerFactory.createDirectoryIfNotExists(dirPath);
return this.global.compilerFactory.createDirectory(dirPath);
}

/**
* Gets a directory by the specified path or throws it doesn't exist.
* @param dirPath - Directory path.
* @param dirPath - Path to create the directory at.
*/
getDirectoryOrThrow(dirPath: string): Directory {
return errors.throwIfNullOrUndefined(this.getDirectory(dirPath),
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/tools/LanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class LanguageService {
return this.global.fileSystem.readFileSync(path, encoding);
},
fileExists: fileExistsSync,
directoryExists: dirName => this.global.compilerFactory.containsFileInDirectory(dirName) || this.global.fileSystem.directoryExistsSync(dirName)
directoryExists: dirName => this.global.compilerFactory.containsDirectoryAtPath(dirName) || this.global.fileSystem.directoryExistsSync(dirName)
};

this.compilerHost = {
Expand Down
41 changes: 26 additions & 15 deletions src/factories/CompilerFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ export class CompilerFactory {
* @param sourceText - Text to create the source file with.
*/
createSourceFileFromText(filePath: string, sourceText: string) {
const absoluteFilePath = FileUtils.getStandardizedAbsolutePath(filePath);
if (this.containsSourceFileAtPath(absoluteFilePath))
throw new errors.InvalidOperationError(`A source file already exists at the provided file path: ${absoluteFilePath}`);
const compilerSourceFile = ts.createSourceFile(absoluteFilePath, sourceText, this.global.manipulationSettings.getScriptTarget(), true);
filePath = FileUtils.getStandardizedAbsolutePath(filePath);
if (this.containsSourceFileAtPath(filePath) || this.global.fileSystem.fileExistsSync(filePath))
throw new errors.InvalidOperationError(`A source file already exists at the provided file path: ${filePath}`);
const compilerSourceFile = ts.createSourceFile(filePath, sourceText, this.global.manipulationSettings.getScriptTarget(), true);
return this.getSourceFile(compilerSourceFile);
}

Expand All @@ -105,12 +105,13 @@ export class CompilerFactory {
* @param filePath - File path to get the file from.
*/
getSourceFileFromFilePath(filePath: string): compiler.SourceFile | undefined {
const absoluteFilePath = FileUtils.getStandardizedAbsolutePath(filePath);
let sourceFile = this.sourceFileCacheByFilePath.get(absoluteFilePath);
filePath = FileUtils.getStandardizedAbsolutePath(filePath);
let sourceFile = this.sourceFileCacheByFilePath.get(filePath);
if (sourceFile == null) {
if (this.global.fileSystem.fileExistsSync(absoluteFilePath)) {
Logger.log(`Loading file: ${absoluteFilePath}`);
sourceFile = this.createSourceFileFromText(absoluteFilePath, this.global.fileSystem.readFileSync(absoluteFilePath));
if (this.global.fileSystem.fileExistsSync(filePath)) {
Logger.log(`Loading file: ${filePath}`);
const compilerSourceFile = ts.createSourceFile(filePath, this.global.fileSystem.readFileSync(filePath), this.global.manipulationSettings.getScriptTarget(), true);
sourceFile = this.getSourceFile(compilerSourceFile);
sourceFile.setIsSaved(true); // source files loaded from the disk are saved to start with
}

Expand All @@ -130,14 +131,14 @@ export class CompilerFactory {
*/
containsSourceFileAtPath(filePath: string) {
const absoluteFilePath = FileUtils.getStandardizedAbsolutePath(filePath);
return this.sourceFileCacheByFilePath.get(absoluteFilePath) != null;
return this.sourceFileCacheByFilePath.has(absoluteFilePath);
}

/**
* Gets if the internal cache contains a source file with the specified directory path.
* @param dirPath - Directory path to check.
*/
containsFileInDirectory(dirPath: string) {
containsDirectoryAtPath(dirPath: string) {
const normalizedDirPath = FileUtils.getStandardizedAbsolutePath(dirPath);
return this.directoryCache.has(normalizedDirPath);
}
Expand Down Expand Up @@ -204,7 +205,7 @@ export class CompilerFactory {

// add to list of directories
const dirPath = sourceFile.getDirectoryPath();
this.directoryCache.addIfNotExists(dirPath);
this.directoryCache.createOrAddIfNotExists(dirPath);
this.directoryCache.get(dirPath)!._addSourceFile(sourceFile);

// fire the event
Expand All @@ -215,11 +216,21 @@ export class CompilerFactory {
}

/**
* Creates a directory if it doesn't exist.
* Adds a directory if it doesn't exist.
* @param dirPath - Directory path.
*/
addDirectoryIfNotExists(dirPath: string) {
return this.directoryCache.createOrAddIfNotExists(dirPath);
}

/**
* Creates a directory.
* @param dirPath - Directory path.
*/
createDirectoryIfNotExists(dirPath: string) {
return this.directoryCache.addIfNotExists(dirPath);
createDirectory(dirPath: string) {
if (this.containsDirectoryAtPath(dirPath) || this.global.fileSystem.directoryExistsSync(dirPath))
throw new errors.InvalidOperationError(`A directory already exists at the provided path: ${dirPath}`);
return this.directoryCache.createOrAddIfNotExists(dirPath);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/factories/DirectoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class DirectoryCache {
return this.directoriesByPath.removeByKey(dirPath);
}

addIfNotExists(dirPath: string) {
createOrAddIfNotExists(dirPath: string) {
if (this.has(dirPath))
return this.get(dirPath)!;

Expand Down
4 changes: 2 additions & 2 deletions src/fileSystem/Directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export class Directory {
const dirPath = FileUtils.getStandardizedAbsolutePath(FileUtils.pathJoin(this.getPath(), relativePath));
if (!this.global.fileSystem.directoryExistsSync(dirPath))
throw new errors.DirectoryNotFoundError(dirPath);
return this.global.compilerFactory.createDirectoryIfNotExists(dirPath);
return this.global.compilerFactory.addDirectoryIfNotExists(dirPath);
}

/**
Expand All @@ -211,7 +211,7 @@ export class Directory {
*/
createDirectory(relativePath: string) {
const dirPath = FileUtils.getStandardizedAbsolutePath(FileUtils.pathJoin(this.getPath(), relativePath));
return this.global.compilerFactory.createDirectoryIfNotExists(dirPath);
return this.global.compilerFactory.createDirectory(dirPath);
}

/**
Expand Down
20 changes: 17 additions & 3 deletions src/tests/fileSystem/directoryTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ describe(nameof(Directory), () => {
children?: TreeNode[];
}

function getAst() {
const ast = new TsSimpleAst(undefined, getFileSystemHostWithFiles([]));
function getAst(initialFiles: { filePath: string; text: string; }[] = [], initialDirectories: string[] = []) {
const ast = new TsSimpleAst(undefined, getFileSystemHostWithFiles(initialFiles, initialDirectories));
return ast;
}

Expand Down Expand Up @@ -208,6 +208,12 @@ describe(nameof(Directory), () => {
it("should create a source file in the directory when specifying a structure", () => {
doTest({ enums: [{ name: "MyEnum" }] }, "enum MyEnum {\n}\n");
});

it("should throw an exception if creating a source file at an existing path on the disk", () => {
const ast = getAst([{ filePath: "file.ts", text: "" }], [""]);
const directory = ast.addExistingDirectory("");
expect(() => directory.createSourceFile("file.ts", "")).to.throw(errors.InvalidOperationError);
});
});

describe(nameof<Directory>(d => d.addExistingSourceFile), () => {
Expand All @@ -230,7 +236,7 @@ describe(nameof(Directory), () => {
});

describe(nameof<Directory>(d => d.createDirectory), () => {
const ast = getAst();
const ast = getAst([], ["", "childDir"]);
const directory = ast.createDirectory("some/path");
directory.createDirectory("child");
directory.createDirectory("../../dir/other/deep/path");
Expand All @@ -256,6 +262,14 @@ describe(nameof(Directory), () => {
}]
});
});

it("should throw when a directory already exists at the specified path", () => {
expect(() => directory.createDirectory("child")).to.throw(errors.InvalidOperationError);
});

it("should throw when a directory already exists on the file system at the specified path", () => {
expect(() => ast.addExistingDirectory("").createDirectory("childDir")).to.throw(errors.InvalidOperationError);
});
});

describe(nameof<Directory>(d => d.addExistingDirectory), () => {
Expand Down
19 changes: 19 additions & 0 deletions src/tests/tsSimpleAstTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ describe(nameof(TsSimpleAst), () => {
expect(createdDir).to.not.be.undefined;
expect(ast.getDirectoryOrThrow("someDir")).to.equal(createdDir);
});

it("should throw when a directory already exists at the specified path", () => {
const fileSystem = testHelpers.getFileSystemHostWithFiles([]);
const ast = new TsSimpleAst(undefined, fileSystem);
const createdDir = ast.createDirectory("someDir");
expect(() => ast.createDirectory("someDir")).to.throw(errors.InvalidOperationError);
});

it("should throw when a directory already exists on the file system at the specified path", () => {
const fileSystem = testHelpers.getFileSystemHostWithFiles([], ["childDir"]);
const ast = new TsSimpleAst(undefined, fileSystem);
expect(() => ast.createDirectory("childDir")).to.throw(errors.InvalidOperationError);
});
});

describe(nameof<TsSimpleAst>(ast => ast.getDirectory), () => {
Expand Down Expand Up @@ -202,6 +215,12 @@ describe(nameof(TsSimpleAst), () => {
}).to.throw(errors.InvalidOperationError, `A source file already exists at the provided file path: ${FileUtils.getStandardizedAbsolutePath("file.ts")}`);
});

it("should throw an exception if creating a source file at an existing path on the disk", () => {
const fileSystem = testHelpers.getFileSystemHostWithFiles([{ filePath: "file.ts", text: "" }]);
const ast = new TsSimpleAst(undefined, fileSystem);
expect(() => ast.createSourceFile("file.ts", "")).to.throw(errors.InvalidOperationError);
});

it("should mark the source file as having not been saved", () => {
const ast = new TsSimpleAst();
const sourceFile = ast.createSourceFile("file.ts", "");
Expand Down

0 comments on commit 93a9da2

Please sign in to comment.