Repository: rust-lang/rust-analyzer
Pull request #13156 (2022-08-31 15:56:17)

Remove hir::Expr::MacroStmts
This hir expression isn't needed and only existed as it was simpler to
deal with at first as it gave us a direct mapping for the ast version of
the same construct. This PR removes it, properly handling the statements
that are introduced by macro call expressions.


Patch:
diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs
index f6ec8bf7e9..8ebac5cb1c 100644
--- a/crates/hir-def/src/body/lower.rs
+++ b/crates/hir-def/src/body/lower.rs
@@ -550,20 +550,6 @@ impl ExprCollector<'_> {
                     None => self.alloc_expr(Expr::Missing, syntax_ptr),
                 }
             }
-            ast::Expr::MacroStmts(e) => {
-                let statements: Box<[_]> =
-                    e.statements().filter_map(|s| self.collect_stmt(s)).collect();
-                let tail = e.expr().map(|e| self.collect_expr(e));
-
-                if e.syntax().children().next().is_none() {
-                    // HACK: make sure that macros that expand to nothing aren't treated as a `()`
-                    // expression when used in block tail position.
-                    cov_mark::hit!(empty_macro_in_trailing_position_is_removed);
-                    return None;
-                }
-
-                self.alloc_expr(Expr::MacroStmts { tail, statements }, syntax_ptr)
-            }
             ast::Expr::UnderscoreExpr(_) => self.alloc_expr(Expr::Underscore, syntax_ptr),
         })
     }
@@ -640,11 +626,46 @@ impl ExprCollector<'_> {
         }
     }
 
