fix: modify diff calculation to handle end-of-file newlines better (#35)

lua/conform/log.lua

@@ -83,6 +83,13 @@ local function initialize()
   end
 end
 
+---Override the file handler e.g. for tests
+---@param handler fun(line: string)
+function Log.set_handler(handler)
+  write = handler
+  initialized = true
+end
+
 function Log.log(level, msg, ...)
   if Log.level <= level then
     initialize()

lua/conform/runner.lua

@@ -150,7 +150,7 @@ local function create_text_edit(
   -- If we're inserting text, make sure the text includes a newline at the end.
   -- The one exception is if we're inserting at the end of the file, in which case the newline is
   -- implicit
-  if is_insert and start_line < #original_lines - 1 then
+  if is_insert and start_line < #original_lines then
     table.insert(replacement, "")
   end
   local new_text = table.concat(replacement, "\n")
@@ -180,25 +180,26 @@ M.apply_format = function(bufnr, original_lines, new_lines, range, only_apply_ra
     return
   end
   local bufname = vim.api.nvim_buf_get_name(bufnr)
-  -- If the formatter output didn't have a trailing newline, add one
-  if new_lines[#new_lines] ~= "" then
-    table.insert(new_lines, "")
-  end
-
-  -- Vim buffers end with an implicit newline, so append an empty line to stand in for that
-  if vim.bo[bufnr].eol then
-    table.insert(original_lines, "")
-  end
+  log.trace("Applying formatting to %s", bufname)
+  -- The vim.diff algorithm doesn't handle changes in newline-at-end-of-file well. The unified
+  -- result_type has some text to indicate that the eol changed, but the indices result_type has no
+  -- such indication. To work around this, we just add a trailing newline to the end of both the old
+  -- and the new text.
+  table.insert(original_lines, "")
+  table.insert(new_lines, "")
   local original_text = table.concat(original_lines, "\n")
   local new_text = table.concat(new_lines, "\n")
-  log.trace("Creating diff for %s", bufname)
+  table.remove(original_lines)
+  table.remove(new_lines)
+
+  log.trace("Comparing lines %s and %s", original_lines, new_lines)
   local indices = vim.diff(original_text, new_text, {
     result_type = "indices",
     algorithm = "histogram",
   })
   assert(indices)
+  log.trace("Diff indices %s", indices)
   local text_edits = {}
-  log.trace("Creating TextEdits for %s", bufname)
   for _, idx in ipairs(indices) do
     local orig_line_start, orig_line_count, new_line_start, new_line_count = unpack(idx)
     local is_insert = orig_line_count == 0
@@ -232,7 +233,7 @@ M.apply_format = function(bufnr, original_lines, new_lines, range, only_apply_ra
     end
   end
 
-  log.trace("Applying text edits for %s", bufname)
+  log.trace("Applying text edits: %s", text_edits)
   vim.lsp.util.apply_text_edits(text_edits, bufnr, "utf-8")
   log.trace("Done formatting %s", bufname)
 end
@@ -272,12 +273,14 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, callbac
   log.info("Run %s on %s", formatter.name, vim.api.nvim_buf_get_name(bufnr))
   local buffer_text
   -- If the buffer has a newline at the end, make sure we include that in the input to the formatter
-  if vim.bo[bufnr].eol then
+  local add_extra_newline = vim.bo[bufnr].eol
+  if add_extra_newline then
     table.insert(input_lines, "")
-    buffer_text = table.concat(input_lines, "\n")
+  end
+  log.trace("Input lines: %s", input_lines)
+  buffer_text = table.concat(input_lines, "\n")
+  if add_extra_newline then
     table.remove(input_lines)
-  else
-    buffer_text = table.concat(input_lines, "\n")
   end
 
   if not config.stdin then
@@ -317,18 +320,27 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, callbac
       stderr = data
     end,
     on_exit = function(_, code)
-      local output
-      if not config.stdin then
-        local fd = assert(uv.fs_open(ctx.filename, "r", 448)) -- 0700
-        local stat = assert(uv.fs_fstat(fd))
-        local content = assert(uv.fs_read(fd, stat.size))
-        uv.fs_close(fd)
-        output = vim.split(content, "\n", { plain = true })
-      else
-        output = stdout
-      end
       if vim.tbl_contains(exit_codes, code) then
+        local output
+        if not config.stdin then
+          local fd = assert(uv.fs_open(ctx.filename, "r", 448)) -- 0700
+          local stat = assert(uv.fs_fstat(fd))
+          local content = assert(uv.fs_read(fd, stat.size))
+          uv.fs_close(fd)
+          output = vim.split(content, "\n", { plain = true })
+        else
+          output = stdout
+        end
+        -- Remove the trailing newline from the output to convert back to vim lines representation
+        if add_extra_newline and output[#output] == "" then
+          table.remove(output)
+        end
+        -- Vim will never let the lines array be empty. An empty file will still look like { "" }
+        if #output == 0 then
+          table.insert(output, "")
+        end
         log.debug("%s exited with code %d", formatter.name, code)
+        log.trace("Output lines: %s", output)
         callback(nil, output)
       else
         log.info("%s exited with code %d", formatter.name, code)

tests/fuzzer_spec.lua

@@ -1,6 +1,7 @@
 require("plenary.async").tests.add_to_env()
 local test_util = require("tests.test_util")
 local conform = require("conform")
+local log = require("conform.log")
 local runner = require("conform.runner")
 
 describe("fuzzer", function()
@@ -25,12 +26,6 @@ describe("fuzzer", function()
     vim.api.nvim_buf_set_lines(bufnr, 0, -1, true, buf_content)
     vim.bo[bufnr].modified = false
     runner.apply_format(0, buf_content, expected, nil, false)
-    -- We expect the last newline to be effectively "swallowed" by the formatter
-    -- because vim will use that as the EOL at the end of the file. The exception is that we always
-    -- expect at least one line in the output
-    if #expected > 1 and expected[#expected] == "" then
-      table.remove(expected)
-    end
     assert.are.same(expected, vim.api.nvim_buf_get_lines(0, 0, -1, false))
   end
 
@@ -95,6 +90,10 @@ describe("fuzzer", function()
     for _ = 1, num_lines do
       table.remove(lines, idx)
     end
+    -- vim will never let the lines be empty. An empty file has a single blank line.
+    if #lines == 0 then
+      table.insert(lines, "")
+    end
   end
 
   local function make_edits(lines)
@@ -112,8 +111,10 @@ describe("fuzzer", function()
   end
 
   it("formats correctly", function()
+    -- log.level = vim.log.levels.TRACE
     for i = 1, 50000 do
       math.randomseed(i)
+      log.info("Fuzz testing with seed %d", i)
       local content = make_file(20)
       local formatted = make_edits(content)
       run_formatter(content, formatted)

tests/runner_spec.lua

@@ -125,12 +125,6 @@ describe("runner", function()
       local expected_lines = vim.split(expected, "\n", { plain = true })
       test_util.set_formatter_output(expected_lines)
       conform.format(vim.tbl_extend("force", opts or {}, { formatters = { "test" }, quiet = true }))
-      -- We expect the last newline to be effectively "swallowed" by the formatter
-      -- because vim will use that as the EOL at the end of the file. The exception is that we always
-      -- expect at least one line in the output
-      if #expected_lines > 1 and expected_lines[#expected_lines] == "" then
-        table.remove(expected_lines)
-      end
       return expected_lines
     end
 
@@ -193,11 +187,14 @@ print("a")
       run_formatter_test("\nfoo", "\nhello\nfoo")
       run_formatter_test("hello", "hello\n")
       run_formatter_test("hello", "hello\n\n")
-      run_formatter_test("hello", "hello\n")
-      -- This should generate no changes to the buffer
-      assert.falsy(vim.bo.modified)
       run_formatter_test("hello\n", "hello")
       run_formatter_test("hello\n ", "hello")
+
+      -- These should generate no changes to the buffer
+      run_formatter_test("hello\n", "hello\n")
+      assert.falsy(vim.bo.modified)
+      run_formatter_test("hello", "hello")
+      assert.falsy(vim.bo.modified)
     end)
 
     it("does not change output if formatter fails", function()

tests/test_util.lua

@@ -1,5 +1,6 @@
 require("plenary.async").tests.add_to_env()
 local conform = require("conform")
+local log = require("conform.log")
 local M = {}
 
 local OUTPUT_FILE = "tests/fake_formatter_output"
@@ -21,13 +22,17 @@ M.reset_editor = function()
   if vim.fn.filereadable(OUTPUT_FILE) == 1 then
     vim.fn.delete(OUTPUT_FILE)
   end
+  log.level = vim.log.levels.ERROR
+  log.set_handler(print)
 end
 
 ---@param lines string[]
 M.set_formatter_output = function(lines)
   local content = table.concat(lines, "\n")
   local fd = assert(vim.loop.fs_open(OUTPUT_FILE, "w", 420)) -- 0644
   vim.loop.fs_write(fd, content)
+  -- Make sure we add the final newline
+  vim.loop.fs_write(fd, "\n")
   vim.loop.fs_close(fd)
 end