Auto merge of #14266 - Veykril:generalize-eager-lazy, r=Veykril

feature: Make replace_or_with_or_else assists more generally applicable
This commit is contained in:
bors 2023-03-06 21:41:46 +00:00
commit 31c12ec282
10 changed files with 415 additions and 522 deletions

View file

@ -1677,6 +1677,10 @@ impl Function {
.collect()
}
pub fn num_params(self, db: &dyn HirDatabase) -> usize {
db.function_data(self.id).params.len()
}
pub fn method_params(self, db: &dyn HirDatabase) -> Option<Vec<Param>> {
if self.self_param(db).is_none() {
return None;
@ -3857,11 +3861,13 @@ impl Type {
}
}
// FIXME: Document this
#[derive(Debug)]
pub struct Callable {
ty: Type,
sig: CallableSig,
callee: Callee,
/// Whether this is a method that was called with method call syntax.
pub(crate) is_bound_method: bool,
}
@ -3895,14 +3901,14 @@ impl Callable {
Other => CallableKind::Other,
}
}
pub fn receiver_param(&self, db: &dyn HirDatabase) -> Option<ast::SelfParam> {
pub fn receiver_param(&self, db: &dyn HirDatabase) -> Option<(ast::SelfParam, Type)> {
let func = match self.callee {
Callee::Def(CallableDefId::FunctionId(it)) if self.is_bound_method => it,
_ => return None,
};
let src = func.lookup(db.upcast()).source(db.upcast());
let param_list = src.value.param_list()?;
param_list.self_param()
Some((param_list.self_param()?, self.ty.derived(self.sig.params()[0].clone())))
}
pub fn n_params(&self) -> usize {
self.sig.params().len() - if self.is_bound_method { 1 } else { 0 }

View file

@ -0,0 +1,310 @@
use ide_db::assists::{AssistId, AssistKind};
use syntax::{
ast::{self, make, Expr, HasArgList},
AstNode,
};
use crate::{AssistContext, Assists};
// Assist: replace_with_lazy_method
//
// Replace `unwrap_or` with `unwrap_or_else` and `ok_or` with `ok_or_else`.
//
// ```
// # //- minicore:option, fn
// fn foo() {
// let a = Some(1);
// a.unwra$0p_or(2);
// }
// ```
// ->
// ```
// fn foo() {
// let a = Some(1);
// a.unwrap_or_else(|| 2);
// }
// ```
pub(crate) fn replace_with_lazy_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
let scope = ctx.sema.scope(call.syntax())?;
let last_arg = call.arg_list()?.args().next()?;
let method_name = call.name_ref()?;
let callable = ctx.sema.resolve_method_call_as_callable(&call)?;
let (_, receiver_ty) = callable.receiver_param(ctx.sema.db)?;
let n_params = callable.n_params() + 1;
let method_name_lazy = format!(
"{method_name}{}",
if method_name.text().ends_with("or") { "_else" } else { "_with" }
);
receiver_ty.iterate_method_candidates_with_traits(
ctx.sema.db,
&scope,
&scope.visible_traits().0,
None,
None,
|func| {
let valid = func.name(ctx.sema.db).as_str() == Some(&*method_name_lazy)
&& func.num_params(ctx.sema.db) == n_params
&& {
let params = func.params_without_self(ctx.sema.db);
let last_p = params.first()?;
// FIXME: Check that this has the form of `() -> T` where T is the current type of the argument
last_p.ty().impls_fnonce(ctx.sema.db)
};
valid.then_some(func)
},
)?;
acc.add(
AssistId("replace_with_lazy_method", AssistKind::RefactorRewrite),
format!("Replace {method_name} with {method_name_lazy}"),
call.syntax().text_range(),
|builder| {
builder.replace(method_name.syntax().text_range(), method_name_lazy);
let closured = into_closure(&last_arg);
builder.replace_ast(last_arg, closured);
},
)
}
fn into_closure(param: &Expr) -> Expr {
(|| {
if let ast::Expr::CallExpr(call) = param {
if call.arg_list()?.args().count() == 0 {
Some(call.expr()?)
} else {
None
}
} else {
None
}
})()
.unwrap_or_else(|| make::expr_closure(None, param.clone()))
}
// Assist: replace_with_eager_method
//
// Replace `unwrap_or_else` with `unwrap_or` and `ok_or_else` with `ok_or`.
//
// ```
// # //- minicore:option, fn
// fn foo() {
// let a = Some(1);
// a.unwra$0p_or_else(|| 2);
// }
// ```
// ->
// ```
// fn foo() {
// let a = Some(1);
// a.unwrap_or(2);
// }
// ```
pub(crate) fn replace_with_eager_method(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
let scope = ctx.sema.scope(call.syntax())?;
let last_arg = call.arg_list()?.args().next()?;
let method_name = call.name_ref()?;
let callable = ctx.sema.resolve_method_call_as_callable(&call)?;
let (_, receiver_ty) = callable.receiver_param(ctx.sema.db)?;
let n_params = callable.n_params() + 1;
let params = callable.params(ctx.sema.db);
// FIXME: Check that the arg is of the form `() -> T`
if !params.first()?.1.impls_fnonce(ctx.sema.db) {
return None;
}
let method_name_text = method_name.text();
let method_name_eager = method_name_text
.strip_suffix("_else")
.or_else(|| method_name_text.strip_suffix("_with"))?;
receiver_ty.iterate_method_candidates_with_traits(
ctx.sema.db,
&scope,
&scope.visible_traits().0,
None,
None,
|func| {
let valid = func.name(ctx.sema.db).as_str() == Some(&*method_name_eager)
&& func.num_params(ctx.sema.db) == n_params;
valid.then_some(func)
},
)?;
acc.add(
AssistId("replace_with_eager_method", AssistKind::RefactorRewrite),
format!("Replace {method_name} with {method_name_eager}"),
call.syntax().text_range(),
|builder| {
builder.replace(method_name.syntax().text_range(), method_name_eager);
let called = into_call(&last_arg);
builder.replace_ast(last_arg, called);
},
)
}
fn into_call(param: &Expr) -> Expr {
(|| {
if let ast::Expr::ClosureExpr(closure) = param {
if closure.param_list()?.params().count() == 0 {
Some(closure.body()?)
} else {
None
}
} else {
None
}
})()
.unwrap_or_else(|| make::expr_call(param.clone(), make::arg_list(Vec::new())))
}
#[cfg(test)]
mod tests {
use crate::tests::check_assist;
use super::*;
#[test]
fn replace_or_with_or_else_simple() {
check_assist(
replace_with_lazy_method,
r#"
//- minicore: option, fn
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or(2);
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or_else(|| 2);
}
"#,
)
}
#[test]
fn replace_or_with_or_else_call() {
check_assist(
replace_with_lazy_method,
r#"
//- minicore: option, fn
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or(x());
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or_else(x);
}
"#,
)
}
#[test]
fn replace_or_with_or_else_block() {
check_assist(
replace_with_lazy_method,
r#"
//- minicore: option, fn
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or({
let mut x = bar();
for i in 0..10 {
x += i;
}
x
});
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or_else(|| {
let mut x = bar();
for i in 0..10 {
x += i;
}
x
});
}
"#,
)
}
#[test]
fn replace_or_else_with_or_simple() {
check_assist(
replace_with_eager_method,
r#"
//- minicore: option, fn
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or_else(|| 2);
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or(2);
}
"#,
)
}
#[test]
fn replace_or_else_with_or_call() {
check_assist(
replace_with_eager_method,
r#"
//- minicore: option, fn
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or_else(x);
}
fn x() -> i32 { 0 }
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or(x());
}
fn x() -> i32 { 0 }
"#,
)
}
#[test]
fn replace_or_else_with_or_map() {
check_assist(
replace_with_eager_method,
r#"
//- minicore: option, fn
fn foo() {
let foo = Some("foo");
return foo.map$0_or_else(|| 42, |v| v.len());
}
"#,
r#"
fn foo() {
let foo = Some("foo");
return foo.map_or(42, |v| v.len());
}
"#,
)
}
}

