Filter generic param list to only params used in function

Author: DorianListens

crates/hir/src/lib.rs

@@ -3280,6 +3280,15 @@ impl Type {
         let tys = hir_ty::replace_errors_with_variables(&(self.ty.clone(), to.ty.clone()));
         hir_ty::could_coerce(db, self.env.clone(), &tys)
     }
+
+    pub fn as_type_param(&self, db: &dyn HirDatabase) -> Option<TypeParam> {
+        match self.ty.kind(Interner) {
+            TyKind::Placeholder(p) => Some(TypeParam {
+                id: TypeParamId::from_unchecked(hir_ty::from_placeholder_idx(db, *p)),
+            }),
+            _ => None,
+        }
+    }
 }
 
 #[derive(Debug)]

crates/ide-assists/src/handlers/extract_function.rs

@@ -2,7 +2,7 @@ use std::iter;
 
 use ast::make;
 use either::Either;
-use hir::{HirDisplay, InFile, Local, ModuleDef, Semantics, TypeInfo};
+use hir::{HirDisplay, InFile, Local, ModuleDef, PathResolution, Semantics, TypeInfo, TypeParam};
 use ide_db::{
     defs::{Definition, NameRefClass},
     famous_defs::FamousDefs,
@@ -469,6 +469,24 @@ impl FunctionBody {
         }
     }
 
