7620: Support control flow in `extract_function` assist r=matklad a=cpud36

Support `return`ing from outer function, `break`ing and `continue`ing outer loops when extracting function.

# Example
Transforms
```rust
fn foo() -> i32 {
  let items = [1,2,3];
  let mut sum = 0;
  for &item in items {
    <|>if item == 42 {
      break;
    }<|>
    sum += item;
  }
  sum
}
```
Into 
```rust
fn foo() -> i32 {
  let items = [1,2,3];
  let mut sum = 0;
  for &item in items {
    if fun_name(item) {
      break;
    }
    sum += item;
  }
  sum
}

fn fun_name(item: i32) -> bool {
  if item == 42 {
    return true;
  }
  false
}
```

![add_explicit_type_infer_type](https://user-images.githubusercontent.com/4218373/107544222-0fadf280-6bdb-11eb-9625-ed6194ba92c0.gif)

# Features

Supported variants
- break and function does not return => uses `bool` and plain if
- break and function does return => uses `Option<T>` and matches on it
- break with value and function does not return => uses `Option<T>` and if let
- break with value and function does return => uses `Result<T, U>` and matches on t
- same for `return` and `continue`(but we can't continue with value)

Assist does handle nested loops and nested items(like functions, modules, impls)

Try `expr?` operator is allowed together with `return Err(_)` and `return None`.
`return expr` is not allowed.

# Not supported
## Mixing `return` with `break` or `continue`
If we have e.g. a `return` and a `break` in the selected code, it is unclear what the produced code should look like.
We can try `Result<T, Option<U>>` or something like that, but it isn't idiomatic, nor it is established. Otherwise, implementation
is relatively simple.

## `break` with label
Not sure how to handle different labels for multiple `break`s.

[edit] implemented try `expr?`

Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
This commit is contained in:
bors[bot] 2021-02-16 14:01:09 +00:00 committed by GitHub
commit 88e8b0a5fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 1527 additions and 249 deletions

View File

@ -88,7 +88,7 @@ pub(crate) fn convert_to_guarded_return(acc: &mut Assists, ctx: &AssistContext)
let early_expression: ast::Expr = match parent_container.kind() { let early_expression: ast::Expr = match parent_container.kind() {
WHILE_EXPR | LOOP_EXPR => make::expr_continue(), WHILE_EXPR | LOOP_EXPR => make::expr_continue(),
FN => make::expr_return(), FN => make::expr_return(None),
_ => return None, _ => return None,
}; };

File diff suppressed because it is too large Load Diff

View File

@ -215,8 +215,11 @@ fn fn_args(
}); });
} }
deduplicate_arg_names(&mut arg_names); deduplicate_arg_names(&mut arg_names);
let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| make::param(name, ty)); let params = arg_names
Some((None, make::param_list(params))) .into_iter()
.zip(arg_types)
.map(|(name, ty)| make::param(make::ident_pat(make::name(&name)).into(), make::ty(&ty)));
Some((None, make::param_list(None, params)))
} }
/// Makes duplicate argument names unique by appending incrementing numbers. /// Makes duplicate argument names unique by appending incrementing numbers.

View File

