Repository: rust-lang/rust-analyzer (rust)

Issue:
`"editor.formatOnType": true` (vscode) behaves weirdly with calls (and attributes on calls)
**rust-analyzer version**: 0.4.1881-standalone (14558af15 2024-03-14)

**relevant settings**: `"editor.formatOnType": true` 

**code snippet to reproduce**: 
```rust
fn main() {
    f();

    #[allow(unreachable_code)]
    g();
}
```

If I type `{` before `f();` nothing happens (well, except the `{` that was typed, obv). If I type `{` before `g();` a matching `}` is added but not where I'd expect:

```rust
fn main() {
    {f();

    #[allow(unreachable_code)]
    {g}();
}
```

This is what I would expect to see instead:

```rust
fn main() {
    { f() };

    #[allow(unreachable_code)]
    { g() };
}
```

<!-- TRIAGEBOT_START -->

<!-- TRIAGEBOT_ASSIGN_START -->

<!-- TRIAGEBOT_ASSIGN_DATA_START$${"user":"roife"}$$TRIAGEBOT_ASSIGN_DATA_END -->

<!-- TRIAGEBOT_ASSIGN_END -->
<!-- TRIAGEBOT_END -->

Pull request description:
fix: handle attributes when typing curly bracket
fix #16848.

When inserting a `{`, if it is identified that the front part of `expr` is `attr`, we consider it as inserting `{}` around the entire `expr` (excluding the attr part).

