Prove the traversal property over the identifier alphabets

[thedavidmeister]

Closes#58

What changed

testRequireContractNameAcceptedNamesCannotTraverse is deleted and replaced by
testRequireContractNameAlphabetCannotTraverse, plus an
assertNoFilesystemBytes helper alongside assertAccepted / assertRejected.

The property the old test named — an accepted name carries no byte that means
anything to a filesystem — now comes from two halves that each hold on every
run:

• testRequireContractNameMatchesAlphabet pins acceptance to membership of
  SLOW_HEAD_ALPHABET at the first byte and SLOW_TAIL_ALPHABET at every later
  one, over arbitrary bytes.
• testRequireContractNameAlphabetCannotTraverse asserts that neither of those
  alphabets contains /, \, . or nul.

Together those cover the whole accepted set. The empty-name half of the old
test's assertions is already carried by testRequireContractNameRejectsEmpty
and by isContractNameSlow returning false on the empty name.

Where the issue's proposed fix was wrong

Two corrections, both load-bearing, both with a mutant below that the proposed
version survives:

1. The proposal checked SLOW_TAIL_ALPHABET only. An accepted name draws
   its first byte from SLOW_HEAD_ALPHABET and nothing in the suite states that
   the head alphabet is contained in the tail one, so a tail-only assertion
   leaves the first byte of every accepted name unproven. Mutant M7: with
   the proposed tail-only body, putting a dot in SLOW_HEAD_ALPHABETpasses.
2. The proposal's loop is vacuous on an empty alphabet. A zero-length
   alphabet runs the loop zero times and reports a pass. assertGt(length, 0)
   is what turns that into a failure. Mutant M8: with the guard removed,
   emptying SLOW_TAIL_ALPHABETpasses.

QA

• Discriminating tests: testRequireContractNameAlphabetCannotTraverse - fails on
  each of M1-M6 below (mutants applied in this clone and run one at a time,
  Ran 1 test printed each time); it cannot fail on base because base is the
  state it asserts, so the discrimination is shown against the mutants rather
  than against an unmutated base.
• Mutations applied: SLOW_HEAD_ALPHABET gains . -> killed by
  testRequireContractNameAlphabetCannotTraverse; SLOW_TAIL_ALPHABET gains
  /, \ and nul -> each killed by the same test; SLOW_HEAD_ALPHABET emptied
  and SLOW_TAIL_ALPHABET emptied -> each killed by its assertGt guard;
  assert(false) at the top of LibCodeGen.requireContractName -> survived by
  the deleted testRequireContractNameAcceptedNamesCannotTraverse and killed by
  the other 14 tests in the file. Full table below.
• Oracle: SLOW_HEAD_ALPHABET and SLOW_TAIL_ALPHABET in
  test/lib/LibCodeGenSlow.sol, spelled out character by character, against the
  four bytes that mean something to a filesystem written as literals in the test.
  Neither side is derived from LibCodeGen's range arithmetic, so the two can
  only agree by being independently right.
• Category check: issue testRequireContractNameAcceptedNamesCannotTraverse asserts nothing on 95% of its runs #58 asks for the traversal property to be proven
  deterministically and completely rather than on the 5% of fuzz runs that get a
  name accepted; covered, with two corrections to the proposed fix (head alphabet
  as well as tail, and a non-vacuity guard) each backed by a mutant the proposed
  version survives.

Everything below was run in this clone with nix develop -c, foundry from the
flake, [fuzz] runs = 2048.

1. The finding reproduced — the old test asserts nothing on 96% of its runs

Instrumented the old test's two branches with vm.writeLine and ran it at
--fuzz-seed 1 (instrumentation reverted afterwards):

Ran 1 test for test/lib/LibCodeGen.requireContractName.t.sol:LibCodeGenRequireContractNameTest
[PASS] testRequireContractNameAcceptedNamesCannotTraverse(string) (runs: 2048, μ: 6248, ~: 5514)
--- counts ---
83 <- accepted, i.e. runs that asserted anything
1965 <- swallowed by the empty catch
2048 <- total

4.1% of the budget carried the property. (The issue measured 101/1947 on the
same seed; the vm.writeLine call perturbs the run, so the split moves a little.
Same conclusion either way.)

2. The finding reproduced — the untyped catch hides a totally broken library

Put assert(false) at the top of LibCodeGen.requireContractName so every call
reverts with Panic(0x01) and nothing is ever accepted, then ran the file:

