aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-04-13 12:39:03 +0100
committerGitHub <[email protected]>2021-04-13 12:39:03 +0100
commite6728a8cd39f9111dcdd654c7c65e99e5a2f1190 (patch)
tree2532d24e0c10c7e3284033df4256f7adf96d2e82
parent0c02208fd8d9ff3ce8184e2eb5dba915d5b54656 (diff)
parent09a78caca403f1b4be8711d00519094896928e58 (diff)
Merge #8415
8415: Fix faulty assertion when extracting function with macro call r=matklad a=brandondong **Reproduction:** ```rust fn main() { let n = 1; let k = n * n; dbg!(n); } ``` 1. Select the second and third lines of the main function. Use the "Extract into function" code assist. 2. Panic occurs in debug, error is logged in release: "[ERROR ide_assists::handlers::extract_function] assertion failed: matches!(path, ast :: Expr :: PathExpr(_))". 3. Function generates successfully on release where the panic was bypassed. ```rust fn fun_name(n: i32) { let k = n * n; dbg!(n); } ``` **Cause:** - The generated function will take `n` as a parameter. The extraction logic needs to search the usages of `n` to determine whether it is used mutably or not. The helper `path_element_of_reference` is called for each usage but the second usage is a macro call and fails the `Expr::PathExpr(_)` match assertion. - The caller of `path_element_of_reference` does implicitly assume it to be a `Expr::PathExpr(_)` in how it looks at its parent node for determining whether it is used mutably. This logic will not work for macros. - I'm not sure if there are any other cases besides macros where it could be something other than a `Expr::PathExpr(_)`. I tried various examples and could not find any. **Fix:** - Update assertion to include the macro case. - Add a FIXME to properly handle checking if a macro usage requires mutable access. For now, return false instead of running the existing logic that is tailored for `Expr::PathExpr(_)`'s. Co-authored-by: Brandon <[email protected]>
-rw-r--r--crates/ide_assists/src/handlers/extract_function.rs48
1 files changed, 46 insertions, 2 deletions
diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs
index af9543766..059414274 100644
--- a/crates/ide_assists/src/handlers/extract_function.rs
+++ b/crates/ide_assists/src/handlers/extract_function.rs
@@ -736,6 +736,14 @@ fn reference_is_exclusive(
736 736
737/// checks if this expr requires `&mut` access, recurses on field access 737/// checks if this expr requires `&mut` access, recurses on field access
738fn expr_require_exclusive_access(ctx: &AssistContext, expr: &ast::Expr) -> Option<bool> { 738fn expr_require_exclusive_access(ctx: &AssistContext, expr: &ast::Expr) -> Option<bool> {
739 match expr {
740 ast::Expr::MacroCall(_) => {
741 // FIXME: expand macro and check output for mutable usages of the variable?
742 return None;
743 }
744 _ => (),
745 }
746
739 let parent = expr.syntax().parent()?; 747 let parent = expr.syntax().parent()?;
740 748
741 if let Some(bin_expr) = ast::BinExpr::cast(parent.clone()) { 749 if let Some(bin_expr) = ast::BinExpr::cast(parent.clone()) {
@@ -794,7 +802,7 @@ impl HasTokenAtOffset for SyntaxNode {
794 } 802 }
795} 803}
796 804
797/// find relevant `ast::PathExpr` for reference 805/// find relevant `ast::Expr` for reference
798/// 806///
799/// # Preconditions 807/// # Preconditions
800/// 808///
@@ -811,7 +819,11 @@ fn path_element_of_reference(
811 stdx::never!(false, "cannot find path parent of variable usage: {:?}", token); 819 stdx::never!(false, "cannot find path parent of variable usage: {:?}", token);
812 None 820 None
813 })?; 821 })?;
814 stdx::always!(matches!(path, ast::Expr::PathExpr(_))); 822 stdx::always!(
823 matches!(path, ast::Expr::PathExpr(_) | ast::Expr::MacroCall(_)),
824 "unexpected expression type for variable usage: {:?}",
825 path
826 );
815 Some(path) 827 Some(path)
816} 828}
817 829
@@ -3462,4 +3474,36 @@ fn foo() -> Result<(), i64> {
3462}"##, 3474}"##,
3463 ); 3475 );
3464 } 3476 }
3477
3478 #[test]
3479 fn param_usage_in_macro() {
3480 check_assist(
3481 extract_function,
3482 r"
3483macro_rules! m {
3484 ($val:expr) => { $val };
3485}
3486
3487fn foo() {
3488 let n = 1;
3489 $0let k = n * m!(n);$0
3490 let m = k + 1;
3491}",
3492 r"
3493macro_rules! m {
3494 ($val:expr) => { $val };
3495}
3496
3497fn foo() {
3498 let n = 1;
3499 let k = fun_name(n);
3500 let m = k + 1;
3501}
3502
3503fn $0fun_name(n: i32) -> i32 {
3504 let k = n * m!(n);
3505 k
3506}",
3507 );
3508 }
3465} 3509}