fix: review follow-ups for #5 and the Swift lexical preview #8

Merged
trevon merged 9 commits from fix/review-followups into main 2026-09-25 13:33:41 +00:00
3 changed files with 89 additions and 13 deletions
Showing only changes of commit 04653788fa - Show all commits

fix(rust): keep impls written on a qualified path when leaf names collide

When several files declare the same type leaf name, rust_method keeps
only the impls of the type a call site means, asking each impl's file
which `Foo` it means. An impl in a third file written as
`impl crate:🅰️:Foo` resolved nothing (that file has no `Foo`
declaration or `use` binding), so its methods were dropped and
`x.run()` on an `a::Foo` read "member-not-found". It resolved before #5.

The extractor now records the impl's type path when it is written as
one (`impl_type_path`, generic arguments removed), the method index
keeps it, and the impl filter resolves that path to its declaring file
before falling back to the file's own view of the leaf name. Glob
imports (`use super::*`) are still not expanded; such an impl stays
unresolved rather than guessed.

Test: with `Foo` declared in a.rs and c.rs and `impl crate:🅰️:Foo` in
ext.rs, `x.run()` on `crate:🅰️:Foo` resolves to ext.rs's `run`; it
failed before this change.
trvon 2026-09-24 21:07:34 -06:00
Signed by: trevon
GPG key ID: 38686EA797D7A8A7

View file

