refactor: extract logic into file so it can be tested

Author: stevearc

lua/conform/dir_manager.lua

@@ -0,0 +1,42 @@
+local log = require("conform.log")
+local uv = vim.uv or vim.loop
+
+local M = {}
+
+---Set of directories that have been created
+---@type string[]
+M._dirs = {}
+
+---Ensure that all parent directories of a path exist
+---@param path string
+M.ensure_parent = function(path)
+  local current_parent_dir = vim.fs.dirname(path)
+  -- Keep track of the current parent directories created, so we can delete them later
+  while current_parent_dir and not uv.fs_stat(current_parent_dir) do
+    table.insert(M._dirs, current_parent_dir)
+    current_parent_dir = vim.fs.dirname(current_parent_dir)
+  end
+  vim.fn.mkdir(vim.fs.dirname(path), "p")
+end
+
+---Clean up temporary directories
+M.cleanup = function()
+  -- Before cleanup we make sure to order the deepest paths first
+  table.sort(M._dirs, function(a, b)
+    return a:len() > b:len()
+  end)
+  local temp_dir_idx = 1
+  while temp_dir_idx <= #M._dirs do
+    local temp_dir_to_remove = M._dirs[temp_dir_idx]
+    log.trace("Cleaning up temp dir %s", temp_dir_to_remove)
+    local success, err_name, err_msg = uv.fs_rmdir(temp_dir_to_remove)
+    if not success then
+      log.warn("Failed to remove temp directory %s: %s: %s", temp_dir_to_remove, err_name, err_msg)
+      temp_dir_idx = temp_dir_idx + 1
+    else
+      table.remove(M._dirs, temp_dir_idx)
+    end
+  end
+end
+
+return M

lua/conform/runner.lua

@@ -1,3 +1,4 @@
+local dir_manager = require("conform.dir_manager")
 local errors = require("conform.errors")
 local fs = require("conform.fs")
 local ft_to_ext = require("conform.ft_to_ext")
@@ -293,10 +294,6 @@ end
 ---@type table<string, boolean>
 local last_run_errored = {}
 
----Set of temporary directories to remove after formatting
----@type string[]
-local temp_dirs = {}
-
 ---@param bufnr integer
 ---@param formatter conform.FormatterInfo
 ---@param config conform.FormatterConfig
@@ -374,41 +371,14 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, opts, c
 
   if not config.stdin then
     log.debug("Creating temp file %s", ctx.filename)