Patch:
diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs
index 0885d92ae8..a679a114b4 100644
--- a/crates/hir-ty/src/method_resolution.rs
+++ b/crates/hir-ty/src/method_resolution.rs
@@ -966,7 +966,7 @@ pub fn iterate_method_candidates_dyn(
             // the methods by autoderef order of *receiver types*, not *self
             // types*.
 
-            let mut table = InferenceTable::new(db, env);
+            let mut table = InferenceTable::new(db, env.clone());
             let ty = table.instantiate_canonical(ty.clone());
             let deref_chain = autoderef_method_receiver(&mut table, ty);
 
@@ -1044,7 +1044,7 @@ fn iterate_method_candidates_with_autoref(
     let ref_muted = Canonical {
         value: TyKind::Ref(Mutability::Mut, static_lifetime(), receiver_ty.value.clone())
             .intern(Interner),
-        binders: receiver_ty.binders,
+        binders: receiver_ty.binders.clone(),
     };
 
     iterate_method_candidates_by_receiver(ref_muted, first_adjustment.with_autoref(Mutability::Mut))
@@ -1060,7 +1060,7 @@ fn iterate_method_candidates_by_receiver(
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(ReceiverAdjustments, AssocItemId, bool) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
-    let receiver_ty = table.instantiate_canonical(receiver_ty);
+    let receiver_ty = table.instantiate_canonical(receiver_ty.clone());
     // We're looking for methods with *receiver* type receiver_ty. These could
     // be found in any of the derefs of receiver_ty, so we have to go through
     // that, including raw derefs.
@@ -1456,7 +1456,7 @@ fn is_valid_trait_method_candidate(
                 if let Some(receiver_ty) = receiver_ty {
                     check_that!(data.has_self_param());
 
-                    let fn_subst = TyBuilder::subst_for_def(db, fn_id, Some(impl_subst))
+                    let fn_subst = TyBuilder::subst_for_def(db, fn_id, Some(impl_subst.clone()))
                         .fill_with_inference_vars(table)
                         .build();
 
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 4f9697f7fe..b922aa8e46 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -1099,7 +1099,7 @@ impl Field {
             VariantDef::Union(it) => it.id.into(),
             VariantDef::Variant(it) => it.parent_enum(db).id.into(),
         };
-        let mut generics = generics.map(|it| it.ty);
+        let mut generics = generics.map(|it| it.ty.clone());
         let substs = TyBuilder::subst_for_def(db, def_id, None)
             .fill(|x| match x {
                 ParamKind::Type => {
@@ -1440,7 +1440,7 @@ impl Adt {
     /// the greatest API, FIXME find a better one.
     pub fn ty_with_args(self, db: &dyn HirDatabase, args: impl Iterator<Item = Type>) -> Type {
         let id = AdtId::from(self);
-        let mut it = args.map(|t| t.ty);
+        let mut it = args.map(|t| t.ty.clone());
         let ty = TyBuilder::def_ty(db, id.into(), None)
             .fill(|x| {
                 let r = it.next().unwrap_or_else(|| TyKind::Error.intern(Interner));
@@ -1859,7 +1859,7 @@ impl Function {
             ItemContainerId::TraitId(it) => Some(it.into()),
             ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => None,
         };
-        let mut generics = generics.map(|it| it.ty);
+        let mut generics = generics.map(|it| it.ty.clone());
         let mut filler = |x: &_| match x {
             ParamKind::Type => {
                 generics.next().unwrap_or_else(|| TyKind::Error.intern(Interner)).cast(Interner)
@@ -1954,7 +1954,7 @@ impl Function {
             ItemContainerId::TraitId(it) => Some(it.into()),
             ItemContainerId::ModuleId(_) | ItemContainerId::ExternBlockId(_) => None,
         };
-        let mut generics = generics.map(|it| it.ty);
+        let mut generics = generics.map(|it| it.ty.clone());
         let parent_substs = parent_id.map(|id| {
             TyBuilder::subst_for_def(db, id, None)
                 .fill(|x| match x {
@@ -2215,7 +2215,7 @@ impl SelfParam {
             }
         };
 
-        let mut generics = generics.map(|it| it.ty);
+        let mut generics = generics.map(|it| it.ty.clone());
         let mut filler = |x: &_| match x {
             ParamKind::Type => {
                 generics.next().unwrap_or_else(|| TyKind::Error.intern(Interner)).cast(Interner)
diff --git a/crates/hir/src/term_search/tactics.rs b/crates/hir/src/term_search/tactics.rs
index 0ccca21e04..102e0ca4c3 100644
--- a/crates/hir/src/term_search/tactics.rs
+++ b/crates/hir/src/term_search/tactics.rs
@@ -177,7 +177,7 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
                 // Note that we need special case for 0 param constructors because of multi cartesian
                 // product
                 let variant_exprs: Vec<Expr> = if param_exprs.is_empty() {
-                    vec![Expr::Variant { variant, generics, params: Vec::new() }]
+                    vec![Expr::Variant { variant, generics: generics.clone(), params: Vec::new() }]
                 } else {
                     param_exprs
                         .into_iter()
diff --git a/crates/ide-completion/src/render.rs b/crates/ide-completion/src/render.rs
index e88b36c52d..6d1a5a0bc5 100644
--- a/crates/ide-completion/src/render.rs
+++ b/crates/ide-completion/src/render.rs
@@ -312,7 +312,7 @@ pub(crate) fn render_expr(
         None => ctx.source_range(),
     };
 
-    let mut item = CompletionItem::new(CompletionItemKind::Expression, source_range, label);
+    let mut item = CompletionItem::new(CompletionItemKind::Expression, source_range, label.clone());
 
     let snippet = format!(
         "{}$0",
diff --git a/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/crates/ide-diagnostics/src/handlers/mutability_errors.rs
index 6602aca2fa..34a0038295 100644
--- a/crates/ide-diagnostics/src/handlers/mutability_errors.rs
+++ b/crates/ide-diagnostics/src/handlers/mutability_errors.rs
@@ -7,11 +7,7 @@ use crate::{fix, Diagnostic, DiagnosticCode, DiagnosticsContext};
 // Diagnostic: need-mut
 //
 // This diagnostic is triggered on mutating an immutable variable.
-pub(crate) fn need_mut(ctx: &DiagnosticsContext<'_>, d: &hir::NeedMut) -> Option<Diagnostic> {
-    if d.span.file_id.macro_file().is_some() {
-        // FIXME: Our infra can't handle allow from within macro expansions rn
-        return None;
-    }
+pub(crate) fn need_mut(ctx: &DiagnosticsContext<'_>, d: &hir::NeedMut) -> Diagnostic {
     let fixes = (|| {
         if d.local.is_ref(ctx.sema.db) {
             // There is no simple way to add `mut` to `ref x` and `ref mut x`
@@ -33,19 +29,17 @@ pub(crate) fn need_mut(ctx: &DiagnosticsContext<'_>, d: &hir::NeedMut) -> Option
             use_range,
         )])
     })();
-    Some(
-        Diagnostic::new_with_syntax_node_ptr(
-            ctx,
-            // FIXME: `E0384` is not the only error that this diagnostic handles
-            DiagnosticCode::RustcHardError("E0384"),
-            format!(
-                "cannot mutate immutable variable `{}`",
-                d.local.name(ctx.sema.db).display(ctx.sema.db)
-            ),
-            d.span,
-        )
-        .with_fixes(fixes),
+    Diagnostic::new_with_syntax_node_ptr(
+        ctx,
+        // FIXME: `E0384` is not the only error that this diagnostic handles
+        DiagnosticCode::RustcHardError("E0384"),
+        format!(
+            "cannot mutate immutable variable `{}`",
+            d.local.name(ctx.sema.db).display(ctx.sema.db)
+        ),
+        d.span,
     )
+    .with_fixes(fixes)
 }
 
 // Diagnostic: unused-mut
diff --git a/crates/ide-diagnostics/src/handlers/remove_trailing_return.rs b/crates/ide-diagnostics/src/handlers/remove_trailing_return.rs
index d831878044..7a040e46e3 100644
--- a/crates/ide-diagnostics/src/handlers/remove_trailing_return.rs
+++ b/crates/ide-diagnostics/src/handlers/remove_trailing_return.rs
@@ -12,12 +12,7 @@ use crate::{adjusted_display_range, fix, Diagnostic, DiagnosticCode, Diagnostics
 pub(crate) fn remove_trailing_return(
     ctx: &DiagnosticsContext<'_>,
     d: &RemoveTrailingReturn,
-) -> Option<Diagnostic> {
-    if d.return_expr.file_id.macro_file().is_some() {
-        // FIXME: Our infra can't handle allow from within macro expansions rn
-        return None;
-    }
-
+) -> Diagnostic {
     let display_range = adjusted_display_range(ctx, d.return_expr, &|return_expr| {
         return_expr
             .syntax()
@@ -25,14 +20,12 @@ pub(crate) fn remove_trailing_return(
             .and_then(ast::ExprStmt::cast)
             .map(|stmt| stmt.syntax().text_range())
     });
-    Some(
-        Diagnostic::new(
-            DiagnosticCode::Clippy("needless_return"),
-            "replace return <expr>; with <expr>",
-            display_range,
-        )
-        .with_fixes(fixes(ctx, d)),
+    Diagnostic::new(
+        DiagnosticCode::Clippy("needless_return"),
+        "replace return <expr>; with <expr>",
+        display_range,
     )
+    .with_fixes(fixes(ctx, d))
 }
 
 fn fixes(ctx: &DiagnosticsContext<'_>, d: &RemoveTrailingReturn) -> Option<Vec<Assist>> {
diff --git a/crates/ide-diagnostics/src/handlers/remove_unnecessary_else.rs b/crates/ide-diagnostics/src/handlers/remove_unnecessary_else.rs
index 448df1ca16..f68e698238 100644
--- a/crates/ide-diagnostics/src/handlers/remove_unnecessary_else.rs
+++ b/crates/ide-diagnostics/src/handlers/remove_unnecessary_else.rs
@@ -21,30 +21,23 @@ use crate::{
 pub(crate) fn remove_unnecessary_else(
     ctx: &DiagnosticsContext<'_>,
     d: &RemoveUnnecessaryElse,
-) -> Option<Diagnostic> {
-    if d.if_expr.file_id.macro_file().is_some() {
-        // FIXME: Our infra can't handle allow from within macro expansions rn
-        return None;
-    }
-
+) -> Diagnostic {
     let display_range = adjusted_display_range(ctx, d.if_expr, &|if_expr| {
         if_expr.else_token().as_ref().map(SyntaxToken::text_range)
     });
-    Some(
-        Diagnostic::new(
-            DiagnosticCode::Ra("remove-unnecessary-else", Severity::WeakWarning),
-            "remove unnecessary else block",
-            display_range,
-        )
-        .experimental()
-        .with_fixes(fixes(ctx, d)),
+    Diagnostic::new(
+        DiagnosticCode::Ra("remove-unnecessary-else", Severity::WeakWarning),
+        "remove unnecessary else block",
+        display_range,
     )
+    .experimental()
+    .with_fixes(fixes(ctx, d))
 }
 
 fn fixes(ctx: &DiagnosticsContext<'_>, d: &RemoveUnnecessaryElse) -> Option<Vec<Assist>> {
     let root = ctx.sema.db.parse_or_expand(d.if_expr.file_id);
     let if_expr = d.if_expr.value.to_node(&root);
-    let if_expr = ctx.sema.original_ast_node(if_expr)?;
+    let if_expr = ctx.sema.original_ast_node(if_expr.clone())?;
 
     let mut indent = IndentLevel::from_node(if_expr.syntax());
     let has_parent_if_expr = if_expr.syntax().parent().and_then(ast::IfExpr::cast).is_some();
diff --git a/crates/ide-diagnostics/src/handlers/unused_variables.rs b/crates/ide-diagnostics/src/handlers/unused_variables.rs
index cd251faab9..a9e1d07d7c 100644
--- a/crates/ide-diagnostics/src/handlers/unused_variables.rs
+++ b/crates/ide-diagnostics/src/handlers/unused_variables.rs
@@ -14,24 +14,18 @@ use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
 pub(crate) fn unused_variables(
     ctx: &DiagnosticsContext<'_>,
     d: &hir::UnusedVariable,
-) -> Option<Diagnostic> {
+) -> Diagnostic {
     let ast = d.local.primary_source(ctx.sema.db).syntax_ptr();
-    if ast.file_id.macro_file().is_some() {
-        // FIXME: Our infra can't handle allow from within macro expansions rn
-        return None;
-    }
     let diagnostic_range = ctx.sema.diagnostics_display_range(ast);
     let var_name = d.local.primary_source(ctx.sema.db).syntax().to_string();
-    Some(
-        Diagnostic::new_with_syntax_node_ptr(
-            ctx,
-            DiagnosticCode::RustcLint("unused_variables"),
-            "unused variable",
-            ast,
-        )
-        .with_fixes(fixes(&var_name, diagnostic_range, ast.file_id.is_macro()))
-        .experimental(),
+    Diagnostic::new_with_syntax_node_ptr(
+        ctx,
+        DiagnosticCode::RustcLint("unused_variables"),
+        "unused variable",
+        ast,
     )
+    .with_fixes(fixes(&var_name, diagnostic_range, ast.file_id.is_macro()))
+    .experimental()
 }
 
 fn fixes(var_name: &String, diagnostic_range: FileRange, is_in_marco: bool) -> Option<Vec<Assist>> {
@@ -53,7 +47,7 @@ fn fixes(var_name: &String, diagnostic_range: FileRange, is_in_marco: bool) -> O
 
 #[cfg(test)]
 mod tests {
-    use crate::tests::{check_diagnostics, check_fix};
+    use crate::tests::{check_diagnostics, check_fix, check_no_fix};
 
     #[test]
     fn unused_variables_simple() {
@@ -199,7 +193,7 @@ fn main() {
 
     #[test]
     fn no_fix_for_marco() {
-        check_diagnostics(
+        check_no_fix(
             r#"
 macro_rules! my_macro {
     () => {
@@ -208,7 +202,7 @@ macro_rules! my_macro {
 }
 
 fn main() {
-    my_macro!();
+    $0my_macro!();
 }
 "#,
         );
diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
index dea802204a..0df6f0e037 100644
--- a/crates/ide-diagnostics/src/lib.rs
+++ b/crates/ide-diagnostics/src/lib.rs
@@ -330,6 +330,7 @@ pub fn diagnostics(
     }
 
     for diag in diags {
+        #[rustfmt::skip]
         let d = match diag {
             AnyDiagnostic::ExpectedFunction(d) => handlers::expected_function::expected_function(&ctx, &d),
             AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) {
@@ -360,10 +361,7 @@ pub fn diagnostics(
             AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d),
             AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d),
             AnyDiagnostic::MovedOutOfRef(d) => handlers::moved_out_of_ref::moved_out_of_ref(&ctx, &d),
-            AnyDiagnostic::NeedMut(d) => match handlers::mutability_errors::need_mut(&ctx, &d) {
-                Some(it) => it,
-                None => continue,
-            },
+            AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d),
             AnyDiagnostic::NonExhaustiveLet(d) => handlers::non_exhaustive_let::non_exhaustive_let(&ctx, &d),
             AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d),
             AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d),
@@ -388,20 +386,11 @@ pub fn diagnostics(
             AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d),
             AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d, config.proc_macros_enabled, config.proc_attr_macros_enabled),
             AnyDiagnostic::UnusedMut(d) => handlers::mutability_errors::unused_mut(&ctx, &d),
-            AnyDiagnostic::UnusedVariable(d) => match handlers::unused_variables::unused_variables(&ctx, &d) {
-                Some(it) => it,
-                None => continue,
-            },
+            AnyDiagnostic::UnusedVariable(d) => handlers::unused_variables::unused_variables(&ctx, &d),
             AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d),
             AnyDiagnostic::MismatchedTupleStructPatArgCount(d) => handlers::mismatched_arg_count::mismatched_tuple_struct_pat_arg_count(&ctx, &d),
-            AnyDiagnostic::RemoveTrailingReturn(d) => match handlers::remove_trailing_return::remove_trailing_return(&ctx, &d) {
-                Some(it) => it,
-                None => continue,
-            },
-            AnyDiagnostic::RemoveUnnecessaryElse(d) => match handlers::remove_unnecessary_else::remove_unnecessary_else(&ctx, &d) {
-                Some(it) => it,
-                None => continue,
-            },
+            AnyDiagnostic::RemoveTrailingReturn(d) => handlers::remove_trailing_return::remove_trailing_return(&ctx, &d),
+            AnyDiagnostic::RemoveUnnecessaryElse(d) => handlers::remove_unnecessary_else::remove_unnecessary_else(&ctx, &d),
         };
         res.push(d)
     }
@@ -410,9 +399,9 @@ pub fn diagnostics(
         .iter_mut()
         .filter_map(|it| {
             Some((
-                it.main_node.map(|ptr| {
-                    ptr.map(|node| node.to_node(&ctx.sema.parse_or_expand(ptr.file_id)))
-                })?,
+                it.main_node
+                    .map(|ptr| ptr.map(|node| node.to_node(&ctx.sema.parse_or_expand(ptr.file_id))))
+                    .clone()?,
                 it,
             ))
         })
diff --git a/crates/ide/src/typing.rs b/crates/ide/src/typing.rs
index e87fc89fea..d3eee0e02e 100644
--- a/crates/ide/src/typing.rs
+++ b/crates/ide/src/typing.rs
@@ -175,9 +175,21 @@ fn on_opening_bracket_typed(
             }
         }
 
-        // If it's a statement in a block, we don't know how many statements should be included
-        if ast::ExprStmt::can_cast(expr.syntax().parent()?.kind()) {
-            return None;
+        if let Some(parent) = expr.syntax().parent().and_then(ast::Expr::cast) {
+            let mut node = expr.syntax().clone();
+            let all_prev_sib_attr = loop {
+                match node.prev_sibling() {
+                    Some(sib) if sib.kind().is_trivia() || sib.kind() == SyntaxKind::ATTR => {
+                        node = sib
+                    }
+                    Some(_) => break false,
+                    None => break true,
+                };
+            };
+
+            if all_prev_sib_attr {
+                expr = parent;
+            }
         }
 
         // Insert the closing bracket right after the expression.
@@ -824,6 +836,21 @@ fn f() {
         0 => {()},
         1 => (),
     }
+}
+            "#,
+        );
+        type_char(
+            '{',
+            r#"
+fn main() {
+    #[allow(unreachable_code)]
+    $0g();
+}
+            "#,
+            r#"
+fn main() {
+    #[allow(unreachable_code)]
+    {g()};
 }
             "#,
         );


Test patch:
diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs
index 2a46becbfd..d699067b5a 100644
--- a/crates/hir-ty/src/tests.rs
+++ b/crates/hir-ty/src/tests.rs
@@ -298,7 +298,7 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String {
             if let Some(syntax_ptr) = body_source_map.self_param_syntax() {
                 let root = db.parse_or_expand(syntax_ptr.file_id);
                 let node = syntax_ptr.map(|ptr| ptr.to_node(&root).syntax().clone());
-                types.push((node, ty));
+                types.push((node.clone(), ty));
             }
         }
 
diff --git a/crates/rust-analyzer/src/cli/rustc_tests.rs b/crates/rust-analyzer/src/cli/rustc_tests.rs
index eeec13a14b..84f2e60087 100644
--- a/crates/rust-analyzer/src/cli/rustc_tests.rs
+++ b/crates/rust-analyzer/src/cli/rustc_tests.rs
@@ -76,7 +76,7 @@ impl Tester {
         );
 
         let workspace = ProjectWorkspace::DetachedFiles {
-            files: vec![tmp_file],
+            files: vec![tmp_file.clone()],
             sysroot,
             rustc_cfg: vec![],
             toolchain: None,