@ -149,7 +149,8 @@ struct RustCall {
}
/// A method node id, the trait it implements (if any), and the file of its `impl`.
type RustMethod = (String, Option<String>, String);
/// (method id, implemented trait, impl file, impl type path when written as one).
type RustMethod = (String, Option<String>, String, Option<String>);
/// Crate-wide Rust impl/trait facts keyed by leaf type names. Leaf names are not
/// identities: when several files declare the same name, `rust_type_file` settles
@ -764,7 +765,12 @@ impl ScanState {
.methods
.entry((owner, declaration.name.clone()))
.or_default()
.push((id.clone(), implemented, path.to_owned()));
.push((
id.clone(),
implemented,
path.to_owned(),
declaration.impl_type_path.clone(),
));
if let Some(returns) = &declaration.return_type {
self.rust.returns.insert(id.clone(), returns.clone());
}
@ -1973,8 +1979,17 @@ impl ScanState {
};
candidates
.iter()
.filter(|(_, _, impl_path)| {
self.rust_type_file(ty, impl_path).as_deref() == Some(meant.as_str())
.filter(|(_, _, impl_path, impl_type_path)| {
// `impl crate::a::Foo` says which Foo itself; otherwise ask what
// the impl's own file means by the leaf name.
let implemented = match impl_type_path {
Some(qualified) => self
.resolve_import(qualified, "rust", impl_path)
.and_then(|id| self.node(&id))
.and_then(|node| node.path.clone()),
None => self.rust_type_file(ty, impl_path),
};
implemented.as_deref() == Some(meant.as_str())
})
.collect()
} else {
@ -1983,7 +1998,7 @@ impl ScanState {
let inherent: Vec<_> = candidates
.iter()
.copied()
.filter(|(_, t, _)| t.is_none())
.filter(|(_, t, _, _)| t.is_none())
.collect();
let chosen: Vec<_> = if inherent.is_empty() {
candidates
@ -1991,7 +2006,7 @@ impl ScanState {
inherent
};
match chosen.as_slice() {
[(id, _, _)] => Ok(id.clone()),
[(id, _, _, _)] => Ok(id.clone()),
[] if self.rust.types.contains(ty) => Err("member-not-found"),
[] => Err("receiver-type-external"),
_ => Err("ambiguous"),

View file

@ -23,6 +23,9 @@ pub(crate) struct Declaration {
pub impl_type: Option<String>,
/// Rust: trait of the enclosing `impl Trait for T` or `trait` block.
pub impl_trait: Option<String>,
/// Rust: the enclosing impl's type as written when it is a path
/// (`impl crate::a::Foo`), generic arguments removed; `None` for a bare name.
pub impl_type_path: Option<String>,
/// Rust: declared return type, whitespace removed.
pub return_type: Option<String>,
/// C++ `virtual` or `override`/`final` on this definition.
@ -378,10 +381,10 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result<Docume
{
doc.declared.push(qualified.clone());
}
let (impl_type, impl_trait) = if language == "rust" {
let (impl_type, impl_trait, impl_type_path) = if language == "rust" {
rust_impl_context(node, source)
} else {
(None, None)
(None, None, None)
};
let return_type = (language == "rust")
.then(|| node.child_by_field_name("return_type"))
@ -409,6 +412,7 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result<Docume
internal,
impl_type,
impl_trait,
impl_type_path,
return_type,
});
}
@ -1644,16 +1648,20 @@ fn cpp_first_template_type(node: Node<'_>, source: &str) -> Option<String> {
}
/// Rust `impl T` / `impl Trait for T` / `trait Trait` enclosing a declaration.
fn rust_impl_context(node: Node<'_>, source: &str) -> (Option<String>, Option<String>) {
type RustImplContext = (Option<String>, Option<String>, Option<String>);
/// (impl type leaf, implemented or declared trait, impl type path) around `node`.
fn rust_impl_context(node: Node<'_>, source: &str) -> RustImplContext {
let mut parent = node.parent();
while let Some(n) = parent {
match n.kind() {
"impl_item" => {
let ty = n.child_by_field_name("type");
return (
n.child_by_field_name("type")
.and_then(|ty| rust_type_name(ty, source)),
ty.and_then(|ty| rust_type_name(ty, source)),
n.child_by_field_name("trait")
.and_then(|ty| rust_type_name(ty, source)),
ty.and_then(|ty| rust_type_path(ty, source)),
);
}
"trait_item" => {
@ -1661,13 +1669,30 @@ fn rust_impl_context(node: Node<'_>, source: &str) -> (Option<String>, Option<St
None,
n.child_by_field_name("name")
.map(|name| text(name, source).to_owned()),
None,
);
}
"function_item" | "closure_expression" => return (None, None),
"function_item" | "closure_expression" => return (None, None, None),
_ => parent = n.parent(),
}
}
(None, None)
(None, None, None)
}
/// A type written as a path (`crate::a::Foo`), references and generic arguments
/// removed; `None` for a bare `Foo`.
fn rust_type_path(ty: Node<'_>, source: &str) -> Option<String> {
match ty.kind() {
"scoped_type_identifier" => Some(format!(
"{}::{}",
text(ty.child_by_field_name("path")?, source),
text(ty.child_by_field_name("name")?, source)
)),
"generic_type" | "reference_type" => {
rust_type_path(ty.child_by_field_name("type")?, source)
}
_ => None,
}
}
/// Leaf type name with references and generic arguments removed.

View file

@ -202,3 +202,39 @@ fn same_named_types_in_different_modules_resolve_through_use_bindings() {
let guess = function(&model, "src/other.rs", "guess");
assert_eq!(targets(guess), [(String::new(), "ambiguous".to_owned())]);
}
#[test]
fn an_impl_on_a_qualified_path_in_another_file_keeps_its_methods() {
// With several files declaring `Foo`, an impl was kept only if its own file
// resolved `Foo`; `impl crate::a::Foo` in a third file resolved nothing and its
// methods were dropped, so the call read "member-not-found".
let model = scan_files(&[
(
"src/lib.rs",
"pub mod a;\npub mod c;\npub mod ext;\npub mod main_use;\n",
),
("src/a.rs", "pub struct Foo;\n"),
("src/c.rs", "pub struct Foo;\n"),
(
"src/ext.rs",
"impl crate::a::Foo {\n pub fn run(&self) {}\n}\n",
),
(
"src/main_use.rs",
"use crate::a::Foo;\npub fn f(x: Foo) {\n x.run();\n}\n",
),
]);
let caller = function(&model, "src/main_use.rs", "f");
let targets = calls(&model, caller, "source.resolution");
assert!(
targets.iter().any(|(_, name, _)| name == "run"),
"{targets:?}"
);
let run = function(&model, "src/ext.rs", "run");
assert!(
model
.edges
.iter()
.any(|e| e.kind == EdgeKind::Calls && e.source == caller.id && e.target == run.id)
);
}