New lint manual_try_fold

Author: Centri3

CHANGELOG.md

@@ -4959,6 +4959,7 @@ Released 2018-09-13
 [`manual_string_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_string_new
 [`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
 [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
+[`manual_try_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold
 [`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
 [`manual_while_let_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_while_let_some
 [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names

clippy_lints/src/declared_lints.rs

@@ -364,6 +364,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO,
     crate::methods::MANUAL_SPLIT_ONCE_INFO,
     crate::methods::MANUAL_STR_REPEAT_INFO,
+    crate::methods::MANUAL_TRY_FOLD_INFO,
     crate::methods::MAP_CLONE_INFO,
     crate::methods::MAP_COLLECT_RESULT_UNIT_INFO,
     crate::methods::MAP_ERR_IGNORE_INFO,

clippy_lints/src/methods/manual_try_fold.rs

@@ -0,0 +1,55 @@
+use clippy_utils::{
+    diagnostics::span_lint_and_sugg,
+    is_from_proc_macro,
+    msrvs::{Msrv, ITERATOR_TRY_FOLD},
+    source::snippet_opt,
+    ty::implements_trait,
+};
+use rustc_errors::Applicability;
+use rustc_hir::{
+    def::{DefKind, Res},
+    Expr, ExprKind,
+};
+use rustc_lint::{LateContext, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_span::Span;
+
+use super::MANUAL_TRY_FOLD;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &Expr<'tcx>,
+    init: &Expr<'_>,
+    acc: &Expr<'_>,
+    fold_span: Span,
+    msrv: &Msrv,
+) {
+    if !in_external_macro(cx.sess(), fold_span)
+        && msrv.meets(ITERATOR_TRY_FOLD)
+        && let init_ty = cx.typeck_results().expr_ty(init)
+        && let Some(try_trait) = cx.tcx.lang_items().try_trait()
+        && implements_trait(cx, init_ty, try_trait, &[])
+        && let ExprKind::Call(path, [first, rest @ ..]) = init.kind
+        && let ExprKind::Path(qpath) = path.kind
+        && let Res::Def(DefKind::Ctor(_, _), _) = cx.qpath_res(&qpath, path.hir_id)
+        && let ExprKind::Closure(closure) = acc.kind
+        && let Some(args_snip) = closure.fn_arg_span.and_then(|fn_arg_span| snippet_opt(cx, fn_arg_span))
+        && !is_from_proc_macro(cx, expr)
+    {
+        let init_snip = rest
+            .is_empty()
+            .then_some(first.span)
+            .and_then(|span| snippet_opt(cx, span))
+            .unwrap_or("...".to_owned());
+
+        span_lint_and_sugg(
+            cx,
+            MANUAL_TRY_FOLD,
+            fold_span,
+            "you seem to be using `Iterator::fold` on a type that implements `Try`",
+            "use `try_fold` instead",
+            format!("try_fold({init_snip}, {args_snip}, ...)", ),
+            Applicability::HasPlaceholders,
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -50,6 +50,7 @@ mod manual_next_back;
 mod manual_ok_or;
 mod manual_saturating_arithmetic;
 mod manual_str_repeat;
+mod manual_try_fold;
 mod map_clone;
 mod map_collect_result_unit;
 mod map_err_ignore;
@@ -3286,6 +3287,30 @@ declare_clippy_lint! {
     "calling `.drain(..).collect()` to move all elements into a new collection"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `Iterator::fold` with a type that implements `Try`.
+    ///
+    /// ### Why is this bad?
+    /// This is better represented with `try_fold`, but this has one major difference: It will
+    /// short-circuit on failure. *This is almost always what you want*. This can also open the door
+    /// for additional optimizations as well, as rustc can guarantee the function is never
+    /// called on `None`, `Err`, etc., alleviating otherwise necessary checks.
+    ///
+    /// ### Example
+    /// ```rust
+    /// vec![1, 2, 3].iter().fold(Some(0i32), |sum, i| sum?.checked_add(*i));
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// vec![1, 2, 3].iter().try_fold(0i32, |sum, i| sum.checked_add(*i));
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub MANUAL_TRY_FOLD,
+    perf,
+    "checks for usage of `Iterator::fold` with a type that implements `Try`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3416,7 +3441,8 @@ impl_lint_pass!(Methods => [
     CLEAR_WITH_DRAIN,
     MANUAL_NEXT_BACK,
     UNNECESSARY_LITERAL_UNWRAP,
-    DRAIN_COLLECT
+    DRAIN_COLLECT,
+    MANUAL_TRY_FOLD,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3709,7 +3735,10 @@ impl Methods {
                     Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
                     _ => {},
                 },
-                ("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span),
+                ("fold", [init, acc]) => {
+                    manual_try_fold::check(cx, expr, init, acc, call_span, &self.msrv);
+                    unnecessary_fold::check(cx, expr, init, acc, span);
+                },
                 ("for_each", [_]) => {
                     if let Some(("inspect", _, [_], span2, _)) = method_call(recv) {
                         inspect_for_each::check(cx, expr, span2);

clippy_utils/src/msrvs.rs

@@ -44,6 +44,7 @@ msrv_aliases! {
     1,34,0 { TRY_FROM }
     1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
     1,28,0 { FROM_BOOL }
+    1,27,0 { ITERATOR_TRY_FOLD }
     1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN }
     1,24,0 { IS_ASCII_DIGIT }
     1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }

tests/ui/manual_try_fold.rs

@@ -0,0 +1,84 @@
+//@aux-build:proc_macros.rs
+#![allow(clippy::unnecessary_fold, unused)]
+#![warn(clippy::manual_try_fold)]
+#![feature(try_trait_v2)]
+
+use std::ops::ControlFlow;
+use std::ops::FromResidual;
+use std::ops::Try;
+
+#[macro_use]
+extern crate proc_macros;
+
+// Test custom `Try` with more than 1 argument
+struct NotOption(i32, i32);
+
+impl<R> FromResidual<R> for NotOption {
+    fn from_residual(_: R) -> Self {
+        todo!()
+    }
+}
+
+impl Try for NotOption {
+    type Output = ();
+    type Residual = ();
+
+    fn from_output(_: Self::Output) -> Self {
+        todo!()
+    }
+
+    fn branch(self) -> ControlFlow<Self::Residual, Self::Output> {
+        todo!()
+    }
+}
+
+// Test custom `Try` with only 1 argument
+#[derive(Default)]
+struct NotOptionButWorse(i32);
+
+impl<R> FromResidual<R> for NotOptionButWorse {
+    fn from_residual(_: R) -> Self {
+        todo!()
+    }
+}
+
+impl Try for NotOptionButWorse {
+    type Output = ();
+    type Residual = ();
+
+    fn from_output(_: Self::Output) -> Self {
+        todo!()
+    }
+
+    fn branch(self) -> ControlFlow<Self::Residual, Self::Output> {
+        todo!()
+    }
+}
+
+fn main() {
+    [1, 2, 3]
+        .iter()
+        .fold(Some(0i32), |sum, i| sum?.checked_add(*i))
+        .unwrap();
+    [1, 2, 3]
+        .iter()
+        .fold(NotOption(0i32, 0i32), |sum, i| NotOption(0i32, 0i32));
+    [1, 2, 3]
+        .iter()
+        .fold(NotOptionButWorse(0i32), |sum, i| NotOptionButWorse(0i32));
+    // Do not lint
+    [1, 2, 3].iter().try_fold(0i32, |sum, i| sum.checked_add(*i)).unwrap();
+    [1, 2, 3].iter().fold(0i32, |sum, i| sum + i);
+    [1, 2, 3]
+        .iter()
+        .fold(NotOptionButWorse::default(), |sum, i| NotOptionButWorse::default());
+    external! {
+        [1, 2, 3].iter().fold(Some(0i32), |sum, i| sum?.checked_add(*i)).unwrap();
+        [1, 2, 3].iter().try_fold(0i32, |sum, i| sum.checked_add(*i)).unwrap();
+    }
+    with_span! {
+        span
+        [1, 2, 3].iter().fold(Some(0i32), |sum, i| sum?.checked_add(*i)).unwrap();
+        [1, 2, 3].iter().try_fold(0i32, |sum, i| sum.checked_add(*i)).unwrap();
+    }
+}

tests/ui/manual_try_fold.stderr

@@ -0,0 +1,22 @@
+error: you seem to be using `Iterator::fold` on a type that implements `Try`
+  --> $DIR/manual_try_fold.rs:61:10
+   |
+LL |         .fold(Some(0i32), |sum, i| sum?.checked_add(*i))
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i|, ...)`
+   |
+   = note: `-D clippy::manual-try-fold` implied by `-D warnings`
+
+error: you seem to be using `Iterator::fold` on a type that implements `Try`
+  --> $DIR/manual_try_fold.rs:65:10
+   |
+LL |         .fold(NotOption(0i32, 0i32), |sum, i| NotOption(0i32, 0i32));
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(..., |sum, i|, ...)`
+
+error: you seem to be using `Iterator::fold` on a type that implements `Try`
+  --> $DIR/manual_try_fold.rs:68:10
+   |
+LL |         .fold(NotOptionButWorse(0i32), |sum, i| NotOptionButWorse(0i32));
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i|, ...)`
+
+error: aborting due to 3 previous errors
+