Repository: rust-analyzer/rust-analyzer
Pull request #11202 (2022-01-05 21:30:33)

fix: Fix `apply_demorgan` assist hanging for certain binary expressions
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10963
bors r+


Patch:
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 9a1a893650..6306ae534d 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -1540,11 +1540,11 @@ impl Const {
         let infer = infer.as_ref();
         let result = eval_const(
             root,
-            &mut ConstEvalCtx {
+            ConstEvalCtx {
                 exprs: &body.exprs,
                 pats: &body.pats,
                 local_data: HashMap::default(),
-                infer: &mut |x| infer[x].clone(),
+                infer,
             },
         );
         result
diff --git a/crates/hir_ty/src/consteval.rs b/crates/hir_ty/src/consteval.rs
index 68e6e0582e..0005a86b7f 100644
--- a/crates/hir_ty/src/consteval.rs
+++ b/crates/hir_ty/src/consteval.rs
@@ -4,13 +4,14 @@ use std::{collections::HashMap, convert::TryInto, fmt::Display};
 
 use chalk_ir::{IntTy, Scalar};
 use hir_def::{
+    builtin_type::BuiltinUint,
     expr::{ArithOp, BinaryOp, Expr, Literal, Pat},
     type_ref::ConstScalar,
 };
 use hir_expand::name::Name;
-use la_arena::{Arena, Idx};
+use la_arena::Arena;
 
-use crate::{Const, ConstData, ConstValue, Interner, Ty, TyKind};
+use crate::{Const, ConstData, ConstValue, InferenceResult, Interner, TyKind};
 
 /// Extension trait for [`Const`]
 pub trait ConstExt {
@@ -40,11 +41,12 @@ impl ConstExt for Const {
     }
 }
 
+#[derive(Clone)]
 pub struct ConstEvalCtx<'a> {
     pub exprs: &'a Arena<Expr>,
     pub pats: &'a Arena<Pat>,
     pub local_data: HashMap<Name, ComputedExpr>,
-    pub infer: &'a mut dyn FnMut(Idx<Expr>) -> Ty,
+    pub infer: &'a InferenceResult,
 }
 
 #[derive(Debug, Clone)]
@@ -55,7 +57,7 @@ pub enum ConstEvalError {
     Panic(String),
 }
 
-#[derive(Debug, Clone)]
+#[derive(Clone)]
 pub enum ComputedExpr {
     Literal(Literal),
     Tuple(Box<[ComputedExpr]>),
@@ -128,11 +130,11 @@ fn is_valid(scalar: &Scalar, value: i128) -> bool {
     }
 }
 
-pub fn eval_const(expr: &Expr, ctx: &mut ConstEvalCtx<'_>) -> Result<ComputedExpr, ConstEvalError> {
+pub fn eval_const(expr: &Expr, mut ctx: ConstEvalCtx<'_>) -> Result<ComputedExpr, ConstEvalError> {
     match expr {
         Expr::Literal(l) => Ok(ComputedExpr::Literal(l.clone())),
         &Expr::UnaryOp { expr, op } => {
-            let ty = &(ctx.infer)(expr);
+            let ty = &ctx.infer[expr];
             let ev = eval_const(&ctx.exprs[expr], ctx)?;
             match op {
                 hir_def::expr::UnaryOp::Deref => Err(ConstEvalError::NotSupported("deref")),
@@ -188,9 +190,9 @@ pub fn eval_const(expr: &Expr, ctx: &mut ConstEvalCtx<'_>) -> Result<ComputedExp
             }
         }
         &Expr::BinaryOp { lhs, rhs, op } => {
-            let ty = &(ctx.infer)(lhs);
-            let lhs = eval_const(&ctx.exprs[lhs], ctx)?;
-            let rhs = eval_const(&ctx.exprs[rhs], ctx)?;
+            let ty = &ctx.infer[lhs];
+            let lhs = eval_const(&ctx.exprs[lhs], ctx.clone())?;
+            let rhs = eval_const(&ctx.exprs[rhs], ctx.clone())?;
             let op = op.ok_or(ConstEvalError::IncompleteExpr)?;
             let v1 = match lhs {
                 ComputedExpr::Literal(Literal::Int(v, _)) => v,
@@ -239,7 +241,6 @@ pub fn eval_const(expr: &Expr, ctx: &mut ConstEvalCtx<'_>) -> Result<ComputedExp
             }
         }
         Expr::Block { statements, tail, .. } => {
-            let mut prev_values = HashMap::<Name, Option<ComputedExpr>>::default();
             for statement in &**statements {
                 match statement {
                     &hir_def::expr::Statement::Let { pat, initializer, .. } => {
@@ -251,33 +252,21 @@ pub fn eval_const(expr: &Expr, ctx: &mut ConstEvalCtx<'_>) -> Result<ComputedExp
                             }
                         };
                         let value = match initializer {
-                            Some(x) => eval_const(&ctx.exprs[x], ctx)?,
+                            Some(x) => eval_const(&ctx.exprs[x], ctx.clone())?,
                             None => continue,
                         };
-                        if !prev_values.contains_key(&name) {
-                            let prev = ctx.local_data.insert(name.clone(), value);
-                            prev_values.insert(name, prev);
-                        } else {
-                            ctx.local_data.insert(name, value);
-                        }
+                        ctx.local_data.insert(name, value);
                     }
                     &hir_def::expr::Statement::Expr { .. } => {
                         return Err(ConstEvalError::NotSupported("this kind of statement"))
                     }
                 }
             }
-            let r = match tail {
-                &Some(x) => eval_const(&ctx.exprs[x], ctx),
-                None => Ok(ComputedExpr::Tuple(Box::new([]))),
+            let tail_expr = match tail {
+                &Some(x) => &ctx.exprs[x],
+                None => return Ok(ComputedExpr::Tuple(Box::new([]))),
             };
-            // clean up local data, so caller will receive the exact map that passed to us
-            for (name, val) in prev_values {
-                match val {
-                    Some(x) => ctx.local_data.insert(name, x),
-                    None => ctx.local_data.remove(&name),
-                };
-            }
-            r
+            eval_const(tail_expr, ctx)
         }
         Expr::Path(p) => {
             let name = p.mod_path().as_ident().ok_or(ConstEvalError::NotSupported("big paths"))?;
@@ -291,16 +280,12 @@ pub fn eval_const(expr: &Expr, ctx: &mut ConstEvalCtx<'_>) -> Result<ComputedExp
     }
 }
 
-pub fn eval_usize(expr: Idx<Expr>, mut ctx: ConstEvalCtx<'_>) -> Option<u64> {
-    let expr = &ctx.exprs[expr];
-    if let Ok(ce) = eval_const(&expr, &mut ctx) {
-        match ce {
-            ComputedExpr::Literal(Literal::Int(x, _)) => return x.try_into().ok(),
-            ComputedExpr::Literal(Literal::Uint(x, _)) => return x.try_into().ok(),
-            _ => {}
-        }
+// FIXME: support more than just evaluating literals
+pub fn eval_usize(expr: &Expr) -> Option<u64> {
+    match expr {
+        Expr::Literal(Literal::Uint(v, None | Some(BuiltinUint::Usize))) => (*v).try_into().ok(),
+        _ => None,
     }
-    None
 }
 
 /// Interns a possibly-unknown target usize
diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs
index 54b1680214..a892a680d7 100644
--- a/crates/hir_ty/src/infer/expr.rs
+++ b/crates/hir_ty/src/infer/expr.rs
@@ -799,15 +799,8 @@ impl<'a> InferenceContext<'a> {
                             ),
                         );
 
-                        consteval::eval_usize(
-                            repeat,
-                            consteval::ConstEvalCtx {
-                                exprs: &body.exprs,
-                                pats: &body.pats,
-                                local_data: Default::default(),
-                                infer: &mut |x| self.infer_expr(x, &expected),
-                            },
-                        )
+                        let repeat_expr = &self.body.exprs[repeat];
+                        consteval::eval_usize(repeat_expr)
                     }
                 };
 
diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs
index f1280fcc10..1950820a37 100644
--- a/crates/hir_ty/src/lib.rs
+++ b/crates/hir_ty/src/lib.rs
@@ -358,7 +358,7 @@ where
     struct ErrorReplacer {
         vars: usize,
     }
-    impl Folder<Interner> for ErrorReplacer {
+    impl<'i> Folder<Interner> for ErrorReplacer {
         type Error = NoSolution;
 
         fn as_dyn(&mut self) -> &mut dyn Folder<Interner, Error = Self::Error> {
diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs
index 29fc1fd2e0..4d638e687a 100644
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@@ -4,7 +4,7 @@ use crate::{doc_links::token_as_doc_comment, FilePosition, NavigationTarget, Ran
 use hir::{AsAssocItem, Semantics};
 use ide_db::{
     base_db::{AnchoredPath, FileId, FileLoader},
-    defs::{Definition, IdentClass},
+    defs::Definition,
     helpers::pick_best_token,
     RootDatabase,
 };
@@ -46,20 +46,20 @@ pub(crate) fn goto_definition(
         .filter_map(|token| {
             let parent = token.parent()?;
             if let Some(tt) = ast::TokenTree::cast(parent) {
-                if let Some(x) = try_lookup_include_path(sema, tt, token.clone(), position.file_id)
+                if let x @ Some(_) =
+                    try_lookup_include_path(sema, tt, token.clone(), position.file_id)
                 {
-                    return Some(vec![x]);
+                    return x;
                 }
             }
             Some(
-                IdentClass::classify_token(sema, &token)?
-                    .definitions()
+                Definition::from_token(sema, &token)
                     .into_iter()
                     .flat_map(|def| {
                         try_find_trait_item_definition(sema.db, &def)
                             .unwrap_or_else(|| def_to_nav(sema.db, def))
                     })
-                    .collect(),
+                    .collect::<Vec<_>>(),
             )
         })
         .flatten()
@@ -74,7 +74,7 @@ fn try_lookup_include_path(
     tt: ast::TokenTree,
     token: SyntaxToken,
     file_id: FileId,
-) -> Option<NavigationTarget> {
+) -> Option<Vec<NavigationTarget>> {
     let token = ast::String::cast(token)?;
     let path = token.value()?.into_owned();
     let macro_call = tt.syntax().parent().and_then(ast::MacroCall::cast)?;
@@ -84,7 +84,7 @@ fn try_lookup_include_path(
     }
     let file_id = sema.db.resolve_path(AnchoredPath { anchor: file_id, path: &path })?;
     let size = sema.db.file_text(file_id).len().try_into().ok()?;
-    Some(NavigationTarget {
+    Some(vec![NavigationTarget {
         file_id,
         full_range: TextRange::new(0.into(), size),
         name: path.into(),
@@ -93,7 +93,7 @@ fn try_lookup_include_path(
         container_name: None,
         description: None,
         docs: None,
-    })
+    }])
 }
 
 /// finds the trait definition of an impl'd item
diff --git a/crates/ide/src/highlight_related.rs b/crates/ide/src/highlight_related.rs
index b6d9e4021d..054b8f297b 100644
--- a/crates/ide/src/highlight_related.rs
+++ b/crates/ide/src/highlight_related.rs
@@ -1,7 +1,7 @@
 use hir::Semantics;
 use ide_db::{
     base_db::{FileId, FilePosition},
-    defs::{Definition, IdentClass},
+    defs::Definition,
     helpers::{for_each_break_expr, for_each_tail_expr, node_ext::walk_expr, pick_best_token},
     search::{FileReference, ReferenceCategory, SearchScope},
     RootDatabase,
@@ -293,8 +293,7 @@ fn cover_range(r0: Option<TextRange>, r1: Option<TextRange>) -> Option<TextRange
 fn find_defs(sema: &Semantics<RootDatabase>, token: SyntaxToken) -> FxHashSet<Definition> {
     sema.descend_into_macros(token)
         .into_iter()
-        .filter_map(|token| IdentClass::classify_token(sema, &token).map(IdentClass::definitions))
-        .flatten()
+        .flat_map(|token| Definition::from_token(sema, &token))
         .collect()
 }
 
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 21bea25a5f..9d19d7c1c0 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -9,7 +9,7 @@ use either::Either;
 use hir::{HasSource, Semantics};
 use ide_db::{
     base_db::FileRange,
-    defs::{Definition, IdentClass},
+    defs::Definition,
     helpers::{pick_best_token, FamousDefs},
     FxIndexSet, RootDatabase,
 };
@@ -129,8 +129,8 @@ pub(crate) fn hover(
         .iter()
         .filter_map(|token| {
             let node = token.parent()?;
-            let class = IdentClass::classify_token(sema, token)?;
-            Some(class.definitions().into_iter().zip(iter::once(node).cycle()))
+            let defs = Definition::from_token(sema, token);
+            Some(defs.into_iter().zip(iter::once(node).cycle()))
         })
         .flatten()
         .unique_by(|&(def, _)| def)
diff --git a/crates/ide/src/moniker.rs b/crates/ide/src/moniker.rs
index 53f6d7ec7a..c93c471f49 100644
--- a/crates/ide/src/moniker.rs
+++ b/crates/ide/src/moniker.rs
@@ -4,7 +4,7 @@
 use hir::{db::DefDatabase, AsAssocItem, AssocItemContainer, Crate, Name, Semantics};
 use ide_db::{
     base_db::{CrateOrigin, FileId, FileLoader, FilePosition},
-    defs::{Definition, IdentClass},
+    defs::Definition,
     helpers::pick_best_token,
     RootDatabase,
 };
@@ -82,10 +82,11 @@ pub(crate) fn moniker(
     let navs = sema
         .descend_into_macros(original_token.clone())
         .into_iter()
-        .filter_map(|token| {
-            IdentClass::classify_token(sema, &token).map(IdentClass::definitions).map(|it| {
-                it.into_iter().flat_map(|def| def_to_moniker(sema.db, def, current_crate))
-            })
+        .map(|token| {
+            Definition::from_token(sema, &token)
+                .into_iter()
+                .flat_map(|def| def_to_moniker(sema.db, def, current_crate))
+                .collect::<Vec<_>>()
         })
         .flatten()
         .unique()
diff --git a/crates/ide/src/static_index.rs b/crates/ide/src/static_index.rs
index d5bfbd1894..77d202cc39 100644
--- a/crates/ide/src/static_index.rs
+++ b/crates/ide/src/static_index.rs
@@ -6,7 +6,7 @@ use std::collections::HashMap;
 use hir::{db::HirDatabase, Crate, Module, Semantics};
 use ide_db::{
     base_db::{FileId, FileRange, SourceDatabaseExt},
-    defs::{Definition, IdentClass},
+    defs::Definition,
     RootDatabase,
 };
 use rustc_hash::FxHashSet;
@@ -195,9 +195,9 @@ impl StaticIndex<'_> {
 
 fn get_definition(sema: &Semantics<RootDatabase>, token: SyntaxToken) -> Option<Definition> {
     for token in sema.descend_into_macros(token) {
-        let def = IdentClass::classify_token(sema, &token).map(IdentClass::definitions);
-        if let Some(&[x]) = def.as_deref() {
-            return Some(x);
+        let def = Definition::from_token(sema, &token);
+        if let [x] = def.as_slice() {
+            return Some(*x);
         } else {
             continue;
         };
diff --git a/crates/ide_assists/src/handlers/add_explicit_type.rs b/crates/ide_assists/src/handlers/add_explicit_type.rs
index 945edfd999..d7e1be900f 100644
--- a/crates/ide_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ide_assists/src/handlers/add_explicit_type.rs
@@ -208,10 +208,8 @@ fn main() {
         check_assist_not_applicable(
             add_explicit_type,
             r#"
-//- minicore: option
-
 fn main() {
-    let $0l = [0.0; Some(2).unwrap()];
+    let $0l = [0.0; 2+2];
 }
 "#,
         );
diff --git a/crates/ide_assists/src/handlers/add_missing_impl_members.rs b/crates/ide_assists/src/handlers/add_missing_impl_members.rs
index b6af72cbac..a10eca10d1 100644
--- a/crates/ide_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ide_assists/src/handlers/add_missing_impl_members.rs
@@ -938,47 +938,6 @@ struct Foo(usize);
 impl FooB for Foo {
     $0fn foo< 'lt>(& 'lt self){}
 
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn macro_trait_dyn_absolute_path() {
-        // https://github.com/rust-analyzer/rust-analyzer/issues/11100
-        check_assist(
-            add_missing_impl_members,
-            r#"
-macro_rules! foo {
-    () => {
-        trait MacroTrait {
-            fn trait_method(_: &dyn ::core::marker::Sized);
-        }
-    }
-}
-foo!();
-struct Foo;
-
-impl MacroTrait for Foo {
-    $0
-}
-"#,
-            r#"
-macro_rules! foo {
-    () => {
-        trait MacroTrait {
-            fn trait_method(_: &dyn ::core::marker::Sized);
-        }
-    }
-}
-foo!();
-struct Foo;
-
-impl MacroTrait for Foo {
-    fn trait_method(_: &dyn ::core::marker::Sized) {
-        ${0:todo!()}
-    }
-
 }
 "#,
         )
diff --git a/crates/ide_assists/src/handlers/apply_demorgan.rs b/crates/ide_assists/src/handlers/apply_demorgan.rs
index b3fcf6578a..21907ab41f 100644
--- a/crates/ide_assists/src/handlers/apply_demorgan.rs
+++ b/crates/ide_assists/src/handlers/apply_demorgan.rs
@@ -42,10 +42,11 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext) -> Option<(
 
     // Walk up the tree while we have the same binary operator
     while let Some(parent_expr) = expr.syntax().parent().and_then(ast::BinExpr::cast) {
-        if let Some(parent_op) = expr.op_kind() {
-            if parent_op == op {
-                expr = parent_expr
+        match expr.op_kind() {
+            Some(parent_op) if parent_op == op => {
+                expr = parent_expr;
             }
+            _ => break,
         }
     }
 
@@ -220,4 +221,14 @@ fn f() { !(S <= S || S < S) }
         cov_mark::check!(demorgan_double_parens);
         check_assist(apply_demorgan, "fn f() { (x ||$0 x) }", "fn f() { !(!x && !x) }")
     }
+
+    // https://github.com/rust-analyzer/rust-analyzer/issues/10963
+    #[test]
+    fn demorgan_doesnt_hang() {
+        check_assist(
+            apply_demorgan,
+            "fn f() { 1 || 3 &&$0 4 || 5 }",
+            "fn f() { !(!1 || !3 || !4) || 5 }",
+        )
+    }
 }
diff --git a/crates/ide_completion/src/completions/fn_param.rs b/crates/ide_completion/src/completions/fn_param.rs
index bb4ce0a24d..5acda46498 100644
--- a/crates/ide_completion/src/completions/fn_param.rs
+++ b/crates/ide_completion/src/completions/fn_param.rs
@@ -3,7 +3,7 @@
 use rustc_hash::FxHashMap;
 use syntax::{
     ast::{self, HasModuleItem},
-    match_ast, AstNode, SyntaxKind,
+    match_ast, AstNode,
 };
 
 use crate::{
@@ -16,22 +16,24 @@ use crate::{
 /// `spam: &mut Spam` insert text/label and `spam` lookup string will be
 /// suggested.
 pub(crate) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> {
-    let param_of_fn =
-        matches!(ctx.pattern_ctx, Some(PatternContext { is_param: Some(ParamKind::Function), .. }));
-
-    if !param_of_fn {
+    if !matches!(ctx.pattern_ctx, Some(PatternContext { is_param: Some(ParamKind::Function), .. }))
+    {
         return None;
     }
 
-    let mut file_params = FxHashMap::default();
+    let mut params = FxHashMap::default();
 
-    let mut extract_params = |f: ast::Fn| {
-        f.param_list().into_iter().flat_map(|it| it.params()).for_each(|param| {
+    let me = ctx.token.ancestors().find_map(ast::Fn::cast);
+    let mut process_fn = |func: ast::Fn| {
+        if Some(&func) == me.as_ref() {
+            return;
+        }
+        func.param_list().into_iter().flat_map(|it| it.params()).for_each(|param| {
             if let Some(pat) = param.pat() {
                 // FIXME: We should be able to turn these into SmolStr without having to allocate a String
-                let whole_param = param.syntax().text().to_string();
-                let binding = pat.syntax().text().to_string();
-                file_params.entry(whole_param).or_insert(binding);
+                let text = param.syntax().text().to_string();
+                let lookup = pat.syntax().text().to_string();
+                params.entry(text).or_insert(lookup);
             }
         });
     };
@@ -42,93 +44,32 @@ pub(crate) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext)
                 ast::SourceFile(it) => it.items().filter_map(|item| match item {
                     ast::Item::Fn(it) => Some(it),
                     _ => None,
-                }).for_each(&mut extract_params),
+                }).for_each(&mut process_fn),
                 ast::ItemList(it) => it.items().filter_map(|item| match item {
                     ast::Item::Fn(it) => Some(it),
                     _ => None,
-                }).for_each(&mut extract_params),
+                }).for_each(&mut process_fn),
                 ast::AssocItemList(it) => it.assoc_items().filter_map(|item| match item {
                     ast::AssocItem::Fn(it) => Some(it),
                     _ => None,
-                }).for_each(&mut extract_params),
+                }).for_each(&mut process_fn),
                 _ => continue,
             }
         };
     }
 
-    let function = ctx.token.ancestors().find_map(ast::Fn::cast)?;
-    let param_list = function.param_list()?;
-
-    remove_duplicated(&mut file_params, param_list.params());
-
     let self_completion_items = ["self", "&self", "mut self", "&mut self"];
-    if should_add_self_completions(ctx, param_list) {
+    if ctx.impl_def.is_some() && me?.param_list()?.params().next().is_none() {
         self_completion_items.into_iter().for_each(|self_item| {
             add_new_item_to_acc(ctx, acc, self_item.to_string(), self_item.to_string())
         });
     }
 
-    file_params.into_iter().try_for_each(|(whole_param, binding)| {
-        Some(add_new_item_to_acc(ctx, acc, surround_with_commas(ctx, whole_param), binding))
-    })?;
+    params.into_iter().for_each(|(label, lookup)| add_new_item_to_acc(ctx, acc, label, lookup));
 
     Some(())
 }
 
-fn remove_duplicated(
-    file_params: &mut FxHashMap<String, String>,
-    fn_params: ast::AstChildren<ast::Param>,
-) {
-    fn_params.for_each(|param| {
-        let whole_param = param.syntax().text().to_string();
-        file_params.remove(&whole_param);
-
-        if let Some(pattern) = param.pat() {
-            let binding = pattern.syntax().text().to_string();
-            file_params.retain(|_, v| v != &binding);
-        }
-    })
-}
-
-fn should_add_self_completions(ctx: &CompletionContext, param_list: ast::ParamList) -> bool {
-    let inside_impl = ctx.impl_def.is_some();
-    let no_params = param_list.params().next().is_none() && param_list.self_param().is_none();
-
-    inside_impl && no_params
-}
-
-fn surround_with_commas(ctx: &CompletionContext, param: String) -> String {
-    match fallible_surround_with_commas(ctx, &param) {
-        Some(surrounded) => surrounded,
-        // fallback to the original parameter
-        None => param,
-    }
-}
-
-fn fallible_surround_with_commas(ctx: &CompletionContext, param: &str) -> Option<String> {
-    let next_token = {
-        let t = ctx.token.next_token()?;
-        match t.kind() {
-            SyntaxKind::WHITESPACE => t.next_token()?,
-            _ => t,
-        }
-    };
-
-    let trailing_comma_missing = matches!(next_token.kind(), SyntaxKind::IDENT);
-    let trailing = if trailing_comma_missing { "," } else { "" };
-
-    let previous_token = if matches!(ctx.token.kind(), SyntaxKind::IDENT | SyntaxKind::WHITESPACE) {
-        ctx.previous_token.as_ref()?
-    } else {
-        &ctx.token
-    };
-
-    let needs_leading = !matches!(previous_token.kind(), SyntaxKind::L_PAREN | SyntaxKind::COMMA);
-    let leading = if needs_leading { ", " } else { "" };
-
-    Some(format!("{}{}{}", leading, param, trailing))
-}
-
 fn add_new_item_to_acc(
     ctx: &CompletionContext,
     acc: &mut Completions,
diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs
index 172acdbc3c..1501c4eda5 100644
--- a/crates/ide_db/src/defs.rs
+++ b/crates/ide_db/src/defs.rs
@@ -42,6 +42,74 @@ pub enum Definition {
 }
 
 impl Definition {
+    pub fn from_token(
+        sema: &Semantics<RootDatabase>,
+        token: &SyntaxToken,
+    ) -> ArrayVec<Definition, 2> {
+        let parent = match token.parent() {
+            Some(parent) => parent,
+            None => return Default::default(),
+        };
+        // resolve derives if possible
+        if let Some(ident) = ast::Ident::cast(token.clone()) {
+            let attr = ast::TokenTree::cast(parent.clone())
+                .and_then(|tt| tt.parent_meta())
+                .and_then(|meta| meta.parent_attr());
+            if let Some(attr) = attr {
+                return sema
+                    .resolve_derive_ident(&attr, &ident)
+                    .map(Into::into)
+                    .into_iter()
+                    .collect();
+            }
+        }
+        Self::from_node(sema, &parent)
+    }
+
+    pub fn from_node(sema: &Semantics<RootDatabase>, node: &SyntaxNode) -> ArrayVec<Definition, 2> {
+        let mut res = ArrayVec::new();
+        (|| {
+            match_ast! {
+                match node {
+                    ast::Name(name) => {
+                        match NameClass::classify(&sema, &name)? {
+                            NameClass::Definition(it) | NameClass::ConstReference(it) => res.push(it),
+                            NameClass::PatFieldShorthand { local_def, field_ref } => {
+                                res.push(Definition::Local(local_def));
+                                res.push(Definition::Field(field_ref));
+                            }
+                        }
+                    },
+                    ast::NameRef(name_ref) => {
+                        match NameRefClass::classify(sema, &name_ref)? {
+                            NameRefClass::Definition(it) => res.push(it),
+                            NameRefClass::FieldShorthand { local_ref, field_ref } => {
+                                res.push(Definition::Local(local_ref));
+                                res.push(Definition::Field(field_ref));
+                            }
+                        }
+                    },
+                    ast::Lifetime(lifetime) => {
+                        let def = if let Some(x) = NameClass::classify_lifetime(&sema, &lifetime) {
+                            NameClass::defined(x)
+                        } else {
+                            NameRefClass::classify_lifetime(&sema, &lifetime).and_then(|class| match class {
+                                NameRefClass::Definition(it) => Some(it),
+                                _ => None,
+                            })
+                        };
+                        if let Some(def) = def {
+                            res.push(def);
+                        }
+                    },
+                    _ => (),
+                }
+            }
+            Some(())
+        })();
+        res
+    }
+
     pub fn canonical_module_path(&self, db: &RootDatabase) -> Option<impl Iterator<Item = Module>> {
         self.module(db).map(|it| it.path_to_root(db).into_iter().rev())
     }
@@ -116,65 +184,6 @@ impl Definition {
     }
 }
 
-pub enum IdentClass {
-    NameClass(NameClass),
-    NameRefClass(NameRefClass),
-}
-
-impl IdentClass {
-    pub fn classify_node(sema: &Semantics<RootDatabase>, node: &SyntaxNode) -> Option<IdentClass> {
-        match_ast! {
-            match node {
-                ast::Name(name) => NameClass::classify(sema, &name).map(IdentClass::NameClass),
-                ast::NameRef(name_ref) => NameRefClass::classify(sema, &name_ref).map(IdentClass::NameRefClass),
-                ast::Lifetime(lifetime) => {
-                    NameClass::classify_lifetime(sema, &lifetime)
-                        .map(IdentClass::NameClass)
-                        .or_else(|| NameRefClass::classify_lifetime(sema, &lifetime).map(IdentClass::NameRefClass))
-                },
-                _ => None,
-            }
-        }
-    }
-
-    pub fn classify_token(
-        sema: &Semantics<RootDatabase>,
-        token: &SyntaxToken,
-    ) -> Option<IdentClass> {
-        let parent = token.parent()?;
-        // resolve derives if possible
-        if let Some(ident) = ast::Ident::cast(token.clone()) {
-            let attr = ast::TokenTree::cast(parent.clone())
-                .and_then(|tt| tt.parent_meta())
-                .and_then(|meta| meta.parent_attr());
-            if let Some(attr) = attr {
-                return NameRefClass::classify_derive(sema, &attr, &ident)
-                    .map(IdentClass::NameRefClass);
-            }
-        }
-        Self::classify_node(sema, &parent)
-    }
-
-    pub fn definitions(self) -> ArrayVec<Definition, 2> {
-        let mut res = ArrayVec::new();
-        match self {
-            IdentClass::NameClass(NameClass::Definition(it) | NameClass::ConstReference(it)) => {
-                res.push(it)
-            }
-            IdentClass::NameClass(NameClass::PatFieldShorthand { local_def, field_ref }) => {
-                res.push(Definition::Local(local_def));
-                res.push(Definition::Field(field_ref));
-            }
-            IdentClass::NameRefClass(NameRefClass::Definition(it)) => res.push(it),
-            IdentClass::NameRefClass(NameRefClass::FieldShorthand { local_ref, field_ref }) => {
-                res.push(Definition::Local(local_ref));
-                res.push(Definition::Field(field_ref));
-            }
-        }
-        res
-    }
-}
-
 /// On a first blush, a single `ast::Name` defines a single definition at some
 /// scope. That is, that, by just looking at the syntactical category, we can
 /// unambiguously define the semantic category.
@@ -456,14 +465,6 @@ impl NameRefClass {
             _ => None,
         }
     }
-
-    pub fn classify_derive(
-        sema: &Semantics<RootDatabase>,
-        attr: &ast::Attr,
-        ident: &ast::Ident,
-    ) -> Option<NameRefClass> {
-        sema.resolve_derive_ident(&attr, &ident).map(Definition::from).map(NameRefClass::Definition)
-    }
 }
 
 impl_from!(
diff --git a/crates/ide_db/src/helpers/insert_whitespace_into_node.rs b/crates/ide_db/src/helpers/insert_whitespace_into_node.rs
index 9d31966cea..251a4caa13 100644
--- a/crates/ide_db/src/helpers/insert_whitespace_into_node.rs
+++ b/crates/ide_db/src/helpers/insert_whitespace_into_node.rs
@@ -88,7 +88,7 @@ pub fn insert_ws_into(syn: SyntaxNode) -> SyntaxNode {
             LIFETIME_IDENT if is_next(|it| is_text(it), true) => {
                 mods.push(do_ws(after, tok));
             }
-            AS_KW | DYN_KW => {
+            AS_KW => {
                 mods.push(do_ws(after, tok));
             }
             T![;] => {


Test patch:
diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs
index ed76c84ab4..2bfda1aff2 100644
--- a/crates/ide/src/hover/tests.rs
+++ b/crates/ide/src/hover/tests.rs
@@ -3350,31 +3350,6 @@ const FOO$0: usize = 1 << 10;
     check(
         r#"
 /// This is a doc
-const FOO$0: usize = {
-    let b = 4;
-    let a = { let b = 2; let a = b; a } + { let a = 1; a + b };
-    a
-};
-"#,
-        expect![[r#"
-            *FOO*
-
-            ```rust
-            test
-            ```
-
-            ```rust
-            const FOO: usize = 7
-            ```
-
-            ---
-
-            This is a doc
-        "#]],
-    );
-    check(
-        r#"
-/// This is a doc
 const FOO$0: usize = 2 - 3;
 "#,
         expect![[r#"
@@ -3468,24 +3443,6 @@ fn foo() {
     );
 }
 
-#[test]
-fn array_repeat_exp() {
-    check(
-        r#"
-fn main() {
-    let til$0e4 = [0_u32; (4 * 8 * 8) / 32];
-}
-        "#,
-        expect![[r#"
-            *tile4*
-
-            ```rust
-            let tile4: [u32; 8]
-            ```
-            "#]],
-    );
-}
-
 #[test]
 fn hover_mod_def() {
     check(
diff --git a/crates/ide_completion/src/tests/fn_param.rs b/crates/ide_completion/src/tests/fn_param.rs
index 940cecf395..8a07aefafe 100644
--- a/crates/ide_completion/src/tests/fn_param.rs
+++ b/crates/ide_completion/src/tests/fn_param.rs
@@ -46,20 +46,7 @@ fn bar(file_id: usize) {}
 fn baz(file$0 id: u32) {}
 "#,
         expect![[r#"
-            bn file_id: usize,
-            kw mut
-        "#]],
-    );
-}
-
-#[test]
-fn repeated_param_name() {
-    check(
-        r#"
-fn foo(file_id: usize) {}
-fn bar(file_id: u32, $0) {}
-"#,
-        expect![[r#"
+            bn file_id: usize
             kw mut
         "#]],
     );
@@ -139,6 +126,7 @@ impl A {
 
 #[test]
 fn in_impl_after_self() {
+    // FIXME: self completions should not be here
     check(
         r#"
 struct A {}
@@ -149,6 +137,10 @@ impl A {
 }
 "#,
         expect![[r#"
+            bn self
+            bn &self
+            bn mut self
+            bn &mut self
             bn file_id: usize
             kw mut
             sp Self