Skip to content

Commit 639ea2a

Browse files
gadievrongadievronclaude
authored
fix(rust): 10 call-graph correctness bugs (edge recovery + phantom removal) (#205)
Co-authored-by: gadievron <gadi@unpromptedcon.org> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9e20af1 commit 639ea2a

20 files changed

Lines changed: 610 additions & 25 deletions

apps/openant-cli/internal/languages/registry_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestSupportedMatchesConfig(t *testing.T) {
2828
if err != nil {
2929
t.Fatalf("Supported() error: %v", err)
3030
}
31-
want := []string{"c", "go", "javascript", "php", "python", "ruby", "swift", "zig"}
31+
want := []string{"c", "go", "javascript", "php", "python", "ruby", "rust", "swift", "zig"}
3232
if len(got) != len(want) {
3333
t.Fatalf("Supported() = %v, want %v", got, want)
3434
}

libs/openant-core/parsers/rust/call_graph_builder.py

Lines changed: 202 additions & 15 deletions
Large diffs are not rendered by default.

libs/openant-core/parsers/rust/function_extractor.py

Lines changed: 100 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import re
2727
from datetime import datetime
2828
from pathlib import Path
29-
from typing import Any, Dict, List, Optional, Tuple
29+
from typing import Any, Dict, List, Optional, Set, Tuple
3030

3131
from utilities.file_io import write_json
3232

@@ -75,6 +75,17 @@ def _load_rust_language() -> Language:
7575
# `impl` block, or as the RHS of a `let x: <type> = ...` annotation.
7676
_TYPE_NODE_KINDS = ("type_identifier", "generic_type", "scoped_type_identifier")
7777

78+
# Non-nominal Self-type node kinds that are absent from _TYPE_NODE_KINDS but CAN be
79+
# an impl target: `impl Trait for u32 / [u8;4] / (i32,i32) / () / &T / *const T /
80+
# dyn X`. Collected in _handle_impl so their methods are extracted; for the
81+
# genuinely non-nominal ones _bare_type_name returns None and _handle_impl falls
82+
# back to the raw type text, while reference/dynamic types unwrap to their nominal
83+
# base as usual.
84+
_IMPL_SELF_EXTRA_KINDS = (
85+
"primitive_type", "array_type", "tuple_type", "unit_type",
86+
"reference_type", "pointer_type", "dynamic_type",
87+
)
88+
7889

7990
def _bare_type_name(node: Optional[Node], source: bytes) -> Optional[str]:
8091
"""Reduce a type node to its bare (unqualified, non-generic) name.
@@ -106,16 +117,16 @@ def _bare_type_name(node: Optional[Node], source: bytes) -> Optional[str]:
106117
return _text(last, source) if last is not None else None
107118
if t == "reference_type":
108119
# `&Point` / `&mut Point` / `&'a Point` -> unwrap to Point;
109-
# `&dyn Shape` -> unwrap through the dynamic_type to Shape (val_3_19).
120+
# `&dyn Shape` -> unwrap through the dynamic_type to Shape.
110121
for child in node.children:
111122
if child.type in _TYPE_NODE_KINDS or child.type == "dynamic_type":
112123
return _bare_type_name(child, source)
113124
return None
114125
if t == "dynamic_type":
115126
# `dyn Shape` / `dyn Shape + Send` -> the trait's bare name, so a
116127
# `&dyn Shape` receiver types as `Shape` and dispatches to Shape's
117-
# conformers via trait_impls (val_3_19; `dyn Shape` is a `dynamic_type`
118-
# node, verified against the installed grammar per pr_2_1).
128+
# conformers via trait_impls (`dyn Shape` is a `dynamic_type`
129+
# node, verified against the installed grammar).
119130
for child in node.children:
120131
r = _bare_type_name(child, source)
121132
if r:
@@ -129,6 +140,53 @@ def _bare_type_name(node: Optional[Node], source: bytes) -> Optional[str]:
129140
return None
130141

131142

143+
def _impl_generic_bounds(impl_node: Node, source: bytes) -> Dict[str, List[str]]:
144+
"""Map an impl block's OWN generic param letter -> its bound trait(s).
145+
146+
`impl<T: Shape + Draw> Foo<T>` and `impl<T> Foo<T> where T: Shape` both yield
147+
`{"T": ["Shape", ...]}`. Only the impl header's own generics (the `<...>` before
148+
the self type, plus the where-clause) are read. Threaded onto each method so a
149+
receiver typed as the impl's generic param (`x: &T`) dispatches to the bound
150+
trait's conformers -- the SAME reachability-safe closure fn-level bounds use --
151+
instead of falling to a bare lookup on the letter `T` (which a blanket impl's
152+
pseudo-type `T` would poison). Mirrors CallGraphBuilder._collect_type_param_bounds.
153+
"""
154+
bounds: Dict[str, List[str]] = {}
155+
156+
def _traits(tb: Node) -> List[str]:
157+
return [_text(c, source) for c in tb.children if c.type == "type_identifier"]
158+
159+
def _add(param: Optional[str], tb: Node) -> None:
160+
if not param:
161+
return
162+
bounds.setdefault(param, [])
163+
for t in _traits(tb):
164+
if t not in bounds[param]:
165+
bounds[param].append(t)
166+
167+
def _param(node: Node) -> None:
168+
pid = None
169+
for cc in node.children:
170+
if cc.type == "type_identifier" and pid is None:
171+
pid = _text(cc, source)
172+
elif cc.type == "trait_bounds":
173+
_add(pid, cc)
174+
175+
seen_for = False
176+
for child in impl_node.children:
177+
if child.type == "for":
178+
seen_for = True
179+
elif child.type == "type_parameters" and not seen_for:
180+
for tp in child.children:
181+
if tp.type in ("type_parameter", "constrained_type_parameter"):
182+
_param(tp)
183+
elif child.type == "where_clause":
184+
for wp in child.children:
185+
if wp.type == "where_predicate":
186+
_param(wp)
187+
return bounds
188+
189+
132190
def _text(node: Optional[Node], source: bytes) -> str:
133191
if node is None:
134192
return ""
@@ -403,7 +461,7 @@ def _handle_impl(
403461
if cc.type == "type_identifier":
404462
impl_generics.add(_text(cc, source))
405463
break
406-
if child.type in _TYPE_NODE_KINDS:
464+
if child.type in _TYPE_NODE_KINDS or child.type in _IMPL_SELF_EXTRA_KINDS:
407465
type_nodes.append((child, seen_for))
408466
elif child.type == "declaration_list":
409467
body = child
@@ -418,6 +476,14 @@ def _handle_impl(
418476
self_node = type_nodes[0][0] if type_nodes else None
419477

420478
self_type = _bare_type_name(self_node, source)
479+
if not self_type and self_node is not None:
480+
# Non-nominal Self type (primitive `u32`, array `[u8; 4]`, tuple
481+
# `(i32, i32)`, unit `()`): `_bare_type_name` only names nominal types,
482+
# so `impl Serialize for u32` would be dropped ENTIRELY -- every method
483+
# in the block lost from extraction, the graph, and reachability. Fall
484+
# back to the raw type text so the methods are still extracted (keyed by
485+
# that type spelling).
486+
self_type = _text(self_node, source).strip()
421487
trait_name = _bare_type_name(trait_node, source)
422488

423489
if not self_type or body is None:
@@ -439,6 +505,8 @@ def _handle_impl(
439505
"module_path": ctx["module_path"],
440506
"in_test_scope": ctx["in_test_scope"],
441507
"in_trait_impl": trait_name is not None,
508+
"impl_trait": trait_name,
509+
"impl_type_param_bounds": _impl_generic_bounds(node, source),
442510
}
443511
worklist.append((body, new_ctx))
444512

@@ -514,7 +582,29 @@ def _handle_function(
514582

515583
module_name = "::".join(ctx["module_path"]) if ctx["module_path"] else None
516584

585+
# Bare return-type name (`-> Widget` -> "Widget"), so a binding
586+
# `let w = Type::assoc()` can be typed by the assoc fn's ACTUAL return type
587+
# rather than the constructor-idiom assumption that it returns `Type`.
588+
rt_node = node.child_by_field_name("return_type")
589+
return_type = _bare_type_name(rt_node, source) if rt_node is not None else None
590+
517591
func_id = f"{file_path}:{qualified_name}"
592+
if func_id in functions:
593+
# Same qualified_name already taken -- e.g. `impl Display for P` and
594+
# `impl Debug for P` both yield `P.fmt`, or an inherent method plus a
595+
# same-named trait method. Without disambiguation the second silently
596+
# clobbers the first (a whole unit lost from the graph AND reachability).
597+
# Append the trait (or `impl`) so both survive. The FIRST occurrence
598+
# keeps the plain id, so class_name-based resolution and existing
599+
# `Type.method` references are unchanged; only the colliding sibling
600+
# gets the `#trait` suffix.
601+
disc = ctx.get("impl_trait") or "impl"
602+
candidate = f"{file_path}:{qualified_name}#{disc}"
603+
n = 2
604+
while candidate in functions:
605+
candidate = f"{file_path}:{qualified_name}#{disc}{n}"
606+
n += 1
607+
func_id = candidate
518608
functions[func_id] = {
519609
"name": name,
520610
"qualified_name": qualified_name,
@@ -529,6 +619,11 @@ def _handle_function(
529619
"is_exported": is_exported,
530620
"has_self": has_self,
531621
"decorators": attrs,
622+
# Bounds of the enclosing impl's own generics (`impl<T: Shape> Foo<T>`),
623+
# so a receiver typed as `T` in this method dispatches to the trait's
624+
# conformers. Empty for free functions / inherent-non-generic impls.
625+
"impl_type_param_bounds": ctx.get("impl_type_param_bounds", {}),
626+
"return_type": return_type,
532627
}
533628

534629
block = None

libs/openant-core/parsers/rust/unit_generator.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ def _generate_unit(self, func_id: str, func_info: Dict[str, Any]) -> Dict[str, A
160160
"generator": "rust_unit_generator.py",
161161
"direct_calls": direct_calls,
162162
"direct_callers": direct_callers,
163+
# Parity with the Swift parser's unit metadata: carry the function's
164+
# attributes/decorators onto dataset units.
165+
"decorators": func_info.get("decorators", []),
163166
},
164167
}
165168

File renamed without changes.
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
"""`let x = Type::assoc()` should type `x` by the assoc fn's ACTUAL return
2+
type, not the blanket assumption that `Type::assoc()` returns `Type`. `Factory::
3+
make() -> Widget` must type the binding as Widget (recovering `w.process()` ->
4+
Widget.process and NOT fabricating Factory.process). The dominant `Type::new() ->
5+
Self` idiom must still resolve to Type."""
6+
import pathlib, sys
7+
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
8+
from _rust_helpers import build, edges # noqa: E402
9+
10+
11+
def test_factory_return_type_used_for_receiver(tmp_path):
12+
repo = {"lib.rs": """
13+
pub struct Widget; impl Widget { pub fn process(&self) {} }
14+
pub struct Factory; impl Factory { pub fn make() -> Widget { Widget } pub fn process(&self) {} }
15+
pub fn run() { let w = Factory::make(); w.process(); }
16+
"""}
17+
e = edges(build(tmp_path, repo)[1])
18+
assert ("run", "Widget.process") in e, e # real: w is a Widget
19+
assert ("run", "Factory.process") not in e, e # phantom: w is NOT a Factory
20+
21+
22+
def test_new_returns_self_still_resolves(tmp_path):
23+
# the dominant constructor idiom (Type::new() -> Self) must keep working.
24+
repo = {"lib.rs": """
25+
pub struct Point; impl Point { pub fn new() -> Self { Point } pub fn dist(&self) -> f64 { 0.0 } }
26+
pub fn run() { let p = Point::new(); p.dist(); }
27+
"""}
28+
assert ("run", "Point.dist") in edges(build(tmp_path, repo)[1])
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
"""a multi-bound generic must not lose all edges when one bound trait has
2+
no recorded conformers (marker/blanket/derive/cross-crate impls are invisible to
3+
the extractor). An unseen conformer set is 'unconstrained', not 'empty' — it must
4+
not annihilate the edges the other bounds establish (reachability over-approx)."""
5+
import pathlib, sys
6+
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
7+
from _rust_helpers import build, edges # noqa: E402
8+
9+
10+
def test_multi_bound_survives_impl_less_marker(tmp_path):
11+
repo = {"lib.rs": """
12+
pub trait Shape { fn area(&self) -> f64; }
13+
pub struct Circle; impl Shape for Circle { fn area(&self) -> f64 { 1.0 } }
14+
pub struct Square; impl Shape for Square { fn area(&self) -> f64 { 2.0 } }
15+
pub trait Marker {}
16+
impl<X: Shape> Marker for X {}
17+
pub fn total<B: Shape + Marker>(b: &B) -> f64 { b.area() }
18+
"""}
19+
e = edges(build(tmp_path, repo)[1])
20+
assert ("total", "Circle.area") in e, e
21+
assert ("total", "Square.area") in e, e
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
"""RUST_BUILTINS filter must not drop TYPED/resolvable method-call edges.
2+
3+
A method call whose method name happens to be in RUST_BUILTINS (get/parse/new/...)
4+
on a KNOWN receiver type must still resolve — the builtin guard is a precision knob
5+
for the UNKNOWN-receiver fallback only (see _resolve_unknown_receiver_method), not a
6+
reason to delete a fully-resolvable typed method edge at extraction time.
7+
"""
8+
import pathlib, sys
9+
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
10+
from _rust_helpers import build, edges # noqa: E402
11+
12+
13+
def test_typed_cross_file_builtin_named_method_resolves(tmp_path):
14+
repo = {
15+
"a.rs": "pub struct Cache;\nimpl Cache { pub fn get(&self) -> i32 { secret() } }\nfn secret() -> i32 { 1 }\n",
16+
"b.rs": "use crate::a::Cache;\npub fn run(c: &Cache) -> i32 { c.get() }\n",
17+
}
18+
e = edges(build(tmp_path, repo)[1])
19+
# 'get' is in RUST_BUILTINS but the receiver is typed (&Cache) and Cache::get is
20+
# unambiguous -> the edge MUST exist (previously dropped by the pre-resolution filter).
21+
assert ("run", "Cache.get") in e, e
22+
23+
24+
def test_bare_builtin_named_free_fn_resolves(tmp_path):
25+
# a bare call to a free fn named like a builtin ('parse') is a real edge.
26+
repo = {
27+
"a.rs": "pub fn parse() -> i32 { 1 }\n",
28+
"b.rs": "use crate::a::parse;\npub fn run() -> i32 { parse() }\n",
29+
}
30+
e = edges(build(tmp_path, repo)[1])
31+
assert ("run", "parse") in e, e

libs/openant-core/tests/parsers/rust/test_rust_callgraph_symmetry.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import sys
1111

1212
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
13-
from _helpers import build # noqa: E402
13+
from _rust_helpers import build # noqa: E402
1414

1515
_REPO = {
1616
"lib.rs": """
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
"""a receiver bound from a free-function call
2+
`let c = load()` where `load() -> Cfg` must type `c` as Cfg, so `c.validate()`
3+
resolves PRECISELY to Cfg.validate -- recovering the unknown-receiver blackout the
4+
unknown-receiver decline gate would otherwise cause, with NO phantom to a same-named method on an
5+
unrelated type. This makes the receiver KNOWN rather than relaxing the gate."""
6+
import pathlib, sys
7+
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
8+
from _rust_helpers import build, edges # noqa: E402
9+
10+
11+
def test_free_fn_return_type_types_the_binding(tmp_path):
12+
repo = {
13+
"a.rs": "pub struct Cfg; impl Cfg { fn validate(&self) {} }\n"
14+
"pub struct Form; impl Form { fn validate(&self) {} }\n"
15+
"pub fn load() -> Cfg { Cfg }\n",
16+
"b.rs": "use crate::a::{Cfg, Form, load};\npub fn run() { let c = load(); c.validate(); }\n",
17+
}
18+
e = edges(build(tmp_path, repo)[1])
19+
assert ("run", "Cfg.validate") in e, e # recovered precisely via load() -> Cfg
20+
assert ("run", "Form.validate") not in e, e # no phantom to the other same-named type

0 commit comments

Comments
 (0)