unnecessary_fold improvements

clippy_lints/src/methods/unnecessary_fold.rs

@@ -8,7 +8,7 @@ use rustc_hir as hir;
 use rustc_hir::PatKind;
 use rustc_lint::LateContext;
 use rustc_middle::ty;
-use rustc_span::{Span, sym};
+use rustc_span::{Span, Symbol, sym};
 
 use super::UNNECESSARY_FOLD;
 
@@ -40,6 +40,19 @@ fn needs_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
         return false;
     }
 
+    // - the final expression in the body of a function with a simple return type
+    if let hir::Node::Block(block) = parent
+        && let grandparent = cx.tcx.parent_hir_node(block.hir_id)
+        && let hir::Node::Expr(grandparent_expr) = grandparent
+        && let ggparent = cx.tcx.parent_hir_node(grandparent_expr.hir_id)
+        && let hir::Node::Item(enclosing_item) = ggparent
+        && let hir::ItemKind::Fn(fn_sig, _, _) = enclosing_item.kind
+        && let hir::FnRetTy::Return(fn_return_ty) = fn_sig.decl.output
+        && matches!(fn_return_ty.kind, hir::TyKind::Path(..))
+    {
+        return false;
+    }
+
     // if it's neither of those, stay on the safe side and suggest turbofish,
     // even if it could work!
     true
@@ -80,7 +93,7 @@ fn check_fold_with_op(
         let mut applicability = Applicability::MachineApplicable;
 
         let turbofish = if replacement.has_generic_return {
-            format!("::<{}>", cx.typeck_results().expr_ty_adjusted(right_expr).peel_refs())
+            format!("::<{}>", cx.typeck_results().expr_ty(expr))
         } else {
             String::new()
         };
@@ -107,6 +120,40 @@ fn check_fold_with_op(
     }
 }
 
+fn check_fold_with_fn(
+    cx: &LateContext<'_>,
+    expr: &hir::Expr<'_>,
+    acc: &hir::Expr<'_>,
+    fold_span: Span,
+    op: Symbol,
+    replacement: Replacement,
+) {
+    if let hir::ExprKind::Path(hir::QPath::Resolved(None, p)) = acc.kind
+        // Extract the name of the function passed to fold
+        && let hir::def::Res::Def(hir::def::DefKind::AssocFn, fn_did) = p.res
+        // Check if the function belongs to the operator
+        && cx.tcx.is_diagnostic_item(op, fn_did)
+    {
+        let applicability = Applicability::MachineApplicable;
+
+        let turbofish = if replacement.has_generic_return {
+            format!("::<{}>", cx.typeck_results().expr_ty(expr))
+        } else {
+            String::new()
+        };
+
+        span_lint_and_sugg(
+            cx,
+            UNNECESSARY_FOLD,
+            fold_span.with_hi(expr.span.hi()),
+            "this `.fold` can be written more succinctly using another method",
+            "try",
+            format!("{method}{turbofish}()", method = replacement.method_name,),
+            applicability,
+        );
+    }
+}
+
 pub(super) fn check(
     cx: &LateContext<'_>,
     expr: &hir::Expr<'_>,
@@ -142,13 +189,23 @@ pub(super) fn check(
                     has_generic_return: needs_turbofish(cx, expr),
                     method_name: "sum",
                 });
+                check_fold_with_fn(cx, expr, acc, fold_span, sym::add, Replacement {
+                    has_args: false,
+                    has_generic_return: needs_turbofish(cx, expr),
+                    method_name: "sum",
+                });
             },
             ast::LitKind::Int(Pu128(1), _) => {
                 check_fold_with_op(cx, expr, acc, fold_span, hir::BinOpKind::Mul, Replacement {
                     has_args: false,
                     has_generic_return: needs_turbofish(cx, expr),
                     method_name: "product",
                 });
+                check_fold_with_fn(cx, expr, acc, fold_span, sym::mul, Replacement {
+                    has_args: false,
+                    has_generic_return: needs_turbofish(cx, expr),
+                    method_name: "product",
+                });
             },
             _ => (),
         }

tests/ui/unnecessary_fold.fixed

@@ -14,8 +14,10 @@ fn unnecessary_fold() {
     let _ = (0..3).all(|x| x > 2);
     // Can be replaced by .sum
     let _: i32 = (0..3).sum();
+    let _: i32 = (0..3).sum();
     // Can be replaced by .product
     let _: i32 = (0..3).product();
+    let _: i32 = (0..3).product();
 }
 
 /// Should trigger the `UNNECESSARY_FOLD` lint, with an error span including exactly `.fold(...)`
