From 1376c5a617be5c25655d0d7cb63e3beaa5a6e026 Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Wed, 17 Apr 2024 14:20:39 +0200 Subject: Merging upstream version 1.70.0+dfsg1. Signed-off-by: Daniel Baumann --- .../src/handlers/break_outside_of_loop.rs | 23 +- .../src/handlers/expected_function.rs | 39 ++ .../src/handlers/incoherent_impl.rs | 77 +++ .../ide-diagnostics/src/handlers/incorrect_case.rs | 2 +- .../ide-diagnostics/src/handlers/missing_fields.rs | 4 +- .../src/handlers/missing_match_arms.rs | 5 +- .../ide-diagnostics/src/handlers/missing_unsafe.rs | 381 +++++++++++- .../src/handlers/mutability_errors.rs | 649 +++++++++++++++++++++ .../ide-diagnostics/src/handlers/no_such_field.rs | 2 +- .../src/handlers/private_assoc_item.rs | 38 ++ .../ide-diagnostics/src/handlers/private_field.rs | 20 + .../replace_filter_map_next_with_find_map.rs | 15 +- .../ide-diagnostics/src/handlers/type_mismatch.rs | 86 ++- .../src/handlers/unresolved_field.rs | 148 +++++ .../src/handlers/unresolved_method.rs | 131 +++++ .../src/handlers/unresolved_module.rs | 2 +- .../crates/ide-diagnostics/src/lib.rs | 12 +- 17 files changed, 1585 insertions(+), 49 deletions(-) create mode 100644 src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/expected_function.rs create mode 100644 src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incoherent_impl.rs create mode 100644 src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/mutability_errors.rs create mode 100644 src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_field.rs create mode 100644 src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs (limited to 'src/tools/rust-analyzer/crates/ide-diagnostics') diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs index 10e637979..114face2d 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs @@ -7,10 +7,15 @@ pub(crate) fn break_outside_of_loop( ctx: &DiagnosticsContext<'_>, d: &hir::BreakOutsideOfLoop, ) -> Diagnostic { - let construct = if d.is_break { "break" } else { "continue" }; + let message = if d.bad_value_break { + "can't break with a value in this position".to_owned() + } else { + let construct = if d.is_break { "break" } else { "continue" }; + format!("{construct} outside of loop") + }; Diagnostic::new( "break-outside-of-loop", - format!("{construct} outside of loop"), + message, ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, ) } @@ -132,6 +137,20 @@ fn foo() { //^^^^^^^^^^^ error: continue outside of loop } } +"#, + ); + } + + #[test] + fn value_break_in_for_loop() { + check_diagnostics( + r#" +fn test() { + for _ in [()] { + break 3; + // ^^^^^^^ error: can't break with a value in this position + } +} "#, ); } diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/expected_function.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/expected_function.rs new file mode 100644 index 000000000..d2f27664d --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/expected_function.rs @@ -0,0 +1,39 @@ +use hir::HirDisplay; + +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: expected-function +// +// This diagnostic is triggered if a call is made on something that is not callable. +pub(crate) fn expected_function( + ctx: &DiagnosticsContext<'_>, + d: &hir::ExpectedFunction, +) -> Diagnostic { + Diagnostic::new( + "expected-function", + format!("expected function, found {}", d.found.display(ctx.sema.db)), + ctx.sema.diagnostics_display_range(d.call.clone().map(|it| it.into())).range, + ) + .experimental() +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn smoke_test() { + check_diagnostics( + r#" +fn foo() { + let x = 3; + x(); + // ^^^ error: expected function, found i32 + ""(); + // ^^^^ error: expected function, found &str + foo(); +} +"#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incoherent_impl.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incoherent_impl.rs new file mode 100644 index 000000000..72af9ebfc --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incoherent_impl.rs @@ -0,0 +1,77 @@ +use hir::InFile; + +use crate::{Diagnostic, DiagnosticsContext, Severity}; + +// Diagnostic: incoherent-impl +// +// This diagnostic is triggered if the targe type of an impl is from a foreign crate. +pub(crate) fn incoherent_impl(ctx: &DiagnosticsContext<'_>, d: &hir::IncoherentImpl) -> Diagnostic { + Diagnostic::new( + "incoherent-impl", + format!("cannot define inherent `impl` for foreign type"), + ctx.sema.diagnostics_display_range(InFile::new(d.file_id, d.impl_.clone().into())).range, + ) + .severity(Severity::Error) +} + +#[cfg(test)] +mod change_case { + use crate::tests::check_diagnostics; + + #[test] + fn primitive() { + check_diagnostics( + r#" + impl bool {} +//^^^^^^^^^^^^ error: cannot define inherent `impl` for foreign type +"#, + ); + } + + #[test] + fn primitive_rustc_allow_incoherent_impl() { + check_diagnostics( + r#" +impl bool { + #[rustc_allow_incoherent_impl] + fn falsch(self) -> Self { false } +} +"#, + ); + } + + #[test] + fn rustc_allow_incoherent_impl() { + check_diagnostics( + r#" +//- /lib.rs crate:foo +#[rustc_has_incoherent_inherent_impls] +pub struct S; +//- /main.rs crate:main deps:foo +impl foo::S { + #[rustc_allow_incoherent_impl] + fn func(self) {} +} +"#, + ); + check_diagnostics( + r#" +//- /lib.rs crate:foo +pub struct S; +//- /main.rs crate:main deps:foo + impl foo::S { #[rustc_allow_incoherent_impl] fn func(self) {} } +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: cannot define inherent `impl` for foreign type +"#, + ); + check_diagnostics( + r#" +//- /lib.rs crate:foo +#[rustc_has_incoherent_inherent_impls] +pub struct S; +//- /main.rs crate:main deps:foo + impl foo::S { fn func(self) {} } +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: cannot define inherent `impl` for foreign type +"#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incorrect_case.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incorrect_case.rs index 6a78c08d4..db88bf7b9 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incorrect_case.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/incorrect_case.rs @@ -1,4 +1,4 @@ -use hir::{db::AstDatabase, InFile}; +use hir::{db::ExpandDatabase, InFile}; use ide_db::{assists::Assist, defs::NameClass}; use syntax::AstNode; diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_fields.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_fields.rs index 43af4d4f1..5c4327ff9 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_fields.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_fields.rs @@ -1,11 +1,11 @@ use either::Either; use hir::{ - db::{AstDatabase, HirDatabase}, + db::{ExpandDatabase, HirDatabase}, known, AssocItem, HirDisplay, InFile, Type, }; use ide_db::{ assists::Assist, famous_defs::FamousDefs, imports::import_assets::item_for_path_search, - source_change::SourceChange, use_trivial_contructor::use_trivial_constructor, FxHashMap, + source_change::SourceChange, use_trivial_constructor::use_trivial_constructor, FxHashMap, }; use stdx::format_to; use syntax::{ diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_match_arms.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_match_arms.rs index c24430ce6..ac4463331 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_match_arms.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_match_arms.rs @@ -1,5 +1,3 @@ -use hir::InFile; - use crate::{Diagnostic, DiagnosticsContext}; // Diagnostic: missing-match-arm @@ -12,7 +10,7 @@ pub(crate) fn missing_match_arms( Diagnostic::new( "missing-match-arm", format!("missing match arm: {}", d.uncovered_patterns), - ctx.sema.diagnostics_display_range(InFile::new(d.file, d.match_expr.clone().into())).range, + ctx.sema.diagnostics_display_range(d.scrutinee_expr.clone().map(Into::into)).range, ) } @@ -1038,7 +1036,6 @@ fn main() { #[test] fn reference_patterns_in_fields() { cov_mark::check_count!(validate_match_bailed_out, 2); - check_diagnostics( r#" fn main() { diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index ea1ea5a21..eb32db250 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -1,4 +1,10 @@ -use crate::{Diagnostic, DiagnosticsContext}; +use hir::db::ExpandDatabase; +use ide_db::{assists::Assist, source_change::SourceChange}; +use syntax::{ast, SyntaxNode}; +use syntax::{match_ast, AstNode}; +use text_edit::TextEdit; + +use crate::{fix, Diagnostic, DiagnosticsContext}; // Diagnostic: missing-unsafe // @@ -9,11 +15,83 @@ pub(crate) fn missing_unsafe(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsaf "this operation is unsafe and requires an unsafe function or block", ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, ) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Option> { + // The fixit will not work correctly for macro expansions, so we don't offer it in that case. + if d.expr.file_id.is_macro() { + return None; + } + + let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; + let expr = d.expr.value.to_node(&root); + + let node_to_add_unsafe_block = pick_best_node_to_add_unsafe_block(&expr)?; + + let replacement = format!("unsafe {{ {} }}", node_to_add_unsafe_block.text()); + let edit = TextEdit::replace(node_to_add_unsafe_block.text_range(), replacement); + let source_change = + SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + Some(vec![fix("add_unsafe", "Add unsafe block", source_change, expr.syntax().text_range())]) +} + +// Pick the first ancestor expression of the unsafe `expr` that is not a +// receiver of a method call, a field access, the left-hand side of an +// assignment, or a reference. As all of those cases would incur a forced move +// if wrapped which might not be wanted. That is: +// - `unsafe_expr.foo` -> `unsafe { unsafe_expr.foo }` +// - `unsafe_expr.foo.bar` -> `unsafe { unsafe_expr.foo.bar }` +// - `unsafe_expr.foo()` -> `unsafe { unsafe_expr.foo() }` +// - `unsafe_expr.foo.bar()` -> `unsafe { unsafe_expr.foo.bar() }` +// - `unsafe_expr += 1` -> `unsafe { unsafe_expr += 1 }` +// - `&unsafe_expr` -> `unsafe { &unsafe_expr }` +// - `&&unsafe_expr` -> `unsafe { &&unsafe_expr }` +fn pick_best_node_to_add_unsafe_block(unsafe_expr: &ast::Expr) -> Option { + // The `unsafe_expr` might be: + // - `ast::CallExpr`: call an unsafe function + // - `ast::MethodCallExpr`: call an unsafe method + // - `ast::PrefixExpr`: dereference a raw pointer + // - `ast::PathExpr`: access a static mut variable + for (node, parent) in + unsafe_expr.syntax().ancestors().zip(unsafe_expr.syntax().ancestors().skip(1)) + { + match_ast! { + match parent { + // If the `parent` is a `MethodCallExpr`, that means the `node` + // is the receiver of the method call, because only the receiver + // can be a direct child of a method call. The method name + // itself is not an expression but a `NameRef`, and an argument + // is a direct child of an `ArgList`. + ast::MethodCallExpr(_) => continue, + ast::FieldExpr(_) => continue, + ast::RefExpr(_) => continue, + ast::BinExpr(it) => { + // Check if the `node` is the left-hand side of an + // assignment, if so, we don't want to wrap it in an unsafe + // block, e.g. `unsafe_expr += 1` + let is_left_hand_side_of_assignment = { + if let Some(ast::BinaryOp::Assignment { .. }) = it.op_kind() { + it.lhs().map(|lhs| lhs.syntax().text_range().contains_range(node.text_range())).unwrap_or(false) + } else { + false + } + }; + if !is_left_hand_side_of_assignment { + return Some(node); + } + }, + _ => { return Some(node); } + + } + } + } + None } #[cfg(test)] mod tests { - use crate::tests::check_diagnostics; + use crate::tests::{check_diagnostics, check_fix, check_no_fix}; #[test] fn missing_unsafe_diagnostic_with_raw_ptr() { @@ -23,7 +101,7 @@ fn main() { let x = &5 as *const usize; unsafe { let y = *x; } let z = *x; -} //^^ error: this operation is unsafe and requires an unsafe function or block +} //^^💡 error: this operation is unsafe and requires an unsafe function or block "#, ) } @@ -48,9 +126,9 @@ unsafe fn unsafe_fn() { fn main() { unsafe_fn(); - //^^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block HasUnsafe.unsafe_fn(); - //^^^^^^^^^^^^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block unsafe { unsafe_fn(); HasUnsafe.unsafe_fn(); @@ -72,7 +150,7 @@ static mut STATIC_MUT: Ty = Ty { a: 0 }; fn main() { let x = STATIC_MUT.a; - //^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block unsafe { let x = STATIC_MUT.a; } @@ -94,9 +172,298 @@ extern "rust-intrinsic" { fn main() { let _ = bitreverse(12); let _ = floorf32(12.0); - //^^^^^^^^^^^^^^ error: this operation is unsafe and requires an unsafe function or block + //^^^^^^^^^^^^^^💡 error: this operation is unsafe and requires an unsafe function or block } "#, ); } + + #[test] + fn add_unsafe_block_when_dereferencing_a_raw_pointer() { + check_fix( + r#" +fn main() { + let x = &5 as *const usize; + let z = *x$0; +} +"#, + r#" +fn main() { + let x = &5 as *const usize; + let z = unsafe { *x }; +} +"#, + ); + } + + #[test] + fn add_unsafe_block_when_calling_unsafe_function() { + check_fix( + r#" +unsafe fn func() { + let x = &5 as *const usize; + let z = *x; +} +fn main() { + func$0(); +} +"#, + r#" +unsafe fn func() { + let x = &5 as *const usize; + let z = *x; +} +fn main() { + unsafe { func() }; +} +"#, + ) + } + + #[test] + fn add_unsafe_block_when_calling_unsafe_method() { + check_fix( + r#" +struct S(usize); +impl S { + unsafe fn func(&self) { + let x = &self.0 as *const usize; + let z = *x; + } +} +fn main() { + let s = S(5); + s.func$0(); +} +"#, + r#" +struct S(usize); +impl S { + unsafe fn func(&self) { + let x = &self.0 as *const usize; + let z = *x; + } +} +fn main() { + let s = S(5); + unsafe { s.func() }; +} +"#, + ) + } + + #[test] + fn add_unsafe_block_when_accessing_mutable_static() { + check_fix( + r#" +struct Ty { + a: u8, +} + +static mut STATIC_MUT: Ty = Ty { a: 0 }; + +fn main() { + let x = STATIC_MUT$0.a; +} +"#, + r#" +struct Ty { + a: u8, +} + +static mut STATIC_MUT: Ty = Ty { a: 0 }; + +fn main() { + let x = unsafe { STATIC_MUT.a }; +} +"#, + ) + } + + #[test] + fn add_unsafe_block_when_calling_unsafe_intrinsic() { + check_fix( + r#" +extern "rust-intrinsic" { + pub fn floorf32(x: f32) -> f32; +} + +fn main() { + let _ = floorf32$0(12.0); +} +"#, + r#" +extern "rust-intrinsic" { + pub fn floorf32(x: f32) -> f32; +} + +fn main() { + let _ = unsafe { floorf32(12.0) }; +} +"#, + ) + } + + #[test] + fn unsafe_expr_as_a_receiver_of_a_method_call() { + check_fix( + r#" +unsafe fn foo() -> String { + "string".to_string() +} + +fn main() { + foo$0().len(); +} +"#, + r#" +unsafe fn foo() -> String { + "string".to_string() +} + +fn main() { + unsafe { foo().len() }; +} +"#, + ) + } + + #[test] + fn unsafe_expr_as_an_argument_of_a_method_call() { + check_fix( + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let mut v = vec![]; + v.push(STATIC_MUT$0); +} +"#, + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let mut v = vec![]; + v.push(unsafe { STATIC_MUT }); +} +"#, + ) + } + + #[test] + fn unsafe_expr_as_left_hand_side_of_assignment() { + check_fix( + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + STATIC_MUT$0 = 1; +} +"#, + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + unsafe { STATIC_MUT = 1 }; +} +"#, + ) + } + + #[test] + fn unsafe_expr_as_right_hand_side_of_assignment() { + check_fix( + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x; + x = STATIC_MUT$0; +} +"#, + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x; + x = unsafe { STATIC_MUT }; +} +"#, + ) + } + + #[test] + fn unsafe_expr_in_binary_plus() { + check_fix( + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x = STATIC_MUT$0 + 1; +} +"#, + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x = unsafe { STATIC_MUT } + 1; +} +"#, + ) + } + + #[test] + fn ref_to_unsafe_expr() { + check_fix( + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x = &STATIC_MUT$0; +} +"#, + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x = unsafe { &STATIC_MUT }; +} +"#, + ) + } + + #[test] + fn ref_ref_to_unsafe_expr() { + check_fix( + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x = &&STATIC_MUT$0; +} +"#, + r#" +static mut STATIC_MUT: u8 = 0; + +fn main() { + let x = unsafe { &&STATIC_MUT }; +} +"#, + ) + } + + #[test] + fn unsafe_expr_in_macro_call() { + check_no_fix( + r#" +unsafe fn foo() -> u8 { + 0 +} + +fn main() { + let x = format!("foo: {}", foo$0()); +} + "#, + ) + } } diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/mutability_errors.rs new file mode 100644 index 000000000..96470265d --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/mutability_errors.rs @@ -0,0 +1,649 @@ +use ide_db::source_change::SourceChange; +use syntax::{AstNode, SyntaxKind, SyntaxNode, SyntaxToken, T}; +use text_edit::TextEdit; + +use crate::{fix, Diagnostic, DiagnosticsContext, Severity}; + +// Diagnostic: need-mut +// +// This diagnostic is triggered on mutating an immutable variable. +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` + return None; + } + let file_id = d.span.file_id.file_id()?; + let mut edit_builder = TextEdit::builder(); + let use_range = d.span.value.text_range(); + for source in d.local.sources(ctx.sema.db) { + let Some(ast) = source.name() else { continue }; + edit_builder.insert(ast.syntax().text_range().start(), "mut ".to_string()); + } + let edit = edit_builder.finish(); + Some(vec![fix( + "add_mut", + "Change it to be mutable", + SourceChange::from_text_edit(file_id, edit), + use_range, + )]) + })(); + Diagnostic::new( + "need-mut", + format!("cannot mutate immutable variable `{}`", d.local.name(ctx.sema.db)), + ctx.sema.diagnostics_display_range(d.span.clone()).range, + ) + .with_fixes(fixes) +} + +// Diagnostic: unused-mut +// +// This diagnostic is triggered when a mutable variable isn't actually mutated. +pub(crate) fn unused_mut(ctx: &DiagnosticsContext<'_>, d: &hir::UnusedMut) -> Diagnostic { + let ast = d.local.primary_source(ctx.sema.db).syntax_ptr(); + let fixes = (|| { + let file_id = ast.file_id.file_id()?; + let mut edit_builder = TextEdit::builder(); + let use_range = ast.value.text_range(); + for source in d.local.sources(ctx.sema.db) { + let ast = source.syntax(); + let Some(mut_token) = token(ast, T![mut]) else { continue }; + edit_builder.delete(mut_token.text_range()); + if let Some(token) = mut_token.next_token() { + if token.kind() == SyntaxKind::WHITESPACE { + edit_builder.delete(token.text_range()); + } + } + } + let edit = edit_builder.finish(); + Some(vec![fix( + "remove_mut", + "Remove unnecessary `mut`", + SourceChange::from_text_edit(file_id, edit), + use_range, + )]) + })(); + let ast = d.local.primary_source(ctx.sema.db).syntax_ptr(); + Diagnostic::new( + "unused-mut", + "variable does not need to be mutable", + ctx.sema.diagnostics_display_range(ast).range, + ) + .severity(Severity::WeakWarning) + .experimental() // Not supporting `#[allow(unused_mut)]` leads to false positive. + .with_fixes(fixes) +} + +pub(super) fn token(parent: &SyntaxNode, kind: SyntaxKind) -> Option { + parent.children_with_tokens().filter_map(|it| it.into_token()).find(|it| it.kind() == kind) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_diagnostics, check_fix}; + + #[test] + fn unused_mut_simple() { + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let mut x = 2; + //^^^^^ 💡 weak: variable does not need to be mutable + f(x); +} +"#, + ); + } + + #[test] + fn no_false_positive_simple() { + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + f(x); +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let mut x = 2; + x = 5; + f(x); +} +"#, + ); + } + + #[test] + fn multiple_errors_for_single_variable() { + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + x = 10; + //^^^^^^ 💡 error: cannot mutate immutable variable `x` + x = 5; + //^^^^^ 💡 error: cannot mutate immutable variable `x` + &mut x; + //^^^^^^ 💡 error: cannot mutate immutable variable `x` + f(x); +} +"#, + ); + } + + #[test] + fn unused_mut_fix() { + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let mu$0t x = 2; + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + f(x); +} +"#, + ); + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let ((mu$0t x, _) | (_, mut x)) = (2, 3); + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let ((x, _) | (_, x)) = (2, 3); + f(x); +} +"#, + ); + } + + #[test] + fn need_mut_fix() { + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + x$0 = 5; + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let mut x = 2; + x = 5; + f(x); +} +"#, + ); + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let ((x, _) | (_, x)) = (2, 3); + x =$0 4; + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let ((mut x, _) | (_, mut x)) = (2, 3); + x = 4; + f(x); +} +"#, + ); + + check_fix( + r#" +struct Foo(i32); + +impl Foo { + fn foo(self) { + self = Fo$0o(5); + } +} +"#, + r#" +struct Foo(i32); + +impl Foo { + fn foo(mut self) { + self = Foo(5); + } +} +"#, + ); + } + + #[test] + fn need_mut_fix_not_applicable_on_ref() { + check_diagnostics( + r#" +fn main() { + let ref x = 2; + x = &5; + //^^^^^^ error: cannot mutate immutable variable `x` +} +"#, + ); + check_diagnostics( + r#" +fn main() { + let ref mut x = 2; + x = &mut 5; + //^^^^^^^^^^ error: cannot mutate immutable variable `x` +} +"#, + ); + } + + #[test] + fn field_mutate() { + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let mut x = (2, 7); + //^^^^^ 💡 weak: variable does not need to be mutable + f(x.1); +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let mut x = (2, 7); + x.0 = 5; + f(x.1); +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let x = (2, 7); + x.0 = 5; + //^^^^^^^ 💡 error: cannot mutate immutable variable `x` + f(x.1); +} +"#, + ); + } + + #[test] + fn mutable_reference() { + check_diagnostics( + r#" +fn main() { + let mut x = &mut 2; + //^^^^^ 💡 weak: variable does not need to be mutable + *x = 5; +} +"#, + ); + check_diagnostics( + r#" +fn main() { + let x = 2; + &mut x; + //^^^^^^ 💡 error: cannot mutate immutable variable `x` +} +"#, + ); + check_diagnostics( + r#" +fn main() { + let x_own = 2; + let ref mut x_ref = x_own; + //^^^^^^^^^^^^^ 💡 error: cannot mutate immutable variable `x_own` +} +"#, + ); + check_diagnostics( + r#" +struct Foo; +impl Foo { + fn method(&mut self, x: i32) {} +} +fn main() { + let x = Foo; + x.method(2); + //^ 💡 error: cannot mutate immutable variable `x` +} +"#, + ); + } + + #[test] + fn regression_14310() { + check_diagnostics( + r#" + fn clone(mut i: &!) -> ! { + //^^^^^ 💡 weak: variable does not need to be mutable + *i + } + "#, + ); + } + + #[test] + fn match_bindings() { + check_diagnostics( + r#" +fn main() { + match (2, 3) { + (x, mut y) => { + //^^^^^ 💡 weak: variable does not need to be mutable + x = 7; + //^^^^^ 💡 error: cannot mutate immutable variable `x` + } + } +} +"#, + ); + } + + #[test] + fn mutation_in_dead_code() { + // This one is interesting. Dead code is not represented at all in the MIR, so + // there would be no mutablility error for locals in dead code. Rustc tries to + // not emit `unused_mut` in this case, but since it works without `mut`, and + // special casing it is not trivial, we emit it. + check_diagnostics( + r#" +fn main() { + return; + let mut x = 2; + //^^^^^ 💡 weak: variable does not need to be mutable + &mut x; +} +"#, + ); + check_diagnostics( + r#" +fn main() { + loop {} + let mut x = 2; + //^^^^^ 💡 weak: variable does not need to be mutable + &mut x; +} +"#, + ); + check_diagnostics( + r#" +enum X {} +fn g() -> X { + loop {} +} +fn f() -> ! { + loop {} +} +fn main(b: bool) { + if b { + f(); + } else { + g(); + } + let mut x = 2; + //^^^^^ 💡 weak: variable does not need to be mutable + &mut x; +} +"#, + ); + check_diagnostics( + r#" +fn main(b: bool) { + if b { + loop {} + } else { + return; + } + let mut x = 2; + //^^^^^ 💡 weak: variable does not need to be mutable + &mut x; +} +"#, + ); + } + + #[test] + fn initialization_is_not_mutation() { + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let mut x; + //^^^^^ 💡 weak: variable does not need to be mutable + x = 5; + f(x); +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main(b: bool) { + let mut x; + //^^^^^ 💡 weak: variable does not need to be mutable + if b { + x = 1; + } else { + x = 3; + } + f(x); +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main(b: bool) { + let x; + if b { + x = 1; + } + x = 3; + //^^^^^ 💡 error: cannot mutate immutable variable `x` + f(x); +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let x; + loop { + x = 1; + //^^^^^ 💡 error: cannot mutate immutable variable `x` + f(x); + } +} +"#, + ); + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + loop { + let mut x = 1; + //^^^^^ 💡 weak: variable does not need to be mutable + f(x); + if let mut y = 2 { + //^^^^^ 💡 weak: variable does not need to be mutable + f(y); + } + match 3 { + mut z => f(z), + //^^^^^ 💡 weak: variable does not need to be mutable + } + } +} +"#, + ); + } + + #[test] + fn initialization_is_not_mutation_in_loop() { + check_diagnostics( + r#" +fn main() { + let a; + loop { + let c @ ( + mut b, + //^^^^^ 💡 weak: variable does not need to be mutable + mut d + //^^^^^ 💡 weak: variable does not need to be mutable + ); + a = 1; + //^^^^^ 💡 error: cannot mutate immutable variable `a` + b = 1; + c = (2, 3); + d = 3; + } +} +"#, + ); + } + + #[test] + fn function_arguments_are_initialized() { + check_diagnostics( + r#" +fn f(mut x: i32) { + //^^^^^ 💡 weak: variable does not need to be mutable +} +"#, + ); + check_diagnostics( + r#" +fn f(x: i32) { + x = 5; + //^^^^^ 💡 error: cannot mutate immutable variable `x` +} +"#, + ); + } + + #[test] + fn for_loop() { + check_diagnostics( + r#" +//- minicore: iterators +fn f(x: [(i32, u8); 10]) { + for (a, mut b) in x { + //^^^^^ 💡 weak: variable does not need to be mutable + a = 2; + //^^^^^ 💡 error: cannot mutate immutable variable `a` + } +} +"#, + ); + } + + #[test] + fn overloaded_deref() { + // FIXME: check for false negative + check_diagnostics( + r#" +//- minicore: deref_mut +use core::ops::{Deref, DerefMut}; + +struct Foo; +impl Deref for Foo { + type Target = i32; + fn deref(&self) -> &i32 { + &5 + } +} +impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut i32 { + &mut 5 + } +} +fn f() { + let x = Foo; + let y = &*x; + let x = Foo; + let mut x = Foo; + let y: &mut i32 = &mut x; +} +"#, + ); + } + + #[test] + fn or_pattern() { + check_diagnostics( + r#" +//- minicore: option +fn f(_: i32) {} +fn main() { + let ((Some(mut x), None) | (_, Some(mut x))) = (None, Some(7)); + //^^^^^ 💡 weak: variable does not need to be mutable + f(x); +} +"#, + ); + } + + #[test] + fn or_pattern_no_terminator() { + check_diagnostics( + r#" +enum Foo { + A, B, C, D +} + +use Foo::*; + +fn f(inp: (Foo, Foo, Foo, Foo)) { + let ((A, B, _, x) | (B, C | D, x, _)) = inp else { + return; + }; + x = B; + //^^^^^ 💡 error: cannot mutate immutable variable `x` +} +"#, + ); + } + + #[test] + fn respect_allow_unused_mut() { + // FIXME: respect + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + #[allow(unused_mut)] + let mut x = 2; + //^^^^^ 💡 weak: variable does not need to be mutable + f(x); +} +"#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/no_such_field.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/no_such_field.rs index 8da04e628..24c521ed1 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/no_such_field.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/no_such_field.rs @@ -1,4 +1,4 @@ -use hir::{db::AstDatabase, HasSource, HirDisplay, Semantics}; +use hir::{db::ExpandDatabase, HasSource, HirDisplay, Semantics}; use ide_db::{base_db::FileId, source_change::SourceChange, RootDatabase}; use syntax::{ ast::{self, edit::IndentLevel, make}, diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_assoc_item.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_assoc_item.rs index 0b3121c76..67da5c7f2 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_assoc_item.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_assoc_item.rs @@ -115,6 +115,44 @@ mod module { fn main(s: module::Struct) { s.method(); } +"#, + ); + } + + #[test] + fn can_see_through_top_level_anonymous_const() { + // regression test for #14046. + check_diagnostics( + r#" +struct S; +mod m { + const _: () = { + impl crate::S { + pub(crate) fn method(self) {} + pub(crate) const A: usize = 42; + } + }; + mod inner { + const _: () = { + impl crate::S { + pub(crate) fn method2(self) {} + pub(crate) const B: usize = 42; + pub(super) fn private(self) {} + pub(super) const PRIVATE: usize = 42; + } + }; + } +} +fn main() { + S.method(); + S::A; + S.method2(); + S::B; + S.private(); + //^^^^^^^^^^^ error: function `private` is private + S::PRIVATE; + //^^^^^^^^^^ error: const `PRIVATE` is private +} "#, ); } diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_field.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_field.rs index e630ae368..be83ad6aa 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_field.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/private_field.rs @@ -62,6 +62,26 @@ mod module { fn main(s: module::Struct) { s.field; } +"#, + ); + } + + #[test] + fn block_module_madness() { + check_diagnostics( + r#" +fn main() { + let strukt = { + use crate as ForceParentBlockDefMap; + { + pub struct Struct { + field: (), + } + Struct { field: () } + } + }; + strukt.field; +} "#, ); } diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs index 9826e1c70..9b1c65983 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs @@ -1,4 +1,4 @@ -use hir::{db::AstDatabase, InFile}; +use hir::{db::ExpandDatabase, InFile}; use ide_db::source_change::SourceChange; use syntax::{ ast::{self, HasArgList}, @@ -55,7 +55,18 @@ fn fixes( #[cfg(test)] mod tests { - use crate::tests::{check_diagnostics, check_fix}; + use crate::{ + tests::{check_diagnostics_with_config, check_fix}, + DiagnosticsConfig, + }; + + #[track_caller] + pub(crate) fn check_diagnostics(ra_fixture: &str) { + let mut config = DiagnosticsConfig::test_sample(); + config.disabled.insert("inactive-code".to_string()); + config.disabled.insert("unresolved-method".to_string()); + check_diagnostics_with_config(config, ra_fixture) + } #[test] fn replace_filter_map_next_with_find_map2() { diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/type_mismatch.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/type_mismatch.rs index 2adae165e..4abc25a28 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/type_mismatch.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/type_mismatch.rs @@ -1,8 +1,9 @@ -use hir::{db::AstDatabase, HirDisplay, Type}; +use either::Either; +use hir::{db::ExpandDatabase, HirDisplay, InFile, Type}; use ide_db::{famous_defs::FamousDefs, source_change::SourceChange}; use syntax::{ ast::{self, BlockExpr, ExprStmt}, - AstNode, + AstNode, AstPtr, }; use text_edit::TextEdit; @@ -10,19 +11,23 @@ use crate::{adjusted_display_range, fix, Assist, Diagnostic, DiagnosticsContext} // Diagnostic: type-mismatch // -// This diagnostic is triggered when the type of an expression does not match +// This diagnostic is triggered when the type of an expression or pattern does not match // the expected type. pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Diagnostic { - let display_range = adjusted_display_range::( - ctx, - d.expr.clone().map(|it| it.into()), - &|block| { - let r_curly_range = block.stmt_list()?.r_curly_token()?.text_range(); - cov_mark::hit!(type_mismatch_on_block); - Some(r_curly_range) - }, - ); - + let display_range = match &d.expr_or_pat { + Either::Left(expr) => adjusted_display_range::( + ctx, + expr.clone().map(|it| it.into()), + &|block| { + let r_curly_range = block.stmt_list()?.r_curly_token()?.text_range(); + cov_mark::hit!(type_mismatch_on_block); + Some(r_curly_range) + }, + ), + Either::Right(pat) => { + ctx.sema.diagnostics_display_range(pat.clone().map(|it| it.into())).range + } + }; let mut diag = Diagnostic::new( "type-mismatch", format!( @@ -42,10 +47,15 @@ pub(crate) fn type_mismatch(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Option> { let mut fixes = Vec::new(); - add_reference(ctx, d, &mut fixes); - add_missing_ok_or_some(ctx, d, &mut fixes); - remove_semicolon(ctx, d, &mut fixes); - str_ref_to_owned(ctx, d, &mut fixes); + match &d.expr_or_pat { + Either::Left(expr_ptr) => { + add_reference(ctx, d, expr_ptr, &mut fixes); + add_missing_ok_or_some(ctx, d, expr_ptr, &mut fixes); + remove_semicolon(ctx, d, expr_ptr, &mut fixes); + str_ref_to_owned(ctx, d, expr_ptr, &mut fixes); + } + Either::Right(_pat_ptr) => {} + } if fixes.is_empty() { None @@ -57,9 +67,10 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch) -> Option, d: &hir::TypeMismatch, + expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let range = ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range; + let range = ctx.sema.diagnostics_display_range(expr_ptr.clone().map(|it| it.into())).range; let (_, mutability) = d.expected.as_reference()?; let actual_with_ref = Type::reference(&d.actual, mutability); @@ -71,7 +82,7 @@ fn add_reference( let edit = TextEdit::insert(range.start(), ampersands); let source_change = - SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit); acc.push(fix("add_reference_here", "Add reference here", source_change, range)); Some(()) } @@ -79,10 +90,11 @@ fn add_reference( fn add_missing_ok_or_some( ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch, + expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; - let expr = d.expr.value.to_node(&root); + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); let expr_range = expr.syntax().text_range(); let scope = ctx.sema.scope(expr.syntax())?; @@ -109,7 +121,7 @@ fn add_missing_ok_or_some( builder.insert(expr.syntax().text_range().start(), format!("{variant_name}(")); builder.insert(expr.syntax().text_range().end(), ")".to_string()); let source_change = - SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), builder.finish()); + SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), builder.finish()); let name = format!("Wrap in {variant_name}"); acc.push(fix("wrap_in_constructor", &name, source_change, expr_range)); Some(()) @@ -118,10 +130,11 @@ fn add_missing_ok_or_some( fn remove_semicolon( ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch, + expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { - let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; - let expr = d.expr.value.to_node(&root); + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); if !d.actual.is_unit() { return None; } @@ -136,7 +149,7 @@ fn remove_semicolon( let edit = TextEdit::delete(semicolon_range); let source_change = - SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit); acc.push(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon_range)); Some(()) @@ -145,24 +158,26 @@ fn remove_semicolon( fn str_ref_to_owned( ctx: &DiagnosticsContext<'_>, d: &hir::TypeMismatch, + expr_ptr: &InFile>, acc: &mut Vec, ) -> Option<()> { let expected = d.expected.display(ctx.sema.db); let actual = d.actual.display(ctx.sema.db); + // FIXME do this properly if expected.to_string() != "String" || actual.to_string() != "&str" { return None; } - let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; - let expr = d.expr.value.to_node(&root); + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); let expr_range = expr.syntax().text_range(); let to_owned = format!(".to_owned()"); let edit = TextEdit::insert(expr.syntax().text_range().end(), to_owned); let source_change = - SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + SourceChange::from_text_edit(expr_ptr.file_id.original_file(ctx.sema.db), edit); acc.push(fix("str_ref_to_owned", "Add .to_owned() here", source_change, expr_range)); Some(()) @@ -592,6 +607,21 @@ fn f() -> i32 { let _ = x + y; } //^ error: expected i32, found () +"#, + ); + } + + #[test] + fn type_mismatch_pat_smoke_test() { + check_diagnostics( + r#" +fn f() { + let &() = &mut (); + match &() { + &9 => () + //^ error: expected (), found i32 + } +} "#, ); } diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_field.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_field.rs new file mode 100644 index 000000000..cefa74e52 --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_field.rs @@ -0,0 +1,148 @@ +use hir::{db::ExpandDatabase, HirDisplay, InFile}; +use ide_db::{ + assists::{Assist, AssistId, AssistKind}, + base_db::FileRange, + label::Label, + source_change::SourceChange, +}; +use syntax::{ast, AstNode, AstPtr}; +use text_edit::TextEdit; + +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unresolved-field +// +// This diagnostic is triggered if a field does not exist on a given type. +pub(crate) fn unresolved_field( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedField, +) -> Diagnostic { + let method_suffix = if d.method_with_same_name_exists { + ", but a method with a similar name exists" + } else { + "" + }; + Diagnostic::new( + "unresolved-field", + format!( + "no field `{}` on type `{}`{method_suffix}", + d.name, + d.receiver.display(ctx.sema.db) + ), + ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, + ) + .with_fixes(fixes(ctx, d)) + .experimental() +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedField) -> Option> { + if d.method_with_same_name_exists { + method_fix(ctx, &d.expr) + } else { + // FIXME: add quickfix + + None + } +} + +// FIXME: We should fill out the call here, mvoe the cursor and trigger signature help +fn method_fix( + ctx: &DiagnosticsContext<'_>, + expr_ptr: &InFile>, +) -> Option> { + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); + let FileRange { range, file_id } = ctx.sema.original_range_opt(expr.syntax())?; + Some(vec![Assist { + id: AssistId("expected-field-found-method-call-fix", AssistKind::QuickFix), + label: Label::new("Use parentheses to call the method".to_string()), + group: None, + target: range, + source_change: Some(SourceChange::from_text_edit( + file_id, + TextEdit::insert(range.end(), "()".to_owned()), + )), + trigger_signature_help: false, + }]) +} +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn smoke_test() { + check_diagnostics( + r#" +fn main() { + ().foo; + // ^^^^^^ error: no field `foo` on type `()` +} +"#, + ); + } + + #[test] + fn method_clash() { + check_diagnostics( + r#" +struct Foo; +impl Foo { + fn bar(&self) {} +} +fn foo() { + Foo.bar; + // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists +} +"#, + ); + } + + #[test] + fn method_trait_() { + check_diagnostics( + r#" +struct Foo; +trait Bar { + fn bar(&self) {} +} +impl Bar for Foo {} +fn foo() { + Foo.bar; + // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists +} +"#, + ); + } + + #[test] + fn method_trait_2() { + check_diagnostics( + r#" +struct Foo; +trait Bar { + fn bar(&self); +} +impl Bar for Foo { + fn bar(&self) {} +} +fn foo() { + Foo.bar; + // ^^^^^^^ 💡 error: no field `bar` on type `Foo`, but a method with a similar name exists +} +"#, + ); + } + + #[test] + fn no_diagnostic_on_unknown() { + check_diagnostics( + r#" +fn foo() { + x.foo; + (&x).foo; + (&((x,),),).foo; +} +"#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs new file mode 100644 index 000000000..f3ec6efa7 --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs @@ -0,0 +1,131 @@ +use hir::{db::ExpandDatabase, HirDisplay}; +use ide_db::{ + assists::{Assist, AssistId, AssistKind}, + base_db::FileRange, + label::Label, + source_change::SourceChange, +}; +use syntax::{ast, AstNode, TextRange}; +use text_edit::TextEdit; + +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unresolved-method +// +// This diagnostic is triggered if a method does not exist on a given type. +pub(crate) fn unresolved_method( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedMethodCall, +) -> Diagnostic { + let field_suffix = if d.field_with_same_name.is_some() { + ", but a field with a similar name exists" + } else { + "" + }; + Diagnostic::new( + "unresolved-method", + format!( + "no method `{}` on type `{}`{field_suffix}", + d.name, + d.receiver.display(ctx.sema.db) + ), + ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, + ) + .with_fixes(fixes(ctx, d)) + .experimental() +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option> { + if let Some(ty) = &d.field_with_same_name { + field_fix(ctx, d, ty) + } else { + // FIXME: add quickfix + None + } +} + +fn field_fix( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnresolvedMethodCall, + ty: &hir::Type, +) -> Option> { + if !ty.impls_fnonce(ctx.sema.db) { + return None; + } + let expr_ptr = &d.expr; + let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?; + let expr = expr_ptr.value.to_node(&root); + let (file_id, range) = match expr { + ast::Expr::MethodCallExpr(mcall) => { + let FileRange { range, file_id } = + ctx.sema.original_range_opt(mcall.receiver()?.syntax())?; + let FileRange { range: range2, file_id: file_id2 } = + ctx.sema.original_range_opt(mcall.name_ref()?.syntax())?; + if file_id != file_id2 { + return None; + } + (file_id, TextRange::new(range.start(), range2.end())) + } + _ => return None, + }; + Some(vec![Assist { + id: AssistId("expected-method-found-field-fix", AssistKind::QuickFix), + label: Label::new("Use parentheses to call the value of the field".to_string()), + group: None, + target: range, + source_change: Some(SourceChange::from_iter([ + (file_id, TextEdit::insert(range.start(), "(".to_owned())), + (file_id, TextEdit::insert(range.end(), ")".to_owned())), + ])), + trigger_signature_help: false, + }]) +} + +#[cfg(test)] +mod tests { + use crate::tests::{check_diagnostics, check_fix}; + + #[test] + fn smoke_test() { + check_diagnostics( + r#" +fn main() { + ().foo(); + // ^^^^^^^^ error: no method `foo` on type `()` +} +"#, + ); + } + + #[test] + fn field() { + check_diagnostics( + r#" +struct Foo { bar: i32 } +fn foo() { + Foo { bar: i32 }.bar(); + // ^^^^^^^^^^^^^^^^^^^^^^ error: no method `bar` on type `Foo`, but a field with a similar name exists +} +"#, + ); + } + + #[test] + fn callable_field() { + check_fix( + r#" +//- minicore: fn +struct Foo { bar: fn() } +fn foo() { + Foo { bar: foo }.b$0ar(); +} +"#, + r#" +struct Foo { bar: fn() } +fn foo() { + (Foo { bar: foo }.bar)(); +} +"#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_module.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_module.rs index 91395f1d8..94614f11c 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_module.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_module.rs @@ -1,4 +1,4 @@ -use hir::db::AstDatabase; +use hir::db::ExpandDatabase; use ide_db::{assists::Assist, base_db::AnchoredPathBuf, source_change::FileSystemEdit}; use itertools::Itertools; use syntax::AstNode; diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs index 64ba08ac8..71f136b8c 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs @@ -27,7 +27,9 @@ mod handlers { pub(crate) mod break_outside_of_loop; + pub(crate) mod expected_function; pub(crate) mod inactive_code; + pub(crate) mod incoherent_impl; pub(crate) mod incorrect_case; pub(crate) mod invalid_derive_target; pub(crate) mod macro_error; @@ -36,6 +38,7 @@ mod handlers { pub(crate) mod missing_fields; pub(crate) mod missing_match_arms; pub(crate) mod missing_unsafe; + pub(crate) mod mutability_errors; pub(crate) mod no_such_field; pub(crate) mod private_assoc_item; pub(crate) mod private_field; @@ -43,6 +46,8 @@ mod handlers { pub(crate) mod type_mismatch; pub(crate) mod unimplemented_builtin_macro; pub(crate) mod unresolved_extern_crate; + pub(crate) mod unresolved_field; + pub(crate) mod unresolved_method; pub(crate) mod unresolved_import; pub(crate) mod unresolved_macro_call; pub(crate) mod unresolved_module; @@ -248,7 +253,9 @@ pub fn diagnostics( #[rustfmt::skip] let d = match diag { AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d), + AnyDiagnostic::ExpectedFunction(d) => handlers::expected_function::expected_function(&ctx, &d), AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d), + AnyDiagnostic::IncoherentImpl(d) => handlers::incoherent_impl::incoherent_impl(&ctx, &d), AnyDiagnostic::MacroError(d) => handlers::macro_error::macro_error(&ctx, &d), AnyDiagnostic::MalformedDerive(d) => handlers::malformed_derive::malformed_derive(&ctx, &d), AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d), @@ -267,7 +274,10 @@ 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::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d), - + AnyDiagnostic::UnresolvedField(d) => handlers::unresolved_field::unresolved_field(&ctx, &d), + AnyDiagnostic::UnresolvedMethodCall(d) => handlers::unresolved_method::unresolved_method(&ctx, &d), + AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d), + AnyDiagnostic::UnusedMut(d) => handlers::mutability_errors::unused_mut(&ctx, &d), AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { Some(it) => it, None => continue, -- cgit v1.2.3