From 084f3badbb6736d091e2b40c8722b6d4dc056d73 Mon Sep 17 00:00:00 2001 From: Peter Cardenas <16930781+PeterCardenas@users.noreply.github.com> Date: Sat, 8 Nov 2025 19:21:11 -0800 Subject: [PATCH 1/4] feat: support nested directory for tmpfile_format --- lua/conform/runner.lua | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lua/conform/runner.lua b/lua/conform/runner.lua index 4fc2309a..ae49bdd6 100644 --- a/lua/conform/runner.lua +++ b/lua/conform/runner.lua @@ -370,6 +370,36 @@ 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) + -- ctx.filename can contain directories relative to ctx.dirname + -- Thus, we check which directories relative to ctx.dirname exist and remove + -- the first one that does not exist. + local res = vim + .iter( + vim.split( + assert( + vim.fs.relpath(ctx.dirname, ctx.filename), + "filename " .. ctx.filename .. " is not a child of dirname " .. ctx.dirname + ), + "/", + { plain = true } + ) + ) + :rskip(1) + :fold( + { path = ctx.dirname, found_not_exists = false }, + ---@param path_part string + function(acc, path_part) + local new_path = vim.fs.joinpath(acc.path, path_part) + if not acc.found_not_exists then + acc.path = new_path + end + acc.found_not_exists = vim.fn.isdirectory(new_path) == 0 + return acc + end + ) + local dir_to_remove = res.path + local can_remove = res.found_not_exists + vim.fn.mkdir(vim.fs.dirname(ctx.filename), "p") local fd = assert(uv.fs_open(ctx.filename, "w", 448)) -- 0700 uv.fs_write(fd, buffer_text) @@ -377,6 +407,11 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, opts, c callback = util.wrap_callback(callback, function() log.debug("Cleaning up temp file %s", ctx.filename) uv.fs_unlink(ctx.filename) + -- If all directories relative to ctx.dirname exist, there are no directories to remove. + if can_remove then + log.debug("Cleaning up temp dir %s", dir_to_remove) + vim.fs.rm(dir_to_remove, { recursive = true }) + end end) end From 56633658a4d768097a917a8c55a57d2f17bf22be Mon Sep 17 00:00:00 2001 From: Peter Cardenas <16930781+PeterCardenas@users.noreply.github.com> Date: Mon, 1 Dec 2025 03:25:56 -0800 Subject: [PATCH 2/4] fix: handle race conditions when removing temporary directories --- lua/conform/runner.lua | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/lua/conform/runner.lua b/lua/conform/runner.lua index ae49bdd6..fa0288af 100644 --- a/lua/conform/runner.lua +++ b/lua/conform/runner.lua @@ -370,35 +370,6 @@ 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) - -- ctx.filename can contain directories relative to ctx.dirname - -- Thus, we check which directories relative to ctx.dirname exist and remove - -- the first one that does not exist. - local res = vim - .iter( - vim.split( - assert( - vim.fs.relpath(ctx.dirname, ctx.filename), - "filename " .. ctx.filename .. " is not a child of dirname " .. ctx.dirname - ), - "/", - { plain = true } - ) - ) - :rskip(1) - :fold( - { path = ctx.dirname, found_not_exists = false }, - ---@param path_part string - function(acc, path_part) - local new_path = vim.fs.joinpath(acc.path, path_part) - if not acc.found_not_exists then - acc.path = new_path - end - acc.found_not_exists = vim.fn.isdirectory(new_path) == 0 - return acc - end - ) - local dir_to_remove = res.path - local can_remove = res.found_not_exists vim.fn.mkdir(vim.fs.dirname(ctx.filename), "p") local fd = assert(uv.fs_open(ctx.filename, "w", 448)) -- 0700 @@ -407,10 +378,15 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, opts, c callback = util.wrap_callback(callback, function() log.debug("Cleaning up temp file %s", ctx.filename) uv.fs_unlink(ctx.filename) - -- If all directories relative to ctx.dirname exist, there are no directories to remove. - if can_remove then - log.debug("Cleaning up temp dir %s", dir_to_remove) - vim.fs.rm(dir_to_remove, { recursive = true }) + local current = vim.fs.dirname(ctx.filename) + -- Remove empty directories from filename to dirname + while current and vim.startswith(current, ctx.dirname) and current ~= ctx.dirname do + log.debug("Cleaning up temp dir %s", current) + local success, err_name, err_msg = uv.fs_rmdir(current) + if not success then + log.trace("Failed to remove directory %s: %s: %s", current, err_name, err_msg) + end + current = vim.fs.dirname(current) end end) end From 08832e135091d4adaafa784b17bd6aca7aad1c6d Mon Sep 17 00:00:00 2001 From: Peter Cardenas <16930781+PeterCardenas@users.noreply.github.com> Date: Tue, 2 Dec 2025 10:00:58 -0800 Subject: [PATCH 3/4] fix: only delete temporary directories created --- lua/conform/runner.lua | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/lua/conform/runner.lua b/lua/conform/runner.lua index fa0288af..2ce54dc4 100644 --- a/lua/conform/runner.lua +++ b/lua/conform/runner.lua @@ -293,6 +293,10 @@ end ---@type table 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 @@ -370,6 +374,16 @@ 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") local fd = assert(uv.fs_open(ctx.filename, "w", 448)) -- 0700 @@ -378,15 +392,22 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, opts, c callback = util.wrap_callback(callback, function() log.debug("Cleaning up temp file %s", ctx.filename) uv.fs_unlink(ctx.filename) - local current = vim.fs.dirname(ctx.filename) - -- Remove empty directories from filename to dirname - while current and vim.startswith(current, ctx.dirname) and current ~= ctx.dirname do - log.debug("Cleaning up temp dir %s", current) - local success, err_name, err_msg = uv.fs_rmdir(current) + 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.trace("Failed to remove directory %s: %s: %s", current, err_name, err_msg) + 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 - current = vim.fs.dirname(current) end end) end From e1f815a0d70d66b63860077e2c4f0e9a30b0db38 Mon Sep 17 00:00:00 2001 From: Steven Arcangeli Date: Sun, 21 Dec 2025 14:31:06 -0500 Subject: [PATCH 4/4] refactor: extract logic into file so it can be tested --- lua/conform/dir_manager.lua | 42 ++++++++++++++++++++ lua/conform/runner.lua | 36 ++--------------- tests/dir_manager_spec.lua | 79 +++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 33 deletions(-) create mode 100644 lua/conform/dir_manager.lua create mode 100644 tests/dir_manager_spec.lua diff --git a/lua/conform/dir_manager.lua b/lua/conform/dir_manager.lua new file mode 100644 index 00000000..3393ca5a --- /dev/null +++ b/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 diff --git a/lua/conform/runner.lua b/lua/conform/runner.lua index 2ce54dc4..7a374f17 100644 --- a/lua/conform/runner.lua +++ b/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 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 diff --git a/tests/dir_manager_spec.lua b/tests/dir_manager_spec.lua new file mode 100644 index 00000000..ff27b218 --- /dev/null +++ b/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_XXXXXX") + 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_XXXXXX") + 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_XXXXXX") + 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)