From 349aecba9528427fa635975e246f5b8959b14f30 Mon Sep 17 00:00:00 2001 From: Ilya Brykau Date: Wed, 2 Sep 2026 10:39:03 +0200 Subject: [PATCH] fix(pipeline): key the Go Field guard on the recorded selector shape, not reference text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cbm_go_suppress_bare_field_ref dropped a Field-targeted reference when strchr(ref_name, '.') == NULL — but the extractor strips the receiver on every path that reaches the resolver (resolve_lhs_write_name records the trailing field name of a selector LHS; is_reference_node records the inner field_identifier), so the dot test could never be false for Go. The guard was a blanket veto: all ~4588 Go Field nodes recovered by one 61de19bb were unreachable by USAGE/READS/WRITES, including genuine selector references (issue #1962, found in maintainer post-merge review of #1944). The selector-vs-bare distinction exists at extraction time and was discarded; record it and let the resolver consume it: - CBMUsage/CBMReadWrite gain is_member_access. The usage recorder sets it when the reference node is a field_identifier (the member half of a selector); resolve_lhs_write_name reports it through an out-param when it takes the field/member LHS branch. - cbm_go_suppress_bare_field_ref(is_go, is_member_access, target_label) refuses a Field bind only for references that were never the member half of a selector. All four resolver sites (sequential and parallel, USAGE and READS/WRITES) pass the recorded flag. Reproduce-first: with the veto restored, the extended #1942 fixtures go RED on both resolver paths at the new asserts (t.err = nil must WRITE the field; t.n must produce USAGE); with the fix both twins are GREEN and the original bare-local negatives still hold. The dead ASSERT_FALSE(..., "t.err", ...) unit case — an input Go's extractor cannot produce — is replaced by flag-based cases. Fixes #1962. Signed-off-by: Ilya Brykau --- internal/cbm/cbm.h | 7 +++++++ internal/cbm/extract_semantic.c | 16 +++++++++++++--- internal/cbm/extract_usages.c | 4 ++++ src/pipeline/pass_parallel.c | 12 +++++++----- src/pipeline/pass_usages.c | 12 +++++++----- src/pipeline/pipeline.h | 2 +- src/pipeline/registry.c | 27 +++++++++++++-------------- tests/test_pipeline.c | 20 ++++++++++++++++++++ tests/test_registry.c | 27 ++++++++++++++------------- 9 files changed, 86 insertions(+), 41 deletions(-) diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index 4f06bebb7..e9d307c57 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -292,6 +292,10 @@ typedef struct { uint32_t site_start_byte; // exact reference-token span; end > start when present uint32_t site_end_byte; // exclusive byte offset in the source file CBMSourceOrigin source_origin; // raw source or C-family preprocessed buffer + bool is_member_access; // token is the member half of a selector/attribute + // (Go x.f — field_identifier). The extractor strips + // the receiver, so this is the only surviving record + // of selector shape (#1962). Default false. } CBMUsage; typedef struct { @@ -303,6 +307,9 @@ typedef struct { const char *var_name; // variable name const char *enclosing_func_qn; // QN of enclosing function bool is_write; // true = write, false = read + bool is_member_access; // var_name is the field half of a selector/member LHS + // (`t.err = x` → "err"); the receiver is stripped here, + // so this is the only record of selector shape (#1962) } CBMReadWrite; typedef struct { diff --git a/internal/cbm/extract_semantic.c b/internal/cbm/extract_semantic.c index 9379f7171..36709d2dd 100644 --- a/internal/cbm/extract_semantic.c +++ b/internal/cbm/extract_semantic.c @@ -146,7 +146,10 @@ static void walk_throws(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec // - field/member/selector access (`self.total = ...`, `obj.Field = ...` → // write the trailing field name `total`/`Field`) // Returns NULL if no simple write target can be determined. -static char *resolve_lhs_write_name(CBMExtractCtx *ctx, TSNode left) { +// Sets *is_member_out when the written name is the field half of a +// member/selector LHS — the receiver is stripped from the returned name, so +// this out-param is the only surviving record of the selector shape (#1962). +static char *resolve_lhs_write_name(CBMExtractCtx *ctx, TSNode left, bool *is_member_out) { // Unwrap a single-element expression_list (Go). if (strcmp(ts_node_type(left), "expression_list") == 0) { if (ts_node_named_child_count(left) != 1) { @@ -184,6 +187,9 @@ static char *resolve_lhs_write_name(CBMExtractCtx *ctx, TSNode left) { fld = ts_node_child_by_field_name(left, TS_FIELD("name")); } if (!ts_node_is_null(fld)) { + if (is_member_out) { + *is_member_out = true; + } return cbm_node_text(ctx->arena, fld, ctx->source); } return NULL; @@ -251,12 +257,14 @@ static void try_emit_assignment_write(CBMExtractCtx *ctx, TSNode node, const cha if (ts_node_is_null(left)) { return; } - char *name = resolve_lhs_write_name(ctx, left); + bool is_member = false; + char *name = resolve_lhs_write_name(ctx, left, &is_member); if (name && name[0] && !cbm_is_keyword(name, ctx->language)) { CBMReadWrite rw; rw.var_name = name; rw.is_write = true; rw.enclosing_func_qn = func_qn; + rw.is_member_access = is_member; cbm_rw_push(&ctx->result->rw, ctx->arena, rw); } } @@ -329,12 +337,14 @@ void handle_readwrites(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec *spec, TSNode left = resolve_write_lhs_node(node); if (!ts_node_is_null(left)) { - char *name = resolve_lhs_write_name(ctx, left); + bool is_member = false; + char *name = resolve_lhs_write_name(ctx, left, &is_member); if (name && name[0] && !cbm_is_keyword(name, ctx->language)) { CBMReadWrite rw; rw.var_name = name; rw.is_write = true; rw.enclosing_func_qn = state->enclosing_func_qn; + rw.is_member_access = is_member; cbm_rw_push(&ctx->result->rw, ctx->arena, rw); } } diff --git a/internal/cbm/extract_usages.c b/internal/cbm/extract_usages.c index 4e3218fd1..fd83864ac 100644 --- a/internal/cbm/extract_usages.c +++ b/internal/cbm/extract_usages.c @@ -2673,6 +2673,10 @@ void handle_usages(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec *spec, Wal usage.ref_name = name; usage.enclosing_func_qn = state->enclosing_func_qn; usage.lexical_scope_id = usage_lexical_scope_id_for_node(ctx, state, node); + /* The member half of a selector is its own reference node + * (field_identifier); record that shape — the name alone cannot carry + * it, and the Go Field guard keys on it (#1962). */ + usage.is_member_access = strcmp(ts_node_type(node), "field_identifier") == 0; stamp_usage_site(ctx, &usage, node, name, state); cbm_usages_push(&ctx->result->usages, ctx->arena, usage); } diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 1ce766927..77a64c408 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2670,9 +2670,10 @@ static void resolve_file_usages(resolve_ctx_t *rc, resolve_worker_state_t *ws, if (tgt && cbm_suppress_cross_language_ref(lang, tgt->file_path)) { continue; } - /* #1942: a bare Go reference can never denote a struct field. */ - if (tgt && - cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, usage->ref_name, tgt->label)) { + /* #1942/#1962: a bare Go reference can never denote a struct + * field; the member half of a selector may. */ + if (tgt && cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, usage->is_member_access, + tgt->label)) { continue; } if (usage->semantic_reference_blocked && (usage->semantic_reference_local_shadow || @@ -2760,8 +2761,9 @@ static void resolve_file_rw(resolve_ctx_t *rc, resolve_worker_state_t *ws, CBMFi if (cbm_suppress_cross_language_ref(lang, tgt->file_path)) { continue; } - /* #1942: a bare Go reference can never denote a struct field. */ - if (cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, rw->var_name, tgt->label)) { + /* #1942/#1962: a bare Go reference can never denote a struct field; + * a selector-LHS write (`t.err = x`) may bind it. */ + if (cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, rw->is_member_access, tgt->label)) { continue; } const char *etype = rw->is_write ? "WRITES" : "READS"; diff --git a/src/pipeline/pass_usages.c b/src/pipeline/pass_usages.c index 690b45ad9..9738f7198 100644 --- a/src/pipeline/pass_usages.c +++ b/src/pipeline/pass_usages.c @@ -216,9 +216,10 @@ static int resolve_usage_edges(cbm_pipeline_ctx_t *ctx, const CBMFileResult *res if (tgt && cbm_suppress_cross_language_ref(lang, tgt->file_path)) { continue; } - /* #1942: a bare Go reference can never denote a struct field. */ - if (tgt && - cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, usage->ref_name, tgt->label)) { + /* #1942/#1962: a bare Go reference can never denote a struct + * field; the member half of a selector may. */ + if (tgt && cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, usage->is_member_access, + tgt->label)) { continue; } if (usage->semantic_reference_blocked && (usage->semantic_reference_local_shadow || @@ -309,8 +310,9 @@ static int resolve_rw_edges(cbm_pipeline_ctx_t *ctx, const CBMFileResult *result if (cbm_suppress_cross_language_ref(lang, tgt->file_path)) { continue; } - /* #1942: a bare Go reference can never denote a struct field. */ - if (cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, rw->var_name, tgt->label)) { + /* #1942/#1962: a bare Go reference can never denote a struct field; + * a selector-LHS write (`t.err = x`) may bind it. */ + if (cbm_go_suppress_bare_field_ref(lang == CBM_LANG_GO, rw->is_member_access, tgt->label)) { continue; } diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index d174bb6d6..109da9781 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -302,7 +302,7 @@ bool cbm_suppress_cross_language_ref(CBMLanguage caller_lang, const char *target * Field when the reference text carries no '.'. Go only: other OO languages * legitimately reference their own members bare inside method bodies. Pure; * unit-tested in test_registry.c. */ -bool cbm_go_suppress_bare_field_ref(bool is_go, const char *ref_name, const char *target_label); +bool cbm_go_suppress_bare_field_ref(bool is_go, bool is_member_access, const char *target_label); /* Get the label of a qualified name, or NULL if not found. */ const char *cbm_registry_label_of(const cbm_registry_t *r, const char *qn); diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index eefdb7596..b29eba494 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -546,22 +546,21 @@ bool cbm_suppress_cross_language_ref(CBMLanguage caller_lang, const char *target return true; } -bool cbm_go_suppress_bare_field_ref(bool is_go, const char *ref_name, const char *target_label) { - /* #1942: a bare (dot-less) Go reference can never denote a struct field — - * field access is always a selector expression (x.f), and selector - * references resolve through the LSP join, never through the bare-name - * registry fallback. Every Field-targeted reference edge in the field - * census carried dot-less text, so dropping the bind loses nothing real. - * Go-gated: a C#/Java/C++/Python method body legitimately references its - * own members bare (cp_reads_writes_cs_static_field pins that shape as - * required), so a global veto would break those languages. */ - if (!is_go || !ref_name || !ref_name[0] || !target_label) { +bool cbm_go_suppress_bare_field_ref(bool is_go, bool is_member_access, const char *target_label) { + /* #1942/#1962: a bare Go identifier can never denote a struct field — + * field access is always a selector expression (x.f). The extractor + * strips the receiver before the resolver runs (resolve_lhs_write_name + * records the trailing field name; is_reference_node records the inner + * field_identifier), so the reference TEXT is always dot-less and cannot + * carry the distinction — the recorded is_member_access shape can. Only a + * reference that was never the member half of a selector is refused a + * Field bind. Go-gated: a C#/Java/C++/Python method body legitimately + * references its own members bare (cp_reads_writes_cs_static_field pins + * that shape as required), so a global veto would break those languages. */ + if (!is_go || is_member_access || !target_label) { return false; } - if (strcmp(target_label, "Field") != 0) { - return false; - } - return strchr(ref_name, '.') == NULL; + return strcmp(target_label, "Field") == 0; } /* ── Lifecycle ──────────────────────────────────────────────────── */ diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index d62a5572f..2af0366f8 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -4857,6 +4857,17 @@ static void write_go_bare_field_fixture(const char *tmp, int pad_files) { "\terr := errors.New(\"x\")\n" "\treturn err\n" "}\n"); + /* #1962: genuine selector references from a sibling file of the same + * package. `t.err = nil` writes the field through a selector; `t.n` reads + * it. The extractor strips the receiver on both paths, so only the + * is_member_access signal can distinguish these from Run's bare local. */ + write_temp_file(tmp, "state/reset.go", + "package state\n" + "\n" + "func (t *Tracker) Reset() int {\n" + "\tt.err = nil\n" + "\treturn t.n\n" + "}\n"); for (int i = 0; i < pad_files; i++) { char name[64]; char body[128]; @@ -4896,6 +4907,11 @@ TEST(pipeline_go_bare_ref_never_binds_field) { ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "WRITES")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "READS")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "USAGE")); + /* #1962, reproduce-first: RED while the guard is a blanket veto — genuine + * selector references must reach the field (write via `t.err = nil`, + * value use via `t.n`). */ + ASSERT_TRUE(cross_file_edge_exists(s, project, "Reset", "err", "WRITES")); + ASSERT_TRUE(cross_file_edge_exists(s, project, "Reset", "n", "USAGE")); cbm_store_close(s); cbm_pipeline_free(p); @@ -4927,6 +4943,10 @@ TEST(pipeline_go_bare_ref_never_binds_field_parallel) { ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "WRITES")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "READS")); ASSERT_FALSE(cross_file_edge_exists(s, project, "Run", "err", "USAGE")); + /* #1962 parallel twin: resolve_file_rw / resolve_file_usages must honour + * the member-access signal exactly like the sequential resolvers. */ + ASSERT_TRUE(cross_file_edge_exists(s, project, "Reset", "err", "WRITES")); + ASSERT_TRUE(cross_file_edge_exists(s, project, "Reset", "n", "USAGE")); cbm_store_close(s); cbm_pipeline_free(p); diff --git a/tests/test_registry.c b/tests/test_registry.c index 3e1d604af..0f3f3861a 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -840,22 +840,23 @@ TEST(cross_language_ref_drops_go_vs_c) { } TEST(go_bare_ref_never_binds_field) { - /* #1942: a bare (dot-less) Go reference can never denote a struct field — - * field access is always a selector expression. */ - ASSERT_TRUE(cbm_go_suppress_bare_field_ref(true, "err", "Field")); - ASSERT_TRUE(cbm_go_suppress_bare_field_ref(true, "config", "Field")); - /* A selector-shaped reference may bind a field. */ - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "t.err", "Field")); + /* #1942/#1962: a bare Go identifier can never denote a struct field — + * field access is always a selector expression. The extractor strips the + * receiver before the resolver runs (resolve_lhs_write_name writes the + * trailing name; is_reference_node records the inner field_identifier), + * so the selector-vs-bare distinction arrives as the recorded + * is_member_access signal, never as a dot in the reference text. */ + ASSERT_TRUE(cbm_go_suppress_bare_field_ref(true, false, "Field")); + /* The member half of a selector may bind a field. */ + ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, true, "Field")); /* Bare references to non-fields are untouched. */ - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "err", "Variable")); - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "err", "Function")); + ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, false, "Variable")); + ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, false, "Function")); /* Other languages reference their own members bare inside methods — * never suppressed (cp_reads_writes_cs_static_field pins the C# shape). */ - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(false, "_count", "Field")); - /* Degenerate inputs → nothing to judge. */ - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, NULL, "Field")); - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "", "Field")); - ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "err", NULL)); + ASSERT_FALSE(cbm_go_suppress_bare_field_ref(false, false, "Field")); + /* Degenerate input → nothing to judge. */ + ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, false, NULL)); PASS(); }