Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow generate_function to generate in different local crate #14238

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
148 changes: 125 additions & 23 deletions crates/ide-assists/src/handlers/generate_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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,
}

Expand All @@ -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(
Expand Down Expand Up @@ -300,7 +309,7 @@ impl FunctionBuilder {
params,
ret_type,
should_focus_return_type,
needs_pub,
visibility,
is_async,
})
}
Expand All @@ -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()));
Expand Down Expand Up @@ -346,15 +356,19 @@ impl FunctionBuilder {
params,
ret_type,
should_focus_return_type,
needs_pub,
visibility,
is_async,
})
}

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,
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
",
);
}
}
2 changes: 1 addition & 1 deletion crates/ide-assists/src/handlers/generate_new.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
11 changes: 6 additions & 5 deletions crates/ide-completion/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 8 additions & 2 deletions crates/ide-db/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion crates/ide-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/missing_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down