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 @@ pub fn parent(self, db: &dyn HirDatabase) -> Option { } /// 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 @@ 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, should_focus_return_type: bool, - needs_pub: bool, + visibility: Visibility, is_async: bool, } @@ -264,12 +271,14 @@ fn from_call( ctx: &AssistContext<'_>, call: &ast::CallExpr, fn_name: &str, - target_module: Option, + target_module: Option, target: GeneratedFunctionTarget, ) -> Option { - 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 @@ fn from_call( params, ret_type, should_focus_return_type, - needs_pub, + visibility, is_async, }) } @@ -313,8 +322,9 @@ fn from_method_call( target_module: Module, target: GeneratedFunctionTarget, ) -> Option { - 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 @@ fn from_method_call( params, ret_type, should_focus_return_type, - needs_pub, + visibility, is_async, }) } @@ -354,7 +364,11 @@ fn from_method_call( 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 @@ fn parent(&self) -> SyntaxNode { /// 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, ) -> Option { @@ -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, ) -> String { fn maybe_displayed_type( ctx: &AssistContext<'_>, - target_module: hir::Module, + target_module: Module, fn_arg: &ast::Expr, generic_params: &mut FxHashSet, ) -> Option { @@ -1048,16 +1062,29 @@ fn next_space_for_fn_in_impl(impl_: &ast::Impl) -> Option) -> bool { - if module == ans { - 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 } - for c in ans.children(ctx.sema.db) { - if module_is_descendant(module, &c, ctx) { - return true; - } - } - 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-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 @@ 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 @@ fn is_visible_impl( 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 +}