Skip to content

Parameterise the generated file's licence and copyright - #135

Merged
thedavidmeister merged 14 commits into
mainfrom
2026-08-16-issue-75
Aug 17, 2026
Merged

Parameterise the generated file's licence and copyright#135
thedavidmeister merged 14 commits into
mainfrom
2026-08-16-issue-75

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Closes #75

LibCodeGen.filePrefix() returned a hardcoded LicenseRef-DCL-1.0 +
Copyright (c) 2020 Rain Open Source Software Ltd, and
LibFs.buildFileForContract stamped it onto every file it writes, in every
consumer repo, in any org. S01-Issuer/st0x.deploy is live on this library:
its ten src/generated/0_1_1/*.pointers.sol files — append-only deploy-pin
snapshots, the release record consumers pin addresses and codehashes against —
each open with Rain's licence and Rain's copyright holder. reuse lint there
passes, because it checks the tag is present, not that it is right.

What changed

  • filePrefix()filePrefix(string spdxLicenseIdentifier, string copyrightText).
    Both are interpolated verbatim into their own SPDX tag. The pragma and the
    autogenerated notice stay the library's: those are facts about the code it
    emits, not claims about who owns the repo the file lands in.
  • New LibCodeGen.isSingleLine(string) and two errors,
    InvalidSpdxLicenseIdentifier(string) and InvalidCopyrightText(string).
    Each value must be non-empty and free of 0x0A/0x0D; the licence is
    checked first.
  • LibFs.buildFileForContract takes both and passes them through.

What the merge with main changed in this PR

origin/main at dbcd5e1 is merged in. main moved three times while this was
being finished — #100, then #136, then #127 and #123 together — and three of
those touch the function these parameters run through, so each round is a real
resolution rather than a formality. Seven resolutions that are not mechanical,
and one branch head absorbed rather than taken.

#100 landed, and it rebuilt the function these parameters run through.
buildFileForContract now computes the whole file content into a local before
it touches disk:

string memory content = string.concat(
    LibCodeGen.filePrefix(spdxLicenseIdentifier, copyrightText),
    LibCodeGen.bytecodeHashConstantString(vm, instance),
    body
);
vm.createDir(dir, true);
while (isPresent(vm, path)) { vm.removeFile(path); }
vm.writeFile(path, content);

Resolved onto that ordering rather than restoring this branch's
compute-at-vm.writeFile. That inverts the "Decisions" entry this PR used to
carry, which is rewritten below. #100's own
testBuildFileForContractFailedBuildKeepsExistingFile is kept, passes the two
values like every other call site, and passes — it is the proof the resolution
is the right way round. The docstring paragraph #100 added is widened by one
clause, from "building it reverts for an instance that holds no code" to
"...and for a licence or copyright filePrefix refuses", because this branch is
what adds the second revert path to the step that paragraph is about.

#127 landed too, and turned the unlink into a loop. if (isPresent(...))
became while (isPresent(...)), in the same function, after the content local.
Taken as main has it; nothing about the two threaded values interacts with how
many times the path is unlinked, and #127's own docstring paragraphs about the
symlink cases sit alongside the licence paragraph this PR adds rather than in it.

#127 also added testBuildFileForContractReplacesLiveSymlink, and git merged
it silently with the wrong arity.
It calls buildFileForContract twice in
test/src/lib/LibFs.isPresent.t.sol; both sides of the merge left those lines
alone, so there was no conflict marker on them and the tree compiled to
Error (9582): Member "buildFileForContract" not found or not visible after argument-dependent lookup. Threaded, with the same two-line comment the file's
other generating test already carries, saying this repo's own values stand in
where nothing reads the header. This is the whole reason the suite is run on the
merge commit rather than on either parent.

#123 renamed requireContractName to requireIdentifier and
InvalidContractName to InvalidIdentifier.
Conflicted with this branch's
import block, which had grown the two new error names. Both sides taken:
{InvalidIdentifier, InvalidSpdxLicenseIdentifier, InvalidCopyrightText, CodelessInstance}. assertNameRejected takes main's new selector and this
branch's argument list.

The remote branch head at 5c68149 is absorbed with git merge -s ours, not
force-pushed over.
It was an automated merge of the same origin/main@c8440fb
under a different resolution; it adds no commit this branch lacks. Two things
make this tree the one to keep: 5c68149 restores testFilePrefixNamesNoScript,
which is exactly the test the reasoning below says is subsumed, and it predates
the reorder of the two tests this PR owns to read → clean up → assert. The
ours-merge keeps it in the history and its reason in the commit message rather
than discarding it silently.

#126's forge-std prefix rewrite was a textual conflict. Both sides
edited the import block of test/src/lib/LibCodeGen.filePrefix.t.sol: main
moved Test from forge-std-1.16.1 to forge-std-1.16.2, this branch added the
two error imports and opened the REUSE-ignore region. Both sides are taken.
forge-std-1.16.1 appears nowhere in the tree outside dependencies/, out/
and cache/.

#112's restructure is where the parameters go now. buildFileForContract
is a four-argument wrapper delegating to a five-argument overload taking dir,
and the overload is where the content build, the unlink and the vm.writeFile
live. The two values
are threaded through both — the overload takes seven arguments and the wrapper
six — rather than through the flat function this branch was written against.
testBuildFileForContractCreatesTheDirectory, added on main by #112 and
driving the overload directly, passes them too.

#138 deleted script/Build.sol and src/generated/CodeGennable.sol. The
hunk that made the script pass this repo's own values was a modify/delete; the
deletion is taken and the library now has no in-repo caller.
testBuildFileForContractCommittedArtifactIsCurrent went with them — it read
the deleted artifact — so the two file-scoped constants it existed to pin,
SPDX_LICENSE_IDENTIFIER and COPYRIGHT_TEXT, are now documented as what they
actually are: this repo's own values standing in wherever a test does not care
which values it passes.

#56 moved every .t.sol into the test/src/ mirror. Both test files this
PR changes moved with it; the changes are on the new paths and no old path is
recreated.

testFilePrefixNamesNoScript is not restored. main dropped it in
95a1e89 as implied by testFilePrefixExact in the same file, and main's
docstring for testFilePrefixExact, which carries that reason, is what this
branch now has — with one clause added to say the pinned call passes this repo's
own values as the caller's, which is the only thing parameterisation changes
about it.

Every file this branch touches was diffed against main's version after the
merge. test/src/lib/LibFs.isPresent.t.sol differs from main by exactly the
four threaded call sites — two in the dangling-symlink test, two in #127's new
live-symlink test — and the same two-line comment above each pair saying why
this repo's own values stand in there. Nothing else in that file moves.

Decisions

Required parameters, not an overload keeping a Rain default. A default is
exactly the silent-wrong-header this finding is about: every consumer that never
looks at the new argument keeps emitting Rain's copyright and keeps passing
reuse lint. Required parameters make the compile error the migration notice.
This is also what #75's own proposed fix is — it shows a two-parameter
filePrefix with no default, and says that a single global header leaves the
finding unresolved until something asserts the consumer's repo licence agrees
with it. Ledger — build cost: this diff, plus two arguments at each of the 26
consumer call sites enumerated below, at the version each consumer chooses to
bump. Carrying cost: two extra arguments on a function that is already called
from a build script that has the values in front of it. Cost of removing it
later: reverting to a default would silently re-stamp Rain's holder on whatever
had not been migrated, so it is a one-way door in the correct direction.

Validation is in scope, and the issue's snippet has none. Measured, not
reasoned: a generated file whose SPDX tag has no value is refused by solc with
Error (1114): Invalid SPDX license identifier, so an empty value does not
produce a licensed file, it produces a repo that stops compiling — after the file
has been written and committed. A value carrying \n ends the tag's comment line
and writes a second, contradicting SPDX tag one line down, which a presence check
accepts. Both are the same "present but wrong" failure the finding names, one
level up, so the two guards ship with the parameters rather than after them.
0x0D counts with 0x0A: solc ends a // comment at either.

The revert lands before anything on disk is touched — #100 closed the window
this PR used to argue for leaving open.
The earlier version of this body argued
that a refused licence reverts with the path already unlinked, that the window
was not new because LibCodeGen.bytecodeHashConstantString's CodelessInstance
already reverted from the same argument list, and that closing it was #61's job,
in flight as #100. #100 has since merged, so there is no window left to argue
about: filePrefix is called building the content local, which happens before
vm.createDir, before the unlink and before the write. A refused licence or
copyright now leaves the directory exactly as it found it, on the same path
CodelessInstance takes. Nothing was hoisted to get this — the guards are where
they always were, inside filePrefix; #100 moved the call site out in front of
the disk mutation and both revert paths came with it.

testBuildFileForContractRejectsInvalidLicenceAndCopyright asserted
assertFalse(written, "a refused header still wrote a file") under the old
ordering and asserts it under the new one. It held before because the write was
never reached; it holds now because nothing at all is reached. The assertion did
not need changing, which is the useful signal: it was written against the
property, not against the ordering.

One //REUSE-IgnoreStart / //REUSE-IgnoreEnd region in
test/src/lib/LibCodeGen.filePrefix.t.sol
, opened after the imports and closed
at EOF. The tag prefixes now appear in source without a value attached, and the
whole subject of that file is emitted licence text, so per-assertion regions
would be false precision. It opens below the file's own header, so that header
still registers — reuse lint reports Files with license information: 51 / 51.
Measured on the merge commit by deleting the two markers and running
nix develop -c reuse lint: exit 1, Invalid SPDX License Expressions: 3, the
three being ", spdxLicenseIdentifier));, GPL-3.0-only" and
LicenseRef-DCL-1.0\n" — fragments of test source that happen to follow an
SPDX-License-Identifier: prefix. With the markers, exit 0 and
Invalid SPDX License Expressions: 0. Files with license information: 51 / 51
either way, which is the point of opening the region below the header rather
than above it.

Failing before, passing after

The change has two halves and each has its own red, both measured on the merge
commit.

The signature. src/lib/LibCodeGen.sol restored to main's version with the new
tests and everything else at HEAD, nix develop -c forge test:

Compiling 22 files with Solc 0.8.25
Solc 0.8.25 finished in 173.11ms
Error: Compiler run failed:
Error (2904): Declaration "InvalidSpdxLicenseIdentifier" not found in "src/lib/LibCodeGen.sol" (referenced as "src/lib/LibCodeGen.sol").
 --> test/src/lib/LibCodeGen.filePrefix.t.sol:6:1:
  |
6 | import {LibCodeGen, InvalidSpdxLicenseIdentifier, InvalidCopyrightText} from "src/lib/LibCodeGen.sol";
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Error (2904): Declaration "InvalidCopyrightText" not found in "src/lib/LibCodeGen.sol" (referenced as "src/lib/LibCodeGen.sol").
 --> test/src/lib/LibCodeGen.filePrefix.t.sol:6:1:
  |
6 | import {LibCodeGen, InvalidSpdxLicenseIdentifier, InvalidCopyrightText} from "src/lib/LibCodeGen.sol";
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The interpolation. Parameters present and ignored — the body still hardcoding
Rain's licence, which is M01 below:

Ran 23 test suites in 3.33s (34.61s CPU time): 182 tests passed, 3 failed, 0 skipped (185 total tests)

Failing tests:
Encountered 2 failing tests in test/src/lib/LibCodeGen.filePrefix.t.sol:LibCodeGenFilePrefixTest
[FAIL: assertion failed: // SPDX-License-Identifier: LicenseRef-DCL-1.0
// SPDX-FileCopyrightText: Copyright (c) 2026 S01 Issuer GmbH
[…] != // SPDX-License-Identifier: MIT
// SPDX-FileCopyrightText: Copyright (c) 2026 S01 Issuer GmbH
[…] testFilePrefixCarriesTheCallersLicenceAndCopyright() (gas: 11786)
[FAIL: assertion failed: // SPDX-License-Identifier: LicenseRef-DCL-1.0 != // SPDX-License-Identifier: X-r|$wQ_o lPgW?nx60_Y; counterexample: […]] testFilePrefixLinesAreTheCallersValues(bytes,bytes) (runs: 0, μ: 0, ~: 0)

Encountered 1 failing test in test/src/lib/LibFs.buildFileForContract.t.sol:LibFsBuildFileForContractTest
[FAIL: assertion failed: // SPDX-License-Identifier: LicenseRef-DCL-1.0
// SPDX-FileCopyrightText: Copyright (c) 2026 S01 Issuer GmbH
[…] != // SPDX-License-Identifier: MIT
// SPDX-FileCopyrightText: Copyright (c) 2026 S01 Issuer GmbH
[…] testBuildFileForContractCarriesTheCallersLicenceAndCopyright() (gas: 103002)

Encountered a total of 3 failing tests, 182 tests succeeded

[…] elides the rest of each assertion's two whole file contents and the fuzz
counterexample's calldata. Three tests fail and 182 pass: the caller's licence
does not reach the header, at both levels, and neither the exact-prefix pin nor
any other test in the suite notices — which is why the two "carries the caller's
values" tests exist. The copyright half is symmetric and is M02.

After the fix, nix develop -c forge test reports
185 tests passed, 0 failed, 0 skipped (185 total tests).

Mutation matrix

Run with mutation-probe, over the pushed tree at 63f727a — the head this PR
carries, not an earlier merge. It refuses to score anything until the baseline is
green (185 passed), requires each mutant's target text to occur exactly once in
its file, and scores a run with no tally from forge as NO-RUN rather than as a
survivor. cache/fuzz/failures is deleted before every run, so no verdict rests
on a counterexample replayed from a previous mutant.

Re-run in full after each main merge, because #100 and #127 both rewrote the
body of the function M10 and M13 mutate. Both target strings survived those
rewrites unchanged: LibCodeGen.filePrefix(spdxLicenseIdentifier, copyrightText),
moved from inside the vm.writeFile call to inside the content local and kept
its trailing comma, and the wrapper's delegation line was never touched. So all
13 are real runs, not silently skipped ones. That is the thing to check rather
than assume — a mutant whose target text no longer occurs does not run at all,
and reporting it as KILLED would be a matrix that lies. What rules it out here
is the probe's own "target must occur exactly once" precondition and the
no-run: 0 in the tally below, not reading the diff and hoping.

Mutant File Verdict Killed by
M01 filePrefix interpolates the literal LicenseRef-DCL-1.0 instead of spdxLicenseIdentifier LibCodeGen.sol KILLED testBuildFileForContractCarriesTheCallersLicenceAndCopyright, testFilePrefixCarriesTheCallersLicenceAndCopyright, testFilePrefixLinesAreTheCallersValues
M02 filePrefix interpolates the literal Rain copyright instead of copyrightText LibCodeGen.sol KILLED testBuildFileForContractCarriesTheCallersLicenceAndCopyright, testFilePrefixCarriesTheCallersLicenceAndCopyright, testFilePrefixLinesAreTheCallersValues
M03 isSingleLine stops rejecting the empty string LibCodeGen.sol KILLED testBuildFileForContractRejectsInvalidLicenceAndCopyright, testFilePrefixChecksLicenceFirst, testFilePrefixRejectsEmptyCopyright, testFilePrefixRejectsEmptyLicence
M04 isSingleLine stops rejecting 0x0A LibCodeGen.sol KILLED testBuildFileForContractRejectsInvalidLicenceAndCopyright, testFilePrefixRejectsLineBreakAtAnyPosition, testFilePrefixRejectsLineBreakInCopyright, testFilePrefixRejectsLineBreakInLicence
M05 isSingleLine stops rejecting 0x0D LibCodeGen.sol KILLED testFilePrefixRejectsLineBreakAtAnyPosition, testFilePrefixRejectsLineBreakInCopyright, testFilePrefixRejectsLineBreakInLicence
M06 isSingleLine never inspects the last byte LibCodeGen.sol KILLED testBuildFileForContractRejectsInvalidLicenceAndCopyright, testFilePrefixRejectsLineBreakAtAnyPosition, testFilePrefixRejectsLineBreakInCopyright, testFilePrefixRejectsLineBreakInLicence
M07 filePrefix drops the InvalidSpdxLicenseIdentifier guard LibCodeGen.sol KILLED testBuildFileForContractRejectsInvalidLicenceAndCopyright, testFilePrefixChecksLicenceFirst, testFilePrefixRejectsEmptyLicence, testFilePrefixRejectsLineBreakAtAnyPosition, testFilePrefixRejectsLineBreakInLicence
M08 filePrefix drops the InvalidCopyrightText guard LibCodeGen.sol KILLED testBuildFileForContractRejectsInvalidLicenceAndCopyright, testFilePrefixRejectsEmptyCopyright, testFilePrefixRejectsLineBreakAtAnyPosition, testFilePrefixRejectsLineBreakInCopyright
M09 filePrefix checks the copyright before the licence LibCodeGen.sol KILLED testFilePrefixChecksLicenceFirst
M10 buildFileForContract passes the two values to filePrefix in the wrong order LibFs.sol KILLED testBuildFileForContractBodyVerbatim, testBuildFileForContractCarriesTheCallersLicenceAndCopyright, testBuildFileForContractCreatesTheDirectory, testBuildFileForContractEmptyBody, testBuildFileForContractExactContent
M11 filePrefix swaps the two SPDX tag names LibCodeGen.sol KILLED testBuildFileForContractBodyVerbatim, testBuildFileForContractCarriesTheCallersLicenceAndCopyright, testBuildFileForContractCreatesTheDirectory, testBuildFileForContractEmptyBody, testBuildFileForContractExactContent
M12 isSingleLine never inspects the first byte LibCodeGen.sol KILLED testFilePrefixRejectsLineBreakAtAnyPosition
M13 the four-argument wrapper hands the two values to the overload in the wrong order LibFs.sol KILLED testBuildFileForContractBodyVerbatim, testBuildFileForContractCarriesTheCallersLicenceAndCopyright, testBuildFileForContractEmptyBody, testBuildFileForContractExactContent, testBuildFileForContractFreshPath

13/13 killed; survived: 0; no-run: 0; harness errors: 0.

Every name above is the probe's own killed_by list. The probe stops after the
first five names it extracts from a red run, so a five-name row is a witness
list, not the whole set of tests that kill that mutant. The fail-pattern is
anchored on [FAIL and matches lazily across newlines, so a fuzz test — whose
name forge prints after a bracket containing both args=[…] and the
counterexample — is named like any other, and a [PASS] line can never be
mistaken for a killer.

The test that could poison the next compile, and does not any more

The first matrix run of this work stopped producing results at M04 and produced
neither a summary line nor a compilation error — the signature of a harness that
is lying. Reproduced and diagnosed rather than worked around:
testBuildFileForContractRejectsInvalidLicenceAndCopyright used
vm.expectRevert, which ends the test at the call that is supposed to revert and
does not. Under M03 that call succeeds, vm.writeFile puts
src/generated/LibFsBuildInvalidHeader.sol on disk with an empty SPDX tag, and
the revert that expectRevert then synthesises does not undo a filesystem write.
The file survived git checkout -- src test, and every later run died at
Error (1114): Invalid SPDX license identifier before a single test ran.

Fixed in the test: the call is made through try/catch so control comes back
either way, every file is removed, and only then does anything assert. The revert
data is compared whole — selector and arguments — so nothing is weakened relative
to vm.expectRevert.

The same shape bit testBuildFileForContractCarriesTheCallersLicenceAndCopyright
in this run: under M01 it asserts before it cleans up, so
src/generated/LibFsBuildForeignLicence.sol was left behind by the pass. That
file is valid Solidity with a well-formed MIT tag, so it poisons nothing, but
it is untracked residue from a suite that is supposed to leave the tree as it
found it. That test now reads the file, removes it, and asserts afterwards.

Breaking change

14 repos pin rain-sol-codegen: rain.deploy 0.1.6, S01-Issuer/st0x.deploy
0.1.3 (issue #75 says 0.1.0; that is stale), and the other twelve at 0.1.0. Read
from each repo's foundry.toml on its default branch, across every repo in
rainlanguage and S01-Issuer.

Nothing breaks on merge. Soldeer pins are exact and every import carries the
version in its path prefix (rain-sol-codegen-0.1.0/src/lib/LibCodeGen.sol), so
an unbumped repo keeps resolving the zip it pinned. Each repo breaks only when it
chooses to bump, and migrates by forge soldeer update, rewriting the versioned
import prefixes, and passing its own SPDX identifier and copyright — which its
REUSE.toml and LICENSES/ already state.

Call sites that will need the two arguments, counted in each repo's default
branch:

Repo File Calls
rainlanguage/rainlang script/Build.sol 6
rainlanguage/raindex script/Build.sol 6
rainlanguage/dvin.deploy script/BuildPointers.sol 3
rainlanguage/rain.verify script/BuildPointers.sol 2
rainlanguage/rain.deploy src/lib/LibRainDeploySnapshot.sol 1 + 2 filePrefix()
rainlanguage/rain.flare script/Build.sol 1
rainlanguage/rain.merkle script/Build.sol 1
rainlanguage/rain.dia script/Build.sol 1
rainlanguage/rain.erc4626.words script/Build.sol 1
rainlanguage/rain.pyth script/Build.sol 1
rainlanguage/rain.math.float script/BuildPointers.sol 1
rainlanguage/rain.factory.deploy script/BuildPointers.sol 1
S01-Issuer/st0x.deploy script/BuildPointers.sol 1
rainlanguage/rainlang.interface 0

The cascade runs through rain.deploy. It is itself a published package that
calls this API from src/lib/LibRainDeploySnapshot.solbuildFileForContract
once and filePrefix() twice — and 15 repos pin rain-deploy. It has to
decide whether to take the two values as its own parameters or hardcode Rain's;
until it threads them through, st0x.deploy cannot reach a correct header no
matter what it passes — its ten committed pointer files are written through
LibRainDeploySnapshot, not through buildFileForContract. This PR is the first
half of that. No consumer repo is touched here.

Version step

This is a breaking API change and wants a minor step, and the version is not
edited in this PR. It is expressible but not through a workflow input:
rainix-autopublish.yaml's level reaches only the cargo path, while the soldeer
path publishes [package].version verbatim and derives the next as bump_patch
(rainix-static/src/soldeer_gate.rs), whose only invariant is
ver_gt(local, published). So 0.2.0 is a one-line hand edit to foundry.toml
that the gate accepts: it publishes 0.2.0, tags sol-v0.2.0, and bumps to
0.2.1. Left out deliberately — it is a release decision that belongs with the
merge, main is at 0.1.31 and moves on every merge, and every sibling PR in
flight would conflict on that line.

Found, not touched

S01-Issuer/st0x.deploy does not agree with itself about its own copyright: its
REUSE.toml and 68 files under src/ say Rain Open Source Software Ltd, while 6
— including src/concrete/ST0xOrchestrator.sol — say
Copyright (c) 2026 S01 Issuer GmbH. Parameterising here does not settle that;
someone has to decide what that repo declares before it can pass the right value.

isSingleLine accepts more than testFilePrefixAcceptsEveryNonLineBreakValue
reaches, and the name does not say so.
The guard refuses only 0x0A and
0x0D, so it accepts NUL, tab, the rest of the C0 controls, 0x7F and every
byte 0x800xFF. The test reaches only 0x200x7E, because textFromSeed
has to produce something the string cheatcodes can read back and arbitrary bytes
are not valid UTF-8. Whether the guard should refuse the other control
characters — a NUL or a tab in an SPDX tag is not a line break but is not a
usable licence identifier either — is a design decision, not a test gap, and is
left for a human. Widening the test's domain without settling that would only pin
the current behaviour harder.

Every generating test in LibFs.buildFileForContract.t.sol asserts before it
cleans up
, so any of them leaves its file in src/generated/ when it fails.
That is pre-existing and repo-wide, not introduced here; only the two tests this
PR adds are reordered, because those are the ones it owns. Visible in the matrix
run above: after 13 mutants the tree held nine untracked files under
src/generated/, all of them from tests that predate this PR
(LibFsBuildIdempotent.sol, LibFsBuildEmptyBody.sol, LibFsBuildOverwrite.sol,
LibFsBuildExact.sol, LibFsBuildInstance.sol, LibFsBuildBodyVerbatim.sol,
LibFsBuildFresh.sol, LibFsBuildSiblingA.sol, LibFsBuildSiblingB.sol), and
neither LibFsBuildForeignLicence.sol nor LibFsBuildInvalidHeader.sol — the
two this PR owns and reordered.

QA

  • Discriminating tests: testFilePrefixCarriesTheCallersLicenceAndCopyright and
    testBuildFileForContractCarriesTheCallersLicenceAndCopyright pin a foreign
    caller's values through to the header and assert this repo's own licence and
    copyright holder appear nowhere in the result;
    testFilePrefixLinesAreTheCallersValues fuzzes both values and reads the
    emitted prefix by splitting it on newlines, so tag text, tag order and line
    count are all pinned; testFilePrefixRejects{EmptyLicence,EmptyCopyright, LineBreakInLicence,LineBreakInCopyright,LineBreakAtAnyPosition} and
    testFilePrefixChecksLicenceFirst pin the guards and their order;
    testFilePrefixAcceptsEveryNonLineBreakValue pins that a printable-ASCII
    value is not refused — its name overclaims relative to that domain, see
    "Found, not touched"; testBuildFileForContractRejectsInvalidLicenceAndCopyright
    pins that the write inherits all of it and leaves no file behind.
  • Mutations applied: 13, listed with verdicts and killers in the matrix above —
    13/13 killed; survived: 0; no-run: 0; harness errors: 0, over a baseline the
    probe proved green at 185 passed before probing anything. M13 is new to this
    merge: it covers the wrapper-to-overload delegation that Cover the generated directory being created, by making the directory injectable #112's restructure
    created, which did not exist when this branch was written. The two documented
    ways a mutation run lies in this repo are both closed: cache/fuzz/failures is
    deleted before every run so no verdict rests on a replayed counterexample, and
    a run that produces no tally is scored NO-RUN rather than SURVIVED.
  • Oracle: the consumer repo's own REUSE.toml and LICENSES/, which is what the
    header has to agree with, plus solc's own SPDX rule for the empty case —
    Error (1114): Invalid SPDX license identifier, observed on a real generated
    file, not reasoned from the spec. The tests read the library's output rather
    than restate its implementation: the prefix is read with vm.split rather than
    rebuilt with the string.concat the library uses.
  • Category check: filePrefix() stamps this org's licence and copyright into other repos' generated files #75 asks that a generated file stop claiming this org's licence
    and copyright in repos that are not this org's. Covered at both levels
    (filePrefix and buildFileForContract), for both values, with validation so
    that "parameterised" cannot mean "parameterised into a tag that says nothing".
    The consumer census above is the rest of the answer — nothing outside this repo
    is touched, and foundry.toml is deliberately not bumped.
  • Suite, on the merge commit: nix develop -c forge test reports
    185 tests passed, 0 failed, 0 skipped (185 total tests) in 23 suites, against
    174 tests passed, 0 failed, 0 skipped (174 total tests) in 23 suites on
    main at dbcd5e1, measured the same way — 11 added, none lost.
    nix develop -c forge fmt --check exits 0 with no output.
    nix develop -c reuse lint exits 0 with Invalid SPDX License Expressions: 0
    and 51 / 51 files carrying both copyright and licence information.
    nix develop -c forge coverage reports src/lib at 100% lines (107/107),
    statements (125/125), branches (14/14) and functions (21/21).
  • Not re-run, because the tree it ran against is gone: the old QA cited
    forge script script/Build.sol regenerating src/generated/CodeGennable.sol
    with an empty git status --porcelain afterwards. Strip the script/Build.sol worked example, leave the library lean #138 deleted both. There is
    no in-repo generation to check for idempotence any more; what replaces it is
    testBuildFileForContractIdempotent, which asserts a second call over the same
    inputs produces the same bytes.

thedavidmeister and others added 3 commits August 16, 2026 18:45
filePrefix takes the SPDX licence identifier and the copyright text from the
caller and interpolates them verbatim, and buildFileForContract threads both
through. Each is required to be a non-empty single line, so the tags the
generated file carries say what they appear to.

Closes #75

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`vm.expectRevert` ends the test at the call that is supposed to revert and
does not, and a write that already reached disk is not undone by the revert
that follows it, so the file stays in `src/generated/` and solc refuses the
next compile with "Invalid SPDX license identifier". Catching the revert
instead keeps every assertion for after the file is removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Aug 16, 2026
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 28 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9c84ef5b-18d5-4234-81dd-1452da11e817

📥 Commits

Reviewing files that changed from the base of the PR and between dbcd5e1 and 63f727a.

📒 Files selected for processing (6)
  • src/lib/LibCodeGen.sol
  • src/lib/LibFs.sol
  • test/concrete/LibFsExternal.sol
  • test/src/lib/LibCodeGen.filePrefix.t.sol
  • test/src/lib/LibFs.buildFileForContract.t.sol
  • test/src/lib/LibFs.isPresent.t.sol

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

thedavidmeister and others added 11 commits August 16, 2026 19:27
`isSingleLine` in the test restated the library's own loop with `vm.contains`
and asserted it about `textFromSeed`'s output, which builds every byte as
`0x20 + (uint8(seed[i]) % 95)`. Provably printable ASCII, so the assertion
could never be false and never reached the library. The bare
`this.callFilePrefix(text, text)` stays: the does-not-revert claim is real and
nothing else holds it.

The four per-assertion `//REUSE-IgnoreStart`/`End` regions become one, opened
after the imports and closed at EOF. The whole subject of the file is emitted
licence text; per-assertion regions are false precision. The region opens below
the file's own header so that header still registers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Take #138's deletion of script/Build.sol and src/generated/CodeGennable.sol,
and drop testBuildFileForContractCommittedArtifactIsCurrent with them: it read
the deleted artifact.

Re-site the licence and copyright parameters into #112's five-arg
buildFileForContract overload, which is where the write now lives, and thread
them through the wrapper. testBuildFileForContractCreatesTheDirectory, added on
main, passes them too.

Take main's testFilePrefixExact docstring, which subsumes the
testFilePrefixNamesNoScript main deliberately dropped in 95a1e89.
A failing assertion ends the test where it stands, so asserting before the
cleanup leaves the written file in `src/generated/` for whatever runs next.
The file is read, then removed, and only then asserted on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts:
#	test/src/lib/LibCodeGen.filePrefix.t.sol
Four conflicts, all from main moving under this branch rather than from the
two sides disagreeing about the licence/copyright parameters.

script/Build.sol (modify/delete). Main stripped the worked example and the
artifact it produced. This branch's only change to that file was threading the
two new arguments through the example, so the deletion subsumes it. Deletion
taken; the QA note that `forge script script/Build.sol` regenerates
src/generated/CodeGennable.sol no longer applies, because neither file exists.

src/lib/LibFs.sol. Main split buildFileForContract into a GENERATED_DIR
convenience plus a `dir` overload that does the work. Both sides kept: the
convenience now takes the licence and copyright and forwards them, and the
`dir` overload carries them as parameters six and seven. The body had already
auto-merged to the union — main's createDir(dir) beside this branch's
filePrefix(spdxLicenseIdentifier, copyrightText).

test/src/lib/LibCodeGen.filePrefix.t.sol. Imports unioned onto main's
forge-std 1.16.2 bump. testFilePrefixNamesNoScript is KEPT: main deleted it on
the argument that pinning the whole prefix exactly refuses every string naming
a .sol file, which held while filePrefix() was a constant and does not once the
licence and copyright are the caller's. testFilePrefixExact's docstring says so
rather than repeating main's now-false subsumption claim.

test/src/lib/LibFs.buildFileForContract.t.sol. Imports unioned; LibCodeGen
itself dropped from them as no longer referenced.
testBuildFileForContractCommittedArtifactIsCurrent taken as deleted — it reads
src/generated/CodeGennable.sol off disk and that file is gone with the worked
example, so it cannot pass. Main's testBuildFileForContractCreatesTheDirectory
was left calling the 5-argument `dir` form and is updated to pass the two
values, which is what makes it compile.

Verified in the toolchain this checkout's CI uses, rainix 53e96a7 #sol-shell:
forge build clean, `forge test` 176 passed / 0 failed / 0 skipped,
`forge fmt --check` clean, `reuse lint` compliant at 51/51 files. No generated
file left behind in the working tree after the suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The automated merge at 5c68149 adds no commit this branch lacks - it is the
same origin/main@c8440fb merged with a different resolution. Two reasons this
tree wins:

- 5c68149 restores testFilePrefixNamesNoScript, which #99 deliberately removed
  as subsumed by testFilePrefixExact. filePrefix() is nullary and pure, so the
  exact pin is a real subsumption and the name assertion is dead weight.
- 5c68149 predates the reorder of testBuildFileForContractCarriesTheCallers-
  LicenceAndCopyright to read then clean up then assert, and the constant
  docstrings that went with it.
#100 rebuilt buildFileForContract so the whole file content is computed into
a local before any disk mutation. Resolved onto that ordering rather than
restoring compute-at-write: the local is built from
filePrefix(spdxLicenseIdentifier, copyrightText), so a refused licence or
copyright now reverts before createDir, the unlink and the write, exactly as
a codeless instance does. Its docstring paragraph is widened to say so, since
the build step this branch adds a revert path to is the one that paragraph is
about.

#100's testBuildFileForContractFailedBuildKeepsExistingFile is kept and
passes the two values like every other call site.
Comment-only. The clause added when resolving onto #100's ordering left the
paragraph wrapping mid-sentence at a short line.
#127 turned the unlink into `while (isPresent(...))` in the same function these
parameters run through. Taken as main has it: how many times the path is
unlinked is independent of what the header says.

#127's new testBuildFileForContractReplacesLiveSymlink merged with no conflict
marker and the wrong arity, because neither side touched those two lines. It is
threaded here, with the same comment the file's other generating test carries
about this repo's own values standing in where nothing reads the header. The
compiler is what caught it, not the merge.

#123 renamed requireContractName to requireIdentifier and InvalidContractName
to InvalidIdentifier, conflicting with the import block this branch had grown.
Both sides taken.
@thedavidmeister
thedavidmeister merged commit 84fd8ef into main Aug 17, 2026
4 checks passed
thedavidmeister added a commit that referenced this pull request Aug 17, 2026
Re-sites the orphaned-artifact refusal onto main's current `LibFs`:

- `buildFileForContract` is now the six-arg call applied to `GENERATED_DIR`
  over a seven-arg `dir` overload (#112), builds the whole file content
  before touching disk (#100), and unlinks in a `while` loop (#127). The
  check goes into the shared body, after `vm.createDir` because it is a read
  of that directory, and before the unlink so a refusal leaves the existing
  artifact where it found it.
- `requireNoOrphanedArtifact(vm, contractName)` is that check applied to
  `GENERATED_DIR`, over a private `requireNoOrphanedArtifactIn`, mirroring
  `pathForContract` / `pathForContractIn`. The overload reads the directory
  it writes into rather than always `GENERATED_DIR`.
- The test moves from `test/lib/` to `test/src/lib/` (#56), and its calls
  carry the licence and copyright `filePrefix` now takes (#135).
- `InvalidContractName` / `isContractNameSlow` are `InvalidIdentifier` /
  `isIdentifierSlow`, and forge-std is 1.16.2.
- The README's "Generated paths" section anchors ahead of "Formatter
  requirements": the worked-example section it sat under is gone (#138) and
  the publish section it appended to was rewritten (#140).

Drops the hand-set `[package].version = "0.2.0"` and the README paragraph
that justified it. Autopublish owns the version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

filePrefix() stamps this org's licence and copyright into other repos' generated files

2 participants