Improve generateDiff

The `generateDiff` function now has a comprehensive test suite. It has
been updated to ensure that the resulting diff displays a notice about
whether or not the file has a trailing newline.

I started this journey by trying to ensure we were trimming each line
properly. I found that `diff.diffLines` always returned change sets
that ended in a newline, except when returning the very last change to
a file that didn't terminate in a newline. So in order to ensure we
were correctly showing all changes, I needed to add special handling
for end-of-file changes. We could display this differently than I've
chosen to here, but I figured showing what `git` shows was a good
start.

The `generateDiff` function now also ensures the correct context is
shown in between each change.

The dependency `outdent` was added to help with writing inline test
fixtures. This saves us from having to come up with a name for each
fixture and cross-reference them in each test.

When adding that dependency, an ESLint error brought to my attention
that we were accidentally including test files in our `files` property
in `package.json`. It was updated to exclude test files, which resolved
the ESLint error.

package.json

@@ -12,7 +12,8 @@
     "node": ">=12.0.0"
   },
   "files": [
-    "src/"
+    "src/*.js",
+    "!src/*.test.js"
   ],
   "repository": {
     "type": "git",
@@ -45,6 +46,7 @@
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.3.1",
     "jest": "^26.4.2",
+    "outdent": "^0.8.0",
     "prettier": "^2.2.1"
   }
 }

src/generateDiff.js

@@ -1,8 +1,15 @@
 const diff = require('diff');
 
+function getTrimmedLines(value) {
+  const trimmedValue = value.endsWith('\n')
+    ? value.substring(0, value.length - 1)
+    : value;
+  return trimmedValue.split('\n');
+}
+
 /**
- * Generates a diff between two multi-line strings. The resulting diff shows
- * any changes using '-' and '+' to indicate the "old" and "new" version
+ * Generates a diff between two multi-line string files. The resulting diff
+ * shows any changes using '-' and '+' to indicate the "old" and "new" version
  * respectively, and includes 2 lines of unchanged content around each changed
  * section where possible.
  * @param {string} before - The string representing the base for the comparison.
@@ -11,30 +18,64 @@ const diff = require('diff');
  */
 function generateDiff(before, after) {
   const changes = diff.diffLines(before, after);
-  const diffLines = [];
-  const preceedingContext = [];
-  for (const { added, removed, value } of changes) {
-    const lines = value.split('\n');
-    // remove trailing newline
-    lines.pop();
-    if (added || removed) {
-      if (preceedingContext.length) {
-        diffLines.push(...preceedingContext);
-        preceedingContext.splice(0, preceedingContext.length);
-      }
-      diffLines.push(...lines.map((line) => `${added ? '+' : '-'}${line}`));
-    } else {
-      // If a changed line has been included already, add up to 2 lines of context
-      if (diffLines.length) {
-        diffLines.push(...lines.slice(0, 2).map((line) => ` ${line}`));
-        lines.splice(0, 2);
+  // `diffLines` will always return at least one change
+  const lastChange = changes[changes.length - 1];
+  const penultimateChange = changes[changes.length - 2] || {};
+
+  // Add notice about newline at end of file
+  if (!lastChange.value.endsWith('\n')) {
+    lastChange.noNewline = true;
+  }
+  // If the last change is an addition and the penultimate change is a
+  // removal, then the last line of the file is also in the penultimate change.
+  // That's why we're checking to see if the newline notice is needed here as
+  // well.
+  if (
+    lastChange.added &&
+    penultimateChange.removed &&
+    !penultimateChange.value.endsWith('\n')
+  ) {
+    penultimateChange.noNewline = true;
+  }
+
+  const diffLines = changes.flatMap(
+    ({ added, noNewline, removed, value }, index) => {
+      const lines = getTrimmedLines(value);
+      const changedLines = [];
+      if (added || removed) {
+        // Add up to 2 lines of context before each change
+        const previousContext = changes[index - 1];
+        if (
+          previousContext &&
+          !previousContext.added &&
+          !previousContext.removed
+        ) {
+          const hasPreviousChange = index > 1;
+          const previousContextLines = getTrimmedLines(previousContext.value);
+          // Avoid repeating context that has already been included in diff
+          if (!hasPreviousChange || previousContextLines.length >= 3) {
+            const linesOfContext =
+              hasPreviousChange && previousContextLines.length === 3 ? 1 : 2;
+            const previousTwoLines = previousContextLines
+              .slice(-1 * linesOfContext)
+              .map((line) => ` ${line}`);
+            changedLines.push(...previousTwoLines);
+          }
+        }
+        changedLines.push(
+          ...lines.map((line) => `${added ? '+' : '-'}${line}`),
+        );
+      } else if (index > 0) {
+        // Add up to 2 lines of context following a change
+        const nextTwoLines = lines.slice(0, 2).map((line) => ` ${line}`);
+        changedLines.push(...nextTwoLines);
       }
-      // stash last 2 lines for context in case there is another change
-      if (lines.length) {
-        preceedingContext.push(...lines.slice(-2));
+      if (noNewline) {
+        changedLines.push('\\ No newline at end of file');
       }
-    }
-  }
+      return changedLines;
+    },
+  );
   return diffLines.join('\n');
 }