@ -195,6 +195,7 @@ pub fn test_some_range(a: int) -> bool {
let assists = Assist::get(&db, &TEST_CONFIG, false, frange); let assists = Assist::get(&db, &TEST_CONFIG, false, frange);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").label, "Extract into function");
assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
assert_eq!(assists.next().expect("expected assist").label, "Replace with match"); assert_eq!(assists.next().expect("expected assist").label, "Replace with match");
} }
@ -220,6 +221,7 @@ pub fn test_some_range(a: int) -> bool {
let assists = Assist::get(&db, &cfg, false, frange); let assists = Assist::get(&db, &cfg, false, frange);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").label, "Extract into function");
assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
assert_eq!(assists.next().expect("expected assist").label, "Replace with match"); assert_eq!(assists.next().expect("expected assist").label, "Replace with match");
} }
@ -228,9 +230,10 @@ pub fn test_some_range(a: int) -> bool {
let mut cfg = TEST_CONFIG; let mut cfg = TEST_CONFIG;
cfg.allowed = Some(vec![AssistKind::RefactorExtract]); cfg.allowed = Some(vec![AssistKind::RefactorExtract]);
let assists = Assist::get(&db, &cfg, false, frange); let assists = Assist::get(&db, &cfg, false, frange);
assert_eq!(assists.len(), 1); assert_eq!(assists.len(), 2);
let mut assists = assists.iter(); let mut assists = assists.iter();
assert_eq!(assists.next().expect("expected assist").label, "Extract into function");
assert_eq!(assists.next().expect("expected assist").label, "Extract into variable"); assert_eq!(assists.next().expect("expected assist").label, "Extract into variable");
} }

View File

