mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-06 16:53:56 +00:00
fix(extract): receiver-qualify Go method QNs
A Go method's QN was the flat package form (proj.pkg.method) — the receiver was ignored, so every same-name method in a package collided on one QN and the graph upsert kept exactly one node. Measured on a real Go repo: 20 Process/Name methods across 15 files kept 2 nodes; 9 different Task() methods fused into one chimera node carrying all nine bodies' call edges; 19 structs pointed DEFINES_METHOD at a single shared method node; a _test.go mock Close outranked the production Close in the dedupe tie-break. The upsert's own comment calls kind-disambiguated QNs 'the real cure'. Qualify the QN with the receiver type (proj.pkg.Recv.method), the same shape as Go interface members and the C++ out-of-line method path right below it in extract_func_def: - extract_defs.c: def.qualified_name = parent_class + name whenever the receiver type resolves; go_receiver_type_name becomes the shared cbm_go_receiver_type_name (exported via helpers.h) so both sides of the contract use one formula. - extract_unified.c (compute_func_qn): mirror branch for method_declaration, so method-body calls keep exact source attribution instead of degrading to File-node fallback (calls_find_source). - Consumers already agree: pxc_build_lsp_def passes the def QN and parent_class (receiver_type) verbatim into the Go LSP registries, and check_go_class_implements explicitly supports class-qualified method QNs (its path (b)). Side effect: resolve_same_module's exact module.name hash no longer matches concrete methods, which kills the conf-0.9 false edges where an interface-typed call bound to an unrelated same-package method. Fixes #1909 Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
This commit is contained in:
1 parent
50ee111714
commit
da7619474d
7 files changed
+156
-15
No files matched your search
@@ -3521,7 +3521,7 @@ static void set_def_complexity(CBMDefinition *def, TSNode body, const CBMLangSpe
|
||||
* Walks to the parameter_declaration's `type` field, unwrapping pointer_type
|
||||
* and generic_type, and returns the type_identifier text (e.g. "OrderService").
|
||||
* Returns NULL if no type_identifier is found. */
|
||||
static char *go_receiver_type_name(CBMArena *a, TSNode recv, const char *source) {
|
||||
char *cbm_go_receiver_type_name(CBMArena *a, TSNode recv, const char *source) {
|
||||
uint32_t nc = ts_node_child_count(recv);
|
||||
for (uint32_t i = 0; i < nc; i++) {
|
||||
TSNode child = ts_node_child(recv, i);
|
||||
@@ -3733,12 +3733,22 @@ static void extract_func_def(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec
|
||||
* (and downstream Go IMPLEMENTS/OVERRIDE) link the method to its owning
|
||||
* struct/type node. The parent QN must match the type's node QN, which
|
||||
* is computed the same way (cbm_fqn_compute on the type name). */
|
||||
char *recv_type = go_receiver_type_name(a, recv, ctx->source);
|
||||
char *recv_type = cbm_go_receiver_type_name(a, recv, ctx->source);
|
||||
if (recv_type && recv_type[0]) {
|
||||
/* Must match the Go type node QN (directory-based module) so the
|
||||
* DEFINES_METHOD edge links the method to its owning type. */
|
||||
def.parent_class = cbm_fqn_compute_source_lang(a, ctx->project, ctx->rel_path,
|
||||
recv_type, ctx->language);
|
||||
/* Receiver-qualify the method QN (proj.pkg.Recv.method) — same
|
||||
* shape as the C++ out-of-line path below and Go interface
|
||||
* members. With the flat proj.pkg.method QN every same-name
|
||||
* method in a package collided in the graph upsert: one body
|
||||
* survived and the twins' call edges accreted onto it. The
|
||||
* call-scope side (compute_func_qn in extract_unified.c) mirrors
|
||||
* this formula, and go_lsp consumers read the def QN and
|
||||
* parent_class (receiver_type) verbatim, so resolution joins
|
||||
* stay exact. */
|
||||
def.qualified_name = cbm_arena_sprintf(a, "%s.%s", def.parent_class, name);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1087,6 +1087,25 @@ static const char *compute_func_qn(CBMExtractCtx *ctx, TSNode node, const CBMLan
|
||||
}
|
||||
}
|
||||
|
||||
/* Go method `func (s *Storage) Close() {...}`: the def extractor records
|
||||
* this as Method "proj.pkg.Storage.Close" (receiver-qualified, mirroring
|
||||
* the C++ out-of-line rule above). The call-scope QN must match — a bare
|
||||
* "proj.pkg.Close" names a node that no longer exists, so every call
|
||||
* inside the method body would fall back to File-node attribution
|
||||
* (calls_find_source). Same ONE-formula contract as the def side:
|
||||
* cbm_go_receiver_type_name + cbm_fqn_compute_source_lang. */
|
||||
if (ctx->language == CBM_LANG_GO && strcmp(ts_node_type(node), "method_declaration") == 0) {
|
||||
TSNode recv = ts_node_child_by_field_name(node, TS_FIELD("receiver"));
|
||||
if (!ts_node_is_null(recv)) {
|
||||
char *recv_type = cbm_go_receiver_type_name(ctx->arena, recv, ctx->source);
|
||||
if (recv_type && recv_type[0]) {
|
||||
const char *type_qn = cbm_fqn_compute_source_lang(
|
||||
ctx->arena, ctx->project, ctx->rel_path, recv_type, ctx->language);
|
||||
return cbm_arena_sprintf(ctx->arena, "%s.%s", type_qn, name);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/* Nix: a binding's own attrpath contributes scope (`a.b.fn = …`), and the def
|
||||
* extractor bakes it into the def QN. Compose it identically here — otherwise
|
||||
* an in-body call sources to a QN one or more segments short of the def, and
|
||||
|
||||
@@ -119,6 +119,12 @@ TSNode cbm_resolve_func_name(TSNode node, CBMLanguage lang);
|
||||
// def extractor — drift dropped the class qualifier from in-body calls (#554/#621).
|
||||
char *cbm_cpp_out_of_line_parent_class(CBMArena *a, TSNode node, const char *source);
|
||||
|
||||
/* Go: resolve a method_declaration's receiver parameter_list down to the bare
|
||||
* receiver type_identifier (unwrapping pointer_type / generic_type). Shared by
|
||||
* def extraction (extract_defs.c) and call-scope attribution
|
||||
* (extract_unified.c) so the receiver-qualified method QN has ONE formula. */
|
||||
char *cbm_go_receiver_type_name(CBMArena *a, TSNode recv, const char *source);
|
||||
|
||||
// Find a child node by kind string.
|
||||
TSNode cbm_find_child_by_kind(TSNode parent, const char *kind);
|
||||
|
||||
|
||||
@@ -1861,15 +1861,24 @@ static void process_function(GoLSPContext* ctx, TSNode func_node) {
|
||||
char* func_name = lsp_node_text(ctx, name_node);
|
||||
if (!func_name || !func_name[0]) return;
|
||||
|
||||
// Enclosing-function QN must be the BARE package.Func form (no receiver
|
||||
// type segment). The textual call events (extract_unified.c) source calls
|
||||
// as package_qn.func_name — methods included — and the defs pass creates
|
||||
// the graph Method node under the same QN, so any other form breaks the
|
||||
// caller-QN join in cbm_pipeline_find_lsp_resolution and the LSP-resolved
|
||||
// call silently falls back to the registry short-name resolver. The
|
||||
// receiver type still reaches the registry via the def's parent_class /
|
||||
// method->receiver_type; it just does not appear in the caller QN.
|
||||
// Enclosing-function QN must be EXACTLY the QN the textual call events
|
||||
// (extract_unified.c compute_func_qn) and the defs pass produce, or the
|
||||
// caller-QN join in cbm_pipeline_find_lsp_resolution breaks and the
|
||||
// LSP-resolved call silently falls back to the registry short-name
|
||||
// resolver. Since #1909 receiver-qualified Go method QNs, that shared
|
||||
// form is package_qn.ReceiverType.method for methods (the ONE-formula
|
||||
// contract: cbm_go_receiver_type_name) and package_qn.func for functions.
|
||||
ctx->enclosing_func_qn = cbm_arena_sprintf(ctx->arena, "%s.%s", ctx->package_qn, func_name);
|
||||
if (strcmp(ts_node_type(func_node), "method_declaration") == 0) {
|
||||
TSNode recv = ts_node_child_by_field_name(func_node, "receiver", 8);
|
||||
if (!ts_node_is_null(recv)) {
|
||||
char *recv_type = cbm_go_receiver_type_name(ctx->arena, recv, ctx->source);
|
||||
if (recv_type && recv_type[0]) {
|
||||
ctx->enclosing_func_qn = cbm_arena_sprintf(ctx->arena, "%s.%s.%s", ctx->package_qn,
|
||||
recv_type, func_name);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Push function scope
|
||||
CBMScope* saved_scope = ctx->current_scope;
|
||||
|
||||
+47
-3
@@ -4715,12 +4715,12 @@ TEST(extract_go_no_filename_in_module_qn) {
|
||||
ASSERT_NOT_NULL(conn);
|
||||
ASSERT_STR_EQ(conn->qualified_name, "proj.myapp.db.Conn");
|
||||
|
||||
/* Go method nodes keep a FLAT QN (module + name) with a separate
|
||||
* parent_class link to the receiver type — the QN must carry the
|
||||
/* Go method nodes carry a receiver-qualified QN (module + receiver type +
|
||||
* name) plus the parent_class link — the QN must carry the
|
||||
* directory-based module and NOT the `.conn.` filename segment. */
|
||||
const CBMDefinition *query = find_def_by_name(r, "Query");
|
||||
ASSERT_NOT_NULL(query);
|
||||
ASSERT_STR_EQ(query->qualified_name, "proj.myapp.db.Query");
|
||||
ASSERT_STR_EQ(query->qualified_name, "proj.myapp.db.Conn.Query");
|
||||
ASSERT_EQ(strstr(query->qualified_name, ".conn."), NULL);
|
||||
/* The method's parent_class must match the type node QN (for DEFINES_METHOD). */
|
||||
ASSERT_NOT_NULL(query->parent_class);
|
||||
@@ -5501,6 +5501,49 @@ TEST(extract_python_bare_call_flag_is_depth_independent) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(extract_go_method_receiver_qualified_qn) {
|
||||
/* A Go method QN carries the receiver type (proj.pkg.Recv.method), same
|
||||
* shape as C++ out-of-line methods and Go interface members — so two
|
||||
* same-name methods on different receivers no longer collide in the
|
||||
* graph upsert. Free functions keep the flat package QN. */
|
||||
CBMFileResult *r = extract("package m\n"
|
||||
"type Storage struct{}\n"
|
||||
"type Cache struct{}\n"
|
||||
"func (s *Storage) Close() {}\n"
|
||||
"func (c Cache) Close() {}\n"
|
||||
"func Shutdown() {}\n",
|
||||
CBM_LANG_GO, "t", "x.go");
|
||||
ASSERT_NOT_NULL(r);
|
||||
ASSERT_FALSE(r->has_error);
|
||||
bool saw_storage = false;
|
||||
bool saw_cache = false;
|
||||
bool saw_free = false;
|
||||
for (int i = 0; i < r->defs.count; i++) {
|
||||
const CBMDefinition *d = &r->defs.items[i];
|
||||
if (!d->name || !d->qualified_name) {
|
||||
continue;
|
||||
}
|
||||
if (strcmp(d->name, "Close") == 0 && strcmp(d->qualified_name, "t.Storage.Close") == 0) {
|
||||
ASSERT_STR_EQ(d->label, "Method");
|
||||
ASSERT_STR_EQ(d->parent_class, "t.Storage");
|
||||
saw_storage = true;
|
||||
}
|
||||
if (strcmp(d->name, "Close") == 0 && strcmp(d->qualified_name, "t.Cache.Close") == 0) {
|
||||
ASSERT_STR_EQ(d->parent_class, "t.Cache");
|
||||
saw_cache = true;
|
||||
}
|
||||
if (strcmp(d->name, "Shutdown") == 0) {
|
||||
ASSERT_STR_EQ(d->qualified_name, "t.Shutdown");
|
||||
saw_free = true;
|
||||
}
|
||||
}
|
||||
ASSERT_TRUE(saw_storage);
|
||||
ASSERT_TRUE(saw_cache);
|
||||
ASSERT_TRUE(saw_free);
|
||||
cbm_free_result(r);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* TS/JS/TSX receiver-aware flag (#592/#606; same intent as the Perl flag above).
|
||||
* A member call x.foo() with a non-this/super receiver is flagged is_method so
|
||||
* the resolver can suppress a weak short-name match (`re.test()` must not bind a
|
||||
@@ -7541,6 +7584,7 @@ SUITE(extraction) {
|
||||
RUN_TEST(extract_python_bare_call_flags_locally_bound_callee);
|
||||
RUN_TEST(extract_python_bare_call_flag_is_depth_independent);
|
||||
RUN_TEST(extract_go_selector_call_flags_is_method);
|
||||
RUN_TEST(extract_go_method_receiver_qualified_qn);
|
||||
RUN_TEST(extract_ts_member_call_flags_is_method);
|
||||
RUN_TEST(extract_ts_this_super_receiver_not_flagged);
|
||||
RUN_TEST(extract_js_member_call_flags_is_method);
|
||||
|
||||
@@ -3444,8 +3444,9 @@ TEST(parallel_go_cross_package_field_chain_resolves) {
|
||||
cbm_gbuf_t *gbuf = run_go_field_chain_sequential("go_field_fold", tmpdir, files, 2);
|
||||
ASSERT_NOT_NULL(gbuf);
|
||||
|
||||
const cbm_gbuf_edge_t *edge =
|
||||
find_call_edge_to_target_fragment(gbuf, "handler.PlaceOrder", ".service.PlaceOrder");
|
||||
/* Since #1909, Go method QNs are receiver-qualified on both endpoints. */
|
||||
const cbm_gbuf_edge_t *edge = find_call_edge_to_target_fragment(
|
||||
gbuf, "OrderHandler.PlaceOrder", ".service.OrderService.PlaceOrder");
|
||||
const bool found = edge != NULL;
|
||||
const bool dispatch = edge && edge->properties_json &&
|
||||
strstr(edge->properties_json, "\"strategy\":\"lsp_type_dispatch\"");
|
||||
|
||||
@@ -5467,6 +5467,57 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(pipeline_go_method_caller_keeps_lsp_join) {
|
||||
/* #1909: receiver-qualifying method QNs moves the def/textual-call QN to
|
||||
* package.Type.method — go_lsp's enclosing-function QN must move with it
|
||||
* (the ONE-formula contract), or every LSP resolution sourced from inside
|
||||
* a method body loses its caller join and the edge dies. The callee name
|
||||
* collides across receivers so no short-name fallback can mask the loss. */
|
||||
char tmp[256];
|
||||
snprintf(tmp, sizeof(tmp), "/tmp/cbm_go_mcall_XXXXXX");
|
||||
if (!cbm_mkdtemp(tmp)) {
|
||||
FAIL("tmpdir");
|
||||
}
|
||||
write_temp_file(tmp, "go.mod", "module example.com/fxmcall\n\ngo 1.22\n");
|
||||
write_temp_file(tmp, "svc/repo.go",
|
||||
"package svc\n"
|
||||
"\n"
|
||||
"type Repo struct {\n"
|
||||
"\tn int\n"
|
||||
"}\n"
|
||||
"\n"
|
||||
"func (r *Repo) Install() int {\n"
|
||||
"\treturn r.helper()\n"
|
||||
"}\n");
|
||||
write_temp_file(tmp, "svc/helper.go",
|
||||
"package svc\n"
|
||||
"\n"
|
||||
"func (r *Repo) helper() int {\n"
|
||||
"\treturn r.n\n"
|
||||
"}\n");
|
||||
write_temp_file(tmp, "other/other.go",
|
||||
"package other\n"
|
||||
"\n"
|
||||
"type Decoy struct{}\n"
|
||||
"\n"
|
||||
"func (d *Decoy) helper() int { return 2 }\n");
|
||||
|
||||
char db_path[512];
|
||||
snprintf(db_path, sizeof(db_path), "%s/go_mcall.db", tmp);
|
||||
cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL);
|
||||
ASSERT_NOT_NULL(p);
|
||||
ASSERT_EQ(cbm_pipeline_run(p), 0);
|
||||
const char *project = cbm_pipeline_project_name(p);
|
||||
|
||||
cbm_store_t *s = cbm_store_open_path(db_path);
|
||||
ASSERT_NOT_NULL(s);
|
||||
ASSERT_TRUE(cross_file_call_exists(s, project, "Install", "helper"));
|
||||
cbm_store_close(s);
|
||||
cbm_pipeline_free(p);
|
||||
th_rmtree(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(pipeline_go_receiver_suppresses_weak_method_edge) {
|
||||
char tmp[256];
|
||||
snprintf(tmp, sizeof(tmp), "/tmp/cbm_go_recv_XXXXXX");
|
||||
@@ -14687,6 +14738,7 @@ SUITE(pipeline) {
|
||||
#endif
|
||||
RUN_TEST(pipeline_tsjs_receiver_suppresses_weak_method_edge);
|
||||
RUN_TEST(pipeline_python_receiver_suppresses_weak_method_edge);
|
||||
RUN_TEST(pipeline_go_method_caller_keeps_lsp_join);
|
||||
RUN_TEST(pipeline_go_receiver_suppresses_weak_method_edge);
|
||||
RUN_TEST(pipeline_go_rw_usage_never_cross_into_c);
|
||||
RUN_TEST(pipeline_go_rw_usage_never_cross_into_c_parallel);
|
||||
|
||||
Reference in new issue
Block a user