about summary refs log tree commit diff
diff options
context:
space:
mode:
authorkoka <koka.code@gmail.com>2022-10-22 16:36:36 +0900
committerkoka <koka.code@gmail.com>2022-10-23 23:20:20 +0900
commita41cb7adbeaa1dae9cf5f371bc02061c63caaf81 (patch)
treeffe23af0fb559e88c93afa4c323f99d4e431f720
parentb72e451310d65ddf69441a641e2ebd6886c813b8 (diff)
downloadrust-a41cb7adbeaa1dae9cf5f371bc02061c63caaf81.tar.gz
rust-a41cb7adbeaa1dae9cf5f371bc02061c63caaf81.zip
fix: support `map_or` for `or_fun_call` lint
* add `rest_arg` to pass second argument from `map_or(U, F)`
* extract some procedures into closure

refac: rename rest_arg/second_arg

refac: organize function argument order

* put `second_arg` next to `arg` argument
-rw-r--r--clippy_lints/src/methods/or_fun_call.rs57
-rw-r--r--tests/ui/manual_ok_or.fixed1
-rw-r--r--tests/ui/manual_ok_or.rs1
-rw-r--r--tests/ui/manual_ok_or.stderr8
-rw-r--r--tests/ui/or_fun_call.fixed16
-rw-r--r--tests/ui/or_fun_call.rs16
-rw-r--r--tests/ui/or_fun_call.stderr14
7 files changed, 92 insertions, 21 deletions
diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs
index 991d3dd538b..4460f38fcc1 100644
--- a/clippy_lints/src/methods/or_fun_call.rs
+++ b/clippy_lints/src/methods/or_fun_call.rs
@@ -83,6 +83,8 @@ pub(super) fn check<'tcx>(
         method_span: Span,
         self_expr: &hir::Expr<'_>,
         arg: &'tcx hir::Expr<'_>,
+        // `Some` if fn has second argument
+        second_arg: Option<&hir::Expr<'_>>,
         span: Span,
         // None if lambda is required
         fun_span: Option<Span>,
