mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #9871

Browse Source 
9871: Jump to generated func r=mahdi-frms a=rylev

Worked on this with `@yoshuawuyts.`

We thought we ran into an issue with the `generate_function` assist where the code was not being generated in a certain situations. However, it wasn't actually a bug just a very confusing implementation where the cursor is not moved to the newly generated function. This happened when the return type was successfully inferred (and not unit). The function would be generated, but selection would not be changed.

This can be very confusing: If the function is generated somewhat far from where the assist is being invoked, the user never sees that the code was generated (nor are they given the chance to actually implement the function body). 

This PR makes it so that the cursor is _always_ moved to somewhere in the newly generated function. In addition, if we can infer unit as the type, then we do not still generate `-> ()` as the return type. Instead, we simply omit the return type.

This means the selection will move to either one of two places:
* A generated `-> ()` return type when we cannot successfully infer the return type.
* The `todo!()` body when we can successfully infer the return type.

Co-authored-by: Ryan Levick <me@ryanlevick.com>
This commit is contained in:
bors[bot] 2021-08-13 11:19:16 +00:00 committed by GitHub
commit 43af7920af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 103 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

213
crates/ide_assists/src/handlers/generate_function.rs
View File

@ -171,20 +171,32 @@ struct FunctionTemplate {
insert_offset: TextSize,
leading_ws: String,
fn_def: ast::Fn,
ret_type: ast::RetType,
should_render_snippet: bool,
ret_type: Option<ast::RetType>,
should_focus_return_type: bool,
trailing_ws: String,
file: FileId,
tail_expr: ast::Expr,
}
impl FunctionTemplate {
fn to_string(&self, cap: Option<SnippetCap>) -> String {
let f = match (cap, self.should_render_snippet) {
(Some(cap), true) => {
render_snippet(cap, self.fn_def.syntax(), Cursor::Replace(self.ret_type.syntax()))
let f = match cap {
Some(cap) => {
let cursor = if self.should_focus_return_type {
// Focus the return type if there is one
if let Some(ref ret_type) = self.ret_type {
ret_type.syntax()
} else {
self.tail_expr.syntax()
}
} else {
self.tail_expr.syntax()
};
render_snippet(cap, self.fn_def.syntax(), Cursor::Replace(cursor))
}
_ => self.fn_def.to_string(),
None => self.fn_def.to_string(),
};
format!("{}{}{}", self.leading_ws, f, self.trailing_ws)
}
}
@ -194,8 +206,8 @@ struct FunctionBuilder {
fn_name: ast::Name,
type_params: Option<ast::GenericParamList>,
params: ast::ParamList,
ret_type: ast::RetType,
should_render_snippet: bool,
ret_type: Option<ast::RetType>,
should_focus_return_type: bool,
file: FileId,
needs_pub: bool,
is_async: bool,
@ -228,33 +240,8 @@ impl FunctionBuilder {
let await_expr = call.syntax().parent().and_then(ast::AwaitExpr::cast);
let is_async = await_expr.is_some();
// should_render_snippet intends to express a rough level of confidence about
// the correctness of the return type.
//
// If we are able to infer some return type, and that return type is not unit, we
// don't want to render the snippet. The assumption here is in this situation the
// return type is just as likely to be correct as any other part of the generated
// function.
//
// In the case where the return type is inferred as unit it is likely that the
// user does in fact intend for this generated function to return some non unit
// type, but that the current state of their code doesn't allow that return type
// to be accurately inferred.
let (ret_ty, should_render_snippet) = {
match ctx.sema.type_of_expr(&ast::Expr::CallExpr(call.clone())).map(TypeInfo::original)
{
Some(ty) if ty.is_unknown() || ty.is_unit() => (make::ty_unit(), true),
Some(ty) => {
let rendered = ty.display_source_code(ctx.db(), target_module.into());
match rendered {
Ok(rendered) => (make::ty(&rendered), false),
Err(_) => (make::ty_unit(), true),
}
}
None => (make::ty_unit(), true),
}
};
let ret_type = make::ret_type(ret_ty);
let (ret_type, should_focus_return_type) =
make_return_type(ctx, &ast::Expr::CallExpr(call.clone()), target_module);
Some(Self {
target,
@ -262,7 +249,7 @@ impl FunctionBuilder {
type_params,
params,
ret_type,
should_render_snippet,
should_focus_return_type,
file,
needs_pub,
is_async,
@ -298,36 +285,8 @@ impl FunctionBuilder {
let await_expr = call.syntax().parent().and_then(ast::AwaitExpr::cast);
let is_async = await_expr.is_some();
// should_render_snippet intends to express a rough level of confidence about
// the correctness of the return type.
//
// If we are able to infer some return type, and that return type is not unit, we
// don't want to render the snippet. The assumption here is in this situation the
// return type is just as likely to be correct as any other part of the generated
// function.
//
// In the case where the return type is inferred as unit it is likely that the
// user does in fact intend for this generated function to return some non unit
// type, but that the current state of their code doesn't allow that return type
// to be accurately inferred.
let (ret_ty, should_render_snippet) = {
match ctx
.sema
.type_of_expr(&ast::Expr::MethodCallExpr(call.clone()))
.map(TypeInfo::original)
{
Some(ty) if ty.is_unknown() || ty.is_unit() => (make::ty_unit(), true),
Some(ty) => {
let rendered = ty.display_source_code(ctx.db(), target_module.into());
match rendered {
Ok(rendered) => (make::ty(&rendered), false),
Err(_) => (make::ty_unit(), true),
}
}
None => (make::ty_unit(), true),
}
};
let ret_type = make::ret_type(ret_ty);
let (ret_type, should_focus_return_type) =
make_return_type(ctx, &ast::Expr::MethodCallExpr(call.clone()), target_module);
Some(Self {
target,
@ -335,7 +294,7 @@ impl FunctionBuilder {
type_params,
params,
ret_type,
should_render_snippet,
should_focus_return_type,
file,
needs_pub,
is_async,
@ -352,7 +311,7 @@ impl FunctionBuilder {
self.type_params,
self.params,
fn_body,
Some(self.ret_type),
self.ret_type,
self.is_async,
);
let leading_ws;
@ -378,8 +337,10 @@ impl FunctionBuilder {
FunctionTemplate {
insert_offset,
leading_ws,
ret_type: fn_def.ret_type().unwrap(),
should_render_snippet: self.should_render_snippet,
ret_type: fn_def.ret_type(),
// PANIC: we guarantee we always create a function body with a tail expr
tail_expr: fn_def.body().unwrap().tail_expr().unwrap(),
should_focus_return_type: self.should_focus_return_type,
fn_def,
trailing_ws,
file: self.file,
@ -387,6 +348,38 @@ impl FunctionBuilder {
}
}
/// Makes an optional return type along with whether the return type should be focused by the cursor.
/// If we cannot infer what the return type should be, we create unit as a placeholder.
///
/// The rule for whether we focus a return type or not (and thus focus the function body),
/// is rather simple:
/// * If we could *not* infer what the return type should be, focus it (so the user can fill-in
/// the correct return type).
/// * If we could infer the return type, don't focus it (and thus focus the function body) so the
/// user can change the `todo!` function body.
fn make_return_type(
ctx: &AssistContext,
call: &ast::Expr,
target_module: Module,
) -> (Option<ast::RetType>, bool) {
let (ret_ty, should_focus_return_type) = {
match ctx.sema.type_of_expr(call).map(TypeInfo::original) {
Some(ty) if ty.is_unknown() => (Some(make::ty_unit()), true),
None => (Some(make::ty_unit()), true),
Some(ty) if ty.is_unit() => (None, false),
Some(ty) => {
let rendered = ty.display_source_code(ctx.db(), target_module.into());
match rendered {
Ok(rendered) => (Some(make::ty(&rendered)), false),
Err(_) => (Some(make::ty_unit()), true),
}
}
}
};
let ret_type = ret_ty.map(|rt| make::ret_type(rt));
(ret_type, should_focus_return_type)
}
enum GeneratedFunctionTarget {
BehindItem(SyntaxNode),
InEmptyItemList(SyntaxNode),
@ -795,7 +788,7 @@ impl Baz {
}
fn bar(baz: Baz) -> Baz {
todo!()
${0:todo!()}
}
",
)
@ -815,8 +808,8 @@ fn foo() {
bar("bar")
}
fn bar(arg: &str) ${0:-> ()} {
todo!()
fn bar(arg: &str) {
${0:todo!()}
}
"#,
)
@ -836,8 +829,8 @@ fn foo() {
bar('x')
}
fn bar(arg: char) ${0:-> ()} {
todo!()
fn bar(arg: char) {
${0:todo!()}
}
"#,
)
@ -857,8 +850,8 @@ fn foo() {
bar(42)
}
fn bar(arg: i32) ${0:-> ()} {
todo!()
fn bar(arg: i32) {
${0:todo!()}
}
",
)
@ -878,8 +871,8 @@ fn foo() {
bar(42 as u8)
}
fn bar(arg: u8) ${0:-> ()} {
todo!()
fn bar(arg: u8) {
${0:todo!()}
}
",
)
@ -903,8 +896,8 @@ fn foo() {
bar(x as u8)
}
fn bar(x: u8) ${0:-> ()} {
todo!()
fn bar(x: u8) {
${0:todo!()}
}
",
)
@ -926,8 +919,8 @@ fn foo() {
bar(worble)
}
fn bar(worble: ()) ${0:-> ()} {
todo!()
fn bar(worble: ()) {
${0:todo!()}
}
",
)
@ -956,8 +949,8 @@ fn baz() {
bar(foo())
}
fn bar(foo: impl Foo) ${0:-> ()} {
todo!()
fn bar(foo: impl Foo) {
${0:todo!()}
}
"#,
)
@ -983,8 +976,8 @@ fn foo() {
bar(&baz())
}
fn bar(baz: &Baz) ${0:-> ()} {
todo!()
fn bar(baz: &Baz) {
${0:todo!()}
}
",
)
@ -1012,8 +1005,8 @@ fn foo() {
bar(Baz::baz())
}
fn bar(baz: Baz::Bof) ${0:-> ()} {
todo!()
fn bar(baz: Baz::Bof) {
${0:todo!()}
}
",
)
@ -1034,8 +1027,8 @@ fn foo<T>(t: T) {
bar(t)
}
fn bar(t: T) ${0:-> ()} {
todo!()
fn bar(t: T) {
${0:todo!()}
}
",
)
@ -1088,8 +1081,8 @@ fn foo() {
bar(closure)
}
fn bar(closure: ()) ${0:-> ()} {
todo!()
fn bar(closure: ()) {
${0:todo!()}
}
",
)
@ -1109,8 +1102,8 @@ fn foo() {
bar(baz)
}
fn bar(baz: ()) ${0:-> ()} {
todo!()
fn bar(baz: ()) {
${0:todo!()}
}
",
)
@ -1134,8 +1127,8 @@ fn foo() {
bar(baz(), baz())
}
fn bar(baz_1: Baz, baz_2: Baz) ${0:-> ()} {
todo!()
fn bar(baz_1: Baz, baz_2: Baz) {
${0:todo!()}
}
",
)
@ -1159,8 +1152,8 @@ fn foo() {
bar(baz(), baz(), "foo", "bar")
}
fn bar(baz_1: Baz, baz_2: Baz, arg_1: &str, arg_2: &str) ${0:-> ()} {
todo!()
fn bar(baz_1: Baz, baz_2: Baz, arg_1: &str, arg_2: &str) {
${0:todo!()}
}
"#,
)
@ -1179,8 +1172,8 @@ fn foo() {
",
r"
mod bar {
pub(crate) fn my_fn() ${0:-> ()} {
todo!()
pub(crate) fn my_fn() {
${0:todo!()}
}
}
@ -1215,8 +1208,8 @@ fn bar() {
baz(foo)
}
fn baz(foo: foo::Foo) ${0:-> ()} {
todo!()
fn baz(foo: foo::Foo) {
${0:todo!()}
}
"#,
)
@ -1239,8 +1232,8 @@ fn foo() {
mod bar {
fn something_else() {}
pub(crate) fn my_fn() ${0:-> ()} {
todo!()
pub(crate) fn my_fn() {
${0:todo!()}
}
}
@ -1267,8 +1260,8 @@ fn foo() {
r"
mod bar {
mod baz {
pub(crate) fn my_fn() ${0:-> ()} {
todo!()
pub(crate) fn my_fn() {
${0:todo!()}
}
}
}
@ -1296,8 +1289,8 @@ fn main() {
r"
pub(crate) fn bar() ${0:-> ()} {
todo!()
pub(crate) fn bar() {
${0:todo!()}
}",
)
}
@ -1317,7 +1310,7 @@ fn main() {
}
fn foo() -> u32 {
todo!()
${0:todo!()}
}
",
)