Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #15662 - rmehri01:fix_panic_with_return_in_match, r=Veykril
fix: panic with wrapping/unwrapping result return type assists With the `wrap_return_type_in_result` assist, the following code results in a panic (note the lack of a semicolon): ```rust fn foo(num: i32) -> $0i32 { return num } => thread 'handlers::wrap_return_type_in_result::tests::wrap_return_in_tail_position' panicked at crates/syntax/src/ted.rs:137:41: called `Option::unwrap()` on a `None` value ``` I think this is because it first walks the body expression to change any `return` expressions and then walks all tail expressions, resulting in the `return num` being changed twice since it is both a `return` and in tail position. This can also happen when a `match` is in tail position and `return` is used in a branch for example. Not really sure how big of an issue this is in practice though since this seems to be the only case that is impacted and can be reduced to just `num` instead of `return num`. This also occurs with the `unwrap_result_return_type` assist but panics with the following instead: ``` thread 'handlers::unwrap_result_return_type::tests::wrap_return_in_tail_position' panicked at /rustc/3223b0b5e8dadda3f76c3fd1a8d6c5addc09599e/library/alloc/src/string.rs:1766:29: assertion failed: self.is_char_boundary(n) ```
bors 2023-09-26
parent c945f90 · parent 7306504 · commit 3b1b58c
-rw-r--r--crates/ide-assists/src/handlers/unwrap_result_return_type.rs24
-rw-r--r--crates/ide-assists/src/handlers/wrap_return_type_in_result.rs24
2 files changed, 40 insertions, 8 deletions
diff --git a/crates/ide-assists/src/handlers/unwrap_result_return_type.rs b/crates/ide-assists/src/handlers/unwrap_result_return_type.rs
index f235b554e6..03e6dfebeb 100644
--- a/crates/ide-assists/src/handlers/unwrap_result_return_type.rs
+++ b/crates/ide-assists/src/handlers/unwrap_result_return_type.rs
@@ -123,10 +123,8 @@ fn tail_cb_impl(acc: &mut Vec<ast::Expr>, e: &ast::Expr) {
for_each_tail_expr(&break_expr_arg, &mut |e| tail_cb_impl(acc, e))
}
}
- Expr::ReturnExpr(ret_expr) => {
- if let Some(ret_expr_arg) = &ret_expr.expr() {
- for_each_tail_expr(ret_expr_arg, &mut |e| tail_cb_impl(acc, e));
- }
+ Expr::ReturnExpr(_) => {
+ // all return expressions have already been handled by the walk loop
}
e => acc.push(e.clone()),
}
@@ -801,6 +799,24 @@ fn foo() -> i32 {
}
#[test]
+ fn wrap_return_in_tail_position() {
+ check_assist(
+ unwrap_result_return_type,
+ r#"
+//- minicore: result
+fn foo(num: i32) -> $0Result<i32, String> {
+ return Ok(num)
+}
+"#,
+ r#"
+fn foo(num: i32) -> i32 {
+ return num
+}
+"#,
+ );
+ }
+
+ #[test]
fn unwrap_result_return_type_simple_with_closure() {
check_assist(
unwrap_result_return_type,
diff --git a/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs b/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs
index 61e9bcdcc5..b68ed00f77 100644
--- a/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs
+++ b/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs
@@ -98,10 +98,8 @@ fn tail_cb_impl(acc: &mut Vec<ast::Expr>, e: &ast::Expr) {
for_each_tail_expr(&break_expr_arg, &mut |e| tail_cb_impl(acc, e))
}
}
- Expr::ReturnExpr(ret_expr) => {
- if let Some(ret_expr_arg) = &ret_expr.expr() {
- for_each_tail_expr(ret_expr_arg, &mut |e| tail_cb_impl(acc, e));
- }
+ Expr::ReturnExpr(_) => {
+ // all return expressions have already been handled by the walk loop
}
e => acc.push(e.clone()),
}
@@ -733,6 +731,24 @@ fn foo() -> Result<i32, ${0:_}> {
}
#[test]
+ fn wrap_return_in_tail_position() {
+ check_assist(
+ wrap_return_type_in_result,
+ r#"
+//- minicore: result
+fn foo(num: i32) -> $0i32 {
+ return num
+}
+"#,
+ r#"
+fn foo(num: i32) -> Result<i32, ${0:_}> {
+ return Ok(num)
+}
+"#,
+ );
+ }
+
+ #[test]
fn wrap_return_type_in_result_simple_with_closure() {
check_assist(
wrap_return_type_in_result,