From ce0d4a3b1deaca318a1ed2ff83cca652ef8e8c82 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov <aleksey.kladov@gmail.com>
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(-)
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<CrateModuleId, Vec<(CrateModuleId, raw::ImportId)>>,
unresolved_imports: Vec<(CrateModuleId, raw::ImportId, raw::ImportData)>,
unexpanded_macros: Vec<(CrateModuleId, AstId<ast::MacroCall>, Path)>,
+ mod_dirs: FxHashMap<CrateModuleId, ModDir>,
/// 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<ParentModule<'a>>,
+ mod_dir: ModDir,
}
impl<DB> 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<ParentModule<'_>>,
-) -> Result<FileId, RelativePathBuf> {
- 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<str> {
+fn normalize_attribute_path(file_path: &str) -> Cow<str> {
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<str> {
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<SourceRoot>) -> Result<FileId, RelativePathBuf> {
- 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<SourceRoot>) -> Result<FileId, RelativePathBuf> {
- 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<SourceRoot>) -> Result<FileId, RelativePathBuf> {
- 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<SourceRoot>,
- path: &RelativePathBuf,
-) -> Result<FileId, RelativePathBuf> {
- resolve_find_result(source_root.file_by_relative_path(path), path)
-}
-
-fn resolve_find_result(
- file_id: Option<FileId>,
- path: &RelativePathBuf,
-) -> Result<FileId, RelativePathBuf> {
- 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