diff --git a/clippy_lints/src/methods/unnecessary_fold.rs b/clippy_lints/src/methods/unnecessary_fold.rs index b5d8972d7aad..fe03eea10828 100644 --- a/clippy_lints/src/methods/unnecessary_fold.rs +++ b/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,6 +189,11 @@ 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 { @@ -149,6 +201,11 @@ pub(super) fn check( 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", + }); }, _ => (), } diff --git a/tests/ui/unnecessary_fold.fixed b/tests/ui/unnecessary_fold.fixed index c5bc11b55ab5..525be487589f 100644 --- a/tests/ui/unnecessary_fold.fixed +++ b/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) {} fn num(_: i32) {} @@ -65,16 +68,37 @@ fn issue10000() { // more cases: let _ = map.values().sum::(); + let _ = map.values().sum::(); + let _ = map.values().product::(); let _ = map.values().product::(); let _: i32 = map.values().sum(); + let _: i32 = map.values().sum(); + let _: i32 = map.values().product(); let _: i32 = map.values().product(); anything(map.values().sum::()); + anything(map.values().sum::()); + anything(map.values().product::()); anything(map.values().product::()); 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::() + } + fn mul_turbofish_necessary() -> impl Mul { + (0..3).product::() + } } fn main() {} diff --git a/tests/ui/unnecessary_fold.rs b/tests/ui/unnecessary_fold.rs index 3a5136eeeaeb..268d09662cee 100644 --- a/tests/ui/unnecessary_fold.rs +++ b/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) {} 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() {} diff --git a/tests/ui/unnecessary_fold.stderr b/tests/ui/unnecessary_fold.stderr index 31abab62f4c6..2c4ca7821902 100644 --- a/tests/ui/unnecessary_fold.stderr +++ b/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::()` 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::()` 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::()` + +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::()` 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::()` + +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::()` 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::()` + +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::()` 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::()` + +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::()` + +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::()` + +error: aborting due to 30 previous errors