Repository: rust-analyzer/rust-analyzer
Pull request #11186 (2022-01-04 09:57:07)

minor: Simplify
bors r+


Patch:
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index b7d765c59b..a8c4026e31 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -81,8 +81,9 @@ impl ExprValidator {
     }
 
     fn validate_body(&mut self, db: &dyn HirDatabase) {
+        self.check_for_filter_map_next(db);
+
         let body = db.body(self.owner);
-        let mut filter_map_next_checker = None;
 
         for (id, expr) in body.exprs.iter() {
             if let Some((variant, missed_fields, true)) =
@@ -100,7 +101,7 @@ impl ExprValidator {
                     self.validate_match(id, *expr, arms, db, self.infer.clone());
                 }
                 Expr::Call { .. } | Expr::MethodCall { .. } => {
-                    self.validate_call(db, id, expr, &mut filter_map_next_checker);
+                    self.validate_call(db, id, expr);
                 }
                 _ => {}
             }
@@ -142,13 +143,58 @@ impl ExprValidator {
             });
     }
 
-    fn validate_call(
-        &mut self,
-        db: &dyn HirDatabase,
-        call_id: ExprId,
-        expr: &Expr,
-        filter_map_next_checker: &mut Option<FilterMapNextChecker>,
-    ) {
+    fn check_for_filter_map_next(&mut self, db: &dyn HirDatabase) {
+        // Find the FunctionIds for Iterator::filter_map and Iterator::next
+        let iterator_path = path![core::iter::Iterator];
+        let resolver = self.owner.resolver(db.upcast());
+        let iterator_trait_id = match resolver.resolve_known_trait(db.upcast(), &iterator_path) {
+            Some(id) => id,
+            None => return,
+        };
+        let iterator_trait_items = &db.trait_data(iterator_trait_id).items;
+        let filter_map_function_id =
+            match iterator_trait_items.iter().find(|item| item.0 == name![filter_map]) {
+                Some((_, AssocItemId::FunctionId(id))) => id,
+                _ => return,
+            };
+        let next_function_id = match iterator_trait_items.iter().find(|item| item.0 == name![next])
+        {
+            Some((_, AssocItemId::FunctionId(id))) => id,
+            _ => return,
+        };
+
+        // Search function body for instances of .filter_map(..).next()
+        let body = db.body(self.owner);
+        let mut prev = None;
+        for (id, expr) in body.exprs.iter() {
+            if let Expr::MethodCall { receiver, .. } = expr {
+                let function_id = match self.infer.method_resolution(id) {
+                    Some((id, _)) => id,
+                    None => continue,
+                };
+
+                if function_id == *filter_map_function_id {
+                    prev = Some(id);
+                    continue;
+                }
+
+                if function_id == *next_function_id {
+                    if let Some(filter_map_id) = prev {
+                        if *receiver == filter_map_id {
+                            self.diagnostics.push(
+                                BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap {
+                                    method_call_expr: id,
+                                },
+                            );
+                        }
+                    }
+                }
+            }
+            prev = None;
+        }
+    }
+
+    fn validate_call(&mut self, db: &dyn HirDatabase, call_id: ExprId, expr: &Expr) {
         // Check that the number of arguments matches the number of parameters.
 
         // FIXME: Due to shortcomings in the current type system implementation, only emit this
@@ -168,24 +214,6 @@ impl ExprValidator {
                 (sig, args.len())
             }
             Expr::MethodCall { receiver, args, .. } => {
-                let (callee, subst) = match self.infer.method_resolution(call_id) {
-                    Some(it) => it,
-                    None => return,
-                };
-
-                if filter_map_next_checker
-                    .get_or_insert_with(|| {
-                        FilterMapNextChecker::new(&self.owner.resolver(db.upcast()), db)
-                    })
-                    .check(call_id, receiver, &callee)
-                    .is_some()
-                {
-                    self.diagnostics.push(
-                        BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap {
-                            method_call_expr: call_id,
-                        },
-                    );
-                }
                 let receiver = &self.infer.type_of_expr[*receiver];
                 if receiver.strip_references().is_unknown() {
                     // if the receiver is of unknown type, it's very likely we
@@ -194,6 +222,10 @@ impl ExprValidator {
                     return;
                 }
 
+                let (callee, subst) = match self.infer.method_resolution(call_id) {
+                    Some(it) => it,
+                    None => return,
+                };
                 let sig = db.callable_item_signature(callee.into()).substitute(Interner, &subst);
 
                 (sig, args.len() + 1)
@@ -392,63 +424,6 @@ impl ExprValidator {
     }
 }
 
-struct FilterMapNextChecker {
-    filter_map_function_id: Option<hir_def::FunctionId>,
-    next_function_id: Option<hir_def::FunctionId>,
-    prev_filter_map_expr_id: Option<ExprId>,
-}
-
-impl FilterMapNextChecker {
-    fn new(resolver: &hir_def::resolver::Resolver, db: &dyn HirDatabase) -> Self {
-        // Find and store the FunctionIds for Iterator::filter_map and Iterator::next
-        let iterator_path = path![core::iter::Iterator];
-        let mut filter_map_function_id = None;
-        let mut next_function_id = None;
-
-        if let Some(iterator_trait_id) = resolver.resolve_known_trait(db.upcast(), &iterator_path) {
-            let iterator_trait_items = &db.trait_data(iterator_trait_id).items;
-            for item in iterator_trait_items.iter() {
-                if let (name, AssocItemId::FunctionId(id)) = item {
-                    if *name == name![filter_map] {
-                        filter_map_function_id = Some(*id);
-                    }
-                    if *name == name![next] {
-                        next_function_id = Some(*id);
-                    }
-                }
-                if filter_map_function_id.is_some() && next_function_id.is_some() {
-                    break;
-                }
-            }
-        }
-        Self { filter_map_function_id, next_function_id, prev_filter_map_expr_id: None }
-    }
-
-    // check for instances of .filter_map(..).next()
-    fn check(
-        &mut self,
-        current_expr_id: ExprId,
-        receiver_expr_id: &ExprId,
-        function_id: &hir_def::FunctionId,
-    ) -> Option<()> {
-        if *function_id == self.filter_map_function_id? {
-            self.prev_filter_map_expr_id = Some(current_expr_id);
-            return None;
-        }
-
-        if *function_id == self.next_function_id? {
-            if let Some(prev_filter_map_expr_id) = self.prev_filter_map_expr_id {
-                if *receiver_expr_id == prev_filter_map_expr_id {
-                    return Some(());
-                }
-            }
-        }
-
-        self.prev_filter_map_expr_id = None;
-        None
-    }
-}
-
 pub fn record_literal_missing_fields(
     db: &dyn HirDatabase,
     infer: &InferenceResult,
diff --git a/crates/hir_ty/src/display.rs b/crates/hir_ty/src/display.rs
index 95d1550afc..cd898a45d2 100644
--- a/crates/hir_ty/src/display.rs
+++ b/crates/hir_ty/src/display.rs
@@ -567,27 +567,7 @@ impl HirDisplay for Ty {
                     };
                     if !parameters_to_write.is_empty() {
                         write!(f, "<")?;
-
-                        if f.display_target.is_source_code() {
-                            let mut first = true;
-                            for generic_arg in parameters_to_write {
-                                if !first {
-                                    write!(f, ", ")?;
-                                }
-                                first = false;
-
-                                if generic_arg.ty(Interner).map(|ty| ty.kind(Interner))
-                                    == Some(&TyKind::Error)
-                                {
-                                    write!(f, "_")?;
-                                } else {
-                                    generic_arg.hir_fmt(f)?;
-                                }
-                            }
-                        } else {
-                            f.write_joined(parameters_to_write, ", ")?;
-                        }
-
+                        f.write_joined(parameters_to_write, ", ")?;
                         write!(f, ">")?;
                     }
                 }
diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs
index cc304cb10a..b479a20beb 100644
--- a/crates/ide/src/inlay_hints.rs
+++ b/crates/ide/src/inlay_hints.rs
@@ -257,23 +257,26 @@ fn is_named_constructor(
     }?;
     let expr = match expr {
         ast::Expr::CallExpr(call) => match call.expr()? {
-            ast::Expr::PathExpr(path) => path,
+            ast::Expr::PathExpr(p) => p,
             _ => return None,
         },
-        ast::Expr::PathExpr(path) => path,
         _ => return None,
     };
     let path = expr.path()?;
 
+    // Check for tuple-struct or tuple-variant in which case we can check the last segment
     let callable = sema.type_of_expr(&ast::Expr::PathExpr(expr))?.original.as_callable(sema.db);
     let callable_kind = callable.map(|it| it.kind());
-    let qual_seg = match callable_kind {
-        Some(hir::CallableKind::Function(_) | hir::CallableKind::TupleEnumVariant(_)) => {
-            path.qualifier()?.segment()
+    if let Some(hir::CallableKind::TupleStruct(_) | hir::CallableKind::TupleEnumVariant(_)) =
+        callable_kind
+    {
+        if let Some(ctor) = path.segment() {
+            return (ctor.to_string() == ty_name).then(|| ());
         }
-        _ => path.segment(),
-    }?;
+    }
 
+    // otherwise use the qualifying segment as the constructor name
+    let qual_seg = path.qualifier()?.segment()?;
     let ctor_name = match qual_seg.kind()? {
         ast::PathSegmentKind::Name(name_ref) => {
             match qual_seg.generic_arg_list().map(|it| it.generic_args()) {
@@ -1338,7 +1341,7 @@ fn main() {
     }
 
     #[test]
-    fn skip_constructor_and_enum_type_hints() {
+    fn skip_constructor_type_hints() {
         check_with_config(
             InlayHintsConfig {
                 type_hints: true,
@@ -1348,16 +1351,9 @@ fn main() {
                 max_length: None,
             },
             r#"
-//- minicore: try, option
+//- minicore: try
 use core::ops::ControlFlow;
 
-mod x {
-    pub mod y { pub struct Foo; }
-    pub struct Foo;
-    pub enum AnotherEnum {
-        Variant()
-    };
-}
 struct Struct;
 struct TupleStruct();
 
@@ -1377,39 +1373,13 @@ impl Generic<i32> {
     }
 }
 
-enum Enum {
-    Variant(u32)
-}
-
-fn times2(value: i32) -> i32 {
-    2 * value
-}
-
 fn main() {
-    let enumb = Enum::Variant(0);
-
-    let strukt = x::Foo;
-    let strukt = x::y::Foo;
-    let strukt = Struct;
     let strukt = Struct::new();
-
     let tuple_struct = TupleStruct();
-
     let generic0 = Generic::new();
-    //  ^^^^^^^^ Generic<i32>
-    let generic1 = Generic(0);
-    //  ^^^^^^^^ Generic<i32>
-    let generic2 = Generic::<i32>::new();
-    let generic3 = <Generic<i32>>::new();
-    let generic4 = Generic::<i32>(0);
-
-
-    let option = Some(0);
-    //  ^^^^^^ Option<i32>
-    let func = times2;
-    //  ^^^^ fn times2(i32) -> i32
-    let closure = |x: i32| x * 2;
-    //  ^^^^^^^ |i32| -> i32
+     // ^^^^^^^^ Generic<i32>
+    let generic1 = Generic::<i32>::new();
+    let generic2 = <Generic<i32>>::new();
 }
 
 fn fallible() -> ControlFlow<()> {
diff --git a/crates/ide/src/syntax_highlighting.rs b/crates/ide/src/syntax_highlighting.rs
index f20d629fbf..28c7c546ff 100644
--- a/crates/ide/src/syntax_highlighting.rs
+++ b/crates/ide/src/syntax_highlighting.rs
@@ -282,10 +282,7 @@ fn traverse(
 
         // only attempt to descend if we are inside a macro call or attribute
         // as calling `descend_into_macros_single` gets rather expensive if done for every single token
-        // additionally, do not descend into comments, descending maps down to doc attributes which get
-        // tagged as string literals.
-        let descend_token = (current_macro_call.is_some() || current_attr_call.is_some())
-            && element.kind() != COMMENT;
+        let descend_token = current_macro_call.is_some() || current_attr_call.is_some();
         let element_to_highlight = if descend_token {
             let token = match &element {
                 NodeOrToken::Node(_) => continue,
diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs
index 41b13170bb..4a09d341f7 100644
--- a/crates/ide/src/syntax_highlighting/highlight.rs
+++ b/crates/ide/src/syntax_highlighting/highlight.rs
@@ -712,13 +712,11 @@ fn parent_matches<N: AstNode>(token: &SyntaxToken) -> bool {
     token.parent().map_or(false, |it| N::can_cast(it.kind()))
 }
 
-fn is_in_fn_with_self_param(node: &ast::NameRef) -> bool {
+fn is_in_fn_with_self_param<N: AstNode>(node: &N) -> bool {
     node.syntax()
         .ancestors()
-        .find_map(ast::Item::cast)
-        .and_then(|item| match item {
-            ast::Item::Fn(fn_) => fn_.param_list()?.self_param(),
-            _ => None,
-        })
+        .take_while(|node| ast::Expr::can_cast(node.kind()) || ast::Fn::can_cast(node.kind()))
+        .find_map(ast::Fn::cast)
+        .and_then(|s| s.param_list()?.self_param())
         .is_some()
 }
diff --git a/crates/ide_assists/src/handlers/add_explicit_type.rs b/crates/ide_assists/src/handlers/add_explicit_type.rs
index d7e1be900f..169bb0cbf4 100644
--- a/crates/ide_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ide_assists/src/handlers/add_explicit_type.rs
@@ -60,8 +60,8 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext) -> Optio
     }
     .adjusted();
 
-    // Fully unresolved or unnameable types can't be annotated
-    if (ty.contains_unknown() && ty.type_arguments().count() == 0) || ty.is_closure() {
+    // Unresolved or unnameable types can't be annotated
+    if ty.contains_unknown() || ty.is_closure() {
         cov_mark::hit!(add_explicit_type_not_applicable_if_ty_not_inferred);
         return None;
     }
@@ -139,34 +139,11 @@ fn f() {
     }
 
     #[test]
-    fn add_explicit_type_not_applicable_for_fully_unresolved() {
+    fn add_explicit_type_not_applicable_unresolved() {
         cov_mark::check!(add_explicit_type_not_applicable_if_ty_not_inferred);
         check_assist_not_applicable(add_explicit_type, r#"fn f() { let a$0 = None; }"#);
     }
 
-    #[test]
-    fn add_explicit_type_applicable_for_partially_unresolved() {
-        check_assist(
-            add_explicit_type,
-            r#"
-        struct Vec<T, V> { t: T, v: V }
-        impl<T> Vec<T, Vec<ZZZ, i32>> {
-            fn new() -> Self {
-                panic!()
-            }
-        }
-        fn f() { let a$0 = Vec::new(); }"#,
-            r#"
-        struct Vec<T, V> { t: T, v: V }
-        impl<T> Vec<T, Vec<ZZZ, i32>> {
-            fn new() -> Self {
-                panic!()
-            }
-        }
-        fn f() { let a: Vec<_, Vec<_, i32>> = Vec::new(); }"#,
-        );
-    }
-
     #[test]
     fn add_explicit_type_not_applicable_closure_expr() {
         check_assist_not_applicable(add_explicit_type, r#"fn f() { let a$0 = || {}; }"#);
diff --git a/crates/ide_assists/src/handlers/move_guard.rs b/crates/ide_assists/src/handlers/move_guard.rs
index 366f308f6e..8a2c51d33b 100644
--- a/crates/ide_assists/src/handlers/move_guard.rs
+++ b/crates/ide_assists/src/handlers/move_guard.rs
@@ -1,9 +1,7 @@
 use syntax::{
-    ast::{
-        edit::AstNodeEdit, make, AstNode, BlockExpr, Condition, ElseBranch, Expr, IfExpr, MatchArm,
-        Pat,
-    },
-    SyntaxKind::WHITESPACE,
+    ast::{edit::AstNodeEdit, make, AstNode, BlockExpr, ElseBranch, Expr, IfExpr, MatchArm},
+    NodeOrToken,
+    SyntaxKind::{COMMA, WHITESPACE},
 };
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -117,107 +115,73 @@ pub(crate) fn move_arm_cond_to_match_guard(acc: &mut Assists, ctx: &AssistContex
         }
     })?;
     let replace_node = replace_node.unwrap_or_else(|| if_expr.syntax().clone());
-    let needs_dedent = replace_node != *if_expr.syntax();
-    let (conds_blocks, tail) = parse_if_chain(if_expr)?;
+
+    let cond = if_expr.condition()?;
+    let then_block = if_expr.then_branch()?;
+
+    // Not support moving if let to arm guard
+    if cond.is_pattern_cond() {
+        return None;
+    }
+
+    let buf = format!(" if {}", cond.syntax().text());
 
     acc.add(
         AssistId("move_arm_cond_to_match_guard", AssistKind::RefactorRewrite),
         "Move condition to match guard",
         replace_node.text_range(),
         |edit| {
-            edit.delete(match_arm.syntax().text_range());
-            // Dedent if if_expr is in a BlockExpr
-            let dedent = if needs_dedent {
-                cov_mark::hit!(move_guard_ifelse_in_block);
-                1
-            } else {
-                cov_mark::hit!(move_guard_ifelse_else_block);
-                0
-            };
-            let then_arm_end = match_arm.syntax().text_range().end();
-            let indent_level = match_arm.indent_level();
-            let spaces = "    ".repeat(indent_level.0 as _);
-
-            let mut first = true;
-            for (cond, block) in conds_blocks {
-                if !first {
-                    edit.insert(then_arm_end, format!("\n{}", spaces));
-                } else {
-                    first = false;
-                }
-                let guard = format!("{} if {} => ", match_pat, cond.syntax().text());
-                edit.insert(then_arm_end, guard);
-                let only_expr = block.statements().next().is_none();
-                match &block.tail_expr() {
-                    Some(then_expr) if only_expr => {
-                        edit.insert(then_arm_end, then_expr.syntax().text());
-                        edit.insert(then_arm_end, ",");
-                    }
-                    _ => {
-                        let to_insert = block.dedent(dedent.into()).syntax().text();
-                        edit.insert(then_arm_end, to_insert)
+            let then_only_expr = then_block.statements().next().is_none();
+
+            match &then_block.tail_expr() {
+                Some(then_expr) if then_only_expr => {
+                    edit.replace(replace_node.text_range(), then_expr.syntax().text());
+                    // Insert comma for expression if there isn't one
+                    match match_arm.syntax().last_child_or_token() {
+                        Some(NodeOrToken::Token(t)) if t.kind() == COMMA => {}
+                        _ => {
+                            cov_mark::hit!(move_guard_if_add_comma);
+                            edit.insert(match_arm.syntax().text_range().end(), ",");
+                        }
                     }
                 }
+                _ if replace_node != *if_expr.syntax() => {
+                    // Dedent because if_expr is in a BlockExpr
+                    let replace_with = then_block.dedent(1.into()).syntax().text();
+                    edit.replace(replace_node.text_range(), replace_with)
+                }
+                _ => edit.replace(replace_node.text_range(), then_block.syntax().text()),
             }
-            if let Some(e) = tail {
-                cov_mark::hit!(move_guard_ifelse_else_tail);
-                let guard = format!("\n{}{} => ", spaces, match_pat);
-                edit.insert(then_arm_end, guard);
-                let only_expr = e.statements().next().is_none();
-                match &e.tail_expr() {
-                    Some(expr) if only_expr => {
+
+            edit.insert(match_pat.syntax().text_range().end(), buf);
+
+            // If with only an else branch
+            if let Some(ElseBranch::Block(else_block)) = if_expr.else_branch() {
+                let then_arm_end = match_arm.syntax().text_range().end();
+                let else_only_expr = else_block.statements().next().is_none();
+                let indent_level = match_arm.indent_level();
+                let spaces = "    ".repeat(indent_level.0 as _);
+                edit.insert(then_arm_end, format!("\n{}{} => ", spaces, match_pat));
+                match &else_block.tail_expr() {
+                    Some(else_expr) if else_only_expr => {
                         cov_mark::hit!(move_guard_ifelse_expr_only);
-                        edit.insert(then_arm_end, expr.syntax().text());
+                        edit.insert(then_arm_end, else_expr.syntax().text());
                         edit.insert(then_arm_end, ",");
                     }
-                    _ => {
-                        let to_insert = e.dedent(dedent.into()).syntax().text();
-                        edit.insert(then_arm_end, to_insert)
+                    _ if replace_node != *if_expr.syntax() => {
+                        cov_mark::hit!(move_guard_ifelse_in_block);
+                        edit.insert(then_arm_end, else_block.dedent(1.into()).syntax().text());
                     }
-                }
-            } else {
-                // There's no else branch. Add a pattern without guard, unless the following match
-                // arm is `_ => ...`
-                cov_mark::hit!(move_guard_ifelse_notail);
-                match match_arm.syntax().next_sibling().and_then(MatchArm::cast) {
-                    Some(next_arm)
-                        if matches!(next_arm.pat(), Some(Pat::WildcardPat(_)))
-                            && next_arm.guard().is_none() =>
-                    {
-                        cov_mark::hit!(move_guard_ifelse_has_wildcard);
+                    _ => {
+                        cov_mark::hit!(move_guard_ifelse_else_block);
+                        edit.insert(then_arm_end, else_block.syntax().text());
                     }
-                    _ => edit.insert(then_arm_end, format!("\n{}{} => {{}}", spaces, match_pat)),
                 }
             }
         },
     )
 }
 
-// Parses an if-else-if chain to get the conditons and the then branches until we encounter an else
-// branch or the end.
-fn parse_if_chain(if_expr: IfExpr) -> Option<(Vec<(Condition, BlockExpr)>, Option<BlockExpr>)> {
-    let mut conds_blocks = Vec::new();
-    let mut curr_if = if_expr;
-    let tail = loop {
-        let cond = curr_if.condition()?;
-        // Not support moving if let to arm guard
-        if cond.is_pattern_cond() {
-            return None;
-        }
-        conds_blocks.push((cond, curr_if.then_branch()?));
-        match curr_if.else_branch() {
-            Some(ElseBranch::IfExpr(e)) => {
-                curr_if = e;
-            }
-            Some(ElseBranch::Block(b)) => {
-                break Some(b);
-            }
-            None => break None,
-        }
-    };
-    Some((conds_blocks, tail))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -330,7 +294,6 @@ fn main() {
 
     #[test]
     fn move_arm_cond_in_block_to_match_guard_works() {
-        cov_mark::check!(move_guard_ifelse_has_wildcard);
         check_assist(
             move_arm_cond_to_match_guard,
             r#"
@@ -356,64 +319,9 @@ fn main() {
         );
     }
 
-    #[test]
-    fn move_arm_cond_in_block_to_match_guard_no_wildcard_works() {
-        cov_mark::check_count!(move_guard_ifelse_has_wildcard, 0);
-        check_assist(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        x => {
-            $0if x > 10 {
-                false
-            }
-        }
-    }
-}
-"#,
-            r#"
-fn main() {
-    match 92 {
-        x if x > 10 => false,
-        x => {}
-    }
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn move_arm_cond_in_block_to_match_guard_wildcard_guard_works() {
-        cov_mark::check_count!(move_guard_ifelse_has_wildcard, 0);
-        check_assist(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        x => {
-            $0if x > 10 {
-                false
-            }
-        }
-        _ if x > 10 => true,
-    }
-}
-"#,
-            r#"
-fn main() {
-    match 92 {
-        x if x > 10 => false,
-        x => {}
-        _ if x > 10 => true,
-    }
-}
-"#,
-        );
-    }
-
     #[test]
     fn move_arm_cond_in_block_to_match_guard_add_comma_works() {
+        cov_mark::check!(move_guard_if_add_comma);
         check_assist(
             move_arm_cond_to_match_guard,
             r#"
@@ -469,7 +377,7 @@ fn main() {
             r#"
 fn main() {
     match 92 {
-        x if x > 10 => {  }
+        x if x > 10 => {  },
         _ => true
     }
 }
@@ -498,7 +406,7 @@ fn main() {
         x if x > 10 => {
             92;
             false
-        }
+        },
         _ => true
     }
 }
@@ -611,7 +519,7 @@ fn main() {
             r#"
 fn main() {
     match 92 {
-        x if x > 10 => {  }
+        x if x > 10 => {  },
         x => { }
         _ => true
     }
@@ -782,177 +690,6 @@ fn main() {
         }
     }
 }
-"#,
-        )
-    }
-
-    #[test]
-    fn move_arm_cond_to_match_guard_elseif() {
-        check_assist(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        3 => true,
-        x => if x > 10 {$0
-            false
-        } else if x > 5 {
-            true
-        } else if x > 4 {
-            false
-        } else {
-            true
-        },
-    }
-}
-"#,
-            r#"
-fn main() {
-    match 92 {
-        3 => true,
-        x if x > 10 => false,
-        x if x > 5 => true,
-        x if x > 4 => false,
-        x => true,
-    }
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn move_arm_cond_to_match_guard_elseif_in_block() {
-        cov_mark::check!(move_guard_ifelse_in_block);
-        check_assist(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        3 => true,
-        x => {
-            if x > 10 {$0
-                false
-            } else if x > 5 {
-                true
-            } else if x > 4 {
-                false
-            } else {
-                true
-            }
-        }
-    }
-}
-"#,
-            r#"
-fn main() {
-    match 92 {
-        3 => true,
-        x if x > 10 => false,
-        x if x > 5 => true,
-        x if x > 4 => false,
-        x => true,
-    }
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn move_arm_cond_to_match_guard_elseif_chain() {
-        cov_mark::check!(move_guard_ifelse_else_tail);
-        check_assist(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        3 => 0,
-        x => if x > 10 {$0
-            1
-        } else if x > 5 {
-            2
-        } else if x > 3 {
-            42;
-            3
-        } else {
-            4
-        },
-    }
-}
-"#,
-            r#"
-fn main() {
-    match 92 {
-        3 => 0,
-        x if x > 10 => 1,
-        x if x > 5 => 2,
-        x if x > 3 => {
-            42;
-            3
-        }
-        x => 4,
-    }
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn move_arm_cond_to_match_guard_elseif_iflet() {
-        check_assist_not_applicable(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        3 => 0,
-        x => if x > 10 {$0
-            1
-        } else if x > 5 {
-            2
-        } else if let 4 = 4 {
-            42;
-            3
-        } else {
-            4
-        },
-    }
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn move_arm_cond_to_match_guard_elseif_notail() {
-        cov_mark::check!(move_guard_ifelse_notail);
-        check_assist(
-            move_arm_cond_to_match_guard,
-            r#"
-fn main() {
-    match 92 {
-        3 => 0,
-        x => if x > 10 {$0
-            1
-        } else if x > 5 {
-            2
-        } else if x > 4 {
-            42;
-            3
-        },
-    }
-}
-"#,
-            r#"
-fn main() {
-    match 92 {
-        3 => 0,
-        x if x > 10 => 1,
-        x if x > 5 => 2,
-        x if x > 4 => {
-            42;
-            3
-        }
-        x => {}
-    }
-}
 "#,
         )
     }
diff --git a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
index 7790934792..47d1fbe144 100644
--- a/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
+++ b/crates/ide_assists/src/handlers/replace_if_let_with_match.rs
@@ -207,23 +207,21 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext)
         "Replace match with if let",
         target,
         move |edit| {
-            fn make_block_expr(expr: ast::Expr) -> ast::BlockExpr {
-                // Blocks with modifiers (unsafe, async, etc.) are parsed as BlockExpr, but are
-                // formatted without enclosing braces. If we encounter such block exprs,
-                // wrap them in another BlockExpr.
-                match expr {
-                    ast::Expr::BlockExpr(block) if block.modifier().is_none() => block,
-                    expr => make::block_expr(iter::empty(), Some(expr)),
-                }
-            }
-
             let condition = make::condition(scrutinee, Some(if_let_pat));
-            let then_block = make_block_expr(then_expr.reset_indent());
+            let then_block = match then_expr.reset_indent() {
+                ast::Expr::BlockExpr(block) => block,
+                expr => make::block_expr(iter::empty(), Some(expr)),
+            };
             let else_expr = if is_empty_expr(&else_expr) { None } else { Some(else_expr) };
             let if_let_expr = make::expr_if(
                 condition,
                 then_block,
-                else_expr.map(make_block_expr).map(ast::ElseBranch::Block),
+                else_expr
+                    .map(|expr| match expr {
+                        ast::Expr::BlockExpr(block) => block,
+                        expr => (make::block_expr(iter::empty(), Some(expr))),
+                    })
+                    .map(ast::ElseBranch::Block),
             )
             .indent(IndentLevel::from_node(match_expr.syntax()));
 
@@ -919,30 +917,4 @@ fn foo() {
 "#,
         );
     }
-
-    #[test]
-    fn test_replace_match_with_if_let_keeps_unsafe_block() {
-        check_assist(
-            replace_match_with_if_let,
-            r#"
-impl VariantData {
-    pub fn is_struct(&self) -> bool {
-        $0match *self {
-            VariantData::Struct(..) => true,
-            _ => unsafe { unreachable_unchecked() },
-        }
-    }
-}           "#,
-            r#"
-impl VariantData {
-    pub fn is_struct(&self) -> bool {
-        if let VariantData::Struct(..) = *self {
-            true
-        } else {
-            unsafe { unreachable_unchecked() }
-        }
-    }
-}           "#,
-        )
-    }
 }
diff --git a/crates/ide_completion/src/completions.rs b/crates/ide_completion/src/completions.rs
index 974264c319..19fdc6c244 100644
--- a/crates/ide_completion/src/completions.rs
+++ b/crates/ide_completion/src/completions.rs
@@ -90,7 +90,7 @@ impl Completions {
             cov_mark::hit!(qualified_path_doc_hidden);
             return;
         }
-        self.add_opt(render_resolution(RenderContext::new(ctx), local_name, resolution));
+        self.add(render_resolution(RenderContext::new(ctx), local_name, resolution));
     }
 
     pub(crate) fn add_macro(
@@ -103,7 +103,7 @@ impl Completions {
             Some(it) => it,
             None => return,
         };
-        self.add_opt(render_macro(RenderContext::new(ctx), None, name, macro_));
+        self.add(render_macro(RenderContext::new(ctx), None, name, macro_));
     }
 
     pub(crate) fn add_function(
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs
index 4fa5aa04de..1ec6053a7e 100644
--- a/crates/ide_completion/src/render.rs
+++ b/crates/ide_completion/src/render.rs
@@ -130,7 +130,7 @@ pub(crate) fn render_resolution(
     ctx: RenderContext<'_>,
     local_name: hir::Name,
     resolution: ScopeDef,
-) -> Option<CompletionItem> {
+) -> CompletionItem {
     render_resolution_(ctx, local_name, None, resolution)
 }
 
@@ -145,7 +145,7 @@ pub(crate) fn render_resolution_with_import(
         ScopeDef::ModuleDef(hir::ModuleDef::TypeAlias(t)) => t.name(ctx.completion.db),
         _ => item_name(ctx.db(), import_edit.import.original_item)?,
     };
-    render_resolution_(ctx, local_name, Some(import_edit), resolution)
+    Some(render_resolution_(ctx, local_name, Some(import_edit), resolution))
 }
 
 fn render_resolution_(
@@ -153,7 +153,7 @@ fn render_resolution_(
     local_name: hir::Name,
     import_to_add: Option<ImportEdit>,
     resolution: ScopeDef,
-) -> Option<CompletionItem> {
+) -> CompletionItem {
     let _p = profile::span("render_resolution");
     use hir::ModuleDef::*;
 
@@ -161,10 +161,10 @@ fn render_resolution_(
 
     let kind = match resolution {
         ScopeDef::ModuleDef(Function(func)) => {
-            return Some(render_fn(ctx, import_to_add, Some(local_name), func));
+            return render_fn(ctx, import_to_add, Some(local_name), func);
         }
         ScopeDef::ModuleDef(Variant(var)) if ctx.completion.pattern_ctx.is_none() => {
-            return Some(render_variant(ctx, import_to_add, Some(local_name), var, None));
+            return render_variant(ctx, import_to_add, Some(local_name), var, None);
         }
         ScopeDef::MacroDef(mac) => return render_macro(ctx, import_to_add, local_name, mac),
         ScopeDef::Unknown => {
@@ -176,7 +176,7 @@ fn render_resolution_(
             if let Some(import_to_add) = import_to_add {
                 item.add_import(import_to_add);
             }
-            return Some(item.build());
+            return item.build();
         }
 
         ScopeDef::ModuleDef(Variant(_)) => CompletionItemKind::SymbolKind(SymbolKind::Variant),
@@ -249,7 +249,7 @@ fn render_resolution_(
     if let Some(import_to_add) = import_to_add {
         item.add_import(import_to_add);
     }
-    Some(item.build())
+    item.build()
 }
 
 fn scope_def_docs(db: &RootDatabase, resolution: ScopeDef) -> Option<hir::Documentation> {
diff --git a/crates/ide_completion/src/render/macro_.rs b/crates/ide_completion/src/render/macro_.rs
index 9c7b9a6db3..5386cbcb27 100644
--- a/crates/ide_completion/src/render/macro_.rs
+++ b/crates/ide_completion/src/render/macro_.rs
@@ -19,7 +19,7 @@ pub(crate) fn render_macro(
     import_to_add: Option<ImportEdit>,
     name: hir::Name,
     macro_: hir::MacroDef,
-) -> Option<CompletionItem> {
+) -> CompletionItem {
     let _p = profile::span("render_macro");
     render(ctx, name, macro_, import_to_add)
 }
@@ -29,15 +29,15 @@ fn render(
     name: hir::Name,
     macro_: hir::MacroDef,
     import_to_add: Option<ImportEdit>,
-) -> Option<CompletionItem> {
+) -> CompletionItem {
     let db = completion.db;
 
     let source_range = if completion.is_immediately_after_macro_bang() {
         cov_mark::hit!(completes_macro_call_if_cursor_at_bang_token);
-        completion.token.parent().map(|it| it.text_range())
+        completion.token.parent().map_or_else(|| ctx.source_range(), |it| it.text_range())
     } else {
-        Some(ctx.source_range())
-    }?;
+        ctx.source_range()
+    };
 
     let name = name.to_smol_str();
     let docs = ctx.docs(macro_);
@@ -79,7 +79,7 @@ fn render(
         }
     };
 
-    Some(item.build())
+    item.build()
 }
 
 fn label(
diff --git a/crates/mbe/src/expander/matcher.rs b/crates/mbe/src/expander/matcher.rs
index fb785665b9..910724411d 100644
--- a/crates/mbe/src/expander/matcher.rs
+++ b/crates/mbe/src/expander/matcher.rs
@@ -697,7 +697,6 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
         "block" => parser::PrefixEntryPoint::Block,
         "meta" => parser::PrefixEntryPoint::MetaItem,
         "item" => parser::PrefixEntryPoint::Item,
-        "vis" => parser::PrefixEntryPoint::Vis,
         "expr" => {
             return input
                 .expect_fragment(parser::PrefixEntryPoint::Expr)
@@ -707,10 +706,12 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
             let tt_result = match kind {
                 "ident" => input
                     .expect_ident()
-                    .map(|ident| tt::Leaf::from(ident.clone()).into())
+                    .map(|ident| Some(tt::Leaf::from(ident.clone()).into()))
                     .map_err(|()| err!("expected ident")),
-                "tt" => input.expect_tt().map_err(|()| err!()),
-                "lifetime" => input.expect_lifetime().map_err(|()| err!("expected lifetime")),
+                "tt" => input.expect_tt().map(Some).map_err(|()| err!()),
+                "lifetime" => {
+                    input.expect_lifetime().map(Some).map_err(|()| err!("expected lifetime"))
+                }
                 "literal" => {
                     let neg = input.eat_char('-');
                     input
@@ -718,18 +719,20 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragmen
                         .map(|literal| {
                             let lit = literal.clone();
                             match neg {
-                                None => lit.into(),
-                                Some(neg) => tt::TokenTree::Subtree(tt::Subtree {
+                                None => Some(lit.into()),
+                                Some(neg) => Some(tt::TokenTree::Subtree(tt::Subtree {
                                     delimiter: None,
                                     token_trees: vec![neg, lit.into()],
-                                }),
+                                })),
                             }
                         })
                         .map_err(|()| err!())
                 }
+                // `vis` is optional
+                "vis" => Ok(input.expect_fragment(parser::PrefixEntryPoint::Vis).value),
                 _ => Err(ExpandError::UnexpectedToken),
             };
-            return tt_result.map(|it| Some(Fragment::Tokens(it))).into();
+            return tt_result.map(|it| it.map(Fragment::Tokens)).into();
         }
     };
     input.expect_fragment(fragment).map(|it| it.map(Fragment::Tokens))
diff --git a/crates/mbe/src/tt_iter.rs b/crates/mbe/src/tt_iter.rs
index 9d2b1da9ab..172916c5a1 100644
--- a/crates/mbe/src/tt_iter.rs
+++ b/crates/mbe/src/tt_iter.rs
@@ -114,7 +114,7 @@ impl<'a> TtIter<'a> {
             }
         }
 
-        let err = if error || !cursor.is_root() {
+        let mut err = if !cursor.is_root() || error {
             Some(err!("expected {:?}", entry_point))
         } else {
             None
@@ -132,6 +132,9 @@ impl<'a> TtIter<'a> {
             }
         }
         self.inner = self.inner.as_slice()[res.len()..].iter();
+        if res.is_empty() && err.is_none() {
+            err = Some(err!("no tokens consumed"));
+        }
         let res = match res.len() {
             1 => Some(res[0].cloned()),
             0 => None,


Test patch:
diff --git a/crates/hir_def/src/macro_expansion_tests/mbe/matching.rs b/crates/hir_def/src/macro_expansion_tests/mbe/matching.rs
index bc162d0fa2..517dfb15b6 100644
--- a/crates/hir_def/src/macro_expansion_tests/mbe/matching.rs
+++ b/crates/hir_def/src/macro_expansion_tests/mbe/matching.rs
@@ -122,17 +122,3 @@ stringify!(.. .. ..|);
 "#]],
     );
 }
-
-#[test]
-fn trailing_vis() {
-    check(
-        r#"
-macro_rules! m { ($($i:ident)? $vis:vis) => () }
-m!(x pub);
-"#,
-        expect![[r#"
-macro_rules! m { ($($i:ident)? $vis:vis) => () }
-
-"#]],
-    )
-}