Skip to content

Commit

Permalink
Auto merge of #4382 - jeremystucki:unnecessary_fold_span, r=flip1995
Browse files Browse the repository at this point in the history
Change span of unnecessary_fold lint

Resolves #4381

changelog: Change linted span of `unnecessary_fold`
  • Loading branch information
bors committed Aug 14, 2019
2 parents d829d9f + fdf82eb commit bcd0e44
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 23 deletions.
23 changes: 11 additions & 12 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc::ty::{self, Predicate, Ty};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;
use syntax::ast;
use syntax::source_map::{BytePos, Span};
use syntax::source_map::Span;
use syntax::symbol::LocalInternedString;

use crate::utils::paths;
Expand Down Expand Up @@ -1663,6 +1663,7 @@ fn lint_iter_cloned_collect<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Ex
fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: &[hir::Expr]) {
fn check_fold_with_op(
cx: &LateContext<'_, '_>,
expr: &hir::Expr,
fold_args: &[hir::Expr],
op: hir::BinOpKind,
replacement_method_name: &str,
Expand All @@ -1685,30 +1686,28 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args:
if match_var(&*left_expr, first_arg_ident);
if replacement_has_args || match_var(&*right_expr, second_arg_ident);

then {
// Span containing `.fold(...)`
let next_point = cx.sess().source_map().next_point(fold_args[0].span);
let fold_span = next_point.with_hi(fold_args[2].span.hi() + BytePos(1));
if let hir::ExprKind::MethodCall(_, span, _) = &expr.node;

then {
let mut applicability = Applicability::MachineApplicable;
let sugg = if replacement_has_args {
format!(
".{replacement}(|{s}| {r})",
"{replacement}(|{s}| {r})",
replacement = replacement_method_name,
s = second_arg_ident,
r = snippet_with_applicability(cx, right_expr.span, "EXPR", &mut applicability),
)
} else {
format!(
".{replacement}()",
"{replacement}()",
replacement = replacement_method_name,
)
};

span_lint_and_sugg(
cx,
UNNECESSARY_FOLD,
fold_span,
span.with_hi(expr.span.hi()),
// TODO #2371 don't suggest e.g., .any(|x| f(x)) if we can suggest .any(f)
"this `.fold` can be written more succinctly using another method",
"try",
Expand All @@ -1732,10 +1731,10 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args:
// Check if the first argument to .fold is a suitable literal
if let hir::ExprKind::Lit(ref lit) = fold_args[1].node {
match lit.node {
ast::LitKind::Bool(false) => check_fold_with_op(cx, fold_args, hir::BinOpKind::Or, "any", true),
ast::LitKind::Bool(true) => check_fold_with_op(cx, fold_args, hir::BinOpKind::And, "all", true),
ast::LitKind::Int(0, _) => check_fold_with_op(cx, fold_args, hir::BinOpKind::Add, "sum", false),
ast::LitKind::Int(1, _) => check_fold_with_op(cx, fold_args, hir::BinOpKind::Mul, "product", false),
ast::LitKind::Bool(false) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::Or, "any", true),
ast::LitKind::Bool(true) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::And, "all", true),
ast::LitKind::Int(0, _) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::Add, "sum", false),
ast::LitKind::Int(1, _) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::Mul, "product", false),
_ => (),
}
}
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/unnecessary_fold.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,12 @@ fn unnecessary_fold_should_ignore() {
let _ = [(0..2), (0..3)].iter().fold(1, |a, b| a * b.len());
}

/// Should lint only the line containing the fold
fn unnecessary_fold_over_multiple_lines() {
let _ = (0..3)
.map(|x| x + 1)
.filter(|x| x % 2 == 0)
.any(|x| x > 2);
}

fn main() {}
8 changes: 8 additions & 0 deletions tests/ui/unnecessary_fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,12 @@ fn unnecessary_fold_should_ignore() {
let _ = [(0..2), (0..3)].iter().fold(1, |a, b| a * b.len());
}

/// Should lint only the line containing the fold
fn unnecessary_fold_over_multiple_lines() {
let _ = (0..3)
.map(|x| x + 1)
.filter(|x| x % 2 == 0)
.fold(false, |acc, x| acc || x > 2);
}

fn main() {}
28 changes: 17 additions & 11 deletions tests/ui/unnecessary_fold.stderr
Original file line number Diff line number Diff line change
@@ -1,34 +1,40 @@
error: this `.fold` can be written more succinctly using another method
--> $DIR/unnecessary_fold.rs:8:19
--> $DIR/unnecessary_fold.rs:8:20
|
LL | let _ = (0..3).fold(false, |acc, x| acc || x > 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.any(|x| x > 2)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `any(|x| x > 2)`
|
= note: `-D clippy::unnecessary-fold` implied by `-D warnings`

error: this `.fold` can be written more succinctly using another method
--> $DIR/unnecessary_fold.rs:10:19
--> $DIR/unnecessary_fold.rs:10:20
|
LL | let _ = (0..3).fold(true, |acc, x| acc && x > 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.all(|x| x > 2)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `all(|x| x > 2)`

error: this `.fold` can be written more succinctly using another method
--> $DIR/unnecessary_fold.rs:12:24
--> $DIR/unnecessary_fold.rs:12:25
|
LL | let _: i32 = (0..3).fold(0, |acc, x| acc + x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.sum()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `sum()`

error: this `.fold` can be written more succinctly using another method
--> $DIR/unnecessary_fold.rs:14:24
--> $DIR/unnecessary_fold.rs:14:25
|
LL | let _: i32 = (0..3).fold(1, |acc, x| acc * x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.product()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `product()`

error: this `.fold` can be written more succinctly using another method
--> $DIR/unnecessary_fold.rs:19:40
--> $DIR/unnecessary_fold.rs:19:41
|
LL | let _: bool = (0..3).map(|x| 2 * x).fold(false, |acc, x| acc || x > 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.any(|x| x > 2)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `any(|x| x > 2)`

error: aborting due to 5 previous errors
error: this `.fold` can be written more succinctly using another method
--> $DIR/unnecessary_fold.rs:49:10
|
LL | .fold(false, |acc, x| acc || x > 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `any(|x| x > 2)`

error: aborting due to 6 previous errors

0 comments on commit bcd0e44

Please sign in to comment.