Skip to content

Commit

Permalink
Fix unnecessary_fold bug
Browse files Browse the repository at this point in the history
  • Loading branch information
theotherphil committed Jan 22, 2018
1 parent 96cba36 commit 2132e5c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 4 deletions.
11 changes: 9 additions & 2 deletions clippy_lints/src/methods.rs
Expand Up @@ -1141,6 +1141,13 @@ fn lint_unnecessary_fold(cx: &LateContext, expr: &hir::Expr, fold_args: &[hir::E
assert!(fold_args.len() == 3,
"Expected fold_args to have three entries - the receiver, the initial value and the closure");

fn is_exactly_closure_param(expr: &hir::Expr, closure_param: ast::Name) -> bool {
if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = expr.node {
return path.segments.len() == 1 && &path.segments[0].name == &closure_param;
}
false
}

fn check_fold_with_op(
cx: &LateContext,
fold_args: &[hir::Expr],
Expand All @@ -1162,8 +1169,8 @@ fn lint_unnecessary_fold(cx: &LateContext, expr: &hir::Expr, fold_args: &[hir::E
if let Some(first_arg_ident) = get_arg_name(&closure_body.arguments[0].pat);
if let Some(second_arg_ident) = get_arg_name(&closure_body.arguments[1].pat);

if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = left_expr.node;
if path.segments.len() == 1 && &path.segments[0].name == &first_arg_ident;
if is_exactly_closure_param(&*left_expr, first_arg_ident);
if replacement_has_args || is_exactly_closure_param(&*right_expr, second_arg_ident);

then {
// Span containing `.fold(...)`
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/methods.rs
Expand Up @@ -419,6 +419,9 @@ fn unnecessary_fold_should_ignore() {
let _ = (0..3).fold(true, |acc, x| x > 2 && acc);
let _ = (0..3).fold(0, |acc, x| x + acc);
let _ = (0..3).fold(1, |acc, x| x * acc);

let _ = [(0..2), (0..3)].iter().fold(0, |a, b| a + b.len());
let _ = [(0..2), (0..3)].iter().fold(1, |a, b| a * b.len());
}

#[allow(similar_names)]
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/methods.stderr
Expand Up @@ -526,9 +526,9 @@ error: this `.fold` can be written more succinctly using another method
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.any(|x| x > 2)`

error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
--> $DIR/methods.rs:427:13
--> $DIR/methods.rs:430:13
|
427 | let _ = opt.unwrap();
430 | let _ = opt.unwrap();
| ^^^^^^^^^^^^
|
= note: `-D option-unwrap-used` implied by `-D warnings`
Expand Down

0 comments on commit 2132e5c

Please sign in to comment.