From ce0d4a3b1deaca318a1ed2ff83cca652ef8e8c82 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 10 Oct 2019 14:45:05 +0300 Subject: Refactor and fix some more edge cases around name resolution --- crates/ra_hir/src/nameres/collector.rs | 40 ++-- crates/ra_hir/src/nameres/mod_resolution.rs | 232 +++++++--------------- crates/ra_hir/src/nameres/tests/macros.rs | 29 ++- crates/ra_hir/src/nameres/tests/mod_resolution.rs | 100 +++++++++- 4 files changed, 209 insertions(+), 192 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs index cef2dc9d2..88aee7437 100644 --- a/crates/ra_hir/src/nameres/collector.rs +++ b/crates/ra_hir/src/nameres/collector.rs @@ -2,7 +2,7 @@ use ra_cfg::CfgOptions; use ra_db::FileId; -use ra_syntax::{ast, SmolStr}; +use ra_syntax::ast; use rustc_hash::FxHashMap; use test_utils::tested_by; @@ -11,10 +11,8 @@ use crate::{ ids::{AstItemDef, LocationCtx, MacroCallId, MacroCallLoc, MacroDefId, MacroFileKind}, name::MACRO_RULES, nameres::{ - diagnostics::DefDiagnostic, - mod_resolution::{resolve_submodule, ParentModule}, - raw, Crate, CrateDefMap, CrateModuleId, ModuleData, ModuleDef, PerNs, ReachedFixedPoint, - Resolution, ResolveMode, + diagnostics::DefDiagnostic, mod_resolution::ModDir, raw, Crate, CrateDefMap, CrateModuleId, + ModuleData, ModuleDef, PerNs, ReachedFixedPoint, Resolution, ResolveMode, }, Adt, AstId, Const, Enum, Function, HirFileId, MacroDef, Module, Name, Path, PathKind, Static, Struct, Trait, TypeAlias, Union, @@ -45,6 +43,7 @@ pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> C glob_imports: FxHashMap::default(), unresolved_imports: Vec::new(), unexpanded_macros: Vec::new(), + mod_dirs: FxHashMap::default(), macro_stack_monitor: MacroStackMonitor::default(), cfg_options, }; @@ -87,6 +86,7 @@ struct DefCollector<'a, DB> { glob_imports: FxHashMap>, unresolved_imports: Vec<(CrateModuleId, raw::ImportId, raw::ImportData)>, unexpanded_macros: Vec<(CrateModuleId, AstId, Path)>, + mod_dirs: FxHashMap, /// Some macro use `$tt:tt which mean we have to handle the macro perfectly /// To prevent stack overflow, we add a deep counter here for prevent that. @@ -107,11 +107,10 @@ where self.def_map.modules[module_id].definition = Some(file_id); ModCollector { def_collector: &mut *self, - attr_path: None, module_id, file_id: file_id.into(), raw_items: &raw_items, - parent_module: None, + mod_dir: ModDir::root(), } .collect(raw_items.items()); @@ -481,13 +480,13 @@ where if !self.macro_stack_monitor.is_poison(macro_def_id) { let file_id: HirFileId = macro_call_id.as_file(MacroFileKind::Items); let raw_items = self.db.raw_items(file_id); + let mod_dir = self.mod_dirs[&module_id].clone(); ModCollector { def_collector: &mut *self, file_id, - attr_path: None, module_id, raw_items: &raw_items, - parent_module: None, + mod_dir, } .collect(raw_items.items()); } else { @@ -508,9 +507,8 @@ struct ModCollector<'a, D> { def_collector: D, module_id: CrateModuleId, file_id: HirFileId, - attr_path: Option<&'a SmolStr>, raw_items: &'a raw::RawItems, - parent_module: Option>, + mod_dir: ModDir, } impl ModCollector<'_, &'_ mut DefCollector<'_, DB>> @@ -518,6 +516,10 @@ where DB: DefDatabase, { fn collect(&mut self, items: &[raw::RawItem]) { + // Note: don't assert that inserted value is fresh: it's simply not true + // for macros. + self.def_collector.mod_dirs.insert(self.module_id, self.mod_dir.clone()); + // Prelude module is always considered to be `#[macro_use]`. if let Some(prelude_module) = self.def_collector.def_map.prelude { if prelude_module.krate != self.def_collector.def_map.krate { @@ -561,15 +563,13 @@ where raw::ModuleData::Definition { name, items, ast_id, attr_path, is_macro_use } => { let module_id = self.push_child_module(name.clone(), ast_id.with_file_id(self.file_id), None); - let parent_module = ParentModule { name, attr_path: attr_path.as_ref() }; ModCollector { def_collector: &mut *self.def_collector, module_id, - attr_path: attr_path.as_ref(), file_id: self.file_id, raw_items: self.raw_items, - parent_module: Some(parent_module), + mod_dir: self.mod_dir.descend_into_definition(name, attr_path.as_ref()), } .collect(&*items); if *is_macro_use { @@ -579,26 +579,21 @@ where // out of line module, resolve, parse and recurse raw::ModuleData::Declaration { name, ast_id, attr_path, is_macro_use } => { let ast_id = ast_id.with_file_id(self.file_id); - let is_root = self.def_collector.def_map.modules[self.module_id].parent.is_none(); - match resolve_submodule( + match self.mod_dir.resolve_submodule( self.def_collector.db, self.file_id, - self.attr_path, name, - is_root, attr_path.as_ref(), - self.parent_module, ) { - Ok(file_id) => { + Ok((file_id, mod_dir)) => { let module_id = self.push_child_module(name.clone(), ast_id, Some(file_id)); let raw_items = self.def_collector.db.raw_items(file_id.into()); ModCollector { def_collector: &mut *self.def_collector, module_id, - attr_path: attr_path.as_ref(), file_id: file_id.into(), raw_items: &raw_items, - parent_module: None, + mod_dir, } .collect(raw_items.items()); if *is_macro_use { @@ -747,6 +742,7 @@ mod tests { glob_imports: FxHashMap::default(), unresolved_imports: Vec::new(), unexpanded_macros: Vec::new(), + mod_dirs: FxHashMap::default(), macro_stack_monitor: monitor, cfg_options: &CfgOptions::default(), }; diff --git a/crates/ra_hir/src/nameres/mod_resolution.rs b/crates/ra_hir/src/nameres/mod_resolution.rs index 3aa32bd66..f50f9abe6 100644 --- a/crates/ra_hir/src/nameres/mod_resolution.rs +++ b/crates/ra_hir/src/nameres/mod_resolution.rs @@ -1,111 +1,105 @@ //! This module resolves `mod foo;` declaration to file. +use std::borrow::Cow; -use std::{borrow::Cow, sync::Arc}; - -use ra_db::{FileId, SourceRoot}; +use ra_db::FileId; use ra_syntax::SmolStr; -use relative_path::RelativePathBuf; +use relative_path::{RelativePath, RelativePathBuf}; use crate::{db::DefDatabase, HirFileId, Name}; -#[derive(Clone, Copy)] -pub(super) struct ParentModule<'a> { - pub(super) name: &'a Name, - pub(super) attr_path: Option<&'a SmolStr>, +#[derive(Clone, Debug)] +pub(super) struct ModDir { + /// `.` for `mod.rs`, `lib.rs` + /// `./foo` for `foo.rs` + /// `./foo/bar` for `mod bar { mod x; }` nested in `foo.rs` + path: RelativePathBuf, + /// inside `./foo.rs`, mods with `#[path]` should *not* be relative to `./foo/` + root_non_dir_owner: bool, } -impl<'a> ParentModule<'a> { - fn attribute_path(&self) -> Option<&SmolStr> { - self.attr_path.filter(|p| !p.is_empty()) +impl ModDir { + pub(super) fn root() -> ModDir { + ModDir { path: RelativePathBuf::default(), root_non_dir_owner: false } } -} - -pub(super) fn resolve_submodule( - db: &impl DefDatabase, - file_id: HirFileId, - mod_attr_path: Option<&SmolStr>, - name: &Name, - is_root: bool, - attr_path: Option<&SmolStr>, - parent_module: Option>, -) -> Result { - let file_id = file_id.original_file(db); - let source_root_id = db.file_source_root(file_id); - let path = db.file_relative_path(file_id); - let root = RelativePathBuf::default(); - let dir_path = path.parent().unwrap_or(&root); - let mod_name = path.file_stem().unwrap_or("unknown"); - let resolve_mode = match (attr_path.filter(|p| !p.is_empty()), parent_module) { - (Some(file_path), Some(parent_module)) => { - let file_path = normalize_attribute_path(file_path); - match parent_module.attribute_path() { - Some(parent_module_attr_path) => { - let path = dir_path - .join(format!( - "{}/{}", - normalize_attribute_path(parent_module_attr_path), - file_path - )) - .normalize(); - ResolutionMode::InlineModuleWithAttributePath( - InsideInlineModuleMode::WithAttributePath(path), - ) - } - None => { - let path = - dir_path.join(format!("{}/{}", parent_module.name, file_path)).normalize(); - ResolutionMode::InsideInlineModule(InsideInlineModuleMode::WithAttributePath( - path, - )) + pub(super) fn descend_into_definition( + &self, + name: &Name, + attr_path: Option<&SmolStr>, + ) -> ModDir { + let mut path = self.path.clone(); + match attr_path { + None => path.push(&name.to_string()), + Some(attr_path) => { + if self.root_non_dir_owner { + path = path + .parent() + .map(|it| it.to_relative_path_buf()) + .unwrap_or_else(RelativePathBuf::new); } + let attr_path = &*normalize_attribute_path(attr_path); + path.push(RelativePath::new(attr_path)); } } - (None, Some(parent_module)) => match parent_module.attribute_path() { - Some(parent_module_attr_path) => { - let path = dir_path.join(format!( - "{}/{}.rs", - normalize_attribute_path(parent_module_attr_path), - name - )); - ResolutionMode::InlineModuleWithAttributePath(InsideInlineModuleMode::File(path)) + ModDir { path, root_non_dir_owner: false } + } + + pub(super) fn resolve_submodule( + &self, + db: &impl DefDatabase, + file_id: HirFileId, + name: &Name, + attr_path: Option<&SmolStr>, + ) -> Result<(FileId, ModDir), RelativePathBuf> { + let empty_path = RelativePathBuf::default(); + let file_id = file_id.original_file(db); + let base_dir = { + let path = db.file_relative_path(file_id); + path.parent().unwrap_or(&empty_path).join(&self.path) + }; + + let mut candidate_files = Vec::new(); + match attr_path { + Some(attr) => { + let base = if self.root_non_dir_owner { + base_dir.parent().unwrap_or(&empty_path) + } else { + &base_dir + }; + candidate_files.push(base.join(&*normalize_attribute_path(attr))) } None => { - let path = dir_path.join(format!("{}/{}.rs", parent_module.name, name)); - ResolutionMode::InsideInlineModule(InsideInlineModuleMode::File(path)) + candidate_files.push(base_dir.join(&format!("{}.rs", name))); + candidate_files.push(base_dir.join(&format!("{}/mod.rs", name))); } - }, - (Some(file_path), None) => { - let file_path = normalize_attribute_path(file_path); - let path = dir_path.join(file_path.as_ref()).normalize(); - ResolutionMode::OutOfLine(OutOfLineMode::WithAttributePath(path)) - } - (None, None) => { - let is_dir_owner = is_root || mod_name == "mod" || mod_attr_path.is_some(); - if is_dir_owner { - let file_mod = dir_path.join(format!("{}.rs", name)); - let dir_mod = dir_path.join(format!("{}/mod.rs", name)); - ResolutionMode::OutOfLine(OutOfLineMode::RootOrModRs { - file: file_mod, - directory: dir_mod, - }) - } else { - let path = dir_path.join(format!("{}/{}.rs", mod_name, name)); - ResolutionMode::OutOfLine(OutOfLineMode::FileInDirectory(path)) + }; + + let source_root_id = db.file_source_root(file_id); + let source_root = db.source_root(source_root_id); + for candidate in candidate_files.iter() { + let candidate = candidate.normalize(); + if let Some(file_id) = source_root.file_by_relative_path(&candidate) { + let mut root_non_dir_owner = false; + let mut mod_path = RelativePathBuf::new(); + if !(candidate.ends_with("mod.rs") || attr_path.is_some()) { + root_non_dir_owner = true; + mod_path.push(&name.to_string()); + } + return Ok((file_id, ModDir { path: mod_path, root_non_dir_owner })); } } - }; - - resolve_mode.resolve(db.source_root(source_root_id)) + let suggestion = candidate_files.first().unwrap(); + Err(base_dir.join(suggestion)) + } } -fn normalize_attribute_path(file_path: &SmolStr) -> Cow { +fn normalize_attribute_path(file_path: &str) -> Cow { let current_dir = "./"; let windows_path_separator = r#"\"#; let current_dir_normalize = if file_path.starts_with(current_dir) { &file_path[current_dir.len()..] } else { - file_path.as_str() + file_path }; if current_dir_normalize.contains(windows_path_separator) { Cow::Owned(current_dir_normalize.replace(windows_path_separator, "/")) @@ -113,75 +107,3 @@ fn normalize_attribute_path(file_path: &SmolStr) -> Cow { Cow::Borrowed(current_dir_normalize) } } - -enum OutOfLineMode { - RootOrModRs { file: RelativePathBuf, directory: RelativePathBuf }, - FileInDirectory(RelativePathBuf), - WithAttributePath(RelativePathBuf), -} - -impl OutOfLineMode { - pub fn resolve(&self, source_root: Arc) -> Result { - match self { - OutOfLineMode::RootOrModRs { file, directory } => { - match source_root.file_by_relative_path(file) { - None => resolve_simple_path(source_root, directory).map_err(|_| file.clone()), - file_id => resolve_find_result(file_id, file), - } - } - OutOfLineMode::FileInDirectory(path) => resolve_simple_path(source_root, path), - OutOfLineMode::WithAttributePath(path) => resolve_simple_path(source_root, path), - } - } -} - -enum InsideInlineModuleMode { - File(RelativePathBuf), - WithAttributePath(RelativePathBuf), -} - -impl InsideInlineModuleMode { - pub fn resolve(&self, source_root: Arc) -> Result { - match self { - InsideInlineModuleMode::File(path) => resolve_simple_path(source_root, path), - InsideInlineModuleMode::WithAttributePath(path) => { - resolve_simple_path(source_root, path) - } - } - } -} - -enum ResolutionMode { - OutOfLine(OutOfLineMode), - InsideInlineModule(InsideInlineModuleMode), - InlineModuleWithAttributePath(InsideInlineModuleMode), -} - -impl ResolutionMode { - pub fn resolve(&self, source_root: Arc) -> Result { - use self::ResolutionMode::*; - - match self { - OutOfLine(mode) => mode.resolve(source_root), - InsideInlineModule(mode) => mode.resolve(source_root), - InlineModuleWithAttributePath(mode) => mode.resolve(source_root), - } - } -} - -fn resolve_simple_path( - source_root: Arc, - path: &RelativePathBuf, -) -> Result { - resolve_find_result(source_root.file_by_relative_path(path), path) -} - -fn resolve_find_result( - file_id: Option, - path: &RelativePathBuf, -) -> Result { - match file_id { - Some(file_id) => Ok(file_id.clone()), - None => Err(path.clone()), - } -} diff --git a/crates/ra_hir/src/nameres/tests/macros.rs b/crates/ra_hir/src/nameres/tests/macros.rs index e4b408394..4f52ad2c5 100644 --- a/crates/ra_hir/src/nameres/tests/macros.rs +++ b/crates/ra_hir/src/nameres/tests/macros.rs @@ -38,21 +38,34 @@ fn macro_rules_can_define_modules() { } m!(n1); + mod m { + m!(n3) + } + //- /n1.rs m!(n2) //- /n1/n2.rs struct X; + //- /m/n3.rs + struct Y; ", ); assert_snapshot!(map, @r###" - ⋮crate - ⋮n1: t - ⋮ - ⋮crate::n1 - ⋮n2: t - ⋮ - ⋮crate::n1::n2 - ⋮X: t v + crate + m: t + n1: t + + crate::m + n3: t + + crate::m::n3 + Y: t v + + crate::n1 + n2: t + + crate::n1::n2 + X: t v "###); } diff --git a/crates/ra_hir/src/nameres/tests/mod_resolution.rs b/crates/ra_hir/src/nameres/tests/mod_resolution.rs index e3e6f1e95..755f5723b 100644 --- a/crates/ra_hir/src/nameres/tests/mod_resolution.rs +++ b/crates/ra_hir/src/nameres/tests/mod_resolution.rs @@ -25,6 +25,33 @@ fn name_res_works_for_broken_modules() { "###); } +#[test] +fn nested_module_resolution() { + let map = def_map( + " + //- /lib.rs + mod n1; + + //- /n1.rs + mod n2; + + //- /n1/n2.rs + struct X; + ", + ); + + assert_snapshot!(map, @r###" + ⋮crate + ⋮n1: t + ⋮ + ⋮crate::n1 + ⋮n2: t + ⋮ + ⋮crate::n1::n2 + ⋮X: t v + "###); +} + #[test] fn module_resolution_works_for_non_standard_filenames() { let map = def_map_with_crate_graph( @@ -467,7 +494,7 @@ fn module_resolution_decl_inside_inline_module_empty_path() { mod bar; } - //- /foo/users.rs + //- /users.rs pub struct Baz; "###, crate_graph! { @@ -492,7 +519,7 @@ fn module_resolution_decl_empty_path() { let map = def_map_with_crate_graph( r###" //- /main.rs - #[path = ""] + #[path = ""] // Should try to read `/` (a directory) mod foo; //- /foo.rs @@ -505,10 +532,6 @@ fn module_resolution_decl_empty_path() { assert_snapshot!(map, @r###" ⋮crate - ⋮foo: t - ⋮ - ⋮crate::foo - ⋮Baz: t v "###); } @@ -626,7 +649,7 @@ fn module_resolution_decl_inside_inline_module_in_non_crate_root() { } use self::bar::baz::Baz; - //- /bar/qwe.rs + //- /foo/bar/qwe.rs pub struct Baz; "###, crate_graph! { @@ -737,3 +760,66 @@ fn module_resolution_decl_inside_module_in_non_crate_root_2() { ⋮Baz: t v "###); } + +#[test] +fn nested_out_of_line_module() { + let map = def_map( + r###" + //- /lib.rs + mod a { + mod b { + mod c; + } + } + + //- /a/b/c.rs + struct X; + "###, + ); + + assert_snapshot!(map, @r###" + crate + a: t + + crate::a + b: t + + crate::a::b + c: t + + crate::a::b::c + X: t v + "###); +} + +#[test] +fn nested_out_of_line_module_with_path() { + let map = def_map( + r###" + //- /lib.rs + mod a { + #[path = "d/e"] + mod b { + mod c; + } + } + + //- /a/d/e/c.rs + struct X; + "###, + ); + + assert_snapshot!(map, @r###" + crate + a: t + + crate::a + b: t + + crate::a::b + c: t + + crate::a::b::c + X: t v + "###); +} -- cgit v1.2.3