diff --git a/README.md b/README.md index 877dea1..f402e29 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,28 @@ read a tree that never stops changing as a cycle that does not converge rather than one more pass to run. Committing part-way records a deployed-bytecode hash for a contract compiled against a different pass of the same file. +## Generated paths + +`LibFs.pathForContract` names `src/generated/.sol`, and +`buildFileForContract` writes that file and only that file. Consumers commit it +and import it by path from `src/**`, so the path is a cross repo contract: +moving it, or moving `GENERATED_DIR`, breaks every repo holding the artifact at +the old path. + +`buildFileForContract` therefore refuses to generate while `src/generated/` +holds another artifact for the same contract — any direct child named for that +contract, in full, followed by a `.` and anything other than `sol`. Nothing +regenerates such a file, while the consumer's imports keep resolving to it, so +the build fails with `OrphanedGeneratedArtifact` naming the file rather than +generating beside it. Delete it and repoint the imports at +`src/generated/.sol` in the same commit. + +Only direct children are read, so a generation into `src/generated/` never reads +inside the per release snapshot directories that sit there, and never refuses +one of them either, because a tag carries no `.`. `buildFileForTaggedContract` +writes into one of those directories, and reads that directory rather than +`src/generated/`, so each is checked against its own contents. + ## Formatter requirements `LibCodeGen` wraps the declarations it emits itself, deciding against diff --git a/src/lib/LibFs.sol b/src/lib/LibFs.sol index 8c83984..13b2048 100644 --- a/src/lib/LibFs.sol +++ b/src/lib/LibFs.sol @@ -10,6 +10,14 @@ import {LibCodeGen} from "./LibCodeGen.sol"; /// path, so it is a cross repo contract rather than an internal detail. string constant GENERATED_DIR = "src/generated"; +/// Thrown when the generated directory holds an artifact for a contract other +/// than the one `pathForContract` names for it. Consumers commit these files +/// and import them by path, so an artifact this library does not write is one +/// nothing regenerates: its imports keep resolving to a frozen codehash while +/// the build reports success. +/// @param path The artifact nothing regenerates, relative to the project root. +error OrphanedGeneratedArtifact(string path); + /// Thrown when a tag is not a single path segment drawn from the tag alphabet. /// Such a tag cannot be interpolated into a directory path. /// @param tag The rejected tag. @@ -106,6 +114,97 @@ library LibFs { return string.concat(dir, "/", contractName, ".sol"); } + /// The final segment of `path`: the bytes after its last `/`, or all of + /// `path` when it has none. `vm.readDir` reports absolute paths, so this is + /// what carries an entry's own name. + /// @param path The path to take the final segment of. + /// @return The final segment. + function lastPathSegment(string memory path) internal pure returns (string memory) { + bytes memory pathBytes = bytes(path); + uint256 start = 0; + for (uint256 i = 0; i < pathBytes.length; i++) { + if (pathBytes[i] == "/") { + start = i + 1; + } + } + bytes memory segment = new bytes(pathBytes.length - start); + for (uint256 i = 0; i < segment.length; i++) { + segment[i] = pathBytes[start + i]; + } + return string(segment); + } + + /// @notice Reverts if `GENERATED_DIR` holds an artifact for `contractName` + /// other than the one at `pathForContract(contractName)`. + /// + /// An artifact for a contract is a direct child of `GENERATED_DIR` named + /// for that contract: the name in full, then a `.`, then anything. The full + /// name up to the `.` is what separates one contract's artifact from + /// another's, so a repo generating both `Foo` and `FooBar` keeps both. + /// + /// `pathForContract` names exactly one such child, and that one is asked + /// for here rather than spelled out again, so whichever file that function + /// names is the current artifact and every other one is a file this library + /// does not write. Nothing regenerates those, while the consumer's `src/**` + /// keeps importing them, so they are refused rather than left frozen beside + /// a fresh generation. + /// + /// Only direct children are read. `pathForContract` never names anything + /// deeper, so a per release snapshot directory holds nothing this library + /// wrote and is left alone. + /// + /// Inherits `pathForContract`'s refusal to produce a path for a name that + /// is not a Solidity identifier. + /// @param vm The Vm instance for file operations. + /// @param contractName The name of the contract. + function requireNoOrphanedArtifact(Vm vm, string memory contractName) internal view { + requireNoOrphanedArtifactIn(vm, GENERATED_DIR, contractName); + } + + /// @notice Reverts if `dir` holds an artifact for `contractName` other than + /// the one at `pathForContractIn(dir, contractName)`. + /// @dev `requireNoOrphanedArtifact` is this function applied to + /// `GENERATED_DIR`, so everything stated there holds here too, of `dir` + /// rather than of `GENERATED_DIR`. It is private for the same reason + /// `pathForContractIn` is: the only directory a consumer of this library + /// writes to is `GENERATED_DIR`, and `dir` is interpolated verbatim and is + /// not checked. + /// + /// The whole check is one read of `dir`, and `vm.readDir` does not revert + /// when that read fails: it returns a single entry naming `dir` itself and + /// carrying an `errorMessage`. No artifact name matches that entry, so a + /// directory that cannot be read is accepted rather than refused. That is + /// the answer wanted for a repo with no generated directory yet, and it is + /// why `buildFileForContract` creates `dir` before calling this: after a + /// `vm.createDir` that did not itself revert, the read is of a directory + /// that is there. + /// @param vm The Vm instance for file operations. + /// @param dir The directory to read, without a trailing separator, + /// interpolated verbatim. + /// @param contractName The name of the contract. + function requireNoOrphanedArtifactIn(Vm vm, string memory dir, string memory contractName) private view { + bytes32 currentArtifact = keccak256(bytes(lastPathSegment(pathForContractIn(dir, contractName)))); + bytes memory prefix = bytes(string.concat(contractName, ".")); + //forge-lint: disable-next-line(unsafe-cheatcode) + Vm.DirEntry[] memory entries = vm.readDir(dir); + for (uint256 i = 0; i < entries.length; i++) { + bytes memory name = bytes(lastPathSegment(entries[i].path)); + if (name.length < prefix.length || keccak256(name) == currentArtifact) { + continue; + } + bool isArtifact = true; + for (uint256 j = 0; j < prefix.length; j++) { + if (name[j] != prefix[j]) { + isArtifact = false; + break; + } + } + if (isArtifact) { + revert OrphanedGeneratedArtifact(string.concat(dir, "/", string(name))); + } + } + } + /// @notice Constructs the file path for a contract's generated file inside a /// tag's directory, which is the layout per release deploy pin snapshots /// use. @@ -178,6 +277,13 @@ library LibFs { /// nothing is created, unlinked or written unless there is content to /// write. /// + /// Another artifact for the same contract already in `GENERATED_DIR` + /// refuses the whole call, before anything at the path is unlinked or + /// written, so a generation never lands beside a file that nothing + /// regenerates. The directory is created first, because the check is a read + /// of it and a consumer generating for the first time has neither the + /// directory nor an orphan in it. + /// /// The path is unlinked until it holds nothing, then written, so a symlink /// there is replaced by a regular file rather than written through to its /// target, whether or not that target exists, and the path does not exist @@ -222,8 +328,9 @@ library LibFs { /// 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 + /// created when it is missing, what is read for another artifact of the + /// same contract, 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`. @@ -259,6 +366,7 @@ library LibFs { ); //forge-lint: disable-next-line(unsafe-cheatcode) vm.createDir(dir, true); + requireNoOrphanedArtifactIn(vm, dir, contractName); // `vm.removeFile` resolves the path before it acts, so on a live symlink // it takes what the link points at and leaves the link, now dangling. // Every pass removes something the next one no longer finds, so this diff --git a/test/concrete/LibFsExternal.sol b/test/concrete/LibFsExternal.sol index 31c4938..eb099f6 100644 --- a/test/concrete/LibFsExternal.sol +++ b/test/concrete/LibFsExternal.sol @@ -20,10 +20,26 @@ contract LibFsExternal { LibFs.buildFileForContract(vm, instance, contractName, spdxLicenseIdentifier, copyrightText, body); } + function buildFileForContract( + Vm vm, + address instance, + string memory dir, + string memory contractName, + string memory spdxLicenseIdentifier, + string memory copyrightText, + string memory body + ) external { + LibFs.buildFileForContract(vm, instance, dir, contractName, spdxLicenseIdentifier, copyrightText, body); + } + function pathForContract(string memory contractName) external pure returns (string memory) { return LibFs.pathForContract(contractName); } + function requireNoOrphanedArtifact(Vm vm, string memory contractName) external view { + LibFs.requireNoOrphanedArtifact(vm, contractName); + } + function buildFileForTaggedContract( Vm vm, address instance, diff --git a/test/src/lib/LibFs.buildFileForContract.t.sol b/test/src/lib/LibFs.buildFileForContract.t.sol index af0aa0c..25f7eea 100644 --- a/test/src/lib/LibFs.buildFileForContract.t.sol +++ b/test/src/lib/LibFs.buildFileForContract.t.sol @@ -3,7 +3,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.2/src/Test.sol"; -import {LibFs, GENERATED_DIR} from "src/lib/LibFs.sol"; +import {LibFs, GENERATED_DIR, OrphanedGeneratedArtifact} from "src/lib/LibFs.sol"; import { InvalidIdentifier, InvalidSpdxLicenseIdentifier, @@ -466,4 +466,147 @@ contract LibFsBuildFileForContractTest is Test { vm.assume(!LibCodeGenSlow.isIdentifierSlow(contractName)); assertNameRejected(contractName); } + + /// A consumer holds the artifact this library used to write committed and + /// imported from `src/**`. Generating beside it would leave those imports + /// resolving to a file nothing regenerates, with the build reporting + /// success, so the generation is refused instead and nothing is written. + /// + /// The refusal lands before the unlink as well as before the write, which + /// is what the pre-existing file at the generated path pins: a check placed + /// after the unlink would leave a consumer with neither artifact. + function testBuildFileForContractRefusesToOrphanAnotherArtifact() external { + string memory name = "LibFsBuildOrphan"; + string memory orphan = string.concat(GENERATED_DIR, "/", name, ".pointers.sol"); + //REUSE-IgnoreStart + string memory existing = "// SPDX-License-Identifier: LicenseRef-DCL-1.0\npragma solidity ^0.8.25;\n"; + //REUSE-IgnoreEnd + cleanupPath(orphan); + cleanup(name); + vm.writeFile(orphan, existing); + vm.writeFile(LibFs.pathForContract(name), "PRE-EXISTING"); + address instance = address(new CodeGennable()); + + // Caught rather than expected, and everything read off disk before any + // assertion, so the orphan is removed on the runs that fail too. It is + // a file this refusal fires on, and every test here shares the one + // generated directory, so one left behind is a precondition the next + // run does not get to choose. + bytes memory outcome; + try iExternal.buildFileForContract( + vm, instance, name, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, "\n// body\n" + ) {} + catch (bytes memory reason) { + outcome = reason; + } + bool currentPathIsPresent = vm.exists(LibFs.pathForContract(name)); + string memory currentPathContent = currentPathIsPresent ? vm.readFile(LibFs.pathForContract(name)) : ""; + string memory orphanContent = vm.readFile(orphan); + + cleanupPath(orphan); + cleanup(name); + + assertEq( + outcome, + abi.encodeWithSelector(OrphanedGeneratedArtifact.selector, orphan), + string.concat("not refused as OrphanedGeneratedArtifact(", orphan, ")") + ); + assertTrue(currentPathIsPresent, "a refused generation unlinked the file at the generated path"); + assertEq(currentPathContent, "PRE-EXISTING", "a refused generation wrote a second artifact"); + assertEq(orphanContent, existing, "the artifact that was already there was touched"); + } + + /// The refusal is the exception and not the rule: a generated directory + /// holding nothing for this contract generates, and one holding another + /// contract's artifact generates too. Driven through `buildFileForContract` + /// rather than through the check directly, so the call site is what is + /// asserted to accept. + function testBuildFileForContractGeneratesWhenNoOtherArtifactExists() external { + string memory name = "LibFsBuildNoOrphan"; + string memory otherContract = string.concat(GENERATED_DIR, "/LibFsBuildNoOrphanOther.pointers.sol"); + cleanup(name); + cleanupPath(string.concat(GENERATED_DIR, "/", name, ".pointers.sol")); + cleanupPath(otherContract); + vm.writeFile(otherContract, "// another contract's artifact\n"); + address instance = address(new CodeGennable()); + string memory body = "\n// no orphan\n"; + + LibFs.buildFileForContract(vm, instance, name, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body); + + string memory written = vm.readFile(LibFs.pathForContract(name)); + cleanup(name); + cleanupPath(otherContract); + assertEq(written, expectedFile(instance, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body)); + } + + /// The `dir` overload reads the directory it writes into, not + /// `GENERATED_DIR`. A consumer's snapshot directory and the live one hold + /// artifacts for the same contract names by design, so a check keyed on + /// `GENERATED_DIR` would refuse every snapshot generation for the live + /// artifact sitting beside it, and would miss an orphan in the snapshot + /// directory itself. + function testBuildFileForContractChecksTheDirectoryItWritesTo() external { + string memory name = "LibFsBuildDirOrphan"; + string memory dir = string.concat(GENERATED_DIR, "/LibFsBuildDirOrphanSnapshot"); + string memory liveOrphan = string.concat(GENERATED_DIR, "/", name, ".pointers.sol"); + string memory dirOrphan = string.concat(dir, "/", name, ".pointers.sol"); + cleanupPath(dir); + cleanupPath(liveOrphan); + cleanup(name); + address instance = address(new CodeGennable()); + string memory body = "\n// snapshot\n"; + + // An orphan in `GENERATED_DIR` does not refuse a write into `dir`. + vm.writeFile(liveOrphan, "// live orphan\n"); + LibFs.buildFileForContract(vm, instance, dir, name, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body); + string memory written = vm.readFile(string.concat(dir, "/", name, ".sol")); + + // An orphan in `dir` does refuse it. + vm.writeFile(dirOrphan, "// snapshot orphan\n"); + bytes memory outcome; + try iExternal.buildFileForContract(vm, instance, dir, name, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body) {} + catch (bytes memory reason) { + outcome = reason; + } + + cleanupPath(dir); + cleanupPath(liveOrphan); + + assertEq( + written, + expectedFile(instance, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body), + "an orphan outside the directory written to refused the write" + ); + assertEq( + outcome, + abi.encodeWithSelector(OrphanedGeneratedArtifact.selector, dirOrphan), + string.concat("not refused as OrphanedGeneratedArtifact(", dirOrphan, ")") + ); + } + + /// The check reads what is inside the directory, never the directory's own + /// name, and it reads it after `vm.createDir` rather than before. + /// + /// `vm.readDir` does not revert on a directory that is not there: it + /// returns one entry whose `path` is that directory and whose + /// `errorMessage` says why. A check placed ahead of the create therefore + /// reads that one entry, and for a directory whose own final segment reads + /// as an artifact for the contract it refuses the very first generation — + /// naming, as the orphan, the directory it was about to create. Driven + /// through such a directory, because that is the only shape the two + /// orderings disagree on. + function testBuildFileForContractReadsTheDirectoryNotItsOwnName() external { + string memory name = "LibFsBuildDirName"; + string memory dir = string.concat(GENERATED_DIR, "/", name, ".snapshot"); + cleanupPath(dir); + assertFalse(vm.exists(dir), "dirty precondition"); + address instance = address(new CodeGennable()); + string memory body = "\n// dir name\n"; + + LibFs.buildFileForContract(vm, instance, dir, name, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body); + + string memory written = vm.readFile(string.concat(dir, "/", name, ".sol")); + cleanupPath(dir); + assertEq(written, expectedFile(instance, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body)); + } } diff --git a/test/src/lib/LibFs.lastPathSegment.t.sol b/test/src/lib/LibFs.lastPathSegment.t.sol new file mode 100644 index 0000000..55799b7 --- /dev/null +++ b/test/src/lib/LibFs.lastPathSegment.t.sol @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.2/src/Test.sol"; +import {LibFs} from "src/lib/LibFs.sol"; + +/// @title LibFsLastPathSegmentTest +/// @notice `lastPathSegment` is what turns a directory entry into the name to +/// compare against, so its result is the whole basis of that comparison: a +/// segment that keeps any of the directories above it, or drops any of the name +/// itself, compares against something that is not the file's name. +/// +/// The property is asserted over arbitrary bytes rather than over paths, +/// because the function takes whatever `vm.readDir` reports and nothing +/// upstream of it constrains that to a shape. The cases below are the shapes +/// that carry a boundary: a name with no directory above it, an empty result, a +/// separator with nothing after it. +contract LibFsLastPathSegmentTest is Test { + /// The absolute path a directory read reports, which is the input the + /// library actually receives. + function testLastPathSegmentAbsolutePath() external pure { + assertEq(LibFs.lastPathSegment("/home/user/repo/src/generated/Foo.sol"), "Foo.sol"); + } + + /// The relative path `pathForContract` returns, which is the input the + /// current artifact's name is taken from. + function testLastPathSegmentRelativePath() external pure { + assertEq(LibFs.lastPathSegment("src/generated/Foo.sol"), "Foo.sol"); + } + + /// A path with no separator at all is entirely its own final segment. + function testLastPathSegmentNoSeparator() external pure { + assertEq(LibFs.lastPathSegment("Foo.sol"), "Foo.sol"); + assertEq(LibFs.lastPathSegment(""), ""); + } + + /// Only the last separator counts, so repeated and empty intermediate + /// segments are all above the name and none of them reach it. + function testLastPathSegmentRepeatedSeparators() external pure { + assertEq(LibFs.lastPathSegment("a//b///Foo.sol"), "Foo.sol"); + } + + /// A path ending in a separator has nothing after it, and an empty segment + /// is the honest answer rather than the segment before it. + function testLastPathSegmentTrailingSeparator() external pure { + assertEq(LibFs.lastPathSegment("src/generated/"), ""); + assertEq(LibFs.lastPathSegment("/"), ""); + } + + /// The three halves of the definition, over arbitrary bytes: the result + /// carries no separator, it is a suffix of the input, and it is the LONGEST + /// such suffix, which is what pins it to the last separator rather than to + /// any earlier one. + function testLastPathSegmentIsTheSuffixAfterTheLastSeparator(bytes memory pathBytes) external pure { + bytes memory segment = bytes(LibFs.lastPathSegment(string(pathBytes))); + + assertLe(segment.length, pathBytes.length, "segment is longer than the path"); + uint256 start = pathBytes.length - segment.length; + for (uint256 i = 0; i < segment.length; i++) { + assertTrue(segment[i] != "/", "segment carries a separator"); + assertTrue(segment[i] == pathBytes[start + i], "segment is not a suffix of the path"); + } + if (start > 0) { + assertTrue(pathBytes[start - 1] == "/", "segment does not start after a separator"); + } + } +} diff --git a/test/src/lib/LibFs.requireNoOrphanedArtifact.t.sol b/test/src/lib/LibFs.requireNoOrphanedArtifact.t.sol new file mode 100644 index 0000000..9d81756 --- /dev/null +++ b/test/src/lib/LibFs.requireNoOrphanedArtifact.t.sol @@ -0,0 +1,279 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.2/src/Test.sol"; +import {LibFs, GENERATED_DIR, OrphanedGeneratedArtifact} from "src/lib/LibFs.sol"; +import {InvalidIdentifier} from "src/lib/LibCodeGen.sol"; +import {LibFsExternal} from "test/concrete/LibFsExternal.sol"; +import {LibCodeGenSlow} from "test/lib/LibCodeGenSlow.sol"; + +/// @dev A parseable Solidity source unit, so that a file one of these tests +/// leaves under `src/generated` on failure does not also break the compile. +//REUSE-IgnoreStart +string constant PLACEHOLDER_SOURCE = "// SPDX-License-Identifier: LicenseRef-DCL-1.0\npragma solidity ^0.8.25;\n"; + +//REUSE-IgnoreEnd + +/// @title LibFsRequireNoOrphanedArtifactTest +/// @notice Consumers commit what this library generates and import it by path, +/// so a second artifact for the same contract sitting beside the one being +/// generated is a file nothing regenerates while `src/**` keeps importing it. +/// These assert that such a file is refused, that the refusal is keyed on the +/// contract name rather than on any one extension, and that the file the +/// library does write is the one thing it is not refused for. +/// +/// Every test owns a contract name no other test uses, because the check reads +/// the whole generated directory and suites run in parallel. +contract LibFsRequireNoOrphanedArtifactTest is Test { + /// `vm.expectRevert` needs a call frame, and `requireNoOrphanedArtifact` is + /// an internal library function that is inlined into its caller. + LibFsExternal internal immutable iExternal; + + constructor() { + iExternal = new LibFsExternal(); + } + + /// The check is one read of `GENERATED_DIR`, and `src/generated/` holds no + /// committed file, so nothing in a fresh clone creates it. Every test here + /// also writes its fixture there directly, and a filtered run may be only + /// one of them, so the directory is not something this contract can inherit + /// from a test that happened to run earlier. + function setUp() external { + vm.createDir(GENERATED_DIR, true); + } + + /// Removes whatever is at `path`, so a test establishes its own + /// precondition rather than assuming one. + function cleanupPath(string memory path) internal { + if (vm.exists(path)) { + if (vm.isDir(path)) { + vm.removeDir(path, true); + } else { + vm.removeFile(path); + } + } + } + + /// The path an artifact for `contractName` occupies when its name carries + /// `suffix` in place of the `sol` the library appends. + function artifactPath(string memory contractName, string memory suffix) internal pure returns (string memory) { + return string.concat(GENERATED_DIR, "/", contractName, ".", suffix); + } + + /// Runs the check and hands back what it reverted with, or empty bytes when + /// it did not revert. Nothing here asserts, so a caller removes its + /// fixtures before it asserts anything about them. + /// + /// A revert and a failed assertion both abort the test body at the point + /// they happen, so a fixture removed after either one is removed only on + /// the runs that pass. Every fixture these tests write is a file this check + /// refuses on, and they all share the one generated directory, so a fixture + /// left behind by a failing run is a precondition the next run does not get + /// to choose. + function checkOutcome(string memory contractName) internal returns (bytes memory) { + try iExternal.requireNoOrphanedArtifact(vm, contractName) { + return ""; + } catch (bytes memory reason) { + return reason; + } + } + + /// Asserts `outcome` is the check accepting the contract. + function assertAccepted(bytes memory outcome) internal pure { + assertEq(outcome.length, 0, "the check refused a contract it must accept"); + } + + /// Asserts `outcome` is the check refusing `orphan`, by the exact error and + /// path a consumer is shown. The comparison is over the raw revert bytes, + /// so the message says in words what those bytes are meant to be. + function assertRefused(bytes memory outcome, string memory orphan) internal pure { + assertEq( + outcome, + abi.encodeWithSelector(OrphanedGeneratedArtifact.selector, orphan), + string.concat("not refused as OrphanedGeneratedArtifact(", orphan, ")") + ); + } + + /// No artifact at all for the contract is the first generation in a repo, + /// which must not be refused. + function testRequireNoOrphanedArtifactAcceptsNothingForTheContract() external { + string memory name = "LibFsOrphanNothing"; + cleanupPath(LibFs.pathForContract(name)); + cleanupPath(artifactPath(name, "pointers.sol")); + + LibFs.requireNoOrphanedArtifact(vm, name); + } + + /// The file the library writes is the one artifact for the contract that is + /// never an orphan, so regenerating over it is not refused. + function testRequireNoOrphanedArtifactAcceptsTheFileItWrites() external { + string memory name = "LibFsOrphanCurrent"; + vm.writeFile(LibFs.pathForContract(name), PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(LibFs.pathForContract(name)); + assertAccepted(outcome); + } + + /// The artifact name this library wrote before `src/generated/.sol`. + /// Consumers still hold these committed and imported, so this is the case + /// that reaches the check in the field. + function testRequireNoOrphanedArtifactRejectsTheLegacyName() external { + string memory name = "LibFsOrphanLegacy"; + string memory legacy = artifactPath(name, "pointers.sol"); + cleanupPath(legacy); + vm.writeFile(legacy, PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(legacy); + assertRefused(outcome, legacy); + } + + /// Having generated the current file does not excuse the orphan: adding the + /// new artifact without deleting the old one leaves `src/**` importing the + /// old one, which is the state the refusal exists to reject. + function testRequireNoOrphanedArtifactRejectsAlongsideTheCurrentFile() external { + string memory name = "LibFsOrphanBoth"; + string memory legacy = artifactPath(name, "pointers.sol"); + cleanupPath(legacy); + vm.writeFile(LibFs.pathForContract(name), PLACEHOLDER_SOURCE); + vm.writeFile(legacy, PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(legacy); + cleanupPath(LibFs.pathForContract(name)); + assertRefused(outcome, legacy); + } + + /// Writes an artifact for `contractName` carrying `suffix`, asserts it is + /// refused by the path it occupies, and removes it again. The contract name + /// is the caller's so that each case owns a file no other case names. + function assertSuffixRejected(string memory contractName, string memory suffix) internal { + string memory orphan = artifactPath(contractName, suffix); + cleanupPath(orphan); + vm.writeFile(orphan, PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(contractName); + + cleanupPath(orphan); + assertRefused(outcome, orphan); + } + + /// The refusal is keyed on the contract name, not on the one extension that + /// happens to be stale today, so the next time `pathForContract` moves + /// there is nothing here to update. Spelled out one suffix at a time rather + /// than fuzzed: fuzz cases run concurrently against the one generated + /// directory, two cases whose file names collide race on it, and random + /// identifiers essentially never land near `sol` anyway. These do — every + /// one of them differs from the appended `sol` by a single edit, which is + /// what a comparison that is nearly right survives. + function testRequireNoOrphanedArtifactRejectsEveryOtherSuffix() external { + assertSuffixRejected("LibFsOrphanSuffixLegacy", "pointers.sol"); + assertSuffixRejected("LibFsOrphanSuffixShort", "so"); + assertSuffixRejected("LibFsOrphanSuffixLong", "soll"); + assertSuffixRejected("LibFsOrphanSuffixUpper", "SOL"); + assertSuffixRejected("LibFsOrphanSuffixLead", "asol"); + assertSuffixRejected("LibFsOrphanSuffixDigit", "sol0"); + assertSuffixRejected("LibFsOrphanSuffixDouble", "sol.sol"); + assertSuffixRejected("LibFsOrphanSuffixOther", "json"); + assertSuffixRejected("LibFsOrphanSuffixBare", "pointers"); + assertSuffixRejected("LibFsOrphanSuffixEmpty", ""); + } + + /// An artifact is the contract name followed by a `.`, so the name on its + /// own is not one. Nothing this library writes is extensionless, and a file + /// that is holds no imports for `src/**` to resolve to. + function testRequireNoOrphanedArtifactIgnoresTheBareName() external { + string memory name = "LibFsOrphanBare"; + string memory bare = string.concat(GENERATED_DIR, "/", name); + cleanupPath(bare); + vm.writeFile(bare, PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(bare); + assertAccepted(outcome); + } + + /// An artifact belongs to the contract whose name it carries in full, up to + /// the `.`. A longer name that merely starts with this one is a different + /// contract's artifact and must not be refused, or a repo could not + /// generate both `Foo` and `FooBar`. + function testRequireNoOrphanedArtifactIgnoresOtherContracts() external { + string memory name = "LibFsOrphanPrefix"; + string memory sibling = artifactPath(string.concat(name, "Extra"), "pointers.sol"); + string memory unrelated = artifactPath("LibFsOrphanUnrelated", "pointers.sol"); + cleanupPath(sibling); + cleanupPath(unrelated); + vm.writeFile(sibling, PLACEHOLDER_SOURCE); + vm.writeFile(unrelated, PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(sibling); + cleanupPath(unrelated); + assertAccepted(outcome); + } + + /// A name that is a prefix of this one, and so a shorter contract's + /// artifact, is likewise not this contract's. + function testRequireNoOrphanedArtifactIgnoresShorterContracts() external { + string memory name = "LibFsOrphanLongerName"; + string memory shorter = artifactPath("LibFsOrphanLonger", "pointers.sol"); + cleanupPath(shorter); + vm.writeFile(shorter, PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(shorter); + assertAccepted(outcome); + } + + /// Consumers freeze per release snapshots into subdirectories of the + /// generated directory. `pathForContract` never names anything below a + /// direct child, so nothing down there is an artifact this library wrote + /// and none of it is refused. + function testRequireNoOrphanedArtifactIgnoresSubdirectories() external { + string memory name = "LibFsOrphanNested"; + string memory dir = string.concat(GENERATED_DIR, "/LibFsOrphanTag"); + cleanupPath(dir); + vm.createDir(dir, true); + vm.writeFile(string.concat(dir, "/", name, ".pointers.sol"), PLACEHOLDER_SOURCE); + vm.writeFile(string.concat(dir, "/", name, ".sol"), PLACEHOLDER_SOURCE); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(dir); + assertAccepted(outcome); + } + + /// A directory occupying an artifact's name is not something this library + /// wrote either, and leaving it there means the name is taken by something + /// no regeneration touches. + function testRequireNoOrphanedArtifactRejectsADirectory() external { + string memory name = "LibFsOrphanDir"; + string memory orphan = artifactPath(name, "pointers.sol"); + cleanupPath(orphan); + vm.createDir(orphan, true); + + bytes memory outcome = checkOutcome(name); + + cleanupPath(orphan); + assertRefused(outcome, orphan); + } + + /// The check asks `pathForContract` which file is the current one, so it + /// inherits that function's refusal to produce a path for a name that is + /// not a Solidity identifier. + function testRequireNoOrphanedArtifactRejectsEveryNonIdentifierName(bytes memory nameBytes) external { + string memory contractName = string(nameBytes); + vm.assume(!LibCodeGenSlow.isIdentifierSlow(contractName)); + + vm.expectRevert(abi.encodeWithSelector(InvalidIdentifier.selector, contractName)); + iExternal.requireNoOrphanedArtifact(vm, contractName); + } +}