unwrap_used, expect_used: accept macro result as receiver

Author: samueltardieu

clippy_lints/src/methods/mod.rs

@@ -4661,6 +4661,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)> {
@@ -4858,6 +4860,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]) => {
@@ -5000,29 +5003,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);
@@ -5388,27 +5374,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) {
@@ -5437,9 +5402,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], _, _))
@@ -5477,6 +5439,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,
+                    );
+                },
+                _ => {},
+            }
+        }
     }
 }
 

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
+    }
+}

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