From 45b92db41bc54e201929a322dc2dd16441d34380 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Mon, 15 Apr 2024 18:57:47 +0200 Subject: [PATCH] feat: method resolution (#561) --- crates/mun_hir/src/code_model.rs | 2 +- crates/mun_hir/src/code_model/function.rs | 27 +- crates/mun_hir/src/db.rs | 6 +- crates/mun_hir/src/display.rs | 106 +++++++- crates/mun_hir/src/method_resolution.rs | 298 +++++++++++++++++++++- crates/mun_hir/src/name.rs | 8 + crates/mun_hir/src/path.rs | 35 +++ crates/mun_hir/src/visibility.rs | 18 +- 8 files changed, 488 insertions(+), 12 deletions(-) diff --git a/crates/mun_hir/src/code_model.rs b/crates/mun_hir/src/code_model.rs index 7881eb23..1fdd2e79 100644 --- a/crates/mun_hir/src/code_model.rs +++ b/crates/mun_hir/src/code_model.rs @@ -12,7 +12,7 @@ pub use self::{ function::{Function, FunctionData}, module::{Module, ModuleDef}, package::Package, - r#impl::ImplData, + r#impl::{AssocItem, Impl, ImplData}, r#struct::{Field, Struct, StructData, StructKind, StructMemoryKind}, src::HasSource, type_alias::{TypeAlias, TypeAliasData}, diff --git a/crates/mun_hir/src/code_model/function.rs b/crates/mun_hir/src/code_model/function.rs index 44e83091..d9bc8a63 100644 --- a/crates/mun_hir/src/code_model/function.rs +++ b/crates/mun_hir/src/code_model/function.rs @@ -12,7 +12,7 @@ use crate::{ type_ref::{LocalTypeRefId, TypeRefMap, TypeRefSourceMap}, visibility::RawVisibility, Body, DefDatabase, DiagnosticSink, FileId, HasSource, HasVisibility, HirDatabase, InFile, - InferenceResult, Name, Ty, Visibility, + InferenceResult, Name, Pat, Ty, Visibility, }; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] @@ -96,6 +96,11 @@ impl FunctionData { pub fn type_ref_map(&self) -> &TypeRefMap { &self.type_ref_map } + + /// Returns true if this function is an extern function. + pub fn is_extern(&self) -> bool { + self.is_extern + } } impl Function { @@ -214,12 +219,26 @@ impl Param { .nth(self.idx) .map(|value| InFile { file_id, value }) } + + /// Returns the name of the parameter. + /// + /// Only if the parameter is a named binding will this function return a + /// name. If the function parameter is a wildcard for instance then this + /// function will return `None`. + pub fn name(&self, db: &dyn HirDatabase) -> Option { + let body = self.func.body(db); + let pat_id = body.params().get(self.idx)?.0; + let pat = &body[pat_id]; + if let Pat::Bind { name, .. } = pat { + Some(name.clone()) + } else { + None + } + } } impl HasVisibility for Function { fn visibility(&self, db: &dyn HirDatabase) -> Visibility { - self.data(db.upcast()) - .visibility - .resolve(db.upcast(), &self.id.resolver(db.upcast())) + db.function_visibility(self.id) } } diff --git a/crates/mun_hir/src/db.rs b/crates/mun_hir/src/db.rs index 75e1348f..247ceddd 100644 --- a/crates/mun_hir/src/db.rs +++ b/crates/mun_hir/src/db.rs @@ -19,7 +19,8 @@ use crate::{ name_resolution::Namespace, package_defs::PackageDefs, ty::{lower::LowerTyMap, CallableDef, FnSig, InferenceResult, Ty, TypableDef}, - AstIdMap, Body, ExprScopes, FileId, PackageId, PackageSet, Struct, TypeAlias, + visibility, AstIdMap, Body, ExprScopes, FileId, PackageId, PackageSet, Struct, TypeAlias, + Visibility, }; // TODO(bas): In the future maybe move this to a seperate crate (mun_db?) @@ -104,6 +105,9 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast { #[salsa::invoke(crate::FunctionData::fn_data_query)] fn fn_data(&self, func: FunctionId) -> Arc; + #[salsa::invoke(visibility::function_visibility_query)] + fn function_visibility(&self, def: FunctionId) -> Visibility; + /// Returns the `PackageDefs` for the specified `PackageId`. The /// `PackageDefs` contains all resolved items defined for every module /// in the package. diff --git a/crates/mun_hir/src/display.rs b/crates/mun_hir/src/display.rs index 367cf54e..3323410e 100644 --- a/crates/mun_hir/src/display.rs +++ b/crates/mun_hir/src/display.rs @@ -1,6 +1,11 @@ use std::fmt; -use crate::db::HirDatabase; +use crate::{ + code_model::AssocItem, + db::HirDatabase, + type_ref::{LocalTypeRefId, TypeRef, TypeRefMap}, + Function, HasVisibility, ModuleId, Visibility, +}; pub struct HirFormatter<'a, 'b> { pub db: &'a dyn HirDatabase, @@ -50,3 +55,102 @@ where self.1.hir_fmt(&mut HirFormatter { db: self.0, fmt: f }) } } + +impl HirDisplay for AssocItem { + fn hir_fmt(&self, f: &mut HirFormatter<'_, '_>) -> fmt::Result { + match self { + AssocItem::Function(fun) => fun.hir_fmt(f), + } + } +} + +impl HirDisplay for Function { + fn hir_fmt(&self, f: &mut HirFormatter<'_, '_>) -> fmt::Result { + let db = f.db; + let data = db.fn_data(self.id); + let module = self.module(db); + write_visiblity(module.id, self.visibility(db), f)?; + if data.is_extern() { + write!(f, "extern ")?; + } + write!(f, "fn {}(", self.name(db))?; + + let type_map = data.type_ref_map(); + for (idx, (&type_ref_id, param)) in data.params().iter().zip(self.params(db)).enumerate() { + let name = param.name(db); + if idx != 0 { + write!(f, ", ")?; + } + match name { + Some(name) => write!(f, "{name}: ")?, + None => write!(f, "_: ")?, + } + write_type_ref(type_ref_id, type_map, f)?; + } + + write!(f, ")")?; + + let ret_type_id = *data.ret_type(); + match &type_map[ret_type_id] { + TypeRef::Tuple(elems) if elems.is_empty() => {} + _ => { + write!(f, " -> ")?; + write_type_ref(ret_type_id, type_map, f)?; + } + } + + Ok(()) + } +} + +fn write_type_ref( + type_ref_id: LocalTypeRefId, + container: &TypeRefMap, + f: &mut HirFormatter<'_, '_>, +) -> fmt::Result { + let type_ref = &container[type_ref_id]; + match type_ref { + TypeRef::Path(path) => write!(f, "{path}"), + TypeRef::Array(element_ty) => { + write!(f, "[")?; + write_type_ref(*element_ty, container, f)?; + write!(f, "]") + } + TypeRef::Never => write!(f, "!"), + TypeRef::Tuple(elems) => { + write!(f, "(")?; + for (idx, elem) in elems.iter().enumerate() { + if idx != 0 { + write!(f, ", ")?; + } + write_type_ref(*elem, container, f)?; + } + write!(f, ")") + } + TypeRef::Error => write!(f, "{{error}}"), + } +} + +/// Writes the visibility of an item to the formatter. +fn write_visiblity( + module_id: ModuleId, + vis: Visibility, + f: &mut HirFormatter<'_, '_>, +) -> fmt::Result { + match vis { + Visibility::Public => write!(f, "pub "), + Visibility::Module(vis_id) => { + let module_tree = f.db.module_tree(module_id.package); + if module_id == vis_id { + // Only visible to self + Ok(()) + } else if vis_id.local_id == module_tree.root { + write!(f, "pub(package) ") + } else if module_tree[module_id.local_id].parent == Some(vis_id.local_id) { + write!(f, "pub(super) ") + } else { + write!(f, "pub(in ...) ") + } + } + } +} diff --git a/crates/mun_hir/src/method_resolution.rs b/crates/mun_hir/src/method_resolution.rs index 358999fc..faba98d9 100644 --- a/crates/mun_hir/src/method_resolution.rs +++ b/crates/mun_hir/src/method_resolution.rs @@ -1,5 +1,6 @@ use std::{ collections::{hash_map::Entry, HashMap}, + ops::ControlFlow, sync::Arc, }; @@ -10,11 +11,11 @@ use crate::{ db::HirDatabase, diagnostics::{DuplicateDefinition, ImplForForeignType, InvalidSelfTyImpl}, has_module::HasModule, - ids::{AssocItemId, ImplId, Lookup, StructId}, + ids::{AssocItemId, FunctionId, ImplId, Lookup, StructId}, module_tree::LocalModuleId, package_defs::PackageDefs, ty::lower::LowerDiagnostic, - DefDatabase, DiagnosticSink, HasSource, InFile, ModuleId, PackageId, Ty, TyKind, + DefDatabase, DiagnosticSink, HasSource, InFile, ModuleId, Name, PackageId, Ty, TyKind, }; #[derive(Clone, Debug, PartialEq, Eq)] @@ -228,10 +229,177 @@ fn assoc_item_name(db: &dyn DefDatabase, id: &AssocItemId) -> String { } } +/// An object to iterate over methods associated with a type. +pub struct MethodResolutionCtx<'db> { + pub db: &'db dyn HirDatabase, + + /// The type for which to resolve methods + pub ty: Ty, + + /// Filter based on this name + name: Option, + + /// Filter based on visibility from this module + visible_from: Option, +} + +enum IsValidCandidate { + Yes, + No, + NotVisible, +} + +impl<'db> MethodResolutionCtx<'db> { + pub fn new(db: &'db dyn HirDatabase, ty: Ty) -> Self { + Self { + db, + ty, + name: None, + visible_from: None, + } + } + + /// Only include methods with the specified name. + pub fn with_name(self, name: Name) -> Self { + Self { + name: Some(name), + ..self + } + } + + /// Only include methods that are visible from the specified module. + pub fn visible_from(self, module_id: ModuleId) -> Self { + Self { + visible_from: Some(module_id), + ..self + } + } + + /// Collects all methods that match the specified criteria. + /// + /// If the callback method returns `Some(_)`, the iteration will stop and + /// value will be returned. + pub fn collect( + &self, + mut callback: impl FnMut(AssocItemId, bool) -> Option, + ) -> Option { + match self.collect_inner(|item, visible| match callback(item, visible) { + Some(r) => ControlFlow::Break(r), + None => ControlFlow::Continue(()), + }) { + ControlFlow::Continue(()) => None, + ControlFlow::Break(r) => Some(r), + } + } + + fn collect_inner( + &self, + mut callback: impl FnMut(AssocItemId, bool) -> ControlFlow, + ) -> ControlFlow { + let Some(package_id) = self.defining_package() else { + return ControlFlow::Continue(()); + }; + let inherent_impls = self.db.inherent_impls_in_package(package_id); + let impls = inherent_impls.for_self_ty(&self.ty); + for &self_impl in impls { + let impl_data = self.db.impl_data(self_impl); + for item in impl_data.items.iter().copied() { + let visible = match self.is_valid_candidate(self_impl, item) { + IsValidCandidate::Yes => true, + IsValidCandidate::No => continue, + IsValidCandidate::NotVisible => false, + }; + callback(item, visible)?; + } + } + + ControlFlow::Continue(()) + } + + /// Returns the package in which the type was defined. + fn defining_package(&self) -> Option { + match self.ty.interned() { + TyKind::Struct(s) => { + let module = s.module(self.db); + Some(module.id.package) + } + _ => None, + } + } + + /// Returns whether the specified item is a valid candidate for method + /// resolution based on the filters. + fn is_valid_candidate(&self, impl_id: ImplId, item: AssocItemId) -> IsValidCandidate { + match item { + AssocItemId::FunctionId(f) => self.is_valid_function_candidate(impl_id, f), + } + } + + /// Returns true if the specified function is a valid candidate for method + /// resolution based on the filters. + fn is_valid_function_candidate( + &self, + _impl_id: ImplId, + fun_id: FunctionId, + ) -> IsValidCandidate { + let data = self.db.fn_data(fun_id); + + // Check if the name matches + if let Some(name) = &self.name { + if data.name() != name { + return IsValidCandidate::No; + } + } + + // Check if the function is visible from the selected module + if let Some(visible_from) = self.visible_from { + if !self + .db + .function_visibility(fun_id) + .is_visible_from(self.db, visible_from) + { + return IsValidCandidate::NotVisible; + } + } + + IsValidCandidate::Yes + } +} + +/// Find the method with the specified name on the specified type. +/// +/// Returns `Ok` if the method was found, `Err(None)` if no method by that name +/// was found and `Err(Some(_))` if a method by that name was found but it is +/// not visible from the selected module. +pub(crate) fn lookup_method( + db: &dyn HirDatabase, + ty: &Ty, + visible_from_module: ModuleId, + name: &Name, +) -> Result> { + let mut not_visible = None; + MethodResolutionCtx::new(db, ty.clone()) + .visible_from(visible_from_module) + .with_name(name.clone()) + .collect(|item, visible| match item { + AssocItemId::FunctionId(f) if visible => Some(f), + AssocItemId::FunctionId(f) => { + not_visible = Some(f); + None + } + }) + .ok_or(not_visible) +} + #[cfg(test)] mod tests { use crate::{ - mock::MockDatabase, with_fixture::WithFixture, DiagnosticSink, HirDatabase, SourceDatabase, + code_model::AssocItem, + display::HirDisplay, + method_resolution::{lookup_method, MethodResolutionCtx}, + mock::MockDatabase, + with_fixture::WithFixture, + DiagnosticSink, HirDatabase, Module, ModuleDef, Name, Package, SourceDatabase, Ty, }; #[test] @@ -312,4 +480,128 @@ mod tests { 63..77: the name `bar` is defined multiple times "###); } + + struct Fixture { + db: MockDatabase, + root_module: Module, + foo_ty: Ty, + } + + impl Fixture { + pub fn new() -> Self { + let db = MockDatabase::with_files( + r#" + //- /mod.mun + struct Foo; + impl Foo { + fn bar(); + } + //- /foo.mun + use super::Foo; + + impl Foo { + fn baz(value: i32); + } + "#, + ); + + let package = Package::all(&db).into_iter().next().unwrap(); + let root_module = package.root_module(&db); + + let foo_ty = root_module + .declarations(&db) + .into_iter() + .find_map(|decl| match decl { + ModuleDef::Struct(s) if s.name(&db).as_str() == Some("Foo") => Some(s.ty(&db)), + _ => None, + }) + .unwrap(); + + Self { + db, + root_module, + foo_ty, + } + } + } + + #[test] + fn test_method_resolution_visibility() { + let fixture = Fixture::new(); + + insta::assert_snapshot!( + display_method_resolution( + MethodResolutionCtx::new(&fixture.db, fixture.foo_ty) + .visible_from(fixture.root_module.id)), + @r###" + + fn bar() + - fn baz(value: i32) + "###); + } + + #[test] + fn test_method_resolution_by_name() { + let fixture = Fixture::new(); + + insta::assert_snapshot!( + display_method_resolution( + MethodResolutionCtx::new(&fixture.db, fixture.foo_ty) + .with_name(Name::new("bar"))), + @r###" + + fn bar() + "###); + } + + #[test] + fn test_lookup_method() { + let fixture = Fixture::new(); + assert!(lookup_method( + &fixture.db, + &fixture.foo_ty, + fixture.root_module.id, + &Name::new("bar"), + ) + .is_ok()) + } + + #[test] + fn test_lookup_method_not_found() { + let fixture = Fixture::new(); + assert!(lookup_method( + &fixture.db, + &fixture.foo_ty, + fixture.root_module.id, + &Name::new("not_found"), + ) + .unwrap_err() + .is_none()) + } + + #[test] + fn test_lookup_method_not_visible() { + let fixture = Fixture::new(); + assert!(lookup_method( + &fixture.db, + &fixture.foo_ty, + fixture.root_module.id, + &Name::new("baz"), + ) + .unwrap_err() + .is_some()) + } + + fn display_method_resolution(ctx: MethodResolutionCtx<'_>) -> String { + let mut methods = Vec::new(); + ctx.collect(|item, visible| { + methods.push(format!( + "{}{}", + if visible { "+ " } else { "- " }, + AssocItem::from(item).display(ctx.db) + )); + + None::<()> + }); + + methods.join("\n") + } } diff --git a/crates/mun_hir/src/name.rs b/crates/mun_hir/src/name.rs index d6464e34..f6ee3a32 100644 --- a/crates/mun_hir/src/name.rs +++ b/crates/mun_hir/src/name.rs @@ -61,6 +61,14 @@ impl Name { Repr::Text(_) => None, } } + + /// Returns the text this name represents if it isn't a tuple field. + pub fn as_str(&self) -> Option<&str> { + match &self.0 { + Repr::Text(it) => Some(it), + Repr::TupleField(_) => None, + } + } } pub(crate) trait AsName { diff --git a/crates/mun_hir/src/path.rs b/crates/mun_hir/src/path.rs index 69f49da8..b3a7d6f8 100644 --- a/crates/mun_hir/src/path.rs +++ b/crates/mun_hir/src/path.rs @@ -1,3 +1,5 @@ +use std::{fmt, fmt::Formatter}; + use mun_syntax::{ ast, ast::{NameOwner, PathSegmentKind}, @@ -119,6 +121,39 @@ impl Path { } } +impl fmt::Display for Path { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut first_segment = true; + let mut add_segment = |s| -> fmt::Result { + if !first_segment { + write!(f, "::")?; + } + first_segment = false; + write!(f, "{s}") + }; + + match self.kind { + PathKind::Plain => {} + PathKind::Super(0) => add_segment("self")?, + PathKind::Super(lvl) => { + for _ in 0..lvl { + add_segment("super")?; + } + } + PathKind::Package => add_segment("package")?, + } + + for segment in &self.segments { + if !first_segment { + write!(f, "::")?; + } + first_segment = false; + write!(f, "{segment}")?; + } + Ok(()) + } +} + /// Given an `ast::UseTree` and an optional prefix, call a callback function for /// every item that is contained in the import tree. /// diff --git a/crates/mun_hir/src/visibility.rs b/crates/mun_hir/src/visibility.rs index 600727e0..79a1a7fa 100644 --- a/crates/mun_hir/src/visibility.rs +++ b/crates/mun_hir/src/visibility.rs @@ -3,9 +3,10 @@ use std::iter::successors; use mun_syntax::ast; use crate::{ - ids::ModuleId, + ids::{FunctionId, ModuleId}, module_tree::{LocalModuleId, ModuleTree}, - DefDatabase, HirDatabase, Resolver, + resolve::HasResolver, + DefDatabase, HirDatabase, Module, Resolver, }; /// Visibility of an item, not yet resolved to an actual module. @@ -105,5 +106,18 @@ impl Visibility { } pub trait HasVisibility { + /// Returns the visibility of the item. fn visibility(&self, db: &dyn HirDatabase) -> Visibility; + + /// Returns true if the item is visible from the specified module. + fn is_visible_from(&self, db: &dyn HirDatabase, module: Module) -> bool { + let vis = self.visibility(db); + vis.is_visible_from(db, module.id) + } +} + +/// Resolve visibility of a function. +pub(crate) fn function_visibility_query(db: &dyn DefDatabase, def: FunctionId) -> Visibility { + let resolver = def.resolver(db); + db.fn_data(def).visibility().resolve(db, &resolver) }