+    fn descendants(&self) -> impl Iterator<Item = SyntaxNode> {
+        match self {
+            FunctionBody::Expr(expr) => expr.syntax().descendants(),
+            FunctionBody::Span { parent, .. } => parent.syntax().descendants(),
+        }
+    }
+
+    fn descendant_paths(&self) -> impl Iterator<Item = ast::Path> {
+        self.descendants().filter_map(|node| {
+            match_ast! {
+                match node {
+                    ast::Path(it) => Some(it),
+                    _ => None
+                }
+            }
+        })
+    }
+
     fn from_expr(expr: ast::Expr) -> Option<Self> {
         match expr {
             ast::Expr::BreakExpr(it) => it.expr().map(Self::Expr),
@@ -678,11 +696,16 @@ impl FunctionBody {
                 parent_loop.get_or_insert(loop_.syntax().clone());
             }
         };
-        let (is_const, expr, ty, generic_param_list, where_clause) = loop {
+
+        let (mut generic_param_list, mut where_clause) = (None, None);
+        let (is_const, expr, ty) = loop {
             let anc = ancestors.next()?;
             break match_ast! {
                 match anc {
-                    ast::ClosureExpr(closure) => (false, closure.body(), infer_expr_opt(closure.body()), closure.generic_param_list(), None),
+                    ast::ClosureExpr(closure) => {
+                        generic_param_list = closure.generic_param_list();
+                        (false, closure.body(), infer_expr_opt(closure.body()))
+                    },
                     ast::BlockExpr(block_expr) => {
                         let (constness, block) = match block_expr.modifier() {
                             Some(ast::BlockModifier::Const(_)) => (true, block_expr),
@@ -691,7 +714,7 @@ impl FunctionBody {
                             _ => continue,
                         };
                         let expr = Some(ast::Expr::BlockExpr(block));
-                        (constness, expr.clone(), infer_expr_opt(expr), None, None)
+                        (constness, expr.clone(), infer_expr_opt(expr))
                     },
                     ast::Fn(fn_) => {
                         let func = sema.to_def(&fn_)?;
@@ -701,23 +724,25 @@ impl FunctionBody {
                                 ret_ty = async_ret;
                             }
                         }
-                        (fn_.const_token().is_some(), fn_.body().map(ast::Expr::BlockExpr), Some(ret_ty), fn_.generic_param_list(), fn_.where_clause())
+                        generic_param_list = fn_.generic_param_list();
+                        where_clause = fn_.where_clause();
+                        (fn_.const_token().is_some(), fn_.body().map(ast::Expr::BlockExpr), Some(ret_ty))
                     },
                     ast::Static(statik) => {
-                        (true, statik.body(), Some(sema.to_def(&statik)?.ty(sema.db)), None, None)
+                        (true, statik.body(), Some(sema.to_def(&statik)?.ty(sema.db)))
                     },
                     ast::ConstArg(ca) => {
-                        (true, ca.expr(), infer_expr_opt(ca.expr()), None, None)
+                        (true, ca.expr(), infer_expr_opt(ca.expr()))
                     },
                     ast::Const(konst) => {
-                        (true, konst.body(), Some(sema.to_def(&konst)?.ty(sema.db)), None, None)
+                        (true, konst.body(), Some(sema.to_def(&konst)?.ty(sema.db)))
                     },
                     ast::ConstParam(cp) => {
-                        (true, cp.default_val(), Some(sema.to_def(&cp)?.ty(sema.db)), None, None)
+                        (true, cp.default_val(), Some(sema.to_def(&cp)?.ty(sema.db)))
                     },
                     ast::ConstBlockPat(cbp) => {
                         let expr = cbp.block_expr().map(ast::Expr::BlockExpr);
-                        (true, expr.clone(), infer_expr_opt(expr), None, None)
+                        (true, expr.clone(), infer_expr_opt(expr))
                     },
                     ast::Variant(__) => return None,
                     ast::Meta(__) => return None,
@@ -1320,8 +1345,7 @@ fn format_function(
     let const_kw = if fun.mods.is_const { "const " } else { "" };
     let async_kw = if fun.control_flow.is_async { "async " } else { "" };
     let unsafe_kw = if fun.control_flow.is_unsafe { "unsafe " } else { "" };
-    let generic_params = format_generic_param_list(fun);
-    let where_clause = format_where_clause(fun);
+    let (generic_params, where_clause) = format_generic_params_and_where_clause(ctx, fun);
     match ctx.config.snippet_cap {
         Some(_) => format_to!(
             fn_def,
@@ -1356,13 +1380,50 @@ fn format_function(
     fn_def
 }
 
-fn format_generic_param_list(fun: &Function) -> String {
+fn format_generic_params_and_where_clause(ctx: &AssistContext, fun: &Function) -> (String, String) {
+    (format_generic_param_list(fun, ctx), format_where_clause(fun))
+}
+
+fn format_generic_param_list(fun: &Function, ctx: &AssistContext) -> String {
+    let type_params_in_descendant_paths =
+        fun.body.descendant_paths().filter_map(|it| match ctx.sema.resolve_path(&it) {
+            Some(PathResolution::TypeParam(type_param)) => Some(type_param),
+            _ => None,
+        });
+
+    let type_params_in_params = fun.params.iter().filter_map(|p| p.ty.as_type_param(ctx.db()));
+
+    let used_type_params: Vec<TypeParam> =
+        type_params_in_descendant_paths.chain(type_params_in_params).collect();
+
     match &fun.mods.generic_param_list {
-        Some(it) => format!("{}", it),
+        Some(list) => {
+            let filtered_generic_params = filter_generic_param_list(ctx, list, used_type_params);
+            if filtered_generic_params.is_empty() {
+                return "".to_string();
+            }
+            format!("{}", make::generic_param_list(filtered_generic_params))
+        }
         None => "".to_string(),
     }
 }
 
+fn filter_generic_param_list(
+    ctx: &AssistContext,
+    list: &ast::GenericParamList,
+    used_type_params: Vec<TypeParam>,
+) -> Vec<ast::GenericParam> {
+    list.generic_params()
+        .filter(|p| match p {
+            ast::GenericParam::ConstParam(_) | ast::GenericParam::LifetimeParam(_) => true,
+            ast::GenericParam::TypeParam(type_param) => match &ctx.sema.to_def(type_param) {
+                Some(def) => used_type_params.iter().contains(def),
+                _ => false,
+            },
+        })
+        .collect()
+}
+
 fn format_where_clause(fun: &Function) -> String {
     match &fun.mods.where_clause {
         Some(it) => format!(" {}", it),
@@ -4763,6 +4824,73 @@ fn $0fun_name<T: Debug>(i: T) {
         );
     }
 
+    #[test]
+    fn preserve_generics_from_body() {
+        check_assist(
+            extract_function,
+            r#"
+fn func<T: Default>() -> T {
+    $0T::default()$0
+}
+"#,
+            r#"
+fn func<T: Default>() -> T {
+    fun_name()
+}
+
+fn $0fun_name<T: Default>() -> T {
+    T::default()
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn filter_unused_generics() {
+        check_assist(
+            extract_function,
+            r#"
+fn func<T: Debug, U: Copy>(i: T, u: U) {
+    bar(u);
+    $0foo(i);$0
+}
+"#,
+            r#"
+fn func<T: Debug, U: Copy>(i: T, u: U) {
+    bar(u);
+    fun_name(i);
+}
+
+fn $0fun_name<T: Debug>(i: T) {
+    foo(i);
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn empty_generic_param_list() {
+        check_assist(
+            extract_function,
+            r#"
+fn func<T: Debug>(t: T, i: u32) {
+    bar(t);
+    $0foo(i);$0
+}
+"#,
+            r#"
+fn func<T: Debug>(t: T, i: u32) {
+    bar(t);
+    fun_name(i);
+}
+
+fn $0fun_name(i: u32) {
+    foo(i);
+}
+"#,
+        );
+    }
+
     #[test]
     fn preserve_where_clause() {
         check_assist(