-    local current_parent_dir = vim.fs.dirname(ctx.filename)
-    -- Keep track of the current parent directories created, so we can delete them later
-    while
-      current_parent_dir
-      and current_parent_dir ~= ctx.dirname
-      and vim.fn.isdirectory(current_parent_dir) == 0
-    do
-      temp_dirs[#temp_dirs + 1] = current_parent_dir
-      current_parent_dir = vim.fs.dirname(current_parent_dir)
-    end
-
-    vim.fn.mkdir(vim.fs.dirname(ctx.filename), "p")
+    dir_manager.ensure_parent(ctx.filename)
     local fd = assert(uv.fs_open(ctx.filename, "w", 448)) -- 0700
     uv.fs_write(fd, buffer_text)
     uv.fs_close(fd)
     callback = util.wrap_callback(callback, function()
       log.debug("Cleaning up temp file %s", ctx.filename)
       uv.fs_unlink(ctx.filename)
-      local temp_dir_idx = 1
-      while temp_dir_idx <= #temp_dirs do
-        local temp_dir_to_remove = temp_dirs[temp_dir_idx]
-        log.debug("Cleaning up temp dir %s", temp_dir_to_remove)
-        local success, err_name, err_msg = uv.fs_rmdir(temp_dir_to_remove)
-        if not success then
-          log.debug(
-            "Failed to remove temp directory %s: %s: %s",
-            temp_dir_to_remove,
-            err_name,
-            err_msg
-          )
-          temp_dir_idx = temp_dir_idx + 1
-        else
-          table.remove(temp_dirs, temp_dir_idx)
-        end
-      end
+      dir_manager.cleanup()
     end)
   end
 

tests/dir_manager_spec.lua

@@ -0,0 +1,79 @@
+local dir_manager = require("conform.dir_manager")
+local test_util = require("tests.test_util")
+
+local function touch(path)
+  local fd = assert(vim.uv.fs_open(path, "w", 448))
+  vim.uv.fs_write(fd, "")
+  vim.uv.fs_close(fd)
+end
+
+describe("dir_manager", function()
+  after_each(function()
+    test_util.reset_editor()
+  end)
+
+  it("creates and cleans up nested created directories", function()
+    local root, err = vim.uv.fs_mkdtemp("conform_XXXXX")
+    assert(root, err)
+    dir_manager.ensure_parent(root .. "/foo/bar/baz.txt")
+    assert(vim.uv.fs_stat(root .. "/foo"))
+    assert(vim.uv.fs_stat(root .. "/foo/bar"))
+    assert(not vim.uv.fs_stat(root .. "/foo/bar/baz.txt"))
+    dir_manager.cleanup()
+    assert(not vim.uv.fs_stat(root .. "/foo/bar"))
+    assert(not vim.uv.fs_stat(root .. "/foo"))
+    assert(vim.uv.fs_stat(root))
+    assert(vim.uv.fs_rmdir(root))
+  end)
+
+  it("handles race condition for two concurrent processes", function()
+    local root, err = vim.uv.fs_mkdtemp("conform_XXXXX")
+    assert(root, err)
+    dir_manager.ensure_parent(root .. "/foo/bar/baz.txt")
+    touch(root .. "/foo/bar/baz.txt")
+    assert(vim.uv.fs_stat(root .. "/foo"))
+    assert(vim.uv.fs_stat(root .. "/foo/bar"))
+    assert(vim.uv.fs_stat(root .. "/foo/bar/baz.txt"))
+
+    -- This cleanup will fail because baz.txt exists
+    dir_manager.cleanup()
+    assert(vim.uv.fs_stat(root .. "/foo/bar/baz.txt"))
+
+    assert(vim.uv.fs_unlink(root .. "/foo/bar/baz.txt"))
+    -- This cleanup should succeed
+    dir_manager.cleanup()
+    assert(not vim.uv.fs_stat(root .. "/foo/bar"))
+    assert(not vim.uv.fs_stat(root .. "/foo"))
+
+    assert(vim.uv.fs_stat(root))
+    assert(vim.uv.fs_rmdir(root))
+  end)
+
+  it("handles race condition for semi-matched nested paths", function()
+    local root, err = vim.uv.fs_mkdtemp("conform_XXXXX")
+    assert(root, err)
+    dir_manager.ensure_parent(root .. "/foo/bar/baz.txt")
+    dir_manager.ensure_parent(root .. "/foo/qux/foo.txt")
+    touch(root .. "/foo/qux/foo.txt")
+    assert(vim.uv.fs_stat(root .. "/foo"))
+    assert(vim.uv.fs_stat(root .. "/foo/bar"))
+    assert(vim.uv.fs_stat(root .. "/foo/qux"))
+    assert(vim.uv.fs_stat(root .. "/foo/qux/foo.txt"))
+
+    -- This cleanup will partially succeed because foo.txt exists
+    dir_manager.cleanup()
+    assert(vim.uv.fs_stat(root .. "/foo"))
+    assert(not vim.uv.fs_stat(root .. "/bar"))
+    assert(vim.uv.fs_stat(root .. "/foo/qux"))
+    assert(vim.uv.fs_stat(root .. "/foo/qux/foo.txt"))
+
+    assert(vim.uv.fs_unlink(root .. "/foo/qux/foo.txt"))
+    -- This cleanup should succeed
+    dir_manager.cleanup()
+    assert(not vim.uv.fs_stat(root .. "/foo/qux"))
+    assert(not vim.uv.fs_stat(root .. "/foo"))
+
+    assert(vim.uv.fs_stat(root))
+    assert(vim.uv.fs_rmdir(root))
+  end)
+end)