-    fn collect_stmt(&mut self, s: ast::Stmt) -> Option<Statement> {
+    fn collect_macro_as_stmt(
+        &mut self,
+        statements: &mut Vec<Statement>,
+        mac: ast::MacroExpr,
+    ) -> Option<ExprId> {
+        let mac_call = mac.macro_call()?;
+        let syntax_ptr = AstPtr::new(&ast::Expr::from(mac));
+        let macro_ptr = AstPtr::new(&mac_call);
+        let expansion = self.collect_macro_call(
+            mac_call,
+            macro_ptr,
+            false,
+            |this, expansion: Option<ast::MacroStmts>| match expansion {
+                Some(expansion) => {
+                    expansion.statements().for_each(|stmt| this.collect_stmt(statements, stmt));
+                    expansion.expr().and_then(|expr| match expr {
+                        ast::Expr::MacroExpr(mac) => this.collect_macro_as_stmt(statements, mac),
+                        expr => Some(this.collect_expr(expr)),
+                    })
+                }
+                None => None,
+            },
+        );
+        match expansion {
+            Some(tail) => {
+                // Make the macro-call point to its expanded expression so we can query
+                // semantics on syntax pointers to the macro
+                let src = self.expander.to_source(syntax_ptr);
+                self.source_map.expr_map.insert(src, tail);
+                Some(tail)
+            }
+            None => None,
+        }
+    }
+
+    fn collect_stmt(&mut self, statements: &mut Vec<Statement>, s: ast::Stmt) {
         match s {
             ast::Stmt::LetStmt(stmt) => {
                 if self.check_cfg(&stmt).is_none() {
-                    return None;
+                    return;
                 }
                 let pat = self.collect_pat_opt(stmt.pat());
                 let type_ref =
@@ -654,61 +675,26 @@ impl ExprCollector<'_> {
                     .let_else()
                     .and_then(|let_else| let_else.block_expr())
                     .map(|block| self.collect_block(block));
-                Some(Statement::Let { pat, type_ref, initializer, else_branch })
+                statements.push(Statement::Let { pat, type_ref, initializer, else_branch });
             }
             ast::Stmt::ExprStmt(stmt) => {
                 let expr = stmt.expr();
-                if let Some(expr) = &expr {
-                    if self.check_cfg(expr).is_none() {
-                        return None;
-                    }
+                match &expr {
+                    Some(expr) if self.check_cfg(expr).is_none() => return,
+                    _ => (),
                 }
                 let has_semi = stmt.semicolon_token().is_some();
                 // Note that macro could be expanded to multiple statements
-                if let Some(expr @ ast::Expr::MacroExpr(mac)) = &expr {
-                    let mac_call = mac.macro_call()?;
-                    let syntax_ptr = AstPtr::new(expr);
-                    let macro_ptr = AstPtr::new(&mac_call);
-                    let stmt = self.collect_macro_call(
-                        mac_call,
-                        macro_ptr,
-                        false,
-                        |this, expansion: Option<ast::MacroStmts>| match expansion {
-                            Some(expansion) => {
-                                let statements = expansion
-                                    .statements()
-                                    .filter_map(|stmt| this.collect_stmt(stmt))
-                                    .collect();
-                                let tail = expansion.expr().map(|expr| this.collect_expr(expr));
-
-                                let mac_stmts = this.alloc_expr(
-                                    Expr::MacroStmts { tail, statements },
-                                    AstPtr::new(&ast::Expr::MacroStmts(expansion)),
-                                );
-
-                                Some(mac_stmts)
-                            }
-                            None => None,
-                        },
-                    );
-
-                    let expr = match stmt {
-                        Some(expr) => {
-                            // Make the macro-call point to its expanded expression so we can query
-                            // semantics on syntax pointers to the macro
-                            let src = self.expander.to_source(syntax_ptr);
-                            self.source_map.expr_map.insert(src, expr);
-                            expr
-                        }
-                        None => self.alloc_expr(Expr::Missing, syntax_ptr),
-                    };
-                    Some(Statement::Expr { expr, has_semi })
+                if let Some(ast::Expr::MacroExpr(mac)) = expr {
+                    if let Some(expr) = self.collect_macro_as_stmt(statements, mac) {
+                        statements.push(Statement::Expr { expr, has_semi })
+                    }
                 } else {
                     let expr = self.collect_expr_opt(expr);
-                    Some(Statement::Expr { expr, has_semi })
+                    statements.push(Statement::Expr { expr, has_semi });
                 }
             }
-            ast::Stmt::Item(_item) => None,
+            ast::Stmt::Item(_item) => (),
         }
     }
 
@@ -729,9 +715,12 @@ impl ExprCollector<'_> {
         let prev_def_map = mem::replace(&mut self.expander.def_map, def_map);
         let prev_local_module = mem::replace(&mut self.expander.module, module);
 
-        let mut statements: Vec<_> =
-            block.statements().filter_map(|s| self.collect_stmt(s)).collect();
-        let tail = block.tail_expr().and_then(|e| self.maybe_collect_expr(e));
+        let mut statements = Vec::new();
+        block.statements().for_each(|s| self.collect_stmt(&mut statements, s));
+        let tail = block.tail_expr().and_then(|e| match e {
+            ast::Expr::MacroExpr(mac) => self.collect_macro_as_stmt(&mut statements, mac),
+            expr => self.maybe_collect_expr(expr),
+        });
         let tail = tail.or_else(|| {
             let stmt = statements.pop()?;
             if let Statement::Expr { expr, has_semi: false } = stmt {
diff --git a/crates/hir-def/src/body/pretty.rs b/crates/hir-def/src/body/pretty.rs
index ddd476efe5..f2fed95444 100644
--- a/crates/hir-def/src/body/pretty.rs
+++ b/crates/hir-def/src/body/pretty.rs
@@ -422,19 +422,6 @@ impl<'a> Printer<'a> {
                 }
                 w!(self, "}}");
             }
-            Expr::MacroStmts { statements, tail } => {
-                w!(self, "{{ // macro statements");
-                self.indented(|p| {
-                    for stmt in statements.iter() {
-                        p.print_stmt(stmt);
-                    }
-                    if let Some(tail) = tail {
-                        p.print_expr(*tail);
-                    }
-                });
-                self.newline();
-                w!(self, "}}");
-            }
         }
     }
 
diff --git a/crates/hir-def/src/body/scope.rs b/crates/hir-def/src/body/scope.rs
index f4c390dce2..9b28e38029 100644
--- a/crates/hir-def/src/body/scope.rs
+++ b/crates/hir-def/src/body/scope.rs
@@ -176,9 +176,6 @@ fn compute_expr_scopes(expr: ExprId, body: &Body, scopes: &mut ExprScopes, scope
 
     scopes.set_scope(expr, *scope);
     match &body[expr] {
-        Expr::MacroStmts { statements, tail } => {
-            compute_block_scopes(statements, *tail, body, scopes, scope);
-        }
         Expr::Block { statements, tail, id, label } => {
             let mut scope = scopes.new_block_scope(*scope, *id, make_label(label));
             // Overwrite the old scope for the block expr, so that every block scope can be found
diff --git a/crates/hir-def/src/expr.rs b/crates/hir-def/src/expr.rs
index 4381b43c25..419d3feec3 100644
--- a/crates/hir-def/src/expr.rs
+++ b/crates/hir-def/src/expr.rs
@@ -206,10 +206,6 @@ pub enum Expr {
     Unsafe {
         body: ExprId,
     },
-    MacroStmts {
-        statements: Box<[Statement]>,
-        tail: Option<ExprId>,
-    },
     Array(Array),
     Literal(Literal),
     Underscore,
@@ -263,7 +259,7 @@ impl Expr {
             Expr::Let { expr, .. } => {
                 f(*expr);
             }
-            Expr::MacroStmts { tail, statements } | Expr::Block { statements, tail, .. } => {
+            Expr::Block { statements, tail, .. } => {
                 for stmt in statements.iter() {
                     match stmt {
                         Statement::Let { initializer, .. } => {
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index d753d88470..fc128102f2 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs
@@ -969,7 +969,7 @@ impl ExpandTo {
         if parent.kind() == MACRO_EXPR
             && parent
                 .parent()
-                .map_or(true, |p| matches!(p.kind(), EXPR_STMT | STMT_LIST | MACRO_STMTS))
+                .map_or(false, |p| matches!(p.kind(), EXPR_STMT | STMT_LIST | MACRO_STMTS))
         {
             return ExpandTo::Statements;
         }
diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs
index 2a13106390..a42a00ea59 100644
--- a/crates/hir-ty/src/infer/expr.rs
+++ b/crates/hir-ty/src/infer/expr.rs
@@ -794,9 +794,6 @@ impl<'a> InferenceContext<'a> {
                     None => self.table.new_float_var(),
                 },
             },
-            Expr::MacroStmts { tail, statements } => {
-                self.infer_block(tgt_expr, statements, *tail, expected)
-            }
             Expr::Underscore => {
                 // Underscore expressions may only appear in assignee expressions,
                 // which are handled by `infer_assignee_expr()`, so any underscore
diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs
index bd35af06e2..342912b678 100644
--- a/crates/hir/src/source_analyzer.rs
+++ b/crates/hir/src/source_analyzer.rs
@@ -140,11 +140,19 @@ impl SourceAnalyzer {
     ) -> Option<InFile<ast::Expr>> {
         let macro_file = self.body_source_map()?.node_macro_file(expr.as_ref())?;
         let expanded = db.parse_or_expand(macro_file)?;
-
-        let res = match ast::MacroCall::cast(expanded.clone()) {
-            Some(call) => self.expand_expr(db, InFile::new(macro_file, call))?,
-            _ => InFile::new(macro_file, ast::Expr::cast(expanded)?),
+        let res = if let Some(stmts) = ast::MacroStmts::cast(expanded.clone()) {
+            match stmts.expr()? {
+                ast::Expr::MacroExpr(mac) => {
+                    self.expand_expr(db, InFile::new(macro_file, mac.macro_call()?))?
+                }
+                expr => InFile::new(macro_file, expr),
+            }
+        } else if let Some(call) = ast::MacroCall::cast(expanded.clone()) {
+            self.expand_expr(db, InFile::new(macro_file, call))?
+        } else {
+            InFile::new(macro_file, ast::Expr::cast(expanded)?)
         };
+
         Some(res)
     }
 
diff --git a/crates/ide-assists/src/handlers/convert_two_arm_bool_match_to_matches_macro.rs b/crates/ide-assists/src/handlers/convert_two_arm_bool_match_to_matches_macro.rs
deleted file mode 100644
index 54a7f480a4..0000000000
--- a/crates/ide-assists/src/handlers/convert_two_arm_bool_match_to_matches_macro.rs
+++ /dev/null
@@ -1,294 +0,0 @@
-use syntax::ast::{self, AstNode};
-
-use crate::{AssistContext, AssistId, AssistKind, Assists};
-
-// Assist: convert_two_arm_bool_match_to_matches_macro
-//
-// Convert 2-arm match that evaluates to a boolean into the equivalent matches! invocation.
-//
-// ```
-// fn main() {
-//     match scrutinee$0 {
-//         Some(val) if val.cond() => true,
-//         _ => false,
-//     }
-// }
-// ```
-// ->
-// ```
-// fn main() {
-//     matches!(scrutinee, Some(val) if val.cond())
-// }
-// ```
-pub(crate) fn convert_two_arm_bool_match_to_matches_macro(
-    acc: &mut Assists,
-    ctx: &AssistContext<'_>,
-) -> Option<()> {
-    let match_expr = ctx.find_node_at_offset::<ast::MatchExpr>()?;
-    let match_arm_list = match_expr.match_arm_list()?;
-    let mut arms = match_arm_list.arms();
-    let first_arm = arms.next()?;
-    let second_arm = arms.next()?;
-    if arms.next().is_some() {
-        cov_mark::hit!(non_two_arm_match);
-        return None;
-    }
-    let first_arm_expr = first_arm.expr();
-    let second_arm_expr = second_arm.expr();
-
-    let invert_matches = if is_bool_literal_expr(&first_arm_expr, true)
-        && is_bool_literal_expr(&second_arm_expr, false)
-    {
-        false
-    } else if is_bool_literal_expr(&first_arm_expr, false)
-        && is_bool_literal_expr(&second_arm_expr, true)
-    {
-        true
-    } else {
-        cov_mark::hit!(non_invert_bool_literal_arms);
-        return None;
-    };
-
-    let target_range = ctx.sema.original_range(match_expr.syntax()).range;
-    let expr = match_expr.expr()?;
-
-    acc.add(
-        AssistId("convert_two_arm_bool_match_to_matches_macro", AssistKind::RefactorRewrite),
-        "Convert to matches!",
-        target_range,
-        |builder| {
-            let mut arm_str = String::new();
-            if let Some(ref pat) = first_arm.pat() {
-                arm_str += &pat.to_string();
-            }
-            if let Some(ref guard) = first_arm.guard() {
-                arm_str += &format!(" {}", &guard.to_string());
-            }
-            if invert_matches {
-                builder.replace(target_range, format!("!matches!({}, {})", expr, arm_str));
-            } else {
-                builder.replace(target_range, format!("matches!({}, {})", expr, arm_str));
-            }
-        },
-    )
-}
-
-fn is_bool_literal_expr(expr: &Option<ast::Expr>, expect_bool: bool) -> bool {
-    if let Some(ast::Expr::Literal(lit)) = expr {
-        if let ast::LiteralKind::Bool(b) = lit.kind() {
-            return b == expect_bool;
-        }
-    }
-
-    return false;
-}
-
-#[cfg(test)]
-mod tests {
-    use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
-
-    use super::convert_two_arm_bool_match_to_matches_macro;
-
-    #[test]
-    fn not_applicable_outside_of_range_left() {
-        check_assist_not_applicable(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    $0 match a {
-        Some(_val) => true,
-        _ => false
-    }
-}
-        "#,
-        );
-    }
-
-    #[test]
-    fn not_applicable_non_two_arm_match() {
-        cov_mark::check!(non_two_arm_match);
-        check_assist_not_applicable(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(3) => true,
-        Some(4) => true,
-        _ => false
-    }
-}
-        "#,
-        );
-    }
-
-    #[test]
-    fn not_applicable_non_bool_literal_arms() {
-        cov_mark::check!(non_invert_bool_literal_arms);
-        check_assist_not_applicable(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(val) => val == 3,
-        _ => false
-    }
-}
-        "#,
-        );
-    }
-    #[test]
-    fn not_applicable_both_false_arms() {
-        cov_mark::check!(non_invert_bool_literal_arms);
-        check_assist_not_applicable(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(val) => false,
-        _ => false
-    }
-}
-        "#,
-        );
-    }
-
-    #[test]
-    fn not_applicable_both_true_arms() {
-        cov_mark::check!(non_invert_bool_literal_arms);
-        check_assist_not_applicable(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(val) => true,
-        _ => true
-    }
-}
-        "#,
-        );
-    }
-
-    #[test]
-    fn convert_simple_case() {
-        check_assist(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(_val) => true,
-        _ => false
-    }
-}
-"#,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    matches!(a, Some(_val))
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn convert_simple_invert_case() {
-        check_assist(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(_val) => false,
-        _ => true
-    }
-}
-"#,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    !matches!(a, Some(_val))
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn convert_with_guard_case() {
-        check_assist(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(val) if val > 3 => true,
-        _ => false
-    }
-}
-"#,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    matches!(a, Some(val) if val > 3)
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn convert_enum_match_cases() {
-        check_assist(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-enum X { A, B }
-
-fn foo(a: X) -> bool {
-    match a$0 {
-        X::A => true,
-        _ => false
-    }
-}
-"#,
-            r#"
-enum X { A, B }
-
-fn foo(a: X) -> bool {
-    matches!(a, X::A)
-}
-"#,
-        );
-    }
-
-    #[test]
-    fn convert_target_simple() {
-        check_assist_target(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-fn foo(a: Option<u32>) -> bool {
-    match a$0 {
-        Some(val) => true,
-        _ => false
-    }
-}
-"#,
-            r#"match a {
-        Some(val) => true,
-        _ => false
-    }"#,
-        );
-    }
-
-    #[test]
-    fn convert_target_complex() {
-        check_assist_target(
-            convert_two_arm_bool_match_to_matches_macro,
-            r#"
-enum E { X, Y }
-
-fn main() {
-    match E::X$0 {
-        E::X => true,
-        _ => false,
-    }
-}
-"#,
-            "match E::X {
-        E::X => true,
-        _ => false,
-    }",
-        );
-    }
-}
diff --git a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs
index ddc2052e7a..dfb5652126 100644
--- a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -101,22 +101,21 @@ pub(crate) fn extract_struct_from_enum_variant(
                 });
             }
 
+            let indent = enum_ast.indent_level();
             let generic_params = enum_ast
                 .generic_param_list()
                 .and_then(|known_generics| extract_generic_params(&known_generics, &field_list));
             let generics = generic_params.as_ref().map(|generics| generics.clone_for_update());
             let def =
                 create_struct_def(variant_name.clone(), &variant, &field_list, generics, &enum_ast);
-
-            let enum_ast = variant.parent_enum();
-            let indent = enum_ast.indent_level();
             def.reindent_to(indent);
 
-            ted::insert_all(
-                ted::Position::before(enum_ast.syntax()),
+            let start_offset = &variant.parent_enum().syntax().clone();
+            ted::insert_all_raw(
+                ted::Position::before(start_offset),
                 vec![
                     def.syntax().clone().into(),
-                    make::tokens::whitespace(&format!("\n\n{indent}")).into(),
+                    make::tokens::whitespace(&format!("\n\n{}", indent)).into(),
                 ],
             );
 
@@ -228,7 +227,7 @@ fn tag_generics_in_variant(ty: &ast::Type, generics: &mut [(ast::GenericParam, b
 }
 
 fn create_struct_def(
-    name: ast::Name,
+    variant_name: ast::Name,
     variant: &ast::Variant,
     field_list: &Either<ast::RecordFieldList, ast::TupleFieldList>,
     generics: Option<ast::GenericParamList>,
@@ -270,27 +269,43 @@ fn create_struct_def(
             field_list.into()
         }
     };
+
     field_list.reindent_to(IndentLevel::single());
 
-    let strukt = make::struct_(enum_vis, name, generics, field_list).clone_for_update();
+    let strukt = make::struct_(enum_vis, variant_name, generics, field_list).clone_for_update();
 
-    // take comments from variant
-    ted::insert_all(
-        ted::Position::first_child_of(strukt.syntax()),
-        take_all_comments(variant.syntax()),
-    );
+    // FIXME: Consider making this an actual function somewhere (like in `AttrsOwnerEdit`) after some deliberation
+    let attrs_and_docs = |node: &SyntaxNode| {
+        let mut select_next_ws = false;
+        node.children_with_tokens().filter(move |child| {
+            let accept = match child.kind() {
+                ATTR | COMMENT => {
+                    select_next_ws = true;
+                    return true;
+                }
+                WHITESPACE if select_next_ws => true,
+                _ => false,
+            };
+            select_next_ws = false;
+
+            accept
+        })
+    };
+
+    // copy attributes & comments from variant
+    let variant_attrs = attrs_and_docs(variant.syntax())
+        .map(|tok| match tok.kind() {
+            WHITESPACE => make::tokens::single_newline().into(),
+            _ => tok,
+        })
+        .collect();
+    ted::insert_all(ted::Position::first_child_of(strukt.syntax()), variant_attrs);
 
     // copy attributes from enum
     ted::insert_all(
         ted::Position::first_child_of(strukt.syntax()),
-        enum_
-            .attrs()
-            .flat_map(|it| {
-                vec![it.syntax().clone_for_update().into(), make::tokens::single_newline().into()]
-            })
-            .collect(),
+        enum_.attrs().map(|it| it.syntax().clone_for_update().into()).collect(),
     );
-
     strukt
 }
 
@@ -331,48 +346,16 @@ fn update_variant(variant: &ast::Variant, generics: Option<ast::GenericParamList
         })
         .unwrap_or_else(|| make::ty(&name.text()));
 
-    // change from a record to a tuple field list
     let tuple_field = make::tuple_field(None, ty);
-    let field_list = make::tuple_field_list(iter::once(tuple_field)).clone_for_update();
-    ted::replace(variant.field_list()?.syntax(), field_list.syntax());
-
-    // remove any ws after the name
-    if let Some(ws) = name
-        .syntax()
-        .siblings_with_tokens(syntax::Direction::Next)
-        .find_map(|tok| tok.into_token().filter(|tok| tok.kind() == WHITESPACE))
-    {
-        ted::remove(SyntaxElement::Token(ws));
-    }
-
+    let replacement = make::variant(
+        name,
+        Some(ast::FieldList::TupleFieldList(make::tuple_field_list(iter::once(tuple_field)))),
+    )
+    .clone_for_update();
+    ted::replace(variant.syntax(), replacement.syntax());
     Some(())
 }
 
-// Note: this also detaches whitespace after comments,
-// since `SyntaxNode::splice_children` (and by extension `ted::insert_all_raw`)
-// detaches nodes. If we only took the comments, we'd leave behind the old whitespace.
-fn take_all_comments(node: &SyntaxNode) -> Vec<SyntaxElement> {
-    let mut remove_next_ws = false;
-    node.children_with_tokens()
-        .filter_map(move |child| match child.kind() {
-            COMMENT => {
-                remove_next_ws = true;
-                child.detach();
-                Some(child)
-            }
-            WHITESPACE if remove_next_ws => {
-                remove_next_ws = false;
-                child.detach();
-                Some(make::tokens::single_newline().into())
-            }
-            _ => {
-                remove_next_ws = false;
-                None
-            }
-        })
-        .collect()
-}
-
 fn apply_references(
     insert_use_cfg: InsertUseConfig,
     segment: ast::PathSegment,
@@ -497,14 +480,10 @@ enum En<T> { Var(Var<T>) }"#,
     fn test_extract_struct_carries_over_attributes() {
         check_assist(
             extract_struct_from_enum_variant,
-            r#"
-#[derive(Debug)]
+            r#"#[derive(Debug)]
 #[derive(Clone)]
 enum Enum { Variant{ field: u32$0 } }"#,
-            r#"
-#[derive(Debug)]
-#[derive(Clone)]
-struct Variant{ field: u32 }
+            r#"#[derive(Debug)]#[derive(Clone)] struct Variant{ field: u32 }
 
 #[derive(Debug)]
 #[derive(Clone)]
@@ -635,7 +614,7 @@ enum A { One(One) }"#,
     }
 
     #[test]
-    fn test_extract_struct_move_struct_variant_comments() {
+    fn test_extract_struct_keep_comments_and_attrs_on_variant_struct() {
         check_assist(
             extract_struct_from_enum_variant,
             r#"
@@ -652,19 +631,19 @@ enum A {
 /* comment */
 // other
 /// comment
+#[attr]
 struct One{
     a: u32
 }
 
 enum A {
-    #[attr]
     One(One)
 }"#,
         );
     }
 
     #[test]
-    fn test_extract_struct_move_tuple_variant_comments() {
+    fn test_extract_struct_keep_comments_and_attrs_on_variant_tuple() {
         check_assist(
             extract_struct_from_enum_variant,
             r#"
@@ -679,10 +658,10 @@ enum A {
 /* comment */
 // other
 /// comment
+#[attr]
 struct One(u32, u32);
 
 enum A {
-    #[attr]
     One(One)
 }"#,
         );
diff --git a/crates/ide-assists/src/handlers/replace_or_with_or_else.rs b/crates/ide-assists/src/handlers/replace_or_with_or_else.rs
deleted file mode 100644
index 7d91be6210..0000000000
--- a/crates/ide-assists/src/handlers/replace_or_with_or_else.rs
+++ /dev/null
@@ -1,364 +0,0 @@
-use ide_db::{
-    assists::{AssistId, AssistKind},
-    famous_defs::FamousDefs,
-};
-use syntax::{
-    ast::{self, make, Expr, HasArgList},
-    AstNode,
-};
-
-use crate::{AssistContext, Assists};
-
-// Assist: replace_or_with_or_else
-//
-// Replace `unwrap_or` with `unwrap_or_else` and `ok_or` with `ok_or_else`.
-//
-// ```
-// # //- minicore:option
-// fn foo() {
-//     let a = Some(1);
-//     a.unwra$0p_or(2);
-// }
-// ```
-// ->
-// ```
-// fn foo() {
-//     let a = Some(1);
-//     a.unwrap_or_else(|| 2);
-// }
-// ```
-pub(crate) fn replace_or_with_or_else(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
-    let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
-
-    let kind = is_option_or_result(call.receiver()?, ctx)?;
-
-    let (name, arg_list) = (call.name_ref()?, call.arg_list()?);
-
-    let mut map_or = false;
-
-    let replace = match &*name.text() {
-        "unwrap_or" => "unwrap_or_else".to_string(),
-        "or" => "or_else".to_string(),
-        "ok_or" if kind == Kind::Option => "ok_or_else".to_string(),
-        "map_or" => {
-            map_or = true;
-            "map_or_else".to_string()
-        }
-        _ => return None,
-    };
-
-    let arg = match arg_list.args().collect::<Vec<_>>().as_slice() {
-        [] => make::arg_list(Vec::new()),
-        [first] => {
-            let param = into_closure(first);
-            make::arg_list(vec![param])
-        }
-        [first, second] if map_or => {
-            let param = into_closure(first);
-            make::arg_list(vec![param, second.clone()])
-        }
-        _ => return None,
-    };
-
-    acc.add(
-        AssistId("replace_or_with_or_else", AssistKind::RefactorRewrite),
-        format!("Replace {} with {}", name.text(), replace),
-        call.syntax().text_range(),
-        |builder| {
-            builder.replace(name.syntax().text_range(), replace);
-            builder.replace_ast(arg_list, arg)
-        },
-    )
-}
-
-fn into_closure(param: &Expr) -> Expr {
-    (|| {
-        if let ast::Expr::CallExpr(call) = param {
-            if call.arg_list()?.args().count() == 0 {
-                Some(call.expr()?.clone())
-            } else {
-                None
-            }
-        } else {
-            None
-        }
-    })()
-    .unwrap_or_else(|| make::expr_closure(None, param.clone()))
-}
-
-// Assist: replace_or_else_with_or
-//
-// Replace `unwrap_or_else` with `unwrap_or` and `ok_or_else` with `ok_or`.
-//
-// ```
-// # //- minicore:option
-// fn foo() {
-//     let a = Some(1);
-//     a.unwra$0p_or_else(|| 2);
-// }
-// ```
-// ->
-// ```
-// fn foo() {
-//     let a = Some(1);
-//     a.unwrap_or(2);
-// }
-// ```
-pub(crate) fn replace_or_else_with_or(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
-    let call: ast::MethodCallExpr = ctx.find_node_at_offset()?;
-
-    let kind = is_option_or_result(call.receiver()?, ctx)?;
-
-    let (name, arg_list) = (call.name_ref()?, call.arg_list()?);
-
-    let mut map_or = false;
-    let replace = match &*name.text() {
-        "unwrap_or_else" => "unwrap_or".to_string(),
-        "or_else" => "or".to_string(),
-        "ok_or_else" if kind == Kind::Option => "ok_or".to_string(),
-        "map_or_else" => {
-            map_or = true;
-            "map_or".to_string()
-        }
-        _ => return None,
-    };
-
-    let arg = match arg_list.args().collect::<Vec<_>>().as_slice() {
-        [] => make::arg_list(Vec::new()),
-        [first] => {
-            let param = into_call(first);
-            make::arg_list(vec![param])
-        }
-        [first, second] if map_or => {
-            let param = into_call(first);
-            make::arg_list(vec![param, second.clone()])
-        }
-        _ => return None,
-    };
-
-    acc.add(
-        AssistId("replace_or_else_with_or", AssistKind::RefactorRewrite),
-        format!("Replace {} with {}", name.text(), replace),
-        call.syntax().text_range(),
-        |builder| {
-            builder.replace(name.syntax().text_range(), replace);
-            builder.replace_ast(arg_list, arg)
-        },
-    )
-}
-
-fn into_call(param: &Expr) -> Expr {
-    (|| {
-        if let ast::Expr::ClosureExpr(closure) = param {
-            if closure.param_list()?.params().count() == 0 {
-                Some(closure.body()?.clone())
-            } else {
-                None
-            }
-        } else {
-            None
-        }
-    })()
-    .unwrap_or_else(|| make::expr_call(param.clone(), make::arg_list(Vec::new())))
-}
-
-#[derive(PartialEq, Eq)]
-enum Kind {
-    Option,
-    Result,
-}
-
-fn is_option_or_result(receiver: Expr, ctx: &AssistContext<'_>) -> Option<Kind> {
-    let ty = ctx.sema.type_of_expr(&receiver)?.adjusted().as_adt()?.as_enum()?;
-    let option_enum =
-        FamousDefs(&ctx.sema, ctx.sema.scope(receiver.syntax())?.krate()).core_option_Option();
-
-    if let Some(option_enum) = option_enum {
-        if ty == option_enum {
-            return Some(Kind::Option);
-        }
-    }
-
-    let result_enum =
-        FamousDefs(&ctx.sema, ctx.sema.scope(receiver.syntax())?.krate()).core_result_Result();
-
-    if let Some(result_enum) = result_enum {
-        if ty == result_enum {
-            return Some(Kind::Result);
-        }
-    }
-
-    None
-}
-
-#[cfg(test)]
-mod tests {
-    use crate::tests::{check_assist, check_assist_not_applicable};
-
-    use super::*;
-
-    #[test]
-    fn replace_or_with_or_else_simple() {
-        check_assist(
-            replace_or_with_or_else,
-            r#"
-//- minicore: option
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_$0or(2);
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_or_else(|| 2);
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_with_or_else_call() {
-        check_assist(
-            replace_or_with_or_else,
-            r#"
-//- minicore: option
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_$0or(x());
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_or_else(x);
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_with_or_else_block() {
-        check_assist(
-            replace_or_with_or_else,
-            r#"
-//- minicore: option
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_$0or({
-        let mut x = bar();
-        for i in 0..10 {
-            x += i;
-        }
-        x
-    });
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_or_else(|| {
-        let mut x = bar();
-        for i in 0..10 {
-            x += i;
-        }
-        x
-    });
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_else_with_or_simple() {
-        check_assist(
-            replace_or_else_with_or,
-            r#"
-//- minicore: option
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_$0or_else(|| 2);
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_or(2);
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_else_with_or_call() {
-        check_assist(
-            replace_or_else_with_or,
-            r#"
-//- minicore: option
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_$0or_else(x);
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Some(1);
-    return foo.unwrap_or(x());
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_else_with_or_result() {
-        check_assist(
-            replace_or_else_with_or,
-            r#"
-//- minicore: result
-fn foo() {
-    let foo = Ok(1);
-    return foo.unwrap_$0or_else(x);
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Ok(1);
-    return foo.unwrap_or(x());
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_else_with_or_map() {
-        check_assist(
-            replace_or_else_with_or,
-            r#"
-//- minicore: result
-fn foo() {
-    let foo = Ok("foo");
-    return foo.map$0_or_else(|| 42, |v| v.len());
-}
-"#,
-            r#"
-fn foo() {
-    let foo = Ok("foo");
-    return foo.map_or(42, |v| v.len());
-}
-"#,
-        )
-    }
-
-    #[test]
-    fn replace_or_else_with_or_not_applicable() {
-        check_assist_not_applicable(
-            replace_or_else_with_or,
-            r#"
-fn foo() {
-    let foo = Ok(1);
-    return foo.unwrap_$0or_else(x);
-}
-"#,
-        )
-    }
-}
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index e52544db5f..c558553b1c 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -122,7 +122,6 @@ mod handlers {
     mod convert_let_else_to_match;
     mod convert_tuple_struct_to_named_struct;
     mod convert_to_guarded_return;
-    mod convert_two_arm_bool_match_to_matches_macro;
     mod convert_while_to_loop;
     mod destructure_tuple_binding;
     mod expand_glob_import;
@@ -180,7 +179,6 @@ mod handlers {
     mod replace_try_expr_with_match;
     mod replace_derive_with_manual_impl;
     mod replace_if_let_with_match;
-    mod replace_or_with_or_else;
     mod introduce_named_generic;
     mod replace_let_with_if_let;
     mod replace_qualified_name_with_use;
@@ -218,7 +216,6 @@ mod handlers {
             convert_let_else_to_match::convert_let_else_to_match,
             convert_to_guarded_return::convert_to_guarded_return,
             convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct,
-            convert_two_arm_bool_match_to_matches_macro::convert_two_arm_bool_match_to_matches_macro,
             convert_while_to_loop::convert_while_to_loop,
             destructure_tuple_binding::destructure_tuple_binding,
             expand_glob_import::expand_glob_import,
@@ -277,8 +274,6 @@ mod handlers {
             replace_if_let_with_match::replace_if_let_with_match,
             replace_if_let_with_match::replace_match_with_if_let,
             replace_let_with_if_let::replace_let_with_if_let,
-            replace_or_with_or_else::replace_or_else_with_or,
-            replace_or_with_or_else::replace_or_with_or_else,
             replace_turbofish_with_explicit_type::replace_turbofish_with_explicit_type,
             replace_qualified_name_with_use::replace_qualified_name_with_use,
             sort_items::sort_items,
diff --git a/crates/ide-db/src/syntax_helpers/node_ext.rs b/crates/ide-db/src/syntax_helpers/node_ext.rs
index 84bde4d44d..b890e2b58d 100644
--- a/crates/ide-db/src/syntax_helpers/node_ext.rs
+++ b/crates/ide-db/src/syntax_helpers/node_ext.rs
@@ -315,7 +315,6 @@ pub fn for_each_tail_expr(expr: &ast::Expr, cb: &mut dyn FnMut(&ast::Expr)) {
         | ast::Expr::IndexExpr(_)
         | ast::Expr::Literal(_)
         | ast::Expr::MacroExpr(_)
-        | ast::Expr::MacroStmts(_)
         | ast::Expr::MethodCallExpr(_)
         | ast::Expr::ParenExpr(_)
         | ast::Expr::PathExpr(_)
diff --git a/crates/syntax/rust.ungram b/crates/syntax/rust.ungram
index 62aa478399..8947954354 100644
--- a/crates/syntax/rust.ungram
+++ b/crates/syntax/rust.ungram
@@ -343,7 +343,6 @@ Expr =
 | Literal
 | LoopExpr
 | MacroExpr
-| MacroStmts
 | MatchExpr
 | MethodCallExpr
 | ParenExpr
diff --git a/crates/syntax/src/ast/generated/nodes.rs b/crates/syntax/src/ast/generated/nodes.rs
index 8c4825ad69..449402e5f5 100644
--- a/crates/syntax/src/ast/generated/nodes.rs
+++ b/crates/syntax/src/ast/generated/nodes.rs
@@ -1526,7 +1526,6 @@ pub enum Expr {
     Literal(Literal),
     LoopExpr(LoopExpr),
     MacroExpr(MacroExpr),
-    MacroStmts(MacroStmts),
     MatchExpr(MatchExpr),
     MethodCallExpr(MethodCallExpr),
     ParenExpr(ParenExpr),
@@ -3342,9 +3341,6 @@ impl From<LoopExpr> for Expr {
 impl From<MacroExpr> for Expr {
     fn from(node: MacroExpr) -> Expr { Expr::MacroExpr(node) }
 }
-impl From<MacroStmts> for Expr {
-    fn from(node: MacroStmts) -> Expr { Expr::MacroStmts(node) }
-}
 impl From<MatchExpr> for Expr {
     fn from(node: MatchExpr) -> Expr { Expr::MatchExpr(node) }
 }
@@ -3411,7 +3407,6 @@ impl AstNode for Expr {
                 | LITERAL
                 | LOOP_EXPR
                 | MACRO_EXPR
-                | MACRO_STMTS
                 | MATCH_EXPR
                 | METHOD_CALL_EXPR
                 | PAREN_EXPR
@@ -3448,7 +3443,6 @@ impl AstNode for Expr {
             LITERAL => Expr::Literal(Literal { syntax }),
             LOOP_EXPR => Expr::LoopExpr(LoopExpr { syntax }),
             MACRO_EXPR => Expr::MacroExpr(MacroExpr { syntax }),
-            MACRO_STMTS => Expr::MacroStmts(MacroStmts { syntax }),
             MATCH_EXPR => Expr::MatchExpr(MatchExpr { syntax }),
             METHOD_CALL_EXPR => Expr::MethodCallExpr(MethodCallExpr { syntax }),
             PAREN_EXPR => Expr::ParenExpr(ParenExpr { syntax }),
@@ -3487,7 +3481,6 @@ impl AstNode for Expr {
             Expr::Literal(it) => &it.syntax,
             Expr::LoopExpr(it) => &it.syntax,
             Expr::MacroExpr(it) => &it.syntax,
-            Expr::MacroStmts(it) => &it.syntax,
             Expr::MatchExpr(it) => &it.syntax,
             Expr::MethodCallExpr(it) => &it.syntax,
             Expr::ParenExpr(it) => &it.syntax,


Test patch:
diff --git a/crates/hir-ty/src/tests/macros.rs b/crates/hir-ty/src/tests/macros.rs
index a1ab6060e7..b3adafaafd 100644
--- a/crates/hir-ty/src/tests/macros.rs
+++ b/crates/hir-ty/src/tests/macros.rs
@@ -193,8 +193,6 @@ fn expr_macro_def_expanded_in_various_places() {
             !0..6 '1isize': isize
             !0..6 '1isize': isize
             !0..6 '1isize': isize
-            !0..6 '1isize': isize
-            !0..6 '1isize': isize
             39..442 '{     ...!(); }': ()
             73..94 'spam!(...am!())': {unknown}
             100..119 'for _ ...!() {}': ()
@@ -276,8 +274,6 @@ fn expr_macro_rules_expanded_in_various_places() {
             !0..6 '1isize': isize
             !0..6 '1isize': isize
             !0..6 '1isize': isize
-            !0..6 '1isize': isize
-            !0..6 '1isize': isize
             53..456 '{     ...!(); }': ()
             87..108 'spam!(...am!())': {unknown}
             114..133 'for _ ...!() {}': ()
@@ -312,7 +308,6 @@ fn expr_macro_expanded_in_stmts() {
         }
         "#,
         expect![[r#"
-            !0..8 'leta=();': ()
             !3..4 'a': ()
             !5..7 '()': ()
             57..84 '{     ...); } }': ()
@@ -321,7 +316,7 @@ fn expr_macro_expanded_in_stmts() {
 }
 
 #[test]
-fn recurisve_macro_expanded_in_stmts() {
+fn recursive_macro_expanded_in_stmts() {
     check_infer(
         r#"
         macro_rules! ng {
@@ -340,11 +335,6 @@ fn recurisve_macro_expanded_in_stmts() {
         }
         "#,
         expect![[r#"
-            !0..7 'leta=3;': ()
-            !0..13 'ng!{[leta=3]}': ()
-            !0..13 'ng!{[leta=]3}': ()
-            !0..13 'ng!{[leta]=3}': ()
-            !0..13 'ng!{[let]a=3}': ()
             !3..4 'a': i32
             !5..6 '3': i32
             196..237 '{     ...= a; }': ()
@@ -369,8 +359,6 @@ fn recursive_inner_item_macro_rules() {
         "#,
         expect![[r#"
             !0..1 '1': i32
-            !0..7 'mac!($)': ()
-            !0..26 'macro_...>{1};}': ()
             107..143 '{     ...!(); }': ()
             129..130 'a': i32
         "#]],
diff --git a/crates/hir-ty/src/tests/regression.rs b/crates/hir-ty/src/tests/regression.rs
index c7895db1af..23e51a9c16 100644
--- a/crates/hir-ty/src/tests/regression.rs
+++ b/crates/hir-ty/src/tests/regression.rs
@@ -573,7 +573,6 @@ fn issue_6811() {
         }
         "#,
         expect![[r#"
-            !0..16 'let_a=...t_b=1;': ()
             !3..5 '_a': i32
             !6..7 '1': i32
             !11..13 '_b': i32
@@ -1679,7 +1678,6 @@ fn main() {
 
 #[test]
 fn trailing_empty_macro() {
-    cov_mark::check!(empty_macro_in_trailing_position_is_removed);
     check_no_mismatches(
         r#"
 macro_rules! m2 {
diff --git a/crates/hir-ty/src/tests/simple.rs b/crates/hir-ty/src/tests/simple.rs
index 5b08f55210..707e9e8450 100644
--- a/crates/hir-ty/src/tests/simple.rs
+++ b/crates/hir-ty/src/tests/simple.rs
@@ -2549,7 +2549,6 @@ impl B for Astruct {}
         expect![[r#"
             569..573 'self': Box<[T], A>
             602..634 '{     ...     }': Vec<T, A>
-            612..628 'unimpl...ted!()': Vec<T, A>
             648..761 '{     ...t]); }': ()
             658..661 'vec': Vec<i32, Global>
             664..679 '<[_]>::into_vec': fn into_vec<i32, Global>(Box<[i32], Global>) -> Vec<i32, Global>
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index a8c8622c1c..7b2c16806b 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -472,26 +472,6 @@ impl Point {
     )
 }
 
-#[test]
-fn doctest_convert_two_arm_bool_match_to_matches_macro() {
-    check_doc_test(
-        "convert_two_arm_bool_match_to_matches_macro",
-        r#####"
-fn main() {
-    match scrutinee$0 {
-        Some(val) if val.cond() => true,
-        _ => false,
-    }
-}
-"#####,
-        r#####"
-fn main() {
-    matches!(scrutinee, Some(val) if val.cond())
-}
-"#####,
-    )
-}
-
 #[test]
 fn doctest_convert_while_to_loop() {
     check_doc_test(
@@ -2029,46 +2009,6 @@ fn handle(action: Action) {
     )
 }
 
-#[test]
-fn doctest_replace_or_else_with_or() {
-    check_doc_test(
-        "replace_or_else_with_or",
-        r#####"
-//- minicore:option
-fn foo() {
-    let a = Some(1);
-    a.unwra$0p_or_else(|| 2);
-}
-"#####,
-        r#####"
-fn foo() {
-    let a = Some(1);
-    a.unwrap_or(2);
-}
-"#####,
-    )
-}
-
-#[test]
-fn doctest_replace_or_with_or_else() {
-    check_doc_test(
-        "replace_or_with_or_else",
-        r#####"
-//- minicore:option
-fn foo() {
-    let a = Some(1);
-    a.unwra$0p_or(2);
-}
-"#####,
-        r#####"
-fn foo() {
-    let a = Some(1);
-    a.unwrap_or_else(|| 2);
-}
-"#####,
-    )
-}
-
 #[test]
 fn doctest_replace_qualified_name_with_use() {
     check_doc_test(