From 1bb59a7d08d10d247da265ef58835f9979ec36b7 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Tue, 3 Dec 2019 14:58:29 -0500 Subject: Three-state enum for module origin --- crates/ra_hir_def/src/nameres.rs | 91 ++++++++++++++++++++++++------ crates/ra_hir_def/src/nameres/collector.rs | 7 +-- 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index faf3566f4..cf2a83191 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -100,19 +100,83 @@ impl std::ops::Index for CrateDefMap { } } +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] +pub enum ModuleOrigin { + /// It should not be `None` after collecting definitions. + Root(Option), + /// Note that non-inline modules, by definition, live inside non-macro file. + File(AstId, FileId), + Inline(AstId), + Block(AstId), +} + +impl Default for ModuleOrigin { + fn default() -> Self { + ModuleOrigin::Root(None) + } +} + +impl ModuleOrigin { + pub fn root(file_id: FileId) -> Self { + ModuleOrigin::Root(Some(file_id)) + } + + pub fn not_sure_file(file: Option, module: AstId) -> Self { + match file { + None => ModuleOrigin::Inline(module), + Some(file) => ModuleOrigin::File(module, file), + } + } + + pub fn not_sure_mod(file: FileId, module: Option>) -> Self { + match module { + None => ModuleOrigin::root(file), + Some(module) => ModuleOrigin::File(module, file), + } + } + + pub fn declaration(&self) -> Option> { + match self { + ModuleOrigin::File(m, _) | ModuleOrigin::Inline(m) => Some(*m), + ModuleOrigin::Root(_) | ModuleOrigin::Block(_) => None, + } + } + + pub fn file_id(&self) -> Option { + match self { + ModuleOrigin::File(_, file_id) | ModuleOrigin::Root(Some(file_id)) => Some(*file_id), + _ => None, + } + } + + /// Returns a node which defines this module. + /// That is, a file or a `mod foo {}` with items. + pub fn definition_source( + &self, + db: &impl DefDatabase, + ) -> InFile> { + match self { + ModuleOrigin::File(_, file_id) | ModuleOrigin::Root(Some(file_id)) => { + let file_id = *file_id; + let sf = db.parse(file_id).tree(); + return InFile::new(file_id.into(), Either::Left(sf)); + } + ModuleOrigin::Root(None) => unreachable!(), + ModuleOrigin::Inline(m) => InFile::new(m.file_id, Either::Right(m.to_node(db))), + // FIXME: right now it's never constructed, so it's fine to omit + ModuleOrigin::Block(b) => unimplemented!(), + } + } +} + #[derive(Default, Debug, PartialEq, Eq)] pub struct ModuleData { pub parent: Option, pub children: FxHashMap, pub scope: ModuleScope, - // FIXME: these can't be both null, we need a three-state enum here. - /// None for root - pub declaration: Option>, - /// None for inline modules. - /// - /// Note that non-inline modules, by definition, live inside non-macro file. - pub definition: Option, + /// Where does this module come from? + pub origin: ModuleOrigin, pub impls: Vec, } @@ -262,7 +326,7 @@ impl CrateDefMap { pub fn modules_for_file(&self, file_id: FileId) -> impl Iterator + '_ { self.modules .iter() - .filter(move |(_id, data)| data.definition == Some(file_id)) + .filter(move |(_id, data)| data.origin.file_id() == Some(file_id)) .map(|(id, _data)| id) } @@ -285,18 +349,13 @@ impl ModuleData { &self, db: &impl DefDatabase, ) -> InFile> { - if let Some(file_id) = self.definition { - let sf = db.parse(file_id).tree(); - return InFile::new(file_id.into(), Either::Left(sf)); - } - let decl = self.declaration.unwrap(); - InFile::new(decl.file_id, Either::Right(decl.to_node(db))) + self.origin.definition_source(db) } /// Returns a node which declares this module, either a `mod foo;` or a `mod foo {}`. - /// `None` for the crate root. + /// `None` for the crate root or block. pub fn declaration_source(&self, db: &impl DefDatabase) -> Option> { - let decl = self.declaration?; + let decl = self.origin.declaration()?; let value = decl.to_node(db); Some(InFile { file_id: decl.file_id, value }) } diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index d4bfcae1d..6f4a3e42e 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -19,7 +19,7 @@ use crate::{ db::DefDatabase, nameres::{ diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint, - raw, BuiltinShadowMode, CrateDefMap, ModuleData, Resolution, ResolveMode, + raw, BuiltinShadowMode, CrateDefMap, ModuleData, ModuleOrigin, Resolution, ResolveMode, }, path::{Path, PathKind}, per_ns::PerNs, @@ -131,7 +131,7 @@ where let file_id = crate_graph.crate_root(self.def_map.krate); let raw_items = self.db.raw_items(file_id.into()); let module_id = self.def_map.root; - self.def_map.modules[module_id].definition = Some(file_id); + self.def_map.modules[module_id].origin = ModuleOrigin::root(file_id); ModCollector { def_collector: &mut *self, module_id, @@ -669,8 +669,7 @@ where let modules = &mut self.def_collector.def_map.modules; let res = modules.alloc(ModuleData::default()); modules[res].parent = Some(self.module_id); - modules[res].declaration = Some(declaration); - modules[res].definition = definition; + modules[res].origin = ModuleOrigin::not_sure_file(definition, declaration); modules[res].scope.legacy_macros = modules[self.module_id].scope.legacy_macros.clone(); modules[self.module_id].children.insert(name.clone(), res); let resolution = Resolution { -- cgit v1.2.3 From 7cbedc50bcf048c87f141a85418581076d67fc7a Mon Sep 17 00:00:00 2001 From: ice1000 Date: Tue, 3 Dec 2019 15:23:21 -0500 Subject: Fix test compilation --- crates/ra_hir_def/src/nameres.rs | 2 +- crates/ra_hir_ty/src/test_db.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index cf2a83191..ca8cbcd42 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -164,7 +164,7 @@ impl ModuleOrigin { ModuleOrigin::Root(None) => unreachable!(), ModuleOrigin::Inline(m) => InFile::new(m.file_id, Either::Right(m.to_node(db))), // FIXME: right now it's never constructed, so it's fine to omit - ModuleOrigin::Block(b) => unimplemented!(), + ModuleOrigin::Block(_b) => unimplemented!(), } } } diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs index 1dc9793f9..476f6df52 100644 --- a/crates/ra_hir_ty/src/test_db.rs +++ b/crates/ra_hir_ty/src/test_db.rs @@ -74,7 +74,7 @@ impl TestDB { for &krate in self.relevant_crates(file_id).iter() { let crate_def_map = self.crate_def_map(krate); for (local_id, data) in crate_def_map.modules.iter() { - if data.definition == Some(file_id) { + if data.origin.file_id() == Some(file_id) { return ModuleId { krate, local_id }; } } -- cgit v1.2.3 From 38853459e3d964cc7f635829cdc66f5faee33d85 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Tue, 3 Dec 2019 15:24:02 -0500 Subject: Add `ModuleSource::Block` --- crates/ra_hir/src/code_model.rs | 65 ++---------------------- crates/ra_hir/src/code_model/src.rs | 3 +- crates/ra_hir/src/from_source.rs | 8 +-- crates/ra_hir/src/lib.rs | 5 +- crates/ra_hir/src/source_binder.rs | 3 +- crates/ra_hir_def/src/nameres.rs | 68 +++++++++++++++++++++++++- crates/ra_ide/src/display/navigation_target.rs | 43 ++++++---------- crates/ra_ide/src/references/rename.rs | 2 +- crates/ra_ide/src/references/search_scope.rs | 5 ++ 9 files changed, 102 insertions(+), 100 deletions(-) diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index 5877afefa..7706399ae 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs @@ -11,6 +11,7 @@ use hir_def::{ builtin_type::BuiltinType, docs::Documentation, expr::{BindingAnnotation, Pat, PatId}, + nameres::ModuleSource, per_ns::PerNs, resolver::HasResolver, type_ref::{Mutability, TypeRef}, @@ -21,11 +22,11 @@ use hir_def::{ use hir_expand::{ diagnostics::DiagnosticSink, name::{self, AsName}, - AstId, MacroDefId, + MacroDefId, }; use hir_ty::expr::ExprValidator; -use ra_db::{CrateId, Edition, FileId, FilePosition}; -use ra_syntax::{ast, AstNode, SyntaxNode}; +use ra_db::{CrateId, Edition}; +use ra_syntax::ast; use crate::{ db::{DefDatabase, HirDatabase}, @@ -79,64 +80,6 @@ impl Crate { } } -pub enum ModuleSource { - SourceFile(ast::SourceFile), - Module(ast::Module), -} - -impl ModuleSource { - pub fn new( - db: &impl DefDatabase, - file_id: Option, - decl_id: Option>, - ) -> ModuleSource { - match (file_id, decl_id) { - (Some(file_id), _) => { - let source_file = db.parse(file_id).tree(); - ModuleSource::SourceFile(source_file) - } - (None, Some(item_id)) => { - let module = item_id.to_node(db); - assert!(module.item_list().is_some(), "expected inline module"); - ModuleSource::Module(module) - } - (None, None) => panic!(), - } - } - - // FIXME: this methods do not belong here - pub fn from_position(db: &impl DefDatabase, position: FilePosition) -> ModuleSource { - let parse = db.parse(position.file_id); - match &ra_syntax::algo::find_node_at_offset::( - parse.tree().syntax(), - position.offset, - ) { - Some(m) if !m.has_semi() => ModuleSource::Module(m.clone()), - _ => { - let source_file = parse.tree(); - ModuleSource::SourceFile(source_file) - } - } - } - - pub fn from_child_node(db: &impl DefDatabase, child: InFile<&SyntaxNode>) -> ModuleSource { - if let Some(m) = - child.value.ancestors().filter_map(ast::Module::cast).find(|it| !it.has_semi()) - { - ModuleSource::Module(m) - } else { - let file_id = child.file_id.original_file(db); - let source_file = db.parse(file_id).tree(); - ModuleSource::SourceFile(source_file) - } - } - - pub fn from_file_id(db: &impl DefDatabase, file_id: FileId) -> ModuleSource { - let source_file = db.parse(file_id).tree(); - ModuleSource::SourceFile(source_file) - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Module { pub(crate) id: ModuleId, diff --git a/crates/ra_hir/src/code_model/src.rs b/crates/ra_hir/src/code_model/src.rs index 36cfbc8f1..083946729 100644 --- a/crates/ra_hir/src/code_model/src.rs +++ b/crates/ra_hir/src/code_model/src.rs @@ -2,6 +2,7 @@ use either::Either; use hir_def::{ + nameres::ModuleSource, src::{HasChildSource, HasSource as _}, AstItemDef, Lookup, VariantId, }; @@ -9,7 +10,7 @@ use ra_syntax::ast; use crate::{ db::DefDatabase, Const, Enum, EnumVariant, FieldSource, Function, ImplBlock, Import, MacroDef, - Module, ModuleSource, Static, Struct, StructField, Trait, TypeAlias, Union, + Module, Static, Struct, StructField, Trait, TypeAlias, Union, }; pub use hir_expand::InFile; diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index 82bf641dc..94a5e0b0d 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -1,6 +1,6 @@ //! FIXME: write short doc here -use hir_def::{AstItemDef, LocationCtx, ModuleId}; +use hir_def::{nameres::ModuleSource, AstItemDef, LocationCtx, ModuleId}; use hir_expand::{name::AsName, AstId, MacroDefId, MacroDefKind}; use ra_syntax::{ ast::{self, AstNode, NameOwner}, @@ -10,8 +10,8 @@ use ra_syntax::{ use crate::{ db::{AstDatabase, DefDatabase, HirDatabase}, AssocItem, Const, DefWithBody, Enum, EnumVariant, FieldSource, Function, HasSource, ImplBlock, - InFile, Local, MacroDef, Module, ModuleDef, ModuleSource, Static, Struct, StructField, Trait, - TypeAlias, Union, VariantDef, + InFile, Local, MacroDef, Module, ModuleDef, Static, Struct, StructField, Trait, TypeAlias, + Union, VariantDef, }; pub trait FromSource: Sized { @@ -257,7 +257,7 @@ impl Module { InFile { file_id: src.file_id, value: module.clone() }, ); } - ModuleSource::SourceFile(_) => (), + ModuleSource::SourceFile(_) | ModuleSource::Block(_) => (), }; let original_file = src.file_id.original_file(db); diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index 853760cb1..f12e4ca3f 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -43,8 +43,8 @@ pub use crate::{ code_model::{ src::HasSource, Adt, AssocItem, AttrDef, Const, Container, Crate, CrateDependency, DefWithBody, Docs, Enum, EnumVariant, FieldSource, Function, GenericDef, GenericParam, - HasAttrs, ImplBlock, Import, Local, MacroDef, Module, ModuleDef, ModuleSource, ScopeDef, - Static, Struct, StructField, Trait, Type, TypeAlias, Union, VariantDef, + HasAttrs, ImplBlock, Import, Local, MacroDef, Module, ModuleDef, ScopeDef, Static, Struct, + StructField, Trait, Type, TypeAlias, Union, VariantDef, }, from_source::FromSource, source_binder::{PathResolution, ScopeEntryWithSyntax, SourceAnalyzer}, @@ -59,6 +59,7 @@ pub use hir_def::{ body::scope::ExprScopes, builtin_type::BuiltinType, docs::Documentation, + nameres::ModuleSource, path::{Path, PathKind}, type_ref::Mutability, }; diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs index 28d41b647..db0451059 100644 --- a/crates/ra_hir/src/source_binder.rs +++ b/crates/ra_hir/src/source_binder.rs @@ -14,6 +14,7 @@ use hir_def::{ BodySourceMap, }, expr::{ExprId, PatId}, + nameres::ModuleSource, path::known, resolver::{self, resolver_for_scope, HasResolver, Resolver, TypeNs, ValueNs}, AssocItemId, DefWithBodyId, @@ -46,7 +47,7 @@ fn try_get_resolver_for_node(db: &impl HirDatabase, node: InFile<&SyntaxNode>) - Some(crate::Module::from_declaration(db, src)?.id.resolver(db)) }, ast::SourceFile(it) => { - let src = node.with_value(crate::ModuleSource::SourceFile(it)); + let src = node.with_value(ModuleSource::SourceFile(it)); Some(crate::Module::from_definition(db, src)?.id.resolver(db)) }, ast::StructDef(it) => { diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index ca8cbcd42..feb2a4d32 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -63,9 +63,12 @@ use hir_expand::{ }; use once_cell::sync::Lazy; use ra_arena::Arena; -use ra_db::{CrateId, Edition, FileId}; +use ra_db::{CrateId, Edition, FileId, FilePosition}; use ra_prof::profile; -use ra_syntax::ast; +use ra_syntax::{ + ast::{self, AstNode}, + SyntaxNode, +}; use rustc_hash::FxHashMap; use crate::{ @@ -361,6 +364,67 @@ impl ModuleData { } } +pub enum ModuleSource { + SourceFile(ast::SourceFile), + Module(ast::Module), + Block(ast::Block), +} + +impl ModuleSource { + pub fn new( + db: &impl DefDatabase, + file_id: Option, + decl_id: Option>, + ) -> ModuleSource { + match (file_id, decl_id) { + (Some(file_id), _) => { + let source_file = db.parse(file_id).tree(); + ModuleSource::SourceFile(source_file) + } + (None, Some(item_id)) => { + let module = item_id.to_node(db); + assert!(module.item_list().is_some(), "expected inline module"); + ModuleSource::Module(module) + } + (None, None) => panic!(), + } + } + + // FIXME: this methods do not belong here + pub fn from_position(db: &impl DefDatabase, position: FilePosition) -> ModuleSource { + let parse = db.parse(position.file_id); + match &ra_syntax::algo::find_node_at_offset::( + parse.tree().syntax(), + position.offset, + ) { + Some(m) if !m.has_semi() => ModuleSource::Module(m.clone()), + _ => { + let source_file = parse.tree(); + ModuleSource::SourceFile(source_file) + } + } + } + + pub fn from_child_node(db: &impl DefDatabase, child: InFile<&SyntaxNode>) -> ModuleSource { + if let Some(m) = + child.value.ancestors().filter_map(ast::Module::cast).find(|it| !it.has_semi()) + { + ModuleSource::Module(m) + } else if let Some(b) = child.value.ancestors().filter_map(ast::Block::cast).next() { + ModuleSource::Block(b) + } else { + let file_id = child.file_id.original_file(db); + let source_file = db.parse(file_id).tree(); + ModuleSource::SourceFile(source_file) + } + } + + pub fn from_file_id(db: &impl DefDatabase, file_id: FileId) -> ModuleSource { + let source_file = db.parse(file_id).tree(); + ModuleSource::SourceFile(source_file) + } +} + mod diagnostics { use hir_expand::diagnostics::DiagnosticSink; use ra_db::RelativePathBuf; diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs index f920d3db6..b376fcdae 100644 --- a/crates/ra_ide/src/display/navigation_target.rs +++ b/crates/ra_ide/src/display/navigation_target.rs @@ -231,34 +231,21 @@ impl ToNav for hir::Module { fn to_nav(&self, db: &RootDatabase) -> NavigationTarget { let src = self.definition_source(db); let name = self.name(db).map(|it| it.to_string().into()).unwrap_or_default(); - match &src.value { - ModuleSource::SourceFile(node) => { - let frange = original_range(db, src.with_value(node.syntax())); - - NavigationTarget::from_syntax( - frange.file_id, - name, - None, - frange.range, - node.syntax().kind(), - None, - None, - ) - } - ModuleSource::Module(node) => { - let frange = original_range(db, src.with_value(node.syntax())); - - NavigationTarget::from_syntax( - frange.file_id, - name, - None, - frange.range, - node.syntax().kind(), - node.doc_comment_text(), - node.short_label(), - ) - } - } + let syntax = match &src.value { + ModuleSource::SourceFile(node) => node.syntax(), + ModuleSource::Module(node) => node.syntax(), + ModuleSource::Block(node) => node.syntax(), + }; + let frange = original_range(db, src.with_value(syntax)); + NavigationTarget::from_syntax( + frange.file_id, + name, + None, + frange.range, + syntax.kind(), + None, + None, + ) } } diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs index b804d5f6d..1abf20215 100644 --- a/crates/ra_ide/src/references/rename.rs +++ b/crates/ra_ide/src/references/rename.rs @@ -88,7 +88,7 @@ fn rename_mod( file_system_edits.push(move_file); } } - ModuleSource::Module(..) => {} + ModuleSource::Module(..) | ModuleSource::Block(..) => {} } } diff --git a/crates/ra_ide/src/references/search_scope.rs b/crates/ra_ide/src/references/search_scope.rs index f5c9589f4..92eb2015b 100644 --- a/crates/ra_ide/src/references/search_scope.rs +++ b/crates/ra_ide/src/references/search_scope.rs @@ -96,6 +96,10 @@ impl NameDefinition { let range = Some(m.syntax().text_range()); res.insert(file_id, range); } + ModuleSource::Block(b) => { + let range = Some(b.syntax().text_range()); + res.insert(file_id, range); + } ModuleSource::SourceFile(_) => { res.insert(file_id, None); res.extend(parent_module.children(db).map(|m| { @@ -137,6 +141,7 @@ impl NameDefinition { let mut res = FxHashMap::default(); let range = match module_src.value { ModuleSource::Module(m) => Some(m.syntax().text_range()), + ModuleSource::Block(b) => Some(b.syntax().text_range()), ModuleSource::SourceFile(_) => None, }; res.insert(file_id, range); -- cgit v1.2.3 From 5c5f90ba57c83499a44af33bf8b91b24254fb685 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Tue, 3 Dec 2019 15:28:40 -0500 Subject: Confluent `ModuleSource` usage --- crates/ra_hir/src/code_model/src.rs | 6 +----- crates/ra_hir_def/src/nameres.rs | 18 +++++------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/crates/ra_hir/src/code_model/src.rs b/crates/ra_hir/src/code_model/src.rs index 083946729..d9bccd902 100644 --- a/crates/ra_hir/src/code_model/src.rs +++ b/crates/ra_hir/src/code_model/src.rs @@ -26,11 +26,7 @@ impl Module { /// Returns a node which defines this module. That is, a file or a `mod foo {}` with items. pub fn definition_source(self, db: &impl DefDatabase) -> InFile { let def_map = db.crate_def_map(self.id.krate); - let src = def_map[self.id.local_id].definition_source(db); - src.map(|it| match it { - Either::Left(it) => ModuleSource::SourceFile(it), - Either::Right(it) => ModuleSource::Module(it), - }) + def_map[self.id.local_id].definition_source(db) } /// Returns a node which declares this module, either a `mod foo;` or a `mod foo {}`. diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index feb2a4d32..e356515cb 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -57,7 +57,6 @@ mod tests; use std::sync::Arc; -use either::Either; use hir_expand::{ ast_id_map::FileAstId, diagnostics::DiagnosticSink, name::Name, InFile, MacroDefId, }; @@ -154,20 +153,16 @@ impl ModuleOrigin { /// Returns a node which defines this module. /// That is, a file or a `mod foo {}` with items. - pub fn definition_source( - &self, - db: &impl DefDatabase, - ) -> InFile> { + pub fn definition_source(&self, db: &impl DefDatabase) -> InFile { match self { ModuleOrigin::File(_, file_id) | ModuleOrigin::Root(Some(file_id)) => { let file_id = *file_id; let sf = db.parse(file_id).tree(); - return InFile::new(file_id.into(), Either::Left(sf)); + return InFile::new(file_id.into(), ModuleSource::SourceFile(sf)); } ModuleOrigin::Root(None) => unreachable!(), - ModuleOrigin::Inline(m) => InFile::new(m.file_id, Either::Right(m.to_node(db))), - // FIXME: right now it's never constructed, so it's fine to omit - ModuleOrigin::Block(_b) => unimplemented!(), + ModuleOrigin::Inline(m) => InFile::new(m.file_id, ModuleSource::Module(m.to_node(db))), + ModuleOrigin::Block(b) => InFile::new(b.file_id, ModuleSource::Block(b.to_node(db))), } } } @@ -348,10 +343,7 @@ impl CrateDefMap { impl ModuleData { /// Returns a node which defines this module. That is, a file or a `mod foo {}` with items. - pub fn definition_source( - &self, - db: &impl DefDatabase, - ) -> InFile> { + pub fn definition_source(&self, db: &impl DefDatabase) -> InFile { self.origin.definition_source(db) } -- cgit v1.2.3 From 032eb3d68e07f087ac531d48f956a79948baa4b9 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Tue, 3 Dec 2019 15:58:38 -0500 Subject: Remove almost unused `ModuleSource::new` --- crates/ra_hir/src/from_source.rs | 2 +- crates/ra_hir_def/src/nameres.rs | 20 +------------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index 94a5e0b0d..67b127ba5 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -238,7 +238,7 @@ impl Module { _ => { let src_parent = InFile { file_id: src.file_id, - value: ModuleSource::new(db, Some(src.file_id.original_file(db)), None), + value: ModuleSource::from_file_id(db, src.file_id.original_file(db)), }; Module::from_definition(db, src_parent) } diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index e356515cb..ce318571c 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -356,6 +356,7 @@ impl ModuleData { } } +#[derive(Debug, Clone, PartialEq, Eq)] pub enum ModuleSource { SourceFile(ast::SourceFile), Module(ast::Module), @@ -363,25 +364,6 @@ pub enum ModuleSource { } impl ModuleSource { - pub fn new( - db: &impl DefDatabase, - file_id: Option, - decl_id: Option>, - ) -> ModuleSource { - match (file_id, decl_id) { - (Some(file_id), _) => { - let source_file = db.parse(file_id).tree(); - ModuleSource::SourceFile(source_file) - } - (None, Some(item_id)) => { - let module = item_id.to_node(db); - assert!(module.item_list().is_some(), "expected inline module"); - ModuleSource::Module(module) - } - (None, None) => panic!(), - } - } - // FIXME: this methods do not belong here pub fn from_position(db: &impl DefDatabase, position: FilePosition) -> ModuleSource { let parse = db.parse(position.file_id); -- cgit v1.2.3 From 088f50c0ab351d5ac072547a47c1ce7eeae029f3 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Wed, 4 Dec 2019 13:35:24 -0500 Subject: No block at the moment --- crates/ra_hir/src/from_source.rs | 2 +- crates/ra_hir_def/src/nameres.rs | 7 +------ crates/ra_ide/src/display/navigation_target.rs | 1 - crates/ra_ide/src/references/rename.rs | 2 +- crates/ra_ide/src/references/search_scope.rs | 5 ----- 5 files changed, 3 insertions(+), 14 deletions(-) diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index 67b127ba5..a46bba4f2 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -257,7 +257,7 @@ impl Module { InFile { file_id: src.file_id, value: module.clone() }, ); } - ModuleSource::SourceFile(_) | ModuleSource::Block(_) => (), + ModuleSource::SourceFile(_) => (), }; let original_file = src.file_id.original_file(db); diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index ce318571c..f70235c99 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -109,7 +109,6 @@ pub enum ModuleOrigin { /// Note that non-inline modules, by definition, live inside non-macro file. File(AstId, FileId), Inline(AstId), - Block(AstId), } impl Default for ModuleOrigin { @@ -140,7 +139,7 @@ impl ModuleOrigin { pub fn declaration(&self) -> Option> { match self { ModuleOrigin::File(m, _) | ModuleOrigin::Inline(m) => Some(*m), - ModuleOrigin::Root(_) | ModuleOrigin::Block(_) => None, + ModuleOrigin::Root(_) => None, } } @@ -162,7 +161,6 @@ impl ModuleOrigin { } ModuleOrigin::Root(None) => unreachable!(), ModuleOrigin::Inline(m) => InFile::new(m.file_id, ModuleSource::Module(m.to_node(db))), - ModuleOrigin::Block(b) => InFile::new(b.file_id, ModuleSource::Block(b.to_node(db))), } } } @@ -360,7 +358,6 @@ impl ModuleData { pub enum ModuleSource { SourceFile(ast::SourceFile), Module(ast::Module), - Block(ast::Block), } impl ModuleSource { @@ -384,8 +381,6 @@ impl ModuleSource { child.value.ancestors().filter_map(ast::Module::cast).find(|it| !it.has_semi()) { ModuleSource::Module(m) - } else if let Some(b) = child.value.ancestors().filter_map(ast::Block::cast).next() { - ModuleSource::Block(b) } else { let file_id = child.file_id.original_file(db); let source_file = db.parse(file_id).tree(); diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs index b376fcdae..add11fbc3 100644 --- a/crates/ra_ide/src/display/navigation_target.rs +++ b/crates/ra_ide/src/display/navigation_target.rs @@ -234,7 +234,6 @@ impl ToNav for hir::Module { let syntax = match &src.value { ModuleSource::SourceFile(node) => node.syntax(), ModuleSource::Module(node) => node.syntax(), - ModuleSource::Block(node) => node.syntax(), }; let frange = original_range(db, src.with_value(syntax)); NavigationTarget::from_syntax( diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs index 1abf20215..b804d5f6d 100644 --- a/crates/ra_ide/src/references/rename.rs +++ b/crates/ra_ide/src/references/rename.rs @@ -88,7 +88,7 @@ fn rename_mod( file_system_edits.push(move_file); } } - ModuleSource::Module(..) | ModuleSource::Block(..) => {} + ModuleSource::Module(..) => {} } } diff --git a/crates/ra_ide/src/references/search_scope.rs b/crates/ra_ide/src/references/search_scope.rs index 92eb2015b..f5c9589f4 100644 --- a/crates/ra_ide/src/references/search_scope.rs +++ b/crates/ra_ide/src/references/search_scope.rs @@ -96,10 +96,6 @@ impl NameDefinition { let range = Some(m.syntax().text_range()); res.insert(file_id, range); } - ModuleSource::Block(b) => { - let range = Some(b.syntax().text_range()); - res.insert(file_id, range); - } ModuleSource::SourceFile(_) => { res.insert(file_id, None); res.extend(parent_module.children(db).map(|m| { @@ -141,7 +137,6 @@ impl NameDefinition { let mut res = FxHashMap::default(); let range = match module_src.value { ModuleSource::Module(m) => Some(m.syntax().text_range()), - ModuleSource::Block(b) => Some(b.syntax().text_range()), ModuleSource::SourceFile(_) => None, }; res.insert(file_id, range); -- cgit v1.2.3 From 762915826ac6893036f8b5cd5e63677ed862f6d9 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Thu, 5 Dec 2019 08:19:27 -0500 Subject: Reduce visibility, use struct instead of tuples --- crates/ra_hir_def/src/nameres.rs | 43 +++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index f70235c99..01d67777d 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -107,8 +107,13 @@ pub enum ModuleOrigin { /// It should not be `None` after collecting definitions. Root(Option), /// Note that non-inline modules, by definition, live inside non-macro file. - File(AstId, FileId), - Inline(AstId), + File { + declaration: AstId, + definition: FileId, + }, + Inline { + definition: AstId, + }, } impl Default for ModuleOrigin { @@ -118,49 +123,47 @@ impl Default for ModuleOrigin { } impl ModuleOrigin { - pub fn root(file_id: FileId) -> Self { + fn root(file_id: FileId) -> Self { ModuleOrigin::Root(Some(file_id)) } - pub fn not_sure_file(file: Option, module: AstId) -> Self { + pub(crate) fn not_sure_file(file: Option, declaration: AstId) -> Self { match file { - None => ModuleOrigin::Inline(module), - Some(file) => ModuleOrigin::File(module, file), + None => ModuleOrigin::Inline { definition: declaration }, + Some(definition) => ModuleOrigin::File { declaration, definition }, } } - pub fn not_sure_mod(file: FileId, module: Option>) -> Self { - match module { - None => ModuleOrigin::root(file), - Some(module) => ModuleOrigin::File(module, file), - } - } - - pub fn declaration(&self) -> Option> { + fn declaration(&self) -> Option> { match self { - ModuleOrigin::File(m, _) | ModuleOrigin::Inline(m) => Some(*m), + ModuleOrigin::File { declaration: module, .. } + | ModuleOrigin::Inline { definition: module, .. } => Some(*module), ModuleOrigin::Root(_) => None, } } - pub fn file_id(&self) -> Option { + pub(crate) fn file_id(&self) -> Option { match self { - ModuleOrigin::File(_, file_id) | ModuleOrigin::Root(Some(file_id)) => Some(*file_id), + ModuleOrigin::File { definition: file_id, .. } | ModuleOrigin::Root(Some(file_id)) => { + Some(*file_id) + } _ => None, } } /// Returns a node which defines this module. /// That is, a file or a `mod foo {}` with items. - pub fn definition_source(&self, db: &impl DefDatabase) -> InFile { + fn definition_source(&self, db: &impl DefDatabase) -> InFile { match self { - ModuleOrigin::File(_, file_id) | ModuleOrigin::Root(Some(file_id)) => { + ModuleOrigin::File { definition: file_id, .. } | ModuleOrigin::Root(Some(file_id)) => { let file_id = *file_id; let sf = db.parse(file_id).tree(); return InFile::new(file_id.into(), ModuleSource::SourceFile(sf)); } ModuleOrigin::Root(None) => unreachable!(), - ModuleOrigin::Inline(m) => InFile::new(m.file_id, ModuleSource::Module(m.to_node(db))), + ModuleOrigin::Inline { definition } => { + InFile::new(definition.file_id, ModuleSource::Module(definition.to_node(db))) + } } } } -- cgit v1.2.3 From 7702f690a9592605be71104ec9d0b732af940fcc Mon Sep 17 00:00:00 2001 From: ice1000 Date: Thu, 5 Dec 2019 08:28:31 -0500 Subject: One pub function less is good! --- crates/ra_hir/src/from_source.rs | 9 ++++----- crates/ra_hir_def/src/nameres.rs | 5 ----- crates/ra_ide/src/parent_module.rs | 5 +++-- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/crates/ra_hir/src/from_source.rs b/crates/ra_hir/src/from_source.rs index a46bba4f2..18d87f6d7 100644 --- a/crates/ra_hir/src/from_source.rs +++ b/crates/ra_hir/src/from_source.rs @@ -235,11 +235,10 @@ impl Module { let src_parent = InFile { file_id: src.file_id, value: parent_declaration }; Module::from_declaration(db, src_parent) } - _ => { - let src_parent = InFile { - file_id: src.file_id, - value: ModuleSource::from_file_id(db, src.file_id.original_file(db)), - }; + None => { + let source_file = db.parse(src.file_id.original_file(db)).tree(); + let src_parent = + InFile { file_id: src.file_id, value: ModuleSource::SourceFile(source_file) }; Module::from_definition(db, src_parent) } }?; diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index 01d67777d..0a2b32bbd 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -390,11 +390,6 @@ impl ModuleSource { ModuleSource::SourceFile(source_file) } } - - pub fn from_file_id(db: &impl DefDatabase, file_id: FileId) -> ModuleSource { - let source_file = db.parse(file_id).tree(); - ModuleSource::SourceFile(source_file) - } } mod diagnostics { diff --git a/crates/ra_ide/src/parent_module.rs b/crates/ra_ide/src/parent_module.rs index 616d69fce..aef3fa3df 100644 --- a/crates/ra_ide/src/parent_module.rs +++ b/crates/ra_ide/src/parent_module.rs @@ -1,6 +1,6 @@ //! FIXME: write short doc here -use ra_db::{CrateId, FileId, FilePosition}; +use ra_db::{CrateId, FileId, FilePosition, SourceDatabase}; use crate::{db::RootDatabase, NavigationTarget}; @@ -21,7 +21,8 @@ pub(crate) fn parent_module(db: &RootDatabase, position: FilePosition) -> Vec Vec { - let src = hir::ModuleSource::from_file_id(db, file_id); + let source_file = db.parse(file_id).tree(); + let src = hir::ModuleSource::SourceFile(source_file); let module = match hir::Module::from_definition(db, hir::InFile { file_id: file_id.into(), value: src }) { -- cgit v1.2.3 From 006a583381ab66f229c4d49531cb50687343eea3 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Thu, 5 Dec 2019 08:33:29 -0500 Subject: Use placeholder instead of `Option` --- crates/ra_hir_def/src/nameres.rs | 22 +++++++++------------- crates/ra_hir_def/src/nameres/collector.rs | 2 +- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index 0a2b32bbd..98bab1bfe 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -104,8 +104,9 @@ impl std::ops::Index for CrateDefMap { #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] pub enum ModuleOrigin { - /// It should not be `None` after collecting definitions. - Root(Option), + CrateRoot { + definition: FileId, + }, /// Note that non-inline modules, by definition, live inside non-macro file. File { declaration: AstId, @@ -118,15 +119,11 @@ pub enum ModuleOrigin { impl Default for ModuleOrigin { fn default() -> Self { - ModuleOrigin::Root(None) + ModuleOrigin::CrateRoot { definition: FileId(0) } } } impl ModuleOrigin { - fn root(file_id: FileId) -> Self { - ModuleOrigin::Root(Some(file_id)) - } - pub(crate) fn not_sure_file(file: Option, declaration: AstId) -> Self { match file { None => ModuleOrigin::Inline { definition: declaration }, @@ -138,14 +135,14 @@ impl ModuleOrigin { match self { ModuleOrigin::File { declaration: module, .. } | ModuleOrigin::Inline { definition: module, .. } => Some(*module), - ModuleOrigin::Root(_) => None, + ModuleOrigin::CrateRoot { .. } => None, } } pub(crate) fn file_id(&self) -> Option { match self { - ModuleOrigin::File { definition: file_id, .. } | ModuleOrigin::Root(Some(file_id)) => { - Some(*file_id) + ModuleOrigin::File { definition, .. } | ModuleOrigin::CrateRoot { definition } => { + Some(*definition) } _ => None, } @@ -155,12 +152,11 @@ impl ModuleOrigin { /// That is, a file or a `mod foo {}` with items. fn definition_source(&self, db: &impl DefDatabase) -> InFile { match self { - ModuleOrigin::File { definition: file_id, .. } | ModuleOrigin::Root(Some(file_id)) => { - let file_id = *file_id; + ModuleOrigin::File { definition, .. } | ModuleOrigin::CrateRoot { definition } => { + let file_id = *definition; let sf = db.parse(file_id).tree(); return InFile::new(file_id.into(), ModuleSource::SourceFile(sf)); } - ModuleOrigin::Root(None) => unreachable!(), ModuleOrigin::Inline { definition } => { InFile::new(definition.file_id, ModuleSource::Module(definition.to_node(db))) } diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 6f4a3e42e..9d948d4f4 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -131,7 +131,7 @@ where let file_id = crate_graph.crate_root(self.def_map.krate); let raw_items = self.db.raw_items(file_id.into()); let module_id = self.def_map.root; - self.def_map.modules[module_id].origin = ModuleOrigin::root(file_id); + self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id }; ModCollector { def_collector: &mut *self, module_id, -- cgit v1.2.3 From d15f300268fa955f8e9c04d51ca1dc528766f742 Mon Sep 17 00:00:00 2001 From: ice1000 Date: Thu, 5 Dec 2019 08:37:39 -0500 Subject: Publicize `file_id` to make `test_db` compile --- crates/ra_hir_def/src/nameres.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ra_hir_def/src/nameres.rs b/crates/ra_hir_def/src/nameres.rs index 98bab1bfe..bd237a7b3 100644 --- a/crates/ra_hir_def/src/nameres.rs +++ b/crates/ra_hir_def/src/nameres.rs @@ -139,7 +139,7 @@ impl ModuleOrigin { } } - pub(crate) fn file_id(&self) -> Option { + pub fn file_id(&self) -> Option { match self { ModuleOrigin::File { definition, .. } | ModuleOrigin::CrateRoot { definition } => { Some(*definition) -- cgit v1.2.3