From 76bec2279a0d1db0d90e06810e9e80bd0b49f84c Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 16 Aug 2026 18:20:41 +0000 Subject: [PATCH 1/3] Cover the generated directory being created by buildFileForContract Take the directory as a parameter on an overload of buildFileForContract so a test can drive the create against a directory it owns, and make the existing four argument function that overload applied to GENERATED_DIR. Closes #64 Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibFs.sol | 53 ++++++++++++++++++++--- test/lib/LibFs.buildFileForContract.t.sol | 28 ++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/lib/LibFs.sol b/src/lib/LibFs.sol index 9ad096f..1756845 100644 --- a/src/lib/LibFs.sol +++ b/src/lib/LibFs.sol @@ -31,8 +31,23 @@ library LibFs { /// @param contractName The name of the contract, interpolated verbatim. /// @return The file path as a string. function pathForContract(string memory contractName) internal pure returns (string memory) { + return pathForContractIn(GENERATED_DIR, contractName); + } + + /// @notice Constructs the file path for a contract's generated file inside + /// `dir`. + /// + /// Reverts unless `contractName` is a Solidity identifier, so the returned + /// path is a direct child of `dir` for every name that is accepted at all. + /// `dir` is interpolated verbatim and is not checked, so where `dir` itself + /// sits is entirely the caller's, and only `fs_permissions` confines it. + /// @param dir The directory to put the file in, without a trailing + /// separator, interpolated verbatim. + /// @param contractName The name of the contract, interpolated verbatim. + /// @return The file path as a string. + function pathForContractIn(string memory dir, string memory contractName) private pure returns (string memory) { LibCodeGen.requireContractName(contractName); - return string.concat(GENERATED_DIR, "/", contractName, ".sol"); + return string.concat(dir, "/", contractName, ".sol"); } /// @notice Builds a file for a generated contract at @@ -43,8 +58,9 @@ library LibFs { /// of `GENERATED_DIR` and a rejected name reverts before any cheatcode is /// reached. /// - /// `GENERATED_DIR` is created if it does not exist, so the first generation - /// in a repo does not need it committed already. + /// `GENERATED_DIR` is created if it does not exist, along with any missing + /// parent of it, so the first generation in a repo does not need it + /// committed already. /// /// Anything already at the path is unlinked before the write, so a symlink /// there is replaced by a regular file rather than written through to its @@ -62,9 +78,36 @@ library LibFs { /// @param contractName The name of the contract. /// @param body The body of the contract file to be written. function buildFileForContract(Vm vm, address instance, string memory contractName, string memory body) internal { - string memory path = pathForContract(contractName); + buildFileForContract(vm, instance, GENERATED_DIR, contractName, body); + } + + /// @notice Builds a file for a generated contract inside `dir` rather than + /// inside `GENERATED_DIR`. + /// + /// Identical to `buildFileForContract` in every other respect, and that + /// function is this one applied to `GENERATED_DIR`: `dir` is what gets + /// created when it is missing, and what the file is written a direct child + /// of. `dir` is not checked, so a caller passing something other than a + /// directory it means to own gets whatever `fs_permissions` allows; + /// `contractName` is still required to be a Solidity identifier, so the + /// name can never carry the file out of `dir`. + /// @param vm The Vm instance for file operations. + /// @param instance The contract instance whose bytecode hash is to be + /// included. + /// @param dir The directory to put the file in, without a trailing + /// separator, interpolated verbatim. + /// @param contractName The name of the contract. + /// @param body The body of the contract file to be written. + function buildFileForContract( + Vm vm, + address instance, + string memory dir, + string memory contractName, + string memory body + ) internal { + string memory path = pathForContractIn(dir, contractName); //forge-lint: disable-next-line(unsafe-cheatcode) - vm.createDir(GENERATED_DIR, true); + vm.createDir(dir, true); if (vm.exists(path)) { //forge-lint: disable-next-line(unsafe-cheatcode) vm.removeFile(path); diff --git a/test/lib/LibFs.buildFileForContract.t.sol b/test/lib/LibFs.buildFileForContract.t.sol index a5ed2af..e61055a 100644 --- a/test/lib/LibFs.buildFileForContract.t.sol +++ b/test/lib/LibFs.buildFileForContract.t.sol @@ -257,6 +257,34 @@ contract LibFsBuildFileForContractTest is Test { } } + /// The directory is created when it is not there, so a repo generating for + /// the first time does not have to commit it. Two levels of it are missing, + /// so a create that did not also make the missing parent fails here. + /// + /// Driven through a directory this test owns rather than through + /// `src/generated` itself. The tests in this file run in parallel and every + /// one of them writes under `src/generated`, so removing it to make it + /// missing races all of them, and takes the committed + /// `src/generated/CodeGennable.sol` with it for as long as it is gone. + function testBuildFileForContractCreatesTheDirectory() external { + string memory root = "src/generated/LibFsBuildCreatesDir"; + string memory dir = "src/generated/LibFsBuildCreatesDir/nested"; + string memory name = "LibFsBuildCreatesDir"; + cleanupPath(root); + assertFalse(vm.exists(root), "dirty precondition"); + address instance = address(new CodeGennable()); + string memory body = "\n// created\n"; + + LibFs.buildFileForContract(vm, instance, dir, name, body); + + assertTrue(vm.isDir(dir), "the directory was not created"); + assertEq( + vm.readFile("src/generated/LibFsBuildCreatesDir/nested/LibFsBuildCreatesDir.sol"), + expectedFile(instance, body) + ); + cleanupPath(root); + } + /// A name that is not a Solidity identifier gets no path from /// `pathForContract`, and `buildFileForContract` asks for the path before it /// reaches a cheatcode, so the refusal arrives before anything is written. From 45f6f1d69b691ac485c5b08cbb1b386329aa1000 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 16 Aug 2026 18:28:46 +0000 Subject: [PATCH 2/3] Drop the directory assertion the content read already implies The file cannot be read back at that path unless the directory it is in was created, so the assertion above the read does no work: removing it leaves the suite green. Co-Authored-By: Claude Opus 5 (1M context) --- test/lib/LibFs.buildFileForContract.t.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/LibFs.buildFileForContract.t.sol b/test/lib/LibFs.buildFileForContract.t.sol index e61055a..ba36920 100644 --- a/test/lib/LibFs.buildFileForContract.t.sol +++ b/test/lib/LibFs.buildFileForContract.t.sol @@ -277,7 +277,6 @@ contract LibFsBuildFileForContractTest is Test { LibFs.buildFileForContract(vm, instance, dir, name, body); - assertTrue(vm.isDir(dir), "the directory was not created"); assertEq( vm.readFile("src/generated/LibFsBuildCreatesDir/nested/LibFsBuildCreatesDir.sol"), expectedFile(instance, body) From f657750287425389697c1ccf053f80117fa08969 Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 17 Aug 2026 04:01:35 +0000 Subject: [PATCH 3/3] Cover the generated directory being created, by making the directory injectable The library's vm.createDir(GENERATED_DIR, true) had no test: commenting it out on main leaves 145/145 passing, twice, including after rm -rf src/generated. Both test/lib/LibFs.buildFileForContract.t.sol and test/src/lib/LibFs.isPresent.t.sol create GENERATED_DIR in their own setUp, which masks the library creating it. Deleting GENERATED_DIR to make it missing is not an option: every generating test writes under it and forge runs them in parallel, which is why those setUp calls exist in the first place. So the directory becomes injectable. A private pathForContractIn(dir, name) and an internal buildFileForContract(vm, instance, dir, name, body) overload; the existing four-argument function is that overload applied to GENERATED_DIR and is otherwise untouched. The new test owns src/generated/LibFsBuildCreatesDir/nested, two missing levels deep, so a create that did not also make the missing parent fails too. Mutant proven dead: with vm.createDir(dir, true) removed the new test fails with vm.writeFile: ... No such file or directory. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibFs.sol | 61 +++++++++++++++++++++-- test/lib/LibFs.buildFileForContract.t.sol | 25 ++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/lib/LibFs.sol b/src/lib/LibFs.sol index f95f19f..11bb30f 100644 --- a/src/lib/LibFs.sol +++ b/src/lib/LibFs.sol @@ -32,8 +32,27 @@ library LibFs { /// @param contractName The name of the contract, interpolated verbatim. /// @return The file path as a string. function pathForContract(string memory contractName) internal pure returns (string memory) { + return pathForContractIn(GENERATED_DIR, contractName); + } + + /// @notice Constructs the file path for a contract's generated file inside + /// `dir`. + /// @dev `pathForContract` is this function applied to `GENERATED_DIR`, so + /// everything stated there about the name holds here too: the path is a + /// direct child of `dir` for every name that is accepted at all, and an + /// accepted name is interpolated verbatim. + /// + /// `dir` is interpolated verbatim and is not checked, so where `dir` itself + /// sits is entirely the caller's, and only `fs_permissions` confines it. + /// That is why this is private rather than internal: the only directory a + /// consumer of this library writes to is `GENERATED_DIR`. + /// @param dir The directory to put the file in, without a trailing + /// separator, interpolated verbatim. + /// @param contractName The name of the contract, interpolated verbatim. + /// @return The file path as a string. + function pathForContractIn(string memory dir, string memory contractName) private pure returns (string memory) { LibCodeGen.requireContractName(contractName); - return string.concat(GENERATED_DIR, "/", contractName, ".sol"); + return string.concat(dir, "/", contractName, ".sol"); } /// @notice True if anything occupies `path`, including a symlink whose @@ -67,8 +86,9 @@ library LibFs { /// of `GENERATED_DIR` and a rejected name reverts before any cheatcode is /// reached. /// - /// `GENERATED_DIR` is created if it does not exist, so the first generation - /// in a repo does not need it committed already. + /// `GENERATED_DIR` is created if it does not exist, along with any missing + /// parent of it, so the first generation in a repo does not need it + /// committed already. /// /// Anything already at the path is unlinked before the write, so a symlink /// there is replaced by a regular file rather than written through to its @@ -87,9 +107,40 @@ library LibFs { /// @param contractName The name of the contract. /// @param body The body of the contract file to be written. function buildFileForContract(Vm vm, address instance, string memory contractName, string memory body) internal { - string memory path = pathForContract(contractName); + buildFileForContract(vm, instance, GENERATED_DIR, contractName, body); + } + + /// @notice Builds a file for a generated contract inside `dir` rather than + /// inside `GENERATED_DIR`. + /// @dev Identical to `buildFileForContract` in every other respect, and + /// that function is this one applied to `GENERATED_DIR`: `dir` is what gets + /// created when it is missing, and what the file is written a direct child + /// of. `dir` is interpolated verbatim and is not checked, so a caller + /// passing something other than a directory it means to own gets whatever + /// `fs_permissions` allows; `contractName` is still required to be a + /// Solidity identifier, so the name can never carry the file out of `dir`. + /// + /// This overload exists so that the directory creation is reachable from a + /// test without deleting `GENERATED_DIR`. Every test that generates a file + /// writes under `GENERATED_DIR`, and `forge` runs them in parallel, so + /// removing it to make it missing races all of them. + /// @param vm The Vm instance for file operations. + /// @param instance The contract instance whose bytecode hash is to be + /// included. + /// @param dir The directory to put the file in, without a trailing + /// separator, interpolated verbatim. + /// @param contractName The name of the contract. + /// @param body The body of the contract file to be written. + function buildFileForContract( + Vm vm, + address instance, + string memory dir, + string memory contractName, + string memory body + ) internal { + string memory path = pathForContractIn(dir, contractName); //forge-lint: disable-next-line(unsafe-cheatcode) - vm.createDir(GENERATED_DIR, true); + vm.createDir(dir, true); if (isPresent(vm, path)) { //forge-lint: disable-next-line(unsafe-cheatcode) vm.removeFile(path); diff --git a/test/lib/LibFs.buildFileForContract.t.sol b/test/lib/LibFs.buildFileForContract.t.sol index 79d6858..5855d8a 100644 --- a/test/lib/LibFs.buildFileForContract.t.sol +++ b/test/lib/LibFs.buildFileForContract.t.sol @@ -244,6 +244,31 @@ contract LibFsBuildFileForContractTest is Test { } } + /// The directory is created when it is not there, so a repo generating for + /// the first time does not have to commit it. Two levels of it are missing, + /// so a create that did not also make the missing parent fails here. + /// + /// Driven through a directory this test owns rather than through + /// `GENERATED_DIR` itself. Every generating test in this file writes under + /// `GENERATED_DIR` and `forge` runs them in parallel, so removing it to + /// make it missing races all of them. `setUp` above creates `GENERATED_DIR` + /// for exactly that reason, which is also what makes the library's own + /// create unreachable from any test that writes there directly. + function testBuildFileForContractCreatesTheDirectory() external { + string memory root = string.concat(GENERATED_DIR, "/LibFsBuildCreatesDir"); + string memory dir = string.concat(root, "/nested"); + string memory name = "LibFsBuildCreatesDir"; + cleanupPath(root); + assertFalse(vm.exists(root), "dirty precondition"); + address instance = address(new CodeGennable()); + string memory body = "\n// created\n"; + + LibFs.buildFileForContract(vm, instance, dir, name, body); + + assertEq(vm.readFile(string.concat(dir, "/", name, ".sol")), expectedFile(instance, body)); + cleanupPath(root); + } + /// A name that is not a Solidity identifier gets no path from /// `pathForContract`, and `buildFileForContract` asks for the path before it /// reaches a cheatcode, so the refusal arrives before anything is written.