Uh oh!
There was an error while loading. Please reload this page.
docs(LibRainDeploy): drop the return name from supportedNetworks' NatSpec - #124
Conversation
…already documents The `@return networks` tag sat above a signature whose return was unnamed, so `networks` was never a return-parameter name — solc had nothing to check it against and folded it into the description text. The name already existed in the body as a local, built and returned explicitly, so it was present everywhere except where it is checkable. Naming the return closes that gap. The body is a single straight-line block assigning `networks` on the only path to the function end, so the named return is locally provable and the explicit `return networks;` is redundant. This matches the form the rest of the file already uses. Returned value and compiled behaviour are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Warning Review limit reached
Next review available in:47 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 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 |
thedavidmeister
commented
Aug 15, 2026
@coderabbitai review |
|
…upportednetworks-named-return
…atSpec `/// @return networks` sat above `returns (string[] memory)` — unnamed — so `networks` was never a return-parameter name, solc had nothing to check it against, and the word was folded into the description text. Two forms close that gap: name the return, or drop the name from the tag. Drop it. `zoltuAddress` (src/lib/LibRainDeploy.sol:210-211) already documents its unnamed return as `/// @return The address the creation code deploys to.`, and issue 50's own text calls that form correct. This repo does not name return values: an unnamed return keeps solc's "unnamed return variable can remain unassigned" diagnostic, which naming it gives up — see #103, where the remedy for a named return's silent default is a line no test can kill. The signature, the local `string[] memory networks` and the explicit `return networks;` are unchanged. This reverts the code edit made earlier on this branch and fixes the docstring instead; the net change against `main` is one comment line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
supportedNetworks return its NatSpec already documentssupportedNetworks' NatSpecthedavidmeister
commented
Aug 16, 2026
Reviewed 03b2a23: ready — the fix now points the What changed since the last head
--- a/src/lib/LibRainDeploy.sol+++ b/src/lib/LibRainDeploy.sol@@ -246,7 +246,7 @@ library LibRainDeploy {
}
/// Returns the list of networks currently supported by Rain deployments.
- /// @return networks The list of supported network names.+ /// @return The list of supported network names.
function supportedNetworks() internal pure returns (string[] memory) {
string[] memory networks = new string[](5);
networks[0] = ARBITRUM_ONE;1 file changed, 1 insertion(+), 1 deletion(-). The signature, the local Why this closes #50 rather than dodging itBoth findings on that issue — It is also the form the same file already uses, 39 lines up: Checked the rest of the file's The point of keeping it unnamedsolc emits "Unnamed return variable can remain unassigned" for a body that can QA
Verification at this head
The last three were run locally because CI could not reach them (see below). The one red, and why it is not this PR
That step runs before slither,
All three steps that red masks were run locally instead, so nothing goes |
Uh oh!
There was an error while loading. Please reload this page.
Closes#50
The defect
supportedNetworkscarried/// @return networks The list of supported network names.abovefunction supportedNetworks() internal pure returns (string[] memory)— an unnamed return.
networkswas therefore not a return-parameter name atall: solc only checks an
@return <name>against a declared name, so with nonedeclared it silently folded
networksinto the description text and thegenerated docs read "networks The list of supported network names."
That is the whole finding. Issue 50 carries it twice —
LIBDEPLOY-09(dimension 4) and
D3-08(dimension 3), independently raised, same defect,severity INFO both times.
The fix, and why this direction
There are exactly two ways to make the tag and the signature agree: add the name
to the signature, or remove it from the tag. This repo does not name return
values, so the name comes out of the tag.
One comment line against
main. The signature, the localstring[] memory networksand the explicitreturn networks;are alluntouched.
This is the form the same file already uses 39 lines up:
zoltuAddress(src/lib/LibRainDeploy.sol:210-211) documents its unnamed returnas
/// @return The address the creation code deploys to.Issue 50's own textnames that as the correct handling of an unnamed return — it cites
zoltuAddressas the file's example of getting this right, not as the outlier.
What an unnamed return buys, and what naming it would have cost. solc warns
"unnamed return variable can remain unassigned" on a path that reaches the
closing brace without a
return. A named return has no such warning: it silentlyreturns the type's zero value. That is not hypothetical in this repo —
#103 exists because
suiteNames()'s named return was assigned only inside a loop, so an empty inputreturned
""with nothing to flag it. Its remedy is a barenames = "";seedwhose own QA records the mutant as SURVIVED: a line no test can kill,
because the path it protects is unreachable from anywhere the tests can stand.
The org rule is that a named return must be locally provable; keeping the
return unnamed here means there is nothing to prove, and the compiler keeps
doing the proving on every future edit to this body.
Naming the return would also have deleted
return networks;— the statementthat makes the function's exit value visible at the point of exit — in exchange
for nothing a caller can observe.
Note on this branch's history
The first commit on this branch (
9fa7433) applied the fix in the otherdirection: it named the return and dropped the local and the explicit
return.That direction was rejected — we do not name return values — so
03b2a23reverts that code edit and fixes the docstring instead. The branch's net diff
against
mainis the single comment line shown above; the two commits areretained rather than force-rewritten so the rejected direction stays on the
record.
QA
NatSpec comment. It produces byte-identical bytecode, is invisible at every
call site (
src/abstract/RainDeployBroadcast.sol,src/abstract/RainDeployVerifyChain.soland the test sites all consume thereturned array positionally), and no test can observe a comment. A test that
could fail on base would mean this was a behaviour change, which it is not.
Stating that plainly rather than inventing a test.
this diff to mutate. The function body is unchanged from
main, so itsexisting coverage (
testSupportedNetworks, test/src/lib/LibRainDeploy.t.sol,pinning length 5 and every element against the library constants) applies to
it unmodified and was neither weakened nor duplicated.
@return <name>is checked only against a declared return-parameter name, so an unnamed
return makes the tag's first word description text, which is exactly what the
issue reports. (b) The file's own correct instance of the unnamed form,
zoltuAddressat 210-211, which is the shape this change copies. (c) Theuser's standing ruling that return values are not named in this org, which is
what selects between the two otherwise-equivalent repairs.
signature agree — under two finding IDs with the same proposed fix. Both are
closed. The issue's proposed diff is deliberately not applied, because it
picks the named-return direction; the defect it identifies is fixed the other
way. Every other
@returnin the file was re-read while doing this and eachone already agrees with its signature (
isStartBlock112/113,findDeployBlock157/159,deployZoltu222/223,deployToNetworks418/419,deployAndBroadcast510/511 all name a declared return;zoltuAddress210/211 documents an unnamed one with no name), so no adjacent NatSpec was
touched.
Verification
mainmerged in (not rebased); no conflicts.nix develop -c forge fmt --check— clean, exit 0.nix develop -c forge testwith all five RPC endpoints set — 251 passed, 0failed, 0 skipped across 18 suites, fork tests included. Exit 0.
🤖 Generated with Claude Code