fix: #424 - UTF-8 bom causes incorrect indentation to be inserted.

Better handling of BOM.

src/compiler/file/SourceFile.ts

@@ -52,6 +52,9 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
     /** @internal */
     readonly _referenceContainer = new SourceFileReferenceContainer(this);
 
+    /** @internal */
+    _hasBom: true | undefined;
+
     /**
      * Initializes a new instance.
      * @internal
@@ -392,18 +395,24 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
      * Asynchronously saves this file with any changes.
      */
     async save() {
-        await this.context.fileSystemWrapper.writeFile(this.getFilePath(), this.getFullText());
+        await this.context.fileSystemWrapper.writeFile(this.getFilePath(), this._getTextForSave());
         this._isSaved = true;
     }
 
     /**
      * Synchronously saves this file with any changes.
      */
     saveSync() {
-        this.context.fileSystemWrapper.writeFileSync(this.getFilePath(), this.getFullText());
+        this.context.fileSystemWrapper.writeFileSync(this.getFilePath(), this._getTextForSave());
         this._isSaved = true;
     }
 
+    /** @internal */
+    private _getTextForSave() {
+        const text = this.getFullText();
+        return this._hasBom ? "\uFEFF" + text : text;
+    }
+
     /**
      * Gets any referenced files.
      */

src/factories/CompilerFactory.ts

@@ -7,7 +7,7 @@ import { SourceFileCreateOptions } from "../Project";
 import { SourceFileStructure } from "../structures";
 import { SyntaxKind, ScriptTarget, ts, TypeFlags } from "../typescript";
 import { replaceSourceFileForCacheUpdate } from "../manipulation";
-import { ArrayUtils, EventContainer, FileUtils, KeyValueCache, WeakCache } from "../utils";
+import { ArrayUtils, EventContainer, FileUtils, KeyValueCache, WeakCache, StringUtils } from "../utils";
 import { DirectoryCache } from "./DirectoryCache";
 import { ForgetfulNodeCache } from "./ForgetfulNodeCache";
 import { kindToWrapperMappings } from "./kindToWrapperMappings";
