Skip to content

fix(csharp-extractor): capture aliased usings and qualified new expressions in the call graph#441

Open
tirth8205 wants to merge 2 commits into
Egonex-AI:mainfrom
tirth8205:fix/csharp-extractor-edge-cases
Open

fix(csharp-extractor): capture aliased usings and qualified new expressions in the call graph#441
tirth8205 wants to merge 2 commits into
Egonex-AI:mainfrom
tirth8205:fix/csharp-extractor-edge-cases

Conversation

@tirth8205

Copy link
Copy Markdown
Contributor

Problem

Two C# extractor edge cases silently dropped real declarations:

  1. Aliased using directives with a simple-identifier target. extractUsingSource handled the alias form using Alias = Some.Namespace; by looking only for a qualified_name child after the =. When the alias target is a single, undotted name (e.g. using Alias = System;), tree-sitter emits the target as a plain identifier, not a qualified_name. The alias branch returned null, so the whole using directive was dropped from imports. Falling back to the existing findChild(node, "identifier") would also be wrong, because the first identifier child is the alias name Alias, not the target System.

  2. new of a qualified type. The object_creation_expression handler only looked for a direct identifier or generic_name child to name the constructed type. For a qualified type like new System.Text.StringBuilder(), tree-sitter places the type as a qualified_name child, so the object creation was dropped from the call graph entirely. (new Bar() and new List<int>() were captured correctly.)

Fix

  1. In the alias branch of extractUsingSource, when no qualified_name is found, fall back to the identifier that appears after the = token (tracking a seenEquals flag), so the target — not the alias name — is read.

  2. Add qualified_name to the type-node fallback chain in the object_creation_expression handler:

    const typeNode =
      findChild(node, "identifier") ??
      findChild(node, "generic_name") ??
      findChild(node, "qualified_name");

Testing

  • Added two tests to csharp-extractor.test.ts:
    • using Alias = System; now yields imports == [{ source: "System", specifiers: ["System"], lineNumber: 1 }].
    • new System.Text.StringBuilder() inside method M now produces call-graph entry { caller: "M", callee: "new System.Text.StringBuilder", lineNumber: 1 }.
  • Both tests fail before the fix (imports []; call graph []) and pass after.
  • The full core package vitest suite is green (694 tests, csharp-extractor up to 29). Lint (eslint) and typecheck (tsc --noEmit) on the changed files both exit 0.

🤖 Generated with Claude Code

…ressions in the call graph

Two C# extractor edge cases dropped real declarations:

1. Aliased using directives whose target is a single undotted name
   (`using Alias = System;`) emit the target as a plain `identifier`, not a
   `qualified_name`. The alias branch only looked for `qualified_name` and
   returned null, dropping the whole import. Now it falls back to the
   identifier that follows the `=` token (not the first identifier, which is
   the alias name itself).

2. `object_creation_expression` for a qualified type
   (`new System.Text.StringBuilder()`) places the type as a `qualified_name`
   child, which neither the `identifier` nor `generic_name` lookup matched, so
   the object creation was dropped from the call graph. Added `qualified_name`
   to the fallback chain.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@thejesh23 thejesh23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1. Aliased using drops the alias name entirely.
For using Alias = System; the import is recorded as { source: "System", specifiers: ["System"] }, so downstream consumers can't tell the symbol bound in scope is Alias. The alias is the only name actually resolvable in the file; emitting specifiers: ["Alias"] (or both) would match how import 'x' as y; should surface in PR #435's Dart extractor too.

2. Qualified generic new is untested and likely partially wrong.
new System.Collections.Generic.List<int>() isn't covered by either the existing new List<int>() test or the new qualified-type test. Depending on how tree-sitter-c-sharp shapes this (qualified_name wrapping a generic_name, vs. a distinct node), the callee text may include the type arguments or silently fall through the fallback chain. Worth one explicit case.

3. global using and file-scoped alias forms are unhandled.
The extractUsing switch only matches using_directive; C# 10's global using Alias = System; parses as global_using_directive (or as a using_directive under a global modifier child, depending on grammar version), so the new alias fix doesn't reach it. A one-line note or follow-up issue would be useful.

Nit: the manual seenEquals scan duplicates the work findChild(node, "=") already did; a single pass that records both presence-of-= and the post-= identifier would be tidier.

…ss using parse

Address PR review on the C# extractor edge cases:

- Add call-graph tests for `new List<int>()` (generic_name) and
  `new System.Collections.Generic.List<int>()` (qualified_name wrapping a
  trailing generic_name). The existing fallback chain already produces the
  correct callee text; the tests lock it.
- Add structure tests for `global using` forms (plain, simple-identifier
  alias, qualified alias). tree-sitter-c-sharp parses these as a
  `using_directive` with a leading `global` child, so the existing handler
  already covers them; document this in the `walkTopLevel` switch.
- Consolidate `extractUsingSource` into a single pass over the directive's
  children instead of a `findChild(=)` scan followed by a second seenEquals
  scan, removing the duplicate traversal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tirth8205

Copy link
Copy Markdown
Contributor Author

2. Qualified generic new — added explicit tests.
I dumped the real parse trees first. tree-sitter-c-sharp shapes new System.Collections.Generic.List<int>() as a single qualified_name whose trailing segment is a generic_name, so the node's .text already carries the full dotted path including the type arguments; the existing identifier ?? generic_name ?? qualified_name fallback resolves it correctly. Added two call-graph tests to lock this:

  • new List<int>() -> new List<int> (the previously-unexercised generic_name branch)
  • new System.Collections.Generic.List<int>() -> new System.Collections.Generic.List<int>

3. global using — verified already handled, locked with tests.
In the pinned grammar, global using ... does not produce a global_using_directive; it parses as a using_directive with a leading global child. So the existing case "using_directive" and extractUsingSource already cover it. Rather than a follow-up note, I added tests for all three forms (global using X.Y;, global using A = System;, global using A = System.Collections.Generic;) — all green — and documented the grammar shape in the walkTopLevel switch.

Nit — consolidated the duplicate scan. extractUsingSource is now a single pass over the directive's children: it resets the candidate on =, returns immediately on a qualified_name, and otherwise keeps the first identifier. This drops the separate findChild(node, "=") probe and the second seenEquals traversal.

1. Aliased using specifiers — not changing. In this extractor specifiers is uniformly the trailing component of the target namespace: using System.Collections.Generic; yields ["Generic"], and the PR's contract is source = target namespace. Emitting ["Alias"] would special-case the alias path against that consistent semantic and diverge from how qualified usings behave. The actual bug this PR fixed (the alias branch returning null and dropping the import) is resolved. Preserving the bound local name is worth doing, but as a typed alias/localName field on the import schema, not by overloading specifiers.

Full core suite: 699 passed. tsc --noEmit clean.

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