From d3c06f7252fdca30b19aa6ff8ecf63c86675676e Mon Sep 17 00:00:00 2001 From: Joshua Holmer Date: Fri, 12 Oct 2018 10:26:55 -0400 Subject: [PATCH 1/3] Exclude pattern guards from unnecessary_fold lint Methods like `Iterator::any` borrow the iterator mutably, which is not allowed within a pattern guard and will fail to compile. This commit prevents clippy from suggesting this type of change. Closes #3069 --- clippy_lints/src/methods/mod.rs | 18 ++++++++++++++++++ tests/ui/unnecessary_fold.rs | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7c15eb677cc..66c94746f5a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -9,6 +9,7 @@ use crate::rustc::hir; +use crate::rustc::hir::{ExprKind, Guard, Node}; use crate::rustc::hir::def::Def; use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass}; use crate::rustc::ty::{self, Ty}; @@ -1428,6 +1429,23 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: return; } + // `Iterator::any` cannot be used within a pattern guard + // See https://github.com/rust-lang-nursery/rust-clippy/issues/3069 + if_chain! { + if let Some(fold_parent) = cx.tcx.hir.find(cx.tcx.hir.get_parent_node(expr.id)); + if let Node::Expr(fold_parent) = fold_parent; + if let ExprKind::Match(_, ref arms, _) = fold_parent.node; + if arms.iter().any(|arm| { + if let Some(Guard::If(ref guard)) = arm.guard { + return guard.id == expr.id; + } + false + }); + then { + return; + } + } + assert!(fold_args.len() == 3, "Expected fold_args to have three entries - the receiver, the initial value and the closure"); diff --git a/tests/ui/unnecessary_fold.rs b/tests/ui/unnecessary_fold.rs index e8d84ecea8c..3b70602d4ad 100644 --- a/tests/ui/unnecessary_fold.rs +++ b/tests/ui/unnecessary_fold.rs @@ -45,6 +45,13 @@ fn unnecessary_fold_should_ignore() { 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()); + + // Because `any` takes the iterator as a mutable reference, + // it cannot be used in a pattern guard, and we must use `fold`. + match 1 { + _ if (0..3).fold(false, |acc, x| acc || x > 2) => {} + _ => {} + } } fn main() {} From 863c8e26fc66e856456cb05e75e1d6c821a04ec6 Mon Sep 17 00:00:00 2001 From: Joshua Holmer Date: Fri, 12 Oct 2018 13:15:55 -0400 Subject: [PATCH 2/3] Revert "Exclude pattern guards from unnecessary_fold lint" This reverts commit d3c06f7252fdca30b19aa6ff8ecf63c86675676e. --- clippy_lints/src/methods/mod.rs | 18 ------------------ tests/ui/unnecessary_fold.rs | 7 ------- 2 files changed, 25 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 66c94746f5a..7c15eb677cc 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -9,7 +9,6 @@ use crate::rustc::hir; -use crate::rustc::hir::{ExprKind, Guard, Node}; use crate::rustc::hir::def::Def; use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass}; use crate::rustc::ty::{self, Ty}; @@ -1429,23 +1428,6 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: return; } - // `Iterator::any` cannot be used within a pattern guard - // See https://github.com/rust-lang-nursery/rust-clippy/issues/3069 - if_chain! { - if let Some(fold_parent) = cx.tcx.hir.find(cx.tcx.hir.get_parent_node(expr.id)); - if let Node::Expr(fold_parent) = fold_parent; - if let ExprKind::Match(_, ref arms, _) = fold_parent.node; - if arms.iter().any(|arm| { - if let Some(Guard::If(ref guard)) = arm.guard { - return guard.id == expr.id; - } - false - }); - then { - return; - } - } - assert!(fold_args.len() == 3, "Expected fold_args to have three entries - the receiver, the initial value and the closure"); diff --git a/tests/ui/unnecessary_fold.rs b/tests/ui/unnecessary_fold.rs index 3b70602d4ad..e8d84ecea8c 100644 --- a/tests/ui/unnecessary_fold.rs +++ b/tests/ui/unnecessary_fold.rs @@ -45,13 +45,6 @@ fn unnecessary_fold_should_ignore() { 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()); - - // Because `any` takes the iterator as a mutable reference, - // it cannot be used in a pattern guard, and we must use `fold`. - match 1 { - _ if (0..3).fold(false, |acc, x| acc || x > 2) => {} - _ => {} - } } fn main() {} From 33847b579eb5479f888a0caae37ebc469dd9ccd2 Mon Sep 17 00:00:00 2001 From: Joshua Holmer Date: Tue, 16 Oct 2018 09:04:02 -0400 Subject: [PATCH 3/3] Update known problems for unnecessary_fold --- clippy_lints/src/methods/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7c15eb677cc..6fc563a42ea 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -688,7 +688,8 @@ declare_clippy_lint! { /// /// **Why is this bad?** Readability. /// -/// **Known problems:** None. +/// **Known problems:** False positive in pattern guards. Will be resolved once +/// non-lexical lifetimes are stable. /// /// **Example:** /// ```rust