nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-22 06:44:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #11012 - Centri3:manual_try_fold, r=blyxyas,xFrednet

Browse Source 
New lint [`manual_try_fold`]

Closes #10208

---

changelog: New lint [`manual_try_fold`]
[#11012](https://github.com/rust-lang/rust-clippy/pull/11012)
This commit is contained in:
bors 2023-07-01 22:43:12 +00:00
commit 37f4c1725d
9 changed files with 224 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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

1
book/src/lint_configuration.md
View File

@ -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`

1
clippy_lints/src/declared_lints.rs
View File

@ -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,

55
clippy_lints/src/methods/manual_try_fold.rs Normal file
View File

@ -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
&& !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()
.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,
"usage of `Iterator::fold` on a type that implements `Try`",
"use `try_fold` instead",
format!("try_fold({init_snip}, {args_snip} ...)", ),
Applicability::HasPlaceholders,
);
}
}

38
clippy_lints/src/methods/mod.rs
View File

@ -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,35 @@ 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?
/// 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,
/// i.e., whether short-circuiting will affect behavior. Refactoring to `try_fold` is not
/// desirable in those cases.
///
/// ### 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 +3446,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 +3740,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);

2
clippy_lints/src/utils/conf.rs
View File

@ -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<String> = None),

1
clippy_utils/src/msrvs.rs
View File

@ -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 }

100
tests/ui/manual_try_fold.rs Normal file
View File

@ -0,0 +1,100 @@
//@aux-build:proc_macros.rs:proc-macro
#![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();
}
}
#[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();
}

28
tests/ui/manual_try_fold.stderr Normal file
View File

@ -0,0 +1,28 @@
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))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i| ...)`
|
= note: `-D clippy::manual-try-fold` implied by `-D warnings`
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: 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: 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))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `try_fold` instead: `try_fold(0i32, |sum, i| ...)`
error: aborting due to 4 previous errors