View file

@ -1,364 +0,0 @@
use ide_db::{
assists::{AssistId, AssistKind},
famous_defs::FamousDefs,
};
use syntax::{
ast::{self, make, Expr, HasArgList},
AstNode,
};
use crate::{AssistContext, Assists};
// Assist: replace_or_with_or_else
//
// Replace `unwrap_or` with `unwrap_or_else` and `ok_or` with `ok_or_else`.
//
// ```
// # //- minicore:option
// fn foo() {
// let a = Some(1);
// a.unwra$0p_or(2);
// }
// ```
// ->
// ```
// fn foo() {
// let a = Some(1);
// a.unwrap_or_else(|| 2);
// }
// ```
pub(crate) fn replace_or_with_or_else(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
let kind = is_option_or_result(call.receiver()?, ctx)?;
let (name, arg_list) = (call.name_ref()?, call.arg_list()?);
let mut map_or = false;
let replace = match &*name.text() {
"unwrap_or" => "unwrap_or_else".to_string(),
"or" => "or_else".to_string(),
"ok_or" if kind == Kind::Option => "ok_or_else".to_string(),
"map_or" => {
map_or = true;
"map_or_else".to_string()
}
_ => return None,
};
let arg = match arg_list.args().collect::<Vec<_>>().as_slice() {
[] => make::arg_list(Vec::new()),
[first] => {
let param = into_closure(first);
make::arg_list(vec![param])
}
[first, second] if map_or => {
let param = into_closure(first);
make::arg_list(vec![param, second.clone()])
}
_ => return None,
};
acc.add(
AssistId("replace_or_with_or_else", AssistKind::RefactorRewrite),
format!("Replace {name} with {replace}"),
call.syntax().text_range(),
|builder| {
builder.replace(name.syntax().text_range(), replace);
builder.replace_ast(arg_list, arg)
},
)
}
fn into_closure(param: &Expr) -> Expr {
(|| {
if let ast::Expr::CallExpr(call) = param {
if call.arg_list()?.args().count() == 0 {
Some(call.expr()?)
} else {
None
}
} else {
None
}
})()
.unwrap_or_else(|| make::expr_closure(None, param.clone()))
}
// Assist: replace_or_else_with_or
//
// Replace `unwrap_or_else` with `unwrap_or` and `ok_or_else` with `ok_or`.
//
// ```
// # //- minicore:option
// fn foo() {
// let a = Some(1);
// a.unwra$0p_or_else(|| 2);
// }
// ```
// ->
// ```
// fn foo() {
// let a = Some(1);
// a.unwrap_or(2);
// }
// ```
pub(crate) fn replace_or_else_with_or(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
let kind = is_option_or_result(call.receiver()?, ctx)?;
let (name, arg_list) = (call.name_ref()?, call.arg_list()?);
let mut map_or = false;
let replace = match &*name.text() {
"unwrap_or_else" => "unwrap_or".to_string(),
"or_else" => "or".to_string(),
"ok_or_else" if kind == Kind::Option => "ok_or".to_string(),
"map_or_else" => {
map_or = true;
"map_or".to_string()
}
_ => return None,
};
let arg = match arg_list.args().collect::<Vec<_>>().as_slice() {
[] => make::arg_list(Vec::new()),
[first] => {
let param = into_call(first);
make::arg_list(vec![param])
}
[first, second] if map_or => {
let param = into_call(first);
make::arg_list(vec![param, second.clone()])
}
_ => return None,
};
acc.add(
AssistId("replace_or_else_with_or", AssistKind::RefactorRewrite),
format!("Replace {name} with {replace}"),
call.syntax().text_range(),
|builder| {
builder.replace(name.syntax().text_range(), replace);
builder.replace_ast(arg_list, arg)
},
)
}
fn into_call(param: &Expr) -> Expr {
(|| {
if let ast::Expr::ClosureExpr(closure) = param {
if closure.param_list()?.params().count() == 0 {
Some(closure.body()?)
} else {
None
}
} else {
None
}
})()
.unwrap_or_else(|| make::expr_call(param.clone(), make::arg_list(Vec::new())))
}
#[derive(PartialEq, Eq)]
enum Kind {
Option,
Result,
}
fn is_option_or_result(receiver: Expr, ctx: &AssistContext<'_>) -> Option<Kind> {
let ty = ctx.sema.type_of_expr(&receiver)?.adjusted().as_adt()?.as_enum()?;
let option_enum =
FamousDefs(&ctx.sema, ctx.sema.scope(receiver.syntax())?.krate()).core_option_Option();
if let Some(option_enum) = option_enum {
if ty == option_enum {
return Some(Kind::Option);
}
}
let result_enum =
FamousDefs(&ctx.sema, ctx.sema.scope(receiver.syntax())?.krate()).core_result_Result();
if let Some(result_enum) = result_enum {
if ty == result_enum {
return Some(Kind::Result);
}
}
None
}
#[cfg(test)]
mod tests {
use crate::tests::{check_assist, check_assist_not_applicable};
use super::*;
#[test]
fn replace_or_with_or_else_simple() {
check_assist(
replace_or_with_or_else,
r#"
//- minicore: option
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or(2);
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or_else(|| 2);
}
"#,
)
}
#[test]
fn replace_or_with_or_else_call() {
check_assist(
replace_or_with_or_else,
r#"
//- minicore: option
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or(x());
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or_else(x);
}
"#,
)
}
#[test]
fn replace_or_with_or_else_block() {
check_assist(
replace_or_with_or_else,
r#"
//- minicore: option
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or({
let mut x = bar();
for i in 0..10 {
x += i;
}
x
});
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or_else(|| {
let mut x = bar();
for i in 0..10 {
x += i;
}
x
});
}
"#,
)
}
#[test]
fn replace_or_else_with_or_simple() {
check_assist(
replace_or_else_with_or,
r#"
//- minicore: option
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or_else(|| 2);
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or(2);
}
"#,
)
}
#[test]
fn replace_or_else_with_or_call() {
check_assist(
replace_or_else_with_or,
r#"
//- minicore: option
fn foo() {
let foo = Some(1);
return foo.unwrap_$0or_else(x);
}
"#,
r#"
fn foo() {
let foo = Some(1);
return foo.unwrap_or(x());
}
"#,
)
}
#[test]
fn replace_or_else_with_or_result() {
check_assist(
replace_or_else_with_or,
r#"
//- minicore: result
fn foo() {
let foo = Ok(1);
return foo.unwrap_$0or_else(x);
}
"#,
r#"
fn foo() {
let foo = Ok(1);
return foo.unwrap_or(x());
}
"#,
)
}
#[test]
fn replace_or_else_with_or_map() {
check_assist(
replace_or_else_with_or,
r#"
//- minicore: result
fn foo() {
let foo = Ok("foo");
return foo.map$0_or_else(|| 42, |v| v.len());
}
"#,
r#"
fn foo() {
let foo = Ok("foo");
return foo.map_or(42, |v| v.len());
}
"#,
)
}
#[test]
fn replace_or_else_with_or_not_applicable() {
check_assist_not_applicable(
replace_or_else_with_or,
r#"
fn foo() {
let foo = Ok(1);
return foo.unwrap_$0or_else(x);
}
"#,
)
}
}

