Auto merge of #14238 - lowr:feat/allow-generate-fn-across-local-crates, r=Veykril

feat: allow `generate_function` to generate in different local crate

Closes #14224

This PR allows `generate_function` assist to generate in crates other than the current one. I took a step further from the original request and made it allow to generate in any local crates since it looked reasonable and IDE layer doesn't really know about packages.

(actually we have been checking which crate we're generating in only for methods and not for freestanding functions, so we were providing the assist for `std::foo$0()`; it's both feature and fix in a sense)

The first commit is a drive-by fix unrelated to the feature.

8 files changed, 143 insertions, 34 deletions

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index c64106d3afd..26f678e5a9b 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -489,7 +489,7 @@ impl Module {
     }
 
     /// Finds nearest non-block ancestor `Module` (`self` included).
-    fn nearest_non_block_module(self, db: &dyn HirDatabase) -> Module {
+    pub fn nearest_non_block_module(self, db: &dyn HirDatabase) -> Module {
         let mut id = self.id;
         loop {
             let def_map = id.def_map(db.upcast());
diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs
index 45b27a63ce2..eef037f9875 100644
--- a/crates/ide-assists/src/handlers/generate_function.rs
+++ b/crates/ide-assists/src/handlers/generate_function.rs
@@ -5,6 +5,7 @@ use ide_db::{
     base_db::FileId,
     defs::{Definition, NameRefClass},
     famous_defs::FamousDefs,
+    helpers::is_editable_crate,
     path_transform::PathTransform,
     FxHashMap, FxHashSet, RootDatabase, SnippetCap,
 };
@@ -65,6 +66,13 @@ fn gen_fn(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let fn_name = &*name_ref.text();
     let TargetInfo { target_module, adt_name, target, file, insert_offset } =
         fn_target_info(ctx, path, &call, fn_name)?;
+
+    if let Some(m) = target_module {
+        if !is_editable_crate(m.krate(), ctx.db()) {
+            return None;
+        }
+    }
+
     let function_builder = FunctionBuilder::from_call(ctx, &call, fn_name, target_module, target)?;
     let text_range = call.syntax().text_range();
     let label = format!("Generate {} function", function_builder.fn_name);
@@ -141,12 +149,11 @@ fn gen_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let receiver_ty = ctx.sema.type_of_expr(&call.receiver()?)?.original().strip_references();
     let adt = receiver_ty.as_adt()?;
 
-    let current_module = ctx.sema.scope(call.syntax())?.module();
     let target_module = adt.module(ctx.sema.db);
-
-    if current_module.krate() != target_module.krate() {
+    if !is_editable_crate(target_module.krate(), ctx.db()) {
         return None;
     }
+
     let (impl_, file) = get_adt_source(ctx, &adt, fn_name.text().as_str())?;
     let (target, insert_offset) = get_method_target(ctx, &impl_, &adt)?;
 
@@ -253,7 +260,7 @@ struct FunctionBuilder {
     params: ast::ParamList,
     ret_type: Option<ast::RetType>,
     should_focus_return_type: bool,
-    needs_pub: bool,
+    visibility: Visibility,
     is_async: bool,
 }
 
@@ -264,12 +271,14 @@ impl FunctionBuilder {
         ctx: &AssistContext<'_>,
         call: &ast::CallExpr,
         fn_name: &str,
-        target_module: Option<hir::Module>,
+        target_module: Option<Module>,
         target: GeneratedFunctionTarget,
     ) -> Option<Self> {
-        let needs_pub = target_module.is_some();
         let target_module =
             target_module.or_else(|| ctx.sema.scope(target.syntax()).map(|it| it.module()))?;
+
+        let current_module = ctx.sema.scope(call.syntax())?.module();
+        let visibility = calculate_necessary_visibility(current_module, target_module, ctx);
         let fn_name = make::name(fn_name);
         let mut necessary_generic_params = FxHashSet::default();
         let params = fn_args(
@@ -300,7 +309,7 @@ impl FunctionBuilder {
             params,
             ret_type,
             should_focus_return_type,
-            needs_pub,
+            visibility,
             is_async,
         })
     }
@@ -313,8 +322,9 @@ impl FunctionBuilder {
         target_module: Module,
         target: GeneratedFunctionTarget,
     ) -> Option<Self> {
-        let needs_pub =
-            !module_is_descendant(&ctx.sema.scope(call.syntax())?.module(), &target_module, ctx);
+        let current_module = ctx.sema.scope(call.syntax())?.module();
+        let visibility = calculate_necessary_visibility(current_module, target_module, ctx);
+
         let fn_name = make::name(&name.text());
         let mut necessary_generic_params = FxHashSet::default();
         necessary_generic_params.extend(receiver_ty.generic_params(ctx.db()));
@@ -346,7 +356,7 @@ impl FunctionBuilder {
             params,
             ret_type,
             should_focus_return_type,
-            needs_pub,
+            visibility,
             is_async,
         })
     }
@@ -354,7 +364,11 @@ impl FunctionBuilder {
     fn render(self, is_method: bool) -> FunctionTemplate {
         let placeholder_expr = make::ext::expr_todo();
         let fn_body = make::block_expr(vec![], Some(placeholder_expr));
-        let visibility = if self.needs_pub { Some(make::visibility_pub_crate()) } else { None };
+        let visibility = match self.visibility {
+            Visibility::None => None,
+            Visibility::Crate => Some(make::visibility_pub_crate()),
+            Visibility::Pub => Some(make::visibility_pub()),
+        };
         let mut fn_def = make::fn_(
             visibility,
             self.fn_name,
@@ -527,7 +541,7 @@ impl GeneratedFunctionTarget {
 /// Computes parameter list for the generated function.
 fn fn_args(
     ctx: &AssistContext<'_>,
-    target_module: hir::Module,
+    target_module: Module,
     call: ast::CallableExpr,
     necessary_generic_params: &mut FxHashSet<hir::GenericParam>,
 ) -> Option<ast::ParamList> {
@@ -957,13 +971,13 @@ fn fn_arg_name(sema: &Semantics<'_, RootDatabase>, arg_expr: &ast::Expr) -> Stri
 
 fn fn_arg_type(
     ctx: &AssistContext<'_>,
-    target_module: hir::Module,
+    target_module: Module,
     fn_arg: &ast::Expr,
     generic_params: &mut FxHashSet<hir::GenericParam>,
 ) -> String {
     fn maybe_displayed_type(
         ctx: &AssistContext<'_>,
-        target_module: hir::Module,
+        target_module: Module,
         fn_arg: &ast::Expr,
         generic_params: &mut FxHashSet<hir::GenericParam>,
     ) -> Option<String> {
@@ -1048,16 +1062,29 @@ fn next_space_for_fn_in_impl(impl_: &ast::Impl) -> Option<GeneratedFunctionTarge
     }
 }
 
-fn module_is_descendant(module: &hir::Module, ans: &hir::Module, ctx: &AssistContext<'_>) -> bool {
-    if module == ans {
-        return true;
-    }
-    for c in ans.children(ctx.sema.db) {
-        if module_is_descendant(module, &c, ctx) {
-            return true;
-        }
+#[derive(Clone, Copy)]
+enum Visibility {
+    None,
+    Crate,
+    Pub,
+}
+
+fn calculate_necessary_visibility(
+    current_module: Module,
+    target_module: Module,
+    ctx: &AssistContext<'_>,
+) -> Visibility {
+    let db = ctx.db();
+    let current_module = current_module.nearest_non_block_module(db);
+    let target_module = target_module.nearest_non_block_module(db);
+
+    if target_module.krate() != current_module.krate() {
+        Visibility::Pub
+    } else if current_module.path_to_root(db).contains(&target_module) {
+        Visibility::None
+    } else {
+        Visibility::Crate
     }
-    false
 }
 
 // This is never intended to be used as a generic graph strucuture. If there's ever another need of
@@ -2656,4 +2683,79 @@ fn main() {
 ",
         )
     }
+
+    #[test]
+    fn applicable_in_different_local_crate() {
+        check_assist(
+            generate_function,
+            r"
+//- /lib.rs crate:lib new_source_root:local
+fn dummy() {}
+//- /main.rs crate:main deps:lib new_source_root:local
+fn main() {
+    lib::foo$0();
+}
+",
+            r"
+fn dummy() {}
+
+pub fn foo() ${0:-> _} {
+    todo!()
+}
+",
+        );
+    }
+
+    #[test]
+    fn applicable_in_different_local_crate_method() {
+        check_assist(
+            generate_function,
+            r"
+//- /lib.rs crate:lib new_source_root:local
+pub struct S;
+//- /main.rs crate:main deps:lib new_source_root:local
+fn main() {
+    lib::S.foo$0();
+}
+",
+            r"
+pub struct S;
+impl S {
+    pub fn foo(&self) ${0:-> _} {
+        todo!()
+    }
+}
+",
+        );
+    }
+
+    #[test]
+    fn not_applicable_in_different_library_crate() {
+        check_assist_not_applicable(
+            generate_function,
+            r"
+//- /lib.rs crate:lib new_source_root:library
+fn dummy() {}
+//- /main.rs crate:main deps:lib new_source_root:local
+fn main() {
+    lib::foo$0();
+}
+",
+        );
+    }
+
+    #[test]
+    fn not_applicable_in_different_library_crate_method() {
+        check_assist_not_applicable(
+            generate_function,
+            r"
+//- /lib.rs crate:lib new_source_root:library
+pub struct S;
+//- /main.rs crate:main deps:lib new_source_root:local
+fn main() {
+    lib::S.foo$0();
+}
+",
+        );
+    }
 }
diff --git a/crates/ide-assists/src/handlers/generate_new.rs b/crates/ide-assists/src/handlers/generate_new.rs
index 8d311262a75..e30a3e942c4 100644
--- a/crates/ide-assists/src/handlers/generate_new.rs
+++ b/crates/ide-assists/src/handlers/generate_new.rs
@@ -1,5 +1,5 @@
 use ide_db::{
-    imports::import_assets::item_for_path_search, use_trivial_contructor::use_trivial_constructor,
+    imports::import_assets::item_for_path_search, use_trivial_constructor::use_trivial_constructor,
 };
 use itertools::Itertools;
 use stdx::format_to;
diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs
index ea54068b0f8..d5dda6dae4f 100644
--- a/crates/ide-completion/src/context.rs
+++ b/crates/ide-completion/src/context.rs
@@ -6,13 +6,13 @@ mod tests;
 
 use std::iter;
 
-use base_db::SourceDatabaseExt;
 use hir::{
     HasAttrs, Local, Name, PathResolution, ScopeDef, Semantics, SemanticsScope, Type, TypeInfo,
 };
 use ide_db::{
     base_db::{FilePosition, SourceDatabase},
     famous_defs::FamousDefs,
+    helpers::is_editable_crate,
     FxHashMap, FxHashSet, RootDatabase,
 };
 use syntax::{
@@ -525,10 +525,11 @@ impl<'a> CompletionContext<'a> {
                 return Visible::No;
             }
             // If the definition location is editable, also show private items
-            let root_file = defining_crate.root_file(self.db);
-            let source_root_id = self.db.file_source_root(root_file);
-            let is_editable = !self.db.source_root(source_root_id).is_library;
-            return if is_editable { Visible::Editable } else { Visible::No };
+            return if is_editable_crate(defining_crate, self.db) {
+                Visible::Editable
+            } else {
+                Visible::No
+            };
         }
 
         if self.is_doc_hidden(attrs, defining_crate) {
diff --git a/crates/ide-db/src/helpers.rs b/crates/ide-db/src/helpers.rs
index 6e56efe344d..8e3b1eef15b 100644
--- a/crates/ide-db/src/helpers.rs
+++ b/crates/ide-db/src/helpers.rs
@@ -2,8 +2,8 @@
 
 use std::collections::VecDeque;
 
-use base_db::FileId;
-use hir::{ItemInNs, ModuleDef, Name, Semantics};
+use base_db::{FileId, SourceDatabaseExt};
+use hir::{Crate, ItemInNs, ModuleDef, Name, Semantics};
 use syntax::{
     ast::{self, make},
     AstToken, SyntaxKind, SyntaxToken, TokenAtOffset,
@@ -103,3 +103,9 @@ pub fn lint_eq_or_in_group(lint: &str, lint_is: &str) -> bool {
         false
     }
 }
+
+pub fn is_editable_crate(krate: Crate, db: &RootDatabase) -> bool {
+    let root_file = krate.root_file(db);
+    let source_root_id = db.file_source_root(root_file);
+    !db.source_root(source_root_id).is_library
+}
diff --git a/crates/ide-db/src/lib.rs b/crates/ide-db/src/lib.rs
index 156bbb634e4..e75694e50fd 100644
--- a/crates/ide-db/src/lib.rs
+++ b/crates/ide-db/src/lib.rs
@@ -22,7 +22,7 @@ pub mod source_change;
 pub mod symbol_index;
 pub mod traits;
 pub mod ty_filter;
-pub mod use_trivial_contructor;
+pub mod use_trivial_constructor;
 
 pub mod imports {
     pub mod import_assets;
diff --git a/crates/ide-db/src/use_trivial_contructor.rs b/crates/ide-db/src/use_trivial_constructor.rs
index 39431bed382..39431bed382 100644
--- a/crates/ide-db/src/use_trivial_contructor.rs
+++ b/crates/ide-db/src/use_trivial_constructor.rs
diff --git a/crates/ide-diagnostics/src/handlers/missing_fields.rs b/crates/ide-diagnostics/src/handlers/missing_fields.rs
index 43af4d4f16a..14039087b3f 100644
--- a/crates/ide-diagnostics/src/handlers/missing_fields.rs
+++ b/crates/ide-diagnostics/src/handlers/missing_fields.rs
@@ -5,7 +5,7 @@ use hir::{
 };
 use ide_db::{
     assists::Assist, famous_defs::FamousDefs, imports::import_assets::item_for_path_search,
-    source_change::SourceChange, use_trivial_contructor::use_trivial_constructor, FxHashMap,
+    source_change::SourceChange, use_trivial_constructor::use_trivial_constructor, FxHashMap,
 };
 use stdx::format_to;
 use syntax::{