fix handling of noNewlineAtEndOfFile

Author: ds300

.vscode/launch.json

@@ -0,0 +1,18 @@
+{
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "name": "Debug Jest Tests",
+      "type": "node",
+      "request": "launch",
+      "runtimeArgs": [
+        "--inspect-brk",
+        "${workspaceRoot}/node_modules/.bin/jest",
+        "--runInBand"
+      ],
+      "console": "integratedTerminal",
+      "internalConsoleOptions": "neverOpen",
+      "port": 9229
+    }
+  ]
+}

.vscode/settings.json

@@ -13,5 +13,6 @@
   "prettier.trailingComma": "all",
   "editor.formatOnSave": true,
   "tslint.enable": true,
-  "typescript.tsdk": "node_modules/typescript/lib"
+  "typescript.tsdk": "node_modules/typescript/lib",
+  "debug.node.autoAttach": "on"
 }

property-based-tests/executeTestCase.ts

@@ -7,6 +7,7 @@ import { executeEffects } from "../src/patch/apply"
 import { parsePatch } from "../src/patch/parse"
 
 import { TestCase, Files } from "./testCases"
+import { appendFileSync, existsSync, writeFileSync } from "fs"
 
 jest.mock("fs-extra", () => {
   let workingFiles: Files
@@ -67,13 +68,12 @@ export function executeTestCase(testCase: TestCase) {
     try {
       f()
     } catch (e) {
-      console.error(
-        "TEST CASE FAILED",
-        JSON.stringify({
-          testCase,
-          workingFiles: fs.getWorkingFiles(),
-        }),
-      )
+      const data = JSON.stringify(testCase) + "\n\n"
+      if (!existsSync("generative-test-errors.log")) {
+        writeFileSync("generative-test-errors.log", data)
+      } else {
+        appendFileSync("generative-test-errors.log", data)
+      }
       throw e
     }
   }

property-based-tests/regressionTests.test.ts

@@ -155,4 +155,21 @@ describe("regression tests", () => {
       modifiedFiles: { "1dkfI.J": { contents: "b\nb\nc", mode: 420 } },
     })
   })
+
+  describe("16", () => {
+    executeTestCase({
+      cleanFiles: {
+        "k/1dt4myqe.e1": {
+          contents: "a\n\n\n\nbanana\n",
+          mode: 420,
+        },
+      },
+      modifiedFiles: {
+        "k/1dt4myqe.e1": {
+          contents: "b\n\n\n\nbanana\n",
+          mode: 420,
+        },
+      },
+    })
+  })
 })

property-based-tests/testCases.ts

@@ -122,7 +122,12 @@ function mutateFiles(files: Files): Files {
         break
       }
       case "createFile": {
-        mutatedFiles[makeFileName()] = makeFileContents()
+        // TODO make sure there isn't a dir with that filename already
+        let filename = makeFileName()
+        while (Object.keys(mutatedFiles).some(k => k.startsWith(filename))) {
+          filename = makeFileName()
+        }
+        mutatedFiles[filename] = makeFileContents()
         break
       }
       case "deleteLine": {

src/patch/apply.ts

@@ -26,7 +26,6 @@ export type Effect =
       type: "replace"
       path: string
       lines: string[]
-      noNewlineAtEndOfFile: boolean
     }
 
 export const executeEffects = (effects: Effect[]) => {
@@ -47,10 +46,7 @@ export const executeEffects = (effects: Effect[]) => {
         )
         break
       case "replace":
-        fs.writeFileSync(
-          eff.path,
-          eff.lines.join("\n") + (eff.noNewlineAtEndOfFile ? "" : "\n"),
-        )
+        fs.writeFileSync(eff.path, eff.lines.join("\n"))
         break
     }
   })
@@ -70,16 +66,33 @@ function assertLineEquality(onDisk: string, expected: string) {
   }
 }
 
