fix(extract): scan every attribute_list sibling, not just the first (#1692) - #1759
fix(extract): scan every attribute_list sibling, not just the first (#1692)#1759Jumaga2015 wants to merge 3 commits into
Conversation
A C# attribute stack ([Foo][Bar][Baz]) compiles to separate sibling attribute_list nodes, one per bracket group — not one attribute_list holding several entries. find_jvm_modifiers() used ts_node_child_by_field_name(), which only ever returns the first child registered under a given field, so every attribute after the first bracket group was silently dropped and never produced a DECORATES edge (DeusData#1692). find_jvm_modifiers() now returns every matching wrapper (count + out array, capped at MAX_ATTR_WRAPPERS) instead of a single TSNode, and its two callers (extract_decorators, scan_route_annotations) iterate all of them. The multi-match child scan lives in a new helper, cbm_find_children_by_kind() (helpers.c/helpers.h), the multi-match sibling of the existing cbm_find_child_by_kind() — keeps find_jvm_modifiers() a short per-language dispatch that delegates the traversal, same shape it had before this fix. Both callers size `wrappers` at MAX_ATTR_WRAPPERS. scan_route_annotations appends the owner node after the wrappers, so its fill is capped one lower (MAX_ATTR_WRAPPERS_MINUS_1) rather than the array being declared one larger — the MAX_X / MAX_X_MINUS_1 pair this file already uses for MAX_BASES, MAX_PARAMS and MAX_RETURN_TYPES. No-op for the other languages on this switch: Java/Kotlin/Swift use the `modifiers` wrapper, a single node that never repeats, and PHP groups a `#[A] #[B] #[C]` stack under ONE attribute_list holding three attribute_group children, which the previous lookup already returned whole. Signed-off-by: Jumaga2015 <jumaga2015@gmail.com>
A C# method carrying three stacked attributes ([Foo][Bar("x")][Baz]) must
end up with three DECORATES edges, one per bracket group. The fixture is
indexed through the real production pipeline (rh_index) and asserts
rh_count_edges(..., "DECORATES") == 3, so it exercises the MCP path rather
than the extractor in isolation.
Verified to actually reproduce: with find_jvm_modifiers() reverted to its
first-child lookup the suite goes red (decorates=1, expected 3), and green
again with the fix in place.
C#-only by design. Parsing the equivalent fixtures with the vendored
grammars shows `[A] [B] [C]` yields three sibling `attribute_list` nodes in
C#, while PHP's `#[A] #[B] #[C]` yields ONE `attribute_list` holding three
`attribute_group` children — the pre-fix lookup already found all of them,
so PHP could not hit this bug and a stacked-PHP fixture would pass either
way. Java/Kotlin/Swift use the `modifiers` wrapper, a single node that never
repeats. PHP's DECORATES path stays covered by mkc_c3_php8_attribute
(tests/test_matrix_known_classes.c).
Registered in repro_main.c and Makefile.cbm's TEST_REPRO_SRCS.
Signed-off-by: Jumaga2015 <jumaga2015@gmail.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Thank you for keeping the ASP.NET route-extraction half separate from the Kafka classification defect. I checked current main: find_jvm_modifiers() obtains the matching wrapper with ts_node_child_by_field_name(), so only the first repeated C# attribute_list can reach decorator and route scanning. That grounds the stacked-attribute failure. I have labeled this as a high-priority parsing bug and routed it for review. The small Makefile change is only the focused repro registration, not a broader build-system redesign. Our review queue is full, so detailed review may take a little time. Thank you for splitting the two #1692 defects and covering the production pipeline path. |
|
Approved, and this is now the surviving fix for the stacked-attribute bug. #1895 addressed the same defect and has been closed in favour of this one — its author found the bug independently and their diagnosis was right; yours is the more complete repair. Fixing the helper rather than one caller is why. The other PR iterated wrappers inside Your version fixes the source, so both callers are correct and the next one is correct without having to know it needed to iterate. Scoping to C# with a stated reason is right. Java, Kotlin and Swift use the And the repro is the right shape. Splitting the Kafka misclassification out as its own atomic fix, as the issue thread asked, was also the right call. StatusClearance is
|
Brings the branch onto current main (fe85a6b) as promised on the PR: the only conflict was internal/cbm/helpers.h, where both sides added declarations after cbm_find_child_by_kind; both blocks are kept (the PR's cbm_find_children_by_kind prototype, then main's Lisp-family gates). Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
As promised on 09-01, I've updated this branch myself rather than asking you to: 683d4bc merges current main into it. The only conflict was internal/cbm/helpers.h, where both sides had added declarations after cbm_find_child_by_kind — both blocks are kept, your cbm_find_children_by_kind prototype first. Your two commits are untouched. Verified on the merged tree before pushing: the #1692 repro is red with main's extract_defs.c and green with yours, and extraction plus the Java/Kotlin/PHP/C# LSP suites and pipeline are green (1,472 cases). CI runs now; I'll merge on green. Nothing needed from you. |
Summary
misclassification half, which is a separate call-resolution defect left
for its own atomic fix as requested in the issue thread): a C# method or
class with 2+ stacked attributes only got a
DECORATESedge for thefirst bracket group.
find_jvm_modifiers()usedts_node_child_by_field_name(), which returns only the first matchingchild, so a route attribute stacked behind another one (e.g.
[ServiceFilter(...)]then[Route("api/[controller]")]) was silentlyinvisible to
references/symbolsand to route extraction.find_jvm_modifiers()now returns every matching wrapper via a newcbm_find_children_by_kind()helper; both callers (extract_decorators,scan_route_annotations) iterate all of them.modifierswrapper (never repeats);PHP already groups a stack under one
attribute_list.Test plan
repro_issue1692(Makefile.cbmTEST_REPRO_SRCS) — indexes[Foo][Bar("x")][Baz]through the real production pipeline andasserts
DECORATES== 3. Proven RED with the pre-fix lookup(decorates=1), GREEN with the fix.
scripts/test.sh CC=clang CXX=clang++— full suite, no regressionsscripts/lint.sh CC=clang CXX=clang++— clang-tidy (diff-scoped),clang-format, cppcheck all clean
Made with Claude Code