View file

@ -188,7 +188,7 @@ mod handlers {
mod replace_try_expr_with_match;
mod replace_derive_with_manual_impl;
mod replace_if_let_with_match;
mod replace_or_with_or_else;
mod replace_method_eager_lazy;
mod replace_arith_op;
mod introduce_named_generic;
mod replace_let_with_if_let;
@ -297,8 +297,8 @@ mod handlers {
replace_if_let_with_match::replace_if_let_with_match,
replace_if_let_with_match::replace_match_with_if_let,
replace_let_with_if_let::replace_let_with_if_let,
replace_or_with_or_else::replace_or_else_with_or,
replace_or_with_or_else::replace_or_with_or_else,
replace_method_eager_lazy::replace_with_eager_method,
replace_method_eager_lazy::replace_with_lazy_method,
replace_turbofish_with_explicit_type::replace_turbofish_with_explicit_type,
replace_qualified_name_with_use::replace_qualified_name_with_use,
replace_arith_op::replace_arith_with_wrapping,

View file

@ -2313,46 +2313,6 @@ fn handle(action: Action) {
)
}
#[test]
fn doctest_replace_or_else_with_or() {
check_doc_test(
"replace_or_else_with_or",
r#####"
//- minicore:option
fn foo() {
let a = Some(1);
a.unwra$0p_or_else(|| 2);
}
"#####,
r#####"
fn foo() {
let a = Some(1);
a.unwrap_or(2);
}
"#####,
)
}
#[test]
fn doctest_replace_or_with_or_else() {
check_doc_test(
"replace_or_with_or_else",
r#####"
//- minicore:option
fn foo() {
let a = Some(1);
a.unwra$0p_or(2);
}
"#####,
r#####"
fn foo() {
let a = Some(1);
a.unwrap_or_else(|| 2);
}
"#####,
)
}
#[test]
fn doctest_replace_qualified_name_with_use() {
check_doc_test(
@ -2427,6 +2387,46 @@ fn main() {
)
}
#[test]
fn doctest_replace_with_eager_method() {
check_doc_test(
"replace_with_eager_method",
r#####"
//- minicore:option, fn
fn foo() {
let a = Some(1);
a.unwra$0p_or_else(|| 2);
}
"#####,
r#####"
fn foo() {
let a = Some(1);
a.unwrap_or(2);
}
"#####,
)
}
#[test]
fn doctest_replace_with_lazy_method() {
check_doc_test(
"replace_with_lazy_method",
r#####"
//- minicore:option, fn
fn foo() {
let a = Some(1);
a.unwra$0p_or(2);
}
"#####,
r#####"
fn foo() {
let a = Some(1);
a.unwrap_or_else(|| 2);
}
"#####,
)
}
#[test]
fn doctest_sort_items() {
check_doc_test(

View file

@ -561,7 +561,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> {
.sema
.resolve_method_call_as_callable(code)
.and_then(|callable| callable.receiver_param(self.sema.db))
.map(|self_param| self_param.kind())
.map(|(self_param, _)| self_param.kind())
.unwrap_or(ast::SelfParamKind::Owned);
}
}

View file

@ -176,15 +176,12 @@ fn pat_is_enum_variant(db: &RootDatabase, bind_pat: &ast::IdentPat, pat_ty: &hir
mod tests {
// This module also contains tests for super::closure_ret
use expect_test::expect;
use syntax::{TextRange, TextSize};
use test_utils::extract_annotations;
use crate::{fixture, inlay_hints::InlayHintsConfig};
use crate::inlay_hints::tests::{
check, check_expect, check_with_config, DISABLED_CONFIG, TEST_CONFIG,
};
use crate::inlay_hints::tests::{check, check_with_config, DISABLED_CONFIG, TEST_CONFIG};
use crate::ClosureReturnTypeHints;
#[track_caller]
@ -278,8 +275,7 @@ fn main() {
#[test]
fn iterator_hint_regression_issue_12674() {
// Ensure we don't crash while solving the projection type of iterators.
check_expect(
InlayHintsConfig { chaining_hints: true, ..DISABLED_CONFIG },
let (analysis, file_id) = fixture::file(
r#"
//- minicore: iterators
struct S<T>(T);
@ -306,103 +302,14 @@ fn main(a: SliceIter<'_, Container>) {
.map(|e| e);
}
"#,
expect![[r#"
[
InlayHint {
range: 484..554,
kind: Chaining,
label: [
"impl ",
InlayHintLabelPart {
text: "Iterator",
linked_location: Some(
FileRange {
file_id: FileId(
1,
),
range: 2611..2619,
},
),
tooltip: "",
},
"<",
InlayHintLabelPart {
text: "Item",
linked_location: Some(
FileRange {
file_id: FileId(
1,
),
range: 2643..2647,
},
),
tooltip: "",
},
" = impl ",
InlayHintLabelPart {
text: "Iterator",
linked_location: Some(
FileRange {
file_id: FileId(
1,
),
range: 2611..2619,
},
),
tooltip: "",
},
"<",
InlayHintLabelPart {
text: "Item",
linked_location: Some(
FileRange {
file_id: FileId(
1,
),
range: 2643..2647,
},
),
tooltip: "",
},
" = &&str>>",
],
},
InlayHint {
range: 484..485,
kind: Chaining,
label: [
"",
InlayHintLabelPart {
text: "SliceIter",
linked_location: Some(
FileRange {
file_id: FileId(
0,
),
range: 289..298,
},
),
tooltip: "",
},
"<",
InlayHintLabelPart {
text: "Container",
linked_location: Some(
FileRange {
file_id: FileId(
0,
),
range: 238..247,
},
),
tooltip: "",
},
">",
],
},
]
"#]],
);
analysis
.inlay_hints(
&InlayHintsConfig { chaining_hints: true, ..DISABLED_CONFIG },
file_id,
None,
)
.unwrap();
}
#[test]

View file

@ -435,7 +435,7 @@ fn main() {
file_id: FileId(
1,
),
range: 2611..2619,
range: 3386..3394,
},
),
tooltip: "",
@ -448,7 +448,7 @@ fn main() {
file_id: FileId(
1,
),
range: 2643..2647,
range: 3418..3422,
},
),
tooltip: "",
@ -468,7 +468,7 @@ fn main() {
file_id: FileId(
1,
),
range: 2611..2619,
range: 3386..3394,
},
),
tooltip: "",
@ -481,7 +481,7 @@ fn main() {
file_id: FileId(
1,
),
range: 2643..2647,
range: 3418..3422,
},
),
tooltip: "",
@ -501,7 +501,7 @@ fn main() {
file_id: FileId(
1,
),
range: 2611..2619,
range: 3386..3394,
},
),
tooltip: "",
@ -514,7 +514,7 @@ fn main() {
file_id: FileId(
1,
),
range: 2643..2647,
range: 3418..3422,
},
),
tooltip: "",

View file

@ -172,7 +172,7 @@ fn signature_help_for_call(
res.signature.push('(');
{
if let Some(self_param) = callable.receiver_param(db) {
if let Some((self_param, _)) = callable.receiver_param(db) {
format_to!(res.signature, "{}", self_param)
}
let mut buf = String::new();

View file

@ -534,6 +534,40 @@ pub mod option {
None => panic!("called `Option::unwrap()` on a `None` value"),
}
}
pub fn and<U>(self, optb: Option<U>) -> Option<U> {
loop {}
}
pub fn unwrap_or(self, default: T) -> T {
loop {}
}
// region:fn
pub fn and_then<U, F>(self, f: F) -> Option<U>
where
F: FnOnce(T) -> Option<U>,
{
loop {}
}
pub fn unwrap_or_else<F>(self, f: F) -> T
where
F: FnOnce() -> T,
{
loop {}
}
pub fn map_or<U, F>(self, default: U, f: F) -> U
where
F: FnOnce(T) -> U,
{
loop {}
}
pub fn map_or_else<U, D, F>(self, default: D, f: F) -> U
where
D: FnOnce() -> U,
F: FnOnce(T) -> U,
{
loop {}
}
// endregion:fn
}
}
// endregion:option