Make main pass the new forge lint and pre-commit static gates

[thedavidmeister]

rainix-sol-static.yaml gained forge lint -D warnings and
pre-commit run --all-files, and this repo consumes that workflow at @main.
Main's last CI run predates both, so main is green over code that fails one of
them. Six open PRs sit behind it.

forge lint -D warnings

-D is a compiler flag, so it denies solc's own diagnostics as well as
forge-lint's. Two Function state mutability can be restricted to pure
warnings therefore failed the compile, and the step aborted before
forge-lint ran at all — which is why the four findings behind them had never
been printed by any CI run.

finding	site	decision
solc 2018, view → pure ×2	LibRainDeploySnapshot.t.sol snapshotBytesConstant / snapshotAddressConstant	fixed
boolean-cst	LibRainDeploySnapshot.t.sol testTagPrecedesMatchesComponentOrder	fixed
missing-zero-check ×2	MockChainDependentOwner constructor	scoped disable
block-timestamp	MigrationRegistryApplyMigration.t.sol	scoped disable

Fixed — view → pure. Both helpers read a snapshot out of a string with
vm.contains/vm.split/vm.parseBytes/vm.parseAddress and assert. Nothing
they call touches state, so pure is what they are.

Fixed — boolean-cst. The tail of the tag-ordering oracle read
aPatch != bPatch ? aPatch < bPatch : false. When aPatch == bPatch,
aPatch < bPatch is already false, so the conditional is exactly
aPatch < bPatch and the constant is dead weight. The oracle is unchanged as
an expression; the fuzz test that consumes it is untouched.

Disabled — missing-zero-check. MockChainDependentOwner stores the two
addresses in immutables and answers one of them back from iOwner(). They
authorise nothing, and the fuzz tests that build it
(testCheckResolvedAddressesReadsEachCallSeparately,
testCheckResolvedAddressesDirtyWordNamesTheFailingRead) assume only
first != second, so the fuzzer reaches address(0) — a zero check would
revert runs the mock exists to serve. The hazard the rule names does not exist
here.

Disabled — block-timestamp. The flagged line is
assertTrue(sRegistry.applied(...) != block.timestamp), asserting that
applyMigrationHistory records the moment the CALLER supplied rather than the
block it landed in. The comparison against block.timestamp IS the assertion,
which is the case rainix's own comment on the step says takes a scoped disable.

pre-commit run --all-files

Run twice in the pinned sol-shell: every hook passes, no hook writes
anything, and git status is empty after both. The gate needs no change here,
so this PR carries no pre-commit commit — there was nothing for the hooks to
write. Covered surface is the 1 .nix, 8 .yaml, 3 .toml, 2 .md and 2
.rs tracked files; shellcheck skips (no shell scripts tracked) and JSON is
not gated in this shell.

QA

• Discriminating tests: n/a — no behaviour changes, so no test can discriminate. The discriminator is the gate itself: forge lint -D warnings exits 1 on main and 0 on this branch, both run in nix develop github:rainlanguage/rainix/8657b83b68f41957ab85da91132c3f652c1f32c0#sol-shell, the SHA the static workflow pins.
• Mutations applied: each of the five edits reverted ONE AT A TIME on this branch, each reproducing exactly its own finding and nothing else — snapshotBytesConstant pure→view → Warning (2018) at :667, compile fails; snapshotAddressConstant pure→view → Warning (2018) at :679, compile fails; disable line deleted from MockChainDependentOwner → missing-zero-check ×2 at :27:17/:27:39; disable line deleted from MigrationRegistryApplyMigration.t.sol → block-timestamp at :925:20; : false restored → boolean-cst at :1599:138. Branch with all five in place, and the branch restored after each revert, print nothing.
• Oracle: the gate commands themselves, run at the workflow's pinned rainix SHA rather than this repo's flake.lock — the findings are forge's and solc's output, not a judgement of mine.
• Category check: the two gates the workflow added are forge lint -D warnings and pre-commit run --all-files; both covered, lint by six findings resolved (2 solc + 4 forge-lint) and pre-commit by two clean runs over a stable tree. forge fmt --check re-run and green, since collapsing the ternary shortened the line and fmt then wanted it on one.

Every one of PRs #219, #226, #228, #229, #232 and #234 merges this cleanly, and
each is lint-clean and pre-commit-clean with it applied — none carries a
finding of its own.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN

Summary by CodeRabbit

• Tests

  • Improved test clarity by documenting intentional edge-case assertions and suppressing expected static-analysis warnings.
  • Refined test helpers and simplified validation expressions without changing expected results.
  • Confirmed migration history and ordering behavior continue to be validated correctly.

• Chores

  • Added supporting annotations for clearer linting and code review.
  • No changes to application or contract behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4093cdb4-3a53-4469-927a-eb5e905aebfd

📥 Commits

Reviewing files that changed from the base of the PR and between feefc95 and 408c0da.

📒 Files selected for processing (3)

• test/concrete/MockChainDependentOwner.sol
• test/src/concrete/MigrationRegistryApplyMigration.t.sol
• test/src/lib/LibRainDeploySnapshot.t.sol

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

Changes

Test maintenance

Layer / File(s)	Summary
Lint documentation test/concrete/MockChainDependentOwner.sol, test/src/concrete/MigrationRegistryApplyMigration.t.sol	Comments and lint-suppression directives document intentional zero-address and block.timestamp test patterns.
Snapshot test cleanup test/src/lib/LibRainDeploySnapshot.t.sol	Two helper functions now use pure. The precedence expression is simplified while preserving strict ordering.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 408c0

The PR contains localized test cleanup and lint documentation changes with no established functional or operational risk, so it is mergeable.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: updating the code to pass the new Forge lint and pre-commit static checks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-static-gates

---

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