Less false positive completion candidates

1 files changed, 59 insertions, 44 deletions

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::<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)
+        });
+
     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::<Vec<_>>();
     dbg!(mod_declaration_candidates);
 
     // TODO kb exlude existing children from the candidates
-    let existing_children = current_module.children(ctx.db).collect::<Vec<_>>();
 
     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<VfsPath> {
     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;