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.
This commit is contained in:
bors 2023-03-03 10:15:27 +00:00
commit 3b857e1c38
8 changed files with 143 additions and 34 deletions

View File

@ -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());

View File

@ -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;
#[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();
}
",
);
}
}

View File

@ -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;

View File

@ -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) {

View File

@ -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
}

View File

@ -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;

View File

@ -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::{