mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-06 16:53:56 +00:00
fix(pipeline): suppress weak short-name matches for Go selector calls
A Go selector call x.foo() whose receiver the Go LSP cannot type falls through to the generic registry resolver, which binds it by bare short name to an arbitrary same-named project symbol. Stdlib calls are the worst case: f.Close() on an *os.File gets a CALLS edge to whatever project Close wins candidate ranking (measured on a real Go repo: confidence 0.11, 15 candidates; suffix_match + unique_name were 36% of all CALLS edges, and one 14-line stdlib-only function got 3 out of 3 false outbound edges). Extend the TS/JS receiver-aware guard (#592/#606) to Go: - extract_calls.c: flag Go call_expression with a selector_expression callee as is_method, mirroring the TS/JS member_expression flag. - registry.c: add cbm_go_suppress_weak_method_match. Unlike the TS/JS drop-list, field_type_hint is KEPT (Go struct fields carry declared types, so the hint is receiver-aware — lrp_go_s8_field_type_hint), and unique_name is dropped only when its confidence carries the import-unreachability penalty (the stdlib-hijack shape); an unpenalized lone candidate inside the caller's import closure never enters the field-type-hint upgrade and must survive. - pass_calls.c / pass_parallel.c: feed the Go gate next to the TS/JS one; the drop still defers to the emit path so service/route/HTTP edges stay main-identical. Reproduce-first: pipeline_go_receiver_suppresses_weak_method_edge is RED without the extractor flag (the f.Close -> project Close edge exists) and GREEN with it; typed same-package calls, bare local calls and import-qualified cross-package calls still resolve. The old extraction contract test used Go as the flag-exempt language — Python takes that role, and extract_go_selector_call_flags_is_method pins the new behavior. Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
This commit is contained in:
1 parent
8972ea69c6
commit
50ee111714
8 files changed
+242
-7
No files matched your search
@@ -3712,6 +3712,24 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML
|
||||
}
|
||||
}
|
||||
}
|
||||
// Go receiver-aware guard (same direction as the TS/JS flag above).
|
||||
// Flag a selector call x.foo(). The Go AST cannot separate a method
|
||||
// call on a value from a package-qualified call — but every selector
|
||||
// call the Go LSP or the import/qualified registry strategies CAN
|
||||
// place never reaches the weak short-name guards, so the flag only
|
||||
// bites on unresolvable receivers (`f.Close()` on an os.File,
|
||||
// `sha256.New()` behind an unindexed import), where a project-wide
|
||||
// short-name match fabricates an edge to an unrelated project
|
||||
// symbol sharing the name. Bare calls (helper()) keep
|
||||
// is_method=false and resolve same-module/import paths as before.
|
||||
if (ctx->language == CBM_LANG_GO &&
|
||||
strcmp(ts_node_type(node), "call_expression") == 0) {
|
||||
TSNode gofn = ts_node_child_by_field_name(node, TS_FIELD("function"));
|
||||
if (!ts_node_is_null(gofn) &&
|
||||
strcmp(ts_node_type(gofn), "selector_expression") == 0) {
|
||||
call.is_method = true;
|
||||
}
|
||||
}
|
||||
|
||||
TSNode args = ts_node_child_by_field_name(node, TS_FIELD("arguments"));
|
||||
/* tree-sitter-elixir attaches NO field name to a call's arguments
|
||||
|
||||
@@ -623,7 +623,13 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call,
|
||||
* language gated on only one resolver produces an edge on the sequential
|
||||
* path and not the parallel one (or vice versa), breaking MT determinism.
|
||||
* ArkTS belongs to the JS/TS family here (#1842); dropping it would
|
||||
* reintroduce the #592/#606 false-edge class for .ets files. */
|
||||
* reintroduce the #592/#606 false-edge class for .ets files.
|
||||
*
|
||||
* Go (#1906) rides the same deferred-drop plumbing through its OWN
|
||||
* predicate: its drop-list differs (field_type_hint is receiver-aware for
|
||||
* Go, and unique_name drops only when import-unreachability-penalized), so
|
||||
* it composes via cbm_go_suppress_weak_method_match instead of widening
|
||||
* the shared gate. Same lockstep rule: mirror pass_parallel.c. */
|
||||
bool suppress_weak_member = lang == CBM_LANG_PYTHON || lang == CBM_LANG_JAVASCRIPT ||
|
||||
lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
|
||||
lang == CBM_LANG_ARKTS;
|
||||
@@ -636,7 +642,9 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call,
|
||||
bool drop_plain_call =
|
||||
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) ||
|
||||
cbm_suppress_weak_local_binding_call(suppress_weak_local_binding,
|
||||
call->callee_is_locally_bound, res.strategy);
|
||||
call->callee_is_locally_bound, res.strategy) ||
|
||||
cbm_go_suppress_weak_method_match(lang == CBM_LANG_GO, call->is_method, res.strategy,
|
||||
res.confidence);
|
||||
|
||||
/* Service-pattern HTTP/ASYNC calls to an EXTERNAL client library (e.g.
|
||||
* `requests.get("/api/orders/{id}")`) resolve to a QN containing the library
|
||||
|
||||
@@ -2834,7 +2834,9 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
|
||||
* #606 direction.
|
||||
*
|
||||
* This language set MUST match the one in pass_calls.c exactly — see the
|
||||
* note there. ArkTS belongs to the JS/TS family (#1842). */
|
||||
* note there. ArkTS belongs to the JS/TS family (#1842). Go (#1906)
|
||||
* composes via its own predicate (different drop-list — see
|
||||
* cbm_go_suppress_weak_method_match), mirrored in pass_calls.c. */
|
||||
bool suppress_weak_member = lang == CBM_LANG_PYTHON || lang == CBM_LANG_JAVASCRIPT ||
|
||||
lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
|
||||
lang == CBM_LANG_ARKTS;
|
||||
@@ -2844,7 +2846,9 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
|
||||
bool drop_plain_call =
|
||||
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) ||
|
||||
cbm_suppress_weak_local_binding_call(suppress_weak_local_binding,
|
||||
call->callee_is_locally_bound, res.strategy);
|
||||
call->callee_is_locally_bound, res.strategy) ||
|
||||
cbm_go_suppress_weak_method_match(lang == CBM_LANG_GO, call->is_method, res.strategy,
|
||||
res.confidence);
|
||||
|
||||
/* Service-pattern HTTP/ASYNC client call (`requests.get(url)`): the
|
||||
* service signal lives in the callee_name. The registry can mis-resolve
|
||||
|
||||
@@ -297,6 +297,15 @@ bool cbm_suppress_weak_member_match(bool enabled, bool is_method, const char *st
|
||||
bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_bound,
|
||||
const char *strategy);
|
||||
|
||||
/* Go analog of the TS/JS guard, same failure class: a selector call whose
|
||||
* receiver the Go LSP could not type must not be bound by a receiver-blind
|
||||
* short-name strategy. Drops suffix_match / fuzzy always, and unique_name only
|
||||
* when its confidence is import-unreachability-penalized (the stdlib/vendor
|
||||
* hijack shape). field_type_hint is deliberately NOT dropped for Go — struct
|
||||
* fields carry declared types, so the hint is receiver-aware there. */
|
||||
bool cbm_go_suppress_weak_method_match(bool is_go, bool is_method, const char *strategy,
|
||||
double confidence);
|
||||
|
||||
/* #725: drop a suffix_match CALLS edge when the caller language and the
|
||||
* target file's language disagree. unique_name (candidates == 1) is #1572
|
||||
* and is left alone; same_module / import_map / lsp_* are kept. JS/TS/TSX
|
||||
|
||||
@@ -501,6 +501,33 @@ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_b
|
||||
return weak_short_name_strategy(strategy);
|
||||
}
|
||||
|
||||
bool cbm_go_suppress_weak_method_match(bool is_go, bool is_method, const char *strategy,
|
||||
double confidence) {
|
||||
if (!is_go || !is_method || !strategy || !strategy[0]) {
|
||||
return false;
|
||||
}
|
||||
/* Go analog of the TS/JS guard above, same failure class: a selector call
|
||||
* whose receiver the Go LSP could not type reaches the registry and a bare
|
||||
* short-name strategy binds it to an arbitrary same-named project symbol
|
||||
* (`f.Close()` on an os.File -> a project `Close`, suffix_match over 15
|
||||
* candidates). Unlike the TS/JS list, field_type_hint is KEPT: a Go struct
|
||||
* field carries a declared type, so the parallel resolver's field-type
|
||||
* hint is receiver-aware for Go (lrp_go_s8_field_type_hint), not a
|
||||
* heuristic. */
|
||||
if (strcmp(strategy, "suffix_match") == 0 || strcmp(strategy, "fuzzy") == 0) {
|
||||
return true;
|
||||
}
|
||||
/* unique_name is dropped only when PENALIZED: resolve_name_lookup scales
|
||||
* CONF_UNIQUE_NAME by DEFAULT_CONFIDENCE exactly when the lone candidate
|
||||
* is not reachable through the caller's imports — the stdlib/vendor
|
||||
* hijack shape (`io.Copy` -> a project `Copy`). An unpenalized
|
||||
* unique_name target sits inside the caller's import closure (or the
|
||||
* file has no imports, e.g. a same-package call) and must be kept —
|
||||
* dropping it kills genuinely-typed lone-candidate calls that never
|
||||
* enter the field-type-hint upgrade (candidate_count == 1). */
|
||||
return strcmp(strategy, "unique_name") == 0 && confidence < CONF_UNIQUE_NAME;
|
||||
}
|
||||
|
||||
static bool js_ts_family(CBMLanguage lang) {
|
||||
return lang == CBM_LANG_JAVASCRIPT || lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
|
||||
lang == CBM_LANG_ARKTS;
|
||||
|
||||
+36
-3
@@ -5204,9 +5204,14 @@ TEST(extract_perl_method_call_flags_is_method) {
|
||||
/* Languages OUTSIDE the is_method flag set (only Perl and TS/JS/TSX set it) must
|
||||
* be unaffected: a Go method call never sets is_method. */
|
||||
TEST(extract_flag_exempt_method_call_not_flagged_is_method) {
|
||||
CBMFileResult *r = extract("package m\n"
|
||||
"func run(o Obj) { o.Commit(); helper() }\n",
|
||||
CBM_LANG_GO, "t", "x.go");
|
||||
/* Rust is flag-exempt: only Perl, Python, TS/JS and Go set is_method.
|
||||
* Guards the blast radius of the receiver-aware flags for every other
|
||||
* language. */
|
||||
CBMFileResult *r = extract("fn run(o: Obj) {\n"
|
||||
" o.commit();\n"
|
||||
" helper();\n"
|
||||
"}\n",
|
||||
CBM_LANG_RUST, "t", "x.rs");
|
||||
ASSERT_NOT_NULL(r);
|
||||
ASSERT_FALSE(r->has_error);
|
||||
for (int i = 0; i < r->calls.count; i++) {
|
||||
@@ -5364,6 +5369,33 @@ TEST(extract_python_bare_call_flags_locally_bound_callee) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(extract_go_selector_call_flags_is_method) {
|
||||
/* Go selector calls are flagged so the weak-match guard can fire when the
|
||||
* Go LSP cannot type the receiver; bare calls stay unflagged. */
|
||||
CBMFileResult *r = extract("package m\n"
|
||||
"func run(o Obj) { o.Commit(); helper() }\n",
|
||||
CBM_LANG_GO, "t", "x.go");
|
||||
ASSERT_NOT_NULL(r);
|
||||
ASSERT_FALSE(r->has_error);
|
||||
bool saw_selector = false;
|
||||
bool saw_bare = false;
|
||||
for (int i = 0; i < r->calls.count; i++) {
|
||||
const CBMCall *c = &r->calls.items[i];
|
||||
if (c->callee_name && strstr(c->callee_name, "Commit") != NULL) {
|
||||
ASSERT_TRUE(c->is_method);
|
||||
saw_selector = true;
|
||||
}
|
||||
if (c->callee_name && strcmp(c->callee_name, "helper") == 0) {
|
||||
ASSERT_FALSE(c->is_method);
|
||||
saw_bare = true;
|
||||
}
|
||||
}
|
||||
ASSERT_TRUE(saw_selector);
|
||||
ASSERT_TRUE(saw_bare);
|
||||
cbm_free_result(r);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* The bare-call flag is DEPTH-INDEPENDENT, and the binding is UNWOUND when its
|
||||
* scope closes.
|
||||
*
|
||||
@@ -7508,6 +7540,7 @@ SUITE(extraction) {
|
||||
RUN_TEST(extract_python_member_call_flags_is_method);
|
||||
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_ts_member_call_flags_is_method);
|
||||
RUN_TEST(extract_ts_this_super_receiver_not_flagged);
|
||||
RUN_TEST(extract_js_member_call_flags_is_method);
|
||||
|
||||
@@ -5467,6 +5467,99 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(pipeline_go_receiver_suppresses_weak_method_edge) {
|
||||
char tmp[256];
|
||||
snprintf(tmp, sizeof(tmp), "/tmp/cbm_go_recv_XXXXXX");
|
||||
if (!cbm_mkdtemp(tmp)) {
|
||||
FAIL("tmpdir");
|
||||
}
|
||||
|
||||
/* go.mod makes project imports resolvable — real Go repos always have one,
|
||||
* and import reachability (the unique_name penalty) depends on it. */
|
||||
write_temp_file(tmp, "go.mod", "module example.com/myapp\n\ngo 1.22\n");
|
||||
/* The lone project symbol named "Close" — a real method. */
|
||||
write_temp_file(tmp, "storage/storage.go",
|
||||
"package storage\n"
|
||||
"\n"
|
||||
"type Storage struct{ open bool }\n"
|
||||
"\n"
|
||||
"func NewStorage() *Storage { return &Storage{open: true} }\n"
|
||||
"\n"
|
||||
"func (s *Storage) Close() {\n"
|
||||
"\ts.open = false\n"
|
||||
"}\n"
|
||||
"\n"
|
||||
"func Boot() {\n"
|
||||
"\ts := NewStorage()\n"
|
||||
"\ts.Close()\n"
|
||||
"}\n");
|
||||
/* Cross-package control target: imported by hash.go, so the caller file
|
||||
* has a non-empty import map (like any real Go file) and unreachable
|
||||
* unique_name candidates get the import penalty. */
|
||||
write_temp_file(tmp, "util/util.go",
|
||||
"package util\n"
|
||||
"\n"
|
||||
"func Tag() string { return \"t\" }\n");
|
||||
/* Stdlib receiver: `f.Close()` closes an *os.File, NOT the project method.
|
||||
* The Go LSP cannot bind it to a project symbol → the registry would guess
|
||||
* Close by short name (weak). This is the false edge to suppress —
|
||||
* the exact shape that attached every file/rows/gzip Close in a real Go
|
||||
* repo to one unrelated project method. */
|
||||
write_temp_file(tmp, "hash/hash.go",
|
||||
"package hash\n"
|
||||
"\n"
|
||||
"import (\n"
|
||||
"\t\"os\"\n"
|
||||
"\n"
|
||||
"\t\"example.com/myapp/util\"\n"
|
||||
")\n"
|
||||
"\n"
|
||||
"func FileLen(path string) int64 {\n"
|
||||
"\tf, err := os.Open(path)\n"
|
||||
"\tif err != nil {\n"
|
||||
"\t\treturn 0\n"
|
||||
"\t}\n"
|
||||
"\tdefer f.Close()\n"
|
||||
"\tst, err := f.Stat()\n"
|
||||
"\tif err != nil {\n"
|
||||
"\t\treturn 0\n"
|
||||
"\t}\n"
|
||||
"\treturn st.Size()\n"
|
||||
"}\n"
|
||||
"\n"
|
||||
"func localHelper() int { return 1 }\n"
|
||||
"\n"
|
||||
"func CallsLocal() int { return localHelper() }\n"
|
||||
"\n"
|
||||
"func UsesUtil() string { return util.Tag() }\n");
|
||||
|
||||
char db_path[512];
|
||||
snprintf(db_path, sizeof(db_path), "%s/go_recv.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);
|
||||
|
||||
/* (1) The false edge is suppressed (reproduce-first: RED before the fix). */
|
||||
ASSERT_FALSE(cross_file_call_exists(s, project, "FileLen", "Close"));
|
||||
/* (2) The same-package typed-receiver call survives (LSP / same_module —
|
||||
* both outside the weak drop-list). */
|
||||
ASSERT_TRUE(cross_file_call_exists(s, project, "Boot", "Close"));
|
||||
/* (3) The bare local call survives (is_method stays false for bare calls). */
|
||||
ASSERT_TRUE(cross_file_call_exists(s, project, "CallsLocal", "localHelper"));
|
||||
/* (4) The import-qualified cross-package call survives (import-aware
|
||||
* strategies are outside the drop-list). */
|
||||
ASSERT_TRUE(cross_file_call_exists(s, project, "UsesUtil", "Tag"));
|
||||
|
||||
cbm_store_close(s);
|
||||
cbm_pipeline_free(p);
|
||||
th_rmtree(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Fixture for the #1928 cross-language reference-guard probes (sequential and
|
||||
* parallel twins). pad_files > 0 adds filler files to push the index over the
|
||||
* parallel-pipeline threshold, since USAGE/WRITES/READS have one resolver per
|
||||
@@ -14594,6 +14687,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_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);
|
||||
RUN_TEST(pipeline_go_bare_ref_never_binds_field);
|
||||
|
||||
@@ -973,6 +973,46 @@ TEST(weak_call_guards_share_one_drop_list) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(go_suppress_drops_weak_selector_matches) {
|
||||
/* Go selector call with an untyped receiver, landed via a receiver-blind
|
||||
* short-name strategy → drop (same failure class as #592/#606).
|
||||
* suffix_match/fuzzy drop at any confidence; unique_name drops only when
|
||||
* import-unreachability-penalized (CONF_UNIQUE_NAME 0.75 * 0.5 = 0.375 —
|
||||
* the `io.Copy` -> project `Copy` stdlib-hijack shape). */
|
||||
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "suffix_match", 0.9));
|
||||
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "suffix_match", 0.11));
|
||||
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "fuzzy", 0.9));
|
||||
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "unique_name", 0.375));
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(go_suppress_keeps_typed_and_import_aware_matches) {
|
||||
/* Unpenalized unique_name = lone candidate inside the caller's import
|
||||
* closure (or an import-free file, e.g. same-package) — a genuinely-typed
|
||||
* lone-candidate call never enters the field-type-hint upgrade, so it must
|
||||
* survive (lrp_go_s8_field_type_hint). */
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "unique_name", 0.75));
|
||||
/* field_type_hint is receiver-aware for Go — struct fields carry declared
|
||||
* types (lrp_go_s8_field_type_hint) — so it stays, unlike the TS/JS list. */
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "field_type_hint", 0.85));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "same_module", 0.9));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "import_map", 0.95));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "import_map_suffix", 0.9));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "qualified_suffix", 0.9));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "callee_suffix", 0.5));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "service_pattern", 0.5));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "lsp_strategy_cross_file", 0.92));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "lsp_direct", 0.95));
|
||||
/* A bare call (is_method=false) is a free-function call → never suppressed. */
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, false, "suffix_match", 0.11));
|
||||
/* Non-Go languages are never affected by this gate. */
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(false, true, "suffix_match", 0.11));
|
||||
/* No match (NULL/empty strategy) → nothing to suppress. */
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, NULL, 0.5));
|
||||
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "", 0.5));
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── Suite ─────────────────────────────────────────────────────── */
|
||||
|
||||
/* Method call THROUGH an imported symbol that is itself an indexed node
|
||||
@@ -1072,4 +1112,6 @@ SUITE(registry) {
|
||||
RUN_TEST(local_binding_suppress_drops_weak_shadowed_bare_calls);
|
||||
RUN_TEST(local_binding_suppress_keeps_unshadowed_and_strong_strategies);
|
||||
RUN_TEST(weak_call_guards_share_one_drop_list);
|
||||
RUN_TEST(go_suppress_drops_weak_selector_matches);
|
||||
RUN_TEST(go_suppress_keeps_typed_and_import_aware_matches);
|
||||
}
|
||||
Reference in new issue
Block a user