From fdc944d62e38406f2076fef130da6b682ba18582 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 30 Apr 2026 21:14:54 +0200 Subject: [PATCH 1/3] misc improvements - use `ctx.db()` - move `add_mutable_reference_to_let_stmt` closer to `add_reference_to_let_stmt` --- .../src/handlers/type_mismatch.rs | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index f1b597b44ff8..f984c4d3df78 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -51,10 +51,10 @@ pub(crate) fn type_mismatch( format!( "expected {}, found {}", d.expected - .display(ctx.sema.db, ctx.display_target) + .display(ctx.db(), ctx.display_target) .with_closure_style(ClosureStyle::ClosureWithId), d.actual - .display(ctx.sema.db, ctx.display_target) + .display(ctx.db(), ctx.display_target) .with_closure_style(ClosureStyle::ClosureWithId), ), display_range, @@ -89,7 +89,7 @@ fn add_reference( let (_, mutability) = d.expected.as_reference()?; let actual_with_ref = d.actual.add_reference(ctx.db(), mutability); - if !actual_with_ref.could_coerce_to(ctx.sema.db, &d.expected) { + if !actual_with_ref.could_coerce_to(ctx.db(), &d.expected) { return None; } @@ -107,7 +107,7 @@ fn add_missing_ok_or_some( expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id); + let root = ctx.db().parse_or_expand(expr_ptr.file_id); let expr = expr_ptr.value.to_node(&root); let hir::FileRange { file_id, range: expr_range } = ctx.sema.original_range_opt(expr.syntax())?; @@ -126,14 +126,13 @@ fn add_missing_ok_or_some( let variant_name = if Some(expected_enum) == core_result { "Ok" } else { "Some" }; - let wrapped_actual_ty = - expected_adt.ty(ctx.sema.db).instantiate(std::iter::once(d.actual.clone())); + let wrapped_actual_ty = expected_adt.ty(ctx.db()).instantiate([d.actual.clone()]); - if !d.expected.could_unify_with(ctx.sema.db, &wrapped_actual_ty) { + if !d.expected.could_unify_with(ctx.db(), &wrapped_actual_ty) { return None; } - let file_id = file_id.file_id(ctx.sema.db); + let file_id = file_id.file_id(ctx.db()); if d.actual.is_unit() { if let Expr::BlockExpr(block) = &expr { @@ -202,7 +201,7 @@ fn remove_unnecessary_wrapper( expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let db = ctx.sema.db; + let db = ctx.db(); let root = db.parse_or_expand(expr_ptr.file_id); let expr = expr_ptr.value.to_node(&root); // FIXME: support inside MacroCall? @@ -233,7 +232,7 @@ fn remove_unnecessary_wrapper( let inner_arg = call_expr.arg_list()?.args().next()?; let file_id = expr_ptr.file_id.original_file(db); - let mut builder = SourceChangeBuilder::new(file_id.file_id(ctx.sema.db)); + let mut builder = SourceChangeBuilder::new(file_id.file_id(ctx.db())); let editor; match inner_arg { // We're returning `()` @@ -267,7 +266,7 @@ fn remove_unnecessary_wrapper( } } - builder.add_file_edits(file_id.file_id(ctx.sema.db), editor); + builder.add_file_edits(file_id.file_id(ctx.db()), editor); let name = format!("Remove unnecessary {}() wrapper", variant.name(db).as_str()); acc.push(fix( "remove_unnecessary_wrapper", @@ -284,7 +283,7 @@ fn remove_semicolon( expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id); + let root = ctx.db().parse_or_expand(expr_ptr.file_id); let expr = expr_ptr.value.to_node(&root); if !d.actual.is_unit() { return None; @@ -294,14 +293,14 @@ fn remove_semicolon( let expr_before_semi = block.statements().last().and_then(|s| ExprStmt::cast(s.syntax().clone()))?; let type_before_semi = ctx.sema.type_of_expr(&expr_before_semi.expr()?)?.original(); - if !type_before_semi.could_coerce_to(ctx.sema.db, &d.expected) { + if !type_before_semi.could_coerce_to(ctx.db(), &d.expected) { return None; } let semicolon_range = expr_before_semi.semicolon_token()?.text_range(); let edit = TextEdit::delete(semicolon_range); let source_change = SourceChange::from_text_edit( - expr_ptr.file_id.original_file(ctx.sema.db).file_id(ctx.sema.db), + expr_ptr.file_id.original_file(ctx.db()).file_id(ctx.db()), edit, ); @@ -315,21 +314,21 @@ fn str_ref_to_owned( expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let expected = d.expected.display(ctx.sema.db, ctx.display_target); + let expected = d.expected.display(ctx.db(), ctx.display_target); // FIXME do this properly let is_applicable = d.actual.strip_reference().is_str() && expected.to_string() == "String"; if !is_applicable { return None; } - let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id); + let root = ctx.db().parse_or_expand(expr_ptr.file_id); let expr = expr_ptr.value.to_node(&root); let hir::FileRange { file_id, range } = ctx.sema.original_range_opt(expr.syntax())?; let to_owned = ".to_owned()".to_owned(); let edit = TextEdit::insert(range.end(), to_owned); - let source_change = SourceChange::from_text_edit(file_id.file_id(ctx.sema.db), edit); + let source_change = SourceChange::from_text_edit(file_id.file_id(ctx.db()), edit); acc.push(fix("str_ref_to_owned", "Add .to_owned() here", source_change, range)); Some(()) @@ -483,6 +482,22 @@ fn main() { ); } + #[test] + fn add_mutable_reference_to_let_stmt() { + check_fix( + r#" +fn main() { + let _test: &mut i32 = $0123; +} + "#, + r#" +fn main() { + let _test: &mut i32 = &mut 123; +} + "#, + ); + } + #[test] fn add_reference_to_macro_call() { check_fix( @@ -511,22 +526,6 @@ fn main() { ); } - #[test] - fn add_mutable_reference_to_let_stmt() { - check_fix( - r#" -fn main() { - let _test: &mut i32 = $0123; -} - "#, - r#" -fn main() { - let _test: &mut i32 = &mut 123; -} - "#, - ); - } - #[test] fn const_generic_type_mismatch() { check_diagnostics( From f78d39362f5a13a07679f45eba6ae61f6ca6ed4f Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Thu, 9 Apr 2026 16:04:29 +0200 Subject: [PATCH 2/3] fix(assists/add_reference_here): _modify_ the reference type when dealing with &T->&mut T ..instead of adding another layer of reference, i.e. &T -> &mut &T Co-authored-by: Ana Hobden --- .../src/handlers/type_mismatch.rs | 255 +++++++++++++++++- 1 file changed, 249 insertions(+), 6 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index f984c4d3df78..afd6130bb3e9 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -8,7 +8,7 @@ use ide_db::{ use syntax::{ AstNode, AstPtr, TextSize, ast::{ - self, BlockExpr, Expr, ExprStmt, HasArgList, + self, BlockExpr, Expr, ExprStmt, HasArgList, RefExpr, edit::{AstNodeEdit, IndentLevel}, }, }; @@ -69,7 +69,7 @@ fn fixes(ctx: &DiagnosticsContext<'_, '_>, d: &hir::TypeMismatch<'_>) -> Option< if let Some(expr_ptr) = d.expr_or_pat.value.cast::() { let expr_ptr = &InFile { file_id: d.expr_or_pat.file_id, value: expr_ptr }; - add_reference(ctx, d, expr_ptr, &mut fixes); + add_or_fix_reference(ctx, d, expr_ptr, &mut fixes); add_missing_ok_or_some(ctx, d, expr_ptr, &mut fixes); remove_unnecessary_wrapper(ctx, d, expr_ptr, &mut fixes); remove_semicolon(ctx, d, expr_ptr, &mut fixes); @@ -79,7 +79,7 @@ fn fixes(ctx: &DiagnosticsContext<'_, '_>, d: &hir::TypeMismatch<'_>) -> Option< if fixes.is_empty() { None } else { Some(fixes) } } -fn add_reference( +fn add_or_fix_reference( ctx: &DiagnosticsContext<'_, '_>, d: &hir::TypeMismatch<'_>, expr_ptr: &InFile>, @@ -87,13 +87,44 @@ fn add_reference( ) -> Option<()> { let range = ctx.sema.diagnostics_display_range((*expr_ptr).map(|it| it.into())); - let (_, mutability) = d.expected.as_reference()?; - let actual_with_ref = d.actual.add_reference(ctx.db(), mutability); + let (expected_with_ref_removed, expected_mutability) = d.expected.as_reference()?; + + if let Some((actual_with_ref_removed, hir::Mutability::Shared)) = d.actual.as_reference() + && expected_mutability == hir::Mutability::Mut + && actual_with_ref_removed.could_coerce_to(ctx.db(), &expected_with_ref_removed) + { + // The actual type is `&T`, and the expected type is `&mut T`, (or `U` that `T` can be coerced to). + // It's likely that, instead of adding a reference, we should just change the mutability of + // the existing one. + + let expr = expr_ptr.to_node(ctx.db()); + // If the node comes from a macro expansion, then we shouldn't assist, + // as the suggestion would overwrite the macro _definition_ position + let expr = ctx.sema.original_ast_node(expr)?; + let expr_without_ref = RefExpr::cast(expr.syntax().clone())?.expr()?; + + let file_id = expr_ptr.file_id.original_file(ctx.db()); + let mut builder = SourceChangeBuilder::new(file_id.file_id(ctx.db())); + let editor = builder.make_editor(expr.syntax()); + let make = editor.make(); + let new_expr = make.expr_ref(expr_without_ref, true); + builder.replace_ast(expr, new_expr); + let source_change = builder.finish(); + acc.push(fix( + "make_reference_mutable", + "Make reference mutable", + source_change, + range.range, + )); + return Some(()); + } + + let actual_with_ref = d.actual.add_reference(ctx.db(), expected_mutability); if !actual_with_ref.could_coerce_to(ctx.db(), &d.expected) { return None; } - let ampersands = format!("&{}", mutability.as_keyword_for_ref()); + let ampersands = format!("&{}", expected_mutability.as_keyword_for_ref()); let edit = TextEdit::insert(range.range.start(), ampersands); let source_change = SourceChange::from_text_edit(range.file_id, edit); @@ -390,6 +421,96 @@ fn test(_arg: &mut i32) {} ); } + #[test] + fn fix_reference_to_int() { + check_fix( + r#" +fn main() { + test($0&123); +} +fn test(_arg: &mut i32) {} + "#, + r#" +fn main() { + test(&mut 123); +} +fn test(_arg: &mut i32) {} + "#, + ); + } + + #[test] + fn add_reference_to_parenthesized_int() { + check_fix( + r#" +fn main() { + test(($0123)); +} +fn test(_arg: &i32) {} + "#, + r#" +fn main() { + test((&123)); +} +fn test(_arg: &i32) {} + "#, + ); + } + + #[test] + fn add_mutable_reference_to_parenthesized_int() { + check_fix( + r#" +fn main() { + test(($0123)); +} +fn test(_arg: &mut i32) {} + "#, + r#" +fn main() { + test((&mut 123)); +} +fn test(_arg: &mut i32) {} + "#, + ); + } + + #[test] + fn fix_reference_to_parenthesized_int_paren_inside_ref() { + check_fix( + r#" +fn main() { + test(&$0(123)); +} +fn test(_arg: &mut i32) {} + "#, + r#" +fn main() { + test(&mut (123)); +} +fn test(_arg: &mut i32) {} + "#, + ); + } + + #[test] + fn fix_reference_to_parenthesized_int_ref_inside_paren() { + check_fix( + r#" +fn main() { + test(($0&123)); +} +fn test(_arg: &mut i32) {} + "#, + r#" +fn main() { + test((&mut 123)); +} +fn test(_arg: &mut i32) {} + "#, + ); + } + #[test] fn add_reference_to_array() { check_fix( @@ -409,6 +530,19 @@ fn test(_arg: &[i32]) {} ); } + #[test] + fn fix_reference_to_array() { + check_no_fix( + r#" +//- minicore: coerce_unsized +fn main() { + test($0&[1, 2, 3]); +} +fn test(_arg: &mut [i32]) {} + "#, + ); + } + #[test] fn add_reference_with_autoderef() { check_fix( @@ -442,6 +576,49 @@ fn test(_arg: &Bar) {} ); } + #[test] + // FIXME: this should suggest making the reference mutable instead: `&Foo -> &mut Foo`. + // Currently it doesn't, as the logic for that assist strips away references, and thus checks + // whether `Foo` can be coerced to `Bar` (which it can't), instead of checking `&mut Foo` to + // `&mut Bar` (which it can) + fn fix_reference_with_autoderef() { + check_fix( + r#" +//- minicore: coerce_unsized, deref_mut +struct Foo; +struct Bar; +impl core::ops::Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { loop {} } +} +impl core::ops::DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Self::Target { loop {} } +} + +fn main() { + test($0&Foo); +} +fn test(_arg: &mut Bar) {} + "#, + r#" +struct Foo; +struct Bar; +impl core::ops::Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { loop {} } +} +impl core::ops::DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Self::Target { loop {} } +} + +fn main() { + test(&mut &Foo); +} +fn test(_arg: &mut Bar) {} + "#, + ); + } + #[test] fn add_reference_to_method_call() { check_fix( @@ -498,6 +675,22 @@ fn main() { ); } + #[test] + fn fix_reference_to_let_stmt() { + check_fix( + r#" +fn main() { + let _test: &mut i32 = $0&123; +} + "#, + r#" +fn main() { + let _test: &mut i32 = &mut 123; +} + "#, + ); + } + #[test] fn add_reference_to_macro_call() { check_fix( @@ -526,6 +719,56 @@ fn main() { ); } + #[test] + fn fix_reference_to_macro_call() { + check_fix( + r#" +macro_rules! thousand { + () => { + 1000_u64 + }; +} + +fn test(_foo: &mut u64) {} +fn main() { + test($0&thousand!()); +} + "#, + r#" +macro_rules! thousand { + () => { + 1000_u64 + }; +} + +fn test(_foo: &mut u64) {} +fn main() { + test(&mut thousand!()); +} + "#, + ); + } + + #[test] + // If the immutable reference comes from a macro expansion, + // we can't do anything to change it to a mutable one. + fn dont_fix_reference_inside_macro_call() { + check_no_fix( + r#" +macro_rules! thousand { + () => { + &1000_u64 + }; +} + +fn test(_foo: &mut u64) {} +fn main() { + test($0thousand!()); +} + "#, + ); + } + #[test] fn const_generic_type_mismatch() { check_diagnostics( From 05bc639bbf9530d417f3f387caa9b31ba0aa000b Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Mon, 11 May 2026 20:27:57 +0200 Subject: [PATCH 3/3] switch to text-based edit --- crates/ide-diagnostics/src/handlers/type_mismatch.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/crates/ide-diagnostics/src/handlers/type_mismatch.rs index afd6130bb3e9..f3cf823efe68 100644 --- a/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -103,13 +103,9 @@ fn add_or_fix_reference( let expr = ctx.sema.original_ast_node(expr)?; let expr_without_ref = RefExpr::cast(expr.syntax().clone())?.expr()?; - let file_id = expr_ptr.file_id.original_file(ctx.db()); - let mut builder = SourceChangeBuilder::new(file_id.file_id(ctx.db())); - let editor = builder.make_editor(expr.syntax()); - let make = editor.make(); - let new_expr = make.expr_ref(expr_without_ref, true); - builder.replace_ast(expr, new_expr); - let source_change = builder.finish(); + let pos = expr_without_ref.syntax().text_range().start(); + let edit = TextEdit::insert(pos, expected_mutability.as_keyword_for_ref().to_owned()); + let source_change = SourceChange::from_text_edit(range.file_id, edit); acc.push(fix( "make_reference_mutable", "Make reference mutable",