@ -1802,6 +1802,18 @@ pub fn iterate_assoc_items<T>(
None None
} }
pub fn type_parameters(&self) -> impl Iterator<Item = Type> + '_ {
let ty = self.ty.value.strip_references();
let substs = match ty {
Ty::Apply(apply_ty) => &apply_ty.parameters,
Ty::Opaque(opaque_ty) => &opaque_ty.parameters,
_ => return Either::Left(iter::empty()),
};
let iter = substs.iter().map(move |ty| self.derived(ty.clone()));
Either::Right(iter)
}
pub fn iterate_method_candidates<T>( pub fn iterate_method_candidates<T>(
&self, &self,
db: &dyn HirDatabase, db: &dyn HirDatabase,

View File

@ -24,11 +24,24 @@ pub fn name_ref(text: &str) -> ast::NameRef {
// FIXME: replace stringly-typed constructor with a family of typed ctors, a-la // FIXME: replace stringly-typed constructor with a family of typed ctors, a-la
// `expr_xxx`. // `expr_xxx`.
pub fn ty(text: &str) -> ast::Type { pub fn ty(text: &str) -> ast::Type {
ast_from_text(&format!("impl {} for D {{}};", text)) ast_from_text(&format!("fn f() -> {} {{}}", text))
} }
pub fn ty_unit() -> ast::Type { pub fn ty_unit() -> ast::Type {
ty("()") ty("()")
} }
// FIXME: handle types of length == 1
pub fn ty_tuple(types: impl IntoIterator<Item = ast::Type>) -> ast::Type {
let contents = types.into_iter().join(", ");
ty(&format!("({})", contents))
}
// FIXME: handle path to type
pub fn ty_generic(name: ast::NameRef, types: impl IntoIterator<Item = ast::Type>) -> ast::Type {
let contents = types.into_iter().join(", ");
ty(&format!("{}<{}>", name, contents))
}
pub fn ty_ref(target: ast::Type, exclusive: bool) -> ast::Type {
ty(&if exclusive { format!("&mut {}", target) } else { format!("&{}", target) })
}
pub fn assoc_item_list() -> ast::AssocItemList { pub fn assoc_item_list() -> ast::AssocItemList {
ast_from_text("impl C for D {};") ast_from_text("impl C for D {};")
@ -175,11 +188,20 @@ pub fn expr_path(path: ast::Path) -> ast::Expr {
pub fn expr_continue() -> ast::Expr { pub fn expr_continue() -> ast::Expr {
expr_from_text("continue") expr_from_text("continue")
} }
pub fn expr_break() -> ast::Expr { pub fn expr_break(expr: Option<ast::Expr>) -> ast::Expr {
expr_from_text("break") match expr {
Some(expr) => expr_from_text(&format!("break {}", expr)),
None => expr_from_text("break"),
}
} }
pub fn expr_return() -> ast::Expr { pub fn expr_return(expr: Option<ast::Expr>) -> ast::Expr {
expr_from_text("return") match expr {
Some(expr) => expr_from_text(&format!("return {}", expr)),
None => expr_from_text("return"),
}
}
pub fn expr_try(expr: ast::Expr) -> ast::Expr {
expr_from_text(&format!("{}?", expr))
} }
pub fn expr_match(expr: ast::Expr, match_arm_list: ast::MatchArmList) -> ast::Expr { pub fn expr_match(expr: ast::Expr, match_arm_list: ast::MatchArmList) -> ast::Expr {
expr_from_text(&format!("match {} {}", expr, match_arm_list)) expr_from_text(&format!("match {} {}", expr, match_arm_list))
@ -212,6 +234,10 @@ pub fn expr_ref(expr: ast::Expr, exclusive: bool) -> ast::Expr {
pub fn expr_paren(expr: ast::Expr) -> ast::Expr { pub fn expr_paren(expr: ast::Expr) -> ast::Expr {
expr_from_text(&format!("({})", expr)) expr_from_text(&format!("({})", expr))
} }
pub fn expr_tuple(elements: impl IntoIterator<Item = ast::Expr>) -> ast::Expr {
let expr = elements.into_iter().format(", ");
expr_from_text(&format!("({})", expr))
}
fn expr_from_text(text: &str) -> ast::Expr { fn expr_from_text(text: &str) -> ast::Expr {
ast_from_text(&format!("const C: () = {};", text)) ast_from_text(&format!("const C: () = {};", text))
} }
@ -236,6 +262,13 @@ fn from_text(text: &str) -> ast::IdentPat {
ast_from_text(&format!("fn f({}: ())", text)) ast_from_text(&format!("fn f({}: ())", text))
} }
} }
pub fn ident_mut_pat(name: ast::Name) -> ast::IdentPat {
return from_text(name.text());
fn from_text(text: &str) -> ast::IdentPat {
ast_from_text(&format!("fn f(mut {}: ())", text))
}
}
pub fn wildcard_pat() -> ast::WildcardPat { pub fn wildcard_pat() -> ast::WildcardPat {
return from_text("_"); return from_text("_");
@ -356,17 +389,25 @@ pub fn token(kind: SyntaxKind) -> SyntaxToken {
.unwrap_or_else(|| panic!("unhandled token: {:?}", kind)) .unwrap_or_else(|| panic!("unhandled token: {:?}", kind))
} }
pub fn param(name: String, ty: String) -> ast::Param { pub fn param(pat: ast::Pat, ty: ast::Type) -> ast::Param {
ast_from_text(&format!("fn f({}: {}) {{ }}", name, ty)) ast_from_text(&format!("fn f({}: {}) {{ }}", pat, ty))
} }
pub fn ret_type(ty: ast::Type) -> ast::RetType { pub fn ret_type(ty: ast::Type) -> ast::RetType {
ast_from_text(&format!("fn f() -> {} {{ }}", ty)) ast_from_text(&format!("fn f() -> {} {{ }}", ty))
} }
pub fn param_list(pats: impl IntoIterator<Item = ast::Param>) -> ast::ParamList { pub fn param_list(
self_param: Option<ast::SelfParam>,
pats: impl IntoIterator<Item = ast::Param>,
) -> ast::ParamList {
let args = pats.into_iter().join(", "); let args = pats.into_iter().join(", ");
ast_from_text(&format!("fn f({}) {{ }}", args)) let list = match self_param {
Some(self_param) if args.is_empty() => format!("fn f({}) {{ }}", self_param),
Some(self_param) => format!("fn f({}, {}) {{ }}", self_param, args),
None => format!("fn f({}) {{ }}", args),
};
ast_from_text(&list)
} }
pub fn generic_param(name: String, ty: Option<ast::TypeBoundList>) -> ast::GenericParam { pub fn generic_param(name: String, ty: Option<ast::TypeBoundList>) -> ast::GenericParam {