Skip to content

fix(extract): compose Spring controller route prefixes #772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jonghyunlee-29cm wants to merge 1 commit into
base: main
Choose a base branch
from
+160 −44
Open

fix(extract): compose Spring controller route prefixes #772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 87 additions & 28 deletions internal/cbm/extract_defs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1323,35 +1323,58 @@ static TSNode find_decorator_args(TSNode call_node) {
return args;
}

static bool is_route_string_kind(const char *kind) {
return strcmp(kind, "string") == 0 || strcmp(kind, "string_literal") == 0 ||
strcmp(kind, "interpreted_string_literal") == 0;
}

static const char *route_path_from_string_node(CBMArena *a, TSNode node, const char *source) {
if (!is_route_string_kind(ts_node_type(node))) {
return NULL;
}
char *path = cbm_node_text(a, node, source);
if (!path) {
return NULL;
}
int plen = (int)strlen(path);
if (plen >= PAIR_CHARS && (path[0] == '"' || path[0] == '\'')) {
path = cbm_arena_strndup(a, path + SKIP_CHAR, (size_t)(plen - PAIR_CHARS));
}
return (path && path[0] == '/') ? path : NULL;
}

static const char *find_route_path_literal(CBMArena *a, TSNode node, const char *source,
int max_depth) {
if (ts_node_is_null(node) || max_depth < 0) {
return NULL;
}
const char *path = route_path_from_string_node(a, node, source);
if (path || max_depth == 0) {
return path;
}
uint32_t nc = ts_node_named_child_count(node);
for (uint32_t i = 0; i < nc && i < DECORATOR_SCAN_LIMIT; i++) {
path = find_route_path_literal(a, ts_node_named_child(node, i), source, max_depth - 1);
if (path) {
return path;
}
}
return NULL;
}

