Skip to content

csharp: stop member calls from binding to the calling method itself - #516

Merged
zzet merged 2 commits into
zzet:mainfrom
pbednarcik:fix/csharp-facade-self-steal
Aug 9, 2026
Merged

csharp: stop member calls from binding to the calling method itself#516
zzet merged 2 commits into
zzet:mainfrom
pbednarcik:fix/csharp-facade-self-steal

Conversation

@pbednarcik

Copy link
Copy Markdown
Contributor

Title: csharp: stop member calls from binding to the calling method itself

Problem

Field-testing on my production C# codebase: callers on a repository
interface method never returns the service that calls it through a
ctor-injected field — the dominant DI/delegation pattern there. The graph
shows why: the service's call is bound to the calling method itself as a
0.9-confidence self-loop, and no edge into the interface exists at all.

The trigger is the facade shape — a service wrapping a repository method
under the same name:

public class FolioService {
    private readonly IFolioArchive _archive;

    public IReadOnlyList<int> FetchSealedFolios(int year) {
        return _archive.FetchSealedFolios(year);   // → bound to FolioService.FetchSealedFolios (itself)
    }
}

Sibling methods whose names differ from the wrapped method (e.g. an Async
suffix) bind correctly at 0.95 via csharp-types — which is what hid this for
so long.

Root cause (two layers)

  1. The caller-receiver fallback is receiver-blind. When
    resolveMethodCall reaches the fallback with no receiver_type (fields
    never reach the extraction tenv), it asks only "does the caller's own
    class declare this name?" — and for a facade the answer is yes, so the
    call binds to the enclosing method at 0.9 with no Origin stamp. The PHP
    shield directly above the fallback documents this exact self-bind failure
    for $this->handler->setFormatter(); C# had no equivalent.
  2. The unstamped 0.9 masquerades as AST-grade. DefaultOriginFor maps
    confidence >= 0.9 to ast_resolved, so the tstypes applier's
    claimable() ranks the guess at the AST ceiling. Enrichment then computes
    the correct interface target, hits the same-line self edge in
    upgradeOrCreateCall, and honours don't-double-the-call-site — the right
    edge is silently dropped, permanently.

A member_call edge that reaches the fallback untyped necessarily has an
explicit receiver that is not this/base (those carry receiver_type
from extraction), so the caller's own member set is no evidence about it.

Fix

  • resolveMethodCall: exempt member_call edges from the caller-receiver
    fallback, mirroring the PHP shield; and never let the locality fallback
    bind a member call to its own caller (x.Foo() inside Foo is the facade
    shape, not recursion — recursion is an unqualified call).
  • tstypes claimable: an origin-unstamped member_call bind with no
    resolution/semantic_source marker came from the name-locality tiers by
    construction — treat it as claimable regardless of the backfilled
    confidence. This half also heals existing stores on their next
    enrichment pass: already-resolved edges never re-enter the resolver, so
    without it every deployed index keeps its self-loops until a full rebuild.

The member_call marker is stamped only by the C# extractor, so both gates
are C#-scoped by construction; no other language's fallback behavior
changes. No extractor changes, no version bump.

Behavior changes worth knowing (reviewed for blast radius)

  • Member calls on fields typed as the enclosing class lose the old
    type-filtered 0.9 bind. Wrapped targets heal to a true 0.95 via
    csharp-types enrichment (fields are exactly what it types); in the window
    before enrichment — or with the C# provider disabled — these sites sit at
    text_matched/unresolved instead. Recursion through a self-typed field
    (next.Print() inside Print — linked lists, chains of responsibility)
    is covered: the applier's self-guard now claims the extracted stub when
    typed-field evidence names the calling method itself, while minting fresh
    self-edges stays forbidden.
  • Scoring/tier consumers: the affected population previously backfilled
    to ast_resolved (the conf>=0.9 rule); as explicit text_matched it now
    weighs honestly in path confidence, provenance weighting, and min_tier
    filtering until enrichment lands the real bind at 0.95.
  • Suppression/dispatch precision: the old unstamped 0.9 binds escaped
    SuppressRedundantTextMatches and could seed interface-dispatch fan-out
    from wrong callers; as stamped text_matched they suppress and fan
    correctly.
  • Existing stores: persisted self-loops are reused as-is by incremental
    indexing and heal at each file's next enrichment pass (the claimable
    half), not at reuse time.