[FAIL: Error != expected error: panic: assertion failed (0x01) != InvalidContractName("[")] testRequireContractNameAcceptedNamesAreIdentifiers(string)
[PASS] testRequireContractNameAcceptedNamesCannotTraverse(string) (runs: 2048, μ: 1874, ~: 1868)
[FAIL: panic: assertion failed (0x01)] testRequireContractNameAcceptsContractNames()
[FAIL: panic: assertion failed (0x01)] testRequireContractNameAcceptsGeneratedIdentifiers(bytes)
[FAIL: panic: assertion failed (0x01)] testRequireContractNameAcceptsIdentifierCharacters()
[FAIL: Error != expected error: ...] testRequireContractNameErrorCarriesTheName()
[FAIL: Error != expected error: ... != InvalidContractName("\0")] testRequireContractNameEveryLeadingByte()
[FAIL: Error != expected error: ... != InvalidContractName("A\0")] testRequireContractNameEveryTrailingByte()
[FAIL: Error != expected error: ...] testRequireContractNameMatchesAlphabet(bytes)
[FAIL: panic: assertion failed (0x01)] testRequireContractNameRangeBoundaries()
[FAIL: Error != expected error: ... != InvalidContractName("")] testRequireContractNameRejectsEmpty()
[FAIL: Error != expected error: ... != InvalidContractName("0Foo")] testRequireContractNameRejectsLeadingDigit()
[FAIL: Error != expected error: ...] testRequireContractNameRejectsOneBadByte(bytes,uint256,uint8)
[FAIL: Error != expected error: ... != InvalidContractName("Foo.sol")] testRequireContractNameRejectsOtherCharacters()
[FAIL: Error != expected error: ... != InvalidContractName("sub/Foo")] testRequireContractNameRejectsPathCharacters()
Suite result: FAILED. 1 passed; 14 failed; 0 skipped

The deleted test is the one that passes. It is the only test in the file that
cannot notice the function being wholly broken, which is what catch {} buys.
The 14 reds are also the answer to "does deleting it lose coverage" — it had
none of its own.

3. Mutation matrix for the new test

Each mutant breaks exactly one thing testRequireContractNameAlphabetCannotTraverse
asserts; the test is then run on its own. Ran 1 test / 1 tests passed is
printed for every row, so a survivor is a survivor and not a filter that matched
nothing.

#	mutant	new test
M0	none (baseline)	[PASS] ... (gas: 94674), Ran 1 test suite ... 1 tests passed
M1	. added to SLOW_HEAD_ALPHABET	killed — [FAIL: dot in the alphabet: 46 == 46]
M2	/ added to SLOW_TAIL_ALPHABET	killed — [FAIL: separator in the alphabet: 47 == 47]
M3	\ added to SLOW_TAIL_ALPHABET	killed — [FAIL: backslash in the alphabet: 92 == 92]
M4	nul added to SLOW_TAIL_ALPHABET	killed — [FAIL: nul in the alphabet: 0 == 0]
M5	SLOW_HEAD_ALPHABET emptied	killed — [FAIL: empty alphabet: 0 <= 0]
M6	SLOW_TAIL_ALPHABET emptied	killed — [FAIL: empty alphabet: 0 <= 0]
M7	M1, against the issue's tail-only body	SURVIVES — [PASS] ... (gas: 50983)
M8	M6, with the assertGt guard deleted	SURVIVES — [PASS] ... (gas: 42999)

M7 and M8 are the two deliberate survivors: they are the proposed fix without
the corrections above, and they are why the corrections are there.

4. Full suite and formatting after the change

Ran 16 test suites: 134 tests passed, 0 failed, 0 skipped (134 total tests)

$ nix develop -c forge fmt --check
(no output, exit 0)

The file keeps 15 tests: one deleted, one added.

---

Post-Build.sol-removal sweep (2026-08-17)

main (959d527) merged in, no conflict. Unaffected by #138's removal —
nothing cut. This PR is one test swap inside
test/lib/LibCodeGen.requireContractName.t.sol and touches nothing the removal
deleted.

Re-verified against main rather than restated:

• test/lib/LibCodeGenSlow.sol:20 and :24 still define SLOW_HEAD_ALPHABET
  and SLOW_TAIL_ALPHABET, and isContractNameSlow at :271/:275 still
  decides membership against them — so the new test still asserts over the
  alphabets the oracle actually uses. Share commentPrefix's rule in one place in LibCodeGenSlow #107 and test: parse the literal the library emitted, not one the test formatted #111 rewrote parts of that file
  while this branch was open and did not move these.
• testRequireContractNameMatchesAlphabet — the half this test's docstring
  leans on — is still present on main at :194.

Overlap with #123, stated with both numbers.#123 renames this exact file to
test/lib/LibCodeGen.requireIdentifier.t.sol and rewrites tests inside it. The
two will conflict as a rename-vs-modify whichever order they land in. Not
resolved here: #123 is held for sequencing against #56, which moves every
.t.sol in the repo.

Suite on the merge commit: Ran 19 test suites: 145 tests passed, 0 failed, 0 skipped — unchanged from main, which is right for a one-in-one-out swap.
forge fmt --check clean, git status clean after the run. forge coverage --no-match-coverage "test|script" still 100% lines / statements / branches /
funcs on all three src/lib files (79/79, 92/92, 10/10, 18/18) — the deleted
fuzz test called requireContractName and the new one does not, and nothing
drops.

[coderabbitai]

Warning

Review limit reached

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

Next review available in:55 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: 5a055c85-e21a-4e23-b9be-b653033933c0

📥 Commits

Reviewing files that changed from the base of the PR and between 959d527 and 308d750.

📒 Files selected for processing (1)

• test/lib/LibCodeGen.requireContractName.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

• X
• Mastodon
• Reddit
• LinkedIn

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

[thedavidmeister]

@coderabbitai review

[coderabbitai]

⚠️ 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.