From 86d18b50ed64fa5dd6313d595ad4c53d6eb610a2 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Sun, 11 Sep 2022 12:17:51 +0000 Subject: [PATCH] Fix `unused_peekable` closure and `f(&mut peekable)` false positives --- clippy_lints/src/unused_peekable.rs | 36 +++++++++++++++++------------ tests/ui/unused_peekable.rs | 33 ++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/unused_peekable.rs b/clippy_lints/src/unused_peekable.rs index cfc181e435b..13b08df16a5 100644 --- a/clippy_lints/src/unused_peekable.rs +++ b/clippy_lints/src/unused_peekable.rs @@ -6,6 +6,7 @@ use rustc_hir::lang_items::LangItem; use rustc_hir::{Block, Expr, ExprKind, HirId, Local, Node, PatKind, PathSegment, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -109,8 +110,14 @@ fn new(cx: &'a LateContext<'tcx>, expected_hir_id: HirId) -> Self { } } -impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { - fn visit_expr(&mut self, ex: &'_ Expr<'_>) { +impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { + type NestedFilter = OnlyBodies; + + fn nested_visit_map(&mut self) -> Self::Map { + self.cx.tcx.hir() + } + + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { if self.found_peek_call { return; } @@ -136,12 +143,11 @@ fn visit_expr(&mut self, ex: &'_ Expr<'_>) { return; } - if args.iter().any(|arg| { - matches!(arg.kind, ExprKind::Path(_)) && arg_is_mut_peekable(self.cx, arg) - }) { + if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) { self.found_peek_call = true; - return; } + + return; }, // Catch anything taking a Peekable mutably ExprKind::MethodCall( @@ -190,21 +196,21 @@ fn visit_expr(&mut self, ex: &'_ Expr<'_>) { Node::Local(Local { init: Some(init), .. }) => { if arg_is_mut_peekable(self.cx, init) { self.found_peek_call = true; - return; } - break; + return; }, - Node::Stmt(stmt) => match stmt.kind { - StmtKind::Expr(_) | StmtKind::Semi(_) => {}, - _ => { - self.found_peek_call = true; - return; - }, + Node::Stmt(stmt) => { + match stmt.kind { + StmtKind::Local(_) | StmtKind::Item(_) => self.found_peek_call = true, + StmtKind::Expr(_) | StmtKind::Semi(_) => {}, + } + + return; }, Node::Block(_) | Node::ExprField(_) => {}, _ => { - break; + return; }, } } diff --git a/tests/ui/unused_peekable.rs b/tests/ui/unused_peekable.rs index 153457e3671..7374dfdf92e 100644 --- a/tests/ui/unused_peekable.rs +++ b/tests/ui/unused_peekable.rs @@ -57,12 +57,22 @@ fn takes_peekable(_peek: Peekable>) {} impl PeekableConsumer { fn consume(&self, _: Peekable>) {} fn consume_mut_ref(&self, _: &mut Peekable>) {} + fn consume_assoc(_: Peekable>) {} + fn consume_assoc_mut_ref(_: &mut Peekable>) {} } - let peekable_consumer = PeekableConsumer; - let mut passed_along_to_method = std::iter::empty::().peekable(); - peekable_consumer.consume_mut_ref(&mut passed_along_to_method); - peekable_consumer.consume(passed_along_to_method); + + let peekable = std::iter::empty::().peekable(); + peekable_consumer.consume(peekable); + + let mut peekable = std::iter::empty::().peekable(); + peekable_consumer.consume_mut_ref(&mut peekable); + + let peekable = std::iter::empty::().peekable(); + PeekableConsumer::consume_assoc(peekable); + + let mut peekable = std::iter::empty::().peekable(); + PeekableConsumer::consume_assoc_mut_ref(&mut peekable); // `peek` called in another block let mut peekable_in_block = std::iter::empty::().peekable(); @@ -141,4 +151,19 @@ struct PeekableWrapper { { peekable_last_expr.peek(); } + + let mut peek_in_closure = std::iter::empty::().peekable(); + let _ = || { + let _ = peek_in_closure.peek(); + }; + + trait PeekTrait {} + impl PeekTrait for Peekable where I: Iterator {} + + let mut peekable = std::iter::empty::().peekable(); + let _dyn = &mut peekable as &mut dyn PeekTrait; + + fn takes_dyn(_: &mut dyn PeekTrait) {} + let mut peekable = std::iter::empty::().peekable(); + takes_dyn(&mut peekable); }