Fix manual_ok_err suggests wrongly with references (#15053)

Closes rust-lang/rust-clippy#15051

changelog: [`manual_ok_err`] fix wrong suggestions with references
This commit is contained in:
Jason Newcomb 2025-06-20 21:50:49 +00:00 committed by GitHub
commit 0601337a25
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 108 additions and 4 deletions

View file

@ -1,9 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{indent_of, reindent_multiline};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::option_arg_ty;
use clippy_utils::ty::{option_arg_ty, peel_mid_ty_refs_is_mutable};
use clippy_utils::{get_parent_expr, is_res_lang_ctor, path_res, peel_blocks, span_contains_comment};
use rustc_ast::BindingMode;
use rustc_ast::{BindingMode, Mutability};
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr};
use rustc_hir::def::{DefKind, Res};
@ -133,7 +133,21 @@ fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok
Applicability::MachineApplicable
};
let scrut = Sugg::hir_with_applicability(cx, scrutinee, "..", &mut app).maybe_paren();
let sugg = format!("{scrut}.{method}()");
let scrutinee_ty = cx.typeck_results().expr_ty(scrutinee);
let (_, n_ref, mutability) = peel_mid_ty_refs_is_mutable(scrutinee_ty);
let prefix = if n_ref > 0 {
if mutability == Mutability::Mut {
".as_mut()"
} else {
".as_ref()"
}
} else {
""
};
let sugg = format!("{scrut}{prefix}.{method}()");
// If the expression being expanded is the `if …` part of an `else if …`, it must be blockified.
let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr)
&& let ExprKind::If(_, _, Some(else_part)) = parent_expr.kind

View file

@ -103,3 +103,27 @@ fn issue14239() {
};
//~^^^^^ manual_ok_err
}
mod issue15051 {
struct Container {
field: Result<bool, ()>,
}
#[allow(clippy::needless_borrow)]
fn with_addr_of(x: &Container) -> Option<&bool> {
(&x.field).as_ref().ok()
}
fn from_fn(x: &Container) -> Option<&bool> {
let result_with_ref = || &x.field;
result_with_ref().as_ref().ok()
}
fn result_with_ref_mut(x: &mut Container) -> &mut Result<bool, ()> {
&mut x.field
}
fn from_fn_mut(x: &mut Container) -> Option<&mut bool> {
result_with_ref_mut(x).as_mut().ok()
}
}

View file

@ -141,3 +141,39 @@ fn issue14239() {
};
//~^^^^^ manual_ok_err
}
mod issue15051 {
struct Container {
field: Result<bool, ()>,
}
#[allow(clippy::needless_borrow)]
fn with_addr_of(x: &Container) -> Option<&bool> {
match &x.field {
//~^ manual_ok_err
Ok(panel) => Some(panel),
Err(_) => None,
}
}
fn from_fn(x: &Container) -> Option<&bool> {
let result_with_ref = || &x.field;
match result_with_ref() {
//~^ manual_ok_err
Ok(panel) => Some(panel),
Err(_) => None,
}
}
fn result_with_ref_mut(x: &mut Container) -> &mut Result<bool, ()> {
&mut x.field
}
fn from_fn_mut(x: &mut Container) -> Option<&mut bool> {
match result_with_ref_mut(x) {
//~^ manual_ok_err
Ok(panel) => Some(panel),
Err(_) => None,
}
}
}

View file

@ -111,5 +111,35 @@ LL + "1".parse::<u8>().ok()
LL ~ };
|
error: aborting due to 9 previous errors
error: manual implementation of `ok`
--> tests/ui/manual_ok_err.rs:152:9
|
LL | / match &x.field {
LL | |
LL | | Ok(panel) => Some(panel),
LL | | Err(_) => None,
LL | | }
| |_________^ help: replace with: `(&x.field).as_ref().ok()`
error: manual implementation of `ok`
--> tests/ui/manual_ok_err.rs:161:9
|
LL | / match result_with_ref() {
LL | |
LL | | Ok(panel) => Some(panel),
LL | | Err(_) => None,
LL | | }
| |_________^ help: replace with: `result_with_ref().as_ref().ok()`
error: manual implementation of `ok`
--> tests/ui/manual_ok_err.rs:173:9
|
LL | / match result_with_ref_mut(x) {
LL | |
LL | | Ok(panel) => Some(panel),
LL | | Err(_) => None,
LL | | }
| |_________^ help: replace with: `result_with_ref_mut(x).as_mut().ok()`
error: aborting due to 12 previous errors