Skip to content

fix(csharp): link type arguments on new T<...>(), not just at call sites - #3097

Open
brobl2008 wants to merge 1 commit into
Graphify-Labs:v8from
brobl2008:fix/csharp-generic-args-in-object-creation
Open

fix(csharp): link type arguments on new T<...>(), not just at call sites#3097
brobl2008 wants to merge 1 commit into
Graphify-Labs:v8from
brobl2008:fix/csharp-generic-args-in-object-creation

Conversation

@brobl2008

Copy link
Copy Markdown
Contributor

The gap

The call-site fix in #2911 walks a type-argument list reached through an invocation's
function field, so both of these link their arguments:

recv.Do<Widget>();                       // member call
services.AddScoped<IGadget, Gadget>();   // the DI shape

object_creation_expression keeps the constructed type in its type field instead, and
_read_csharp_type_name names only that type — so the whole argument list was dropped:

var w = new Wrapper<Widget>();   // links Wrapper, loses Widget entirely

Same language feature, two syntactic positions, only one of them emitting edges.

Why it matters

It matters most for a type reached only through a construction — a generic wrapper built
around a collaborator, a typed collection built from a literal. There the dropped argument
is the sole edge recording the dependency, so the type reads as unused and reverse-dependency
queries miss every consumer.

Measured on a private ~90k-node C# graph, roughly 9,000 construction sites carried type
arguments, about a third of them the generic-wrapper shape.

The change

Both positions now go through one helper, so the two cannot drift apart again. Discovery
differs by necessity, and each half says why:

  • A call target is inspected precisely. The receiver of outer<A>.Inner<B>() sits inside
    the same node, and its arguments belong to the receiver — a subtree search would attribute
    them to the call.
  • A constructed type is searched into. new A.B.Wrapper<Widget>() nests the
    generic_name inside a qualified_name. Bounding that search to the type field is what
    keeps constructor arguments out: they are a sibling field, never inside the type.

Type parameters remain excluded — new Wrapper<T>() inside Make<T>() links nothing,
because the existing _csharp_type_parameters_in_scope filter carries over with the shared
helper. There is a test pinning that.

Tests

Nine new tests covering the interface argument, the class argument, a collection element
type, both arguments of a two-parameter type, a nested argument (new Wrapper<Bag<Widget>>()),
a namespace-qualified type, and the object-initializer form — plus two guards: the
pre-existing calls edge to the constructed type must survive, and a type parameter must not
be linked as a type. Seven of the nine fail before the change and all nine pass after; the
two guards pass both ways by design.

Full suite: identical failure set before and after, +9 passing. One pre-existing failure
is flaky between runs (25 on one baseline run, 24 on another), which accounts for any count
difference — the failure sets match.

--no-cluster extraction on a C#-only corpus, no new node kinds, no new relation: the edges
are the same references[generic_arg] the call-site path already emits.

…sites

The call-site fix in Graphify-Labs#2911 walks a type-argument list reached through an
invocation's `function` field, so `Do<T>()` and `services.AddScoped<ISvc,
Impl>()` link their arguments. `object_creation_expression` keeps the
constructed type in its `type` field instead and `_read_csharp_type_name`
names only that type, so the whole argument list was dropped: `new
Wrapper<Widget>()` linked `Wrapper` and lost `Widget` entirely.

That matters most for a type reached only through a construction -- a generic
wrapper built around a collaborator, a typed collection built from a literal.
The dropped argument was the sole edge recording the dependency, so the type
looked unused. Measured on a private ~90k-node C# graph, roughly 9,000 sites
were affected, about a third of them the generic-wrapper shape.

Both positions now share one helper, so the same language feature cannot drift
between them again. Discovery differs by necessity and is documented on each:
a call target is inspected precisely, because the receiver of
`outer<A>.Inner<B>()` sits inside the same node and its arguments belong to the
receiver; a constructed type is searched into, because `new
A.B.Wrapper<Widget>()` nests the `generic_name` inside a `qualified_name`, and
bounding that search to the `type` field is what keeps constructor arguments
out.

Type parameters are still excluded, so `new Wrapper<T>()` inside `Make<T>()`
links nothing -- the existing `_csharp_type_parameters_in_scope` filter carries
over with the shared helper.

Full suite: identical failure set before and after (one pre-existing failure is
flaky between runs), +9 passing from the new tests.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. 1 change(s) tested, no difference found (not proven).


Graphify review — findings

Adds references[generic_arg] edges for C# type arguments on new Foo<Bar>() constructions, so new Wrapper<Widget>() now links both Wrapper and Widget instead of silently dropping the argument. Refactors the existing call-site logic into shared helpers — _csharp_call_type_argument_list locates the type-argument list for static and member invocations, _csharp_constructed_type_argument_list descends the type field for constructions, and _emit_csharp_generic_arg_edges emits the edges — so both call and object-creation positions stay in step. Adds a test covering generic wrappers and collections built only via construction, where the dropped argument was the sole edge recording the dependency.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 637 functions depend on the 225 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _extract_generic() — 18 callers, 24 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: extract_objc() — 27 callers, 9 callees
  • new: extract_js() — 80 callers, 3 callees
  • new: extract_julia() — 17 callers, 7 callees
  • new: extract_cpp() — 27 callers, 3 callees
  • new: extract_vue() — 10 callers, 6 callees
  • new: walk() — 1 callers, 56 callees
  • …and 8 more — each is listed as a finding

Verification — 637 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 577 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify \_infer\_merge\_root.

The verifier did not have enough to check \_infer\_merge\_root, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `graph_path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify prefix\_graph\_for\_global.

The verifier did not have enough to check prefix\_graph\_for\_global, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 115 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_is\_ignored.

The verifier did not have enough to check \_is\_ignored, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

No difference found (not proven): No behavior difference found in \_match\_anchored\_ignore\_pattern (not a proof).

The verifier ran both versions of \_match\_anchored\_ignore\_pattern on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify detect.

The verifier did not have enough to check detect, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_csharp\_extra\_walk.

The verifier did not have enough to check \_csharp\_extra\_walk, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_extract\_generic.

The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_js\_extra\_walk.

The verifier did not have enough to check \_js\_extra\_walk, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_ts\_extra\_walk.

The verifier did not have enough to check \_ts\_extra\_walk, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify introspect\_postgres.

The verifier did not have enough to check introspect\_postgres, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ProgrammingError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_resolve\_source\_path.

The verifier did not have enough to check \_resolve\_source\_path, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `graph_path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify resolve\_ruby\_member\_calls.

The verifier did not have enough to check resolve\_ruby\_member\_calls, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous

Could not verify: Could not verify \_\_init\_\_.

The verifier did not have enough to check \_\_init\_\_, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)

Could not verify: Could not verify on\_any\_event.

The verifier did not have enough to check on\_any\_event, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: method — first parameter is `self`, which needs a constructed instance (not synthesizable)

Could not verify: Could not verify watch.

The verifier did not have enough to check watch, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `watch_path` is annotated `Path` — outside the synthesizable primitive/collection set

· 16 more finding(s) on lines outside this diff (see the check run).

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.

1 participant