@@ -56,6 +58,7 @@ fn unnecessary_fold_over_multiple_lines() {
 fn issue10000() {
     use std::collections::HashMap;
     use std::hash::BuildHasher;
+    use std::ops::{Add, Mul};
 
     fn anything<T>(_: T) {}
     fn num(_: i32) {}
@@ -65,16 +68,37 @@ fn issue10000() {
 
         // more cases:
         let _ = map.values().sum::<i32>();
+        let _ = map.values().sum::<i32>();
+        let _ = map.values().product::<i32>();
         let _ = map.values().product::<i32>();
         let _: i32 = map.values().sum();
+        let _: i32 = map.values().sum();
+        let _: i32 = map.values().product();
         let _: i32 = map.values().product();
         anything(map.values().sum::<i32>());
+        anything(map.values().sum::<i32>());
+        anything(map.values().product::<i32>());
         anything(map.values().product::<i32>());
         num(map.values().sum());
+        num(map.values().sum());
+        num(map.values().product());
         num(map.values().product());
     }
 
     smoketest_map(HashMap::new());
+
+    fn add_turbofish_not_necessary() -> i32 {
+        (0..3).sum()
+    }
+    fn mul_turbofish_not_necessary() -> i32 {
+        (0..3).product()
+    }
+    fn add_turbofish_necessary() -> impl Add {
+        (0..3).sum::<i32>()
+    }
+    fn mul_turbofish_necessary() -> impl Mul {
+        (0..3).product::<i32>()
+    }
 }
 
 fn main() {}

tests/ui/unnecessary_fold.rs

@@ -14,8 +14,10 @@ fn unnecessary_fold() {
     let _ = (0..3).fold(true, |acc, x| acc && x > 2);
     // Can be replaced by .sum
     let _: i32 = (0..3).fold(0, |acc, x| acc + x);
+    let _: i32 = (0..3).fold(0, std::ops::Add::add);
     // Can be replaced by .product
     let _: i32 = (0..3).fold(1, |acc, x| acc * x);
+    let _: i32 = (0..3).fold(1, std::ops::Mul::mul);
 }
 
 /// Should trigger the `UNNECESSARY_FOLD` lint, with an error span including exactly `.fold(...)`
@@ -56,6 +58,7 @@ fn unnecessary_fold_over_multiple_lines() {
 fn issue10000() {
     use std::collections::HashMap;
     use std::hash::BuildHasher;
+    use std::ops::{Add, Mul};
 
     fn anything<T>(_: T) {}
     fn num(_: i32) {}
@@ -65,16 +68,37 @@ fn issue10000() {
 
         // more cases:
         let _ = map.values().fold(0, |x, y| x + y);
+        let _ = map.values().fold(0, Add::add);
         let _ = map.values().fold(1, |x, y| x * y);
+        let _ = map.values().fold(1, Mul::mul);
         let _: i32 = map.values().fold(0, |x, y| x + y);
+        let _: i32 = map.values().fold(0, Add::add);
         let _: i32 = map.values().fold(1, |x, y| x * y);
+        let _: i32 = map.values().fold(1, Mul::mul);
         anything(map.values().fold(0, |x, y| x + y));
+        anything(map.values().fold(0, Add::add));
         anything(map.values().fold(1, |x, y| x * y));
+        anything(map.values().fold(1, Mul::mul));
         num(map.values().fold(0, |x, y| x + y));
+        num(map.values().fold(0, Add::add));
         num(map.values().fold(1, |x, y| x * y));
+        num(map.values().fold(1, Mul::mul));
     }
 
     smoketest_map(HashMap::new());
+
+    fn add_turbofish_not_necessary() -> i32 {
+        (0..3).fold(0, |acc, x| acc + x)
+    }
+    fn mul_turbofish_not_necessary() -> i32 {
+        (0..3).fold(1, |acc, x| acc * x)
+    }
+    fn add_turbofish_necessary() -> impl Add {
+        (0..3).fold(0, |acc, x| acc + x)
+    }
+    fn mul_turbofish_necessary() -> impl Mul {
+        (0..3).fold(1, |acc, x| acc * x)
+    }
 }
 
 fn main() {}

tests/ui/unnecessary_fold.stderr

@@ -29,76 +29,160 @@ LL |     let _: i32 = (0..3).fold(0, |acc, x| acc + x);
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `sum()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:18:25
+  --> tests/ui/unnecessary_fold.rs:17:25
+   |
+LL |     let _: i32 = (0..3).fold(0, std::ops::Add::add);
+   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `sum()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:19:25
    |
 LL |     let _: i32 = (0..3).fold(1, |acc, x| acc * x);
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `product()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:23:41
+  --> tests/ui/unnecessary_fold.rs:20:25
+   |
+LL |     let _: i32 = (0..3).fold(1, std::ops::Mul::mul);
+   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `product()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:25:41
    |
 LL |     let _: bool = (0..3).map(|x| 2 * x).fold(false, |acc, x| acc || x > 2);
    |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `any(|x| x > 2)`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:53:10
+  --> tests/ui/unnecessary_fold.rs:55:10
    |
 LL |         .fold(false, |acc, x| acc || x > 2);
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `any(|x| x > 2)`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:64:33
+  --> tests/ui/unnecessary_fold.rs:67:33
    |
 LL |         assert_eq!(map.values().fold(0, |x, y| x + y), 0);
    |                                 ^^^^^^^^^^^^^^^^^^^^^ help: try: `sum::<i32>()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:67:30
+  --> tests/ui/unnecessary_fold.rs:70:30
    |
 LL |         let _ = map.values().fold(0, |x, y| x + y);
    |                              ^^^^^^^^^^^^^^^^^^^^^ help: try: `sum::<i32>()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:68:30
+  --> tests/ui/unnecessary_fold.rs:71:30
+   |
+LL |         let _ = map.values().fold(0, Add::add);
+   |                              ^^^^^^^^^^^^^^^^^ help: try: `sum::<i32>()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:72:30
    |
 LL |         let _ = map.values().fold(1, |x, y| x * y);
    |                              ^^^^^^^^^^^^^^^^^^^^^ help: try: `product::<i32>()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:69:35
+  --> tests/ui/unnecessary_fold.rs:73:30
+   |
+LL |         let _ = map.values().fold(1, Mul::mul);
+   |                              ^^^^^^^^^^^^^^^^^ help: try: `product::<i32>()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:74:35
    |
 LL |         let _: i32 = map.values().fold(0, |x, y| x + y);
    |                                   ^^^^^^^^^^^^^^^^^^^^^ help: try: `sum()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:70:35
+  --> tests/ui/unnecessary_fold.rs:75:35
+   |
+LL |         let _: i32 = map.values().fold(0, Add::add);
+   |                                   ^^^^^^^^^^^^^^^^^ help: try: `sum()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:76:35
    |
 LL |         let _: i32 = map.values().fold(1, |x, y| x * y);
    |                                   ^^^^^^^^^^^^^^^^^^^^^ help: try: `product()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:71:31
+  --> tests/ui/unnecessary_fold.rs:77:35
+   |
+LL |         let _: i32 = map.values().fold(1, Mul::mul);
+   |                                   ^^^^^^^^^^^^^^^^^ help: try: `product()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:78:31
    |
 LL |         anything(map.values().fold(0, |x, y| x + y));
    |                               ^^^^^^^^^^^^^^^^^^^^^ help: try: `sum::<i32>()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:72:31
+  --> tests/ui/unnecessary_fold.rs:79:31
+   |
+LL |         anything(map.values().fold(0, Add::add));
+   |                               ^^^^^^^^^^^^^^^^^ help: try: `sum::<i32>()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:80:31
    |
 LL |         anything(map.values().fold(1, |x, y| x * y));
    |                               ^^^^^^^^^^^^^^^^^^^^^ help: try: `product::<i32>()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:73:26
+  --> tests/ui/unnecessary_fold.rs:81:31
+   |
+LL |         anything(map.values().fold(1, Mul::mul));
+   |                               ^^^^^^^^^^^^^^^^^ help: try: `product::<i32>()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:82:26
    |
 LL |         num(map.values().fold(0, |x, y| x + y));
    |                          ^^^^^^^^^^^^^^^^^^^^^ help: try: `sum()`
 
 error: this `.fold` can be written more succinctly using another method
-  --> tests/ui/unnecessary_fold.rs:74:26
+  --> tests/ui/unnecessary_fold.rs:83:26
+   |
+LL |         num(map.values().fold(0, Add::add));
+   |                          ^^^^^^^^^^^^^^^^^ help: try: `sum()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:84:26
    |
 LL |         num(map.values().fold(1, |x, y| x * y));
    |                          ^^^^^^^^^^^^^^^^^^^^^ help: try: `product()`
 
-error: aborting due to 16 previous errors
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:85:26
+   |
+LL |         num(map.values().fold(1, Mul::mul));
+   |                          ^^^^^^^^^^^^^^^^^ help: try: `product()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:91:16
+   |
+LL |         (0..3).fold(0, |acc, x| acc + x)
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `sum()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:94:16
+   |
+LL |         (0..3).fold(1, |acc, x| acc * x)
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `product()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:97:16
+   |
+LL |         (0..3).fold(0, |acc, x| acc + x)
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `sum::<i32>()`
+
+error: this `.fold` can be written more succinctly using another method
+  --> tests/ui/unnecessary_fold.rs:100:16
+   |
+LL |         (0..3).fold(1, |acc, x| acc * x)
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `product::<i32>()`
+
+error: aborting due to 30 previous errors