// Extract route path from decorator arguments (first string that starts with /).
static const char *extract_route_path_from_args(CBMArena *a, TSNode args, const char *source) {
uint32_t nc = ts_node_named_child_count(args);
for (uint32_t ai = 0; ai < nc && ai < DECORATOR_SCAN_LIMIT; ai++) {
TSNode arg = ts_node_named_child(args, ai);
const char *ak = ts_node_type(arg);
/* Kotlin wraps each annotation argument in a `value_argument` node
* (and supports the named form `value = "/x"`); unwrap to the string. */
if (strcmp(ak, "value_argument") == 0) {
TSNode s = cbm_find_child_by_kind(arg, "string_literal");
if (ts_node_is_null(s)) {
continue;
}
arg = s;
ak = ts_node_type(arg);
}
if (strcmp(ak, "string") != 0 && strcmp(ak, "string_literal") != 0 &&
strcmp(ak, "interpreted_string_literal") != 0) {
continue;
}
char *path = cbm_node_text(a, arg, source);
/* Spring/Kotlin frequently uses named or array-valued annotation args:
* @RequestMapping(value = ["/internal/v1"])
* @GetMapping(path = {"/orders"})
* Walk a bounded subtree and keep the first string literal that is
* path-shaped, while ignoring non-route literals such as media types. */
const char *path = find_route_path_literal(a, arg, source, CBM_DESCENDANT_MAX_DEPTH);
if (path) {
int plen = (int)strlen(path);
if (plen >= PAIR_CHARS && (path[0] == '"' || path[0] == '\'')) {
path = cbm_arena_strndup(a, path + SKIP_CHAR, (size_t)(plen - PAIR_CHARS));
}
if (path && path[0] == '/') {
return path;
}
return path;
}
}
return NULL;
Expand Down Expand Up @@ -1655,6 +1678,38 @@ static void extract_route_from_decorators(CBMArena *a, TSNode func_node, const c
extract_route_from_annotations(a, func_node, source, spec, out_path, out_method);
}

static const char *join_route_paths(CBMArena *a, const char *prefix, const char *path) {
if (!path || !path[0]) {
return prefix;
}
if (!prefix || !prefix[0] || strcmp(prefix, "/") == 0) {
return path;
}
if (strcmp(path, "/") == 0) {
return prefix;
}
size_t plen = strlen(prefix);
bool prefix_slash = prefix[plen - 1] == '/';
bool path_slash = path[0] == '/';
if (prefix_slash && path_slash) {
return cbm_arena_sprintf(a, "%s%s", prefix, path + SKIP_CHAR);
}
if (!prefix_slash && !path_slash) {
return cbm_arena_sprintf(a, "%s/%s", prefix, path);
}
return cbm_arena_sprintf(a, "%s%s", prefix, path);
}

static const char *spring_class_route_prefix(CBMArena *a, TSNode class_node, const char *source,
const CBMLangSpec *spec) {
const char *prefix = NULL;
const char *method = NULL;
if (extract_route_from_annotations(a, class_node, source, spec, &prefix, &method)) {
return prefix;
}
return NULL;
}

// Extract decorator names from preceding decorator/annotation nodes
// Count annotations inside a Java/Kotlin/C# "modifiers" node.
static int count_modifier_annotations(TSNode modifiers, const CBMLangSpec *spec) {
Expand Down Expand Up @@ -4000,8 +4055,8 @@ static TSNode resolve_method_name(TSNode child, CBMLanguage lang) {
}

// Push a single method definition
static void push_method_def(CBMExtractCtx *ctx, TSNode child, const char *class_qn,
const CBMLangSpec *spec, TSNode name_node) {
static void push_method_def(CBMExtractCtx *ctx, TSNode child, TSNode class_node,
const char *class_qn, const CBMLangSpec *spec, TSNode name_node) {
CBMArena *a = ctx->arena;

char *name = cbm_func_name_node_text(a, name_node, ctx->source);
Expand Down Expand Up @@ -4049,6 +4104,10 @@ static void push_method_def(CBMExtractCtx *ctx, TSNode child, const char *class_

def.decorators = extract_decorators(a, child, ctx->source, ctx->language, spec);
extract_route_from_decorators(a, child, ctx->source, spec, &def.route_path, &def.route_method);
if (def.route_path && (ctx->language == CBM_LANG_JAVA || ctx->language == CBM_LANG_KOTLIN)) {
const char *prefix = spring_class_route_prefix(a, class_node, ctx->source, spec);
def.route_path = join_route_paths(a, prefix, def.route_path);
}
def.docstring = extract_docstring(a, child, ctx->source, ctx->language);

if (spec->branching_node_types && spec->branching_node_types[0]) {
Expand All @@ -4073,7 +4132,7 @@ static void extract_objc_impl_methods(CBMExtractCtx *ctx, TSNode impl_node, cons
if (cbm_kind_in_set(inner, spec->function_node_types)) {
TSNode nm = resolve_method_name(inner, ctx->language);
if (!ts_node_is_null(nm)) {
push_method_def(ctx, inner, class_qn, spec, nm);
push_method_def(ctx, inner, impl_node, class_qn, spec, nm);
}
}
}
Expand Down Expand Up @@ -4136,7 +4195,7 @@ static void extract_class_methods(CBMExtractCtx *ctx, TSNode class_node, const c
if (ts_node_is_null(fname)) {
continue;
}
push_method_def(ctx, value, class_qn, spec, fname);
push_method_def(ctx, value, class_node, class_qn, spec, fname);
continue;
}

Expand All @@ -4149,7 +4208,7 @@ static void extract_class_methods(CBMExtractCtx *ctx, TSNode class_node, const c
continue;
}

push_method_def(ctx, method_node, class_qn, spec, name_node);
push_method_def(ctx, method_node, class_node, class_qn, spec, name_node);
}
}

Expand Down
89 changes: 73 additions & 16 deletions tests/test_edge_types_probe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,66 @@ static int et_edge_present(const EtFile *files, int nfiles, const char *edge, in
return got >= floor;
}

enum { ET_ROUTE_ASSERT_MAX = 16 };

/* Assert the exact Route node set. Edge-count smoke tests cannot catch partial
* Spring paths such as "/orders" when the real route is "/api/orders", and a
* presence-only assertion would still allow stale partial Route nodes to leak. */
static int et_routes_exact(const EtFile *files, int nfiles, const char **routes) {
EtProj lp;
cbm_store_t *store = et_index_files(&lp, files, nfiles);
cbm_node_t *nodes = NULL;
int node_count = 0;
int wanted = 0;
int found[ET_ROUTE_ASSERT_MAX] = {0};
int ok = store != NULL;

while (routes[wanted] && wanted < ET_ROUTE_ASSERT_MAX) {
wanted++;
}
if (routes[wanted]) {
ok = 0;
}

if (!store || cbm_store_find_nodes_by_label(store, lp.project, "Route", &nodes, &node_count) !=
CBM_STORE_OK) {
ok = 0;
} else {
if (node_count != wanted) {
ok = 0;
}
for (int wi = 0; wi < wanted; wi++) {
for (int ni = 0; ni < node_count; ni++) {
if (nodes[ni].name && strcmp(nodes[ni].name, routes[wi]) == 0) {
found[wi] = 1;
break;
}
}
if (!found[wi]) {
ok = 0;
}
}
}

if (!ok) {
fprintf(stderr, " [ET-ROUTE] FAIL expected=%d actual=%d missing:", wanted, node_count);
for (int wi = 0; wi < wanted; wi++) {
if (!found[wi]) {
fprintf(stderr, " %s", routes[wi]);
}
}
fprintf(stderr, " available:");
for (int ni = 0; ni < node_count && ni < ET_ROUTE_ASSERT_MAX; ni++) {
fprintf(stderr, " %s", nodes[ni].name ? nodes[ni].name : "<null>");
}
fprintf(stderr, "\n");
}

cbm_store_free_nodes(nodes, node_count);
et_cleanup(&lp, store);
return ok;
}

/* Index meaningful[] plus PARALLEL_PAD_FILES trivial pad files to force the
* parallel pipeline path (MIN_FILES_FOR_PARALLEL = 50). */
enum { ET_PARALLEL_PAD = 52, ET_PAD_MAX = 68 /* 52 pad + 16 meaningful */ };
Expand Down Expand Up @@ -277,13 +337,12 @@ TEST(handles_gin_go) {
PASS();
}

/* Spring (Java) — @RequestMapping decorator sets route_path in extraction.
* REAL BUG: internal/cbm/extract_defs.c:extract_route_from_decorators only walks
* ts_node_prev_sibling(func) for decorator nodes of type "call". Java annotations
* (@GetMapping/@RequestMapping) live INSIDE the method's `modifiers` child (not a
* prev_sibling) and are `annotation`/`marker_annotation` nodes (not `call`), so
* route_path is never set → no Route/HANDLES for Spring controllers. */
/* Spring (Java) — class-level @RequestMapping must prefix method mappings.
* Reproduce-first: a HANDLES count alone can pass with partial routes
* ("/orders"), but callers/search_graph need the actual endpoint names
* ("/api/orders"). */
TEST(handles_spring_java) {
static const char *routes[] = {"/api/orders", "/api/orders/{id}", NULL};
static const EtFile f[] = {
{"OrderController.java",
"package com.example;\n\n"
Expand All @@ -296,31 +355,29 @@ TEST(handles_spring_java) {
" @GetMapping(\"/orders/{id}\")\n"
" public String getOrder(int id) {\n"
" return \"order:\" + id;\n }\n}\n"}};
ASSERT_TRUE(et_edge_present(f, 1, "HANDLES", 1));
ASSERT_TRUE(et_edge_present(f, 1, "HANDLES", 2));
ASSERT_TRUE(et_routes_exact(f, 1, routes));
PASS();
}

/* Spring (Kotlin) — @RequestMapping/@GetMapping on a Kotlin @RestController.
* REAL BUG: tree-sitter-kotlin annotation nodes have no `name` field (the name
* lives in a nested user_type/type_identifier) and carry args under a
* constructor_invocation `value_arguments` node, so the Java-shaped
* ts_node_child_by_field_name(annotation, "name") / "arguments" lookups in
* try_route_from_annotation missed every Kotlin Spring route → route_path never
* set → no Route/HANDLES. Fixed by annotation_name_node/annotation_args_node. */
/* Spring (Kotlin) — same prefix contract, including Kotlin's named array form
* for class-level RequestMapping values. */
TEST(handles_spring_kotlin) {
static const char *routes[] = {"/internal/v1/api/orders", "/internal/v1/api/orders/{id}", NULL};
static const EtFile f[] = {
{"OrderController.kt",
"package com.example\n\n"
"import org.springframework.web.bind.annotation.RequestMapping\n"
"import org.springframework.web.bind.annotation.GetMapping\n\n"
"@RequestMapping(\"/api\")\nclass OrderController {\n"
"@RequestMapping(value = [\"/internal/v1/api\"])\nclass OrderController {\n"
" @GetMapping(\"/orders\")\n"
" fun listOrders(): String {\n"
" return \"orders\"\n }\n\n"
" @GetMapping(\"/orders/{id}\")\n"
" fun getOrder(id: Int): String {\n"
" return \"order:\" + id\n }\n}\n"}};
ASSERT_TRUE(et_edge_present(f, 1, "HANDLES", 1));
ASSERT_TRUE(et_edge_present(f, 1, "HANDLES", 2));
ASSERT_TRUE(et_routes_exact(f, 1, routes));
PASS();
}

Expand Down
Loading