From baff36f43a00be85c55f9d61533cdc1f17731e03 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 07:25:55 +0000 Subject: [PATCH 1/2] Restore Rain's licence and copyright as defaults alongside the parameters `filePrefix`, `buildFileForContract` (both spellings) and `buildFileForTaggedContract` each gain an overload that takes neither `spdxLicenseIdentifier` nor `copyrightText` and applies `RAIN_SPDX_LICENSE_IDENTIFIER` and `RAIN_COPYRIGHT_TEXT`, two new exported constants holding this org's own values. The parameterised forms are unchanged, so a consumer outside the org still states its own header and #75 stays closed. Each defaulting overload is the parameterised one applied to the two constants and nothing else, so there is no second spelling of the header to drift, and the non-empty-single-line rule holds over the defaults like any other value. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 26 ++++++ src/lib/LibCodeGen.sol | 36 ++++++++ src/lib/LibFs.sol | 82 ++++++++++++++++++- test/src/lib/LibCodeGen.filePrefix.t.sol | 47 ++++++++++- test/src/lib/LibFs.buildFileForContract.t.sol | 56 ++++++++++++- .../LibFs.buildFileForTaggedContract.t.sol | 31 ++++++- 6 files changed, 274 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 5c25fde..56ad344 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,32 @@ 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. +## Generated file header + +Every generated file opens with a licence tag, a copyright tag, +`pragma solidity ^0.8.25;` and an autogenerated notice. + +The file lands in the consuming repo, so the licence it is under and the +copyright holder it names are that repo's statement about its own source, and +`LibCodeGen.filePrefix`, `LibFs.buildFileForContract` and +`LibFs.buildFileForTaggedContract` all take them as arguments. Each value has to +be a non-empty single line: an empty one leaves a tag that `reuse lint` accepts +while it names nothing and `solc` refuses, and a line break ends the tag's line +so that whatever follows lands as source. + +Each of those three also has an overload taking neither, which declares +`RAIN_SPDX_LICENSE_IDENTIFIER` (`LicenseRef-DCL-1.0`) and `RAIN_COPYRIGHT_TEXT` +(`Copyright (c) 2020 Rain Open Source Software Ltd`) — the values every repo in +this org declares for its own source. A Rain repo generates through those and +passes nothing. Both constants are exported from `LibCodeGen`, so a consumer +that threads the header down through its own build library names them rather +than restating the strings. + +A repo this org does not own passes its own two values. The defaulting overloads +would stamp Rain's licence and Rain's copyright holder into its files, and +`reuse lint` there would still pass, because it checks that the tag is present +rather than that it is right. + ## Formatter requirements `LibCodeGen` wraps the declarations it emits itself, deciding against diff --git a/src/lib/LibCodeGen.sol b/src/lib/LibCodeGen.sol index cc481f7..781b401 100644 --- a/src/lib/LibCodeGen.sol +++ b/src/lib/LibCodeGen.sol @@ -17,6 +17,18 @@ uint256 constant MAX_LINE_LENGTH = 120; /// needs to match what formatters expect. string constant NEWLINE_DUE_TO_MAX_LENGTH = "\n "; +/// @dev The SPDX licence identifier every repo in this org declares for its own +/// source, and so the identifier that heads a file generated into one of them +/// when the caller names none. Exported rather than inlined into the defaulting +/// overload, so a consumer that threads the header through its own build can +/// name the value instead of restating the string. +string constant RAIN_SPDX_LICENSE_IDENTIFIER = "LicenseRef-DCL-1.0"; + +/// @dev The copyright text every repo in this org declares for its own source, +/// and so the text that heads a file generated into one of them when the caller +/// names none. Exported for the same reason. +string constant RAIN_COPYRIGHT_TEXT = "Copyright (c) 2020 Rain Open Source Software Ltd"; + /// Thrown when a name is not a Solidity identifier. Such a name cannot be /// interpolated into a file path or a constant declaration. /// @param name The rejected name. @@ -112,6 +124,10 @@ library LibCodeGen { /// state and are taken from the caller. Both are interpolated verbatim into /// their tags, and both have to be a non-empty single line for the tag they /// land on to say what it appears to. + /// + /// The overload that takes neither makes that statement out of this org's + /// values, which only a repo this org owns can make. A consumer outside it + /// calls this one. /// @param spdxLicenseIdentifier The SPDX licence identifier for the /// generated file, interpolated verbatim. /// @param copyrightText The copyright text for the generated file, @@ -140,6 +156,26 @@ library LibCodeGen { //REUSE-IgnoreEnd } + /// The file prefix headed by the licence and the copyright text this org's + /// repos declare for their own source. + /// + /// A generated file lands in the calling project's repo, so what it says + /// about its own licensing is that project's statement to make. This + /// overload makes it on the caller's behalf out of + /// `RAIN_SPDX_LICENSE_IDENTIFIER` and `RAIN_COPYRIGHT_TEXT`, which is right + /// for a repo this org owns and wrong for every other repo. A consumer + /// elsewhere calls the overload that takes the two values, and passes its + /// own. + /// + /// This is that overload applied to those two constants and nothing else, + /// so a header written by defaulting is one the caller could have written + /// out, and every rule that overload puts on either value holds here rather + /// than being skipped because the values came from the library. + /// @return The text that heads the generated file. + function filePrefix() internal pure returns (string memory) { + return filePrefix(RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT); + } + /// Puts the hash of the bytecode of some contract instance into a constant /// string. Often used to ensure that the deployed bytecode matches the /// expected bytecode. diff --git a/src/lib/LibFs.sol b/src/lib/LibFs.sol index 3345a3f..b801860 100644 --- a/src/lib/LibFs.sol +++ b/src/lib/LibFs.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.25; import {Vm, VmSafe} from "forge-std-1.16.2/src/Vm.sol"; -import {LibCodeGen} from "./LibCodeGen.sol"; +import {LibCodeGen, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT} from "./LibCodeGen.sol"; /// @dev The directory that generated contract files are written to, relative to /// the project root. Consumers commit this directory and import from it by @@ -438,6 +438,27 @@ library LibFs { buildFileForContract(vm, instance, GENERATED_DIR, contractName, spdxLicenseIdentifier, copyrightText, body); } + /// @notice Builds a file for a generated contract, declaring the licence and + /// the copyright text this org's repos declare for their own source. + /// @dev This is `buildFileForContract` applied to + /// `RAIN_SPDX_LICENSE_IDENTIFIER` and `RAIN_COPYRIGHT_TEXT`, so everything + /// that function states holds here, including the rule those two values are + /// subject to like any other. + /// + /// The file lands in the calling project's repo, so the header is a + /// statement that project makes about its own source. This overload makes it + /// out of this org's values, which is right for a repo this org owns and + /// wrong for every other repo; a consumer elsewhere calls the overload that + /// takes the two values, and passes its own. + /// @param vm The Vm instance for file operations. + /// @param instance The contract instance whose bytecode hash is to be + /// included. + /// @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 { + buildFileForContract(vm, instance, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, 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 @@ -508,6 +529,35 @@ library LibFs { vm.writeFile(path, content); } + /// @notice Builds a file for a generated contract inside `dir`, declaring + /// the licence and the copyright text this org's repos declare for their own + /// source. + /// @dev This is the `dir` overload of `buildFileForContract` applied to + /// `RAIN_SPDX_LICENSE_IDENTIFIER` and `RAIN_COPYRIGHT_TEXT`, so everything + /// that overload states holds here. Choosing the directory and stating the + /// licence are independent of each other, so a caller that does the first + /// does not thereby have to do the second. + /// + /// The header is this org's, which is right for a repo this org owns and + /// wrong for every other repo; a consumer elsewhere calls the overload that + /// takes the two values, and passes its own. + /// @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 { + buildFileForContract(vm, instance, dir, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, body); + } + /// @notice Builds a file for a generated contract at /// `pathForTaggedContract(tag, contractName)`. /// @@ -573,4 +623,34 @@ library LibFs { ) internal { buildFileForContract(vm, instance, dirForTag(tag), contractName, spdxLicenseIdentifier, copyrightText, body); } + + /// @notice Builds a file for a generated contract at + /// `pathForTaggedContract(tag, contractName)`, declaring the licence and the + /// copyright text this org's repos declare for their own source. + /// @dev This is `buildFileForTaggedContract` applied to + /// `RAIN_SPDX_LICENSE_IDENTIFIER` and `RAIN_COPYRIGHT_TEXT`, so everything + /// that function states holds here. + /// + /// The header is this org's, which is right for a repo this org owns and + /// wrong for every other repo; a consumer elsewhere calls the overload that + /// takes the two values, and passes its own. A tagged file is a per release + /// snapshot that the org's release convention freezes once written, so the + /// header it lands with is the header it keeps. + /// @param vm The Vm instance for file operations. + /// @param instance The contract instance whose bytecode hash is to be + /// included. + /// @param tag The tag whose directory the file lives in. + /// @param contractName The name of the contract. + /// @param body The body of the contract file to be written. + function buildFileForTaggedContract( + Vm vm, + address instance, + string memory tag, + string memory contractName, + string memory body + ) internal { + buildFileForTaggedContract( + vm, instance, tag, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, body + ); + } } diff --git a/test/src/lib/LibCodeGen.filePrefix.t.sol b/test/src/lib/LibCodeGen.filePrefix.t.sol index 74fb606..ad8d1dd 100644 --- a/test/src/lib/LibCodeGen.filePrefix.t.sol +++ b/test/src/lib/LibCodeGen.filePrefix.t.sol @@ -3,7 +3,13 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.2/src/Test.sol"; -import {LibCodeGen, InvalidSpdxLicenseIdentifier, InvalidCopyrightText} from "src/lib/LibCodeGen.sol"; +import { + LibCodeGen, + InvalidSpdxLicenseIdentifier, + InvalidCopyrightText, + RAIN_SPDX_LICENSE_IDENTIFIER, + RAIN_COPYRIGHT_TEXT +} from "src/lib/LibCodeGen.sol"; // The subject of this whole file is emitted licence text, so SPDX tag prefixes // appear throughout it without a value attached. The file's own header is above @@ -187,5 +193,44 @@ contract LibCodeGenFilePrefixTest is Test { vm.expectRevert(abi.encodeWithSelector(InvalidSpdxLicenseIdentifier.selector, "")); this.callFilePrefix("", ""); } + + /// The two exported defaults are this repo's own licence and copyright, + /// pinned as literals rather than read back out of the header they build. + /// They are what every Rain repo's generated files declare, so a change to + /// either rewrites committed source org wide and has to land as a + /// deliberate diff here. + function testRainDefaultsExact() external pure { + assertEq(RAIN_SPDX_LICENSE_IDENTIFIER, "LicenseRef-DCL-1.0"); + assertEq(RAIN_COPYRIGHT_TEXT, "Copyright (c) 2020 Rain Open Source Software Ltd"); + } + + /// A Rain repo gets Rain's licence and copyright without passing them, and + /// what it gets is byte for byte what a caller naming the two exported + /// defaults gets. The short overload is the long one applied to those two + /// constants and nothing else, so there is no second spelling of the header + /// that can drift from the first. + function testFilePrefixDefaultEqualsExplicitRainValues() external pure { + assertEq(LibCodeGen.filePrefix(), LibCodeGen.filePrefix(RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT)); + } + + /// The defaulted header, pinned to the literal. The equality above is + /// satisfied by both sides being wrong in the same way; this is what says + /// which bytes they both have to be. + function testFilePrefixDefaultExact() external pure { + assertEq( + LibCodeGen.filePrefix(), + "// SPDX-License-Identifier: LicenseRef-DCL-1.0\n" + "// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd\n" + "pragma solidity ^0.8.25;\n\n" "// THIS FILE IS AUTOGENERATED BY THE BUILD SCRIPT. DO NOT EDIT BY HAND.\n" + ); + } + + /// The defaults are values the long overload accepts, so the short one is + /// reachable rather than a call that always reverts. Nothing about the two + /// constants is exempt from the rule the header puts on any other value. + function testFilePrefixDefaultsAreAcceptedValues() external pure { + assertTrue(LibCodeGen.isSingleLine(RAIN_SPDX_LICENSE_IDENTIFIER), "default licence is not a single line"); + assertTrue(LibCodeGen.isSingleLine(RAIN_COPYRIGHT_TEXT), "default copyright is not a single line"); + } } //REUSE-IgnoreEnd diff --git a/test/src/lib/LibFs.buildFileForContract.t.sol b/test/src/lib/LibFs.buildFileForContract.t.sol index cc2622e..db63c94 100644 --- a/test/src/lib/LibFs.buildFileForContract.t.sol +++ b/test/src/lib/LibFs.buildFileForContract.t.sol @@ -9,7 +9,9 @@ import { InvalidIdentifier, InvalidSpdxLicenseIdentifier, InvalidCopyrightText, - CodelessInstance + CodelessInstance, + RAIN_SPDX_LICENSE_IDENTIFIER, + RAIN_COPYRIGHT_TEXT } from "src/lib/LibCodeGen.sol"; import {CodeGennable} from "test/concrete/CodeGennable.sol"; import {RemovalVm, WriteReached, RemoveFileReached, FfiReached} from "test/concrete/RemovalVm.sol"; @@ -803,4 +805,56 @@ contract LibFsBuildFileForContractTest is Test { cleanupPath(dir); assertEq(written, expectedFile(instance, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body)); } + + /// A Rain repo writes without naming a licence or a copyright holder, and + /// the file that lands is byte for byte the file a caller naming the two + /// exported defaults gets. Both writes are for the same instance, so the + /// bytecode hash constant is identical in both and the header is the only + /// thing that could differ. + /// + /// Asserted against the literal as well, so the two spellings agreeing with + /// each other is not what makes this pass. A default that drifted from this + /// repo's own licence would move both files together, and only the literal + /// is outside the library to catch it. + function testBuildFileForContractDefaultHeaderIsRainsExplicitHeader() external { + string memory defaulted = "LibFsBuildDefaultHeader"; + string memory named = "LibFsBuildNamedHeader"; + cleanup(defaulted); + cleanup(named); + address instance = address(new CodeGennable()); + string memory body = "\n// default header\n"; + + LibFs.buildFileForContract(vm, instance, defaulted, body); + LibFs.buildFileForContract(vm, instance, named, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, body); + + string memory defaultedFile = vm.readFile(LibFs.pathForContract(defaulted)); + string memory namedFile = vm.readFile(LibFs.pathForContract(named)); + cleanup(defaulted); + cleanup(named); + + assertEq(defaultedFile, namedFile, "the defaulted write is not the write that names the defaults"); + assertEq(defaultedFile, expectedFile(instance, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body)); + } + + /// The `dir` overload defaults the header the same way. A caller that + /// chooses the directory is not thereby a caller that has to state the + /// licence, so the two arguments the overloads differ by are independent. + function testBuildFileForContractInDirDefaultHeaderIsRainsExplicitHeader() external { + string memory dir = string.concat(GENERATED_DIR, "/LibFsBuildDirDefaultHeader"); + string memory defaulted = "LibFsBuildDirDefaultHeader"; + string memory named = "LibFsBuildDirNamedHeader"; + cleanupPath(dir); + address instance = address(new CodeGennable()); + string memory body = "\n// dir default header\n"; + + LibFs.buildFileForContract(vm, instance, dir, defaulted, body); + LibFs.buildFileForContract(vm, instance, dir, named, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, body); + + string memory defaultedFile = vm.readFile(string.concat(dir, "/", defaulted, ".sol")); + string memory namedFile = vm.readFile(string.concat(dir, "/", named, ".sol")); + cleanupPath(dir); + + assertEq(defaultedFile, namedFile, "the defaulted write is not the write that names the defaults"); + assertEq(defaultedFile, expectedFile(instance, SPDX_LICENSE_IDENTIFIER, COPYRIGHT_TEXT, body)); + } } diff --git a/test/src/lib/LibFs.buildFileForTaggedContract.t.sol b/test/src/lib/LibFs.buildFileForTaggedContract.t.sol index e8bf364..fc28032 100644 --- a/test/src/lib/LibFs.buildFileForTaggedContract.t.sol +++ b/test/src/lib/LibFs.buildFileForTaggedContract.t.sol @@ -4,7 +4,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.2/src/Test.sol"; import {LibFs, GENERATED_DIR, InvalidTag} from "src/lib/LibFs.sol"; -import {InvalidIdentifier} from "src/lib/LibCodeGen.sol"; +import {InvalidIdentifier, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT} from "src/lib/LibCodeGen.sol"; import {CodeGennable} from "test/concrete/CodeGennable.sol"; import {LibFsExternal} from "test/concrete/LibFsExternal.sol"; import {LibCodeGenSlow} from "test/lib/LibCodeGenSlow.sol"; @@ -440,4 +440,33 @@ contract LibFsBuildFileForTaggedContractTest is Test { vm.assume(!LibCodeGenSlow.isIdentifierSlow(contractName)); assertNameRejected("0_1_1$taggedFuzzName", contractName); } + + /// A Rain repo writes a snapshot without naming a licence or a copyright + /// holder, and the file that lands is byte for byte the file a caller + /// naming the two exported defaults gets. Both writes are for the same + /// instance and into the same tag, so the header is the only thing that + /// could differ. + /// + /// Asserted against the literal as well, through `expectedFile`, which + /// spells this repo's licence and copyright out rather than reading them + /// back out of the library. A snapshot is frozen once written, so a header + /// that drifted is permanent in the release record. + function testBuildFileForTaggedContractDefaultHeaderIsRainsExplicitHeader() external { + string memory tag = "0_1_1$taggedDefaultHeader"; + cleanup(tag); + address instance = address(new CodeGennable()); + string memory body = "\n// default header\n"; + + LibFs.buildFileForTaggedContract(vm, instance, tag, "LibFsTaggedDefaultHeader", body); + LibFs.buildFileForTaggedContract( + vm, instance, tag, "LibFsTaggedNamedHeader", RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, body + ); + + string memory defaultedFile = vm.readFile(LibFs.pathForTaggedContract(tag, "LibFsTaggedDefaultHeader")); + string memory namedFile = vm.readFile(LibFs.pathForTaggedContract(tag, "LibFsTaggedNamedHeader")); + cleanup(tag); + + assertEq(defaultedFile, namedFile, "the defaulted write is not the write that names the defaults"); + assertEq(defaultedFile, expectedFile(instance, body)); + } } From 2a5a63812280fc453e21790c60112f7ea2ba61f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 07:37:06 +0000 Subject: [PATCH 2/2] Name the overloads in filePrefix's NatSpec instead of pointing at them "A consumer outside it calls this one" put two pronouns next to two competing antecedents: the sentence before it is about the overload taking no arguments, so "this one" reads as that overload rather than as the one the comment documents, which is the opposite instruction. CodeRabbit read it that way on #144. Behaviour is unchanged; the text now names the two argument overload as what a consumer in another org calls. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibCodeGen.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib/LibCodeGen.sol b/src/lib/LibCodeGen.sol index 781b401..b46662b 100644 --- a/src/lib/LibCodeGen.sol +++ b/src/lib/LibCodeGen.sol @@ -125,9 +125,10 @@ library LibCodeGen { /// their tags, and both have to be a non-empty single line for the tag they /// land on to say what it appears to. /// - /// The overload that takes neither makes that statement out of this org's - /// values, which only a repo this org owns can make. A consumer outside it - /// calls this one. + /// The overload taking no arguments makes that statement out of this org's + /// own values, which is a statement only a repo this org owns can make. A + /// consumer in another org calls this two argument overload and passes its + /// own values. /// @param spdxLicenseIdentifier The SPDX licence identifier for the /// generated file, interpolated verbatim. /// @param copyrightText The copyright text for the generated file,