Skip to content

fix(csharp): walk generic type arguments at call sites (#2911) - #2930

Open
santhiprakash wants to merge 1 commit into
Graphify-Labs:v8from
santhiprakash:fix/csharp-call-site-generic-type-args
Open

fix(csharp): walk generic type arguments at call sites (#2911)#2930
santhiprakash wants to merge 1 commit into
Graphify-Labs:v8from
santhiprakash:fix/csharp-call-site-generic-type-args

Conversation

@santhiprakash

Copy link
Copy Markdown

Problem

The C# invocation_expression handler captured the callee name but never walked the call site's type-argument list. Result: generic arguments at call sites were silently dropped from the graph.

r.Do<IEpsilon>()                                 // IEpsilon: no edge
services.AddScoped<IZeta, Box<IZeta>>()          // IZeta, IZeta, Box: no edges
StaticHolder.Invoke<IThing>()                    // IThing: no edge

The graph affected query returns a confident, smaller answer instead of an error -- a real dependency edge is gone. Reproduces with the issue's two-file repro via graphify update . --no-cluster (pure AST, deterministic, no LLM).

Why this fix

The property/return/parameter branches above the call-site branch already close the same bug in adjacent positions -- the property_declaration branch (around line 3746) is the direct analogue: it walks the declared type with _csharp_collect_type_refs and emits a references[generic_arg] edge per type argument. The call-site branch just never got the parallel treatment.

This fix is intentionally bounded to the call-site case:

  • The field_declaration case is the parallel fix in fix(csharp): walk generic type arguments in field position #2913 (in flight from the issue reporter).
  • Properties, returns, parameters, base lists, primary constructor parameters, and inherited interfaces were already correct.
  • The two PRs are complementary: this one closes the second of two halves the issue splits.

Fix

In the C# branch of the walk_calls closure (in graphify/extractors/engine.py, after the existing callee/member-receiver extraction), look for a type_argument_list on the call's function node:

  • recv.Do<T>() -- the function is a member_access_expression whose name is a generic_name carrying the type-arg list.
  • Foo<Bar>() -- the function is a generic_name directly.
  • The fallback path (raw-text name scan) is left alone; it never sees the structured type-arg list and is the same path that drops them for the callee, so reconstructing them there would be lossy.

For each type argument, walk it with the same _csharp_collect_type_refs(generic=True, skip=...) helper the other C# branches use, then emit a references edge with context="generic_arg" from the caller (the method node). The skip set is the class's and method's in-scope type parameters (the same _csharp_type_parameters_in_scope walk), so a Holder<T>.Use(r => r.Do<T>()) doesn't fabricate a node for the type parameter T.

Test

Adds tests/test_csharp_call_site_generic_args.py with six cases that fail on pre-fix code and pass with the fix:

  • r.Do<IThing>() -- one-arg member call (the case the issue lists as position 5).
  • s.AddScoped<IService, IImpl>() -- two-arg call (the Microsoft.Extensions.DependencyInjection shape the issue calls out as position 6).
  • r.Do<Box<IThing>>() -- nested type argument at a call site.
  • r.Do() -- call without type arguments is unchanged (regression guard).
  • Holder<T>.Use(r => r.Do<T>()) -- type parameter used as the call-site type argument must not fabricate a T node.
  • The issue's two-file repro end-to-end: all six positions from the issue's table produce their expected edges (the field case is closed by fix(csharp): walk generic type arguments in field position #2913; this PR is responsible for the call-site half).

Each test asserts the edge EXISTS, not its count -- absence is the bug; counts are an implementation detail.

Verification

  • .venv/bin/python -m pytest tests/test_csharp_call_site_generic_args.py -- 6 passed.
  • .venv/bin/python -m pytest tests/test_languages.py tests/test_csharp_*.py tests/test_extractors_registry.py tests/test_language_resolvers.py tests/test_affected_member_seed.py tests/test_affected_cli.py -- 488 passed, 13 skipped, 0 failed.
  • End-to-end on the issue's two-file repro via graphify update . --no-cluster: positions 2-6 from the issue's table each emit their expected generic_arg reference (position 1, the field case, is the parallel fix(csharp): walk generic type arguments in field position #2913's responsibility).

Relationship to #2913

#2913 (the issue reporter's PR) fixes the field-position case (private Box<IAlpha> _field; losing IAlpha) by walking the field's declared type in the field_declaration handler. This PR fixes the call-site case (r.Do<T>() and s.AddScoped<T1, T2>() losing the type arguments) by walking the call's type-argument list in the invocation_expression handler. They are complementary, non-overlapping, and target the two halves the issue splits into. Both can merge independently; together they close the bug.

The C# invocation_expression handler captured the callee name but never
walked the call's type-argument list, so generic arguments at call sites
were dropped from the graph:

  r.Do<IEpsilon>()                                 -- IEpsilon: no edge
  services.AddScoped<IZeta, Box<IZeta>>()          -- IZeta, IZeta, Box: no edges
  StaticHolder.Invoke<IThing>()                    -- IThing: no edge

That is the same dependency-edge loss that the parallel
field_declaration handler (PR Graphify-Labs#2913) and the property/return/parameter
branches already close -- this fix closes the call-site branch. The
property_declaration branch above is the direct analogue: it walks the
declared type with _csharp_collect_type_refs and emits a generic_arg
reference per type argument. We do the same here on the
invocation_expression's function node, looking for type_argument_list
either on a bare generic_name (static Foo<Bar>()) or on the
member_access_expression's generic_name name (recv.Do<T>()).

The fix is bounded to the call-site branch -- every other position
(properties, returns, parameters, base lists, primary constructor
parameters, fields) was already correct, and the field case is
in-flight as the parallel PR Graphify-Labs#2913. A type parameter used as a call-
site argument (Holder<T>.Use(r => r.Do<T>())) is correctly skipped via
the same _csharp_type_parameters_in_scope walk the other branches use;
predefined types (`int`, `string`) never get a node from this
branch because _csharp_collect_type_refs returns early for
`predefined_type`.

Tests cover the six expected positions end-to-end via the issue's
two-file repro, plus single-arg, two-arg (the DI shape), nested
generic args, plain (no type args) call sites, type-parameter
arguments, and predefined types. Each test asserts the edge exists,
not the count -- absence is the bug.

@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. No changes could be formally verified in this run.


Graphify review — findings

Adds a C# call-site branch in _extract_generic that emits references[generic_arg] edges for type arguments on invocations, handling both static Foo<T>() (generic_name) and member recv.Do<T>() (member_access_expressiongeneric_name) shapes, with type parameters in scope filtered out. Covers the call-site gap left by the earlier field-position fix (#2913), addressing silently dropped dependency edges from #2911. Adds tests/test_csharp_call_site_generic_args.py exercising single/multiple/nested type args, no-arg non-regression, type-parameter suppression, and an end-to-end repro.

No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 627 functions depend on the 216 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() — 16 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 — 627 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: 567 function(s) in the blast radius were not formally verified this run

Formal verification

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

· 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