From 4bed588001a1d6cd5c83a3eefc6ef77c439de40b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 28 Aug 2020 21:28:30 +0300 Subject: First steps for mod<|> completion --- crates/base_db/src/lib.rs | 16 +++++++++++++-- crates/ide/src/completion/completion_context.rs | 25 +++++++++++++++++++++-- crates/ide_db/src/lib.rs | 3 +++ crates/vfs/src/file_set.rs | 27 +++++++++++++++++++++++-- 4 files changed, 65 insertions(+), 6 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index ee3415850..71e85c6ac 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -96,6 +96,7 @@ pub trait FileLoader { /// `#[path = "C://no/way"]` fn resolve_path(&self, anchor: FileId, path: &str) -> Option; fn relevant_crates(&self, file_id: FileId) -> Arc>; + fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)>; } /// Database which stores all significant input facts: source code and project @@ -155,8 +156,8 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { } fn resolve_path(&self, anchor: FileId, path: &str) -> Option { // FIXME: this *somehow* should be platform agnostic... - let source_root = self.0.file_source_root(anchor); - let source_root = self.0.source_root(source_root); + // self.source_root(anchor) + let source_root = self.source_root(anchor); source_root.file_set.resolve_path(anchor, path) } @@ -164,4 +165,15 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { let source_root = self.0.file_source_root(file_id); self.0.source_root_crates(source_root) } + + fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)> { + self.source_root(anchor).file_set.list_some_random_files_todo(anchor) + } +} + +impl FileLoaderDelegate<&'_ T> { + fn source_root(&self, anchor: FileId) -> Arc { + let source_root = self.0.file_source_root(anchor); + self.0.source_root(source_root) + } } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index 47355d5dc..4d8b3670b 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here -use base_db::SourceDatabase; -use hir::{Semantics, SemanticsScope, Type}; +use base_db::{FileLoader, SourceDatabase}; +use hir::{ModuleSource, Semantics, SemanticsScope, Type}; use ide_db::RootDatabase; use syntax::{ algo::{find_covering_element, find_node_at_offset}, @@ -112,6 +112,27 @@ impl<'a> CompletionContext<'a> { }; let fake_ident_token = file_with_fake_ident.syntax().token_at_offset(position.offset).right_biased().unwrap(); + { + let module_names_for_import = sema + .to_module_def(position.file_id) + .and_then(|current_module| { + let definition_source = current_module.definition_source(db); + if !matches!(definition_source.value, ModuleSource::SourceFile(_)) { + return None; + } + let definition_source_file = definition_source.file_id.original_file(db); + + // TODO kb for all possible candidates + let zz = db.list_some_random_files_todo(definition_source_file); + dbg!(zz); + // TODO kb exlude existing children from the candidates + let existing_children = current_module.children(db).collect::>(); + dbg!(existing_children); + None::> + }) + .unwrap_or_default(); + dbg!(module_names_for_import); + }; let krate = sema.to_module_def(position.file_id).map(|m| m.krate()); let original_token = diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index 70ada02f3..f3197cc39 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -74,6 +74,9 @@ impl FileLoader for RootDatabase { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } + fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)> { + FileLoaderDelegate(self).list_some_random_files_todo(anchor) + } } impl salsa::Database for RootDatabase { diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index e9196fcd2..8bce17bc0 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -23,8 +23,31 @@ impl FileSet { let mut base = self.paths[&anchor].clone(); base.pop(); let path = base.join(path)?; - let res = self.files.get(&path).copied(); - res + self.files.get(&path).copied() + } + pub fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)> { + let anchor_path = self.paths[&anchor].clone(); + /* + [crates/vfs/src/file_set.rs:30] anchor_path = "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/lib.rs" + [crates/vfs/src/file_set.rs:31] self.files.keys() = [ + "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1/test_mod_2/test_mod_3.rs", + "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1/test_mod_2.rs", + "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1.rs", + "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/lib.rs", + "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_3/test_mod_3_1.rs", + "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_3.rs", + ] + */ + + // TODO kb determine the ways to list all applicable files + // Maybe leave list directory here only and the move the rest of the logic into the database impl? + + // Need to get the following things: + // * name of the anchor_path file (file_name, validate that it's a file!) + // * list of all files in the file's contai/ning directory (file_dir) + // * list of all files in `file_dir/file_name` or just `file_dir/`, for lib.rs or mod.rs + // * consider special case for /src/bin/foo.rs as a mod<|> source + Vec::new() } pub fn insert(&mut self, file_id: FileId, path: VfsPath) { self.files.insert(path.clone(), file_id); -- cgit v1.2.3 From 17870a3e2c39770a99f9ab5ce090abbe1dc334d2 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 3 Sep 2020 23:18:23 +0300 Subject: Better API --- crates/base_db/src/lib.rs | 23 +++++++++++++++++--- crates/hir_def/src/test_db.rs | 3 +++ crates/hir_expand/src/test_db.rs | 3 +++ crates/hir_ty/src/test_db.rs | 3 +++ crates/ide/src/completion/completion_context.rs | 9 ++++---- crates/ide_db/src/lib.rs | 4 ++-- crates/vfs/src/file_set.rs | 28 ++++++++++++------------- crates/vfs/src/vfs_path.rs | 15 +++++++++++++ 8 files changed, 64 insertions(+), 24 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 71e85c6ac..37a8432bd 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -96,7 +96,7 @@ pub trait FileLoader { /// `#[path = "C://no/way"]` fn resolve_path(&self, anchor: FileId, path: &str) -> Option; fn relevant_crates(&self, file_id: FileId) -> Arc>; - fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)>; + fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)>; } /// Database which stores all significant input facts: source code and project @@ -166,8 +166,25 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { self.0.source_root_crates(source_root) } - fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)> { - self.source_root(anchor).file_set.list_some_random_files_todo(anchor) + fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { + fn possible_sudmobules_opt( + module_files: &FileSet, + module_file: FileId, + ) -> Option> { + // TODO kb resolve path thinks that the input is a file... + let directory_with_module_file = module_files.resolve_path(module_file, "/../")?; + let directory_with_applicable_modules = + match module_files.file_name_and_extension(module_file)? { + ("mod", "rs") | ("lib", "rs") => Some(directory_with_module_file), + (directory_with_module_name, "rs") => module_files + .resolve_path(directory_with_module_file, directory_with_module_name), + _ => None, + }?; + Some(module_files.list_files(directory_with_applicable_modules)) + } + + possible_sudmobules_opt(&self.source_root(module_file).file_set, module_file) + .unwrap_or_default() } } diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index 42a762936..a35ed4f3c 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -63,6 +63,9 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } + fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { + FileLoaderDelegate(self).possible_sudmobules(module_file) + } } impl TestDB { diff --git a/crates/hir_expand/src/test_db.rs b/crates/hir_expand/src/test_db.rs index 86a5d867e..a0d1525b0 100644 --- a/crates/hir_expand/src/test_db.rs +++ b/crates/hir_expand/src/test_db.rs @@ -46,4 +46,7 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } + fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { + FileLoaderDelegate(self).possible_sudmobules(module_file) + } } diff --git a/crates/hir_ty/src/test_db.rs b/crates/hir_ty/src/test_db.rs index 15b8435e9..6f61e7dfe 100644 --- a/crates/hir_ty/src/test_db.rs +++ b/crates/hir_ty/src/test_db.rs @@ -73,6 +73,9 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } + fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { + FileLoaderDelegate(self).possible_sudmobules(module_file) + } } impl TestDB { diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index 4d8b3670b..cbfc77a46 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -120,11 +120,10 @@ impl<'a> CompletionContext<'a> { if !matches!(definition_source.value, ModuleSource::SourceFile(_)) { return None; } - let definition_source_file = definition_source.file_id.original_file(db); - - // TODO kb for all possible candidates - let zz = db.list_some_random_files_todo(definition_source_file); - dbg!(zz); + let module_definition_source_file = definition_source.file_id.original_file(db); + let mod_declaration_candidates = + db.possible_sudmobules(module_definition_source_file); + dbg!(mod_declaration_candidates); // TODO kb exlude existing children from the candidates let existing_children = current_module.children(db).collect::>(); dbg!(existing_children); diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index f3197cc39..dc1d2b9fe 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -74,8 +74,8 @@ impl FileLoader for RootDatabase { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)> { - FileLoaderDelegate(self).list_some_random_files_todo(anchor) + fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { + FileLoaderDelegate(self).possible_sudmobules(module_file) } } diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 8bce17bc0..0caddc3bc 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -20,15 +20,24 @@ impl FileSet { self.files.len() } pub fn resolve_path(&self, anchor: FileId, path: &str) -> Option { - let mut base = self.paths[&anchor].clone(); + let mut base = dbg!(self.paths[&anchor].clone()); base.pop(); - let path = base.join(path)?; + let path = dbg!(base).join(dbg!(path))?; self.files.get(&path).copied() } - pub fn list_some_random_files_todo(&self, anchor: FileId) -> Vec<(FileId, String)> { - let anchor_path = self.paths[&anchor].clone(); + + pub fn file_name_and_extension(&self, file: FileId) -> Option<(&str, &str)> { + self.paths[&file].file_name_and_extension() + } + + pub fn list_files(&self, directory: FileId) -> Vec<(FileId, String)> { + // TODO kb determine the ways to list all applicable files + // Maybe leave list directory here only and the move the rest of the logic into the database impl? + // cache results in Salsa? + + dbg!(directory); /* - [crates/vfs/src/file_set.rs:30] anchor_path = "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/lib.rs" + [crates/vfs/src/file_set.rs:30] directory = "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/" [crates/vfs/src/file_set.rs:31] self.files.keys() = [ "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1/test_mod_2/test_mod_3.rs", "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1/test_mod_2.rs", @@ -38,15 +47,6 @@ impl FileSet { "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_3.rs", ] */ - - // TODO kb determine the ways to list all applicable files - // Maybe leave list directory here only and the move the rest of the logic into the database impl? - - // Need to get the following things: - // * name of the anchor_path file (file_name, validate that it's a file!) - // * list of all files in the file's contai/ning directory (file_dir) - // * list of all files in `file_dir/file_name` or just `file_dir/`, for lib.rs or mod.rs - // * consider special case for /src/bin/foo.rs as a mod<|> source Vec::new() } pub fn insert(&mut self, file_id: FileId, path: VfsPath) { diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index 944a702df..7b965bb4c 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -49,6 +49,16 @@ impl VfsPath { } } + pub fn file_name_and_extension(&self) -> Option<(&str, &str)> { + match &self.0 { + VfsPathRepr::PathBuf(p) => p + .file_stem() + .zip(p.extension()) + .and_then(|(name, extension)| Some((name.to_str()?, extension.to_str()?))), + VfsPathRepr::VirtualPath(p) => p.file_name_and_extension(), + } + } + // Don't make this `pub` pub(crate) fn encode(&self, buf: &mut Vec) { let tag = match &self.0 { @@ -268,4 +278,9 @@ impl VirtualPath { res.0 = format!("{}/{}", res.0, path); Some(res) } + + pub fn file_name_and_extension(&self) -> Option<(&str, &str)> { + // TODO kb check if is a file + Some(("test_mod_1", "rs")) + } } -- cgit v1.2.3 From 0de71f7bc9482c9d1ef7e9d36ec5d6c5fd378781 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 4 Sep 2020 01:32:06 +0300 Subject: Properly use FileSet API --- crates/base_db/src/lib.rs | 18 ++++++++---------- crates/vfs/src/file_set.rs | 46 +++++++++++++++++++++++++--------------------- crates/vfs/src/vfs_path.rs | 23 ++++++++++++++++------- 3 files changed, 49 insertions(+), 38 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 37a8432bd..1bc4690c9 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -171,16 +171,14 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { module_files: &FileSet, module_file: FileId, ) -> Option> { - // TODO kb resolve path thinks that the input is a file... - let directory_with_module_file = module_files.resolve_path(module_file, "/../")?; - let directory_with_applicable_modules = - match module_files.file_name_and_extension(module_file)? { - ("mod", "rs") | ("lib", "rs") => Some(directory_with_module_file), - (directory_with_module_name, "rs") => module_files - .resolve_path(directory_with_module_file, directory_with_module_name), - _ => None, - }?; - Some(module_files.list_files(directory_with_applicable_modules)) + match module_files.file_name_and_extension(module_file)? { + ("mod", Some("rs")) | ("lib", Some("rs")) => { + module_files.list_files(module_file, None) + } + (directory_with_module_name, Some("rs")) => module_files + .list_files(module_file, Some(&format!("../{}/", directory_with_module_name))), + _ => None, + } } possible_sudmobules_opt(&self.source_root(module_file).file_set, module_file) diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 0caddc3bc..3f49f31e5 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -20,34 +20,38 @@ impl FileSet { self.files.len() } pub fn resolve_path(&self, anchor: FileId, path: &str) -> Option { - let mut base = dbg!(self.paths[&anchor].clone()); + let mut base = self.paths[&anchor].clone(); base.pop(); - let path = dbg!(base).join(dbg!(path))?; + let path = base.join(path)?; self.files.get(&path).copied() } - pub fn file_name_and_extension(&self, file: FileId) -> Option<(&str, &str)> { + pub fn file_name_and_extension(&self, file: FileId) -> Option<(&str, Option<&str>)> { self.paths[&file].file_name_and_extension() } - pub fn list_files(&self, directory: FileId) -> Vec<(FileId, String)> { - // TODO kb determine the ways to list all applicable files - // Maybe leave list directory here only and the move the rest of the logic into the database impl? - // cache results in Salsa? - - dbg!(directory); - /* - [crates/vfs/src/file_set.rs:30] directory = "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/" - [crates/vfs/src/file_set.rs:31] self.files.keys() = [ - "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1/test_mod_2/test_mod_3.rs", - "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1/test_mod_2.rs", - "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_1.rs", - "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/lib.rs", - "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_3/test_mod_3_1.rs", - "/Users/someonetoignore/Downloads/tmp_dir/zzzz/src/test_mod_3.rs", - ] - */ - Vec::new() + pub fn list_files( + &self, + anchor: FileId, + anchor_relative_path: Option<&str>, + ) -> Option> { + let anchor_directory = { + let path = self.paths[&anchor].clone(); + match anchor_relative_path { + Some(anchor_relative_path) => path.join(anchor_relative_path), + None => path.join("../"), + } + }?; + + Some( + self.paths + .iter() + .filter(|(_, path)| path.starts_with(&anchor_directory)) + // TODO kb need to ensure that no / exists after the anchor_directory + .filter(|(_, path)| path.ends_with(".rs")) + .map(|(&file_id, path)| (file_id, path.to_string())) + .collect(), + ) } pub fn insert(&mut self, file_id: FileId, path: VfsPath) { self.files.insert(path.clone(), file_id); diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index 7b965bb4c..f2d07038b 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -48,13 +48,19 @@ impl VfsPath { (VfsPathRepr::VirtualPath(_), _) => false, } } + pub fn ends_with(&self, suffix: &str) -> bool { + match &self.0 { + VfsPathRepr::PathBuf(p) => p.ends_with(suffix), + VfsPathRepr::VirtualPath(p) => p.ends_with(suffix), + } + } - pub fn file_name_and_extension(&self) -> Option<(&str, &str)> { + pub fn file_name_and_extension(&self) -> Option<(&str, Option<&str>)> { match &self.0 { - VfsPathRepr::PathBuf(p) => p - .file_stem() - .zip(p.extension()) - .and_then(|(name, extension)| Some((name.to_str()?, extension.to_str()?))), + VfsPathRepr::PathBuf(p) => Some(( + p.file_stem()?.to_str()?, + p.extension().and_then(|extension| extension.to_str()), + )), VfsPathRepr::VirtualPath(p) => p.file_name_and_extension(), } } @@ -259,6 +265,9 @@ impl VirtualPath { fn starts_with(&self, other: &VirtualPath) -> bool { self.0.starts_with(&other.0) } + fn ends_with(&self, suffix: &str) -> bool { + self.0.ends_with(suffix) + } fn pop(&mut self) -> bool { let pos = match self.0.rfind('/') { Some(pos) => pos, @@ -279,8 +288,8 @@ impl VirtualPath { Some(res) } - pub fn file_name_and_extension(&self) -> Option<(&str, &str)> { + pub fn file_name_and_extension(&self) -> Option<(&str, Option<&str>)> { // TODO kb check if is a file - Some(("test_mod_1", "rs")) + Some(("test_mod_1", Some("rs"))) } } -- cgit v1.2.3 From 8aa740dab46f138cacdf6391d46c87d6df810161 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 4 Sep 2020 02:25:00 +0300 Subject: Happy path implemented --- crates/base_db/src/lib.rs | 16 ++++++++++++---- crates/hir_def/src/test_db.rs | 4 ++-- crates/hir_expand/src/test_db.rs | 4 ++-- crates/hir_ty/src/test_db.rs | 4 ++-- crates/ide/src/completion/completion_context.rs | 2 +- crates/ide_db/src/lib.rs | 4 ++-- crates/vfs/src/file_set.rs | 17 +++++++++++------ crates/vfs/src/vfs_path.rs | 13 ++++++------- 8 files changed, 38 insertions(+), 26 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 1bc4690c9..3e0b6637d 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -96,7 +96,7 @@ pub trait FileLoader { /// `#[path = "C://no/way"]` fn resolve_path(&self, anchor: FileId, path: &str) -> Option; fn relevant_crates(&self, file_id: FileId) -> Arc>; - fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)>; + fn possible_sudmobule_names(&self, module_file: FileId) -> Vec; } /// Database which stores all significant input facts: source code and project @@ -166,11 +166,11 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { self.0.source_root_crates(source_root) } - fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { + fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { fn possible_sudmobules_opt( module_files: &FileSet, module_file: FileId, - ) -> Option> { + ) -> Option> { match module_files.file_name_and_extension(module_file)? { ("mod", Some("rs")) | ("lib", Some("rs")) => { module_files.list_files(module_file, None) @@ -181,8 +181,16 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { } } - possible_sudmobules_opt(&self.source_root(module_file).file_set, module_file) + let module_files = &self.source_root(module_file).file_set; + possible_sudmobules_opt(module_files, module_file) .unwrap_or_default() + .into_iter() + .filter_map(|submodule_file| module_files.file_name_and_extension(submodule_file)) + .map(|(file_name, extension)| match extension { + Some(extension) => format!("{}.{}", file_name, extension), + None => file_name.to_owned(), + }) + .collect() } } diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index a35ed4f3c..5bcfaf464 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -63,8 +63,8 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { - FileLoaderDelegate(self).possible_sudmobules(module_file) + fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { + FileLoaderDelegate(self).possible_sudmobule_names(module_file) } } diff --git a/crates/hir_expand/src/test_db.rs b/crates/hir_expand/src/test_db.rs index a0d1525b0..cf42dde7a 100644 --- a/crates/hir_expand/src/test_db.rs +++ b/crates/hir_expand/src/test_db.rs @@ -46,7 +46,7 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { - FileLoaderDelegate(self).possible_sudmobules(module_file) + fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { + FileLoaderDelegate(self).possible_sudmobule_names(module_file) } } diff --git a/crates/hir_ty/src/test_db.rs b/crates/hir_ty/src/test_db.rs index 6f61e7dfe..0696f41dd 100644 --- a/crates/hir_ty/src/test_db.rs +++ b/crates/hir_ty/src/test_db.rs @@ -73,8 +73,8 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { - FileLoaderDelegate(self).possible_sudmobules(module_file) + fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { + FileLoaderDelegate(self).possible_sudmobule_names(module_file) } } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index cbfc77a46..b4c6eeb35 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -122,7 +122,7 @@ impl<'a> CompletionContext<'a> { } let module_definition_source_file = definition_source.file_id.original_file(db); let mod_declaration_candidates = - db.possible_sudmobules(module_definition_source_file); + db.possible_sudmobule_names(module_definition_source_file); dbg!(mod_declaration_candidates); // TODO kb exlude existing children from the candidates let existing_children = current_module.children(db).collect::>(); diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index dc1d2b9fe..9f3be8601 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -74,8 +74,8 @@ impl FileLoader for RootDatabase { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobules(&self, module_file: FileId) -> Vec<(FileId, String)> { - FileLoaderDelegate(self).possible_sudmobules(module_file) + fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { + FileLoaderDelegate(self).possible_sudmobule_names(module_file) } } diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 3f49f31e5..956cffb29 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -34,22 +34,27 @@ impl FileSet { &self, anchor: FileId, anchor_relative_path: Option<&str>, - ) -> Option> { + ) -> Option> { let anchor_directory = { let path = self.paths[&anchor].clone(); match anchor_relative_path { Some(anchor_relative_path) => path.join(anchor_relative_path), - None => path.join("../"), + None => path.parent(), } }?; Some( self.paths .iter() - .filter(|(_, path)| path.starts_with(&anchor_directory)) - // TODO kb need to ensure that no / exists after the anchor_directory - .filter(|(_, path)| path.ends_with(".rs")) - .map(|(&file_id, path)| (file_id, path.to_string())) + .filter_map(|(&file_id, path)| { + if path.parent()? == anchor_directory + && matches!(path.file_name_and_extension(), Some((_, Some("rs")))) + { + Some(file_id) + } else { + None + } + }) .collect(), ) } diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index f2d07038b..9a3690a89 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -48,10 +48,12 @@ impl VfsPath { (VfsPathRepr::VirtualPath(_), _) => false, } } - pub fn ends_with(&self, suffix: &str) -> bool { - match &self.0 { - VfsPathRepr::PathBuf(p) => p.ends_with(suffix), - VfsPathRepr::VirtualPath(p) => p.ends_with(suffix), + pub fn parent(&self) -> Option { + let mut parent = self.clone(); + if parent.pop() { + Some(parent) + } else { + None } } @@ -265,9 +267,6 @@ impl VirtualPath { fn starts_with(&self, other: &VirtualPath) -> bool { self.0.starts_with(&other.0) } - fn ends_with(&self, suffix: &str) -> bool { - self.0.ends_with(suffix) - } fn pop(&mut self) -> bool { let pos = match self.0.rfind('/') { Some(pos) => pos, -- cgit v1.2.3 From d163f9f114c8180bec6285ad9962fabbf3af5b18 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 4 Sep 2020 13:33:07 +0300 Subject: Small refactoring --- crates/base_db/src/lib.rs | 36 +++++++++++++++--------------------- crates/vfs/src/file_set.rs | 22 +++++++++++----------- 2 files changed, 26 insertions(+), 32 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 3e0b6637d..55ef9fc24 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -167,29 +167,23 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { } fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - fn possible_sudmobules_opt( - module_files: &FileSet, - module_file: FileId, - ) -> Option> { - match module_files.file_name_and_extension(module_file)? { - ("mod", Some("rs")) | ("lib", Some("rs")) => { - module_files.list_files(module_file, None) - } - (directory_with_module_name, Some("rs")) => module_files - .list_files(module_file, Some(&format!("../{}/", directory_with_module_name))), - _ => None, - } - } - let module_files = &self.source_root(module_file).file_set; - possible_sudmobules_opt(module_files, module_file) - .unwrap_or_default() + let possible_submodule_files = match module_files.file_name_and_extension(module_file) { + Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) => { + module_files.list_files_with_extensions(module_file, None) + } + Some((directory_with_module_name, Some("rs"))) => module_files + .list_files_with_extensions( + module_file, + Some(&format!("../{}/", directory_with_module_name)), + ), + _ => Vec::new(), + }; + + possible_submodule_files .into_iter() - .filter_map(|submodule_file| module_files.file_name_and_extension(submodule_file)) - .map(|(file_name, extension)| match extension { - Some(extension) => format!("{}.{}", file_name, extension), - None => file_name.to_owned(), - }) + .filter(|(_, extension)| extension == &Some("rs")) + .map(|(file_name, _)| file_name.to_owned()) .collect() } } diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 956cffb29..3085fd818 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -30,33 +30,33 @@ impl FileSet { self.paths[&file].file_name_and_extension() } - pub fn list_files( + pub fn list_files_with_extensions( &self, anchor: FileId, anchor_relative_path: Option<&str>, - ) -> Option> { + ) -> Vec<(&str, Option<&str>)> { let anchor_directory = { let path = self.paths[&anchor].clone(); match anchor_relative_path { Some(anchor_relative_path) => path.join(anchor_relative_path), None => path.parent(), } - }?; + }; - Some( + if let Some(anchor_directory) = anchor_directory { self.paths .iter() - .filter_map(|(&file_id, path)| { - if path.parent()? == anchor_directory - && matches!(path.file_name_and_extension(), Some((_, Some("rs")))) - { - Some(file_id) + .filter_map(|(_, path)| { + if path.parent()? == anchor_directory { + path.file_name_and_extension() } else { None } }) - .collect(), - ) + .collect() + } else { + Vec::new() + } } pub fn insert(&mut self, file_id: FileId, path: VfsPath) { self.files.insert(path.clone(), file_id); -- cgit v1.2.3 From 897a4c702e3d6fa9156ea0bc34af9d397fae3440 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 4 Sep 2020 15:05:56 +0300 Subject: Implement file name & extension retrieval method for VirtualPath --- crates/vfs/src/vfs_path.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'crates') diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index 9a3690a89..113c2e4e6 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -287,8 +287,26 @@ impl VirtualPath { Some(res) } + // FIXME: Currently VirtualPath does is unable to distinguish a directory from a file + // hence this method will return `Some("directory_name", None)` for a directory pub fn file_name_and_extension(&self) -> Option<(&str, Option<&str>)> { - // TODO kb check if is a file - Some(("test_mod_1", Some("rs"))) + let file_name = match self.0.rfind('/') { + Some(position) => &self.0[position + 1..], + None => &self.0, + }; + + if file_name.is_empty() { + None + } else { + let mut file_stem_and_extension = file_name.rsplitn(2, '.'); + let extension = file_stem_and_extension.next(); + let file_stem = file_stem_and_extension.next(); + + match (file_stem, extension) { + (None, None) => None, + (None, Some(_)) | (Some(""), Some(_)) => Some((file_name, None)), + (Some(file_stem), extension) => Some((file_stem, extension)), + } + } } } -- cgit v1.2.3 From 486c5c3285682408b125613475a34a0bc9a2c097 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 4 Sep 2020 15:13:31 +0300 Subject: Exclude special files --- crates/base_db/src/lib.rs | 5 +++++ crates/ide/src/completion/completion_context.rs | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 55ef9fc24..c72e254f4 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -172,17 +172,22 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) => { module_files.list_files_with_extensions(module_file, None) } + // TODO kb for `src/bin/foo.rs`, we need to check for modules in `src/bin/` Some((directory_with_module_name, Some("rs"))) => module_files .list_files_with_extensions( module_file, Some(&format!("../{}/", directory_with_module_name)), ), + // TODO kb also consider the case when there's no `../module_name.rs`, but `../module_name/mod.rs` _ => Vec::new(), }; possible_submodule_files .into_iter() .filter(|(_, extension)| extension == &Some("rs")) + .filter(|(file_name, _)| file_name != &"mod") + .filter(|(file_name, _)| file_name != &"lib") + .filter(|(file_name, _)| file_name != &"main") .map(|(file_name, _)| file_name.to_owned()) .collect() } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index b4c6eeb35..74cd16e0a 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -117,9 +117,6 @@ impl<'a> CompletionContext<'a> { .to_module_def(position.file_id) .and_then(|current_module| { let definition_source = current_module.definition_source(db); - if !matches!(definition_source.value, ModuleSource::SourceFile(_)) { - return None; - } let module_definition_source_file = definition_source.file_id.original_file(db); let mod_declaration_candidates = db.possible_sudmobule_names(module_definition_source_file); -- cgit v1.2.3 From b2bcc5278db23c3ba0a4f47a3ef6ee411aaaa8dc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 6 Sep 2020 01:41:18 +0300 Subject: Properly handle special cases (binaries, mod.rs) --- crates/base_db/src/lib.rs | 24 +------ crates/ide/src/completion/completion_context.rs | 4 +- crates/vfs/src/file_set.rs | 84 +++++++++++++++++-------- 3 files changed, 60 insertions(+), 52 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index c72e254f4..030b96829 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -167,29 +167,7 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { } fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - let module_files = &self.source_root(module_file).file_set; - let possible_submodule_files = match module_files.file_name_and_extension(module_file) { - Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) => { - module_files.list_files_with_extensions(module_file, None) - } - // TODO kb for `src/bin/foo.rs`, we need to check for modules in `src/bin/` - Some((directory_with_module_name, Some("rs"))) => module_files - .list_files_with_extensions( - module_file, - Some(&format!("../{}/", directory_with_module_name)), - ), - // TODO kb also consider the case when there's no `../module_name.rs`, but `../module_name/mod.rs` - _ => Vec::new(), - }; - - possible_submodule_files - .into_iter() - .filter(|(_, extension)| extension == &Some("rs")) - .filter(|(file_name, _)| file_name != &"mod") - .filter(|(file_name, _)| file_name != &"lib") - .filter(|(file_name, _)| file_name != &"main") - .map(|(file_name, _)| file_name.to_owned()) - .collect() + self.source_root(module_file).file_set.possible_sudmobule_names(module_file) } } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index 74cd16e0a..a8fe44083 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here use base_db::{FileLoader, SourceDatabase}; -use hir::{ModuleSource, Semantics, SemanticsScope, Type}; +use hir::{Semantics, SemanticsScope, Type}; use ide_db::RootDatabase; use syntax::{ algo::{find_covering_element, find_node_at_offset}, @@ -123,11 +123,9 @@ impl<'a> CompletionContext<'a> { dbg!(mod_declaration_candidates); // TODO kb exlude existing children from the candidates let existing_children = current_module.children(db).collect::>(); - dbg!(existing_children); None::> }) .unwrap_or_default(); - dbg!(module_names_for_import); }; let krate = sema.to_module_def(position.file_id).map(|m| m.krate()); diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 3085fd818..cb65c17e0 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -26,38 +26,70 @@ impl FileSet { self.files.get(&path).copied() } - pub fn file_name_and_extension(&self, file: FileId) -> Option<(&str, Option<&str>)> { - self.paths[&file].file_name_and_extension() - } - - pub fn list_files_with_extensions( - &self, - anchor: FileId, - anchor_relative_path: Option<&str>, - ) -> Vec<(&str, Option<&str>)> { - let anchor_directory = { - let path = self.paths[&anchor].clone(); - match anchor_relative_path { - Some(anchor_relative_path) => path.join(anchor_relative_path), - None => path.parent(), - } + pub fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { + let directory_to_look_for_submodules = match self.get_directory_with_submodules(module_file) + { + Some(directory) => directory, + None => return Vec::new(), }; - - if let Some(anchor_directory) = anchor_directory { - self.paths - .iter() - .filter_map(|(_, path)| { - if path.parent()? == anchor_directory { - path.file_name_and_extension() + self.paths + .iter() + .filter_map(|(_, path)| { + if path.parent()? == directory_to_look_for_submodules { + path.file_name_and_extension() + } else { + None + } + }) + .filter_map(|file_name_and_extension| match file_name_and_extension { + // TODO kb do not include the module file name itself, if present + // TODO kb wrong resolution for nesСпаted non-file modules (mod tests {mod <|>) + // TODO kb in src/bin when a module is included into another, + // the included file gets "moved" into a directory below and now cannot add any other modules + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, + (file_name, Some("rs")) => Some(file_name.to_owned()), + (subdirectory_name, None) => { + let mod_rs_path = + directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; + if self.files.contains_key(&mod_rs_path) { + Some(subdirectory_name.to_owned()) } else { None } - }) - .collect() - } else { - Vec::new() + } + _ => None, + }) + .collect() + } + + fn get_directory_with_submodules(&self, module_file: FileId) -> Option { + let module_file_path = &self.paths[&module_file]; + let module_directory_path = module_file_path.parent()?; + match module_file_path.file_name_and_extension() { + Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) | Some(("main", Some("rs"))) => { + Some(module_directory_path) + } + Some((regular_rust_file_name, Some("rs"))) => { + if matches!( + ( + module_directory_path + .parent() + .as_ref() + .and_then(|path| path.file_name_and_extension()), + module_directory_path.file_name_and_extension(), + ), + (Some(("src", None)), Some(("bin", None))) + ) { + // files in /src/bin/ can import each other directly + Some(module_directory_path) + } else { + module_directory_path.join(regular_rust_file_name) + } + } + _ => None, } } + pub fn insert(&mut self, file_id: FileId, path: VfsPath) { self.files.insert(path.clone(), file_id); self.paths.insert(file_id, path); -- cgit v1.2.3 From 33179a0ae1ba9a908cc34a4cf87599ed779b9886 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 7 Sep 2020 16:17:50 +0300 Subject: Move rust-related logic from vfs to base_db level --- crates/base_db/src/lib.rs | 70 +++++++++++++++++++++++++++++++++++++++++++++- crates/vfs/src/file_set.rs | 65 ++++-------------------------------------- 2 files changed, 74 insertions(+), 61 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 030b96829..9733e1fd3 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -167,7 +167,7 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { } fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - self.source_root(module_file).file_set.possible_sudmobule_names(module_file) + possible_sudmobule_names(&self.source_root(module_file).file_set, module_file) } } @@ -177,3 +177,71 @@ impl FileLoaderDelegate<&'_ T> { self.0.source_root(source_root) } } + +fn possible_sudmobule_names(module_files: &FileSet, module_file: FileId) -> Vec { + let directory_to_look_for_submodules = match module_files + .path_for_file(&module_file) + .and_then(|module_file_path| get_directory_with_submodules(module_file_path)) + { + Some(directory) => directory, + None => return Vec::new(), + }; + module_files + .iter() + .filter(|submodule_file| submodule_file != &module_file) + .filter_map(|submodule_file| { + let submodule_path = module_files.path_for_file(&submodule_file)?; + if submodule_path.parent()? == directory_to_look_for_submodules { + submodule_path.file_name_and_extension() + } else { + None + } + }) + .filter_map(|file_name_and_extension| { + match file_name_and_extension { + // TODO kb wrong resolution for nested non-file modules (mod tests {mod <|>) + // TODO kb in src/bin when a module is included into another, + // the included file gets "moved" into a directory below and now cannot add any other modules + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, + (file_name, Some("rs")) => Some(file_name.to_owned()), + (subdirectory_name, None) => { + let mod_rs_path = + directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; + if module_files.file_for_path(&mod_rs_path).is_some() { + Some(subdirectory_name.to_owned()) + } else { + None + } + } + _ => None, + } + }) + .collect() +} + +fn get_directory_with_submodules(module_file_path: &VfsPath) -> Option { + let module_directory_path = module_file_path.parent()?; + match module_file_path.file_name_and_extension()? { + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { + Some(module_directory_path) + } + (regular_rust_file_name, Some("rs")) => { + if matches!( + ( + module_directory_path + .parent() + .as_ref() + .and_then(|path| path.file_name_and_extension()), + module_directory_path.file_name_and_extension(), + ), + (Some(("src", None)), Some(("bin", None))) + ) { + // files in /src/bin/ can import each other directly + Some(module_directory_path) + } else { + module_directory_path.join(regular_rust_file_name) + } + } + _ => None, + } +} diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index cb65c17e0..4aa2d6526 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -26,74 +26,19 @@ impl FileSet { self.files.get(&path).copied() } - pub fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - let directory_to_look_for_submodules = match self.get_directory_with_submodules(module_file) - { - Some(directory) => directory, - None => return Vec::new(), - }; - self.paths - .iter() - .filter_map(|(_, path)| { - if path.parent()? == directory_to_look_for_submodules { - path.file_name_and_extension() - } else { - None - } - }) - .filter_map(|file_name_and_extension| match file_name_and_extension { - // TODO kb do not include the module file name itself, if present - // TODO kb wrong resolution for nesСпаted non-file modules (mod tests {mod <|>) - // TODO kb in src/bin when a module is included into another, - // the included file gets "moved" into a directory below and now cannot add any other modules - ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, - (file_name, Some("rs")) => Some(file_name.to_owned()), - (subdirectory_name, None) => { - let mod_rs_path = - directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; - if self.files.contains_key(&mod_rs_path) { - Some(subdirectory_name.to_owned()) - } else { - None - } - } - _ => None, - }) - .collect() + pub fn file_for_path(&self, path: &VfsPath) -> Option<&FileId> { + self.files.get(path) } - fn get_directory_with_submodules(&self, module_file: FileId) -> Option { - let module_file_path = &self.paths[&module_file]; - let module_directory_path = module_file_path.parent()?; - match module_file_path.file_name_and_extension() { - Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) | Some(("main", Some("rs"))) => { - Some(module_directory_path) - } - Some((regular_rust_file_name, Some("rs"))) => { - if matches!( - ( - module_directory_path - .parent() - .as_ref() - .and_then(|path| path.file_name_and_extension()), - module_directory_path.file_name_and_extension(), - ), - (Some(("src", None)), Some(("bin", None))) - ) { - // files in /src/bin/ can import each other directly - Some(module_directory_path) - } else { - module_directory_path.join(regular_rust_file_name) - } - } - _ => None, - } + pub fn path_for_file(&self, file: &FileId) -> Option<&VfsPath> { + self.paths.get(file) } pub fn insert(&mut self, file_id: FileId, path: VfsPath) { self.files.insert(path.clone(), file_id); self.paths.insert(file_id, path); } + pub fn iter(&self) -> impl Iterator + '_ { self.paths.keys().copied() } -- cgit v1.2.3 From 6ba479cd058aa54a9f161085c7ff9ac1f12d8df3 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 7 Sep 2020 19:21:39 +0300 Subject: Finally cretae the mod completion module --- crates/base_db/src/lib.rs | 2 +- crates/ide/src/completion.rs | 2 ++ crates/ide/src/completion/complete_mod.rs | 39 +++++++++++++++++++++++++ crates/ide/src/completion/completion_context.rs | 18 +----------- 4 files changed, 43 insertions(+), 18 deletions(-) create mode 100644 crates/ide/src/completion/complete_mod.rs (limited to 'crates') diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 9733e1fd3..321007d33 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -199,7 +199,7 @@ fn possible_sudmobule_names(module_files: &FileSet, module_file: FileId) -> Vec< }) .filter_map(|file_name_and_extension| { match file_name_and_extension { - // TODO kb wrong resolution for nested non-file modules (mod tests {mod <|>) + // TODO kb wrong resolution for nested non-file modules (mod tests { mod <|> }) // TODO kb in src/bin when a module is included into another, // the included file gets "moved" into a directory below and now cannot add any other modules ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, diff --git a/crates/ide/src/completion.rs b/crates/ide/src/completion.rs index 33bed6991..daea2aa95 100644 --- a/crates/ide/src/completion.rs +++ b/crates/ide/src/completion.rs @@ -19,6 +19,7 @@ mod complete_unqualified_path; mod complete_postfix; mod complete_macro_in_item_position; mod complete_trait_impl; +mod complete_mod; use ide_db::RootDatabase; @@ -124,6 +125,7 @@ pub(crate) fn completions( complete_postfix::complete_postfix(&mut acc, &ctx); complete_macro_in_item_position::complete_macro_in_item_position(&mut acc, &ctx); complete_trait_impl::complete_trait_impl(&mut acc, &ctx); + complete_mod::complete_mod(&mut acc, &ctx); Some(acc) } diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs new file mode 100644 index 000000000..4c1e79603 --- /dev/null +++ b/crates/ide/src/completion/complete_mod.rs @@ -0,0 +1,39 @@ +//! Completes mod declarations. + +use base_db::FileLoader; +use hir::ModuleSource; + +use super::{completion_context::CompletionContext, completion_item::Completions}; + +/// Complete mod declaration, i.e. `mod <|> ;` +pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) { + let module_names_for_import = ctx + .sema + // TODO kb this is wrong, since we need not the file module + .to_module_def(ctx.position.file_id) + .and_then(|current_module| { + dbg!(current_module.name(ctx.db)); + dbg!(current_module.definition_source(ctx.db)); + dbg!(current_module.declaration_source(ctx.db)); + let mut zz = Vec::new(); + let mut vv = Some(current_module); + while let Some(ModuleSource::Module(_)) = + vv.map(|vv| vv.definition_source(ctx.db).value) + { + zz.push(current_module.name(ctx.db)); + vv = current_module.parent(ctx.db); + } + dbg!(zz); + let definition_source = current_module.definition_source(ctx.db); + // TODO kb filter out declarations in possible_sudmobule_names + // let declaration_source = current_module.declaration_source(ctx.db); + let module_definition_source_file = definition_source.file_id.original_file(ctx.db); + let mod_declaration_candidates = + ctx.db.possible_sudmobule_names(module_definition_source_file); + dbg!(mod_declaration_candidates); + // TODO kb exlude existing children from the candidates + let existing_children = current_module.children(ctx.db).collect::>(); + None::> + }) + .unwrap_or_default(); +} diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index a8fe44083..31886942a 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here use base_db::{FileLoader, SourceDatabase}; -use hir::{Semantics, SemanticsScope, Type}; +use hir::{ModuleSource, Semantics, SemanticsScope, Type}; use ide_db::RootDatabase; use syntax::{ algo::{find_covering_element, find_node_at_offset}, @@ -112,22 +112,6 @@ impl<'a> CompletionContext<'a> { }; let fake_ident_token = file_with_fake_ident.syntax().token_at_offset(position.offset).right_biased().unwrap(); - { - let module_names_for_import = sema - .to_module_def(position.file_id) - .and_then(|current_module| { - let definition_source = current_module.definition_source(db); - let module_definition_source_file = definition_source.file_id.original_file(db); - let mod_declaration_candidates = - db.possible_sudmobule_names(module_definition_source_file); - dbg!(mod_declaration_candidates); - // TODO kb exlude existing children from the candidates - let existing_children = current_module.children(db).collect::>(); - None::> - }) - .unwrap_or_default(); - }; - let krate = sema.to_module_def(position.file_id).map(|m| m.krate()); let original_token = original_file.syntax().token_at_offset(position.offset).left_biased()?; -- cgit v1.2.3 From f9c14ac7204c38633e70b3efd47a5b1f9056afd0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 7 Sep 2020 20:52:37 +0300 Subject: Move most of the logic into the completion module --- crates/base_db/src/input.rs | 8 +- crates/base_db/src/lib.rs | 84 +---------------- crates/hir_def/src/test_db.rs | 3 - crates/hir_expand/src/test_db.rs | 3 - crates/hir_ty/src/test_db.rs | 3 - crates/ide/src/completion/complete_mod.rs | 116 +++++++++++++++++++----- crates/ide/src/completion/completion_context.rs | 5 +- crates/ide_db/src/lib.rs | 3 - 8 files changed, 107 insertions(+), 118 deletions(-) (limited to 'crates') diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs index f3d65cdf0..9a61f1d56 100644 --- a/crates/base_db/src/input.rs +++ b/crates/base_db/src/input.rs @@ -12,7 +12,7 @@ use cfg::CfgOptions; use rustc_hash::{FxHashMap, FxHashSet}; use syntax::SmolStr; use tt::TokenExpander; -use vfs::file_set::FileSet; +use vfs::{file_set::FileSet, VfsPath}; pub use vfs::FileId; @@ -43,6 +43,12 @@ impl SourceRoot { pub fn new_library(file_set: FileSet) -> SourceRoot { SourceRoot { is_library: true, file_set } } + pub fn path_for_file(&self, file: &FileId) -> Option<&VfsPath> { + self.file_set.path_for_file(file) + } + pub fn file_for_path(&self, path: &VfsPath) -> Option<&FileId> { + self.file_set.file_for_path(path) + } pub fn iter(&self) -> impl Iterator + '_ { self.file_set.iter() } diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index 321007d33..ee3415850 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -96,7 +96,6 @@ pub trait FileLoader { /// `#[path = "C://no/way"]` fn resolve_path(&self, anchor: FileId, path: &str) -> Option; fn relevant_crates(&self, file_id: FileId) -> Arc>; - fn possible_sudmobule_names(&self, module_file: FileId) -> Vec; } /// Database which stores all significant input facts: source code and project @@ -156,8 +155,8 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { } fn resolve_path(&self, anchor: FileId, path: &str) -> Option { // FIXME: this *somehow* should be platform agnostic... - // self.source_root(anchor) - let source_root = self.source_root(anchor); + let source_root = self.0.file_source_root(anchor); + let source_root = self.0.source_root(source_root); source_root.file_set.resolve_path(anchor, path) } @@ -165,83 +164,4 @@ impl FileLoader for FileLoaderDelegate<&'_ T> { let source_root = self.0.file_source_root(file_id); self.0.source_root_crates(source_root) } - - fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - possible_sudmobule_names(&self.source_root(module_file).file_set, module_file) - } -} - -impl FileLoaderDelegate<&'_ T> { - fn source_root(&self, anchor: FileId) -> Arc { - let source_root = self.0.file_source_root(anchor); - self.0.source_root(source_root) - } -} - -fn possible_sudmobule_names(module_files: &FileSet, module_file: FileId) -> Vec { - let directory_to_look_for_submodules = match module_files - .path_for_file(&module_file) - .and_then(|module_file_path| get_directory_with_submodules(module_file_path)) - { - Some(directory) => directory, - None => return Vec::new(), - }; - module_files - .iter() - .filter(|submodule_file| submodule_file != &module_file) - .filter_map(|submodule_file| { - let submodule_path = module_files.path_for_file(&submodule_file)?; - if submodule_path.parent()? == directory_to_look_for_submodules { - submodule_path.file_name_and_extension() - } else { - None - } - }) - .filter_map(|file_name_and_extension| { - match file_name_and_extension { - // TODO kb wrong resolution for nested non-file modules (mod tests { mod <|> }) - // TODO kb in src/bin when a module is included into another, - // the included file gets "moved" into a directory below and now cannot add any other modules - ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, - (file_name, Some("rs")) => Some(file_name.to_owned()), - (subdirectory_name, None) => { - let mod_rs_path = - directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; - if module_files.file_for_path(&mod_rs_path).is_some() { - Some(subdirectory_name.to_owned()) - } else { - None - } - } - _ => None, - } - }) - .collect() -} - -fn get_directory_with_submodules(module_file_path: &VfsPath) -> Option { - let module_directory_path = module_file_path.parent()?; - match module_file_path.file_name_and_extension()? { - ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { - Some(module_directory_path) - } - (regular_rust_file_name, Some("rs")) => { - if matches!( - ( - module_directory_path - .parent() - .as_ref() - .and_then(|path| path.file_name_and_extension()), - module_directory_path.file_name_and_extension(), - ), - (Some(("src", None)), Some(("bin", None))) - ) { - // files in /src/bin/ can import each other directly - Some(module_directory_path) - } else { - module_directory_path.join(regular_rust_file_name) - } - } - _ => None, - } } diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index 5bcfaf464..42a762936 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -63,9 +63,6 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - FileLoaderDelegate(self).possible_sudmobule_names(module_file) - } } impl TestDB { diff --git a/crates/hir_expand/src/test_db.rs b/crates/hir_expand/src/test_db.rs index cf42dde7a..86a5d867e 100644 --- a/crates/hir_expand/src/test_db.rs +++ b/crates/hir_expand/src/test_db.rs @@ -46,7 +46,4 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - FileLoaderDelegate(self).possible_sudmobule_names(module_file) - } } diff --git a/crates/hir_ty/src/test_db.rs b/crates/hir_ty/src/test_db.rs index 0696f41dd..15b8435e9 100644 --- a/crates/hir_ty/src/test_db.rs +++ b/crates/hir_ty/src/test_db.rs @@ -73,9 +73,6 @@ impl FileLoader for TestDB { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - FileLoaderDelegate(self).possible_sudmobule_names(module_file) - } } impl TestDB { diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index 4c1e79603..da3d93bad 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -1,35 +1,61 @@ //! Completes mod declarations. -use base_db::FileLoader; -use hir::ModuleSource; +use base_db::{SourceDatabaseExt, VfsPath}; +use hir::{Module, ModuleSource}; +use ide_db::RootDatabase; use super::{completion_context::CompletionContext, completion_item::Completions}; /// Complete mod declaration, i.e. `mod <|> ;` pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) { let module_names_for_import = ctx - .sema - // TODO kb this is wrong, since we need not the file module - .to_module_def(ctx.position.file_id) + .scope + .module() .and_then(|current_module| { - dbg!(current_module.name(ctx.db)); - dbg!(current_module.definition_source(ctx.db)); - dbg!(current_module.declaration_source(ctx.db)); - let mut zz = Vec::new(); - let mut vv = Some(current_module); - while let Some(ModuleSource::Module(_)) = - vv.map(|vv| vv.definition_source(ctx.db).value) - { - zz.push(current_module.name(ctx.db)); - vv = current_module.parent(ctx.db); - } - dbg!(zz); - let definition_source = current_module.definition_source(ctx.db); + let module_path = path_to_closest_containing_module_file(current_module, ctx.db); // TODO kb filter out declarations in possible_sudmobule_names // let declaration_source = current_module.declaration_source(ctx.db); - let module_definition_source_file = definition_source.file_id.original_file(ctx.db); - let mod_declaration_candidates = - ctx.db.possible_sudmobule_names(module_definition_source_file); + let module_definition_source_file = + current_module.definition_source(ctx.db).file_id.original_file(ctx.db); + + let source_root_id = ctx.db.file_source_root(module_definition_source_file); + let source_root = ctx.db.source_root(source_root_id); + let directory_to_look_for_submodules = source_root + .path_for_file(&module_definition_source_file) + .and_then(|module_file_path| get_directory_with_submodules(module_file_path))?; + + let mod_declaration_candidates = source_root + .iter() + .filter(|submodule_file| submodule_file != &module_definition_source_file) + .filter_map(|submodule_file| { + let submodule_path = source_root.path_for_file(&submodule_file)?; + if submodule_path.parent()? == directory_to_look_for_submodules { + submodule_path.file_name_and_extension() + } else { + None + } + }) + .filter_map(|file_name_and_extension| { + match file_name_and_extension { + // TODO kb wrong resolution for nested non-file modules (mod tests { mod <|> }) + // TODO kb in src/bin when a module is included into another, + // the included file gets "moved" into a directory below and now cannot add any other modules + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, + (file_name, Some("rs")) => Some(file_name.to_owned()), + (subdirectory_name, None) => { + let mod_rs_path = directory_to_look_for_submodules + .join(subdirectory_name)? + .join("mod.rs")?; + if source_root.file_for_path(&mod_rs_path).is_some() { + Some(subdirectory_name.to_owned()) + } else { + None + } + } + _ => None, + } + }) + .collect::>(); dbg!(mod_declaration_candidates); // TODO kb exlude existing children from the candidates let existing_children = current_module.children(ctx.db).collect::>(); @@ -37,3 +63,51 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) { }) .unwrap_or_default(); } + +fn path_to_closest_containing_module_file( + current_module: Module, + db: &RootDatabase, +) -> Vec { + let mut path = Vec::new(); + + let mut current_module = Some(current_module); + while let Some(ModuleSource::Module(_)) = + current_module.map(|module| module.definition_source(db).value) + { + if let Some(module) = current_module { + path.insert(0, module); + current_module = module.parent(db); + } else { + current_module = None; + } + } + + path +} + +fn get_directory_with_submodules(module_file_path: &VfsPath) -> Option { + let module_directory_path = module_file_path.parent()?; + match module_file_path.file_name_and_extension()? { + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { + Some(module_directory_path) + } + (regular_rust_file_name, Some("rs")) => { + if matches!( + ( + module_directory_path + .parent() + .as_ref() + .and_then(|path| path.file_name_and_extension()), + module_directory_path.file_name_and_extension(), + ), + (Some(("src", None)), Some(("bin", None))) + ) { + // files in /src/bin/ can import each other directly + Some(module_directory_path) + } else { + module_directory_path.join(regular_rust_file_name) + } + } + _ => None, + } +} diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index 31886942a..47355d5dc 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -1,7 +1,7 @@ //! FIXME: write short doc here -use base_db::{FileLoader, SourceDatabase}; -use hir::{ModuleSource, Semantics, SemanticsScope, Type}; +use base_db::SourceDatabase; +use hir::{Semantics, SemanticsScope, Type}; use ide_db::RootDatabase; use syntax::{ algo::{find_covering_element, find_node_at_offset}, @@ -112,6 +112,7 @@ impl<'a> CompletionContext<'a> { }; let fake_ident_token = file_with_fake_ident.syntax().token_at_offset(position.offset).right_biased().unwrap(); + let krate = sema.to_module_def(position.file_id).map(|m| m.krate()); let original_token = original_file.syntax().token_at_offset(position.offset).left_biased()?; diff --git a/crates/ide_db/src/lib.rs b/crates/ide_db/src/lib.rs index 9f3be8601..70ada02f3 100644 --- a/crates/ide_db/src/lib.rs +++ b/crates/ide_db/src/lib.rs @@ -74,9 +74,6 @@ impl FileLoader for RootDatabase { fn relevant_crates(&self, file_id: FileId) -> Arc> { FileLoaderDelegate(self).relevant_crates(file_id) } - fn possible_sudmobule_names(&self, module_file: FileId) -> Vec { - FileLoaderDelegate(self).possible_sudmobule_names(module_file) - } } impl salsa::Database for RootDatabase { -- cgit v1.2.3 From 3fd6f451417fee0e8d95d06fb298c94b22bca917 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 7 Sep 2020 21:39:23 +0300 Subject: Properly handle nested submodules in the same file --- crates/ide/src/completion/complete_mod.rs | 149 ++++++++++++++++-------------- 1 file changed, 79 insertions(+), 70 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index da3d93bad..def8b8968 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -7,87 +7,66 @@ use ide_db::RootDatabase; use super::{completion_context::CompletionContext, completion_item::Completions}; /// Complete mod declaration, i.e. `mod <|> ;` -pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) { - let module_names_for_import = ctx - .scope - .module() - .and_then(|current_module| { - let module_path = path_to_closest_containing_module_file(current_module, ctx.db); - // TODO kb filter out declarations in possible_sudmobule_names - // let declaration_source = current_module.declaration_source(ctx.db); - let module_definition_source_file = - current_module.definition_source(ctx.db).file_id.original_file(ctx.db); +pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { + let current_module = ctx.scope.module()?; - let source_root_id = ctx.db.file_source_root(module_definition_source_file); - let source_root = ctx.db.source_root(source_root_id); - let directory_to_look_for_submodules = source_root - .path_for_file(&module_definition_source_file) - .and_then(|module_file_path| get_directory_with_submodules(module_file_path))?; + // TODO kb filter out declarations in possible_sudmobule_names + // let declaration_source = current_module.declaration_source(ctx.db); + let module_definition_source_file = + current_module.definition_source(ctx.db).file_id.original_file(ctx.db); + let source_root = ctx.db.source_root(ctx.db.file_source_root(module_definition_source_file)); + let directory_to_look_for_submodules = directory_to_look_for_submodules( + current_module, + ctx.db, + source_root.path_for_file(&module_definition_source_file)?, + )?; - let mod_declaration_candidates = source_root - .iter() - .filter(|submodule_file| submodule_file != &module_definition_source_file) - .filter_map(|submodule_file| { - let submodule_path = source_root.path_for_file(&submodule_file)?; - if submodule_path.parent()? == directory_to_look_for_submodules { - submodule_path.file_name_and_extension() + let mod_declaration_candidates = source_root + .iter() + .filter(|submodule_file| submodule_file != &module_definition_source_file) + .filter_map(|submodule_file| { + let submodule_path = source_root.path_for_file(&submodule_file)?; + if submodule_path.parent()? == directory_to_look_for_submodules { + submodule_path.file_name_and_extension() + } else { + None + } + }) + .filter_map(|file_name_and_extension| { + match file_name_and_extension { + // TODO kb in src/bin when a module is included into another, + // the included file gets "moved" into a directory below and now cannot add any other modules + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, + (file_name, Some("rs")) => Some(file_name.to_owned()), + (subdirectory_name, None) => { + let mod_rs_path = + directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; + if source_root.file_for_path(&mod_rs_path).is_some() { + Some(subdirectory_name.to_owned()) } else { None } - }) - .filter_map(|file_name_and_extension| { - match file_name_and_extension { - // TODO kb wrong resolution for nested non-file modules (mod tests { mod <|> }) - // TODO kb in src/bin when a module is included into another, - // the included file gets "moved" into a directory below and now cannot add any other modules - ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, - (file_name, Some("rs")) => Some(file_name.to_owned()), - (subdirectory_name, None) => { - let mod_rs_path = directory_to_look_for_submodules - .join(subdirectory_name)? - .join("mod.rs")?; - if source_root.file_for_path(&mod_rs_path).is_some() { - Some(subdirectory_name.to_owned()) - } else { - None - } - } - _ => None, - } - }) - .collect::>(); - dbg!(mod_declaration_candidates); - // TODO kb exlude existing children from the candidates - let existing_children = current_module.children(ctx.db).collect::>(); - None::> + } + _ => None, + } }) - .unwrap_or_default(); -} + .collect::>(); + dbg!(mod_declaration_candidates); -fn path_to_closest_containing_module_file( - current_module: Module, - db: &RootDatabase, -) -> Vec { - let mut path = Vec::new(); - - let mut current_module = Some(current_module); - while let Some(ModuleSource::Module(_)) = - current_module.map(|module| module.definition_source(db).value) - { - if let Some(module) = current_module { - path.insert(0, module); - current_module = module.parent(db); - } else { - current_module = None; - } - } + // TODO kb exlude existing children from the candidates + let existing_children = current_module.children(ctx.db).collect::>(); - path + Some(()) } -fn get_directory_with_submodules(module_file_path: &VfsPath) -> Option { +fn directory_to_look_for_submodules( + module: Module, + db: &RootDatabase, + module_file_path: &VfsPath, +) -> Option { let module_directory_path = module_file_path.parent()?; - match module_file_path.file_name_and_extension()? { + + let base_directory = match module_file_path.file_name_and_extension()? { ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { Some(module_directory_path) } @@ -109,5 +88,35 @@ fn get_directory_with_submodules(module_file_path: &VfsPath) -> Option } } _ => None, + }?; + + let mut resulting_path = base_directory; + for module in module_chain_to_containing_module_file(module, db) { + if let Some(name) = module.name(db) { + resulting_path = resulting_path.join(&name.to_string())?; + } + } + + Some(resulting_path) +} + +fn module_chain_to_containing_module_file( + current_module: Module, + db: &RootDatabase, +) -> Vec { + let mut path = Vec::new(); + + let mut current_module = Some(current_module); + while let Some(ModuleSource::Module(_)) = + current_module.map(|module| module.definition_source(db).value) + { + if let Some(module) = current_module { + path.insert(0, module); + current_module = module.parent(db); + } else { + current_module = None; + } } + + path } -- cgit v1.2.3 From cc43abcde87ed4a834f3b56a96ef165d8e4f0d86 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 00:00:00 +0300 Subject: Less false positive completion candidates --- crates/ide/src/completion/complete_mod.rs | 103 +++++++++++++++++------------- 1 file changed, 59 insertions(+), 44 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index def8b8968..c5757a310 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -3,6 +3,7 @@ use base_db::{SourceDatabaseExt, VfsPath}; use hir::{Module, ModuleSource}; use ide_db::RootDatabase; +use rustc_hash::FxHashSet; use super::{completion_context::CompletionContext, completion_item::Completions}; @@ -10,84 +11,98 @@ use super::{completion_context::CompletionContext, completion_item::Completions} pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { let current_module = ctx.scope.module()?; - // TODO kb filter out declarations in possible_sudmobule_names - // let declaration_source = current_module.declaration_source(ctx.db); - let module_definition_source_file = + let module_definition_file = current_module.definition_source(ctx.db).file_id.original_file(ctx.db); - let source_root = ctx.db.source_root(ctx.db.file_source_root(module_definition_source_file)); + let source_root = ctx.db.source_root(ctx.db.file_source_root(module_definition_file)); let directory_to_look_for_submodules = directory_to_look_for_submodules( current_module, ctx.db, - source_root.path_for_file(&module_definition_source_file)?, + source_root.path_for_file(&module_definition_file)?, )?; + let existing_mod_declarations = current_module + .children(ctx.db) + .filter_map(|module| Some(module.name(ctx.db)?.to_string())) + .collect::>(); + + let module_declaration_file = + current_module.declaration_source(ctx.db).map(|module_declaration_source_file| { + module_declaration_source_file.file_id.original_file(ctx.db) + }); + let mod_declaration_candidates = source_root .iter() - .filter(|submodule_file| submodule_file != &module_definition_source_file) + .filter(|submodule_candidate_file| submodule_candidate_file != &module_definition_file) + .filter(|submodule_candidate_file| { + Some(submodule_candidate_file) != module_declaration_file.as_ref() + }) .filter_map(|submodule_file| { let submodule_path = source_root.path_for_file(&submodule_file)?; - if submodule_path.parent()? == directory_to_look_for_submodules { + if !is_special_rust_file_path(&submodule_path) + && submodule_path.parent()? == directory_to_look_for_submodules + { submodule_path.file_name_and_extension() } else { None } }) - .filter_map(|file_name_and_extension| { - match file_name_and_extension { - // TODO kb in src/bin when a module is included into another, - // the included file gets "moved" into a directory below and now cannot add any other modules - ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => None, - (file_name, Some("rs")) => Some(file_name.to_owned()), - (subdirectory_name, None) => { - let mod_rs_path = - directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; - if source_root.file_for_path(&mod_rs_path).is_some() { - Some(subdirectory_name.to_owned()) - } else { - None - } + .filter_map(|submodule_file_name_and_extension| match submodule_file_name_and_extension { + (file_name, Some("rs")) => Some(file_name.to_owned()), + (subdirectory_name, None) => { + let mod_rs_path = + directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; + if source_root.file_for_path(&mod_rs_path).is_some() { + Some(subdirectory_name.to_owned()) + } else { + None } - _ => None, } + _ => None, }) + .filter(|name| !existing_mod_declarations.contains(name)) .collect::>(); dbg!(mod_declaration_candidates); // TODO kb exlude existing children from the candidates - let existing_children = current_module.children(ctx.db).collect::>(); Some(()) } +fn is_special_rust_file_path(path: &VfsPath) -> bool { + matches!( + path.file_name_and_extension(), + Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) | Some(("main", Some("rs"))) + ) +} + fn directory_to_look_for_submodules( module: Module, db: &RootDatabase, module_file_path: &VfsPath, ) -> Option { let module_directory_path = module_file_path.parent()?; - - let base_directory = match module_file_path.file_name_and_extension()? { - ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { + let base_directory = if is_special_rust_file_path(module_file_path) { + Some(module_directory_path) + } else if let (regular_rust_file_name, Some("rs")) = + module_file_path.file_name_and_extension()? + { + if matches!( + ( + module_directory_path + .parent() + .as_ref() + .and_then(|path| path.file_name_and_extension()), + module_directory_path.file_name_and_extension(), + ), + (Some(("src", None)), Some(("bin", None))) + ) { + // files in /src/bin/ can import each other directly Some(module_directory_path) + } else { + module_directory_path.join(regular_rust_file_name) } - (regular_rust_file_name, Some("rs")) => { - if matches!( - ( - module_directory_path - .parent() - .as_ref() - .and_then(|path| path.file_name_and_extension()), - module_directory_path.file_name_and_extension(), - ), - (Some(("src", None)), Some(("bin", None))) - ) { - // files in /src/bin/ can import each other directly - Some(module_directory_path) - } else { - module_directory_path.join(regular_rust_file_name) - } - } - _ => None, + } else { + None }?; let mut resulting_path = base_directory; -- cgit v1.2.3 From 57a260f579fec4082aa9e7a30d4b190f06d45877 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 00:54:58 +0300 Subject: Properly reacto to keywords --- crates/ide/src/completion/complete_attribute.rs | 4 ++++ crates/ide/src/completion/complete_mod.rs | 8 +++++++- crates/ide/src/completion/complete_qualified_path.rs | 2 +- crates/ide/src/completion/complete_unqualified_path.rs | 1 + crates/ide/src/completion/completion_context.rs | 7 +++++-- crates/ide/src/completion/patterns.rs | 10 ++++++++++ 6 files changed, 28 insertions(+), 4 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_attribute.rs b/crates/ide/src/completion/complete_attribute.rs index 0abfaebcb..6394189f0 100644 --- a/crates/ide/src/completion/complete_attribute.rs +++ b/crates/ide/src/completion/complete_attribute.rs @@ -13,6 +13,10 @@ use crate::completion::{ }; pub(super) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { + if ctx.mod_is_prev { + return None; + } + let attribute = ctx.attribute_under_caret.as_ref()?; match (attribute.path(), attribute.token_tree()) { (Some(path), Some(token_tree)) if path.to_string() == "derive" => { diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index c5757a310..5d41d0f69 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -9,6 +9,12 @@ use super::{completion_context::CompletionContext, completion_item::Completions} /// Complete mod declaration, i.e. `mod <|> ;` pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { + let _p = profile::span("completion::complete_mod"); + + if !ctx.mod_is_prev { + return None; + } + let current_module = ctx.scope.module()?; let module_definition_file = @@ -63,7 +69,7 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Op .collect::>(); dbg!(mod_declaration_candidates); - // TODO kb exlude existing children from the candidates + // TODO kb actually add the results Some(()) } diff --git a/crates/ide/src/completion/complete_qualified_path.rs b/crates/ide/src/completion/complete_qualified_path.rs index accb09f7e..351461351 100644 --- a/crates/ide/src/completion/complete_qualified_path.rs +++ b/crates/ide/src/completion/complete_qualified_path.rs @@ -13,7 +13,7 @@ pub(super) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon None => return, }; - if ctx.attribute_under_caret.is_some() { + if ctx.attribute_under_caret.is_some() || ctx.mod_is_prev { return; } diff --git a/crates/ide/src/completion/complete_unqualified_path.rs b/crates/ide/src/completion/complete_unqualified_path.rs index 1f1b682a7..9f2dc16ab 100644 --- a/crates/ide/src/completion/complete_unqualified_path.rs +++ b/crates/ide/src/completion/complete_unqualified_path.rs @@ -13,6 +13,7 @@ pub(super) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC if ctx.record_lit_syntax.is_some() || ctx.record_pat_syntax.is_some() || ctx.attribute_under_caret.is_some() + || ctx.mod_is_prev { return; } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index 47355d5dc..d289aac27 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -19,7 +19,7 @@ use crate::{ has_bind_pat_parent, has_block_expr_parent, has_field_list_parent, has_impl_as_prev_sibling, has_impl_parent, has_item_list_or_source_file_parent, has_ref_parent, has_trait_as_prev_sibling, has_trait_parent, if_is_prev, - is_in_loop_body, is_match_arm, unsafe_is_prev, + is_in_loop_body, is_match_arm, mod_is_prev, unsafe_is_prev, }, CompletionConfig, }, @@ -77,6 +77,7 @@ pub(crate) struct CompletionContext<'a> { pub(super) is_path_type: bool, pub(super) has_type_args: bool, pub(super) attribute_under_caret: Option, + pub(super) mod_is_prev: bool, pub(super) unsafe_is_prev: bool, pub(super) if_is_prev: bool, pub(super) block_expr_parent: bool, @@ -152,6 +153,7 @@ impl<'a> CompletionContext<'a> { has_type_args: false, dot_receiver_is_ambiguous_float_literal: false, attribute_under_caret: None, + mod_is_prev: false, unsafe_is_prev: false, in_loop_body: false, ref_pat_parent: false, @@ -238,7 +240,8 @@ impl<'a> CompletionContext<'a> { self.trait_as_prev_sibling = has_trait_as_prev_sibling(syntax_element.clone()); self.is_match_arm = is_match_arm(syntax_element.clone()); self.has_item_list_or_source_file_parent = - has_item_list_or_source_file_parent(syntax_element); + has_item_list_or_source_file_parent(syntax_element.clone()); + self.mod_is_prev = mod_is_prev(syntax_element); } fn fill( diff --git a/crates/ide/src/completion/patterns.rs b/crates/ide/src/completion/patterns.rs index c6ae589db..bc4ce4d6f 100644 --- a/crates/ide/src/completion/patterns.rs +++ b/crates/ide/src/completion/patterns.rs @@ -115,6 +115,16 @@ pub(crate) fn if_is_prev(element: SyntaxElement) -> bool { .filter(|it| it.kind() == IF_KW) .is_some() } + +// TODO kb generify? +pub(crate) fn mod_is_prev(element: SyntaxElement) -> bool { + element + .into_token() + .and_then(|it| previous_non_trivia_token(it)) + .filter(|it| it.kind() == MOD_KW) + .is_some() +} + #[test] fn test_if_is_prev() { check_pattern_is_applicable(r"if l<|>", if_is_prev); -- cgit v1.2.3 From 9fb83211f95a450fdadf05f8f64be053f14dc57e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 01:24:16 +0300 Subject: Complete semicolon when needed --- crates/ide/src/completion/complete_attribute.rs | 2 +- crates/ide/src/completion/complete_mod.rs | 33 +++++++++++++++------- .../ide/src/completion/complete_qualified_path.rs | 2 +- .../src/completion/complete_unqualified_path.rs | 2 +- crates/ide/src/completion/completion_context.rs | 8 +++--- crates/ide/src/completion/patterns.rs | 9 ------ 6 files changed, 30 insertions(+), 26 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_attribute.rs b/crates/ide/src/completion/complete_attribute.rs index 6394189f0..ef4fb6a91 100644 --- a/crates/ide/src/completion/complete_attribute.rs +++ b/crates/ide/src/completion/complete_attribute.rs @@ -13,7 +13,7 @@ use crate::completion::{ }; pub(super) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - if ctx.mod_is_prev { + if ctx.mod_under_caret.is_some() { return None; } diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index 5d41d0f69..f1795d2f7 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -5,15 +5,22 @@ use hir::{Module, ModuleSource}; use ide_db::RootDatabase; use rustc_hash::FxHashSet; -use super::{completion_context::CompletionContext, completion_item::Completions}; +use crate::{CompletionItem, CompletionItemKind}; + +use super::{ + completion_context::CompletionContext, completion_item::CompletionKind, + completion_item::Completions, +}; /// Complete mod declaration, i.e. `mod <|> ;` pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - let _p = profile::span("completion::complete_mod"); + let mod_under_caret = match &ctx.mod_under_caret { + Some(mod_under_caret) if mod_under_caret.item_list().is_some() => return None, + Some(mod_under_caret) => mod_under_caret, + None => return None, + }; - if !ctx.mod_is_prev { - return None; - } + let _p = profile::span("completion::complete_mod"); let current_module = ctx.scope.module()?; @@ -36,7 +43,7 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Op module_declaration_source_file.file_id.original_file(ctx.db) }); - let mod_declaration_candidates = source_root + source_root .iter() .filter(|submodule_candidate_file| submodule_candidate_file != &module_definition_file) .filter(|submodule_candidate_file| { @@ -66,10 +73,16 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Op _ => None, }) .filter(|name| !existing_mod_declarations.contains(name)) - .collect::>(); - dbg!(mod_declaration_candidates); - - // TODO kb actually add the results + .for_each(|submodule_name| { + let mut label = submodule_name; + if mod_under_caret.semicolon_token().is_none() { + label.push(';') + } + acc.add( + CompletionItem::new(CompletionKind::Magic, ctx.source_range(), &label) + .kind(CompletionItemKind::Module), + ) + }); Some(()) } diff --git a/crates/ide/src/completion/complete_qualified_path.rs b/crates/ide/src/completion/complete_qualified_path.rs index 351461351..184488a73 100644 --- a/crates/ide/src/completion/complete_qualified_path.rs +++ b/crates/ide/src/completion/complete_qualified_path.rs @@ -13,7 +13,7 @@ pub(super) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon None => return, }; - if ctx.attribute_under_caret.is_some() || ctx.mod_is_prev { + if ctx.attribute_under_caret.is_some() || ctx.mod_under_caret.is_some() { return; } diff --git a/crates/ide/src/completion/complete_unqualified_path.rs b/crates/ide/src/completion/complete_unqualified_path.rs index 9f2dc16ab..f2189dfde 100644 --- a/crates/ide/src/completion/complete_unqualified_path.rs +++ b/crates/ide/src/completion/complete_unqualified_path.rs @@ -13,7 +13,7 @@ pub(super) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC if ctx.record_lit_syntax.is_some() || ctx.record_pat_syntax.is_some() || ctx.attribute_under_caret.is_some() - || ctx.mod_is_prev + || ctx.mod_under_caret.is_some() { return; } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index d289aac27..ea4830843 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -19,7 +19,7 @@ use crate::{ has_bind_pat_parent, has_block_expr_parent, has_field_list_parent, has_impl_as_prev_sibling, has_impl_parent, has_item_list_or_source_file_parent, has_ref_parent, has_trait_as_prev_sibling, has_trait_parent, if_is_prev, - is_in_loop_body, is_match_arm, mod_is_prev, unsafe_is_prev, + is_in_loop_body, is_match_arm, unsafe_is_prev, }, CompletionConfig, }, @@ -77,7 +77,7 @@ pub(crate) struct CompletionContext<'a> { pub(super) is_path_type: bool, pub(super) has_type_args: bool, pub(super) attribute_under_caret: Option, - pub(super) mod_is_prev: bool, + pub(super) mod_under_caret: Option, pub(super) unsafe_is_prev: bool, pub(super) if_is_prev: bool, pub(super) block_expr_parent: bool, @@ -153,7 +153,7 @@ impl<'a> CompletionContext<'a> { has_type_args: false, dot_receiver_is_ambiguous_float_literal: false, attribute_under_caret: None, - mod_is_prev: false, + mod_under_caret: None, unsafe_is_prev: false, in_loop_body: false, ref_pat_parent: false, @@ -241,7 +241,7 @@ impl<'a> CompletionContext<'a> { self.is_match_arm = is_match_arm(syntax_element.clone()); self.has_item_list_or_source_file_parent = has_item_list_or_source_file_parent(syntax_element.clone()); - self.mod_is_prev = mod_is_prev(syntax_element); + self.mod_under_caret = find_node_at_offset(&file_with_fake_ident, offset); } fn fill( diff --git a/crates/ide/src/completion/patterns.rs b/crates/ide/src/completion/patterns.rs index bc4ce4d6f..b17ddf133 100644 --- a/crates/ide/src/completion/patterns.rs +++ b/crates/ide/src/completion/patterns.rs @@ -116,15 +116,6 @@ pub(crate) fn if_is_prev(element: SyntaxElement) -> bool { .is_some() } -// TODO kb generify? -pub(crate) fn mod_is_prev(element: SyntaxElement) -> bool { - element - .into_token() - .and_then(|it| previous_non_trivia_token(it)) - .filter(|it| it.kind() == MOD_KW) - .is_some() -} - #[test] fn test_if_is_prev() { check_pattern_is_applicable(r"if l<|>", if_is_prev); -- cgit v1.2.3 From dbf70cd015454cf125fda9b006251fa2782fbc7f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 01:45:05 +0300 Subject: Properly handle mod.rs imports --- crates/ide/src/completion/complete_mod.rs | 87 ++++++++++++++----------------- 1 file changed, 40 insertions(+), 47 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index f1795d2f7..b5f2d636c 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -51,26 +51,26 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Op }) .filter_map(|submodule_file| { let submodule_path = source_root.path_for_file(&submodule_file)?; - if !is_special_rust_file_path(&submodule_path) - && submodule_path.parent()? == directory_to_look_for_submodules - { - submodule_path.file_name_and_extension() - } else { - None - } - }) - .filter_map(|submodule_file_name_and_extension| match submodule_file_name_and_extension { - (file_name, Some("rs")) => Some(file_name.to_owned()), - (subdirectory_name, None) => { - let mod_rs_path = - directory_to_look_for_submodules.join(subdirectory_name)?.join("mod.rs")?; - if source_root.file_for_path(&mod_rs_path).is_some() { - Some(subdirectory_name.to_owned()) - } else { - None + let directory_with_submodule = submodule_path.parent()?; + match submodule_path.file_name_and_extension()? { + ("lib", Some("rs")) | ("main", Some("rs")) => None, + ("mod", Some("rs")) => { + if directory_with_submodule.parent()? == directory_to_look_for_submodules { + match directory_with_submodule.file_name_and_extension()? { + (directory_name, None) => Some(directory_name.to_owned()), + _ => None, + } + } else { + None + } } + (file_name, Some("rs")) + if directory_with_submodule == directory_to_look_for_submodules => + { + Some(file_name.to_owned()) + } + _ => None, } - _ => None, }) .filter(|name| !existing_mod_declarations.contains(name)) .for_each(|submodule_name| { @@ -87,41 +87,34 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Op Some(()) } -fn is_special_rust_file_path(path: &VfsPath) -> bool { - matches!( - path.file_name_and_extension(), - Some(("mod", Some("rs"))) | Some(("lib", Some("rs"))) | Some(("main", Some("rs"))) - ) -} - fn directory_to_look_for_submodules( module: Module, db: &RootDatabase, module_file_path: &VfsPath, ) -> Option { - let module_directory_path = module_file_path.parent()?; - let base_directory = if is_special_rust_file_path(module_file_path) { - Some(module_directory_path) - } else if let (regular_rust_file_name, Some("rs")) = - module_file_path.file_name_and_extension()? - { - if matches!( - ( - module_directory_path - .parent() - .as_ref() - .and_then(|path| path.file_name_and_extension()), - module_directory_path.file_name_and_extension(), - ), - (Some(("src", None)), Some(("bin", None))) - ) { - // files in /src/bin/ can import each other directly - Some(module_directory_path) - } else { - module_directory_path.join(regular_rust_file_name) + let directory_with_module_path = module_file_path.parent()?; + let base_directory = match module_file_path.file_name_and_extension()? { + ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { + Some(directory_with_module_path) + } + (regular_rust_file_name, Some("rs")) => { + if matches!( + ( + directory_with_module_path + .parent() + .as_ref() + .and_then(|path| path.file_name_and_extension()), + directory_with_module_path.file_name_and_extension(), + ), + (Some(("src", None)), Some(("bin", None))) + ) { + // files in /src/bin/ can import each other directly + Some(directory_with_module_path) + } else { + directory_with_module_path.join(regular_rust_file_name) + } } - } else { - None + _ => None, }?; let mut resulting_path = base_directory; -- cgit v1.2.3 From 5aebf54fd4d075389ce2c74a460ea0c48ccdbad2 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 02:26:55 +0300 Subject: Add tests --- crates/ide/src/completion/complete_mod.rs | 153 ++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index b5f2d636c..049e99674 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -147,3 +147,156 @@ fn module_chain_to_containing_module_file( path } + +#[cfg(test)] +mod tests { + use crate::completion::{test_utils::completion_list, CompletionKind}; + use expect_test::{expect, Expect}; + + fn check(ra_fixture: &str, expect: Expect) { + let actual = completion_list(ra_fixture, CompletionKind::Magic); + expect.assert_eq(&actual); + } + + #[test] + fn lib_module_completion() { + check( + r#" + //- /lib.rs + mod <|> + //- /foo.rs + fn foo() {} + //- /foo/ignored_foo.rs + fn ignored_foo() {} + //- /bar/mod.rs + fn bar() {} + //- /bar/ignored_bar.rs + fn ignored_bar() {} + "#, + expect![[r#" + md bar; + md foo; + "#]], + ); + } + + #[test] + fn main_module_completion() { + check( + r#" + //- /main.rs + mod <|> + //- /foo.rs + fn foo() {} + //- /foo/ignored_foo.rs + fn ignored_foo() {} + //- /bar/mod.rs + fn bar() {} + //- /bar/ignored_bar.rs + fn ignored_bar() {} + "#, + expect![[r#" + md bar; + md foo; + "#]], + ); + } + + #[test] + fn main_test_module_completion() { + check( + r#" + //- /main.rs + mod tests { + mod <|>; + } + //- /tests/foo.rs + fn foo() {} + "#, + expect![[r#" + md foo + "#]], + ); + } + + #[test] + fn directly_nested_module_completion() { + check( + r#" + //- /lib.rs + mod foo; + //- /foo.rs + mod <|>; + //- /foo/bar.rs + fn bar() {} + //- /foo/bar/ignored_bar.rs + fn ignored_bar() {} + //- /foo/baz/mod.rs + fn baz() {} + //- /foo/moar/ignored_moar.rs + fn ignored_moar() {} + "#, + expect![[r#" + md bar + md baz + "#]], + ); + } + + #[test] + fn nested_in_source_module_completion() { + check( + r#" + //- /lib.rs + mod foo; + //- /foo.rs + mod bar { + mod <|> + } + //- /foo/bar/baz.rs + fn baz() {} + "#, + expect![[r#" + md baz; + "#]], + ); + } + + // FIXME binart modules are not picked up in tests + // #[test] + // fn regular_bin_module_completion() { + // check( + // r#" + // //- /src/main.rs + // fn main() {} + // //- /src/main/foo.rs + // mod <|> + // //- /src/main/bar.rs + // fn bar() {} + // //- /src/main/bar/bar_ignored.rs + // fn bar_ignored() {} + // "#, + // expect![[r#" + // md bar; + // "#]], + // ); + // } + + #[test] + fn already_declared_bin_module_completion_omitted() { + check( + r#" + //- /src/main.rs + fn main() {} + //- /src/main/foo.rs + mod <|> + //- /src/main/bar.rs + mod foo; + fn bar() {} + //- /src/main/bar/bar_ignored.rs + fn bar_ignored() {} + "#, + expect![[r#""#]], + ); + } +} -- cgit v1.2.3 From a7d75463c7d1473e4276601688baa22c10eec255 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 02:34:11 +0300 Subject: Fix the tests --- crates/ide/src/completion/complete_attribute.rs | 2 +- crates/ide/src/completion/complete_mod.rs | 2 +- crates/ide/src/completion/complete_qualified_path.rs | 2 +- crates/ide/src/completion/complete_unqualified_path.rs | 2 +- crates/ide/src/completion/completion_context.rs | 8 +++++--- 5 files changed, 9 insertions(+), 7 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_attribute.rs b/crates/ide/src/completion/complete_attribute.rs index ef4fb6a91..f4a9864d1 100644 --- a/crates/ide/src/completion/complete_attribute.rs +++ b/crates/ide/src/completion/complete_attribute.rs @@ -13,7 +13,7 @@ use crate::completion::{ }; pub(super) fn complete_attribute(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - if ctx.mod_under_caret.is_some() { + if ctx.mod_declaration_under_caret.is_some() { return None; } diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index 049e99674..d457ff6bf 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -14,7 +14,7 @@ use super::{ /// Complete mod declaration, i.e. `mod <|> ;` pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { - let mod_under_caret = match &ctx.mod_under_caret { + let mod_under_caret = match &ctx.mod_declaration_under_caret { Some(mod_under_caret) if mod_under_caret.item_list().is_some() => return None, Some(mod_under_caret) => mod_under_caret, None => return None, diff --git a/crates/ide/src/completion/complete_qualified_path.rs b/crates/ide/src/completion/complete_qualified_path.rs index 184488a73..79de50792 100644 --- a/crates/ide/src/completion/complete_qualified_path.rs +++ b/crates/ide/src/completion/complete_qualified_path.rs @@ -13,7 +13,7 @@ pub(super) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon None => return, }; - if ctx.attribute_under_caret.is_some() || ctx.mod_under_caret.is_some() { + if ctx.attribute_under_caret.is_some() || ctx.mod_declaration_under_caret.is_some() { return; } diff --git a/crates/ide/src/completion/complete_unqualified_path.rs b/crates/ide/src/completion/complete_unqualified_path.rs index f2189dfde..8eda4b64d 100644 --- a/crates/ide/src/completion/complete_unqualified_path.rs +++ b/crates/ide/src/completion/complete_unqualified_path.rs @@ -13,7 +13,7 @@ pub(super) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC if ctx.record_lit_syntax.is_some() || ctx.record_pat_syntax.is_some() || ctx.attribute_under_caret.is_some() - || ctx.mod_under_caret.is_some() + || ctx.mod_declaration_under_caret.is_some() { return; } diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs index ea4830843..161f59c1e 100644 --- a/crates/ide/src/completion/completion_context.rs +++ b/crates/ide/src/completion/completion_context.rs @@ -77,7 +77,7 @@ pub(crate) struct CompletionContext<'a> { pub(super) is_path_type: bool, pub(super) has_type_args: bool, pub(super) attribute_under_caret: Option, - pub(super) mod_under_caret: Option, + pub(super) mod_declaration_under_caret: Option, pub(super) unsafe_is_prev: bool, pub(super) if_is_prev: bool, pub(super) block_expr_parent: bool, @@ -153,7 +153,7 @@ impl<'a> CompletionContext<'a> { has_type_args: false, dot_receiver_is_ambiguous_float_literal: false, attribute_under_caret: None, - mod_under_caret: None, + mod_declaration_under_caret: None, unsafe_is_prev: false, in_loop_body: false, ref_pat_parent: false, @@ -241,7 +241,9 @@ impl<'a> CompletionContext<'a> { self.is_match_arm = is_match_arm(syntax_element.clone()); self.has_item_list_or_source_file_parent = has_item_list_or_source_file_parent(syntax_element.clone()); - self.mod_under_caret = find_node_at_offset(&file_with_fake_ident, offset); + self.mod_declaration_under_caret = + find_node_at_offset::(&file_with_fake_ident, offset) + .filter(|module| module.item_list().is_none()); } fn fill( -- cgit v1.2.3 From f4ee885b3b3019b32d5c481bddf1b2667fba7fb3 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Sep 2020 02:42:45 +0300 Subject: Add VirtualPath tests --- crates/vfs/src/vfs_path.rs | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) (limited to 'crates') diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index 113c2e4e6..dac8393ea 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -290,9 +290,10 @@ impl VirtualPath { // FIXME: Currently VirtualPath does is unable to distinguish a directory from a file // hence this method will return `Some("directory_name", None)` for a directory pub fn file_name_and_extension(&self) -> Option<(&str, Option<&str>)> { - let file_name = match self.0.rfind('/') { - Some(position) => &self.0[position + 1..], - None => &self.0, + let file_path = if self.0.ends_with('/') { &self.0[..&self.0.len() - 1] } else { &self.0 }; + let file_name = match file_path.rfind('/') { + Some(position) => &file_path[position + 1..], + None => file_path, }; if file_name.is_empty() { @@ -310,3 +311,37 @@ impl VirtualPath { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn virtual_path_extensions() { + assert_eq!(VirtualPath("/".to_string()).file_name_and_extension(), None); + assert_eq!( + VirtualPath("/directory".to_string()).file_name_and_extension(), + Some(("directory", None)) + ); + assert_eq!( + VirtualPath("/directory/".to_string()).file_name_and_extension(), + Some(("directory", None)) + ); + assert_eq!( + VirtualPath("/directory/file".to_string()).file_name_and_extension(), + Some(("file", None)) + ); + assert_eq!( + VirtualPath("/directory/.file".to_string()).file_name_and_extension(), + Some((".file", None)) + ); + assert_eq!( + VirtualPath("/directory/.file.rs".to_string()).file_name_and_extension(), + Some((".file", Some("rs"))) + ); + assert_eq!( + VirtualPath("/directory/file.rs".to_string()).file_name_and_extension(), + Some(("file", Some("rs"))) + ); + } +} -- cgit v1.2.3 From 9863798480aa9642e31bfd41ee899d2e7329b5e5 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 10 Sep 2020 01:45:49 +0300 Subject: Rename the method to avoid false promises --- crates/ide/src/completion/complete_mod.rs | 10 +++++----- crates/vfs/src/vfs_path.rs | 22 ++++++++++------------ 2 files changed, 15 insertions(+), 17 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index d457ff6bf..2ea0d5034 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -52,11 +52,11 @@ pub(super) fn complete_mod(acc: &mut Completions, ctx: &CompletionContext) -> Op .filter_map(|submodule_file| { let submodule_path = source_root.path_for_file(&submodule_file)?; let directory_with_submodule = submodule_path.parent()?; - match submodule_path.file_name_and_extension()? { + match submodule_path.name_and_extension()? { ("lib", Some("rs")) | ("main", Some("rs")) => None, ("mod", Some("rs")) => { if directory_with_submodule.parent()? == directory_to_look_for_submodules { - match directory_with_submodule.file_name_and_extension()? { + match directory_with_submodule.name_and_extension()? { (directory_name, None) => Some(directory_name.to_owned()), _ => None, } @@ -93,7 +93,7 @@ fn directory_to_look_for_submodules( module_file_path: &VfsPath, ) -> Option { let directory_with_module_path = module_file_path.parent()?; - let base_directory = match module_file_path.file_name_and_extension()? { + let base_directory = match module_file_path.name_and_extension()? { ("mod", Some("rs")) | ("lib", Some("rs")) | ("main", Some("rs")) => { Some(directory_with_module_path) } @@ -103,8 +103,8 @@ fn directory_to_look_for_submodules( directory_with_module_path .parent() .as_ref() - .and_then(|path| path.file_name_and_extension()), - directory_with_module_path.file_name_and_extension(), + .and_then(|path| path.name_and_extension()), + directory_with_module_path.name_and_extension(), ), (Some(("src", None)), Some(("bin", None))) ) { diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index dac8393ea..022a0be1e 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -57,13 +57,13 @@ impl VfsPath { } } - pub fn file_name_and_extension(&self) -> Option<(&str, Option<&str>)> { + pub fn name_and_extension(&self) -> Option<(&str, Option<&str>)> { match &self.0 { VfsPathRepr::PathBuf(p) => Some(( p.file_stem()?.to_str()?, p.extension().and_then(|extension| extension.to_str()), )), - VfsPathRepr::VirtualPath(p) => p.file_name_and_extension(), + VfsPathRepr::VirtualPath(p) => p.name_and_extension(), } } @@ -287,9 +287,7 @@ impl VirtualPath { Some(res) } - // FIXME: Currently VirtualPath does is unable to distinguish a directory from a file - // hence this method will return `Some("directory_name", None)` for a directory - pub fn file_name_and_extension(&self) -> Option<(&str, Option<&str>)> { + pub fn name_and_extension(&self) -> Option<(&str, Option<&str>)> { let file_path = if self.0.ends_with('/') { &self.0[..&self.0.len() - 1] } else { &self.0 }; let file_name = match file_path.rfind('/') { Some(position) => &file_path[position + 1..], @@ -318,29 +316,29 @@ mod tests { #[test] fn virtual_path_extensions() { - assert_eq!(VirtualPath("/".to_string()).file_name_and_extension(), None); + assert_eq!(VirtualPath("/".to_string()).name_and_extension(), None); assert_eq!( - VirtualPath("/directory".to_string()).file_name_and_extension(), + VirtualPath("/directory".to_string()).name_and_extension(), Some(("directory", None)) ); assert_eq!( - VirtualPath("/directory/".to_string()).file_name_and_extension(), + VirtualPath("/directory/".to_string()).name_and_extension(), Some(("directory", None)) ); assert_eq!( - VirtualPath("/directory/file".to_string()).file_name_and_extension(), + VirtualPath("/directory/file".to_string()).name_and_extension(), Some(("file", None)) ); assert_eq!( - VirtualPath("/directory/.file".to_string()).file_name_and_extension(), + VirtualPath("/directory/.file".to_string()).name_and_extension(), Some((".file", None)) ); assert_eq!( - VirtualPath("/directory/.file.rs".to_string()).file_name_and_extension(), + VirtualPath("/directory/.file.rs".to_string()).name_and_extension(), Some((".file", Some("rs"))) ); assert_eq!( - VirtualPath("/directory/file.rs".to_string()).file_name_and_extension(), + VirtualPath("/directory/file.rs".to_string()).name_and_extension(), Some(("file", Some("rs"))) ); } -- cgit v1.2.3 From 492e3c40f666f53d9e6b8329cadc57ff36e38ba9 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 10 Sep 2020 01:58:29 +0300 Subject: One more test --- crates/ide/src/completion/complete_mod.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index 2ea0d5034..7dbf4aee4 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -180,6 +180,21 @@ mod tests { ); } + #[test] + fn no_module_completion_with_module_body() { + check( + r#" + //- /lib.rs + mod <|> { + + } + //- /foo.rs + fn foo() {} + "#, + expect![[r#""#]], + ); + } + #[test] fn main_module_completion() { check( -- cgit v1.2.3 From ca698a0b8c78e5ba7738fc0f0f6f77718e70a83e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 11 Sep 2020 14:16:15 +0300 Subject: Adjust the test comment --- crates/ide/src/completion/complete_mod.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/completion/complete_mod.rs b/crates/ide/src/completion/complete_mod.rs index 7dbf4aee4..3cfc2e131 100644 --- a/crates/ide/src/completion/complete_mod.rs +++ b/crates/ide/src/completion/complete_mod.rs @@ -277,18 +277,25 @@ mod tests { ); } - // FIXME binart modules are not picked up in tests + // FIXME binary modules are not supported in tests properly + // Binary modules are a bit special, they allow importing the modules from `/src/bin` + // and that's why are good to test two things: + // * no cycles are allowed in mod declarations + // * no modules from the parent directory are proposed + // Unfortunately, binary modules support is in cargo not rustc, + // hence the test does not work now + // // #[test] // fn regular_bin_module_completion() { // check( // r#" - // //- /src/main.rs + // //- /src/bin.rs // fn main() {} - // //- /src/main/foo.rs + // //- /src/bin/foo.rs // mod <|> - // //- /src/main/bar.rs + // //- /src/bin/bar.rs // fn bar() {} - // //- /src/main/bar/bar_ignored.rs + // //- /src/bin/bar/bar_ignored.rs // fn bar_ignored() {} // "#, // expect![[r#" @@ -301,14 +308,14 @@ mod tests { fn already_declared_bin_module_completion_omitted() { check( r#" - //- /src/main.rs + //- /src/bin.rs fn main() {} - //- /src/main/foo.rs + //- /src/bin/foo.rs mod <|> - //- /src/main/bar.rs + //- /src/bin/bar.rs mod foo; fn bar() {} - //- /src/main/bar/bar_ignored.rs + //- /src/bin/bar/bar_ignored.rs fn bar_ignored() {} "#, expect![[r#""#]], -- cgit v1.2.3