Scope boolean-cst lint disables on the tuple-return bool literals - #588
Conversation
The rainix static job now runs `forge lint -D warnings`, which flags 21 `return (true|false, ...)` sites on unmodified main as boolean-cst. Each literal is the bool the function returns, not a condition operand, so each gets a scoped disable with that reason. Comments only: no code changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: rainlanguage/rainlang/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
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 |
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Closes #587
What
A scoped
//forge-lint: disable-next-line(boolean-cst)above each of the 21return (true|false, ...)sites thatforge lint -D warningsflags onmain. Each directive has a one-line reason: the literal is the bool the function returns, not a condition operand. 11 files, 42 added comment lines and nothing else. No code changes, so no bytecode changes.This is commit 864cf10 from #586, cherry-picked onto
mainunchanged. The diff againstmainmatches 864cf10's own diff hunk for hunk. #586 reverts that commit so that each PR closes one issue.Why
rainlanguage/rainix#378 added
forge lint -D warningsto the sharedrainix-sol-staticworkflow.mainhas not run CI since then, and any push now failsrainix-sol / staticon these 21 findings, whatever the push changes.boolean-csttargets a boolean constant used as a condition operand. Each flagged literal is the success/match flag in a returned tuple, and with named returns banned there is no other way to write it. rainlanguage/rainlang.interface#140 made the same call for the same shape inLibParseMeta.lookupWord, and rain.lib.memkv'stest/lib/LibMemoryKVSlow.solcarries the same disable.QA
forge lint -D warningsingithub:rainlanguage/rainix/8657b83b68f41957ab85da91132c3f652c1f32c0#sol-shell(the shell CI pins), afterforge soldeer install.main5874169: 21warning[boolean-cst], 0 of any other rule, thenaborting due to 21 linter warning(s), exit 1.forge fmt --checkexits 0.directive-mutation.sh.boolean-cst, at the exact line that directive guards: 21 pass, 0 fail. So no directive is redundant and none silences a neighbour.src/, one intest/) were repointed fromboolean-csttounsafe-typecast. All 3 let the original finding back, at the guarded line: 3 pass, 0 fail. The disables are per-rule, not blanket.git checkoutand the tree was checked clean. HEAD was still 1842c7d at the end.boolean-cstflags a boolean constant misused as a condition, for exampleif (x == true),require(true), or a constant operand of&&/||. Every flagged site returns a literal as the first element of a tuple whose declared type isbool, and the literal is the value the caller branches on. None is a condition operand. Precedents that made the same call independently: rainlang.interface#140 and rain.lib.memkvLibMemoryKVSlow.exists.src/and 11 intest/. Every one has a directive, and this branch has 21 directives (git grepcount;mainhas 0). The flagged set is the wholeforge lintoutput onmain, so no other rule or site is failing. The other static steps (slither .,pre-commit run --all-files,rainix-sol-single-contract) are covered by CI on this PR./home/gildlab/artifacts/rainlang/boolean-cst-lint/.🤖 Generated with Claude Code