From e864519fbcaef5141e8fda7fdfac1a2c8d363cd2 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 7 Jul 2024 08:11:32 +0700 Subject: [PATCH 1/2] Add test for manual_unwrap_or in issue 13018 --- tests/ui/manual_unwrap_or.fixed | 9 +++++++++ tests/ui/manual_unwrap_or.rs | 13 +++++++++++++ tests/ui/manual_unwrap_or.stderr | 12 +++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index 74afa00e12f..2899c3518de 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -234,4 +234,13 @@ fn implicit_deref_ref() { }; } +mod issue_13018 { + use std::collections::HashMap; + + type RefName = i32; + pub fn get(index: &HashMap>, id: usize) -> &[RefName] { + index.get(&id).unwrap_or(&[]) + } +} + fn main() {} diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index 2d01b8ceaaa..e2c04b01ed9 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -284,4 +284,17 @@ fn implicit_deref_ref() { }; } +mod issue_13018 { + use std::collections::HashMap; + + type RefName = i32; + pub fn get(index: &HashMap>, id: usize) -> &[RefName] { + if let Some(names) = index.get(&id) { + names + } else { + &[] + } + } +} + fn main() {} diff --git a/tests/ui/manual_unwrap_or.stderr b/tests/ui/manual_unwrap_or.stderr index c93a8952a08..28b59df161d 100644 --- a/tests/ui/manual_unwrap_or.stderr +++ b/tests/ui/manual_unwrap_or.stderr @@ -172,5 +172,15 @@ LL | | None => 0, LL | | }; | |_________^ help: replace with: `some_macro!().unwrap_or(0)` -error: aborting due to 16 previous errors +error: this pattern reimplements `Option::unwrap_or` + --> tests/ui/manual_unwrap_or.rs:292:9 + | +LL | / if let Some(names) = index.get(&id) { +LL | | names +LL | | } else { +LL | | &[] +LL | | } + | |_________^ help: replace with: `index.get(&id).unwrap_or(&[])` + +error: aborting due to 17 previous errors From c46c1f6da6725e0aa33e7738e9e0eb2f58b46eb6 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 7 Jul 2024 20:41:55 +0700 Subject: [PATCH 2/2] Fix 13018: self should be T with `Option::unwrap_or(self, T) -> T`. --- clippy_lints/src/matches/manual_unwrap_or.rs | 11 +++++++++++ tests/ui/manual_unwrap_or.fixed | 9 ++++++++- tests/ui/manual_unwrap_or.rs | 11 +++++++---- tests/ui/manual_unwrap_or.stderr | 12 +----------- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/matches/manual_unwrap_or.rs b/clippy_lints/src/matches/manual_unwrap_or.rs index 0940fc3219b..85a08f81c2f 100644 --- a/clippy_lints/src/matches/manual_unwrap_or.rs +++ b/clippy_lints/src/matches/manual_unwrap_or.rs @@ -35,6 +35,17 @@ pub(super) fn check_if_let<'tcx>( else_expr: &'tcx Expr<'_>, ) { let ty = cx.typeck_results().expr_ty(let_expr); + let then_ty = cx.typeck_results().expr_ty(then_expr); + // The signature is `fn unwrap_or(self: Option, default: T) -> T`. + // When `expr_adjustments(then_expr).is_empty()`, `T` should equate to `default`'s type. + // Otherwise, type error will occur. + if cx.typeck_results().expr_adjustments(then_expr).is_empty() + && let rustc_middle::ty::Adt(_did, args) = ty.kind() + && let Some(some_ty) = args.first().and_then(|arg| arg.as_type()) + && some_ty != then_ty + { + return; + } check_and_lint(cx, expr, let_pat, let_expr, then_expr, peel_blocks(else_expr), ty); } diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index 2899c3518de..07e4bdd483a 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -239,7 +239,14 @@ mod issue_13018 { type RefName = i32; pub fn get(index: &HashMap>, id: usize) -> &[RefName] { - index.get(&id).unwrap_or(&[]) + if let Some(names) = index.get(&id) { names } else { &[] } + } + + pub fn get_match(index: &HashMap>, id: usize) -> &[RefName] { + match index.get(&id) { + Some(names) => names, + None => &[], + } } } diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index e2c04b01ed9..cdbe51a4121 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -289,10 +289,13 @@ mod issue_13018 { type RefName = i32; pub fn get(index: &HashMap>, id: usize) -> &[RefName] { - if let Some(names) = index.get(&id) { - names - } else { - &[] + if let Some(names) = index.get(&id) { names } else { &[] } + } + + pub fn get_match(index: &HashMap>, id: usize) -> &[RefName] { + match index.get(&id) { + Some(names) => names, + None => &[], } } } diff --git a/tests/ui/manual_unwrap_or.stderr b/tests/ui/manual_unwrap_or.stderr index 28b59df161d..c93a8952a08 100644 --- a/tests/ui/manual_unwrap_or.stderr +++ b/tests/ui/manual_unwrap_or.stderr @@ -172,15 +172,5 @@ LL | | None => 0, LL | | }; | |_________^ help: replace with: `some_macro!().unwrap_or(0)` -error: this pattern reimplements `Option::unwrap_or` - --> tests/ui/manual_unwrap_or.rs:292:9 - | -LL | / if let Some(names) = index.get(&id) { -LL | | names -LL | | } else { -LL | | &[] -LL | | } - | |_________^ help: replace with: `index.get(&id).unwrap_or(&[])` - -error: aborting due to 17 previous errors +error: aborting due to 16 previous errors