Auto merge of #4382 - jeremystucki:unnecessary_fold_span, r=flip1995

bors · bors · commit 607b8299c5c9 · 2019-08-15T12:57:51.000Z
Change span of unnecessary_fold lint Resolves #4381 changelog: Change linted span of `unnecessary_fold`
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -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;
@@ -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,
@@ -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",
@@ -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),
             _ => (),
         }
     }
diff --git a/tests/ui/unnecessary_fold.fixed b/tests/ui/unnecessary_fold.fixed
@@ -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() {}
diff --git a/tests/ui/unnecessary_fold.rs b/tests/ui/unnecessary_fold.rs
@@ -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() {}
diff --git a/tests/ui/unnecessary_fold.stderr b/tests/ui/unnecessary_fold.stderr
@@ -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
 
