Skip to content

Commit

Permalink
Auto merge of rust-lang#9465 - Alexendoo:peekable-fp, r=flip1995
Browse files Browse the repository at this point in the history
Fix `unused_peekable` closure and `f(&mut peekable)` false positives

changelog: Fix [`unused_peekable`] false positive when peeked in a closure or called as `f(&mut peekable)`

The `return`/`break` changes aren't part of the fix, they allow an earlier return in some cases. `break` is replaced with `return` for style purposes as they do the same thing in this case

Fixes rust-lang#9456
Fixes rust-lang#9462
  • Loading branch information
bors committed Sep 14, 2022
2 parents 826a893 + 86d18b5 commit 9c9aa92
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 19 deletions.
36 changes: 21 additions & 15 deletions clippy_lints/src/unused_peekable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_hir::intravisit::{walk_expr, Visitor};
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;

Expand Down Expand Up @@ -109,8 +110,14 @@ impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> {
}
}

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;
}
Expand All @@ -136,12 +143,11 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
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(
Expand Down Expand Up @@ -190,21 +196,21 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
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;
},
}
}
Expand Down
33 changes: 29 additions & 4 deletions tests/ui/unused_peekable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,22 @@ fn valid() {
impl PeekableConsumer {
fn consume(&self, _: Peekable<Empty<u32>>) {}
fn consume_mut_ref(&self, _: &mut Peekable<Empty<u32>>) {}
fn consume_assoc(_: Peekable<Empty<u32>>) {}
fn consume_assoc_mut_ref(_: &mut Peekable<Empty<u32>>) {}
}

let peekable_consumer = PeekableConsumer;
let mut passed_along_to_method = std::iter::empty::<u32>().peekable();
peekable_consumer.consume_mut_ref(&mut passed_along_to_method);
peekable_consumer.consume(passed_along_to_method);

let peekable = std::iter::empty::<u32>().peekable();
peekable_consumer.consume(peekable);

let mut peekable = std::iter::empty::<u32>().peekable();
peekable_consumer.consume_mut_ref(&mut peekable);

let peekable = std::iter::empty::<u32>().peekable();
PeekableConsumer::consume_assoc(peekable);

let mut peekable = std::iter::empty::<u32>().peekable();
PeekableConsumer::consume_assoc_mut_ref(&mut peekable);

// `peek` called in another block
let mut peekable_in_block = std::iter::empty::<u32>().peekable();
Expand Down Expand Up @@ -141,4 +151,19 @@ fn valid() {
{
peekable_last_expr.peek();
}

let mut peek_in_closure = std::iter::empty::<u32>().peekable();
let _ = || {
let _ = peek_in_closure.peek();
};

trait PeekTrait {}
impl<I> PeekTrait for Peekable<I> where I: Iterator {}

let mut peekable = std::iter::empty::<u32>().peekable();
let _dyn = &mut peekable as &mut dyn PeekTrait;

fn takes_dyn(_: &mut dyn PeekTrait) {}
let mut peekable = std::iter::empty::<u32>().peekable();
takes_dyn(&mut peekable);
}

0 comments on commit 9c9aa92

Please sign in to comment.