Skip to content

fix(rust): resolve a forward-referenced type to its local declaration - #3903

Closed
Yyunozor wants to merge 2 commits into
Graphify-Labs:v8from
Yyunozor:fix/rust-forward-type-reference
Closed

Yyunozor wants to merge 2 commits into
Graphify-Labs:v8from
Yyunozor:fix/rust-forward-type-reference

Conversation

@Yyunozor

@Yyunozor Yyunozor commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #3782.

A Rust struct, enum or trait used above its declaration got a sourceless stub: ensure_named_node only resolves names already in seen_ids. _rewire_unique_stub_nodes folds it back only for a corpus-unique label, so a same-named type in another file leaves the edge on the stub.

extract_rust now pre-scans the file's struct_item/enum_item/trait_item names before walk() (the Verilog, SQL and Bash pre-scan idiom), and ensure_named_node resolves against them.

  • Only these three items are pre-registered: a type alias or union without an impl never becomes a node, so its references would be dropped as dangling.
  • The scan never goes deeper than walk() (no function, static or const bodies), so every registered id becomes a node.
  • Option 2 (the stub's origin_file in the rewire) would cover other extractors too; a natural extract.py follow-up.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

Invariant: a reference above a same-file struct, enum or trait, spelled the same and alone on its casefolded id, resolves like one below it; anything else keeps v8's per-file result.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

Casefold collisions fail closed: when fn handle, const HANDLE, struct HANDLE or impl Tr for HANDLE shares struct Handle's id, or the reference spells HANDLE, it keeps the stub v8's extractor gives it; references after the id's owner are unchanged.

Known limitation: resolution is flat per file and keeps only the last path segment, inheriting its errors: qualified paths (io::Error, imp::Child), Self::X without alias, generic parameters, mod scopes. On tokio 2 of 85 moved references are wrong (imp::Child); on 351 crates of a local cargo registry, roughly 85 of 2,279 (201 in the suspect classes). Pre-scanning only unqualified names loses 57+ correct ones. Failing closed withholds 29 likely correct links (e.g. type declared before its colliding function).

Side effect in the final graph: some targets reached a real node on v8 only via a forward-reference stub sharing their id, later folded onto the type; they now stay unresolved. imports_from: 53 wrong mio use …::io bindings to a private Io, 12 tokio ones to Scheduler, 5 correct ones (e.g. socket2 pub use sys::SockFilter). References: 8 wrong jobserver HANDLE bindings to struct Handle, the only crate that drops (77 → 69); +2,271 = 2,279 moved − 8.

How was this tested?

# issue repro (`graphify update .`)
v8 d6eaa8a:   src_engine_a_before -> sink                (sourceless stub)
this branch:  src_engine_a_before -> src_engine_a_sink

# 7 new tests (tests/test_multilang.py)
v8 d6eaa8a:   4 failed, 3 passed (the 3 negative-space tests pass on both)
this branch:  7 passed

# typed edges landing on a real node, per crate, on v8 then on this branch
python -c "import sys,tempfile,pathlib as p;from graphify.extract import extract;d=p.Path(sys.argv[1]);r=extract(sorted(d.rglob('*.rs')),cache_root=p.Path(tempfile.mkdtemp()),parallel=False);s={n['id'] for n in r['nodes'] if n.get('source_file')};print(sum(e['relation'] in('references','implements','inherits') and e['target'] in s for e in r['edges']))" <crate>
tokio 1.52.3: 2433 -> 2518; mio 1.2.0: +1; socket2 0.6.3: +0
same command over 351 crates of a local cargo registry: +2,271

pytest tests/         -> 6104 passed, 15 skipped (v8: 6097 passed, 15 skipped)
ruff check .          -> All checks passed
pyright <both files>  -> 0 errors (v8: 0)

Removing any of these checks (bodies, modules, collisions, impl, exact name) turns a new test red.

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments.
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables).
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python).
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

…Graphify-Labs#3782)

ensure_named_node only resolved names already in seen_ids, so a struct,
enum or trait used above its declaration in the same file got a
sourceless stub:

    fn before(s: &Sink) {}
    struct Sink { n: u32 }

The corpus-level rewire folds that stub back onto the real struct only
when the name is unique across the corpus. Once another file declares a
`Sink` as well, the reference stays on the stub, and `affected` or
`explain` on the local struct miss that user.

