From 354172a18efcf104db76fd7dba884b329e9bac7d Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Fri, 23 Jun 2023 05:43:53 -0500 Subject: [PATCH 1/5] New lint `manual_try_fold` --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/manual_try_fold.rs | 55 ++++++++++++++ clippy_lints/src/methods/mod.rs | 33 +++++++- clippy_utils/src/msrvs.rs | 1 + tests/ui/manual_try_fold.rs | 84 +++++++++++++++++++++ tests/ui/manual_try_fold.stderr | 22 ++++++ 7 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/methods/manual_try_fold.rs create mode 100644 tests/ui/manual_try_fold.rs create mode 100644 tests/ui/manual_try_fold.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index ff39c6a0a6c3..14d822083d8b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 52bc2010207a..9d9ee6ba3079 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/methods/manual_try_fold.rs b/clippy_lints/src/methods/manual_try_fold.rs new file mode 100644 index 000000000000..9f92524d356d --- /dev/null +++ b/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, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index b1d3b61aea37..a8d03bb95e68 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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); diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 1f70f11bd677..3637476c4087 100644 --- a/clippy_utils/src/msrvs.rs +++ b/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 } diff --git a/tests/ui/manual_try_fold.rs b/tests/ui/manual_try_fold.rs new file mode 100644 index 000000000000..f43519dd44fa --- /dev/null +++ b/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 FromResidual 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 { + todo!() + } +} + +// Test custom `Try` with only 1 argument +#[derive(Default)] +struct NotOptionButWorse(i32); + +impl FromResidual 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 { + 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(); + } +} diff --git a/tests/ui/manual_try_fold.stderr b/tests/ui/manual_try_fold.stderr new file mode 100644 index 000000000000..1a17435b4595 --- /dev/null +++ b/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 + From 24039ca2c66bf32b3b9112095f27c94694a012b9 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Fri, 23 Jun 2023 05:48:59 -0500 Subject: [PATCH 2/5] Add msrv tests --- tests/ui/manual_try_fold.rs | 16 ++++++++++++++++ tests/ui/manual_try_fold.stderr | 8 +++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/ui/manual_try_fold.rs b/tests/ui/manual_try_fold.rs index f43519dd44fa..e00371eb2288 100644 --- a/tests/ui/manual_try_fold.rs +++ b/tests/ui/manual_try_fold.rs @@ -82,3 +82,19 @@ fn main() { [1, 2, 3].iter().try_fold(0i32, |sum, i| sum.checked_add(*i)).unwrap(); } } + +#[clippy::msrv = "1.26.0"] +fn msrv_too_low() { + [1, 2, 3] + .iter() + .fold(Some(0i32), |sum, i| sum?.checked_add(*i)) + .unwrap(); +} + +#[clippy::msrv = "1.27.0"] +fn msrv_juust_right() { + [1, 2, 3] + .iter() + .fold(Some(0i32), |sum, i| sum?.checked_add(*i)) + .unwrap(); +} diff --git a/tests/ui/manual_try_fold.stderr b/tests/ui/manual_try_fold.stderr index 1a17435b4595..0c8a08e6c7d3 100644 --- a/tests/ui/manual_try_fold.stderr +++ b/tests/ui/manual_try_fold.stderr @@ -18,5 +18,11 @@ error: you seem to be using `Iterator::fold` on a type that implements `Try` 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 +error: you seem to be using `Iterator::fold` on a type that implements `Try` + --> $DIR/manual_try_fold.rs:98:10 + | +LL | .fold(Some(0i32), |sum, i| sum?.checked_add(*i)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i|, ...)` + +error: aborting due to 4 previous errors From 04b08576911cf06bc444fb0615f50c29ec82e675 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Sun, 25 Jun 2023 09:01:06 -0500 Subject: [PATCH 3/5] Typo --- clippy_lints/src/methods/manual_try_fold.rs | 4 ++-- tests/ui/manual_try_fold.stderr | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/methods/manual_try_fold.rs b/clippy_lints/src/methods/manual_try_fold.rs index 9f92524d356d..a1d83cf14199 100644 --- a/clippy_lints/src/methods/manual_try_fold.rs +++ b/clippy_lints/src/methods/manual_try_fold.rs @@ -33,8 +33,8 @@ pub(super) fn check<'tcx>( && 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 Some(args_snip) = closure.fn_arg_span.and_then(|fn_arg_span| snippet_opt(cx, fn_arg_span)) { let init_snip = rest .is_empty() @@ -48,7 +48,7 @@ pub(super) fn check<'tcx>( 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}, ...)", ), + format!("try_fold({init_snip}, {args_snip} ...)", ), Applicability::HasPlaceholders, ); } diff --git a/tests/ui/manual_try_fold.stderr b/tests/ui/manual_try_fold.stderr index 0c8a08e6c7d3..2c0b0c67fd16 100644 --- a/tests/ui/manual_try_fold.stderr +++ b/tests/ui/manual_try_fold.stderr @@ -2,7 +2,7 @@ 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|, ...)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i| ...)` | = note: `-D clippy::manual-try-fold` implied by `-D warnings` @@ -10,19 +10,19 @@ 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|, ...)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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|, ...)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i| ...)` error: you seem to be using `Iterator::fold` on a type that implements `Try` --> $DIR/manual_try_fold.rs:98:10 | LL | .fold(Some(0i32), |sum, i| sum?.checked_add(*i)) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i|, ...)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i| ...)` error: aborting due to 4 previous errors From fbb3f759e2c6d90456f42a72eaaa741201d8e463 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Mon, 26 Jun 2023 14:32:01 -0500 Subject: [PATCH 4/5] update docs --- clippy_lints/src/methods/mod.rs | 13 +++++++++---- tests/ui/manual_try_fold.rs | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index a8d03bb95e68..2cd7ca02755f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3292,10 +3292,15 @@ declare_clippy_lint! { /// 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. + /// This should use `try_fold` instead, which short-circuits on failure, thus opening the door + /// for additional optimizations not possible with `fold` as rustc can guarantee the function is + /// never called on `None`, `Err`, etc., alleviating otherwise necessary checks. It's also + /// slightly more idiomatic. + /// + /// ### Known issues + /// This lint doesn't take into account whether a function does something on the failure case, + /// i.e., whether short-circuiting will affect behavior. Refactoring to `try_fold` is not + /// desirable in those cases. /// /// ### Example /// ```rust diff --git a/tests/ui/manual_try_fold.rs b/tests/ui/manual_try_fold.rs index e00371eb2288..4521e9fa1a50 100644 --- a/tests/ui/manual_try_fold.rs +++ b/tests/ui/manual_try_fold.rs @@ -1,4 +1,4 @@ -//@aux-build:proc_macros.rs +//@aux-build:proc_macros.rs:proc-macro #![allow(clippy::unnecessary_fold, unused)] #![warn(clippy::manual_try_fold)] #![feature(try_trait_v2)] From cb5d7e344a4f42eefb5b7d3df42f8899a684e73b Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Thu, 29 Jun 2023 02:26:45 -0500 Subject: [PATCH 5/5] address comments --- book/src/lint_configuration.md | 1 + clippy_lints/src/methods/manual_try_fold.rs | 2 +- clippy_lints/src/methods/mod.rs | 8 ++++---- clippy_lints/src/utils/conf.rs | 2 +- tests/ui/manual_try_fold.stderr | 8 ++++---- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index ae0b51403167..60d7ce6e6155 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -150,6 +150,7 @@ The minimum rust version that the project supports * [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain) * [`type_repetition_in_bounds`](https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds) * [`tuple_array_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions) +* [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold) ## `cognitive-complexity-threshold` diff --git a/clippy_lints/src/methods/manual_try_fold.rs b/clippy_lints/src/methods/manual_try_fold.rs index a1d83cf14199..576a58499b15 100644 --- a/clippy_lints/src/methods/manual_try_fold.rs +++ b/clippy_lints/src/methods/manual_try_fold.rs @@ -46,7 +46,7 @@ pub(super) fn check<'tcx>( cx, MANUAL_TRY_FOLD, fold_span, - "you seem to be using `Iterator::fold` on a type that implements `Try`", + "usage of `Iterator::fold` on a type that implements `Try`", "use `try_fold` instead", format!("try_fold({init_snip}, {args_snip} ...)", ), Applicability::HasPlaceholders, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 2cd7ca02755f..24dbe8c1d751 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3292,10 +3292,10 @@ declare_clippy_lint! { /// Checks for usage of `Iterator::fold` with a type that implements `Try`. /// /// ### Why is this bad? - /// This should use `try_fold` instead, which short-circuits on failure, thus opening the door - /// for additional optimizations not possible with `fold` as rustc can guarantee the function is - /// never called on `None`, `Err`, etc., alleviating otherwise necessary checks. It's also - /// slightly more idiomatic. + /// The code should use `try_fold` instead, which short-circuits on failure, thus opening the + /// door for additional optimizations not possible with `fold` as rustc can guarantee the + /// function is never called on `None`, `Err`, etc., alleviating otherwise necessary checks. It's + /// also slightly more idiomatic. /// /// ### Known issues /// This lint doesn't take into account whether a function does something on the failure case, diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 890639646228..f1d05c7529be 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -294,7 +294,7 @@ define_Conf! { /// /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), - /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS. + /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD. /// /// The minimum rust version that the project supports (msrv: Option = None), diff --git a/tests/ui/manual_try_fold.stderr b/tests/ui/manual_try_fold.stderr index 2c0b0c67fd16..a0cf5b3b5fc8 100644 --- a/tests/ui/manual_try_fold.stderr +++ b/tests/ui/manual_try_fold.stderr @@ -1,4 +1,4 @@ -error: you seem to be using `Iterator::fold` on a type that implements `Try` +error: usage of `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)) @@ -6,19 +6,19 @@ LL | .fold(Some(0i32), |sum, i| sum?.checked_add(*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` +error: usage of `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` +error: usage of `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: you seem to be using `Iterator::fold` on a type that implements `Try` +error: usage of `Iterator::fold` on a type that implements `Try` --> $DIR/manual_try_fold.rs:98:10 | LL | .fold(Some(0i32), |sum, i| sum?.checked_add(*i))