Delete the longhand copy of assertRejected

[thedavidmeister]

Closes#65

What changed

testRequireContractNameErrorCarriesTheName was assertRejected("sub/Foo")
written out longhand. Its two statements were byte-identical to the helper's
body with name substituted, and testRequireContractNameRejectsPathCharacters
already puts that exact string through the helper on its first line. Deleted.

The claim it carried — that the revert carries the rejected name — is a property
of assertRejected, so it now sits in that helper's docstring, where the check
actually lives. Every one of the suite's rejections goes through the helper, so
the claim is now stated once and asserted everywhere instead of stated once and
asserted twice in the same place.

Nothing in src/ changed.

The issue's proposed fix

Followed as written (delete the test, move the claim to the assertRejected
docstring). One correction to the issue's framing: it locates the file at
test/src/lib/LibCodeGen.requireContractName.t.sol, which is where PR #56 will
put it. #56 is still open, so this branch edits test/lib/ — the path on main
today. Whichever of the two lands second takes a rename-vs-edit merge, which is
a merge git resolves.

Evidence: a deletion needs the opposite of a failing test

There is no test that reproduces "this test is redundant" — redundancy is a
property of the suite, not of the code under test. The check that actually
settles it is the mutation run: if the deleted test killed a mutant nothing else
kills, the mutant survives after the deletion. So the mutation matrix below is
the whole proof, run before and after the change, and the failing-before /
passing-after shape is inverted: the mutants fail before and after, and what
must not change is which tests catch them.

Baseline, unmutated

Ran 16 test suites in 1.26s (12.46s CPU time): 134 tests passed, 0 failed, 0 skipped (134 total tests)

M1 — revert InvalidContractName(name) → revert InvalidContractName("") (src/lib/LibCodeGen.sol:52)

This is the mutant the deleted test existed to catch: the rejection stops
carrying the name.

Before the change, 15 tests in the suite, 10 kill it:

Suite result: FAILED. 5 passed; 10 failed; 0 skipped; finished in 81.88ms (180.48ms CPU time)
[FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("sub/Foo")] testRequireContractNameErrorCarriesTheName() (gas: 6235)
[FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("sub/Foo")] testRequireContractNameRejectsPathCharacters() (gas: 6578)
[FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("@")] testRequireContractNameRangeBoundaries() (gas: 16559)
[FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("0Foo")] testRequireContractNameRejectsLeadingDigit() (gas: 5385)
[FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("Foo.sol")] testRequireContractNameRejectsOtherCharacters() (gas: 6562)
[FAIL: ... InvalidContractName("\0")] testRequireContractNameEveryLeadingByte() (gas: 12273)
[FAIL: ... InvalidContractName("A\0")] testRequireContractNameEveryTrailingByte() (gas: 14058)
[FAIL: ... ] testRequireContractNameAcceptedNamesAreIdentifiers(string) (runs: 0)
[FAIL: ... ] testRequireContractNameMatchesAlphabet(bytes) (runs: 0)
[FAIL: ... ] testRequireContractNameRejectsOneBadByte(bytes,uint256,uint8) (runs: 0)

The two sub/Foo lines are the finding, printed by the compiler: the deleted
test and testRequireContractNameRejectsPathCharacters fail on the same input
with the same message.

After the change, 14 tests, 9 kill it — the same set minus the deleted one:

Suite result: FAILED. 5 passed; 9 failed; 0 skipped; finished in 72.75ms (226.84ms CPU time)
[FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("sub/Foo")] testRequireContractNameRejectsPathCharacters() (gas: 6600)

10 killers → 9 killers, count of tests 15 → 14. No mutant moved from killed to
survived, and sub/Foo is still caught.

M2 — revert InvalidContractName(name) → revert CodelessInstance(address(0)) (same line)

The deleted test also asserted the selector, so the selector axis gets its own
mutant. After the change, 9 of 14 kill it, the same 9:

Suite result: FAILED. 5 passed; 9 failed; 0 skipped; finished in 65.04ms (201.22ms CPU time)
[FAIL: Error != expected error: CodelessInstance(0x00...00) != InvalidContractName("sub/Foo")] testRequireContractNameRejectsPathCharacters()

Why the matrix is exhaustive, not just two samples

The deleted body was

vm.expectRevert(abi.encodeWithSelector(InvalidContractName.selector, "sub/Foo"));
this.callRequireContractName("sub/Foo");

and assertRejected's body is the same two statements with name in place of
the literal. testRequireContractNameRejectsPathCharacters:100 calls
assertRejected("sub/Foo"). Same call, same assertion, same argument, so the
set of mutants the deleted test could kill is equal by construction to a
subset of what line 100 kills — there is no mutant to search for. M1 and M2
exercise the two axes the assertion has (error argument, selector) and confirm
the construction empirically.

The three runs: 0 fuzz entries are fuzzers failing on their first case, not
zero-match filters: the suite compiled (Compiling 3 files with Solc 0.8.25,
Compiler run successful), ran 14/15 tests, and reported 5 passing alongside
the failures. A harness error would have reported neither.

QA

• Discriminating tests: testRequireContractNameRejectsPathCharacters — the surviving owner of the deleted test's claim. It fails on base under both mutants below, on the same "sub/Foo" input and with the same message the deleted test produced ([FAIL: Error != expected error: InvalidContractName("") != InvalidContractName("sub/Foo")]), verified by editing src/lib/LibCodeGen.sol:52 and re-running nix develop -c forge test --match-contract LibCodeGenRequireContractNameTest. No test was added: this diff deletes a duplicate, so the discriminating test it needs is the one that already existed.
• Mutations applied: src/lib/LibCodeGen.sol:52revert InvalidContractName(name) -> revert InvalidContractName("") -> killed by 10 of 15 tests before the change and 9 of 14 after (Suite result: FAILED. 5 passed; 10 failed -> Suite result: FAILED. 5 passed; 9 failed), the same killers minus the deleted test. src/lib/LibCodeGen.sol:52revert InvalidContractName(name) -> revert CodelessInstance(address(0)) -> killed by 9 of 14 after the change (Suite result: FAILED. 5 passed; 9 failed), same killer set, covering the selector half of the assertion. Nothing moved from killed to survived. Both mutants restored with git checkout -- src/lib/LibCodeGen.sol; the diff touches no src/ file.
• Oracle: the issue's own claim, checked structurally and empirically rather than against the implementation. Structurally, the deleted body is assertRejected's body with "sub/Foo" substituted for name, and testRequireContractNameRejectsPathCharacters:100 calls assertRejected("sub/Foo") — identical call, assertion and argument, so the mutants the deleted test can kill are equal by construction to a subset of what line 100 kills. Empirically, the killer counts above went 15 tests / 10 killers -> 14 tests / 9 killers with no survivor appearing.
• Category check: issue testRequireContractNameErrorCarriesTheName hand-rolls the assertRejected helper next to it #65 asks for two things — delete testRequireContractNameErrorCarriesTheName, and put the name-carrying claim in the assertRejected docstring where the check lives. Covered both; there is no third item, and nothing in src/ or in the sibling audit issues' files was touched.
• Suite: nix develop -c forge test is 134 tests passed, 0 failed, 0 skipped on unmutated base and 133 tests passed, 0 failed, 0 skipped after — exactly the one deleted test fewer, no other result moved. nix develop -c forge fmt --check exits 0 with no diff.

[coderabbitai]

Warning

Review limit reached

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

Next review available in:7 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: aec52fe1-1718-43a1-ae9e-1bbb834f0f9a

📥 Commits

Reviewing files that changed from the base of the PR and between 935c725 and 799babf.

📒 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.}