Pre-scan the struct/enum/trait names the file declares, as the Verilog,
SQL and Bash extractors already pre-scan their definitions, and let
ensure_named_node resolve against them, so a use above a declaration
resolves exactly as a use below it already does. Only these three items
are pre-registered: walk() never makes a node of a `type` alias or
`union`, so registering one would leave its references to be dropped by
the final dangling-edge filter. The scan never descends further than
walk(), so every registered id ends up as a node.

Adds regression coverage for a struct, enum and trait used above their
declarations, a type declared in an inline module, the two-file
ambiguous case (cold and warm cache), and the items that must not
resolve a forward reference (type alias, union, function body, const
and static initializers).

Assisted-by: assayer harness on Claude Code (https://github.com/Yyunozor/assayer-memory-mcp)
Copilot AI lite review requested due to automatic review settings September 28, 2026 19:03
@github-actions

Copy link
Copy Markdown

Thanks for the pull request, @Yyunozor. 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.

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Unresolved identifier-collision and forward-reference import-edge issues remain.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Fixes Rust forward references to local struct, enum, and trait declarations.

Changes:

  • Pre-scans eligible local type declarations.
  • Adds regression tests for forward references and scan boundaries.
  • Preserves exclusions for unsupported declarations and scopes.
File Description
tests/​test_multilang.py Adds forward-reference regression coverage.
graphify/​extractors/​rust.py Pre-registers local type declarations during extraction.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread graphify/extractors/rust.py Outdated
Comment on lines 192 to 193
if nid in seen_ids or nid in local_type_ids:
return nid

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. The order described (fn handle, then the reference, then struct Handle) goes through seen_ids and resolves to handle() exactly as on v8; this PR leaves it unchanged. The variant with the reference above both declarations was introduced here. Fixed in 93fa872: a type is pre-registered only when no other item (functions, consts, statics, other types, impl blocks) shares its casefolded id, and by its exact name. Regression tests added.

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


Graphify review — findings

Resolves forward type references in the Rust extractor by pre-scanning each file for struct, enum, and trait declarations so a type used above its declaration binds to the local node instead of a sourceless stub that only got folded back when the name was unique across the corpus (#3782). The pre-scan (_scan_type_items) deliberately stops at the same boundaries walk() does — skipping function/const/static initializer bodies, type aliases, and unions — so it never registers an id that won't become a node, leaving references to those cases with their stubs intact.

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

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 164 functions depend on the 160 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract_rust() — 28 callers, 7 callees
  • new: walk() — 1 callers, 9 callees

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

Test selection

Test selection

2 of 304 test file(s) selected (1%) via static blast radius.

  • tests/test_multilang.py — impact, changed-test
  • tests/test_rust_self_member_calls.py — impact

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)

…and 10 more.

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

…des with another item (Graphify-Labs#3782)

Ids are casefolded, so `fn handle`, `const HANDLE`, `struct HANDLE` or an
`impl` for `HANDLE` share the id of `struct Handle`, and walk() keeps
whichever item comes first. The pre-scan registered the type id
regardless, so a reference placed above the declarations was bound to
that shared id and could end on the function or on the other item, where
v8's extractor left a sourceless stub. For the same reason a `HANDLE`
reference (a `type HANDLE` alias, or an imported name) placed above a
local `struct Handle` was bound to the struct.

Only pre-register a type when every file-level item behind its id is
that same type (`#[cfg]` twins count once), and match it by its exact
name. An `impl` block counts as the type it names, since walk() gives it
the node of its type text; methods keep impl-qualified ids and are not
counted. Otherwise the forward reference keeps its stub. A reference
placed after an item that already owns the id still resolves through
seen_ids, unchanged from v8.

Adds regression cases for a type colliding with a function (also in an
`extern` block), a const, a static, another type, an `impl` for another
type and a type alias, and controls where the id is not shared: another
function name, `#[cfg]` twins, an `impl` for the type itself, a method
named like the type, and a generic type only an `impl` names.

Assisted-by: assayer harness on Claude Code (https://github.com/Yyunozor/assayer-memory-mcp)
@safishamsi

Copy link
Copy Markdown
Member

Shipped in v0.9.74 (live on PyPI as graphifyy==0.9.74). Landed on v8 as 5c8df15 + 4babf7b via an authorship-preserving cherry-pick, so your original commit authorship is kept intact. Thanks @Yyunozor for Rust forward-referenced type resolution 🙏

Closing as shipped.

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.

Rust: a type used above its declaration stays a sourceless stub when the name is defined in 2+ files

3 participants