fix(file): handle file diff overwrite properly

Signed-off-by: Emilien Escalle <emilien.escalle@escemi.com>

src/services/CliService.ts

@@ -9,6 +9,12 @@ import { ConsoleService } from "./ConsoleService";
 import { DirectoryService } from "./file/DirectoryService";
 import { ColorService } from "./ColorService";
 
+export enum PromptOverwriteChoice {
+  DIFF = "diff",
+  OVERWRITE = "overwrite",
+  CANCEL = "cancel",
+}
+
 export class CliService {
   private runStartDate: Date | undefined;
 
@@ -120,17 +126,17 @@ export class CliService {
       const action = await this.promptToChoose(
         `File "${file}" exists already, what do you want to do?`,
         {
-          "Show diff": "diff",
-          "Overwrite file": "overwrite",
-          "Keep original file": "cancel",
+          "Show diff": PromptOverwriteChoice.DIFF,
+          "Overwrite file": PromptOverwriteChoice.OVERWRITE,
+          "Keep original file": PromptOverwriteChoice.CANCEL,
         }
       );
 
-      if (action === "cancel") {
+      if (action === PromptOverwriteChoice.CANCEL) {
         return false;
       }
 
-      if (action === "overwrite") {
+      if (action === PromptOverwriteChoice.OVERWRITE) {
         return true;
       }
 

src/services/file/FileDiffService.spec.ts

@@ -30,7 +30,7 @@ line 4 content`;
 
       mockDir({ [fileName]: originalContent });
 
-      const diffs = await service.getFileContentDiff(filePath, originalContent, originalContent);
+      const diffs = await service.getFileContentChanges(filePath, originalContent, originalContent);
 
       expect(diffs).toEqual([]);
     });
@@ -48,14 +48,20 @@ line 4 content`;
 
       mockDir({ [fileName]: originalContent });
 
-      const diffs = await service.getFileContentDiff(filePath, originalContent, newContent);
+      const diffs = await service.getFileContentChanges(filePath, originalContent, newContent);
 
       expect(diffs).toEqual([
         {
           added: false,
-          count: 2,
+          count: 1,
+          removed: false,
+          value: "line 1 content\n",
+        },
+        {
+          added: false,
+          count: 1,
           removed: false,
-          value: "line 1 content\nline 2 content\n",
+          value: "line 2 content\n",
         },
         {
           added: false,

src/services/file/FileDiffService.ts

@@ -12,7 +12,7 @@ export class FileDiffService {
     @inject(FileService) private readonly fileService: FileService
   ) {}
 
-  async getFileContentDiff(
+  async getFileContentChanges(
     filepath: string,
     fileContent: string,
     newFileContent: string
@@ -29,39 +29,58 @@ export class FileDiffService {
       return [];
     }
 
-    const changes = diffLines(fileContent, newFileContent);
+    return diffLines(fileContent, newFileContent, { oneChangePerToken: true });
+  }
+
+  async fileNeedsOverwrite(filePath: string, changes: Change[]): Promise<boolean> {
+    if (!changes.length) {
+      return false;
+    }
 
-    // Retrieve previous changes
-    const overwritedChanges = await this.getOverwritedFilesChanges(filepath);
+    const overwritedChanges = await this.getOverwritedFileChanges(filePath);
     if (!overwritedChanges) {
-      return changes;
+      return true;
     }
 
-    // Check if changes occured on past overwrites
-    const hasSomeNewChange = changes.some((change) => {
-      for (const overwritedChange of overwritedChanges) {
-        // TODO: compare changes
-        const changeDiffs = overwritedChange !== change;
-        if (changeDiffs) {
-          return true;
-        }
+    const nonOverwritedChanges: Change[] = [];
+
+    let currentLine = -1;
+    for (const change of changes) {
+      if (change.count === undefined) {
+        nonOverwritedChanges.push(change);
+        continue;
       }
-      return false;
-    });
+      currentLine += change.count;
 
-    if (hasSomeNewChange) {
-      return changes;
+      const overwritedChange = this.getOverwritedLineChange(currentLine, overwritedChanges);
+      if (!overwritedChange) {
+        nonOverwritedChanges.push(change);
+      }
     }
 
-    return [];
+    return nonOverwritedChanges.length > 0;
   }
 
-  private async getOverwritedFilesChanges(filepath: string): Promise<Change[] | undefined> {
+  private async getOverwritedFileChanges(filepath: string): Promise<Change[] | undefined> {
     const fileRealpath = await this.fileService.getFileRealpath(filepath);
     return FileDiffService.overwritedFilesChanges.get(fileRealpath);
   }
 
-  async setOverwritedFilesChanges(filepath: string, diff: Change[]): Promise<void> {
+  private getOverwritedLineChange(line: number, changes: Change[]): Change | undefined {
+    let currentLine = -1;
+    for (const change of changes) {
+      if (change.count === undefined) {
+        continue;
+      }
+
+      currentLine += change.count;
+      if (currentLine === line) {
+        return change;
+      }
+    }
+  }
+
+  async setOverwritedFileChanges(filepath: string, diff: Change[]): Promise<void> {
     const fileRealpath = await this.fileService.getFileRealpath(filepath);
     FileDiffService.overwritedFilesChanges.set(fileRealpath, diff);
   }

src/services/file/JsonFile.ts

@@ -50,7 +50,7 @@ export class JsonFile extends StdFile {
       return JSON.parse(content);
     } catch (error) {
       throw new Error(
-        `An error occurred while parsing file content "${this.file}": ${JSON.stringify(
+        `An error occurred while parsing file content "${this.filePath}": ${JSON.stringify(
           error instanceof Error ? error.message : error
         )} => "${content.trim()}"`
       );

src/services/file/StdFile.spec.ts

@@ -13,9 +13,6 @@ import { StdFile } from "./StdFile";
 import { DirectoryService } from "./DirectoryService";
 
 describe("services - File - StdFile", () => {
-  const fileName = "test.txt";
-  const filePath = join(mockDirPath, fileName);
-
   let cliService: CliService;
   let directoryService: DirectoryService;
   let fileService: FileService;
@@ -40,6 +37,9 @@ describe("services - File - StdFile", () => {
     it("should save a new file", async () => {
       mockDir();
 
+      const fileName = "test.txt";
+      const filePath = join(mockDirPath, fileName);
+
       const fileContent = "test content";
       const file = new StdFile(
         cliService,
@@ -60,6 +60,9 @@ describe("services - File - StdFile", () => {
     });
 
     it("should override an existing file", async () => {
+      const fileName = "test-override.txt";
+      const filePath = join(mockDirPath, fileName);
+
       mockDir({ [fileName]: "test original content" });
 
       prompts.inject(["overwrite"]);
@@ -83,6 +86,8 @@ describe("services - File - StdFile", () => {
     });
 
     it("should not override an existing file", async () => {
+      const fileName = "test-not-override.txt";
+      const filePath = join(mockDirPath, fileName);
       const originalContent = "test original content";
 
       mockDir({ [fileName]: originalContent });
@@ -106,18 +111,25 @@ describe("services - File - StdFile", () => {
       expect(await readFile(filePath, "utf-8")).toEqual(originalContent);
     });
 
-    // FIXME: This behaviour must be implemented
-    it.skip("should ask for overriding an existing file only onte time if changes occured in the same place", async () => {
-      const originalContent = `
-        line 1 content
-        line 2 content
-        line 3 content
-        line 4 content
-      `;
+    it("should ask for overriding an existing file only once time if changes occured in the same place", async () => {
+      // Arrange
+      const fileName = "test-override-once.txt";
+      const filePath = join(mockDirPath, fileName);
+      const originalContent = `line 1 content
+line 2 content
+line 3 content
+line 4 content`;
+
       mockDir({ [fileName]: originalContent });
 
       prompts.inject(["overwrite"]);
 
+      const newContent = `line 1 content
+line 2 content new
+line 3 content
+line 4 content`;
+
+      // Act
       let file = new StdFile(
         cliService,
         directoryService,
@@ -126,24 +138,21 @@ describe("services - File - StdFile", () => {
         fileFactory,
         filePath,
         undefined,
-        `
-      line 1 content
-      line 2 content new
-      line 3 content
-      line 4 content
-    `
+        newContent
       );
       await file.saveFile();
 
+      // Assert
+      expect(file.getContent()).toEqual(newContent);
+      expect(await readFile(filePath, "utf-8")).toEqual(newContent);
+
       // Mock prompt in case of test failure
       prompts.inject(["cancel"]);
 
-      const newContent = `
-        line 1 content
-        line 2 content new one
-        line 3 content
-        line 4 content
-      `;
+      const anotherNewContent = `line 1 content
+line 2 content new one
+line 3 content
+line 4 content`;
 
       file = new StdFile(
         cliService,
@@ -153,13 +162,13 @@ describe("services - File - StdFile", () => {
         fileFactory,
         filePath,
         undefined,
-        newContent
+        anotherNewContent
       );
 
-      const result = await file.saveFile();
+      await file.saveFile();
 
-      expect(result.getContent()).toEqual(newContent);
-      expect(await readFile(filePath, "utf-8")).toEqual(newContent);
+      expect(file.getContent()).toEqual(anotherNewContent);
+      expect(await readFile(filePath, "utf-8")).toEqual(anotherNewContent);
     });
   });
 });

src/services/file/StdFile.ts

@@ -16,7 +16,7 @@ export class StdFile {
     protected readonly fileService: FileService,
     protected readonly fileDiffService: FileDiffService,
     protected readonly fileFactory: FileFactory,
-    protected readonly file: string | null = null,
+    protected readonly filePath: string | null = null,
     protected readonly encoding: BufferEncoding = "utf8",
     content = ""
   ) {
@@ -77,44 +77,54 @@ export class StdFile {
     return this;
   }
 
-  async saveFile(file: string | null = null, encoding?: BufferEncoding): Promise<this> {
-    if (file === null) {
-      if (this.file === null) {
+  async saveFile(filePath: string | null = null, encoding?: BufferEncoding): Promise<this> {
+    if (filePath === null) {
+      if (this.filePath === null) {
         throw new Error("A file path is mandatory to save file");
       }
-      file = this.file;
+      filePath = this.filePath;
     }
 
     // Check if file directory exist
-    const parentDirPath = dirname(file);
+    const parentDirPath = dirname(filePath);
     const parentDirExists = await this.directoryService.dirExists(parentDirPath);
     if (!parentDirExists) {
       throw new Error(
-        `Unable to create file "${file}, parent directory ${parentDirPath} does not exist`
+        `Unable to create file "${filePath}, parent directory ${parentDirPath} does not exist`
       );
     }
 
     const newFileContent = this.fixContentEOL(this.getContent()).trim();
     encoding = encoding ?? this.encoding;
 
-    const fileExists = await this.fileService.fileExists(file);
+    const fileExists = await this.fileService.fileExists(filePath);
     if (fileExists) {
-      const fileContent = await this.fileService.getFileContent(file, encoding);
-      const diff = await this.fileDiffService.getFileContentDiff(file, fileContent, newFileContent);
+      const fileContent = await this.fileService.getFileContent(filePath, encoding);
+      const changes = await this.fileDiffService.getFileContentChanges(
+        filePath,
+        fileContent,
+        newFileContent
+      );
+
+      const shouldPromptOverwrite = await this.fileDiffService.fileNeedsOverwrite(
+        filePath,
+        changes
+      );
 
-      if (diff.length) {
-        const overwrite = await this.cliService.promptOverwriteFileDiff(file, diff);
+      if (shouldPromptOverwrite) {
+        const overwrite = await this.cliService.promptOverwriteFileDiff(filePath, changes);
 
         if (!overwrite) {
           // Do not update file, set real file content
           this.setContent(fileContent);
           return this;
         }
-        await this.fileDiffService.setOverwritedFilesChanges(file, diff);
+
+        await this.fileDiffService.setOverwritedFileChanges(filePath, changes);
       }
     }
 
-    await this.fileService.writeFileContent(file, newFileContent, encoding);
+    await this.fileService.writeFileContent(filePath, newFileContent, encoding);
     return this;
   }
 }

src/services/file/TomlFile.ts

@@ -42,7 +42,7 @@ export class TomlFile extends StdFile {
       return parse(content);
     } catch (error) {
       throw new Error(
-        `An error occurred while parsing file content "${this.file}": ${JSON.stringify(
+        `An error occurred while parsing file content "${this.filePath}": ${JSON.stringify(
           error instanceof Error ? error.message : error
         )} => "${content.trim()}"`
       );

src/services/file/TypescriptFile.ts

@@ -72,7 +72,11 @@ export class TypescriptFile extends StdFile {
   }
 
   protected parseTypescriptContent(content: string): NodeArray<Statement> {
-    const sourceFile = createSourceFile(this.file ?? "tmp-file.ts", content, ScriptTarget.ES2020);
+    const sourceFile = createSourceFile(
+      this.filePath ?? "tmp-file.ts",
+      content,
+      ScriptTarget.ES2020
+    );
     return sourceFile.statements;
   }