@@ -271,7 +271,13 @@ export class CompilerFactory {
     }
 
     private createSourceFileFromTextInternal(filePath: string, text: string): SourceFile {
-        return this.getSourceFile(this.createCompilerSourceFileFromText(filePath, text));
+        const hasBom = StringUtils.hasBom(text);
+        if (hasBom)
+            text = StringUtils.stripBom(text);
+        const sourceFile = this.getSourceFile(this.createCompilerSourceFileFromText(filePath, text));
+        if (hasBom)
+            sourceFile._hasBom = true;
+        return sourceFile;
     }
 
     createCompilerSourceFileFromText(filePath: string, text: string): ts.SourceFile {

src/manipulation/helpers/getInsertPosFromIndex.ts

@@ -1,5 +1,4 @@
 import { Node, SyntaxList } from "../../compiler";
-import { Chars } from "../../constants";
 import { SyntaxKind, ts } from "../../typescript";
 import { TypeGuards } from "../../utils";
 import { getPosAtStartOfLineOrNonWhitespace } from "../textSeek";
@@ -11,7 +10,7 @@ export function getInsertPosFromIndex(index: number, syntaxList: SyntaxList, chi
     if (index === 0) {
         const parent = syntaxList.getParentOrThrow();
         if (TypeGuards.isSourceFile(parent))
-            return parent.getFullText()[0] === Chars.BOM ? 1 : 0;
+            return 0;
         else if (TypeGuards.isCaseClause(parent) || TypeGuards.isDefaultClause(parent)) {
             const block = parent.getFirstChildIfKind(SyntaxKind.Block);
             const colonToken = parent.getFirstChildByKindOrThrow(SyntaxKind.ColonToken);

src/manipulation/manipulations/insertion.ts

@@ -1,6 +1,5 @@
 import { CodeBlockWriter } from "../../codeBlockWriter";
 import { Node } from "../../compiler";
-import { Chars } from "../../constants";
 import { SyntaxKind, ts } from "../../typescript";
 import { TypeGuards } from "../../utils";
 import { getEndPosFromIndex, getInsertPosFromIndex, getRangeFromArray, verifyAndGetIndex } from "../helpers";
@@ -153,7 +152,7 @@ export function insertIntoBracesOrSourceFile<TStructure = {}>(opts: InsertIntoBr
         opts.write(writer, {
             previousMember: getChild(children[index - 1]),
             nextMember: getChild(children[index]),
-            isStartOfFile: insertPos === 0 || insertPos === 1 && fullText[0] === Chars.BOM
+            isStartOfFile: insertPos === 0
         });
         return writer.toString();
 

src/tests/compiler/file/sourceFileTests.ts

@@ -1,7 +1,6 @@
 import { expect } from "chai";
 import { Node, EmitResult, ExportAssignment, ExportDeclaration, FileSystemRefreshResult, FormatCodeSettings,
     ImportDeclaration, QuoteKind, SourceFile } from "../../../compiler";
-import { Chars } from "../../../constants";
 import * as errors from "../../../errors";
 import { IndentationText, ManipulationSettings } from "../../../options";
 import { Project } from "../../../Project";
@@ -306,12 +305,11 @@ describe(nameof(SourceFile), () => {
     });
 
     describe(nameof<SourceFile>(n => n.save), () => {
-        const fileText = "    interface Identifier {}    ";
-        const filePath = "/Folder/File.ts";
-        const host = getFileSystemHostWithFiles([]);
-        const { sourceFile } = getInfoFromText(fileText, { filePath, host });
-
         it("should save the file", async () => {
+            const fileText = "    interface Identifier {}    ";
+            const filePath = "/Folder/File.ts";
+            const host = getFileSystemHostWithFiles([]);
+            const { sourceFile } = getInfoFromText(fileText, { filePath, host });
             expect(sourceFile.isSaved()).to.be.false;
 
             await sourceFile.save();
@@ -322,6 +320,45 @@ describe(nameof(SourceFile), () => {
             expect(entry.fileText).to.equal(fileText);
             expect(writeLog.length).to.equal(1);
         });
+
+        it("should write with BOM if read with BOM", async () => {
+            const host = getFileSystemHostWithFiles([]);
+            const fileText = "const t: string;";
+            const encodedFileText = `\uFEFF${fileText}`;
+            const { sourceFile } = getInfoFromText(encodedFileText, { filePath: "/file.ts", host });
+            await sourceFile.save();
+            expect(host.getWriteLog()[0].fileText).to.equal(encodedFileText);
+            expect(sourceFile.getFullText()).to.equal(fileText);
+        });
+    });
+
+    describe(nameof<SourceFile>(n => n.saveSync), () => {
+        it("should save the file", () => {
+            const fileText = "    interface Identifier {}    ";
+            const filePath = "/Folder/File.ts";
+            const host = getFileSystemHostWithFiles([]);
+            const { sourceFile } = getInfoFromText(fileText, { filePath, host });
+
+            expect(sourceFile.isSaved()).to.be.false;
+
+            sourceFile.saveSync();
+            expect(sourceFile.isSaved()).to.be.true;
+            const writeLog = host.getWriteLog();
+            const entry = writeLog[0];
+            expect(entry.filePath).to.equal(filePath);
+            expect(entry.fileText).to.equal(fileText);
+            expect(writeLog.length).to.equal(1);
+        });
+
+        it("should write with BOM if read with BOM", () => {
+            const host = getFileSystemHostWithFiles([]);
+            const fileText = "const t: string;";
+            const encodedFileText = `\uFEFF${fileText}`;
+            const { sourceFile } = getInfoFromText(encodedFileText, { filePath: "/file.ts", host });
+            sourceFile.saveSync();
+            expect(host.getWriteLog()[0].fileText).to.equal(encodedFileText);
+            expect(sourceFile.getFullText()).to.equal(fileText);
+        });
     });
 
     describe(nameof<SourceFile>(n => n.delete), () => {
@@ -400,25 +437,6 @@ describe(nameof(SourceFile), () => {
         });
     });
 
-    describe(nameof<SourceFile>(n => n.saveSync), () => {
-        const fileText = "    interface Identifier {}    ";
-        const filePath = "/Folder/File.ts";
-        const host = getFileSystemHostWithFiles([]);
-        const { sourceFile } = getInfoFromText(fileText, { filePath, host });
-
-        it("should save the file", () => {
-            expect(sourceFile.isSaved()).to.be.false;
-
-            sourceFile.saveSync();
-            expect(sourceFile.isSaved()).to.be.true;
-            const writeLog = host.getWriteLog();
-            const entry = writeLog[0];
-            expect(entry.filePath).to.equal(filePath);
-            expect(entry.fileText).to.equal(fileText);
-            expect(writeLog.length).to.equal(1);
-        });
-    });
-
     describe(nameof<SourceFile>(n => n.isDeclarationFile), () => {
         it("should be a source file when the file name ends with .d.ts", () => {
             const project = new Project({ useVirtualFileSystem: true });
@@ -491,8 +509,8 @@ describe(nameof(SourceFile), () => {
             }).to.throw();
         });
 
-        it("should insert an import after a utf-8 bom", () => {
-            doTest(Chars.BOM, 0, [{ moduleSpecifier: "./test" }], `${Chars.BOM}import "./test";\n`);
+        it("should insert an import if the file was read with a utf-8 bom", () => {
+            doTest("\uFEFF", 0, [{ moduleSpecifier: "./test" }], `import "./test";\n`);
         });
 
         it("should insert at the beginning and use single quotes when specified", () => {

src/tests/compiler/statement/statementedNodeTests.ts

@@ -1,6 +1,5 @@
 import { expect } from "chai";
 import { CaseClause, DefaultClause, FunctionDeclaration, NamespaceDeclaration, Node, SourceFile, StatementedNode, Block } from "../../../compiler";
-import { Chars } from "../../../constants";
 import { StatementedNodeStructure } from "../../../structures";
 import { SyntaxKind } from "../../../typescript";
 import { TypeGuards } from "../../../utils";
@@ -117,7 +116,7 @@ describe(nameof(StatementedNode), () => {
         });
 
         it("should insert statements after a utf-8 bom", () => {
-            doSourceFileTest(Chars.BOM, 0, "newText;", 1, Chars.BOM + "newText;\n");
+            doSourceFileTest("\uFEFF", 0, "newText;", 1, "newText;\n");
         });
 
         it("should allow inserting nothing", () => {

src/tests/issues/424tests.ts

@@ -0,0 +1,13 @@
+import { expect } from "chai";
+import { getInfoFromText } from "../compiler/testHelpers";
+
+describe("tests for issue #424", () => {
+    it("should insert with correct indentation when the file has a BOM at the beginning", () => {
+        const { sourceFile } = getInfoFromText("\ufeffmodule Test {\n    enum Test {\n    }\n}");
+        const enumDec = sourceFile.getNamespaceOrThrow("Test").getEnumOrThrow("Test");
+
+        enumDec.addMember({ name: "member", initializer: "5" });
+
+        expect(sourceFile.getFullText()).to.equal("module Test {\n    enum Test {\n        member = 5\n    }\n}");
+    });
+});

src/utils/FileUtils.ts

@@ -1,6 +1,5 @@
 import * as toAbsoluteGlob from "@dsherret/to-absolute-glob";
 import * as path from "path";
-import { Chars } from "../constants";
 import { FileSystemHost, FileSystemWrapper } from "../fileSystem";
 import { ArrayUtils } from "./ArrayUtils";
 import { StringUtils } from "./StringUtils";
@@ -208,9 +207,9 @@ export class FileUtils {
      * @param text - Text.
      */
     static getTextWithByteOrderMark(text: string) {
-        if (text[0] === Chars.BOM)
+        if (StringUtils.hasBom(text))
             return text;
-        return Chars.BOM + text;
+        return "\uFEFF" + text;
     }
 
     /**

src/utils/StringUtils.ts

@@ -7,6 +7,16 @@ export class StringUtils {
     private constructor() {
     }
 
+    static hasBom(text: string) {
+        return text.charCodeAt(0) === 0xFEFF;
+    }
+
+    static stripBom(text: string) {
+        if (StringUtils.hasBom(text))
+            return text.slice(1);
+        return text;
+    }
+
     static isNullOrWhitespace(str: string | undefined): str is undefined {
         return typeof str !== "string" || str.trim().length === 0;
     }
@@ -122,10 +132,12 @@ export class StringUtils {
 
 export class Es5StringUtils {
     static startsWith(str: string, startsWithString: string) {
+        // todo: don't allocate a string
         return str.substr(0, startsWithString.length) === startsWithString;
     }
 
     static endsWith(str: string, endsWithString: string) {
+        // todo: don't allocate a string
         return str.substr(str.length - endsWithString.length, endsWithString.length) === endsWithString;
     }
 }