diff options
Diffstat (limited to 'crates/ide_assists/src/handlers/extract_function.rs')
-rw-r--r-- | crates/ide_assists/src/handlers/extract_function.rs | 128 |
1 files changed, 109 insertions, 19 deletions
diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 719f22053..059414274 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs | |||
@@ -75,7 +75,8 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option | |||
75 | let insert_after = scope_for_fn_insertion(&body, anchor)?; | 75 | let insert_after = scope_for_fn_insertion(&body, anchor)?; |
76 | let module = ctx.sema.scope(&insert_after).module()?; | 76 | let module = ctx.sema.scope(&insert_after).module()?; |
77 | 77 | ||
78 | let vars_defined_in_body_and_outlive = vars_defined_in_body_and_outlive(ctx, &body); | 78 | let vars_defined_in_body_and_outlive = |
79 | vars_defined_in_body_and_outlive(ctx, &body, &node.parent().as_ref().unwrap_or(&node)); | ||
79 | let ret_ty = body_return_ty(ctx, &body)?; | 80 | let ret_ty = body_return_ty(ctx, &body)?; |
80 | 81 | ||
81 | // FIXME: we compute variables that outlive here just to check `never!` condition | 82 | // FIXME: we compute variables that outlive here just to check `never!` condition |
@@ -257,7 +258,7 @@ struct Function { | |||
257 | control_flow: ControlFlow, | 258 | control_flow: ControlFlow, |
258 | ret_ty: RetType, | 259 | ret_ty: RetType, |
259 | body: FunctionBody, | 260 | body: FunctionBody, |
260 | vars_defined_in_body_and_outlive: Vec<Local>, | 261 | vars_defined_in_body_and_outlive: Vec<OutlivedLocal>, |
261 | } | 262 | } |
262 | 263 | ||
263 | #[derive(Debug)] | 264 | #[derive(Debug)] |
@@ -296,9 +297,9 @@ impl Function { | |||
296 | RetType::Expr(ty) => FunType::Single(ty.clone()), | 297 | RetType::Expr(ty) => FunType::Single(ty.clone()), |
297 | RetType::Stmt => match self.vars_defined_in_body_and_outlive.as_slice() { | 298 | RetType::Stmt => match self.vars_defined_in_body_and_outlive.as_slice() { |
298 | [] => FunType::Unit, | 299 | [] => FunType::Unit, |
299 | [var] => FunType::Single(var.ty(ctx.db())), | 300 | [var] => FunType::Single(var.local.ty(ctx.db())), |
300 | vars => { | 301 | vars => { |
301 | let types = vars.iter().map(|v| v.ty(ctx.db())).collect(); | 302 | let types = vars.iter().map(|v| v.local.ty(ctx.db())).collect(); |
302 | FunType::Tuple(types) | 303 | FunType::Tuple(types) |
303 | } | 304 | } |
304 | }, | 305 | }, |
@@ -562,6 +563,12 @@ impl HasTokenAtOffset for FunctionBody { | |||
562 | } | 563 | } |
563 | } | 564 | } |
564 | 565 | ||
566 | #[derive(Debug)] | ||
567 | struct OutlivedLocal { | ||
568 | local: Local, | ||
569 | mut_usage_outside_body: bool, | ||
570 | } | ||
571 | |||
565 | /// Try to guess what user wants to extract | 572 | /// Try to guess what user wants to extract |
566 | /// | 573 | /// |
567 | /// We have basically have two cases: | 574 | /// We have basically have two cases: |
@@ -707,10 +714,10 @@ fn has_exclusive_usages(ctx: &AssistContext, usages: &LocalUsages, body: &Functi | |||
707 | .any(|reference| reference_is_exclusive(reference, body, ctx)) | 714 | .any(|reference| reference_is_exclusive(reference, body, ctx)) |
708 | } | 715 | } |
709 | 716 | ||
710 | /// checks if this reference requires `&mut` access inside body | 717 | /// checks if this reference requires `&mut` access inside node |
711 | fn reference_is_exclusive( | 718 | fn reference_is_exclusive( |
712 | reference: &FileReference, | 719 | reference: &FileReference, |
713 | body: &FunctionBody, | 720 | node: &dyn HasTokenAtOffset, |
714 | ctx: &AssistContext, | 721 | ctx: &AssistContext, |
715 | ) -> bool { | 722 | ) -> bool { |
716 | // we directly modify variable with set: `n = 0`, `n += 1` | 723 | // we directly modify variable with set: `n = 0`, `n += 1` |
@@ -719,7 +726,7 @@ fn reference_is_exclusive( | |||
719 | } | 726 | } |
720 | 727 | ||
721 | // we take `&mut` reference to variable: `&mut v` | 728 | // we take `&mut` reference to variable: `&mut v` |
722 | let path = match path_element_of_reference(body, reference) { | 729 | let path = match path_element_of_reference(node, reference) { |
723 | Some(path) => path, | 730 | Some(path) => path, |
724 | None => return false, | 731 | None => return false, |
725 | }; | 732 | }; |
@@ -832,10 +839,16 @@ fn vars_defined_in_body(body: &FunctionBody, ctx: &AssistContext) -> Vec<Local> | |||
832 | } | 839 | } |
833 | 840 | ||
834 | /// list local variables defined inside `body` that should be returned from extracted function | 841 | /// list local variables defined inside `body` that should be returned from extracted function |
835 | fn vars_defined_in_body_and_outlive(ctx: &AssistContext, body: &FunctionBody) -> Vec<Local> { | 842 | fn vars_defined_in_body_and_outlive( |
836 | let mut vars_defined_in_body = vars_defined_in_body(&body, ctx); | 843 | ctx: &AssistContext, |
837 | vars_defined_in_body.retain(|var| var_outlives_body(ctx, body, var)); | 844 | body: &FunctionBody, |
845 | parent: &SyntaxNode, | ||
846 | ) -> Vec<OutlivedLocal> { | ||
847 | let vars_defined_in_body = vars_defined_in_body(&body, ctx); | ||
838 | vars_defined_in_body | 848 | vars_defined_in_body |
849 | .into_iter() | ||
850 | .filter_map(|var| var_outlives_body(ctx, body, var, parent)) | ||
851 | .collect() | ||
839 | } | 852 | } |
840 | 853 | ||
841 | /// checks if the relevant local was defined before(outside of) body | 854 | /// checks if the relevant local was defined before(outside of) body |
@@ -855,11 +868,23 @@ fn either_syntax(value: &Either<ast::IdentPat, ast::SelfParam>) -> &SyntaxNode { | |||
855 | } | 868 | } |
856 | } | 869 | } |
857 | 870 | ||
858 | /// checks if local variable is used after(outside of) body | 871 | /// returns usage details if local variable is used after(outside of) body |
859 | fn var_outlives_body(ctx: &AssistContext, body: &FunctionBody, var: &Local) -> bool { | 872 | fn var_outlives_body( |
860 | let usages = LocalUsages::find(ctx, *var); | 873 | ctx: &AssistContext, |
874 | body: &FunctionBody, | ||
875 | var: Local, | ||
876 | parent: &SyntaxNode, | ||
877 | ) -> Option<OutlivedLocal> { | ||
878 | let usages = LocalUsages::find(ctx, var); | ||
861 | let has_usages = usages.iter().any(|reference| body.preceedes_range(reference.range)); | 879 | let has_usages = usages.iter().any(|reference| body.preceedes_range(reference.range)); |
862 | has_usages | 880 | if !has_usages { |
881 | return None; | ||
882 | } | ||
883 | let has_mut_usages = usages | ||
884 | .iter() | ||
885 | .filter(|reference| body.preceedes_range(reference.range)) | ||
886 | .any(|reference| reference_is_exclusive(reference, parent, ctx)); | ||
887 | Some(OutlivedLocal { local: var, mut_usage_outside_body: has_mut_usages }) | ||
863 | } | 888 | } |
864 | 889 | ||
865 | fn body_return_ty(ctx: &AssistContext, body: &FunctionBody) -> Option<RetType> { | 890 | fn body_return_ty(ctx: &AssistContext, body: &FunctionBody) -> Option<RetType> { |
@@ -939,16 +964,25 @@ fn format_replacement(ctx: &AssistContext, fun: &Function, indent: IndentLevel) | |||
939 | let mut buf = String::new(); | 964 | let mut buf = String::new(); |
940 | match fun.vars_defined_in_body_and_outlive.as_slice() { | 965 | match fun.vars_defined_in_body_and_outlive.as_slice() { |
941 | [] => {} | 966 | [] => {} |
942 | [var] => format_to!(buf, "let {} = ", var.name(ctx.db()).unwrap()), | 967 | [var] => { |
968 | format_to!(buf, "let {}{} = ", mut_modifier(var), var.local.name(ctx.db()).unwrap()) | ||
969 | } | ||
943 | [v0, vs @ ..] => { | 970 | [v0, vs @ ..] => { |
944 | buf.push_str("let ("); | 971 | buf.push_str("let ("); |
945 | format_to!(buf, "{}", v0.name(ctx.db()).unwrap()); | 972 | format_to!(buf, "{}{}", mut_modifier(v0), v0.local.name(ctx.db()).unwrap()); |
946 | for var in vs { | 973 | for var in vs { |
947 | format_to!(buf, ", {}", var.name(ctx.db()).unwrap()); | 974 | format_to!(buf, ", {}{}", mut_modifier(var), var.local.name(ctx.db()).unwrap()); |
948 | } | 975 | } |
949 | buf.push_str(") = "); | 976 | buf.push_str(") = "); |
950 | } | 977 | } |
951 | } | 978 | } |
979 | fn mut_modifier(var: &OutlivedLocal) -> &'static str { | ||
980 | if var.mut_usage_outside_body { | ||
981 | "mut " | ||
982 | } else { | ||
983 | "" | ||
984 | } | ||
985 | } | ||
952 | format_to!(buf, "{}", expr); | 986 | format_to!(buf, "{}", expr); |
953 | if fun.ret_ty.is_unit() | 987 | if fun.ret_ty.is_unit() |
954 | && (!fun.vars_defined_in_body_and_outlive.is_empty() || !expr.is_block_like()) | 988 | && (!fun.vars_defined_in_body_and_outlive.is_empty() || !expr.is_block_like()) |
@@ -1211,10 +1245,10 @@ fn make_body( | |||
1211 | match fun.vars_defined_in_body_and_outlive.as_slice() { | 1245 | match fun.vars_defined_in_body_and_outlive.as_slice() { |
1212 | [] => {} | 1246 | [] => {} |
1213 | [var] => { | 1247 | [var] => { |
1214 | tail_expr = Some(path_expr_from_local(ctx, *var)); | 1248 | tail_expr = Some(path_expr_from_local(ctx, var.local)); |
1215 | } | 1249 | } |
1216 | vars => { | 1250 | vars => { |
1217 | let exprs = vars.iter().map(|var| path_expr_from_local(ctx, *var)); | 1251 | let exprs = vars.iter().map(|var| path_expr_from_local(ctx, var.local)); |
1218 | let expr = make::expr_tuple(exprs); | 1252 | let expr = make::expr_tuple(exprs); |
1219 | tail_expr = Some(expr); | 1253 | tail_expr = Some(expr); |
1220 | } | 1254 | } |
@@ -2123,6 +2157,30 @@ fn $0fun_name(n: i32) -> i32 { | |||
2123 | } | 2157 | } |
2124 | 2158 | ||
2125 | #[test] | 2159 | #[test] |
2160 | fn variable_defined_inside_and_used_after_mutably_no_ret() { | ||
2161 | check_assist( | ||
2162 | extract_function, | ||
2163 | r" | ||
2164 | fn foo() { | ||
2165 | let n = 1; | ||
2166 | $0let mut k = n * n;$0 | ||
2167 | k += 1; | ||
2168 | }", | ||
2169 | r" | ||
2170 | fn foo() { | ||
2171 | let n = 1; | ||
2172 | let mut k = fun_name(n); | ||
2173 | k += 1; | ||
2174 | } | ||
2175 | |||
2176 | fn $0fun_name(n: i32) -> i32 { | ||
2177 | let mut k = n * n; | ||
2178 | k | ||
2179 | }", | ||
2180 | ); | ||
2181 | } | ||
2182 | |||
2183 | #[test] | ||
2126 | fn two_variables_defined_inside_and_used_after_no_ret() { | 2184 | fn two_variables_defined_inside_and_used_after_no_ret() { |
2127 | check_assist( | 2185 | check_assist( |
2128 | extract_function, | 2186 | extract_function, |
@@ -2149,6 +2207,38 @@ fn $0fun_name(n: i32) -> (i32, i32) { | |||
2149 | } | 2207 | } |
2150 | 2208 | ||
2151 | #[test] | 2209 | #[test] |
2210 | fn multi_variables_defined_inside_and_used_after_mutably_no_ret() { | ||
2211 | check_assist( | ||
2212 | extract_function, | ||
2213 | r" | ||
2214 | fn foo() { | ||
2215 | let n = 1; | ||
2216 | $0let mut k = n * n; | ||
2217 | let mut m = k + 2; | ||
2218 | let mut o = m + 3; | ||
2219 | o += 1;$0 | ||
2220 | k += o; | ||
2221 | m = 1; | ||
2222 | }", | ||
2223 | r" | ||
2224 | fn foo() { | ||
2225 | let n = 1; | ||
2226 | let (mut k, mut m, o) = fun_name(n); | ||
2227 | k += o; | ||
2228 | m = 1; | ||
2229 | } | ||
2230 | |||
2231 | fn $0fun_name(n: i32) -> (i32, i32, i32) { | ||
2232 | let mut k = n * n; | ||
2233 | let mut m = k + 2; | ||
2234 | let mut o = m + 3; | ||
2235 | o += 1; | ||
2236 | (k, m, o) | ||
2237 | }", | ||
2238 | ); | ||
2239 | } | ||
2240 | |||
2241 | #[test] | ||
2152 | fn nontrivial_patterns_define_variables() { | 2242 | fn nontrivial_patterns_define_variables() { |
2153 | check_assist( | 2243 | check_assist( |
2154 | extract_function, | 2244 | extract_function, |