From a44317c2949881a73d90f227c22acc68a652c903 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Tue, 19 Dec 2023 17:55:36 +0100 Subject: [PATCH 1/4] feat: parsing of impls and integration in item tree --- .../src/code_model/struct/validator/tests.rs | 6 +- crates/mun_hir/src/db.rs | 2 + crates/mun_hir/src/ids.rs | 7 ++ crates/mun_hir/src/item_scope.rs | 9 ++ crates/mun_hir/src/item_tree.rs | 30 +++++ crates/mun_hir/src/item_tree/lower.rs | 52 ++++++-- crates/mun_hir/src/item_tree/pretty.rs | 24 +++- .../mun_hir__item_tree__tests__impls.snap | 17 +++ ...ir__item_tree__tests__top_level_items.snap | 7 +- .../mun_hir__item_tree__tests__use.snap | 10 ++ crates/mun_hir/src/item_tree/tests.rs | 21 ++++ crates/mun_hir/src/package_defs/collector.rs | 28 ++++- crates/mun_hir/src/utils.rs | 12 +- crates/mun_syntax/src/ast/generated.rs | 119 +++++++++++++++++- crates/mun_syntax/src/grammar.ron | 25 +++- crates/mun_syntax/src/lib.rs | 9 +- crates/mun_syntax/src/parsing/grammar.rs | 1 + .../src/parsing/grammar/declarations.rs | 36 +++--- .../mun_syntax/src/parsing/grammar/traits.rs | 31 +++++ crates/mun_syntax/src/syntax_error.rs | 4 + .../mun_syntax/src/syntax_kind/generated.rs | 13 ++ crates/mun_syntax/src/tests/lexer.rs | 7 +- crates/mun_syntax/src/tests/parser.rs | 85 +++++++++++++ crates/mun_syntax/src/validation.rs | 57 +++++++++ 24 files changed, 560 insertions(+), 52 deletions(-) create mode 100644 crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__impls.snap create mode 100644 crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__use.snap create mode 100644 crates/mun_syntax/src/parsing/grammar/traits.rs create mode 100644 crates/mun_syntax/src/validation.rs diff --git a/crates/mun_hir/src/code_model/struct/validator/tests.rs b/crates/mun_hir/src/code_model/struct/validator/tests.rs index b7a3b4b69..59596c209 100644 --- a/crates/mun_hir/src/code_model/struct/validator/tests.rs +++ b/crates/mun_hir/src/code_model/struct/validator/tests.rs @@ -5,7 +5,7 @@ use crate::utils::tests::*; fn test_private_leak_struct_fields() { insta::assert_snapshot!(diagnostics( r#" - + struct Foo(usize); pub struct Bar(usize); @@ -27,7 +27,7 @@ fn test_private_leak_struct_fields() { pub bar: Bar, } - pub(crate) struct BarBaz; + pub(package) struct BarBaz; // invalid, exporting pub(crate) to pub pub struct FooBarBaz { @@ -37,6 +37,6 @@ fn test_private_leak_struct_fields() { "#), @r###" 180..183: can't leak private type - 392..395: can't leak private type + 394..397: can't leak private type "###); } diff --git a/crates/mun_hir/src/db.rs b/crates/mun_hir/src/db.rs index 3fbab0e07..05f2f7295 100644 --- a/crates/mun_hir/src/db.rs +++ b/crates/mun_hir/src/db.rs @@ -82,6 +82,8 @@ pub trait InternDatabase: SourceDatabase { fn intern_struct(&self, loc: ids::StructLoc) -> ids::StructId; #[salsa::interned] fn intern_type_alias(&self, loc: ids::TypeAliasLoc) -> ids::TypeAliasId; + #[salsa::interned] + fn intern_impl(self, loc: ids::ImplLoc) -> ids::ImplId; } #[salsa::query_group(DefDatabaseStorage)] diff --git a/crates/mun_hir/src/ids.rs b/crates/mun_hir/src/ids.rs index a249cbb51..18b9f2be7 100644 --- a/crates/mun_hir/src/ids.rs +++ b/crates/mun_hir/src/ids.rs @@ -1,3 +1,4 @@ +use crate::item_tree::Impl; use crate::{ item_tree::{Function, ItemTreeId, ItemTreeNode, Struct, TypeAlias}, module_tree::LocalModuleId, @@ -90,6 +91,12 @@ pub struct ModuleId { pub local_id: LocalModuleId, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct ImplId(salsa::InternId); + +pub(crate) type ImplLoc = AssocItemLoc; +impl_intern!(ImplId, ImplLoc, intern_impl, lookup_intern_impl); + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] pub struct FunctionId(salsa::InternId); diff --git a/crates/mun_hir/src/item_scope.rs b/crates/mun_hir/src/item_scope.rs index fc7f827de..579bcf03c 100644 --- a/crates/mun_hir/src/item_scope.rs +++ b/crates/mun_hir/src/item_scope.rs @@ -1,3 +1,4 @@ +use crate::ids::ImplId; use crate::{ ids::ItemDefinitionId, module_tree::LocalModuleId, primitive_type::PrimitiveType, visibility::Visibility, Name, PerNs, @@ -41,6 +42,9 @@ pub struct ItemScope { /// All items that are defined in this scope defs: Vec, + + /// All implementations that are defined in this scope + impls: Vec, } /// A struct that is returned from `add_resolution_from_import`. @@ -85,6 +89,11 @@ impl ItemScope { self.defs.push(def); } + /// Adds an implementation to the list of implementations + pub(crate) fn define_impl(&mut self, impl_: ImplId) { + self.impls.push(impl_); + } + /// Adds a named item resolution into the scope. Returns true if adding the resolution changes /// the scope. pub(crate) fn add_resolution( diff --git a/crates/mun_hir/src/item_tree.rs b/crates/mun_hir/src/item_tree.rs index 6d63e710d..99075f1da 100644 --- a/crates/mun_hir/src/item_tree.rs +++ b/crates/mun_hir/src/item_tree.rs @@ -111,6 +111,7 @@ struct ItemTreeData { structs: Arena, fields: Arena, type_aliases: Arena, + impls: Arena, visibilities: ItemVisibilities, } @@ -222,6 +223,7 @@ mod_items! { Struct in structs -> ast::StructDef, TypeAlias in type_aliases -> ast::TypeAliasDef, Import in imports -> ast::Use, + Impl in impls -> ast::Impl, } macro_rules! impl_index { @@ -309,6 +311,14 @@ pub struct Struct { pub ast_id: FileAstId, } +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct Impl { + pub types: TypeRefMap, + pub self_ty: LocalTypeRefId, + pub items: Box<[AssociatedItem]>, + pub ast_id: FileAstId, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct TypeAlias { pub name: Name, @@ -318,6 +328,25 @@ pub struct TypeAlias { pub ast_id: FileAstId, } +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub enum AssociatedItem { + Function(LocalItemTreeId), +} + +impl From> for AssociatedItem { + fn from(value: LocalItemTreeId) -> Self { + AssociatedItem::Function(value) + } +} + +impl From for ModItem { + fn from(item: AssociatedItem) -> Self { + match item { + AssociatedItem::Function(it) => it.into(), + } + } +} + #[derive(Debug, Clone, Eq, PartialEq)] pub enum StructDefKind { /// `struct S { ... }` - type namespace only. @@ -430,6 +459,7 @@ mod diagnostics { ModItem::Import(item) => { SyntaxNodePtr::new(item_tree.source(db, item).syntax()) } + ModItem::Impl(item) => SyntaxNodePtr::new(item_tree.source(db, item).syntax()), } } diff --git a/crates/mun_hir/src/item_tree/lower.rs b/crates/mun_hir/src/item_tree/lower.rs index 160a4e48b..7ba4c328d 100644 --- a/crates/mun_hir/src/item_tree/lower.rs +++ b/crates/mun_hir/src/item_tree/lower.rs @@ -1,27 +1,31 @@ //! This module implements the logic to convert an AST to an `ItemTree`. use super::{ - diagnostics, Field, Fields, Function, IdRange, ItemTree, ItemTreeData, ItemTreeNode, - ItemVisibilities, LocalItemTreeId, ModItem, RawVisibilityId, Struct, TypeAlias, + diagnostics, AssociatedItem, Field, Fields, Function, IdRange, Impl, ItemTree, ItemTreeData, + ItemTreeNode, ItemVisibilities, LocalItemTreeId, ModItem, RawVisibilityId, Struct, TypeAlias, }; -use crate::item_tree::Import; -use crate::type_ref::{TypeRefMap, TypeRefMapBuilder}; use crate::{ arena::{Idx, RawId}, + item_tree::Import, name::AsName, source_id::AstIdMap, + type_ref::{TypeRefMap, TypeRefMapBuilder}, visibility::RawVisibility, DefDatabase, FileId, InFile, Name, Path, }; -use mun_syntax::{ - ast, - ast::{ExternOwner, ModuleItemOwner, NameOwner, StructKind, TypeAscriptionOwner}, +use mun_syntax::ast::{ + self, ExternOwner, ModuleItemOwner, NameOwner, StructKind, TypeAscriptionOwner, }; use smallvec::SmallVec; use std::{collections::HashMap, convert::TryInto, marker::PhantomData, sync::Arc}; struct ModItems(SmallVec<[ModItem; 1]>); +struct Foo {} +impl Foo { + fn bar() {} +} + impl From for ModItems where T: Into, @@ -73,7 +77,7 @@ impl Context { ModItem::Function(item) => Some(&self.data.functions[item.index].name), ModItem::Struct(item) => Some(&self.data.structs[item.index].name), ModItem::TypeAlias(item) => Some(&self.data.type_aliases[item.index].name), - ModItem::Import(_) => None, + ModItem::Impl(_) | ModItem::Import(_) => None, }; if let Some(name) = name { if let Some(first_item) = set.get(name) { @@ -106,6 +110,7 @@ impl Context { ast::ModuleItemKind::Use(ast) => Some(ModItems( self.lower_use(&ast).into_iter().map(Into::into).collect(), )), + ast::ModuleItemKind::Impl(ast) => self.lower_impl(&ast).map(Into::into), } } @@ -266,6 +271,37 @@ impl Context { Some(self.data.type_aliases.alloc(res).into()) } + fn lower_impl(&mut self, impl_def: &ast::Impl) -> Option> { + let ast_id = self.source_ast_id_map.ast_id(impl_def); + let mut types = TypeRefMap::builder(); + let self_ty = impl_def.type_ref().map(|ty| types.alloc_from_node(&ty))?; + + let items = impl_def + .associated_item_list() + .into_iter() + .flat_map(|it| it.associated_items()) + .filter_map(|item| self.lower_associated_item(&item)) + .collect(); + + let (types, _types_source_map) = types.finish(); + + let res = Impl { + types, + self_ty, + items, + ast_id, + }; + + Some(self.data.impls.alloc(res).into()) + } + + fn lower_associated_item(&mut self, item: &ast::AssociatedItem) -> Option { + let item: AssociatedItem = match item.kind() { + ast::AssociatedItemKind::FunctionDef(ast) => self.lower_function(&ast).map(Into::into), + }?; + Some(item) + } + /// Returns the `Idx` of the next `Field` fn next_field_idx(&self) -> Idx { let idx: u32 = self.data.fields.len().try_into().expect("too many fields"); diff --git a/crates/mun_hir/src/item_tree/pretty.rs b/crates/mun_hir/src/item_tree/pretty.rs index f066cbb4a..4005f7310 100644 --- a/crates/mun_hir/src/item_tree/pretty.rs +++ b/crates/mun_hir/src/item_tree/pretty.rs @@ -1,4 +1,4 @@ -use crate::item_tree::LocalItemTreeId; +use crate::item_tree::{Impl, LocalItemTreeId}; use crate::{ item_tree::{Fields, Function, Import, ItemTree, ModItem, RawVisibilityId, Struct, TypeAlias}, path::ImportAlias, @@ -64,6 +64,7 @@ impl Printer<'_> { ModItem::Struct(it) => self.print_struct(it), ModItem::TypeAlias(it) => self.print_type_alias(it), ModItem::Import(it) => self.print_use(it), + ModItem::Impl(it) => self.print_impl(it), } } @@ -201,6 +202,27 @@ impl Printer<'_> { fn print_type_ref(&mut self, type_ref: LocalTypeRefId, map: &TypeRefMap) -> fmt::Result { print_type_ref(self.db, map, type_ref, self) } + + /// Prints an `impl` block to the buffer. + fn print_impl(&mut self, it: LocalItemTreeId) -> fmt::Result { + let Impl { + types, + self_ty, + items, + ast_id: _, + } = &self.tree[it]; + write!(self, "impl ")?; + self.print_type_ref(*self_ty, types)?; + self.whitespace()?; + write!(self, "{{")?; + self.indented(|this| { + for item in items.iter().copied() { + this.print_mod_item(item.into())?; + } + Ok(()) + })?; + write!(self, "}}") + } } impl Write for Printer<'_> { diff --git a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__impls.snap b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__impls.snap new file mode 100644 index 000000000..d928ffb88 --- /dev/null +++ b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__impls.snap @@ -0,0 +1,17 @@ +--- +source: crates/mun_hir/src/item_tree/tests.rs +expression: "print_item_tree(r#\"\n impl Bar {\n fn foo(a:i32, b:u8, c:String) -> i32 {}\n pub fn bar(a:i32, b:u8, c:String) -> {}\n }\n \"#).unwrap()" +--- +impl Bar { + fn foo( + i32, + u8, + String, + ) -> i32; + pub fn bar( + i32, + u8, + String, + ) -> (); +} + diff --git a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap index 369f60b50..3d1f48ee1 100644 --- a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap +++ b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap @@ -1,6 +1,6 @@ --- source: crates/mun_hir/src/item_tree/tests.rs -expression: "print_item_tree(r#\"\n fn foo(a:i32, b:u8, c:String) -> i32 {}\n pub fn bar(a:i32, b:u8, c:String) -> {}\n pub(super) fn bar(a:i32, b:u8, c:String) -> {}\n pub(package) fn baz(a:i32, b:, c:String) -> {}\n extern fn eval(a:String) -> bool;\n\n struct Foo {\n a: i32,\n b: u8,\n c: String,\n }\n struct Foo2 {\n a: i32,\n b: ,\n c: String,\n }\n struct Bar (i32, u32, String)\n struct Baz;\n\n type FooBar = Foo;\n type FooBar = package::Foo;\n\n pub use foo;\n use super::bar;\n use super::*;\n use foo::{bar as _, baz::hello as world};\n \"#).unwrap()" +expression: "print_item_tree(r#\"\n fn foo(a:i32, b:u8, c:String) -> i32 {}\n pub fn bar(a:i32, b:u8, c:String) -> {}\n pub(super) fn bar(a:i32, b:u8, c:String) -> {}\n pub(package) fn baz(a:i32, b:, c:String) -> {}\n extern fn eval(a:String) -> bool;\n\n struct Foo {\n a: i32,\n b: u8,\n c: String,\n }\n struct Foo2 {\n a: i32,\n b: ,\n c: String,\n }\n struct Bar (i32, u32, String)\n struct Baz;\n\n type FooBar = Foo;\n type FooBar = package::Foo;\n \"#).unwrap()" --- fn foo( i32, @@ -43,9 +43,4 @@ struct Bar( struct Baz; type FooBar = Foo; type FooBar = package::Foo; -pub use foo; -use super::bar; -use super::*; -use foo::bar as _; -use foo::baz::hello as world; diff --git a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__use.snap b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__use.snap new file mode 100644 index 000000000..18bbdbe6d --- /dev/null +++ b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__use.snap @@ -0,0 +1,10 @@ +--- +source: crates/mun_hir/src/item_tree/tests.rs +expression: "print_item_tree(r#\"\n pub use foo;\n use super::bar;\n use super::*;\n use foo::{bar as _, baz::hello as world};\n \"#).unwrap()" +--- +pub use foo; +use super::bar; +use super::*; +use foo::bar as _; +use foo::baz::hello as world; + diff --git a/crates/mun_hir/src/item_tree/tests.rs b/crates/mun_hir/src/item_tree/tests.rs index a51cd198f..7ca036822 100644 --- a/crates/mun_hir/src/item_tree/tests.rs +++ b/crates/mun_hir/src/item_tree/tests.rs @@ -32,11 +32,32 @@ fn top_level_items() { type FooBar = Foo; type FooBar = package::Foo; + "# + ) + .unwrap()); +} +#[test] +fn test_use() { + insta::assert_snapshot!(print_item_tree( + r#" pub use foo; use super::bar; use super::*; use foo::{bar as _, baz::hello as world}; + "# + ) + .unwrap()); +} + +#[test] +fn test_impls() { + insta::assert_snapshot!(print_item_tree( + r#" + impl Bar { + fn foo(a:i32, b:u8, c:String) -> i32 {} + pub fn bar(a:i32, b:u8, c:String) -> {} + } "# ) .unwrap()); diff --git a/crates/mun_hir/src/package_defs/collector.rs b/crates/mun_hir/src/package_defs/collector.rs index 170dafaf4..6142ae945 100644 --- a/crates/mun_hir/src/package_defs/collector.rs +++ b/crates/mun_hir/src/package_defs/collector.rs @@ -1,13 +1,11 @@ use super::PackageDefs; -use crate::item_tree::Fields; use crate::{ arena::map::ArenaMap, - ids::ItemDefinitionId, - ids::{FunctionLoc, Intern, StructLoc, TypeAliasLoc}, - item_scope::ImportType, - item_scope::{ItemScope, PerNsGlobImports}, + ids::{FunctionLoc, ImplLoc, Intern, ItemDefinitionId, StructLoc, TypeAliasLoc}, + item_scope::{ImportType, ItemScope, PerNsGlobImports}, item_tree::{ - self, Function, ItemTree, ItemTreeId, LocalItemTreeId, ModItem, Struct, TypeAlias, + self, Fields, Function, Impl, ItemTree, ItemTreeId, LocalItemTreeId, ModItem, Struct, + TypeAlias, }, module_tree::LocalModuleId, name_resolution::ReachedFixedPoint, @@ -509,6 +507,10 @@ impl<'a> ModCollectorContext<'a, '_> { self.collect_import(id); continue; } + ModItem::Impl(id) => { + self.collect_impl(id); + continue; + } }; self.def_collector.package_defs.modules[self.module_id].add_definition(id); @@ -524,6 +526,20 @@ impl<'a> ModCollectorContext<'a, '_> { } } + /// Collects the definition data from an `Impl`. + fn collect_impl(&mut self, id: LocalItemTreeId) { + self.def_collector.package_defs.modules[self.module_id].define_impl( + ImplLoc { + module: ModuleId { + package: self.def_collector.package_id, + local_id: self.module_id, + }, + id: ItemTreeId::new(self.file_id, id), + } + .intern(self.def_collector.db), + ); + } + /// Collects the definition data from an import statement. fn collect_import(&mut self, id: LocalItemTreeId) { self.def_collector.unresolved_imports.push(ImportDirective { diff --git a/crates/mun_hir/src/utils.rs b/crates/mun_hir/src/utils.rs index f6069fd94..890168676 100644 --- a/crates/mun_hir/src/utils.rs +++ b/crates/mun_hir/src/utils.rs @@ -12,7 +12,8 @@ pub(crate) fn make_mut_slice(a: &mut Arc<[T]>) -> &mut [T] { #[cfg(test)] pub mod tests { use crate::{ - diagnostics::DiagnosticSink, mock::MockDatabase, with_fixture::WithFixture, Package, + diagnostics::DiagnosticSink, mock::MockDatabase, with_fixture::WithFixture, AstDatabase, + Package, }; pub fn diagnostics(content: &str) -> String { @@ -20,6 +21,15 @@ pub mod tests { let mut diags = Vec::new(); + for module in Package::all(&db).iter().flat_map(|pkg| pkg.modules(&db)) { + if let Some(file_id) = module.file_id(&db) { + let source_file = db.parse(file_id); + for err in source_file.errors() { + diags.push(format!("{:?}: {}", err.location(), err.to_string())); + } + } + } + let mut diag_sink = DiagnosticSink::new(|diag| { diags.push(format!("{:?}: {}", diag.highlight_range(), diag.message())); }); diff --git a/crates/mun_syntax/src/ast/generated.rs b/crates/mun_syntax/src/ast/generated.rs index a52ba0fbb..2b693ffb2 100644 --- a/crates/mun_syntax/src/ast/generated.rs +++ b/crates/mun_syntax/src/ast/generated.rs @@ -100,6 +100,79 @@ impl ArrayType { } } +// AssociatedItem + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct AssociatedItem { + pub(crate) syntax: SyntaxNode, +} + +impl AstNode for AssociatedItem { + fn can_cast(kind: SyntaxKind) -> bool { + matches!(kind, FUNCTION_DEF) + } + fn cast(syntax: SyntaxNode) -> Option { + if Self::can_cast(syntax.kind()) { + Some(AssociatedItem { syntax }) + } else { + None + } + } + fn syntax(&self) -> &SyntaxNode { + &self.syntax + } +} +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AssociatedItemKind { + FunctionDef(FunctionDef), +} +impl From for AssociatedItem { + fn from(n: FunctionDef) -> AssociatedItem { + AssociatedItem { syntax: n.syntax } + } +} + +impl AssociatedItem { + pub fn kind(&self) -> AssociatedItemKind { + match self.syntax.kind() { + FUNCTION_DEF => { + AssociatedItemKind::FunctionDef(FunctionDef::cast(self.syntax.clone()).unwrap()) + } + _ => unreachable!(), + } + } +} + +impl AssociatedItem {} + +// AssociatedItemList + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct AssociatedItemList { + pub(crate) syntax: SyntaxNode, +} + +impl AstNode for AssociatedItemList { + fn can_cast(kind: SyntaxKind) -> bool { + matches!(kind, ASSOCIATED_ITEM_LIST) + } + fn cast(syntax: SyntaxNode) -> Option { + if Self::can_cast(syntax.kind()) { + Some(AssociatedItemList { syntax }) + } else { + None + } + } + fn syntax(&self) -> &SyntaxNode { + &self.syntax + } +} +impl AssociatedItemList { + pub fn associated_items(&self) -> impl Iterator { + super::children(self) + } +} + // BinExpr #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -568,6 +641,40 @@ impl IfExpr { } } +// Impl + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Impl { + pub(crate) syntax: SyntaxNode, +} + +impl AstNode for Impl { + fn can_cast(kind: SyntaxKind) -> bool { + matches!(kind, IMPL) + } + fn cast(syntax: SyntaxNode) -> Option { + if Self::can_cast(syntax.kind()) { + Some(Impl { syntax }) + } else { + None + } + } + fn syntax(&self) -> &SyntaxNode { + &self.syntax + } +} +impl ast::VisibilityOwner for Impl {} +impl ast::DocCommentsOwner for Impl {} +impl Impl { + pub fn associated_item_list(&self) -> Option { + super::child_opt(self) + } + + pub fn type_ref(&self) -> Option { + super::child_opt(self) + } +} + // IndexExpr #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -707,7 +814,10 @@ pub struct ModuleItem { impl AstNode for ModuleItem { fn can_cast(kind: SyntaxKind) -> bool { - matches!(kind, USE | FUNCTION_DEF | STRUCT_DEF | TYPE_ALIAS_DEF) + matches!( + kind, + USE | FUNCTION_DEF | STRUCT_DEF | TYPE_ALIAS_DEF | IMPL + ) } fn cast(syntax: SyntaxNode) -> Option { if Self::can_cast(syntax.kind()) { @@ -726,6 +836,7 @@ pub enum ModuleItemKind { FunctionDef(FunctionDef), StructDef(StructDef), TypeAliasDef(TypeAliasDef), + Impl(Impl), } impl From for ModuleItem { fn from(n: Use) -> ModuleItem { @@ -747,6 +858,11 @@ impl From for ModuleItem { ModuleItem { syntax: n.syntax } } } +impl From for ModuleItem { + fn from(n: Impl) -> ModuleItem { + ModuleItem { syntax: n.syntax } + } +} impl ModuleItem { pub fn kind(&self) -> ModuleItemKind { @@ -759,6 +875,7 @@ impl ModuleItem { TYPE_ALIAS_DEF => { ModuleItemKind::TypeAliasDef(TypeAliasDef::cast(self.syntax.clone()).unwrap()) } + IMPL => ModuleItemKind::Impl(Impl::cast(self.syntax.clone()).unwrap()), _ => unreachable!(), } } diff --git a/crates/mun_syntax/src/grammar.ron b/crates/mun_syntax/src/grammar.ron index eb5dd13fd..b32cafaad 100644 --- a/crates/mun_syntax/src/grammar.ron +++ b/crates/mun_syntax/src/grammar.ron @@ -99,7 +99,9 @@ Grammar( "super", "self", - "extern" + "extern", + + "impl", ], literals: [ "INT_NUMBER", @@ -178,14 +180,18 @@ Grammar( "USE", "USE_TREE", "USE_TREE_LIST", - "RENAME" + "RENAME", + + "IMPL", + "ASSOCIATED_ITEM_LIST", + "ASSOCIATED_ITEM", ], ast: { "SourceFile": ( traits: [ "ModuleItemOwner", "FunctionDefOwner" ], ), "ModuleItem": ( - enum: ["Use", "FunctionDef", "StructDef", "TypeAliasDef"] + enum: ["Use", "FunctionDef", "StructDef", "TypeAliasDef", "Impl"] ), "Visibility": (), "FunctionDef": ( @@ -389,6 +395,17 @@ Grammar( "Rename": ( traits: ("NameOwner") - ) + ), + + "Impl": ( + options: ["AssociatedItemList", "TypeRef"], + traits: ["VisibilityOwner", "DocCommentsOwner"] + ), + "AssociatedItemList": ( + collections: [ ("associated_items", "AssociatedItem") ] + ), + "AssociatedItem": ( + enum: ["FunctionDef"] + ), } ) diff --git a/crates/mun_syntax/src/lib.rs b/crates/mun_syntax/src/lib.rs index 0b4eb35d9..113588093 100644 --- a/crates/mun_syntax/src/lib.rs +++ b/crates/mun_syntax/src/lib.rs @@ -22,6 +22,7 @@ mod token_text; #[cfg(test)] mod tests; pub mod utils; +mod validation; use std::{fmt::Write, marker::PhantomData, sync::Arc}; @@ -143,8 +144,9 @@ use ra_ap_text_edit::Indel; impl SourceFile { pub fn parse(text: &str) -> Parse { - let (green, errors) = parsing::parse_text(text); - //errors.extend(validation::validate(&SourceFile::new(green.clone()))); + let (green, mut errors) = parsing::parse_text(text); + let root = SyntaxNode::new_root(green.clone()); + errors.extend(validation::validate(&root)); Parse { green, errors: Arc::from(errors), @@ -208,7 +210,8 @@ fn api_walkthrough() { ast::ModuleItemKind::FunctionDef(f) => func = Some(f), ast::ModuleItemKind::StructDef(_) | ast::ModuleItemKind::TypeAliasDef(_) - | ast::ModuleItemKind::Use(_) => (), + | ast::ModuleItemKind::Use(_) + | ast::ModuleItemKind::Impl(_) => (), } } diff --git a/crates/mun_syntax/src/parsing/grammar.rs b/crates/mun_syntax/src/parsing/grammar.rs index 6667a8a78..191a21fea 100644 --- a/crates/mun_syntax/src/parsing/grammar.rs +++ b/crates/mun_syntax/src/parsing/grammar.rs @@ -4,6 +4,7 @@ mod expressions; mod params; mod paths; mod patterns; +mod traits; mod types; use super::{ diff --git a/crates/mun_syntax/src/parsing/grammar/declarations.rs b/crates/mun_syntax/src/parsing/grammar/declarations.rs index 8a084dbde..9302ccb55 100644 --- a/crates/mun_syntax/src/parsing/grammar/declarations.rs +++ b/crates/mun_syntax/src/parsing/grammar/declarations.rs @@ -1,20 +1,20 @@ use super::{ - adt, error_block, expressions, name, name_recovery, opt_visibility, params, paths, types, - Marker, Parser, TokenSet, EOF, ERROR, EXTERN, FUNCTION_DEF, RENAME, RET_TYPE, USE, USE_TREE, - USE_TREE_LIST, + adt, error_block, expressions, name, name_recovery, opt_visibility, params, paths, traits, + types, Marker, Parser, TokenSet, EOF, ERROR, EXTERN, FUNCTION_DEF, RENAME, RET_TYPE, USE, + USE_TREE, USE_TREE_LIST, }; use crate::{parsing::grammar::paths::is_use_path_start, T}; pub(super) const DECLARATION_RECOVERY_SET: TokenSet = - TokenSet::new(&[T![fn], T![pub], T![struct], T![use], T![;]]); + TokenSet::new(&[T![fn], T![pub], T![struct], T![use], T![;], T![impl]]); pub(super) fn mod_contents(p: &mut Parser<'_>) { while !p.at(EOF) { - declaration(p); + declaration(p, false); } } -pub(super) fn declaration(p: &mut Parser<'_>) { +pub(super) fn declaration(p: &mut Parser<'_>, stop_on_r_curly: bool) { let m = p.start(); let m = match maybe_declaration(p, m) { Ok(()) => return, @@ -22,17 +22,16 @@ pub(super) fn declaration(p: &mut Parser<'_>) { }; m.abandon(p); - if p.at(T!['{']) { - error_block(p, "expected a declaration"); - } else if p.at(T!['}']) { - let e = p.start(); - p.error("unmatched }"); - p.bump(T!['}']); - e.complete(p, ERROR); - } else if !p.at(EOF) { - p.error_and_bump("expected a declaration"); - } else { - p.error("expected a declaration"); + match p.current() { + T!['{'] => error_block(p, "expected a declaration"), + T!['}'] if !stop_on_r_curly => { + let e = p.start(); + p.error("unmatched }"); + p.bump(T!['}']); + e.complete(p, ERROR); + } + EOF | T!['}'] => p.error("expected a declaration"), + _ => p.error_and_bump("expected a declaration"), } } @@ -76,6 +75,9 @@ fn declarations_without_modifiers(p: &mut Parser<'_>, m: Marker) -> Result<(), M T![type] => { adt::type_alias_def(p, m); } + T![impl] => { + traits::impl_(p, m); + } _ => return Err(m), }; Ok(()) diff --git a/crates/mun_syntax/src/parsing/grammar/traits.rs b/crates/mun_syntax/src/parsing/grammar/traits.rs new file mode 100644 index 000000000..f289533ae --- /dev/null +++ b/crates/mun_syntax/src/parsing/grammar/traits.rs @@ -0,0 +1,31 @@ +use super::{declarations::declaration, error_block, types}; +use crate::{ + parsing::parser::{Marker, Parser}, + SyntaxKind::{ASSOCIATED_ITEM_LIST, EOF, IMPL}, +}; + +pub(super) fn impl_(p: &mut Parser<'_>, m: Marker) { + p.bump(T![impl]); + types::type_(p); + if p.at(T!['{']) { + associated_item_list(p); + } else { + p.error("expected `{`"); + } + m.complete(p, IMPL); +} + +fn associated_item_list(p: &mut Parser<'_>) { + assert!(p.at(T!['{'])); + let m = p.start(); + p.bump(T!['{']); + while !p.at(EOF) && !p.at(T!['}']) { + if p.at(T!['{']) { + error_block(p, "expected an associated item"); + continue; + } + declaration(p, true); + } + p.expect(T!['}']); + m.complete(p, ASSOCIATED_ITEM_LIST); +} diff --git a/crates/mun_syntax/src/syntax_error.rs b/crates/mun_syntax/src/syntax_error.rs index 081b72940..ba77974f7 100644 --- a/crates/mun_syntax/src/syntax_error.rs +++ b/crates/mun_syntax/src/syntax_error.rs @@ -58,6 +58,10 @@ impl SyntaxError { } } + pub fn parse_error>(msg: impl Into, loc: L) -> SyntaxError { + SyntaxError::new(SyntaxErrorKind::ParseError(ParseError(msg.into())), loc) + } + pub fn kind(&self) -> SyntaxErrorKind { self.kind.clone() } diff --git a/crates/mun_syntax/src/syntax_kind/generated.rs b/crates/mun_syntax/src/syntax_kind/generated.rs index 64663c7b2..6bda268a5 100644 --- a/crates/mun_syntax/src/syntax_kind/generated.rs +++ b/crates/mun_syntax/src/syntax_kind/generated.rs @@ -97,6 +97,7 @@ pub enum SyntaxKind { SUPER_KW, SELF_KW, EXTERN_KW, + IMPL_KW, INT_NUMBER, FLOAT_NUMBER, STRING, @@ -156,6 +157,9 @@ pub enum SyntaxKind { USE_TREE, USE_TREE_LIST, RENAME, + IMPL, + ASSOCIATED_ITEM_LIST, + ASSOCIATED_ITEM, // Technical kind so that we can cast from u16 safely #[doc(hidden)] __LAST, @@ -383,6 +387,9 @@ macro_rules! T { (extern) => { $crate::SyntaxKind::EXTERN_KW }; + (impl) => { + $crate::SyntaxKind::IMPL_KW + }; } impl From for SyntaxKind { @@ -428,6 +435,7 @@ impl SyntaxKind { | SUPER_KW | SELF_KW | EXTERN_KW + | IMPL_KW ) } @@ -569,6 +577,7 @@ impl SyntaxKind { SUPER_KW => &SyntaxInfo { name: "SUPER_KW" }, SELF_KW => &SyntaxInfo { name: "SELF_KW" }, EXTERN_KW => &SyntaxInfo { name: "EXTERN_KW" }, + IMPL_KW => &SyntaxInfo { name: "IMPL_KW" }, INT_NUMBER => &SyntaxInfo { name: "INT_NUMBER" }, FLOAT_NUMBER => &SyntaxInfo { name: "FLOAT_NUMBER" }, STRING => &SyntaxInfo { name: "STRING" }, @@ -628,6 +637,9 @@ impl SyntaxKind { USE_TREE => &SyntaxInfo { name: "USE_TREE" }, USE_TREE_LIST => &SyntaxInfo { name: "USE_TREE_LIST" }, RENAME => &SyntaxInfo { name: "RENAME" }, + IMPL => &SyntaxInfo { name: "IMPL" }, + ASSOCIATED_ITEM_LIST => &SyntaxInfo { name: "ASSOCIATED_ITEM_LIST" }, + ASSOCIATED_ITEM => &SyntaxInfo { name: "ASSOCIATED_ITEM" }, TOMBSTONE => &SyntaxInfo { name: "TOMBSTONE" }, EOF => &SyntaxInfo { name: "EOF" }, __LAST => &SyntaxInfo { name: "__LAST" }, @@ -662,6 +674,7 @@ impl SyntaxKind { "super" => SUPER_KW, "self" => SELF_KW, "extern" => EXTERN_KW, + "impl" => IMPL_KW, _ => return None, }; Some(kw) diff --git a/crates/mun_syntax/src/tests/lexer.rs b/crates/mun_syntax/src/tests/lexer.rs index 4697fa00e..4eb8c6e2f 100644 --- a/crates/mun_syntax/src/tests/lexer.rs +++ b/crates/mun_syntax/src/tests/lexer.rs @@ -302,7 +302,8 @@ fn keywords() { break do else false for fn if in nil return true while let mut struct class never loop pub super self package type - "#), @r#" + impl + "#), @r###" WHITESPACE 5 "\n " BREAK_KW 5 "break" WHITESPACE 1 " " @@ -350,7 +351,9 @@ fn keywords() { WHITESPACE 1 " " TYPE_KW 4 "type" WHITESPACE 5 "\n " - "#); + IMPL_KW 4 "impl" + WHITESPACE 5 "\n " + "###); } #[test] diff --git a/crates/mun_syntax/src/tests/parser.rs b/crates/mun_syntax/src/tests/parser.rs index 830a39a8c..5b789836b 100644 --- a/crates/mun_syntax/src/tests/parser.rs +++ b/crates/mun_syntax/src/tests/parser.rs @@ -1,5 +1,89 @@ use crate::SourceFile; +#[test] +fn impl_block() { + insta::assert_snapshot!(SourceFile::parse( + r#" + impl Foo {} + impl Bar { + fn bar() {} + struct Baz {} + } + pub impl FooBar {} + "#).debug_dump(), @r###" + SOURCE_FILE@0..135 + WHITESPACE@0..9 "\n " + IMPL@9..20 + IMPL_KW@9..13 "impl" + WHITESPACE@13..14 " " + PATH_TYPE@14..17 + PATH@14..17 + PATH_SEGMENT@14..17 + NAME_REF@14..17 + IDENT@14..17 "Foo" + WHITESPACE@17..18 " " + ASSOCIATED_ITEM_LIST@18..20 + L_CURLY@18..19 "{" + R_CURLY@19..20 "}" + WHITESPACE@20..29 "\n " + IMPL@29..99 + IMPL_KW@29..33 "impl" + WHITESPACE@33..34 " " + PATH_TYPE@34..37 + PATH@34..37 + PATH_SEGMENT@34..37 + NAME_REF@34..37 + IDENT@34..37 "Bar" + WHITESPACE@37..38 " " + ASSOCIATED_ITEM_LIST@38..99 + L_CURLY@38..39 "{" + FUNCTION_DEF@39..63 + WHITESPACE@39..52 "\n " + FN_KW@52..54 "fn" + WHITESPACE@54..55 " " + NAME@55..58 + IDENT@55..58 "bar" + PARAM_LIST@58..60 + L_PAREN@58..59 "(" + R_PAREN@59..60 ")" + WHITESPACE@60..61 " " + BLOCK_EXPR@61..63 + L_CURLY@61..62 "{" + R_CURLY@62..63 "}" + WHITESPACE@63..76 "\n " + STRUCT_DEF@76..89 + STRUCT_KW@76..82 "struct" + WHITESPACE@82..83 " " + NAME@83..86 + IDENT@83..86 "Baz" + WHITESPACE@86..87 " " + RECORD_FIELD_DEF_LIST@87..89 + L_CURLY@87..88 "{" + R_CURLY@88..89 "}" + WHITESPACE@89..98 "\n " + R_CURLY@98..99 "}" + WHITESPACE@99..108 "\n " + IMPL@108..126 + VISIBILITY@108..111 + PUB_KW@108..111 "pub" + WHITESPACE@111..112 " " + IMPL_KW@112..116 "impl" + WHITESPACE@116..117 " " + PATH_TYPE@117..123 + PATH@117..123 + PATH_SEGMENT@117..123 + NAME_REF@117..123 + IDENT@117..123 "FooBar" + WHITESPACE@123..124 " " + ASSOCIATED_ITEM_LIST@124..126 + L_CURLY@124..125 "{" + R_CURLY@125..126 "}" + WHITESPACE@126..135 "\n " + error Range(76..89): only functions are allowed in impl blocks + error Range(108..111): visibility is not allowed on impl blocks + "###); +} + #[test] fn array_type() { insta::assert_snapshot!(SourceFile::parse( @@ -3008,6 +3092,7 @@ fn type_alias_def() { WHITESPACE@40..45 "\n " "#); } + #[test] fn function_return_path() { insta::assert_snapshot!(SourceFile::parse( diff --git a/crates/mun_syntax/src/validation.rs b/crates/mun_syntax/src/validation.rs new file mode 100644 index 000000000..4c3282071 --- /dev/null +++ b/crates/mun_syntax/src/validation.rs @@ -0,0 +1,57 @@ +//! This module contains the validation pass for the AST. See the [`validate`] function for more +//! information. + +use crate::ast::VisibilityOwner; +use crate::{ast, ast::AstNode, match_ast, SyntaxError, SyntaxNode}; + +/// A validation pass that checks that the AST is valid. +/// +/// Even though the AST could be valid (aka without parse errors), it could still be semantically +/// incorrect. For example, a struct cannot be declared in an impl block. This pass checks for +/// these kinds of errors. +pub(crate) fn validate(root: &SyntaxNode) -> Vec { + let mut errors = Vec::new(); + for node in root.descendants() { + match_ast! { + match node { + ast::Impl(it) => validate_impl(it, &mut errors), + _ => (), + } + } + } + + errors +} + +/// Validates the semantic validity of an `impl` block. +fn validate_impl(node: ast::Impl, errors: &mut Vec) { + validate_impl_visibility(node.clone(), errors); + validate_impl_associated_items(node, errors); +} + +/// Validate that the visibility of an impl block is undefined. +fn validate_impl_visibility(node: ast::Impl, errors: &mut Vec) { + if let Some(vis) = node.visibility() { + errors.push(SyntaxError::parse_error( + "visibility is not allowed on impl blocks", + vis.syntax.text_range(), + )); + } +} + +/// Validate that only valid items are declared in an impl block. For example, a struct +/// cannot be declared in an impl block. +fn validate_impl_associated_items(node: ast::Impl, errors: &mut Vec) { + let Some(assoc_items) = node.associated_item_list() else { + return; + }; + + for item in assoc_items.syntax.children() { + match_ast! { + match item { + ast::FunctionDef(_it) => (), + _ => errors.push(SyntaxError::parse_error("only functions are allowed in impl blocks", item.text_range())), + } + } + } +} From 1cb7fb8f40e8fb608c967040cdf9ed9f52f4cd54 Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 29 Dec 2023 14:17:56 +0100 Subject: [PATCH 2/4] feat: extract all impls for a type --- crates/mun_hir/src/code_model.rs | 2 + crates/mun_hir/src/code_model/function.rs | 5 +- crates/mun_hir/src/code_model/impl.rs | 118 ++++++++++ crates/mun_hir/src/code_model/module.rs | 11 + crates/mun_hir/src/code_model/struct.rs | 5 +- crates/mun_hir/src/code_model/type_alias.rs | 5 +- crates/mun_hir/src/db.rs | 13 +- crates/mun_hir/src/diagnostics.rs | 38 +++ crates/mun_hir/src/has_module.rs | 49 ++++ crates/mun_hir/src/ids.rs | 33 ++- crates/mun_hir/src/item_scope.rs | 7 +- crates/mun_hir/src/lib.rs | 2 + crates/mun_hir/src/method_resolution.rs | 233 +++++++++++++++++++ crates/mun_hir/src/package_defs.rs | 1 + crates/mun_hir/src/package_defs/collector.rs | 6 +- crates/mun_hir/src/resolve.rs | 25 +- crates/mun_hir/src/ty/lower.rs | 7 + 17 files changed, 536 insertions(+), 24 deletions(-) create mode 100644 crates/mun_hir/src/code_model/impl.rs create mode 100644 crates/mun_hir/src/has_module.rs create mode 100644 crates/mun_hir/src/method_resolution.rs diff --git a/crates/mun_hir/src/code_model.rs b/crates/mun_hir/src/code_model.rs index 0e1927bba..81fd8afbd 100644 --- a/crates/mun_hir/src/code_model.rs +++ b/crates/mun_hir/src/code_model.rs @@ -1,4 +1,5 @@ mod function; +mod r#impl; mod module; mod package; pub(crate) mod src; @@ -12,6 +13,7 @@ pub use self::{ function::Function, module::{Module, ModuleDef}, package::Package, + r#impl::{Impl, ImplData}, r#struct::{Field, LocalFieldId, Struct, StructKind, StructMemoryKind}, src::HasSource, type_alias::TypeAlias, diff --git a/crates/mun_hir/src/code_model/function.rs b/crates/mun_hir/src/code_model/function.rs index c91729082..bae3938df 100644 --- a/crates/mun_hir/src/code_model/function.rs +++ b/crates/mun_hir/src/code_model/function.rs @@ -1,6 +1,7 @@ use super::Module; use crate::expr::validator::ExprValidator; use crate::expr::BodySourceMap; +use crate::has_module::HasModule; use crate::ids::{FunctionId, Lookup}; use crate::name_resolution::Namespace; use crate::resolve::HasResolver; @@ -99,9 +100,7 @@ impl FunctionData { impl Function { pub fn module(self, db: &dyn HirDatabase) -> Module { - Module { - id: self.id.lookup(db.upcast()).module, - } + self.id.module(db.upcast()).into() } /// Returns the full name of the function including all module specifiers (e.g: `foo::bar`). diff --git a/crates/mun_hir/src/code_model/impl.rs b/crates/mun_hir/src/code_model/impl.rs new file mode 100644 index 000000000..2c672b1ee --- /dev/null +++ b/crates/mun_hir/src/code_model/impl.rs @@ -0,0 +1,118 @@ +use crate::has_module::HasModule; +use crate::ids::{AssocItemId, FunctionLoc, ImplId, Intern, ItemContainerId, Lookup}; +use crate::item_tree::{AssociatedItem, ItemTreeId}; +use crate::type_ref::{LocalTypeRefId, TypeRefMap, TypeRefMapBuilder, TypeRefSourceMap}; +use crate::{DefDatabase, FileId, Function, HirDatabase, ItemLoc, Module, Package, Ty}; +use std::sync::Arc; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] +pub struct Impl { + pub(crate) id: ImplId, +} + +impl Impl { + /// Returns all the implementations defined in the specified `package`. + pub fn all_in_package(db: &dyn HirDatabase, package: Package) -> Vec { + let inherent_impls = db.inherent_impls_in_package(package.id); + inherent_impls.all_impls().map(Self::from).collect() + } + + /// The module in which the `impl` was defined. + /// + /// Note that this is not necessarily the module in which the self type was defined. `impl`s + /// can be defined in any module from where the self type is visibile. + pub fn module(self, db: &dyn HirDatabase) -> Module { + self.id.module(db.upcast()).into() + } + + /// Returns the file in which the implementation was defined + pub fn file_id(self, db: &dyn HirDatabase) -> FileId { + self.id.lookup(db.upcast()).id.file_id + } + + /// Returns the type for which this is an implementation + pub fn self_ty(self, db: &dyn HirDatabase) -> Ty { + let data = db.impl_data(self.id); + let lowered = db.lower_impl(self.id); + lowered[data.self_ty].clone() + } + + /// Returns all the items in the implementation + pub fn items(self, db: &dyn HirDatabase) -> Vec { + db.impl_data(self.id) + .items + .iter() + .copied() + .map(Into::into) + .collect() + } +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub enum AssocItem { + Function(Function), +} + +impl From for AssocItem { + fn from(value: AssocItemId) -> Self { + match value { + AssocItemId::FunctionId(it) => AssocItem::Function(it.into()), + } + } +} + +impl From for Impl { + fn from(value: ImplId) -> Self { + Impl { id: value } + } +} + +#[derive(Debug, PartialEq, Eq)] +pub struct ImplData { + pub items: Vec, + pub self_ty: LocalTypeRefId, + pub type_ref_map: TypeRefMap, + pub type_ref_source_map: TypeRefSourceMap, +} + +impl ImplData { + pub(crate) fn impl_data_query(db: &dyn DefDatabase, id: ImplId) -> Arc { + let ItemLoc { + module: _, + id: tree_id, + } = id.lookup(db); + + let item_tree = db.item_tree(tree_id.file_id); + let impl_def = &item_tree[tree_id.value]; + let src = item_tree.source(db, tree_id.value); + + // Associate the self type + let mut type_builder = TypeRefMapBuilder::default(); + let self_ty = type_builder.alloc_from_node_opt(src.type_ref().as_ref()); + let (type_ref_map, type_ref_source_map) = type_builder.finish(); + + // Add all the associated items + let container = ItemContainerId::ImplId(id); + let items = impl_def + .items + .iter() + .map(|it| match it { + AssociatedItem::Function(id) => { + let loc = FunctionLoc { + container, + id: ItemTreeId::new(tree_id.file_id, *id), + }; + let func_id = loc.intern(db); + AssocItemId::FunctionId(func_id) + } + }) + .collect(); + + Arc::new(ImplData { + items, + self_ty, + type_ref_map, + type_ref_source_map, + }) + } +} diff --git a/crates/mun_hir/src/code_model/module.rs b/crates/mun_hir/src/code_model/module.rs index 8e9378511..cf050063e 100644 --- a/crates/mun_hir/src/code_model/module.rs +++ b/crates/mun_hir/src/code_model/module.rs @@ -15,6 +15,13 @@ impl From for Module { } impl Module { + /// Returns the package associated with this module + pub fn package(self) -> Package { + Package { + id: self.id.package, + } + } + /// Returns the module that corresponds to the given file pub fn from_file(db: &dyn HirDatabase, file: FileId) -> Option { Package::all(db) @@ -71,6 +78,10 @@ impl Module { let package_defs = db.package_defs(self.id.package); package_defs.add_diagnostics(db.upcast(), self.id.local_id, sink); + // Add diagnostics from impls + let inherent_impls = db.inherent_impls_in_package(self.id.package); + inherent_impls.add_module_diagnostics(db, self.id.local_id, sink); + // Add diagnostics from the item tree if let Some(file_id) = self.file_id(db) { let item_tree = db.item_tree(file_id); diff --git a/crates/mun_hir/src/code_model/struct.rs b/crates/mun_hir/src/code_model/struct.rs index 5ced2047f..8919e02b2 100644 --- a/crates/mun_hir/src/code_model/struct.rs +++ b/crates/mun_hir/src/code_model/struct.rs @@ -14,6 +14,7 @@ use mun_syntax::{ }; use std::{fmt, sync::Arc}; +use crate::has_module::HasModule; use crate::resolve::HasResolver; use crate::visibility::RawVisibility; pub use ast::StructMemoryKind; @@ -66,9 +67,7 @@ impl Field { impl Struct { pub fn module(self, db: &dyn HirDatabase) -> Module { - Module { - id: self.id.lookup(db.upcast()).module, - } + self.id.module(db.upcast()).into() } pub fn file_id(self, db: &dyn HirDatabase) -> FileId { diff --git a/crates/mun_hir/src/code_model/type_alias.rs b/crates/mun_hir/src/code_model/type_alias.rs index 91ca9029d..e5aa4a4e9 100644 --- a/crates/mun_hir/src/code_model/type_alias.rs +++ b/crates/mun_hir/src/code_model/type_alias.rs @@ -10,6 +10,7 @@ use crate::{ use super::Module; use crate::expr::validator::TypeAliasValidator; +use crate::has_module::HasModule; use crate::resolve::HasResolver; use crate::ty::lower::LowerTyMap; @@ -26,9 +27,7 @@ impl From for TypeAlias { impl TypeAlias { pub fn module(self, db: &dyn HirDatabase) -> Module { - Module { - id: self.id.lookup(db.upcast()).module, - } + self.id.module(db.upcast()).into() } pub fn file_id(self, db: &dyn HirDatabase) -> FileId { diff --git a/crates/mun_hir/src/db.rs b/crates/mun_hir/src/db.rs index 05f2f7295..f758d69f0 100644 --- a/crates/mun_hir/src/db.rs +++ b/crates/mun_hir/src/db.rs @@ -1,9 +1,11 @@ #![allow(clippy::type_repetition_in_bounds)] +use crate::code_model::ImplData; use crate::expr::BodySourceMap; -use crate::ids::{DefWithBodyId, FunctionId}; +use crate::ids::{DefWithBodyId, FunctionId, ImplId}; use crate::input::{SourceRoot, SourceRootId}; use crate::item_tree::{self, ItemTree}; +use crate::method_resolution::InherentImpls; use crate::module_tree::ModuleTree; use crate::name_resolution::Namespace; use crate::package_defs::PackageDefs; @@ -115,6 +117,9 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast { #[salsa::invoke(ExprScopes::expr_scopes_query)] fn expr_scopes(&self, def: DefWithBodyId) -> Arc; + + #[salsa::invoke(ImplData::impl_data_query)] + fn impl_data(&self, def: ImplId) -> Arc; } #[salsa::query_group(HirDatabaseStorage)] @@ -139,8 +144,14 @@ pub trait HirDatabase: DefDatabase + Upcast { #[salsa::invoke(crate::ty::callable_item_sig)] fn callable_sig(&self, def: CallableDef) -> FnSig; + #[salsa::invoke(crate::ty::lower::lower_impl_query)] + fn lower_impl(&self, def: ImplId) -> Arc; + #[salsa::invoke(crate::ty::type_for_def)] fn type_for_def(&self, def: TypableDef, ns: Namespace) -> Ty; + + #[salsa::invoke(InherentImpls::inherent_impls_in_package_query)] + fn inherent_impls_in_package(&self, package: PackageId) -> Arc; } fn parse_query(db: &dyn AstDatabase, file_id: FileId) -> Parse { diff --git a/crates/mun_hir/src/diagnostics.rs b/crates/mun_hir/src/diagnostics.rs index b67aa5945..3668305d6 100644 --- a/crates/mun_hir/src/diagnostics.rs +++ b/crates/mun_hir/src/diagnostics.rs @@ -825,3 +825,41 @@ impl Diagnostic for PrivateTypeAlias { self } } + +#[derive(Debug)] +pub struct IncoherentImpl { + pub impl_: InFile>, +} + +impl Diagnostic for IncoherentImpl { + fn message(&self) -> String { + String::from("cannot define inherent `impl` for foreign type") + } + + fn source(&self) -> InFile { + self.impl_.clone().map(Into::into) + } + + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + +#[derive(Debug)] +pub struct InvalidSelfTyImpl { + pub impl_: InFile>, +} + +impl Diagnostic for InvalidSelfTyImpl { + fn message(&self) -> String { + String::from("inherent `impl` blocks can only be added for structs") + } + + fn source(&self) -> InFile { + self.impl_.clone().map(Into::into) + } + + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} diff --git a/crates/mun_hir/src/has_module.rs b/crates/mun_hir/src/has_module.rs new file mode 100644 index 000000000..378190264 --- /dev/null +++ b/crates/mun_hir/src/has_module.rs @@ -0,0 +1,49 @@ +use crate::{ + ids::{AssocItemLoc, FunctionId, ImplId, ItemContainerId, Lookup, StructId, TypeAliasId}, + item_tree::ItemTreeNode, + DefDatabase, ModuleId, +}; + +/// A trait to lookup the module associated with an item. +pub trait HasModule { + fn module(&self, db: &dyn DefDatabase) -> ModuleId; +} + +impl HasModule for ItemContainerId { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + match self { + ItemContainerId::ModuleId(it) => *it, + ItemContainerId::ImplId(it) => it.lookup(db).module, + } + } +} + +impl HasModule for AssocItemLoc { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + self.container.module(db) + } +} + +impl HasModule for StructId { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + self.lookup(db).module + } +} + +impl HasModule for FunctionId { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + self.lookup(db).container.module(db) + } +} + +impl HasModule for ImplId { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + self.lookup(db).module + } +} + +impl HasModule for TypeAliasId { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + self.lookup(db).module + } +} diff --git a/crates/mun_hir/src/ids.rs b/crates/mun_hir/src/ids.rs index 18b9f2be7..a5e8228ef 100644 --- a/crates/mun_hir/src/ids.rs +++ b/crates/mun_hir/src/ids.rs @@ -9,12 +9,13 @@ use std::hash::{Hash, Hasher}; #[derive(Clone, Debug)] pub struct ItemLoc { + pub module: ModuleId, pub id: ItemTreeId, } impl PartialEq for ItemLoc { fn eq(&self, other: &Self) -> bool { - self.id == other.id + self.id == other.id && self.module == other.module } } @@ -22,6 +23,7 @@ impl Eq for ItemLoc {} impl Hash for ItemLoc { fn hash(&self, hasher: &mut H) { + self.module.hash(hasher); self.id.hash(hasher); } } @@ -30,7 +32,7 @@ impl Copy for ItemLoc {} #[derive(Clone, Debug)] pub struct AssocItemLoc { - pub module: ModuleId, + pub container: ItemContainerId, pub id: ItemTreeId, } @@ -38,7 +40,7 @@ impl Copy for AssocItemLoc {} impl PartialEq for AssocItemLoc { fn eq(&self, other: &Self) -> bool { - self.module == other.module && self.id == other.id + self.container == other.container && self.id == other.id } } @@ -46,7 +48,7 @@ impl Eq for AssocItemLoc {} impl Hash for AssocItemLoc { fn hash(&self, state: &mut H) { - self.module.hash(state); + self.container.hash(state); self.id.hash(state); } } @@ -92,9 +94,20 @@ pub struct ModuleId { } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ItemContainerId { + ModuleId(ModuleId), + ImplId(ImplId), +} +impl From for ItemContainerId { + fn from(value: ModuleId) -> Self { + ItemContainerId::ModuleId(value) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] pub struct ImplId(salsa::InternId); -pub(crate) type ImplLoc = AssocItemLoc; +pub(crate) type ImplLoc = ItemLoc; impl_intern!(ImplId, ImplLoc, intern_impl, lookup_intern_impl); #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] @@ -111,13 +124,13 @@ impl_intern!( #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct StructId(salsa::InternId); -pub(crate) type StructLoc = AssocItemLoc; +pub(crate) type StructLoc = ItemLoc; impl_intern!(StructId, StructLoc, intern_struct, lookup_intern_struct); #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct TypeAliasId(salsa::InternId); -pub(crate) type TypeAliasLoc = AssocItemLoc; +pub(crate) type TypeAliasLoc = ItemLoc; impl_intern!( TypeAliasId, TypeAliasLoc, @@ -174,6 +187,12 @@ impl From for ItemDefinitionId { } } +/// Items that are associated with an `impl`. +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub enum AssocItemId { + FunctionId(FunctionId), +} + /// Definitions which have a body #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum DefWithBodyId { diff --git a/crates/mun_hir/src/item_scope.rs b/crates/mun_hir/src/item_scope.rs index 579bcf03c..8335d92be 100644 --- a/crates/mun_hir/src/item_scope.rs +++ b/crates/mun_hir/src/item_scope.rs @@ -80,10 +80,15 @@ impl ItemScope { } /// Returns an iterator over all declarations with this scope - pub fn declarations(&self) -> impl Iterator + '_ { + pub fn declarations(&self) -> impl Iterator + ExactSizeIterator + '_ { self.defs.iter().copied() } + /// Returns an iterator over all impls in this scope + pub fn impls(&self) -> impl Iterator + ExactSizeIterator + '_ { + self.impls.iter().copied() + } + /// Adds an item definition to the list of definitions pub(crate) fn add_definition(&mut self, def: ItemDefinitionId) { self.defs.push(def); diff --git a/crates/mun_hir/src/lib.rs b/crates/mun_hir/src/lib.rs index 083f8539f..32465d375 100644 --- a/crates/mun_hir/src/lib.rs +++ b/crates/mun_hir/src/lib.rs @@ -67,7 +67,9 @@ mod type_ref; mod utils; pub mod fixture; +mod has_module; mod item_scope; +mod method_resolution; #[cfg(test)] mod mock; mod package_defs; diff --git a/crates/mun_hir/src/method_resolution.rs b/crates/mun_hir/src/method_resolution.rs new file mode 100644 index 000000000..e4c5c6e89 --- /dev/null +++ b/crates/mun_hir/src/method_resolution.rs @@ -0,0 +1,233 @@ +use crate::diagnostics::{IncoherentImpl, InvalidSelfTyImpl}; +use crate::{ + db::HirDatabase, + has_module::HasModule, + ids::{ImplId, Lookup, StructId}, + module_tree::LocalModuleId, + package_defs::PackageDefs, + ty::lower::LowerDiagnostic, + DiagnosticSink, HasSource, PackageId, Ty, TyKind, +}; +use mun_syntax::AstPtr; +use rustc_hash::FxHashMap; +use std::sync::Arc; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum InherentImplsDiagnostics { + /// An error occurred when resolving a type in an impl. + LowerDiagnostic(ImplId, LowerDiagnostic), + + /// The type in the impl is not a valid type to implement. + InvalidSelfTy(ImplId), + + /// The type in the impl is not defined in the same package as the impl. + IncoherentType(ImplId), +} + +/// Holds inherit impls defined in some package. +/// +/// Inherent impls are impls that are defined for a type in the same package as the type itself. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct InherentImpls { + map: FxHashMap>, + diagnostics: Vec, +} + +impl InherentImpls { + /// A query function to extract all the inherent impls defined in a package. + pub(crate) fn inherent_impls_in_package_query( + db: &dyn HirDatabase, + package: PackageId, + ) -> Arc { + let mut impls = Self { + map: FxHashMap::default(), + diagnostics: Vec::new(), + }; + + let package_defs = db.package_defs(package); + impls.collect_from_package_defs(db, &package_defs); + impls.shrink_to_fit(); + + Arc::new(impls) + } + + /// A method to ensure that this instance only uses the amount of memory it needs. + /// + /// This effectively removes all extra allocated capacity from the `map` and `invalid_impls` fields. + fn shrink_to_fit(&mut self) { + self.map.values_mut().for_each(Vec::shrink_to_fit); + self.map.shrink_to_fit(); + self.diagnostics.shrink_to_fit(); + } + + /// Collects all the inherent impls defined in a package. + fn collect_from_package_defs(&mut self, db: &dyn HirDatabase, package_defs: &PackageDefs) { + for (_module_id, scope) in package_defs.modules.iter() { + for impl_id in scope.impls() { + let impl_data = db.impl_data(impl_id); + + // Resolve the self type of the impl + let lowered = db.lower_impl(impl_id); + self.diagnostics.extend( + lowered + .diagnostics + .iter() + .map(|d| InherentImplsDiagnostics::LowerDiagnostic(impl_id, d.clone())), + ); + + // Make sure the type is a struct + let self_ty = lowered[impl_data.self_ty].clone(); + let s = match self_ty.interned() { + TyKind::Struct(s) => s, + TyKind::Unknown => continue, + _ => { + self.diagnostics + .push(InherentImplsDiagnostics::InvalidSelfTy(impl_id)); + continue; + } + }; + + // Make sure the struct is defined in the same package + if s.module(db).package().id != package_defs.id { + self.diagnostics + .push(InherentImplsDiagnostics::IncoherentType(impl_id)); + } + + // Add the impl to the map + self.map.entry(s.id).or_default().push(impl_id); + } + } + } + + /// Adds all the `InherentImplsDiagnostics`s of the result of a specific module to the `DiagnosticSink`. + pub(crate) fn add_module_diagnostics( + &self, + db: &dyn HirDatabase, + module_id: LocalModuleId, + sink: &mut DiagnosticSink<'_>, + ) { + self.diagnostics + .iter() + .filter(|it| it.impl_id().module(db.upcast()).local_id == module_id) + .for_each(|it| it.add_to(db, sink)); + } + + /// Adds all the `InherentImplsDiagnostics`s of the result to the `DiagnosticSink`. + pub(crate) fn add_diagnostics(&self, db: &dyn HirDatabase, sink: &mut DiagnosticSink<'_>) { + self.diagnostics.iter().for_each(|it| it.add_to(db, sink)); + } + + /// Returns all implementations defined in this instance. + pub fn all_impls(&self) -> impl Iterator + '_ { + self.map.values().flatten().copied() + } + + // Returns all implementations defined for the specified type. + pub fn for_self_ty(&self, self_ty: &Ty) -> &[ImplId] { + match self_ty.interned() { + TyKind::Struct(s) => self.map.get(&s.id).map_or(&[], AsRef::as_ref), + _ => &[], + } + } +} + +impl InherentImplsDiagnostics { + fn add_to(&self, db: &dyn HirDatabase, sink: &mut DiagnosticSink<'_>) { + match self { + InherentImplsDiagnostics::LowerDiagnostic(impl_id, diag) => { + let impl_data = db.impl_data(*impl_id); + let file_id = impl_id.lookup(db.upcast()).id.file_id; + diag.add_to(db, file_id, &impl_data.type_ref_source_map, sink); + } + InherentImplsDiagnostics::InvalidSelfTy(impl_id) => sink.push(InvalidSelfTyImpl { + impl_: impl_id + .lookup(db.upcast()) + .source(db.upcast()) + .as_ref() + .map(AstPtr::new), + }), + InherentImplsDiagnostics::IncoherentType(impl_id) => sink.push(IncoherentImpl { + impl_: impl_id + .lookup(db.upcast()) + .source(db.upcast()) + .as_ref() + .map(AstPtr::new), + }), + } + } + + fn impl_id(&self) -> ImplId { + match self { + InherentImplsDiagnostics::LowerDiagnostic(impl_id, _) + | InherentImplsDiagnostics::InvalidSelfTy(impl_id) + | InherentImplsDiagnostics::IncoherentType(impl_id) => *impl_id, + } + } +} + +#[cfg(test)] +mod tests { + use crate::{ + mock::MockDatabase, with_fixture::WithFixture, DiagnosticSink, HirDatabase, SourceDatabase, + }; + + #[test] + fn test_query() { + let db = MockDatabase::with_files( + r#" + //- /main.mun + struct Foo; + impl Foo { + fn foo() {} + } + impl Foo {} + "#, + ); + + let package_id = db.packages().iter().next().unwrap(); + let impls = db.inherent_impls_in_package(package_id); + + assert_eq!(impls.diagnostics, Vec::new()); + assert_eq!(impls.map.values().flatten().count(), 2); + } + + fn impl_diagnostics(fixture: &str) -> String { + let db = MockDatabase::with_files(fixture); + + let package_id = db.packages().iter().next().unwrap(); + let impls = db.inherent_impls_in_package(package_id); + + let mut diags = Vec::new(); + let mut diag_sink = DiagnosticSink::new(|diag| { + diags.push(format!("{:?}: {}", diag.highlight_range(), diag.message())); + }); + + impls.add_diagnostics(&db, &mut diag_sink); + + drop(diag_sink); + diags.join("\n") + } + + #[test] + fn test_doesnt_exist() { + insta::assert_snapshot!(impl_diagnostics(r#" + //- /main.mun + impl DoesntExist {} + "#), + @"5..16: undefined type"); + } + + #[test] + fn test_invalid_self_ty() { + insta::assert_snapshot!(impl_diagnostics(r#" + //- /main.mun + struct Foo; + impl i32 {} + impl [Foo] {} + "#), + @r###" + 12..23: inherent `impl` blocks can only be added for structs + 24..37: inherent `impl` blocks can only be added for structs + "###); + } +} diff --git a/crates/mun_hir/src/package_defs.rs b/crates/mun_hir/src/package_defs.rs index 8ada57e70..a60f545a2 100644 --- a/crates/mun_hir/src/package_defs.rs +++ b/crates/mun_hir/src/package_defs.rs @@ -11,6 +11,7 @@ use std::{ops::Index, sync::Arc}; /// Contains all top-level definitions for a package. #[derive(Debug, PartialEq, Eq)] pub struct PackageDefs { + pub id: PackageId, pub modules: ArenaMap, pub module_tree: Arc, diagnostics: Vec, diff --git a/crates/mun_hir/src/package_defs/collector.rs b/crates/mun_hir/src/package_defs/collector.rs index 6142ae945..a3b9a4aed 100644 --- a/crates/mun_hir/src/package_defs/collector.rs +++ b/crates/mun_hir/src/package_defs/collector.rs @@ -1,4 +1,5 @@ use super::PackageDefs; +use crate::ids::ItemContainerId; use crate::{ arena::map::ArenaMap, ids::{FunctionLoc, ImplLoc, Intern, ItemDefinitionId, StructLoc, TypeAliasLoc}, @@ -101,6 +102,7 @@ pub(super) fn collect(db: &dyn DefDatabase, package_id: PackageId) -> PackageDef db, package_id, package_defs: PackageDefs { + id: package_id, modules: ArenaMap::default(), module_tree: db.module_tree(package_id), diagnostics: Vec::default(), @@ -555,10 +557,10 @@ impl<'a> ModCollectorContext<'a, '_> { let func = &self.item_tree[id]; DefData { id: FunctionLoc { - module: ModuleId { + container: ItemContainerId::ModuleId(ModuleId { package: self.def_collector.package_id, local_id: self.module_id, - }, + }), id: ItemTreeId::new(self.file_id, id), } .intern(self.def_collector.db) diff --git a/crates/mun_hir/src/resolve.rs b/crates/mun_hir/src/resolve.rs index 106c72e38..db3c879bc 100644 --- a/crates/mun_hir/src/resolve.rs +++ b/crates/mun_hir/src/resolve.rs @@ -1,5 +1,7 @@ +use crate::has_module::HasModule; use crate::ids::{ - DefWithBodyId, FunctionId, ItemDefinitionId, Lookup, ModuleId, StructId, TypeAliasId, + DefWithBodyId, FunctionId, ImplId, ItemContainerId, ItemDefinitionId, Lookup, ModuleId, + StructId, TypeAliasId, }; use crate::item_scope::BUILTIN_SCOPE; use crate::module_tree::LocalModuleId; @@ -359,19 +361,19 @@ impl HasResolver for ModuleId { impl HasResolver for FunctionId { fn resolver(self, db: &dyn DefDatabase) -> Resolver { - self.lookup(db).module.resolver(db) + self.lookup(db).container.resolver(db) } } impl HasResolver for StructId { fn resolver(self, db: &dyn DefDatabase) -> Resolver { - self.lookup(db).module.resolver(db) + self.module(db).resolver(db) } } impl HasResolver for TypeAliasId { fn resolver(self, db: &dyn DefDatabase) -> Resolver { - self.lookup(db).module.resolver(db) + self.module(db).resolver(db) } } @@ -382,3 +384,18 @@ impl HasResolver for DefWithBodyId { } } } + +impl HasResolver for ItemContainerId { + fn resolver(self, db: &dyn DefDatabase) -> Resolver { + match self { + ItemContainerId::ModuleId(it) => it.resolver(db), + ItemContainerId::ImplId(it) => it.resolver(db), + } + } +} + +impl HasResolver for ImplId { + fn resolver(self, db: &dyn DefDatabase) -> Resolver { + self.module(db).resolver(db) + } +} diff --git a/crates/mun_hir/src/ty/lower.rs b/crates/mun_hir/src/ty/lower.rs index 78c37b814..fd4f10bba 100644 --- a/crates/mun_hir/src/ty/lower.rs +++ b/crates/mun_hir/src/ty/lower.rs @@ -1,6 +1,7 @@ //! Methods for lower the HIR to types. pub(crate) use self::diagnostics::LowerDiagnostic; +use crate::ids::ImplId; use crate::resolve::{HasResolver, TypeNs}; use crate::ty::{Substitution, TyKind}; use crate::{ @@ -315,6 +316,12 @@ fn type_for_type_alias(_db: &dyn HirDatabase, def: TypeAlias) -> Ty { TyKind::TypeAlias(def).intern() } +pub(crate) fn lower_impl_query(db: &dyn HirDatabase, impl_id: ImplId) -> Arc { + let impl_data = db.impl_data(impl_id); + let resolver = impl_id.resolver(db.upcast()); + lower_types(db, &resolver, &impl_data.type_ref_map) +} + pub mod diagnostics { use crate::diagnostics::{PrivateAccess, UnresolvedType}; use crate::{ From d41163ab4a15c2f837a64029e75aa0a4fdfaf1ea Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Sat, 30 Dec 2023 19:45:39 +0100 Subject: [PATCH 3/4] feat: detect duplicate definitions --- .../src/hir/duplicate_definition_error.rs | 17 ++-- crates/mun_hir/src/diagnostics.rs | 7 +- crates/mun_hir/src/has_module.rs | 9 ++ crates/mun_hir/src/item_tree.rs | 45 ++++++---- crates/mun_hir/src/item_tree/lower.rs | 33 ++++--- crates/mun_hir/src/method_resolution.rs | 85 +++++++++++++++++-- crates/mun_hir/src/package_defs.rs | 15 ++-- crates/mun_hir/src/path.rs | 6 +- crates/mun_syntax/src/ast/extensions.rs | 21 +++++ 9 files changed, 176 insertions(+), 62 deletions(-) diff --git a/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs b/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs index 8ae63dcbd..8c8e90cf3 100644 --- a/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs +++ b/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs @@ -85,7 +85,10 @@ pub struct DuplicateDefinition<'db, 'diag, DB: mun_hir::HirDatabase> { impl<'db, 'diag, DB: mun_hir::HirDatabase> Diagnostic for DuplicateDefinition<'db, 'diag, DB> { fn range(&self) -> TextRange { - syntax_node_identifier_range(&self.diag.definition, &self.db.parse(self.diag.file)) + syntax_node_identifier_range( + &self.diag.definition.value, + &self.db.parse(self.diag.definition.file_id), + ) } fn title(&self) -> String { @@ -99,8 +102,8 @@ impl<'db, 'diag, DB: mun_hir::HirDatabase> Diagnostic for DuplicateDefinition<'d fn primary_annotation(&self) -> Option { Some(SourceAnnotation { range: syntax_node_signature_range( - &self.diag.definition, - &self.db.parse(self.diag.file), + &self.diag.definition.value, + &self.db.parse(self.diag.definition.file_id), ), message: format!("`{}` redefined here", self.diag.name), }) @@ -109,10 +112,10 @@ impl<'db, 'diag, DB: mun_hir::HirDatabase> Diagnostic for DuplicateDefinition<'d fn secondary_annotations(&self) -> Vec { vec![SecondaryAnnotation { range: InFile::new( - self.diag.file, + self.diag.first_definition.file_id, syntax_node_signature_range( - &self.diag.first_definition, - &self.db.parse(self.diag.file), + &self.diag.first_definition.value, + &self.db.parse(self.diag.first_definition.file_id), ), ), message: format!( @@ -135,7 +138,7 @@ impl<'db, 'diag, DB: mun_hir::HirDatabase> Diagnostic for DuplicateDefinition<'d impl<'db, 'diag, DB: mun_hir::HirDatabase> DuplicateDefinition<'db, 'diag, DB> { /// Returns either `type` or `value` definition on the type of definition. fn value_or_type_string(&self) -> &'static str { - if self.diag.definition.kind() == SyntaxKind::STRUCT_DEF { + if self.diag.definition.value.kind() == SyntaxKind::STRUCT_DEF { "type" } else { "value" diff --git a/crates/mun_hir/src/diagnostics.rs b/crates/mun_hir/src/diagnostics.rs index 3668305d6..39a9f90a7 100644 --- a/crates/mun_hir/src/diagnostics.rs +++ b/crates/mun_hir/src/diagnostics.rs @@ -359,10 +359,9 @@ impl Diagnostic for CannotApplyUnaryOp { #[derive(Debug)] pub struct DuplicateDefinition { - pub file: FileId, pub name: String, - pub first_definition: SyntaxNodePtr, - pub definition: SyntaxNodePtr, + pub first_definition: InFile, + pub definition: InFile, } impl Diagnostic for DuplicateDefinition { @@ -371,7 +370,7 @@ impl Diagnostic for DuplicateDefinition { } fn source(&self) -> InFile { - InFile::new(self.file, self.definition.clone()) + self.definition.clone() } fn as_any(&self) -> &(dyn Any + Send + 'static) { diff --git a/crates/mun_hir/src/has_module.rs b/crates/mun_hir/src/has_module.rs index 378190264..eedd53f34 100644 --- a/crates/mun_hir/src/has_module.rs +++ b/crates/mun_hir/src/has_module.rs @@ -1,3 +1,4 @@ +use crate::ids::AssocItemId; use crate::{ ids::{AssocItemLoc, FunctionId, ImplId, ItemContainerId, Lookup, StructId, TypeAliasId}, item_tree::ItemTreeNode, @@ -47,3 +48,11 @@ impl HasModule for TypeAliasId { self.lookup(db).module } } + +impl HasModule for AssocItemId { + fn module(&self, db: &dyn DefDatabase) -> ModuleId { + match self { + AssocItemId::FunctionId(it) => it.module(db), + } + } +} diff --git a/crates/mun_hir/src/item_tree.rs b/crates/mun_hir/src/item_tree.rs index 99075f1da..f149ea653 100644 --- a/crates/mun_hir/src/item_tree.rs +++ b/crates/mun_hir/src/item_tree.rs @@ -422,7 +422,7 @@ impl Eq for IdRange {} mod diagnostics { use super::{ItemTree, ModItem}; use crate::diagnostics::DuplicateDefinition; - use crate::{DefDatabase, DiagnosticSink, HirDatabase, Name}; + use crate::{DefDatabase, DiagnosticSink, HirDatabase, InFile, Name, Path}; use mun_syntax::{AstNode, SyntaxNodePtr}; #[derive(Clone, Debug, Eq, PartialEq)] @@ -445,21 +445,37 @@ mod diagnostics { db: &dyn DefDatabase, item_tree: &ItemTree, item: ModItem, - ) -> SyntaxNodePtr { + ) -> InFile { match item { - ModItem::Function(item) => { - SyntaxNodePtr::new(item_tree.source(db, item).syntax()) + ModItem::Function(item) => InFile::new( + item_tree.file_id, + SyntaxNodePtr::new(item_tree.source(db, item).syntax()), + ), + ModItem::Struct(item) => InFile::new( + item_tree.file_id, + SyntaxNodePtr::new(item_tree.source(db, item).syntax()), + ), + ModItem::TypeAlias(item) => InFile::new( + item_tree.file_id, + SyntaxNodePtr::new(item_tree.source(db, item).syntax()), + ), + ModItem::Import(it) => { + let import = &item_tree[it]; + let import_src = item_tree.source(db, it); + let mut use_item = None; + let mut index = 0; + Path::expand_use_item(&import_src, |_, tree, _, _| { + if index == import.index { + use_item = Some(tree.clone()); + } + index += 1; + }); + InFile::new( + item_tree.file_id, + SyntaxNodePtr::new(use_item.expect("cannot find use item").syntax()), + ) } - ModItem::Struct(item) => { - SyntaxNodePtr::new(item_tree.source(db, item).syntax()) - } - ModItem::TypeAlias(item) => { - SyntaxNodePtr::new(item_tree.source(db, item).syntax()) - } - ModItem::Import(item) => { - SyntaxNodePtr::new(item_tree.source(db, item).syntax()) - } - ModItem::Impl(item) => SyntaxNodePtr::new(item_tree.source(db, item).syntax()), + ModItem::Impl(_) => unreachable!("impls cannot be duplicated"), } } @@ -469,7 +485,6 @@ mod diagnostics { first, second, } => sink.push(DuplicateDefinition { - file: item_tree.file_id, name: name.to_string(), first_definition: ast_ptr_from_mod(db.upcast(), item_tree, *first), definition: ast_ptr_from_mod(db.upcast(), item_tree, *second), diff --git a/crates/mun_hir/src/item_tree/lower.rs b/crates/mun_hir/src/item_tree/lower.rs index 7ba4c328d..05a7d9052 100644 --- a/crates/mun_hir/src/item_tree/lower.rs +++ b/crates/mun_hir/src/item_tree/lower.rs @@ -11,7 +11,7 @@ use crate::{ source_id::AstIdMap, type_ref::{TypeRefMap, TypeRefMapBuilder}, visibility::RawVisibility, - DefDatabase, FileId, InFile, Name, Path, + DefDatabase, FileId, Name, Path, }; use mun_syntax::ast::{ self, ExternOwner, ModuleItemOwner, NameOwner, StructKind, TypeAscriptionOwner, @@ -122,23 +122,20 @@ impl Context { // Every use item can expand to many `Import`s. let mut imports = Vec::new(); let tree = &mut self.data; - Path::expand_use_item( - InFile::new(self.file, use_item.clone()), - |path, _use_tree, is_glob, alias| { - imports.push( - tree.imports - .alloc(Import { - path, - alias, - visibility, - is_glob, - ast_id, - index: imports.len(), - }) - .into(), - ); - }, - ); + Path::expand_use_item(use_item, |path, _use_tree, is_glob, alias| { + imports.push( + tree.imports + .alloc(Import { + path, + alias, + visibility, + is_glob, + ast_id, + index: imports.len(), + }) + .into(), + ); + }); imports } diff --git a/crates/mun_hir/src/method_resolution.rs b/crates/mun_hir/src/method_resolution.rs index e4c5c6e89..2d23ef474 100644 --- a/crates/mun_hir/src/method_resolution.rs +++ b/crates/mun_hir/src/method_resolution.rs @@ -1,4 +1,5 @@ -use crate::diagnostics::{IncoherentImpl, InvalidSelfTyImpl}; +use crate::diagnostics::{DuplicateDefinition, IncoherentImpl, InvalidSelfTyImpl}; +use crate::ids::AssocItemId; use crate::{ db::HirDatabase, has_module::HasModule, @@ -6,10 +7,12 @@ use crate::{ module_tree::LocalModuleId, package_defs::PackageDefs, ty::lower::LowerDiagnostic, - DiagnosticSink, HasSource, PackageId, Ty, TyKind, + DefDatabase, DiagnosticSink, HasSource, InFile, ModuleId, PackageId, Ty, TyKind, }; -use mun_syntax::AstPtr; +use mun_syntax::{AstNode, AstPtr, SyntaxNodePtr}; use rustc_hash::FxHashMap; +use std::collections::hash_map::Entry; +use std::collections::HashMap; use std::sync::Arc; #[derive(Clone, Debug, PartialEq, Eq)] @@ -22,6 +25,9 @@ pub enum InherentImplsDiagnostics { /// The type in the impl is not defined in the same package as the impl. IncoherentType(ImplId), + + /// Duplicate definitions of an associated item + DuplicateDefinitions(AssocItemId, AssocItemId), } /// Holds inherit impls defined in some package. @@ -97,6 +103,31 @@ impl InherentImpls { self.map.entry(s.id).or_default().push(impl_id); } } + + // Find duplicate associated items + for (_, impls) in self.map.iter() { + let mut name_to_item = HashMap::new(); + for impl_id in impls.iter() { + let impl_data = db.impl_data(*impl_id); + for item in impl_data.items.iter() { + let name = match item { + AssocItemId::FunctionId(it) => db.fn_data(*it).name().clone(), + }; + match name_to_item.entry(name) { + Entry::Vacant(entry) => { + entry.insert(*item); + } + Entry::Occupied(entry) => { + self.diagnostics + .push(InherentImplsDiagnostics::DuplicateDefinitions( + *entry.get(), + *item, + )); + } + } + } + } + } } /// Adds all the `InherentImplsDiagnostics`s of the result of a specific module to the `DiagnosticSink`. @@ -108,7 +139,7 @@ impl InherentImpls { ) { self.diagnostics .iter() - .filter(|it| it.impl_id().module(db.upcast()).local_id == module_id) + .filter(|it| it.module_id(db.upcast()).local_id == module_id) .for_each(|it| it.add_to(db, sink)); } @@ -153,18 +184,41 @@ impl InherentImplsDiagnostics { .as_ref() .map(AstPtr::new), }), + InherentImplsDiagnostics::DuplicateDefinitions(first, second) => { + sink.push(DuplicateDefinition { + definition: assoc_item_syntax_node_ptr(db.upcast(), second), + first_definition: assoc_item_syntax_node_ptr(db.upcast(), first), + name: assoc_item_name(db.upcast(), first), + }); + } } } - fn impl_id(&self) -> ImplId { + fn module_id(&self, db: &dyn DefDatabase) -> ModuleId { match self { InherentImplsDiagnostics::LowerDiagnostic(impl_id, _) | InherentImplsDiagnostics::InvalidSelfTy(impl_id) - | InherentImplsDiagnostics::IncoherentType(impl_id) => *impl_id, + | InherentImplsDiagnostics::IncoherentType(impl_id) => impl_id.module(db), + InherentImplsDiagnostics::DuplicateDefinitions(_first, second) => second.module(db), } } } +fn assoc_item_syntax_node_ptr(db: &dyn DefDatabase, id: &AssocItemId) -> InFile { + match id { + AssocItemId::FunctionId(it) => it + .lookup(db) + .source(db) + .map(|node| SyntaxNodePtr::new(node.syntax())), + } +} + +fn assoc_item_name(db: &dyn DefDatabase, id: &AssocItemId) -> String { + match id { + AssocItemId::FunctionId(it) => db.fn_data(*it).name().to_string(), + } +} + #[cfg(test)] mod tests { use crate::{ @@ -230,4 +284,23 @@ mod tests { 24..37: inherent `impl` blocks can only be added for structs "###); } + + #[test] + fn test_duplicate() { + insta::assert_snapshot!(impl_diagnostics(r#" + //- /main.mun + struct Foo; + impl Foo { + fn bar(); + fn bar(); + } + impl Foo { + fn bar(); + } + "#), + @r###" + 36..50: the name `bar` is defined multiple times + 63..77: the name `bar` is defined multiple times + "###); + } } diff --git a/crates/mun_hir/src/package_defs.rs b/crates/mun_hir/src/package_defs.rs index a60f545a2..462e3ed04 100644 --- a/crates/mun_hir/src/package_defs.rs +++ b/crates/mun_hir/src/package_defs.rs @@ -112,15 +112,12 @@ mod diagnostics { let use_item = ast.to_node(db); let mut cur = 0; let mut tree = None; - Path::expand_use_item( - InFile::new(ast.file_id, use_item), - |_path, use_tree, _is_glob, _alias| { - if cur == index { - tree = Some(use_tree.clone()); - } - cur += 1; - }, - ); + Path::expand_use_item(&use_item, |_path, use_tree, _is_glob, _alias| { + if cur == index { + tree = Some(use_tree.clone()); + } + cur += 1; + }); tree.map(|t| InFile::new(ast.file_id, AstPtr::new(&t))) } diff --git a/crates/mun_hir/src/path.rs b/crates/mun_hir/src/path.rs index e2541fb25..e8cfe7b3c 100644 --- a/crates/mun_hir/src/path.rs +++ b/crates/mun_hir/src/path.rs @@ -1,4 +1,4 @@ -use crate::{AsName, InFile, Name}; +use crate::{AsName, Name}; use mun_syntax::ast; use mun_syntax::ast::{NameOwner, PathSegmentKind}; @@ -90,10 +90,10 @@ impl Path { /// ``` /// the function will call the callback twice. Once for `foo` and once for `foo::Bar`. pub(crate) fn expand_use_item( - item_src: InFile, + item_src: &ast::Use, mut cb: impl FnMut(Path, &ast::UseTree, /* is_glob */ bool, Option), ) { - if let Some(tree) = item_src.value.use_tree() { + if let Some(tree) = item_src.use_tree() { lower_use_tree(None, &tree, &mut cb); } } diff --git a/crates/mun_syntax/src/ast/extensions.rs b/crates/mun_syntax/src/ast/extensions.rs index efe69f793..07483bd03 100644 --- a/crates/mun_syntax/src/ast/extensions.rs +++ b/crates/mun_syntax/src/ast/extensions.rs @@ -210,3 +210,24 @@ impl ast::UseTree { .any(|it| it.kind() == T![*]) } } + +impl ast::TypeAliasDef { + pub fn signature_range(&self) -> TextRange { + let struct_kw = self + .syntax() + .children_with_tokens() + .find(|p| p.kind() == T![type]) + .map(|kw| kw.text_range()); + let name = self.name().map(|n| n.syntax.text_range()); + + let start = + struct_kw.map_or_else(|| self.syntax.text_range().start(), rowan::TextRange::start); + + let end = name + .map(rowan::TextRange::end) + .or_else(|| struct_kw.map(rowan::TextRange::end)) + .unwrap_or_else(|| self.syntax().text_range().end()); + + TextRange::new(start, end) + } +} From 9290fd5d1282f718747f6f0023e030b835de3a5b Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Mon, 1 Jan 2024 14:38:23 +0100 Subject: [PATCH 4/4] fix: increase code coverage --- .../src/hir/duplicate_definition_error.rs | 16 +++++++--- crates/mun_hir/src/diagnostics.rs | 4 +-- crates/mun_hir/src/ids.rs | 1 + crates/mun_hir/src/item_tree/lower.rs | 18 +++++++---- ...r__item_tree__tests__duplicate_import.snap | 11 +++++++ ...ir__item_tree__tests__top_level_items.snap | 2 ++ crates/mun_hir/src/item_tree/tests.rs | 32 +++++++++++++++++-- crates/mun_hir/src/method_resolution.rs | 24 +++++++------- crates/mun_hir/src/package_defs/tests.rs | 1 + crates/mun_hir/src/path.rs | 14 ++++++++ crates/mun_syntax/src/ast/extensions.rs | 29 +++++++++++++++-- 11 files changed, 123 insertions(+), 29 deletions(-) create mode 100644 crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__duplicate_import.snap diff --git a/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs b/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs index 8c8e90cf3..9d240033b 100644 --- a/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs +++ b/crates/mun_diagnostics/src/hir/duplicate_definition_error.rs @@ -32,6 +32,10 @@ fn syntax_node_signature_range( ast::StructDef::cast(syntax_node_ptr.to_node(parse.tree().syntax())) .map_or_else(|| syntax_node_ptr.range(), |s| s.signature_range()) } + SyntaxKind::TYPE_ALIAS_DEF => { + ast::TypeAliasDef::cast(syntax_node_ptr.to_node(parse.tree().syntax())) + .map_or_else(|| syntax_node_ptr.range(), |s| s.signature_range()) + } _ => syntax_node_ptr.range(), } } @@ -58,11 +62,13 @@ fn syntax_node_identifier_range( parse: &Parse, ) -> TextRange { match syntax_node_ptr.kind() { - SyntaxKind::FUNCTION_DEF | SyntaxKind::STRUCT_DEF => syntax_node_ptr - .to_node(parse.tree().syntax()) - .children() - .find(|n| n.kind() == SyntaxKind::NAME) - .map_or_else(|| syntax_node_ptr.range(), |name| name.text_range()), + SyntaxKind::FUNCTION_DEF | SyntaxKind::STRUCT_DEF | SyntaxKind::TYPE_ALIAS_DEF => { + syntax_node_ptr + .to_node(parse.tree().syntax()) + .children() + .find(|n| n.kind() == SyntaxKind::NAME) + .map_or_else(|| syntax_node_ptr.range(), |name| name.text_range()) + } _ => syntax_node_ptr.range(), } } diff --git a/crates/mun_hir/src/diagnostics.rs b/crates/mun_hir/src/diagnostics.rs index 39a9f90a7..b4b930714 100644 --- a/crates/mun_hir/src/diagnostics.rs +++ b/crates/mun_hir/src/diagnostics.rs @@ -826,11 +826,11 @@ impl Diagnostic for PrivateTypeAlias { } #[derive(Debug)] -pub struct IncoherentImpl { +pub struct ImplForForeignType { pub impl_: InFile>, } -impl Diagnostic for IncoherentImpl { +impl Diagnostic for ImplForForeignType { fn message(&self) -> String { String::from("cannot define inherent `impl` for foreign type") } diff --git a/crates/mun_hir/src/ids.rs b/crates/mun_hir/src/ids.rs index a5e8228ef..0c8e16b17 100644 --- a/crates/mun_hir/src/ids.rs +++ b/crates/mun_hir/src/ids.rs @@ -93,6 +93,7 @@ pub struct ModuleId { pub local_id: LocalModuleId, } +/// Represents an id of an item inside a item container such as a module or a `impl` block. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum ItemContainerId { ModuleId(ModuleId), diff --git a/crates/mun_hir/src/item_tree/lower.rs b/crates/mun_hir/src/item_tree/lower.rs index 05a7d9052..1689ff703 100644 --- a/crates/mun_hir/src/item_tree/lower.rs +++ b/crates/mun_hir/src/item_tree/lower.rs @@ -21,11 +21,6 @@ use std::{collections::HashMap, convert::TryInto, marker::PhantomData, sync::Arc struct ModItems(SmallVec<[ModItem; 1]>); -struct Foo {} -impl Foo { - fn bar() {} -} - impl From for ModItems where T: Into, @@ -77,7 +72,18 @@ impl Context { ModItem::Function(item) => Some(&self.data.functions[item.index].name), ModItem::Struct(item) => Some(&self.data.structs[item.index].name), ModItem::TypeAlias(item) => Some(&self.data.type_aliases[item.index].name), - ModItem::Impl(_) | ModItem::Import(_) => None, + ModItem::Import(item) => { + let import = &self.data.imports[item.index]; + if import.is_glob { + None + } else { + import + .alias + .as_ref() + .map_or_else(|| import.path.last_segment(), |alias| alias.as_name()) + } + } + ModItem::Impl(_) => None, }; if let Some(name) = name { if let Some(first_item) = set.get(name) { diff --git a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__duplicate_import.snap b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__duplicate_import.snap new file mode 100644 index 000000000..0de4a8e11 --- /dev/null +++ b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__duplicate_import.snap @@ -0,0 +1,11 @@ +--- +source: crates/mun_hir/src/item_tree/tests.rs +expression: "print_item_tree(r#\"\n use foo::Bar;\n use baz::Bar;\n\n struct Bar {}\n \"#).unwrap()" +--- +use foo::Bar; +use baz::Bar; +struct Bar { +} + +18..26: the name `Bar` is defined multiple times +29..42: the name `Bar` is defined multiple times diff --git a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap index 3d1f48ee1..c604270ba 100644 --- a/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap +++ b/crates/mun_hir/src/item_tree/snapshots/mun_hir__item_tree__tests__top_level_items.snap @@ -44,3 +44,5 @@ struct Baz; type FooBar = Foo; type FooBar = package::Foo; +80..128: the name `bar` is defined multiple times +379..406: the name `FooBar` is defined multiple times diff --git a/crates/mun_hir/src/item_tree/tests.rs b/crates/mun_hir/src/item_tree/tests.rs index 7ca036822..a87c79d39 100644 --- a/crates/mun_hir/src/item_tree/tests.rs +++ b/crates/mun_hir/src/item_tree/tests.rs @@ -1,10 +1,25 @@ -use crate::{mock::MockDatabase, with_fixture::WithFixture, DefDatabase, Upcast}; +use crate::{mock::MockDatabase, with_fixture::WithFixture, DefDatabase, DiagnosticSink, Upcast}; use std::fmt; fn print_item_tree(text: &str) -> Result { let (db, file_id) = MockDatabase::with_single_file(text); let item_tree = db.item_tree(file_id); - super::pretty::print_item_tree(db.upcast(), &item_tree) + let mut result_str = super::pretty::print_item_tree(db.upcast(), &item_tree)?; + let mut sink = DiagnosticSink::new(|diag| { + result_str.push_str(&format!( + "\n{:?}: {}", + diag.highlight_range(), + diag.message() + )) + }); + + item_tree + .diagnostics + .iter() + .for_each(|diag| diag.add_to(&db, &item_tree, &mut sink)); + + drop(sink); + Ok(result_str) } #[test] @@ -62,3 +77,16 @@ fn test_impls() { ) .unwrap()); } + +#[test] +fn test_duplicate_import() { + insta::assert_snapshot!(print_item_tree( + r#" + use foo::Bar; + use baz::Bar; + + struct Bar {} + "# + ) + .unwrap()); +} diff --git a/crates/mun_hir/src/method_resolution.rs b/crates/mun_hir/src/method_resolution.rs index 2d23ef474..cc44d0c51 100644 --- a/crates/mun_hir/src/method_resolution.rs +++ b/crates/mun_hir/src/method_resolution.rs @@ -1,4 +1,4 @@ -use crate::diagnostics::{DuplicateDefinition, IncoherentImpl, InvalidSelfTyImpl}; +use crate::diagnostics::{DuplicateDefinition, ImplForForeignType, InvalidSelfTyImpl}; use crate::ids::AssocItemId; use crate::{ db::HirDatabase, @@ -24,7 +24,7 @@ pub enum InherentImplsDiagnostics { InvalidSelfTy(ImplId), /// The type in the impl is not defined in the same package as the impl. - IncoherentType(ImplId), + ImplForForeignType(ImplId), /// Duplicate definitions of an associated item DuplicateDefinitions(AssocItemId, AssocItemId), @@ -96,7 +96,7 @@ impl InherentImpls { // Make sure the struct is defined in the same package if s.module(db).package().id != package_defs.id { self.diagnostics - .push(InherentImplsDiagnostics::IncoherentType(impl_id)); + .push(InherentImplsDiagnostics::ImplForForeignType(impl_id)); } // Add the impl to the map @@ -177,13 +177,15 @@ impl InherentImplsDiagnostics { .as_ref() .map(AstPtr::new), }), - InherentImplsDiagnostics::IncoherentType(impl_id) => sink.push(IncoherentImpl { - impl_: impl_id - .lookup(db.upcast()) - .source(db.upcast()) - .as_ref() - .map(AstPtr::new), - }), + InherentImplsDiagnostics::ImplForForeignType(impl_id) => { + sink.push(ImplForForeignType { + impl_: impl_id + .lookup(db.upcast()) + .source(db.upcast()) + .as_ref() + .map(AstPtr::new), + }); + } InherentImplsDiagnostics::DuplicateDefinitions(first, second) => { sink.push(DuplicateDefinition { definition: assoc_item_syntax_node_ptr(db.upcast(), second), @@ -198,7 +200,7 @@ impl InherentImplsDiagnostics { match self { InherentImplsDiagnostics::LowerDiagnostic(impl_id, _) | InherentImplsDiagnostics::InvalidSelfTy(impl_id) - | InherentImplsDiagnostics::IncoherentType(impl_id) => impl_id.module(db), + | InherentImplsDiagnostics::ImplForForeignType(impl_id) => impl_id.module(db), InherentImplsDiagnostics::DuplicateDefinitions(_first, second) => second.module(db), } } diff --git a/crates/mun_hir/src/package_defs/tests.rs b/crates/mun_hir/src/package_defs/tests.rs index 5313ecf99..9d3d5849b 100644 --- a/crates/mun_hir/src/package_defs/tests.rs +++ b/crates/mun_hir/src/package_defs/tests.rs @@ -48,6 +48,7 @@ fn use_duplicate_name() { mod mod +-- mod bar | +-- ERROR: 4..20: a second item with the same name imported. Try to use an alias. + | +-- ERROR: 23..37: the name `Ok` is defined multiple times | '-- struct Ok '-- mod foo '-- struct Ok diff --git a/crates/mun_hir/src/path.rs b/crates/mun_hir/src/path.rs index e8cfe7b3c..5ae4b4e2b 100644 --- a/crates/mun_hir/src/path.rs +++ b/crates/mun_hir/src/path.rs @@ -25,6 +25,15 @@ pub enum ImportAlias { Alias(Name), } +impl ImportAlias { + pub fn as_name(&self) -> Option<&Name> { + match self { + ImportAlias::Underscore => None, + ImportAlias::Alias(name) => Some(name), + } + } +} + impl Path { /// Converts an `ast::Path` to `Path`. pub fn from_ast(mut path: ast::Path) -> Option { @@ -97,6 +106,11 @@ impl Path { lower_use_tree(None, &tree, &mut cb); } } + + /// Returns the last segment of the path, if any. + pub fn last_segment(&self) -> Option<&Name> { + self.segments.last() + } } /// Given an `ast::UseTree` and an optional prefix, call a callback function for every item that is diff --git a/crates/mun_syntax/src/ast/extensions.rs b/crates/mun_syntax/src/ast/extensions.rs index 07483bd03..5dc9e6b68 100644 --- a/crates/mun_syntax/src/ast/extensions.rs +++ b/crates/mun_syntax/src/ast/extensions.rs @@ -30,6 +30,14 @@ impl ast::NameRef { } impl ast::FunctionDef { + /// Returns the signature range. + /// + /// ```rust, ignore + /// fn foo_bar() { + /// ^^^^^^^^^^^^___ this part + /// // ... + /// } + /// ``` pub fn signature_range(&self) -> TextRange { let fn_kw = self .syntax() @@ -153,6 +161,15 @@ impl ast::StructDef { pub fn kind(&self) -> StructKind { StructKind::from_node(self) } + + /// Returns the signature range. + /// + /// ```rust, ignore + /// pub(gc) struct Foo { + /// ^^^^^^^^^^___ this part + /// // ... + /// } + /// ``` pub fn signature_range(&self) -> TextRange { let struct_kw = self .syntax() @@ -212,8 +229,14 @@ impl ast::UseTree { } impl ast::TypeAliasDef { + /// Returns the signature range. + /// + /// ```rust, ignore + /// type FooBar = i32 + /// ^^^^^^^^^^^___ this part + /// ``` pub fn signature_range(&self) -> TextRange { - let struct_kw = self + let type_kw = self .syntax() .children_with_tokens() .find(|p| p.kind() == T![type]) @@ -221,11 +244,11 @@ impl ast::TypeAliasDef { let name = self.name().map(|n| n.syntax.text_range()); let start = - struct_kw.map_or_else(|| self.syntax.text_range().start(), rowan::TextRange::start); + type_kw.map_or_else(|| self.syntax.text_range().start(), rowan::TextRange::start); let end = name .map(rowan::TextRange::end) - .or_else(|| struct_kw.map(rowan::TextRange::end)) + .or_else(|| type_kw.map(rowan::TextRange::end)) .unwrap_or_else(|| self.syntax().text_range().end()); TextRange::new(start, end)