Caret pragma on test/lib/LibCodeGenSlow.sol - #104
Conversation
… wide `test/lib/LibCodeGenSlow.sol` is a library, so the org convention is a caret range rather than an exact pin. Adds `test/lint/PragmaConvention.t.sol`, which asserts the property the convention states over every first party `.sol` file: a file that declares a concrete contract pins its solc version, every other file carries a caret range. Closes #67 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 59 seconds 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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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. Comment |
…cter fix Resolves the foundry.toml conflict by taking main's `ffi = true` block and reverting this branch's three `fs_permissions` read grants (`src`, `test`, `script`) and their comment. Deletes test/lint/PragmaConvention.t.sol; the convention check is rehomed to rainlanguage/rainix#317. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test/lib/LibCodeGenSlow.sol, plus a repo wide lint for the conventiontest/lib/LibCodeGenSlow.sol
Closes #67
What changed
test/lib/LibCodeGenSlow.sol:3moves frompragma solidity =0.8.25topragma solidity ^0.8.25. It is a library file, and the org convention is^for library and abstract files,
=for concrete. That one character is theentire diff against
main.This can regress silently, and no test guards it
There is no test for this change, and there deliberately is not one.
solcis pinned to0.8.25infoundry.toml, so widening the accepted rangeon one file changes no bytecode, no ABI, no gas and no test outcome. There is
nothing for a test to observe. The consequence is stated plainly: this fix can
regress silently. Someone can re-pin this file to
=0.8.25, or add a newlibrary file that pins, and the suite,
forge fmt --checkand every CI job inthis repo will stay green and say nothing.
The convention check that would catch that is being rehomed to
rainlanguage/rainix#317(rainlanguage/rainix#317). It belongs there: the
pragma convention is an org rule spanning many repos, and a rule that every repo
must satisfy is a job for
rainix-static, not a Solidity test re-implemented ineach consumer. Until #317 lands, this convention is held by review alone.
What was removed from this PR
This PR previously carried its own repo wide lint. That is gone:
test/lint/PragmaConvention.t.soldeleted (164 lines, 5 tests). It walkedsrc,testandscriptwithvm.readDir, classified each file by its toplevel declaration keyword, and asserted the pragma operator matched. Its
subject was the repo's source tree as text — hand-rolled static analysis in
Solidity, which is exactly the shape
rainlanguage/rainix#317exists toabsorb.
fs_permissionsread grants it needed (src,test,script) andtheir explanatory comment are reverted.
fs_permissionsis back tomain'sblock exactly.
The fixer brief driving this fleet of PRs said "every fix ships a test,
verify-by-hand is never acceptable", with no exemption for changes that have no
behaviour. That was wrong and is corrected. It is what produced hand-rolled
static analysis in Solidity across nine PRs, this one among them.
QA
origin/mainis merged in (not rebased); the branch predated merged PRs #110and #124. The only conflict was
foundry.toml, resolved by takingmain'sffi = trueblock and dropping this branch's three read grants.Everything below ran with
nix develop -c, foundry from the flake.Full suite
nix develop -c forge testorigin/main(b422d97) in a clean worktree gives the same 142 tests in 18suites, which is the expected result: after the strip this branch adds no test
and removes none that
mainhas. Before the strip the branch ran 139 tests in17 suites against a 134-test
main; the five extra tests were the deleted lintfile's, and 5 is the drop.
forge fmt --checknix develop -c forge fmt --checkexits 0 with no diff.Mutation
Not applicable. The change alters no behaviour under the pinned compiler, so
there is no line whose mutation any test could observe. That is the same fact as
"no test guards it" above, stated from the mutation side.