@@ -109,30 +111,40 @@ pub(super) fn check<'tcx>(
             if poss.contains(&name);
 
             then {
-                let macro_expanded_snipped;
-                let sugg: Cow<'_, str> = {
+                let sugg = {
                     let (snippet_span, use_lambda) = match (fn_has_arguments, fun_span) {
                         (false, Some(fun_span)) => (fun_span, false),
                         _ => (arg.span, true),
                     };
-                    let snippet = {
-                        let not_macro_argument_snippet = snippet_with_macro_callsite(cx, snippet_span, "..");
-                        if not_macro_argument_snippet == "vec![]" {
-                            macro_expanded_snipped = snippet(cx, snippet_span, "..");
+
+                    let format_span = |span: Span| {
+                        let not_macro_argument_snippet = snippet_with_macro_callsite(cx, span, "..");
+                        let snip = if not_macro_argument_snippet == "vec![]" {
+                            let macro_expanded_snipped = snippet(cx, snippet_span, "..");
                             match macro_expanded_snipped.strip_prefix("$crate::vec::") {
-                                Some(stripped) => Cow::from(stripped),
+                                Some(stripped) => Cow::Owned(stripped.to_owned()),
                                 None => macro_expanded_snipped,
                             }
                         } else {
                             not_macro_argument_snippet
-                        }
+                        };
+
+                        snip.to_string()
                     };
 
-                    if use_lambda {
+                    let snip = format_span(snippet_span);
+                    let snip = if use_lambda {
                         let l_arg = if fn_has_arguments { "_" } else { "" };
-                        format!("|{l_arg}| {snippet}").into()
+                        format!("|{l_arg}| {snip}")
                     } else {
-                        snippet
+                        snip
+                    };
+
+                    if let Some(f) = second_arg {
+                        let f = format_span(f.span);
+                        format!("{snip}, {f}")
+                    } else {
+                        snip
                     }
                 };
                 let span_replace_word = method_span.with_hi(span.hi());
@@ -149,8 +161,8 @@ pub(super) fn check<'tcx>(
         }
     }
 
-    if let [arg] = args {
-        let inner_arg = if let hir::ExprKind::Block(
+    let extract_inner_arg = |arg: &'tcx hir::Expr<'_>| {
+        if let hir::ExprKind::Block(
             hir::Block {
                 stmts: [],
                 expr: Some(expr),
@@ -162,19 +174,32 @@ pub(super) fn check<'tcx>(
             expr
         } else {
             arg
-        };
+        }
+    };
+
+    if let [arg] = args {
+        let inner_arg = extract_inner_arg(arg);
         match inner_arg.kind {
             hir::ExprKind::Call(fun, or_args) => {
                 let or_has_args = !or_args.is_empty();
                 if !check_unwrap_or_default(cx, name, fun, arg, or_has_args, expr.span, method_span) {
                     let fun_span = if or_has_args { None } else { Some(fun.span) };
-                    check_general_case(cx, name, method_span, receiver, arg, expr.span, fun_span);
+                    check_general_case(cx, name, method_span, receiver, arg, None, expr.span, fun_span);
                 }
             },
             hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
-                check_general_case(cx, name, method_span, receiver, arg, expr.span, None);
+                check_general_case(cx, name, method_span, receiver, arg, None, expr.span, None);
             },
             _ => (),
         }
     }
+
+    // `map_or` takes two arguments
+    if let [arg, lambda] = args {
+        let inner_arg = extract_inner_arg(arg);
+        if let hir::ExprKind::Call(fun, or_args) = inner_arg.kind {
+            let fun_span = if or_args.is_empty() { Some(fun.span) } else { None };
+            check_general_case(cx, name, method_span, receiver, arg, Some(lambda), expr.span, fun_span);
+        }
+    }
 }
diff --git a/tests/ui/manual_ok_or.fixed b/tests/ui/manual_ok_or.fixed
index d864f855453..fc8511626b3 100644
--- a/tests/ui/manual_ok_or.fixed
+++ b/tests/ui/manual_ok_or.fixed
@@ -1,5 +1,6 @@
 // run-rustfix
 #![warn(clippy::manual_ok_or)]
+#![allow(clippy::or_fun_call)]
 #![allow(clippy::disallowed_names)]
 #![allow(clippy::redundant_closure)]
 #![allow(dead_code)]
diff --git a/tests/ui/manual_ok_or.rs b/tests/ui/manual_ok_or.rs
index 6264768460e..b5303d33f5f 100644
--- a/tests/ui/manual_ok_or.rs
+++ b/tests/ui/manual_ok_or.rs
@@ -1,5 +1,6 @@
 // run-rustfix
 #![warn(clippy::manual_ok_or)]
+#![allow(clippy::or_fun_call)]
 #![allow(clippy::disallowed_names)]
 #![allow(clippy::redundant_closure)]
 #![allow(dead_code)]
diff --git a/tests/ui/manual_ok_or.stderr b/tests/ui/manual_ok_or.stderr
index 65459a09738..b4a17f143e3 100644
--- a/tests/ui/manual_ok_or.stderr
+++ b/tests/ui/manual_ok_or.stderr
@@ -1,5 +1,5 @@
 error: this pattern reimplements `Option::ok_or`
-  --> $DIR/manual_ok_or.rs:11:5
+  --> $DIR/manual_ok_or.rs:12:5
    |
 LL |     foo.map_or(Err("error"), |v| Ok(v));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
@@ -7,19 +7,19 @@ LL |     foo.map_or(Err("error"), |v| Ok(v));
    = note: `-D clippy::manual-ok-or` implied by `-D warnings`
 
 error: this pattern reimplements `Option::ok_or`
-  --> $DIR/manual_ok_or.rs:14:5
+  --> $DIR/manual_ok_or.rs:15:5
    |
 LL |     foo.map_or(Err("error"), Ok);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")`
 
 error: this pattern reimplements `Option::ok_or`
-  --> $DIR/manual_ok_or.rs:17:5
+  --> $DIR/manual_ok_or.rs:18:5
    |
 LL |     None::<i32>.map_or(Err("error"), |v| Ok(v));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `None::<i32>.ok_or("error")`
 
 error: this pattern reimplements `Option::ok_or`
-  --> $DIR/manual_ok_or.rs:21:5
+  --> $DIR/manual_ok_or.rs:22:5
    |
 LL | /     foo.map_or(Err::<i32, &str>(
 LL | |         &format!(
diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed
index 23b1aa8bebd..be9a65506e1 100644
--- a/tests/ui/or_fun_call.fixed
+++ b/tests/ui/or_fun_call.fixed
@@ -236,4 +236,20 @@ mod issue9608 {
     }
 }
 
+mod issue8993 {
+    fn g() -> i32 {
+        3
+    }
+
+    fn f(n: i32) -> i32 {
+        n
+    }
+
+    fn test_map_or() {
+        let _ = Some(4).map_or_else(g, |v| v);
+        let _ = Some(4).map_or_else(g, f);
+        let _ = Some(4).map_or(0, f);
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs
index 039998f22dd..628c9704638 100644
--- a/tests/ui/or_fun_call.rs
+++ b/tests/ui/or_fun_call.rs
@@ -236,4 +236,20 @@ mod issue9608 {
     }
 }
 
+mod issue8993 {
+    fn g() -> i32 {
+        3
+    }
+
+    fn f(n: i32) -> i32 {
+        n
+    }
+
+    fn test_map_or() {
+        let _ = Some(4).map_or(g(), |v| v);
+        let _ = Some(4).map_or(g(), f);
+        let _ = Some(4).map_or(0, f);
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr
index 113ba150c61..ba3001db7a5 100644
--- a/tests/ui/or_fun_call.stderr
+++ b/tests/ui/or_fun_call.stderr
@@ -156,5 +156,17 @@ error: use of `unwrap_or` followed by a call to `new`
 LL |         .unwrap_or(String::new());
    |          ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`
 
-error: aborting due to 26 previous errors
+error: use of `map_or` followed by a function call
+  --> $DIR/or_fun_call.rs:249:25
+   |
+LL |         let _ = Some(4).map_or(g(), |v| v);
+   |                         ^^^^^^^^^^^^^^^^^^ help: try this: `map_or_else(g, |v| v)`
+
+error: use of `map_or` followed by a function call
+  --> $DIR/or_fun_call.rs:250:25
+   |
+LL |         let _ = Some(4).map_or(g(), f);
+   |                         ^^^^^^^^^^^^^^ help: try this: `map_or_else(g, f)`
+
+error: aborting due to 28 previous errors