Merge #5969

5969: Propose module name completion options r=jonas-schievink a=SomeoneToIgnore

<img width="539" alt="image" src="https://user-images.githubusercontent.com/2690773/92663009-cb0aec00-f308-11ea-9ef5-1faa91518031.png">

Currently traverses the whole file set every time we try to complete the module, as discussed in https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/mod.3C.7C.3E.20completion

Co-authored-by: Kirill Bulatov <[email protected]>

10 files changed, 431 insertions, 5 deletions

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<Item = FileId> + '_ {
         self.file_set.iter()
     }
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_attribute.rs b/crates/ide/src/completion/complete_attribute.rs
index 0abfaebcb..f4a9864d1 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_declaration_under_caret.is_some() {
+        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
new file mode 100644
index 000000000..3cfc2e131
--- /dev/null
+++ b/crates/ide/src/completion/complete_mod.rs
@@ -0,0 +1,324 @@
+//! Completes mod declarations.
+
+use base_db::{SourceDatabaseExt, VfsPath};
+use hir::{Module, ModuleSource};
+use ide_db::RootDatabase;
+use rustc_hash::FxHashSet;
+
+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 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,
+    };
+
+    let _p = profile::span("completion::complete_mod");
+
+    let current_module = ctx.scope.module()?;
+
+    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_file));
+    let directory_to_look_for_submodules = directory_to_look_for_submodules(
+        current_module,
+        ctx.db,
+        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::<FxHashSet<_>>();
+
+    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)
+        });
+
+    source_root
+        .iter()
+        .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)?;
+            let directory_with_submodule = submodule_path.parent()?;
+            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.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,
+            }
+        })
+        .filter(|name| !existing_mod_declarations.contains(name))
+        .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(())
+}
+
+fn directory_to_look_for_submodules(
+    module: Module,
+    db: &RootDatabase,
+    module_file_path: &VfsPath,
+) -> Option<VfsPath> {
+    let directory_with_module_path = module_file_path.parent()?;
+    let base_directory = match module_file_path.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.name_and_extension()),
+                    directory_with_module_path.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)
+            }
+        }
+        _ => 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<Module> {
+    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
+}
+
+#[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 no_module_completion_with_module_body() {
+        check(
+            r#"
+            //- /lib.rs
+            mod <|> {
+
+            }
+            //- /foo.rs
+            fn foo() {}
+        "#,
+            expect![[r#""#]],
+        );
+    }
+
+    #[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 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/bin.rs
+    //         fn main() {}
+    //         //- /src/bin/foo.rs
+    //         mod <|>
+    //         //- /src/bin/bar.rs
+    //         fn bar() {}
+    //         //- /src/bin/bar/bar_ignored.rs
+    //         fn bar_ignored() {}
+    //     "#,
+    //         expect![[r#"
+    //             md bar;
+    //         "#]],
+    //     );
+    // }
+
+    #[test]
+    fn already_declared_bin_module_completion_omitted() {
+        check(
+            r#"
+            //- /src/bin.rs
+            fn main() {}
+            //- /src/bin/foo.rs
+            mod <|>
+            //- /src/bin/bar.rs
+            mod foo;
+            fn bar() {}
+            //- /src/bin/bar/bar_ignored.rs
+            fn bar_ignored() {}
+        "#,
+            expect![[r#""#]],
+        );
+    }
+}
diff --git a/crates/ide/src/completion/complete_qualified_path.rs b/crates/ide/src/completion/complete_qualified_path.rs
index accb09f7e..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() {
+    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 1f1b682a7..8eda4b64d 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_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 47355d5dc..161f59c1e 100644
--- a/crates/ide/src/completion/completion_context.rs
+++ b/crates/ide/src/completion/completion_context.rs
@@ -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<ast::Attr>,
+    pub(super) mod_declaration_under_caret: Option<ast::Module>,
     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_declaration_under_caret: None,
             unsafe_is_prev: false,
             in_loop_body: false,
             ref_pat_parent: false,
@@ -238,7 +240,10 @@ 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_declaration_under_caret =
+            find_node_at_offset::<ast::Module>(&file_with_fake_ident, offset)
+                .filter(|module| module.item_list().is_none());
     }
 
     fn fill(
diff --git a/crates/ide/src/completion/patterns.rs b/crates/ide/src/completion/patterns.rs
index c6ae589db..b17ddf133 100644
--- a/crates/ide/src/completion/patterns.rs
+++ b/crates/ide/src/completion/patterns.rs
@@ -115,6 +115,7 @@ pub(crate) fn if_is_prev(element: SyntaxElement) -> bool {
         .filter(|it| it.kind() == IF_KW)
         .is_some()
 }
+
 #[test]
 fn test_if_is_prev() {
     check_pattern_is_applicable(r"if l<|>", if_is_prev);
diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs
index e9196fcd2..4aa2d6526 100644
--- a/crates/vfs/src/file_set.rs
+++ b/crates/vfs/src/file_set.rs
@@ -23,13 +23,22 @@ 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 file_for_path(&self, path: &VfsPath) -> Option<&FileId> {
+        self.files.get(path)
     }
+
+    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<Item = FileId> + '_ {
         self.paths.keys().copied()
     }
diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs
index 944a702df..022a0be1e 100644
--- a/crates/vfs/src/vfs_path.rs
+++ b/crates/vfs/src/vfs_path.rs
@@ -48,6 +48,24 @@ impl VfsPath {
             (VfsPathRepr::VirtualPath(_), _) => false,
         }
     }
+    pub fn parent(&self) -> Option<VfsPath> {
+        let mut parent = self.clone();
+        if parent.pop() {
+            Some(parent)
+        } else {
+            None
+        }
+    }
+
+    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.name_and_extension(),
+        }
+    }
 
     // Don't make this `pub`
     pub(crate) fn encode(&self, buf: &mut Vec<u8>) {
@@ -268,4 +286,60 @@ impl VirtualPath {
         res.0 = format!("{}/{}", res.0, path);
         Some(res)
     }
+
+    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..],
+            None => file_path,
+        };
+
+        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)),
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn virtual_path_extensions() {
+        assert_eq!(VirtualPath("/".to_string()).name_and_extension(), None);
+        assert_eq!(
+            VirtualPath("/directory".to_string()).name_and_extension(),
+            Some(("directory", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/".to_string()).name_and_extension(),
+            Some(("directory", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/file".to_string()).name_and_extension(),
+            Some(("file", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/.file".to_string()).name_and_extension(),
+            Some((".file", None))
+        );
+        assert_eq!(
+            VirtualPath("/directory/.file.rs".to_string()).name_and_extension(),
+            Some((".file", Some("rs")))
+        );
+        assert_eq!(
+            VirtualPath("/directory/file.rs".to_string()).name_and_extension(),
+            Some(("file", Some("rs")))
+        );
+    }
 }