fix(shell,perl): args-counting for languages with no formal parameter-list syntax - #1520
Merged
Merged
Conversation
…-list syntax Bash functions and traditional Perl subs have no formal parameter-list syntax at all, permanently, by grammar -- args arrive via body-level idioms instead ($1/$2/"$@" in bash; my(...)=@_ / shift in perl). This was scored 0/3 (shell) and 137/937 (perl, 14.6%) on args_exact_match, reading as "GitGalaxy's args regex is broken." It wasn't -- both the ground-truth measurement and the engine had real, fixable gaps: Ground truth (tests/tools/tree_sitter_accuracy_audit.py): the original _get_param_count only checked the declaration for a formal parameter field, found nothing (correctly -- there is none), and reported real=0 regardless of true arity. New _count_shell_real_positional_max / _count_perl_real_args walk the same body idiom GitGalaxy reads, using tree-sitter's own real parse. Engine (detector.py + language_standards.py): - shell: args-counting only ever took the first $N/"$@" match in a function, silently dropping every reference after it. New _args_findall_max_groups routing scans the whole block and takes the max positional index referenced. - perl: only counted the first my(...)=@_ or shift statement, missing the extremely common "shift the invocant, then unpack the rest" multi-statement pattern. New _args_findall_sum_groups routing sums every matching statement across the block. Tightened the bare "shift" alternative to exclude shift @other/shift(@other) (shifting a DIFFERENT array, not @_). Added the bare "my @array = @_" variadic catch-all idiom, previously unrecognized entirely. args_exact_match: shell 0/3 -> 3/3 (100%), perl 137/937 -> 769/937 (82.1%). Also documents why this isn't "GitGalaxy loses to tree-sitter" -- docs/why_gitgalaxy_beats_ast_here.md, linked from README.md. For this one signal, in this one circumstance (no formal signature exists), a declaration-only AST read has nothing to count and can only report 0; GitGalaxy's body-aware read is the more informative measurement. Two related findings filed as separate follow-ups, not fixed here: #1517 (perl func_start block-slicing swallows 432 extra lines for one real corpus function, pre-existing and unrelated to args-counting) and a documented, not-fixed nested-anonymous-sub overcounting edge case (see #1519). Fixes #1518, #1519. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bash functions and traditional Perl subs have no formal parameter-list syntax at all, permanently, by grammar -- args arrive via body-level idioms instead (
$1/$2/"$@"in bash;my (...) = @_/shiftin perl). This measured 0/3 (shell) and 137/937 (perl, 14.6%) onargs_exact_match, reading as "GitGalaxy's args regex is badly broken." It wasn't -- both the ground-truth measurement and the engine had real, fixable gaps.Ground truth (
tests/tools/tree_sitter_accuracy_audit.py):_get_param_countonly ever checked the declaration for a formal parameter field, found nothing (correctly -- there is none), and reportedreal=0regardless of true arity. New_count_shell_real_positional_max/_count_perl_real_argswalk the same body idiom GitGalaxy reads, using tree-sitter's own real parse.Engine (
detector.py+language_standards.py):$N/"$@"match in a function, silently dropping every reference after it. New_args_findall_max_groupsrouting scans the whole block and takes the max positional index referenced.my (...) = @_orshiftstatement, missing the extremely common "shift the invocant, then unpack the rest" multi-statement pattern (e.g.bugzilla/Bug.pm::ValidateDependencies, 3 sequential shifts). New_args_findall_sum_groupsrouting sums every matching statement across the block. Tightened the bareshiftalternative to excludeshift @other/shift(@other)(shifting a DIFFERENT array, not@_). Added the baremy @array = @_variadic catch-all idiom, previously unrecognized entirely.args_exact_match: shell 0/3 → 3/3 (100%), perl 137/937 → 769/937 (82.1%).Why this isn't "GitGalaxy loses to tree-sitter"
New doc:
docs/why_gitgalaxy_beats_ast_here.md, linked from README.md's "One Graph, Not Five Separate Tools" section. For this one signal, in this one circumstance (no formal signature exists), a declaration-only AST read has nothing to count and can only ever report 0 -- correct, but useless for coupling/complexity scoring. GitGalaxy's body-aware read is the more informative measurement here, not a lower-precision tradeoff. Scoped narrowly -- doesn't apply to languages with real formal signatures, where tree-sitter's ground truth is still the fair, correct comparison (see the six prior fixes cited in the same doc).Two related findings, filed separately, not fixed here
func_startblock-slicing bug:mojo/Template.pm::_line(a real 4-line sub) measures as spanning 437 lines through the fullgalaxyscopepipeline, but correctly as 4 lines via a direct_function_slice()call. Confirmed pre-existing (present inmain's committed golden master before this PR) and unrelated to args-counting -- this PR's fix just made its symptom more visible (the phantom-huge block's args count jumped from a plausible-looking 1 to a clearly-wrong 13, since summing now scans the whole -- wrongly huge -- block). Root cause not pinned; likely candidate isprism.py's string-shielding for theqq{...@{[expr]}...}delimiter-collision shape, not confirmed._count_perl_real_args's own docstring and issue perl args: traditional subs have no formal signature -- ground truth only checked the declaration, and GitGalaxy's own shift/my@_ counting only saw the first statement #1519): a closure's ownmy (...) = @_gets summed into the OUTER sub's count too, since the engine's flat regex scan has no real block-nesting awareness the ground-truth walker has. Known, accepted limitation.Verification
Fixes #1518, #1519.
🤖 Generated with Claude Code