From cd1c8532e99c4c594c6704be6d4274131bb71004 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 4 Jul 2023 00:29:14 +0200 Subject: [PATCH 1/2] [`unnecessary_literal_unwrap`]: lint `unwrap_unchecked` --- clippy_lints/src/methods/mod.rs | 3 + .../src/methods/unnecessary_literal_unwrap.rs | 16 ++++ tests/ui/unnecessary_literal_unwrap.fixed | 10 +++ tests/ui/unnecessary_literal_unwrap.rs | 10 +++ tests/ui/unnecessary_literal_unwrap.stderr | 74 ++++++++++++++++++- 5 files changed, 112 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c99f7c1fdd9..6e936e92cbd 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4000,6 +4000,9 @@ impl Methods { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests); }, + ("unwrap_unchecked", []) => { + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); + } ("unwrap_err", []) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests); diff --git a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs index ce93d7c3eb3..13895b67ed0 100644 --- a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs +++ b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs @@ -67,6 +67,22 @@ pub(super) fn check( (expr.span.with_hi(args[0].span.lo()), "panic!(".to_string()), (expr.span.with_lo(args[0].span.hi()), ")".to_string()), ]), + ("Some" | "Ok", "unwrap_unchecked", _) => { + let mut suggs = vec![ + (recv.span.with_hi(call_args[0].span.lo()), String::new()), + (expr.span.with_lo(call_args[0].span.hi()), String::new()), + ]; + // try to also remove the unsafe block if present + if let hir::Node::Block(block) = cx.tcx.hir().get_parent(expr.hir_id) + && let hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::UserProvided) = block.rules + { + suggs.extend([ + (block.span.shrink_to_lo().to(expr.span.shrink_to_lo()), String::new()), + (expr.span.shrink_to_hi().to(block.span.shrink_to_hi()), String::new()) + ]); + } + Some(suggs) + }, (_, _, Some(_)) => None, ("Ok", "unwrap_err", None) | ("Err", "unwrap", None) => Some(vec![ ( diff --git a/tests/ui/unnecessary_literal_unwrap.fixed b/tests/ui/unnecessary_literal_unwrap.fixed index 092bf78feab..0e7ba6bcbbb 100644 --- a/tests/ui/unnecessary_literal_unwrap.fixed +++ b/tests/ui/unnecessary_literal_unwrap.fixed @@ -78,6 +78,15 @@ fn unwrap_from_binding() { let _ = val.unwrap_or(""); } +fn unwrap_unchecked() { + let _ = 1; + let _ = unsafe { 1 + *(&1 as *const i32) }; // needs to keep the unsafe block + let _ = 1 + 1; + let _ = 1; + let _ = unsafe { 1 + *(&1 as *const i32) }; + let _ = 1 + 1; +} + fn main() { unwrap_option_some(); unwrap_option_none(); @@ -85,4 +94,5 @@ fn main() { unwrap_result_err(); unwrap_methods_option(); unwrap_methods_result(); + unwrap_unchecked(); } diff --git a/tests/ui/unnecessary_literal_unwrap.rs b/tests/ui/unnecessary_literal_unwrap.rs index b9df893d1d2..fda7fd36ff9 100644 --- a/tests/ui/unnecessary_literal_unwrap.rs +++ b/tests/ui/unnecessary_literal_unwrap.rs @@ -78,6 +78,15 @@ fn unwrap_from_binding() { let _ = val.unwrap_or(""); } +fn unwrap_unchecked() { + let _ = unsafe { Some(1).unwrap_unchecked() }; + let _ = unsafe { Some(1).unwrap_unchecked() + *(&1 as *const i32) }; // needs to keep the unsafe block + let _ = unsafe { Some(1).unwrap_unchecked() } + 1; + let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() }; + let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() + *(&1 as *const i32) }; + let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() } + 1; +} + fn main() { unwrap_option_some(); unwrap_option_none(); @@ -85,4 +94,5 @@ fn main() { unwrap_result_err(); unwrap_methods_option(); unwrap_methods_result(); + unwrap_unchecked(); } diff --git a/tests/ui/unnecessary_literal_unwrap.stderr b/tests/ui/unnecessary_literal_unwrap.stderr index 0c71ee05323..e5b93c3d9af 100644 --- a/tests/ui/unnecessary_literal_unwrap.stderr +++ b/tests/ui/unnecessary_literal_unwrap.stderr @@ -409,5 +409,77 @@ LL - Ok::<_, ()>(1).unwrap_or_else(|_| 2); LL + 1; | -error: aborting due to 36 previous errors +error: used `unwrap_unchecked()` on `Some` value + --> $DIR/unnecessary_literal_unwrap.rs:82:22 + | +LL | let _ = unsafe { Some(1).unwrap_unchecked() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Some` and `unwrap_unchecked()` + | +LL - let _ = unsafe { Some(1).unwrap_unchecked() }; +LL + let _ = 1; + | + +error: used `unwrap_unchecked()` on `Some` value + --> $DIR/unnecessary_literal_unwrap.rs:83:22 + | +LL | let _ = unsafe { Some(1).unwrap_unchecked() + *(&1 as *const i32) }; // needs to keep the unsafe block + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Some` and `unwrap_unchecked()` + | +LL - let _ = unsafe { Some(1).unwrap_unchecked() + *(&1 as *const i32) }; // needs to keep the unsafe block +LL + let _ = unsafe { 1 + *(&1 as *const i32) }; // needs to keep the unsafe block + | + +error: used `unwrap_unchecked()` on `Some` value + --> $DIR/unnecessary_literal_unwrap.rs:84:22 + | +LL | let _ = unsafe { Some(1).unwrap_unchecked() } + 1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Some` and `unwrap_unchecked()` + | +LL - let _ = unsafe { Some(1).unwrap_unchecked() } + 1; +LL + let _ = 1 + 1; + | + +error: used `unwrap_unchecked()` on `Ok` value + --> $DIR/unnecessary_literal_unwrap.rs:85:22 + | +LL | let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Ok` and `unwrap_unchecked()` + | +LL - let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() }; +LL + let _ = 1; + | + +error: used `unwrap_unchecked()` on `Ok` value + --> $DIR/unnecessary_literal_unwrap.rs:86:22 + | +LL | let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() + *(&1 as *const i32) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Ok` and `unwrap_unchecked()` + | +LL - let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() + *(&1 as *const i32) }; +LL + let _ = unsafe { 1 + *(&1 as *const i32) }; + | + +error: used `unwrap_unchecked()` on `Ok` value + --> $DIR/unnecessary_literal_unwrap.rs:87:22 + | +LL | let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() } + 1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Ok` and `unwrap_unchecked()` + | +LL - let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() } + 1; +LL + let _ = 1 + 1; + | + +error: aborting due to 42 previous errors From 0b5dac09757237a407a1cdfbff16eeb4606f6390 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 4 Jul 2023 00:49:18 +0200 Subject: [PATCH 2/2] [`unnecessary_literal_unwrap`]: also handle unwrap_err_unchecked --- clippy_lints/src/methods/mod.rs | 5 +---- .../src/methods/unnecessary_literal_unwrap.rs | 2 +- tests/ui/unnecessary_literal_unwrap.fixed | 1 + tests/ui/unnecessary_literal_unwrap.rs | 1 + tests/ui/unnecessary_literal_unwrap.stderr | 14 +++++++++++++- 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6e936e92cbd..d6057ce45f6 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4000,9 +4000,6 @@ impl Methods { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests); }, - ("unwrap_unchecked", []) => { - unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - } ("unwrap_err", []) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests); @@ -4022,7 +4019,7 @@ impl Methods { } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, - ("unwrap_or_default", []) => { + ("unwrap_or_default" | "unwrap_unchecked" | "unwrap_err_unchecked", []) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); } ("unwrap_or_else", [u_arg]) => { diff --git a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs index 13895b67ed0..54de300e6bc 100644 --- a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs +++ b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs @@ -67,7 +67,7 @@ pub(super) fn check( (expr.span.with_hi(args[0].span.lo()), "panic!(".to_string()), (expr.span.with_lo(args[0].span.hi()), ")".to_string()), ]), - ("Some" | "Ok", "unwrap_unchecked", _) => { + ("Some" | "Ok", "unwrap_unchecked", _) | ("Err", "unwrap_err_unchecked", _) => { let mut suggs = vec![ (recv.span.with_hi(call_args[0].span.lo()), String::new()), (expr.span.with_lo(call_args[0].span.hi()), String::new()), diff --git a/tests/ui/unnecessary_literal_unwrap.fixed b/tests/ui/unnecessary_literal_unwrap.fixed index 0e7ba6bcbbb..276cd800b89 100644 --- a/tests/ui/unnecessary_literal_unwrap.fixed +++ b/tests/ui/unnecessary_literal_unwrap.fixed @@ -85,6 +85,7 @@ fn unwrap_unchecked() { let _ = 1; let _ = unsafe { 1 + *(&1 as *const i32) }; let _ = 1 + 1; + let _ = 123; } fn main() { diff --git a/tests/ui/unnecessary_literal_unwrap.rs b/tests/ui/unnecessary_literal_unwrap.rs index fda7fd36ff9..3065778d779 100644 --- a/tests/ui/unnecessary_literal_unwrap.rs +++ b/tests/ui/unnecessary_literal_unwrap.rs @@ -85,6 +85,7 @@ fn unwrap_unchecked() { let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() }; let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() + *(&1 as *const i32) }; let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() } + 1; + let _ = unsafe { Err::<(), i32>(123).unwrap_err_unchecked() }; } fn main() { diff --git a/tests/ui/unnecessary_literal_unwrap.stderr b/tests/ui/unnecessary_literal_unwrap.stderr index e5b93c3d9af..5823313b736 100644 --- a/tests/ui/unnecessary_literal_unwrap.stderr +++ b/tests/ui/unnecessary_literal_unwrap.stderr @@ -481,5 +481,17 @@ LL - let _ = unsafe { Ok::<_, ()>(1).unwrap_unchecked() } + 1; LL + let _ = 1 + 1; | -error: aborting due to 42 previous errors +error: used `unwrap_err_unchecked()` on `Err` value + --> $DIR/unnecessary_literal_unwrap.rs:88:22 + | +LL | let _ = unsafe { Err::<(), i32>(123).unwrap_err_unchecked() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `Err` and `unwrap_err_unchecked()` + | +LL - let _ = unsafe { Err::<(), i32>(123).unwrap_err_unchecked() }; +LL + let _ = 123; + | + +error: aborting due to 43 previous errors