Skip to content

fix(csharp): resolve calls through member chains, element access and constructors - #4258

Closed
Mpasha17 wants to merge 2 commits into
Graphify-Labs:v8from
Mpasha17:fix/csharp-member-chain-calls-4246
Closed

Mpasha17 wants to merge 2 commits into
Graphify-Labs:v8from
Mpasha17:fix/csharp-member-chain-calls-4246

Conversation

@Mpasha17

@Mpasha17 Mpasha17 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Closes #4246

Three more C# call shapes got no calls edge when the callee lived in another file: a call through a field of a typed receiver (run.Sequence.Apply()), a call on an array or list element (_configs[i].WriteIds()), and any call inside a constructor, since constructor bodies were never walked for calls at all.

Now the C# extractor records [type of run, "Sequence"] for a member chain and the element type for xs[i].M() (from T[], List<T>, IList<T> or IReadOnlyList<T> fields, properties, parameters and locals, with the same scoping and shadowing rules as other receivers). _resolve_csharp_member_calls resolves the root type, looks up the field's declared type on that class or its bases through the field tables C# already exports, and then finds the method there like any other typed receiver. If any step is unknown or ambiguous there's no edge. Constructor bodies are walked now too, and since constructors have no node of their own, their calls come from the type node (the issue mentioned that as an option). this.M() and base.M() inside a constructor resolve through that type. Chains and element access that already bound within the same file are left as they were.

Limits: chains are only one field deep (a.B.C.M() and this.a.B.M() stay unresolved), and element access only works on a plain identifier (this._items[i] and run.Items[i] don't resolve). A constructor's call to a method of its own class shares a node pair with the existing method edge, so the cross-file pass skips it. Walking constructor bodies also adds the usual call-site references edges (for example, generic args of CreateMap<A, B>() inside an AutoMapper Profile constructor). Constructor calls also go through v8's existing name lookup for new X(), so the one wrong binding that already exists on v8 (new System.ArgumentNullException linking to AutoMapper's own internal ArgumentNullException, 11 edges) picks up 2 more from constructors.

Testing: new tests/test_csharp_member_chain_calls.py with 12 tests. The 6 positive ones fail on current v8, and the ambiguity, unknown-type, type-parameter, nested-list and #3797 control tests guard against wrong edges. The full suite on 3.10/3.12/3.13/3.14 has the same failures as v8. Ruff is clean and pyright shows no new findings. The three repros from the issue now give exactly the expected edges, and Other.WriteIds gets nothing. On MediatR, Polly, Newtonsoft.Json and AutoMapper the C# calls edges went from 469 to 480, 5483 to 5546, 14252 to 14421 and 3479 to 3537, with none removed. I spot-checked a sample of the new edges against the source and they were right.

AI-assisted (Grok Bot); each commit carries a Co-Authored-By line.

