diff options
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 98 | ||||
| -rw-r--r-- | tests/ui/unwrap_expect_used.rs | 17 | ||||
| -rw-r--r-- | tests/ui/unwrap_expect_used.stderr | 34 |
3 files changed, 105 insertions, 44 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 10f4637d08f..4690119ba3d 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4709,6 +4709,8 @@ impl_lint_pass!(Methods => [ ]); /// Extracts a method call name, args, and `Span` of the method name. +/// This ensures that neither the receiver nor any of the arguments +/// come from expansion. pub fn method_call<'tcx>( recv: &'tcx Expr<'tcx>, ) -> Option<(&'tcx str, &'tcx Expr<'tcx>, &'tcx [Expr<'tcx>], Span, Span)> { @@ -4907,6 +4909,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { impl Methods { #[allow(clippy::too_many_lines)] fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // Handle method calls whose receiver and arguments may not come from expansion if let Some((name, recv, args, span, call_span)) = method_call(expr) { match (name, args) { ("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => { @@ -5049,29 +5052,12 @@ impl Methods { Some(("err", recv, [], err_span, _)) => { err_expect::check(cx, expr, recv, span, err_span, self.msrv); }, - _ => unwrap_expect_used::check( - cx, - expr, - recv, - false, - self.allow_expect_in_consts, - self.allow_expect_in_tests, - unwrap_expect_used::Variant::Expect, - ), + _ => {}, } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, - ("expect_err", [_]) => { + ("expect_err", [_]) | ("unwrap_err" | "unwrap_unchecked" | "unwrap_err_unchecked", []) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - unwrap_expect_used::check( - cx, - expr, - recv, - true, - self.allow_expect_in_consts, - self.allow_expect_in_tests, - unwrap_expect_used::Variant::Expect, - ); }, ("extend", [arg]) => { string_extend_chars::check(cx, expr, recv, arg); @@ -5437,27 +5423,6 @@ impl Methods { _ => {}, } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - unwrap_expect_used::check( - cx, - expr, - recv, - false, - self.allow_unwrap_in_consts, - self.allow_unwrap_in_tests, - unwrap_expect_used::Variant::Unwrap, - ); - }, - ("unwrap_err", []) => { - unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - unwrap_expect_used::check( - cx, - expr, - recv, - true, - self.allow_unwrap_in_consts, - self.allow_unwrap_in_tests, - unwrap_expect_used::Variant::Unwrap, - ); }, ("unwrap_or", [u_arg]) => { match method_call(recv) { @@ -5486,9 +5451,6 @@ impl Methods { } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, - ("unwrap_unchecked" | "unwrap_err_unchecked", []) => { - unnecessary_literal_unwrap::check(cx, expr, recv, name, args); - }, ("unwrap_or_else", [u_arg]) => { match method_call(recv) { Some(("map", recv, [map_arg], _, _)) @@ -5526,6 +5488,56 @@ impl Methods { _ => {}, } } + // Handle method calls whose receiver and arguments may come from expansion + if let ExprKind::MethodCall(path, recv, args, _call_span) = expr.kind { + match (path.ident.name.as_str(), args) { + ("expect", [_]) if !matches!(method_call(recv), Some(("ok" | "err", _, [], _, _))) => { + unwrap_expect_used::check( + cx, + expr, + recv, + false, + self.allow_expect_in_consts, + self.allow_expect_in_tests, + unwrap_expect_used::Variant::Expect, + ); + }, + ("expect_err", [_]) => { + unwrap_expect_used::check( + cx, + expr, + recv, + true, + self.allow_expect_in_consts, + self.allow_expect_in_tests, + unwrap_expect_used::Variant::Expect, + ); + }, + ("unwrap", []) => { + unwrap_expect_used::check( + cx, + expr, + recv, + false, + self.allow_unwrap_in_consts, + self.allow_unwrap_in_tests, + unwrap_expect_used::Variant::Unwrap, + ); + }, + ("unwrap_err", []) => { + unwrap_expect_used::check( + cx, + expr, + recv, + true, + self.allow_unwrap_in_consts, + self.allow_unwrap_in_tests, + unwrap_expect_used::Variant::Unwrap, + ); + }, + _ => {}, + } + } } } diff --git a/tests/ui/unwrap_expect_used.rs b/tests/ui/unwrap_expect_used.rs index d0bb571273b..b429f3a8a0b 100644 --- a/tests/ui/unwrap_expect_used.rs +++ b/tests/ui/unwrap_expect_used.rs @@ -66,3 +66,20 @@ fn main() { SOME.expect("Still not three?"); } } + +mod with_expansion { + macro_rules! open { + ($file:expr) => { + std::fs::File::open($file) + }; + } + + fn test(file: &str) { + use std::io::Read; + let mut s = String::new(); + let _ = open!(file).unwrap(); //~ unwrap_used + let _ = open!(file).expect("can open"); //~ expect_used + let _ = open!(file).unwrap_err(); //~ unwrap_used + let _ = open!(file).expect_err("can open"); //~ expect_used + } +} diff --git a/tests/ui/unwrap_expect_used.stderr b/tests/ui/unwrap_expect_used.stderr index 79eac3f58cc..6fd1b84d812 100644 --- a/tests/ui/unwrap_expect_used.stderr +++ b/tests/ui/unwrap_expect_used.stderr @@ -50,5 +50,37 @@ LL | a.expect_err("Hello error!"); | = note: if this value is an `Ok`, it will panic -error: aborting due to 6 previous errors +error: used `unwrap()` on a `Result` value + --> tests/ui/unwrap_expect_used.rs:80:17 + | +LL | let _ = open!(file).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: if this value is an `Err`, it will panic + +error: used `expect()` on a `Result` value + --> tests/ui/unwrap_expect_used.rs:81:17 + | +LL | let _ = open!(file).expect("can open"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: if this value is an `Err`, it will panic + +error: used `unwrap_err()` on a `Result` value + --> tests/ui/unwrap_expect_used.rs:82:17 + | +LL | let _ = open!(file).unwrap_err(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: if this value is an `Ok`, it will panic + +error: used `expect_err()` on a `Result` value + --> tests/ui/unwrap_expect_used.rs:83:17 + | +LL | let _ = open!(file).expect_err("can open"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: if this value is an `Ok`, it will panic + +error: aborting due to 10 previous errors |