From 0d94bbf022055f5b42540c53c95529e3b09e53db Mon Sep 17 00:00:00 2001 From: syvb Date: Wed, 5 Jul 2023 23:24:20 -0400 Subject: [PATCH] Add handling for None.unwrap_or(_else) --- .../src/methods/unnecessary_literal_unwrap.rs | 18 ++ tests/ui/unnecessary_literal_unwrap.fixed | 9 + tests/ui/unnecessary_literal_unwrap.rs | 9 + tests/ui/unnecessary_literal_unwrap.stderr | 166 ++++++++++++++---- 4 files changed, 167 insertions(+), 35 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs index 3becd7d17da0..784c5aba776e 100644 --- a/clippy_lints/src/methods/unnecessary_literal_unwrap.rs +++ b/clippy_lints/src/methods/unnecessary_literal_unwrap.rs @@ -72,6 +72,24 @@ pub(super) fn check( }; Some(vec![(expr.span, format!("{default_ty_string}::default()"))]) }, + ("None", "unwrap_or", _) => Some(vec![ + (expr.span.with_hi(args[0].span.lo()), String::new()), + (expr.span.with_lo(args[0].span.hi()), String::new()), + ]), + ("None", "unwrap_or_else", _) => match args[0].kind { + hir::ExprKind::Closure(hir::Closure { + fn_decl: + hir::FnDecl { + output: hir::FnRetTy::DefaultReturn(span) | hir::FnRetTy::Return(hir::Ty { span, .. }), + .. + }, + .. + }) => Some(vec![ + (expr.span.with_hi(span.hi()), String::new()), + (expr.span.with_lo(args[0].span.hi()), String::new()), + ]), + _ => None, + }, _ if call_args.is_empty() => None, (_, _, 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 ac05ca29fa8b..44530d8b1e8d 100644 --- a/tests/ui/unnecessary_literal_unwrap.fixed +++ b/tests/ui/unnecessary_literal_unwrap.fixed @@ -16,14 +16,23 @@ fn unwrap_option_some() { 1; } +#[rustfmt::skip] // force rustfmt not to remove braces in `|| { 234 }` fn unwrap_option_none() { let _val = panic!(); let _val = panic!("this always happens"); let _val: String = String::default(); + let _val: u16 = 234; + let _val: u16 = 234; + let _val: u16 = { 234 }; + let _val: u16 = { 234 }; panic!(); panic!("this always happens"); String::default(); + 234; + 234; + { 234 }; + { 234 }; } fn unwrap_result_ok() { diff --git a/tests/ui/unnecessary_literal_unwrap.rs b/tests/ui/unnecessary_literal_unwrap.rs index c16750734d9c..b43e4d3a347e 100644 --- a/tests/ui/unnecessary_literal_unwrap.rs +++ b/tests/ui/unnecessary_literal_unwrap.rs @@ -16,14 +16,23 @@ fn unwrap_option_some() { Some(1).expect("this never happens"); } +#[rustfmt::skip] // force rustfmt not to remove braces in `|| { 234 }` fn unwrap_option_none() { let _val = None::<()>.unwrap(); let _val = None::<()>.expect("this always happens"); let _val: String = None.unwrap_or_default(); + let _val: u16 = None.unwrap_or(234); + let _val: u16 = None.unwrap_or_else(|| 234); + let _val: u16 = None.unwrap_or_else(|| { 234 }); + let _val: u16 = None.unwrap_or_else(|| -> u16 { 234 }); None::<()>.unwrap(); None::<()>.expect("this always happens"); None::.unwrap_or_default(); + None::.unwrap_or(234); + None::.unwrap_or_else(|| 234); + None::.unwrap_or_else(|| { 234 }); + None::.unwrap_or_else(|| -> u16 { 234 }); } fn unwrap_result_ok() { diff --git a/tests/ui/unnecessary_literal_unwrap.stderr b/tests/ui/unnecessary_literal_unwrap.stderr index dc512ce21efc..905384bc8c22 100644 --- a/tests/ui/unnecessary_literal_unwrap.stderr +++ b/tests/ui/unnecessary_literal_unwrap.stderr @@ -48,13 +48,13 @@ LL + 1; | error: used `unwrap()` on `None` value - --> $DIR/unnecessary_literal_unwrap.rs:20:16 + --> $DIR/unnecessary_literal_unwrap.rs:21:16 | LL | let _val = None::<()>.unwrap(); | ^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap()`: `panic!()` error: used `expect()` on `None` value - --> $DIR/unnecessary_literal_unwrap.rs:21:16 + --> $DIR/unnecessary_literal_unwrap.rs:22:16 | LL | let _val = None::<()>.expect("this always happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -65,19 +65,67 @@ LL | let _val = panic!("this always happens"); | ~~~~~~~ ~ error: used `unwrap_or_default()` on `None` value - --> $DIR/unnecessary_literal_unwrap.rs:22:24 + --> $DIR/unnecessary_literal_unwrap.rs:23:24 | LL | let _val: String = None.unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap_or_default()`: `String::default()` +error: used `unwrap_or()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:24:21 + | +LL | let _val: u16 = None.unwrap_or(234); + | ^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or()` + | +LL - let _val: u16 = None.unwrap_or(234); +LL + let _val: u16 = 234; + | + +error: used `unwrap_or_else()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:25:21 + | +LL | let _val: u16 = None.unwrap_or_else(|| 234); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or_else()` + | +LL - let _val: u16 = None.unwrap_or_else(|| 234); +LL + let _val: u16 = 234; + | + +error: used `unwrap_or_else()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:26:21 + | +LL | let _val: u16 = None.unwrap_or_else(|| { 234 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or_else()` + | +LL - let _val: u16 = None.unwrap_or_else(|| { 234 }); +LL + let _val: u16 = { 234 }; + | + +error: used `unwrap_or_else()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:27:21 + | +LL | let _val: u16 = None.unwrap_or_else(|| -> u16 { 234 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or_else()` + | +LL - let _val: u16 = None.unwrap_or_else(|| -> u16 { 234 }); +LL + let _val: u16 = { 234 }; + | + error: used `unwrap()` on `None` value - --> $DIR/unnecessary_literal_unwrap.rs:24:5 + --> $DIR/unnecessary_literal_unwrap.rs:29:5 | LL | None::<()>.unwrap(); | ^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap()`: `panic!()` error: used `expect()` on `None` value - --> $DIR/unnecessary_literal_unwrap.rs:25:5 + --> $DIR/unnecessary_literal_unwrap.rs:30:5 | LL | None::<()>.expect("this always happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,13 +136,61 @@ LL | panic!("this always happens"); | ~~~~~~~ ~ error: used `unwrap_or_default()` on `None` value - --> $DIR/unnecessary_literal_unwrap.rs:26:5 + --> $DIR/unnecessary_literal_unwrap.rs:31:5 | LL | None::.unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap_or_default()`: `String::default()` +error: used `unwrap_or()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:32:5 + | +LL | None::.unwrap_or(234); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or()` + | +LL - None::.unwrap_or(234); +LL + 234; + | + +error: used `unwrap_or_else()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:33:5 + | +LL | None::.unwrap_or_else(|| 234); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or_else()` + | +LL - None::.unwrap_or_else(|| 234); +LL + 234; + | + +error: used `unwrap_or_else()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:34:5 + | +LL | None::.unwrap_or_else(|| { 234 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or_else()` + | +LL - None::.unwrap_or_else(|| { 234 }); +LL + { 234 }; + | + +error: used `unwrap_or_else()` on `None` value + --> $DIR/unnecessary_literal_unwrap.rs:35:5 + | +LL | None::.unwrap_or_else(|| -> u16 { 234 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the `None` and `unwrap_or_else()` + | +LL - None::.unwrap_or_else(|| -> u16 { 234 }); +LL + { 234 }; + | + error: used `unwrap()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:30:16 + --> $DIR/unnecessary_literal_unwrap.rs:39:16 | LL | let _val = Ok::<_, ()>(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -106,7 +202,7 @@ LL + let _val = 1; | error: used `expect()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:31:16 + --> $DIR/unnecessary_literal_unwrap.rs:40:16 | LL | let _val = Ok::<_, ()>(1).expect("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -118,7 +214,7 @@ LL + let _val = 1; | error: used `unwrap_err()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:32:16 + --> $DIR/unnecessary_literal_unwrap.rs:41:16 | LL | let _val = Ok::<_, ()>(1).unwrap_err(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -129,7 +225,7 @@ LL | let _val = panic!("{:?}", 1); | ~~~~~~~~~~~~~~ ~ error: used `expect_err()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:33:16 + --> $DIR/unnecessary_literal_unwrap.rs:42:16 | LL | let _val = Ok::<_, ()>(1).expect_err("this always happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -140,7 +236,7 @@ LL | let _val = panic!("{1}: {:?}", 1, "this always happens"); | ~~~~~~~~~~~~~~~~~~~ ~ error: used `unwrap()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:35:5 + --> $DIR/unnecessary_literal_unwrap.rs:44:5 | LL | Ok::<_, ()>(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -152,7 +248,7 @@ LL + 1; | error: used `expect()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:36:5 + --> $DIR/unnecessary_literal_unwrap.rs:45:5 | LL | Ok::<_, ()>(1).expect("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -164,7 +260,7 @@ LL + 1; | error: used `unwrap_err()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:37:5 + --> $DIR/unnecessary_literal_unwrap.rs:46:5 | LL | Ok::<_, ()>(1).unwrap_err(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -175,7 +271,7 @@ LL | panic!("{:?}", 1); | ~~~~~~~~~~~~~~ ~ error: used `expect_err()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:38:5 + --> $DIR/unnecessary_literal_unwrap.rs:47:5 | LL | Ok::<_, ()>(1).expect_err("this always happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -186,7 +282,7 @@ LL | panic!("{1}: {:?}", 1, "this always happens"); | ~~~~~~~~~~~~~~~~~~~ ~ error: used `unwrap_err()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:42:16 + --> $DIR/unnecessary_literal_unwrap.rs:51:16 | LL | let _val = Err::<(), _>(1).unwrap_err(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -198,7 +294,7 @@ LL + let _val = 1; | error: used `expect_err()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:43:16 + --> $DIR/unnecessary_literal_unwrap.rs:52:16 | LL | let _val = Err::<(), _>(1).expect_err("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -210,7 +306,7 @@ LL + let _val = 1; | error: used `unwrap()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:44:16 + --> $DIR/unnecessary_literal_unwrap.rs:53:16 | LL | let _val = Err::<(), _>(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -221,7 +317,7 @@ LL | let _val = panic!("{:?}", 1); | ~~~~~~~~~~~~~~ ~ error: used `expect()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:45:16 + --> $DIR/unnecessary_literal_unwrap.rs:54:16 | LL | let _val = Err::<(), _>(1).expect("this always happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -232,7 +328,7 @@ LL | let _val = panic!("{1}: {:?}", 1, "this always happens"); | ~~~~~~~~~~~~~~~~~~~ ~ error: used `unwrap_err()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:47:5 + --> $DIR/unnecessary_literal_unwrap.rs:56:5 | LL | Err::<(), _>(1).unwrap_err(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -244,7 +340,7 @@ LL + 1; | error: used `expect_err()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:48:5 + --> $DIR/unnecessary_literal_unwrap.rs:57:5 | LL | Err::<(), _>(1).expect_err("this never happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -256,7 +352,7 @@ LL + 1; | error: used `unwrap()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:49:5 + --> $DIR/unnecessary_literal_unwrap.rs:58:5 | LL | Err::<(), _>(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -267,7 +363,7 @@ LL | panic!("{:?}", 1); | ~~~~~~~~~~~~~~ ~ error: used `expect()` on `Err` value - --> $DIR/unnecessary_literal_unwrap.rs:50:5 + --> $DIR/unnecessary_literal_unwrap.rs:59:5 | LL | Err::<(), _>(1).expect("this always happens"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -278,7 +374,7 @@ LL | panic!("{1}: {:?}", 1, "this always happens"); | ~~~~~~~~~~~~~~~~~~~ ~ error: used `unwrap_or()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:54:16 + --> $DIR/unnecessary_literal_unwrap.rs:63:16 | LL | let _val = Some(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^ @@ -290,7 +386,7 @@ LL + let _val = 1; | error: used `unwrap_or_default()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:55:16 + --> $DIR/unnecessary_literal_unwrap.rs:64:16 | LL | let _val = Some(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -302,7 +398,7 @@ LL + let _val = 1; | error: used `unwrap_or_else()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:56:16 + --> $DIR/unnecessary_literal_unwrap.rs:65:16 | LL | let _val = Some(1).unwrap_or_else(|| 2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -314,7 +410,7 @@ LL + let _val = 1; | error: used `unwrap_or()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:58:5 + --> $DIR/unnecessary_literal_unwrap.rs:67:5 | LL | Some(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^ @@ -326,7 +422,7 @@ LL + 1; | error: used `unwrap_or_default()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:59:5 + --> $DIR/unnecessary_literal_unwrap.rs:68:5 | LL | Some(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -338,7 +434,7 @@ LL + 1; | error: used `unwrap_or_else()` on `Some` value - --> $DIR/unnecessary_literal_unwrap.rs:60:5 + --> $DIR/unnecessary_literal_unwrap.rs:69:5 | LL | Some(1).unwrap_or_else(|| 2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -350,7 +446,7 @@ LL + 1; | error: used `unwrap_or()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:64:16 + --> $DIR/unnecessary_literal_unwrap.rs:73:16 | LL | let _val = Ok::<_, ()>(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -362,7 +458,7 @@ LL + let _val = 1; | error: used `unwrap_or_default()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:65:16 + --> $DIR/unnecessary_literal_unwrap.rs:74:16 | LL | let _val = Ok::<_, ()>(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -374,7 +470,7 @@ LL + let _val = 1; | error: used `unwrap_or_else()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:66:16 + --> $DIR/unnecessary_literal_unwrap.rs:75:16 | LL | let _val = Ok::<_, ()>(1).unwrap_or_else(|_| 2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -386,7 +482,7 @@ LL + let _val = 1; | error: used `unwrap_or()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:68:5 + --> $DIR/unnecessary_literal_unwrap.rs:77:5 | LL | Ok::<_, ()>(1).unwrap_or(2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -398,7 +494,7 @@ LL + 1; | error: used `unwrap_or_default()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:69:5 + --> $DIR/unnecessary_literal_unwrap.rs:78:5 | LL | Ok::<_, ()>(1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -410,7 +506,7 @@ LL + 1; | error: used `unwrap_or_else()` on `Ok` value - --> $DIR/unnecessary_literal_unwrap.rs:70:5 + --> $DIR/unnecessary_literal_unwrap.rs:79:5 | LL | Ok::<_, ()>(1).unwrap_or_else(|_| 2); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -421,5 +517,5 @@ LL - Ok::<_, ()>(1).unwrap_or_else(|_| 2); LL + 1; | -error: aborting due to 38 previous errors +error: aborting due to 46 previous errors