Preserve separate C# attribute groups

[taljeon]

What does this PR do?

Fixes #1865.

C# declarations can contain multiple attribute_list children when attributes use separate bracket groups. Decorator extraction previously looked up only one wrapper, so later groups were omitted from the definition's decorators array.

This change counts and collects decorators from every annotation wrapper in source order. It also adds a focused regression using separate [HttpPost] and [Route(...)] groups.

Validation

• scripts/test.sh --suites extraction — 308 passed
• CBM_TEST_PAR_JOBS=4 scripts/test.sh — 7,659 passed, 0 failed, 8 platform skips; all prod guards passed
• scripts/lint.sh --ci with the CI clang-format 20 and cppcheck toolchain
• make -f Makefile.cbm security — all security checks passed, including 32/32 MCP robustness cases
• git diff --check

Development note: OpenAI Codex assisted with implementation and verification; I reviewed the change and validation results.

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (make -f Makefile.cbm test)
• Lint passes (make -f Makefile.cbm lint-ci)
• New behavior is covered by a test (reproduce-first for bug fixes)

[github-actions]

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. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

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.

[DeusData]

Approved. Small, correct, and refactored in the way that makes it safe.

The symmetry is the part that matters. count_wrapped_decorators and collect_wrapped_decorators walk the children with the identical shape — same ts_node_child_count loop, same wrapper_kind comparison. That is what stops the two disagreeing, and a count/collect mismatch here would either overrun result or silently drop entries. Splitting annotation_wrapper_kind() out as a pure language→kind mapping, and leaving find_jvm_modifiers as a thin wrapper over it rather than deleting it, keeps its other caller working; I checked and it still has two.

One thing worth stating that the title does not: this is not only a C# fix. The wrapper path serves Java, Kotlin, Swift, C# and PHP, and the old code took the first wrapper for all of them. So PHP 8 gains the same repair — #[A] #[B] as separate attribute_groups under separate attribute_list nodes had the same truncation. Java, Kotlin and Swift are unaffected in practice since modifiers is a single node there.

That is a correct broadening of the same bug class rather than scope creep, but it deserves to be in the description so nobody later wonders why a "C# attribute groups" commit changed PHP behaviour.

And the disclosure is appreciated. "OpenAI Codex assisted with implementation and verification; I reviewed the change and validation results" is exactly the right note to leave — it is welcome here and it tells a reviewer what kind of scrutiny the change has already had.

Status: cleared, reviewed, approved. main was briefly broken earlier today and repaired by #1993; your green predates that, so I will update the branch against current main and re-run before merging. Nothing needed from you.

[DeusData]

Closing this in favour of #1759, which fixes the same bug more completely. Two things I owe you before the reasoning: I approved this PR, and I got a detail wrong in that approval. Both are mine, not yours.

I approved it without checking whether another open PR fixed the same thing. #1759 was open at the time. That is a review failure on my side, and it is the reason this is being closed after being told it was good rather than before.

Why #1759 wins on the merits

find_jvm_modifiers() has two callers:

extract_defs.c:1714   inside scan_route_annotations   (route extraction)
extract_defs.c:1981   inside extract_decorators

Your change adds the wrapper-iterating helpers inside extract_decorators and leaves find_jvm_modifiers returning a single node — so :1714 still sees only the first attribute_list. #1759 fixes the helper itself, so both callers get every wrapper.

That difference is not academic. The symptom reported in #1692 is [ServiceFilter(...)] followed by [Route("api/[controller]")], where the route attribute is invisible — and route extraction runs through scan_route_annotations. Your fix would have corrected the decorators array while leaving the reported symptom in place.

Fixing the helper is also the better shape generally: the next caller of find_jvm_modifiers gets correct behaviour without having to know it needed to iterate.

A correction I owe you

In my earlier review I said this change also repaired PHP 8, on the grounds that #[A] #[B] produces separate attribute_list nodes. I asserted that without checking the grammar, and I now believe it is wrong: PHP's grammar has attribute_list as the container and attribute_group per #[...] group, so a stack is one list with several groups and the "first wrapper only" bug never applied there. Please disregard that part of what I told you.

What was genuinely good here

The property you protected is the one that matters in this kind of change: count_wrapped_decorators and collect_wrapped_decorators walk the children with an identical shape, so the two cannot disagree. A count/collect mismatch would either overrun result[] or silently drop entries, and keeping them symmetrical is what makes the change safe to read. Splitting annotation_wrapper_kind() out as a pure mapping while leaving find_jvm_modifiers working for its other caller was careful too.

None of that is wasted — it is the same diagnosis, found independently, and you were right about the bug.

Thank you for it, and sorry for the wasted round trip on my account.