Tests

TDD, both layers watched red first:

  • TestCSharpFacade_MemberCallDoesNotStealToSelf — the facade fixture
    through the real extractor + ResolveAll; red on the self-loop at
    0.9/no-origin, green with the target off-self and any unstamped bind below
    0.9. Control: this.Helper() still binds its own class at ≥0.9.
  • TestCSharp_FacadeSelfLoopIsClaimable — reproduces the resolver damage on
    an extracted fixture (self-target, 0.9, Origin=""), runs Enrich, and
    requires the site retargeted to the interface method at AST provenance
    with the self-loop gone. Guard rail:
    TestCSharp_ExplicitASTResolvedBindStaysUnclaimed pins that an edge
    explicitly stamped ast_resolved is still never retargeted.
  • TestCSharp_SelfTypedFieldRecursionClaimsStub — the population an
    adversarial review round caught the first cut orphaning: recursion
    through a field typed as the enclosing class. The resolver gate leaves it
    a stub by design, and the applier's self-guard used to refuse the claim
    too, stranding the site. upgradeOrCreateCall is split so the
    self-target path claims an existing stub without ever being allowed to
    create; watched red before the split.

Same review round: cross_repo's exact-type tiers now stamp
OriginASTResolved like the main resolver's typed passes — unstamped,
their receiver-typed binds were indistinguishable from the name-tier family
claimable() may reclaim.

Suites: resolver/indexer failure name-sets byte-identical to clean main on
my Windows box (pre-existing platform failures only, worktree-verified);
tstypes and languages fully green.

Validation

Deployed on a rebuilt binary with a from-scratch index:

  • My counted C# fixture repo's facade cell flipped from the pinned
    self-loop to the wrapped interface method at 0.95 (csharp-types) plus the
    concrete impl at 0.85 — and a second, older pinned cell (the original
    sighting of this steal: a controller's _repository.Read grabbed by its
    own Read) flipped green with it. No other pinned cell drifted.
  • On the production repo, the acceptance pair now exists in the graph: the
    service's call binds the repository interface method at 0.95 with the
    impl fan-out at 0.85; the self-loop and the spurious service-interface
    mirror are gone. Re-running my acceptance probe closed the loop — a
    single callers query on the interface method now walks the full
    production chain, including a second interface-dispatch hop to the
    consumer the original investigation started from.

A member call through an untyped receiver (a ctor-injected field, the
dominant DI pattern) could bind to the enclosing class's own same-named
method: the caller-receiver fallback asks only whether the caller's
class declares the name, so a facade wrapping a same-named repository
method became a 0.9 self-loop with no Origin stamp. DefaultOriginFor
then backfills conf>=0.9 to ast_resolved, so the tstypes applier
computed the correct target and refused to claim the site.

- resolveMethodCall: exempt member_call edges from the caller-receiver
  fallback (mirrors the PHP shield above it) and never locality-bind a
  member call to its own caller.
- tstypes claimable: an origin-unstamped member_call bind without a
  resolution/semantic_source marker is name evidence regardless of its
  backfilled confidence — claim it, so enrichment heals existing
  stores without a reindex.
…repo exact-type origins

Review found the facade gate orphaned one legitimate population: recursion
through a field typed as the enclosing class (next.Print() inside Print).
The resolver now leaves it a stub by design, but applyCall's self-guard
also refused it, so the site went permanently unresolved. Split
upgradeOrCreateCall so the self-target path claims an existing stub
without ever minting a fresh self-edge.

Also: cross_repo's exact-type tiers now stamp OriginASTResolved like the
main resolver's typed passes — unstamped, their receiver-typed binds were
indistinguishable from the name-tier family claimable() may reclaim.
Comment accuracy fixes and a non-vacuity assert in the facade test.
@zzet
zzet merged commit e73576a into zzet:main Aug 9, 2026
10 checks passed
@pbednarcik
pbednarcik deleted the fix/csharp-facade-self-steal branch August 9, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants