Skip to content

Commit 5b8217d

Browse files
authored
Merge pull request #1759 from Jumaga2015/fix/1692-csharp-multi-attribute-decorates
fix(extract): scan every attribute_list sibling, not just the first (#1692)
2 parents c43ea2a + fbe9eae commit 5b8217d

6 files changed

Lines changed: 145 additions & 29 deletions

File tree

‎Makefile.cbm‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,7 @@ TEST_REPRO_SRCS = \
707707
tests/repro/repro_issue787.c \
708708
tests/repro/repro_issue842.c \
709709
tests/repro/repro_issue964.c \
710+
tests/repro/repro_issue1692.c \
710711
tests/repro/repro_invariant_calls.c \
711712
tests/repro/repro_invariant_graph.c \
712713
tests/repro/repro_invariant_breadth.c \

‎internal/cbm/extract_defs.c‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#define MAX_PARAMS_MINUS_1 31
2424
#define MAX_RETURN_TYPES 16
2525
#define MAX_RETURN_TYPES_MINUS_1 15
26+
#define MAX_ATTR_WRAPPERS 16 // stacked C#/PHP attribute_list siblings per declaration (#1692)
27+
#define MAX_ATTR_WRAPPERS_MINUS_1 15
2628

2729
// Tree traversal limits.
2830
enum {
@@ -1323,7 +1325,7 @@ static const char *extract_docstring(CBMArena *a, TSNode node, const char *sourc
13231325
return NULL;
13241326
}
13251327

1326-
static TSNode find_jvm_modifiers(TSNode node, CBMLanguage lang);
1328+
static int find_jvm_modifiers(TSNode node, CBMLanguage lang, TSNode *out, int max);
13271329

13281330
/* HTTP method names recognized in decorator calls (e.g., @router.post → "POST") */
13291331
static const char *decorator_method_name(const char *attr_text) {
@@ -1713,12 +1715,9 @@ static void scan_route_annotations(CBMArena *a, TSNode owner, const char *source
17131715
*out_method = NULL;
17141716
*out_jax_path = NULL;
17151717

1716-
TSNode wrappers[2];
1717-
int wn = 0;
1718-
TSNode modifiers = find_jvm_modifiers(owner, spec->language);
1719-
if (!ts_node_is_null(modifiers)) {
1720-
wrappers[wn++] = modifiers;
1721-
}
1718+
/* MINUS_1: the owner node itself is appended below, after the wrappers. */
1719+
TSNode wrappers[MAX_ATTR_WRAPPERS];
1720+
int wn = find_jvm_modifiers(owner, spec->language, wrappers, MAX_ATTR_WRAPPERS_MINUS_1);
17221721
/* Direct-child annotations (some grammars attach the annotation as a child
17231722
* of the method node rather than under `modifiers`). */
17241723
wrappers[wn++] = owner;
@@ -1872,13 +1871,23 @@ static int count_modifier_annotations(TSNode modifiers, const CBMLangSpec *spec)
18721871
return count;
18731872
}
18741873

1875-
// Find the wrapper child that holds annotations/attributes for languages where
1876-
// they are nested under an intermediate node rather than being a prev-sibling:
1877-
// Java/Kotlin/C#/Swift → `modifiers` (contains annotation/attribute)
1878-
// PHP 8 → `attribute_list` (contains attribute_group)
1879-
// Returns a null node when the language has no such wrapper.
1880-
static TSNode find_jvm_modifiers(TSNode node, CBMLanguage lang) {
1881-
TSNode null_node = {0};
1874+
// Find every wrapper child that holds annotations/attributes for languages
1875+
// where they are nested under an intermediate node rather than being a
1876+
// prev-sibling:
1877+
// Java/Kotlin/Swift → `modifiers` (one node, contains every annotation)
1878+
// C#/PHP 8 → `attribute_list` (contains attribute/attribute_group)
1879+
//
1880+
// C#/PHP attribute stacks are NOT a single wrapper: each bracketed group
1881+
// (`[Foo]`, `[Bar]`, ...) compiles to its own `attribute_list` node, so
1882+
// `[A] [B] [C]` above a declaration produces three separate `attribute_list`
1883+
// siblings among that declaration's children — not one `attribute_list`
1884+
// holding three entries. A field-name lookup (`ts_node_child_by_field_name`)
1885+
// only ever returns the first child registered under a given field, so using
1886+
// it here silently dropped every attribute after the first bracket group
1887+
// (#1692). Scanning all children by kind fixes that for C#/PHP and is a
1888+
// no-op change for Java/Kotlin/Swift, where `modifiers` never repeats.
1889+
// Writes up to `max` wrapper nodes into `out`; returns how many were found.
1890+
static int find_jvm_modifiers(TSNode node, CBMLanguage lang, TSNode *out, int max) {
18821891
const char *wrapper = NULL;
18831892
switch (lang) {
18841893
case CBM_LANG_JAVA:
@@ -1888,19 +1897,12 @@ static TSNode find_jvm_modifiers(TSNode node, CBMLanguage lang) {
18881897
break;
18891898
case CBM_LANG_CSHARP:
18901899
case CBM_LANG_PHP:
1891-
/* C# attributes live in an `attribute_list` child (modifiers like
1892-
* `public` are separate `modifier` nodes); PHP 8 likewise nests
1893-
* `attribute_group` under `attribute_list`. */
18941900
wrapper = "attribute_list";
18951901
break;
18961902
default:
1897-
return null_node;
1898-
}
1899-
TSNode w = ts_node_child_by_field_name(node, wrapper, (uint32_t)strlen(wrapper));
1900-
if (ts_node_is_null(w)) {
1901-
w = cbm_find_child_by_kind(node, wrapper);
1903+
return 0;
19021904
}
1903-
return w;
1905+
return cbm_find_children_by_kind(node, wrapper, out, max);
19041906
}
19051907

19061908
// Count direct children of `node` that are decorator/annotation nodes (used by
@@ -1978,13 +1980,14 @@ static const char **extract_decorators(CBMArena *a, TSNode node, const char *sou
19781980
prev = ts_node_prev_sibling(prev);
19791981
}
19801982

1981-
TSNode modifiers = {0};
1983+
TSNode wrappers[MAX_ATTR_WRAPPERS];
1984+
int wn = 0;
19821985
int mod_count = 0;
19831986
int child_count = 0;
19841987
if (count == 0) {
1985-
modifiers = find_jvm_modifiers(node, lang);
1986-
if (!ts_node_is_null(modifiers)) {
1987-
mod_count = count_modifier_annotations(modifiers, spec);
1988+
wn = find_jvm_modifiers(node, lang, wrappers, MAX_ATTR_WRAPPERS);
1989+
for (int w = 0; w < wn; w++) {
1990+
mod_count += count_modifier_annotations(wrappers[w], spec);
19881991
}
19891992
/* Languages like Scala attach the annotation directly as a child of the
19901993
* definition node (no wrapper, no prev-sibling). */
@@ -2014,8 +2017,8 @@ static const char **extract_decorators(CBMArena *a, TSNode node, const char *sou
20142017
}
20152018
prev = ts_node_prev_sibling(prev);
20162019
}
2017-
if (!ts_node_is_null(modifiers)) {
2018-
idx = collect_modifier_decorators(a, modifiers, source, spec, result, idx, total);
2020+
for (int w = 0; w < wn && mod_count > 0; w++) {
2021+
idx = collect_modifier_decorators(a, wrappers[w], source, spec, result, idx, total);
20192022
}
20202023
if (child_count > 0) {
20212024
idx = collect_child_decorators(a, node, source, spec, result, idx, total);

‎internal/cbm/helpers.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,18 @@ TSNode cbm_find_child_by_kind(TSNode parent, const char *kind) {
466466
return null_node;
467467
}
468468

469+
int cbm_find_children_by_kind(TSNode parent, const char *kind, TSNode *out, int max) {
470+
int n = 0;
471+
uint32_t count = ts_node_child_count(parent);
472+
for (uint32_t i = 0; i < count && n < max; i++) {
473+
TSNode child = ts_node_child(parent, i);
474+
if (strcmp(ts_node_type(child), kind) == 0) {
475+
out[n++] = child;
476+
}
477+
}
478+
return n;
479+
}
480+
469481
/* ── Node-type classification: TSSymbol bitset acceleration ───────────────
470482
* cbm_kind_in_set is called for nearly every AST node (function/class/call/
471483
* import/branching sets), so a linear strcmp over the type-name array is a hot

‎internal/cbm/helpers.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ char *cbm_cpp_out_of_line_parent_class(CBMArena *a, TSNode node, const char *sou
122122
// Find a child node by kind string.
123123
TSNode cbm_find_child_by_kind(TSNode parent, const char *kind);
124124

125+
// Find every child node matching `kind` (unlike cbm_find_child_by_kind,
126+
// which stops at the first match). Writes up to `max` nodes into `out`;
127+
// returns how many were found. Repeated sibling nodes of the same kind are
128+
// common in some grammars (e.g. C#/PHP stack each `[Attr]` as its own
129+
// attribute_list child) — see #1692.
130+
int cbm_find_children_by_kind(TSNode parent, const char *kind, TSNode *out, int max);
131+
125132
/* --- Lisp-family shared gates ---------------------------------------------
126133
* The defs, calls and unified extractors each walk the same generic `list`
127134
* node and must agree on what it means. The predicates below therefore live

‎tests/repro/repro_issue1692.c‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* repro_issue1692.c — Reproduce-first case for OPEN bug #1692.
3+
*
4+
* Issue: #1692 — "C#: ... ASP.NET Core [Route]/[HttpGet] attribute routes
5+
* not extracted (cross-repo-intelligence returns 0 edges)". The reporter's
6+
* repro 2 shows a class carrying three stacked attributes
7+
* ([ServiceFilter]/[Route]/[ApiController]) where DECORATES exists 13,964
8+
* times elsewhere in the same graph but zero times for that declaration.
9+
*
10+
* Root cause (traced further than the issue body): C# does not attach one
11+
* `attribute_list` wrapper holding every bracket group above a declaration —
12+
* each `[A]` / `[B]` / `[C]` line compiles to its OWN `attribute_list` node,
13+
* so a stack of three produces three separate `attribute_list` siblings
14+
* among the declaration's children. `find_jvm_modifiers()`
15+
* (internal/cbm/extract_defs.c) located that wrapper with
16+
* `ts_node_child_by_field_name(node, "attribute_list", ...)`, which only
17+
* ever returns the FIRST child registered under a repeated field — so only
18+
* the first bracket group's attribute(s) ever produced a DECORATES edge;
19+
* every attribute after the first was silently dropped, with no warning and
20+
* no staleness signal (confirmed indexed, not stale, via grafo_coverage in
21+
* the downstream McpDevGrafo-bugs.md BUG-004 report that led here).
22+
*
23+
* Expected (correct) behaviour:
24+
* A method carrying N stacked attributes gets exactly N outbound
25+
* DECORATES edges — not just one for the first bracket group.
26+
*
27+
* Why RED on current code: only the first attribute's DECORATES edge
28+
* exists, so the project-wide DECORATES count is 1, not 3.
29+
*
30+
* Scope — why this file is C#-only:
31+
* find_jvm_modifiers() shares ONE switch across Java/Kotlin/Swift/C#/PHP,
32+
* but only C# can reach this bug. Verified against the vendored grammars:
33+
* `[A] [B] [C]` in C# parses as THREE sibling `attribute_list` nodes, while
34+
* PHP's `#[A] #[B] #[C]` parses as ONE `attribute_list` holding three
35+
* `attribute_group` children — so the first-child lookup already found them
36+
* all and PHP never dropped an attribute. Java/Kotlin/Swift use the
37+
* `modifiers` wrapper, a single node that never repeats. PHP's DECORATES
38+
* path stays covered by mkc_c3_php8_attribute (test_matrix_known_classes.c);
39+
* a stacked-PHP fixture here would pass with or without the fix and prove
40+
* nothing. No other language reaches this function (`default` returns 0).
41+
*/
42+
43+
#include <foundation/compat.h>
44+
#include "test_framework.h"
45+
#include "repro_harness.h"
46+
47+
#include <string.h>
48+
#include <stdlib.h>
49+
#include <stdio.h>
50+
51+
/* ── Fixture: C# ───────────────────────────────────────────────────────────
52+
*
53+
* One C# method, three stacked attributes, nothing else in the file that
54+
* could carry a DECORATES edge — so a project-wide edge count is as precise
55+
* as scoping to the method node, without the extra node lookup.
56+
* ──────────────────────────────────────────────────────────────────────── */
57+
static const char *kStackedAttrsCs = "using System;\n"
58+
"\n"
59+
"namespace ReproNS\n"
60+
"{\n"
61+
" public class AttrController\n"
62+
" {\n"
63+
" [Foo]\n"
64+
" [Bar(\"x\")]\n"
65+
" [Baz]\n"
66+
" public void StackedAttrsMethod() { }\n"
67+
" }\n"
68+
"}\n";
69+
70+
TEST(repro_issue1692_csharp_stacked_attributes_all_decorate) {
71+
RProj lp;
72+
cbm_store_t *store = rh_index(&lp, "StackedAttrs.cs", kStackedAttrsCs);
73+
ASSERT_NOT_NULL(store);
74+
75+
int decorates_count = rh_count_edges(store, lp.project, "DECORATES");
76+
if (decorates_count != 3) {
77+
fprintf(stderr,
78+
" [1692] FAIL decorates=%d (expected 3 — one per stacked [Foo][Bar][Baz]; "
79+
"only the first attribute_list's attribute(s) survive find_jvm_modifiers' "
80+
"single-field lookup)\n",
81+
decorates_count);
82+
}
83+
ASSERT_EQ(3, decorates_count); /* one DECORATES per stacked attribute, not just the first */
84+
85+
rh_cleanup(&lp, store);
86+
PASS();
87+
}
88+
89+
SUITE(repro_issue1692) {
90+
RUN_TEST(repro_issue1692_csharp_stacked_attributes_all_decorate);
91+
}

‎tests/repro/repro_main.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ extern void suite_repro_issue581(void);
9191
extern void suite_repro_issue787(void);
9292
extern void suite_repro_issue842(void);
9393
extern void suite_repro_issue964(void);
94+
extern void suite_repro_issue1692(void);
9495
/* NEW bugs found by the discovery sweep */
9596
extern void suite_repro_new_ts_class_field_arrow(void);
9697
extern void suite_repro_new_py_tuple_unpack(void);
@@ -192,6 +193,7 @@ int main(void) {
192193
RUN_SUITE(repro_issue787);
193194
RUN_SUITE(repro_issue842);
194195
RUN_SUITE(repro_issue964);
196+
RUN_SUITE(repro_issue1692);
195197
RUN_SUITE(repro_invariant_calls);
196198
RUN_SUITE(repro_invariant_graph);
197199
RUN_SUITE(repro_invariant_breadth);

0 commit comments

Comments
 (0)