Skip to content

fix(#6028): capture the full Solidity declaration signature, not just the name - #6029

Merged
cajasmota merged 2 commits into
cajasmota:mainfrom
Blueprint-Finance:fix/solidity-full-signatures
Jul 28, 2026
Merged

fix(#6028): capture the full Solidity declaration signature, not just the name#6029
cajasmota merged 2 commits into
cajasmota:mainfrom
Blueprint-Finance:fix/solidity-full-signatures

Conversation

@arthurgeron

@arthurgeron arthurgeron commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

declSignature scans forward from the declaration regex match with paren depth and stops at the first { or ; at depth zero, so the parameter list and any returns (...) clause are captured. The extractor previously stored the regex match span itself, which ends at the opening paren.

  • It does not repeat the comment and string handling in extractBracedBody. findContracts is fed stripCommentsAndStrings output, so a { or ; inside a comment or a literal cannot end a signature early.
  • The literal's contents are blanked too, so onlyRole("ADMIN)ROLE") is captured as onlyRole( ). The third test pins that rather than leaving it to be found in review.
  • A declaration with neither terminator falls back to the previous span, so truncated source keeps today's behaviour instead of running to the end of the contract.

Tests cover the multi-line, nested-paren and ;-terminated interface shapes, events, modifiers, and the comment/string case. Each fails against v0.1.9.

go test ./... green (224 packages), go vet and gofmt -l internal/extractors/solidity clean, and tools/coverage validate, backfill --check and fmt --check exit 0 with gen leaving docs/coverage byte-identical. Coverage matrix updated per AGENTS.md.

Closes #6028

functionRE, eventRE and modifierRE end at the opening paren, and the
extractor recorded the regex match span as Signature. Every Solidity
function, event and modifier therefore carried a signature truncated at
that paren, such as "function transfer(". Measured across seven
production Solidity repositories: 5255 declarations with a signature,
5255 truncated, none complete.

declSignature scans on from the match with paren depth and stops at the
first { or ; at depth zero, which takes in the parameter list and a
returns (...) clause in one pass. It does not repeat the comment and
string handling in extractBracedBody: findContracts is fed
stripCommentsAndStrings output, so a delimiter written inside a comment
or a literal is already blank and cannot end a signature early. A
declaration with no terminator falls back to the previous span, so
truncated source degrades to today's behaviour rather than running to
the end of the contract.

On the same seven repositories the change affects one field and nothing
else: entity ids are unchanged, no entity differs in any field other
than signature, and all 745 cross-repository edges derived from the
graph are identical.
The capability entry was added before the issue existed, so
issues_implemented was empty.
@cajasmota

Copy link
Copy Markdown
Owner

Thank you for this — it's a genuinely nice piece of work, and a few things stand out.

The diagnosis is exact. "The regexes end at the opening paren" is the whole bug in one line, and noting that SCOPE.Component was never affected because contractRE runs to the { is the kind of detail that saves the next person from widening the fix past where it belongs.

declSignature is the right shape: one pass, paren depth, stop at the first { or ; at depth zero. That picks up the parameter list, the mutability/visibility keywords and the returns (...) clause without special-casing any of them. And the forEach(function(uint256) external returns (bool) cb) test is exactly the case I'd have wanted to see — a function-type parameter is precisely where naive paren matching falls over, and you covered it without being asked.

Two judgement calls I particularly appreciated:

You documented the limitation instead of hiding it. onlyRole("ADMIN)ROLE") coming out as onlyRole( ) is a real consequence of scanning scrubbed source, and you wrote it down and pinned it with a test. A known, tested limitation is worth far more than an undocumented edge case someone trips over in six months.

The fallback degrades instead of running away. A declaration with no terminator returning the previous span means malformed source gets the old behaviour rather than swallowing the rest of the contract. That's the conservative choice and it's the right one.

The before/after across seven production repositories — 5255 truncated to 0, entity ids unchanged, no field other than signature differing — is more evidence than most changes this size arrive with. Thank you for measuring it rather than asserting it.

And thank you for updating docs/coverage/registry.json and the capability map, including keeping the registry cell's original "issue": "5371" and only touching notes and verified_at. That step gets missed often, and getting it exactly right without prompting is appreciated.

Merging as-is. Nothing needed from you.

@cajasmota
cajasmota merged commit d5d0fa7 into cajasmota:main Jul 28, 2026
@arthurgeron
arthurgeron deleted the fix/solidity-full-signatures branch July 28, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

solidity: function, event and modifier Signature is truncated at the opening paren

2 participants