+/**
+ * How does noNewLineAtEndOfFile work?
+ *
+ * if you remove the newline from a file that had one without editing other bits:
+ *
+ *    it creates an insertion/removal pair where the insertion has \ No new line at end of file
+ *
+ * if you edit a file that didn't have a new line and don't add one:
+ *
+ *    both insertion and deletion have \ No new line at end of file
+ *
+ * if you edit a file that didn't have a new line and add one:
+ *
+ *    deletion has \ No new line at end of file
+ *    but not insertion
+ *
+ * if you edit a file that had a new line and leave it in:
+ *
+ *    neither insetion nor deletion have the annoation
+ *
+ */
+
 function applyPatch({ parts, path }: FilePatch): Effect {
   // modifying the file in place
   const fileContents = fs.readFileSync(path).toString()
 
   const fileLines: string[] = fileContents.split(/\n/)
-  if (fileLines[fileLines.length - 1] === "") {
-    fileLines.pop()
-  }
-
-  let noNewlineAtEndOfFile = true
 
   // when adding or removing lines from a file, gotta
   // make sure that the original lines in hunk headers match up
@@ -117,30 +130,19 @@ function applyPatch({ parts, path }: FilePatch): Effect {
             )
             contextIndex -= part.lines.length
 
-            if (contextIndex >= fileLines.length) {
-              if (
-                (hunkHeader.patched.length > 0 && part.noNewlineAtEndOfFile) ||
-                (parts[i - 2] &&
-                  (parts[i - 2].type === "insertion" ||
-                    parts[i - 2].type === "context") &&
-                  !(parts[i - 2] as any).noNewlineAtEndOfFile)
-              ) {
-                // delete the fact that there was no newline at the end of the file
-                // by adding a newline to the end of the file
-                noNewlineAtEndOfFile = false
-              }
-            }
-          } else {
-            if (contextIndex >= fileLines.length) {
-              noNewlineAtEndOfFile = part.noNewlineAtEndOfFile
+            if (part.noNewlineAtEndOfFile) {
+              fileLines.push("")
             }
           }
           break
         case "insertion":
           fileLines.splice(contextIndex, 0, ...part.lines)
           contextIndex += part.lines.length
-          if (contextIndex >= fileLines.length) {
-            noNewlineAtEndOfFile = part.noNewlineAtEndOfFile
+          if (part.noNewlineAtEndOfFile) {
+            if (contextIndex !== fileLines.length - 1) {
+              throw new Error("Invalid patch application state.")
+            }
+            fileLines.pop()
           }
           break
       }
@@ -149,7 +151,6 @@ function applyPatch({ parts, path }: FilePatch): Effect {
 
   return {
     type: "replace",
-    noNewlineAtEndOfFile,
     path,
     lines: fileLines,
   }

src/patch/parse.ts

@@ -31,31 +31,18 @@ export const parseHunkHeaderLine = (headerLine: string): HunkHeader => {
   }
 }
 
-interface Insertion {
-  type: "insertion"
+interface PatchMutationPart {
+  type: "context" | "insertion" | "deletion"
   lines: string[]
   noNewlineAtEndOfFile: boolean
 }
-
-interface Deletion {
-  type: "deletion"
-  lines: string[]
-  noNewlineAtEndOfFile: boolean
-}
-
-interface Context {
-  type: "context"
-  lines: string[]
-  noNewlineAtEndOfFile: boolean
-}
-
-type PatchMutationPart = Insertion | Deletion | Context
 export type PatchHunk = HunkHeader | PatchMutationPart
 
 interface FileRename {
   type: "rename"
   fromPath: string
   toPath: string
+  // TODO: check wheter renaming a file can have \ No newline at end of file
 }
 
 export interface FilePatch {
@@ -201,7 +188,7 @@ class PatchParser {
             type: "insertion",
             lines: [],
             noNewlineAtEndOfFile: true,
-          } as PatchMutationPart
+          }
         } else {
           throw new Error(`unexpected patch file comment ${this.currentLine}`)
         }
@@ -246,7 +233,7 @@ class PatchParser {
       type: blockType,
       lines,
       noNewlineAtEndOfFile,
-    } as PatchMutationPart
+    }
   }
 
   private currentLineIsPartOfHunk(): boolean {