…constructors (Graphify-Labs#4246)

run.Sequence.Apply(), _configs[i].WriteIds() and any call inside a
constructor body produced no calls edge when the callee lived in another
file. Record the root's type plus the field for a one-level member chain
and the element type for xs[i] on T[] / List<T> / IList<T> /
IReadOnlyList<T>, and resolve them through the exported C# field tables.
Walk constructor bodies too, attributing their calls to the declaring
type. Any unknown or ambiguous step yields no edge.

Co-Authored-By: Grok Bot <noreply@x.ai>
@Mpasha17
Mpasha17 requested a review from safishamsi as a code owner October 9, 2026 10:33
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Thanks for the pull request, @Mpasha17. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@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.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.


Graphify review — findings

Extends C# member-call resolution to chained receivers (x.F.M()) and indexed elements (xs[i].M()) by recording each field's, property's and local's declared type, plus an element type for T[], List<T>, IList<T> and IReadOnlyList<T>. _field_type_nid walks the base chain and gives up, leaving the call unresolved, on an unresolved base, an ambiguous declaration, or an indexed field accessed without an index. Constructor bodies are now walked for calls and attributed to the declaring type, so this./base. calls inside a constructor resolve against that type.

Worth a look

  • C# generic property element types are not filtered — graphify/extractors/engine.py:5769 · Escalate · medium · 2 independent checks
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Nested C# collection elements are treated as concrete List receivers — graphify/extractors/engine.py:2201 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2902 functions depend on the 577 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 821 callers, 51 callees
  • new: _rebuild_code() — 162 callers, 56 callees
  • new: _extract_generic() — 19 callers, 34 callees
  • new: extract_js() — 87 callers, 5 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: main() — 102 callers, 3 callees
  • new: extract_objc() — 27 callers, 10 callees
  • new: dispatch_command() — 2 callers, 130 callees
  • …and 50 more — each is listed as a finding

Verification — 2902 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: 2707 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

160 of 356 test file(s) selected (45%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_astro_import_ids.py — impact
  • tests/test_blade_extractor.py — impact
  • tests/test_build.py — impact
  • tests/test_builtin_global_type_refs.py — impact
  • tests/test_cache.py — impact
  • tests/test_case_sensitive_resolution.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cobol_extractor.py — impact
  • tests/test_cpp_method_declarations.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_cpp_objc_cross_file_calls.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_cross_language_call_resolution.py — impact
  • tests/test_cross_repo_external_call_guards.py — impact
  • tests/test_cross_repo_member_calls.py — impact
  • tests/test_csharp_call_site_generic_args.py — impact
  • tests/test_csharp_enum_members.py — impact
  • tests/test_csharp_field_generic_args.py — impact
  • tests/test_csharp_generic_callsites.py — impact
  • tests/test_csharp_interface_dispatch.py — impact
  • tests/test_csharp_member_calls.py — impact
  • tests/test_csharp_member_chain_calls.py — impact, changed-test
  • tests/test_csharp_member_nodes.py — impact
  • tests/test_csharp_object_creation.py — impact
  • tests/test_csharp_partial_classes.py — impact
  • tests/test_csharp_tuple_type_refs.py — impact
  • tests/test_csharp_type_resolution.py — impact
  • tests/test_definition_file_portability.py — impact
  • tests/test_detect.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_duplicate_annotation_edges.py — impact
  • tests/test_elixir_import_resolution.py — impact
  • tests/test_elixir_keyword_def_calls.py — impact
  • tests/test_elixir_qualified_calls.py — impact
  • tests/test_elixir_unqualified_call_scope.py — impact
  • tests/test_erlang_extractor.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cache_location.py — impact
  • tests/test_extract_path_memo.py — impact
  • tests/test_extract_php_closures.py — impact
  • tests/test_file_label_disambiguation.py — impact
  • tests/test_file_node_id_spec.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_go_builtin_call_targets.py — impact
  • tests/test_go_import_repoint.py — impact
  • tests/test_go_interface_methods.py — impact
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_import_self_loops.py — impact
  • … and 110 more

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Docs that may be stale (advisory)

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

@Mpasha17

Mpasha17 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Both findings were real, so I fixed them in 247ec07. I reproduced each one live first: List<Config> Items inside class Holder<Config> (where Config is the class's type parameter) linked Items[0].WriteIds() to the real Demo.Config.WriteIds, and List<List<Config>> _grid linked _grid[0].WriteIds() to a project class named List. _csharp_element_type_name now returns nothing for a type parameter in scope or a nested list, so this covers fields, properties, parameters and locals all at once (it replaces the field-only type-param check). I added two tests for these cases, and both fail without the change. The full suite on 3.10/3.12/3.13/3.14 still has the same failures as v8, and MediatR, Polly, Newtonsoft.Json and AutoMapper give the same graphs as before the fix.

safishamsi pushed a commit that referenced this pull request Oct 9, 2026
…constructors (#4258, #4246)

Resolve a.b.C(), xs[i].M(), and calls inside constructor bodies: a fail-closed
chain/element-type resolver (one declared type per hop, collection element only
for T[]/List<T>, bail on ambiguity) plus a ctor-body walk attributing calls to
the type node. Reuses the existing C# scoping; avoids same-pair method/calls
duplicates.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Member

Landed in v0.9.83 via an authorship-preserving cherry-pick, so your commit is on v8 with you credited as the author. Closing as shipped — thanks @Mpasha17!

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.

C#: calls through a member chain, through element access, or inside a constructor produce no edge (follow-up to #3797)

2 participants