From d28e72ba9074f6b0354a8d09432bddcb11bb5455 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Fri, 5 Aug 2022 13:49:43 -0400 Subject: [PATCH] Better handle method/function calls --- clippy_lints/src/unused_peekable.rs | 61 +++++++++++++---------------- tests/ui/unused_peekable.rs | 17 ++++++++ tests/ui/unused_peekable.stderr | 8 ++-- 3 files changed, 49 insertions(+), 37 deletions(-) diff --git a/clippy_lints/src/unused_peekable.rs b/clippy_lints/src/unused_peekable.rs index 4988ec0eca12..796fd1e7c432 100644 --- a/clippy_lints/src/unused_peekable.rs +++ b/clippy_lints/src/unused_peekable.rs @@ -4,9 +4,8 @@ use clippy_utils::{fn_def_id, path_to_local_id, paths, peel_ref_operators}; use rustc_ast::Mutability; 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_hir::{Block, Expr, ExprKind, HirId, Local, Node, PatKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::Ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; declare_clippy_lint! { @@ -117,6 +116,10 @@ impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> { impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { fn visit_expr(&mut self, ex: &'_ Expr<'_>) { + if self.found_peek_call { + return; + } + if path_to_local_id(ex, self.expected_hir_id) { for (_, node) in self.cx.tcx.hir().parent_iter(ex.hir_id) { match node { @@ -138,50 +141,31 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { return; } - for arg in args.iter().map(|arg| peel_ref_operators(self.cx, arg)) { - if let ExprKind::Path(_) = arg.kind - && let Some(ty) = self - .cx - .typeck_results() - .expr_ty_opt(arg) - .map(Ty::peel_refs) - && match_type(self.cx, ty, &paths::PEEKABLE) - { - self.found_peek_call = true; - return; - } + if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) { + self.found_peek_call = true; + return; } }, - // Peekable::peek() - ExprKind::MethodCall(PathSegment { ident: method_name, .. }, [arg, ..], _) => { + // Catch anything taking a Peekable mutably + ExprKind::MethodCall(_, [arg, ..], _) => { let arg = peel_ref_operators(self.cx, arg); - let method_name = method_name.name.as_str(); - - if (method_name == "peek" - || method_name == "peek_mut" - || method_name == "next_if" - || method_name == "next_if_eq") - && let ExprKind::Path(_) = arg.kind - && let Some(ty) = self.cx.typeck_results().expr_ty_opt(arg).map(Ty::peel_refs) - && match_type(self.cx, ty, &paths::PEEKABLE) - { + + if arg_is_mut_peekable(self.cx, arg) { self.found_peek_call = true; return; } }, - // Don't bother if moved into a struct - ExprKind::Struct(..) => { + ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => { + }, + ExprKind::AddrOf(_, Mutability::Not, _) => return, + _ => { self.found_peek_call = true; return; }, - _ => {}, } }, Node::Local(Local { init: Some(init), .. }) => { - if let Some(ty) = self.cx.typeck_results().expr_ty_opt(init) - && let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty) - && match_type(self.cx, ty, &paths::PEEKABLE) - { + if arg_is_mut_peekable(self.cx, init) { self.found_peek_call = true; return; } @@ -206,3 +190,14 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> { walk_expr(self, ex); } } + +fn arg_is_mut_peekable(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool { + if let Some(ty) = cx.typeck_results().expr_ty_opt(arg) + && let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty) + && match_type(cx, ty, &paths::PEEKABLE) + { + true + } else { + false + } +} diff --git a/tests/ui/unused_peekable.rs b/tests/ui/unused_peekable.rs index 12cab9956219..3427a7724867 100644 --- a/tests/ui/unused_peekable.rs +++ b/tests/ui/unused_peekable.rs @@ -32,6 +32,11 @@ fn invalid() { let mut peekable_using_iterator_method = std::iter::empty::().peekable(); peekable_using_iterator_method.next(); + // Passed by ref to another function + fn takes_ref(_peek: &Peekable>) {} + let passed_along_ref = std::iter::empty::().peekable(); + takes_ref(&passed_along_ref); + let mut peekable_in_for_loop = std::iter::empty::().peekable(); for x in peekable_in_for_loop {} } @@ -43,6 +48,18 @@ fn valid() { let passed_along = std::iter::empty::().peekable(); takes_peekable(passed_along); + // Passed to another method + struct PeekableConsumer; + impl PeekableConsumer { + fn consume(&self, _: Peekable>) {} + fn consume_mut_ref(&self, _: &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); + // `peek` called in another block let mut peekable_in_block = std::iter::empty::().peekable(); { diff --git a/tests/ui/unused_peekable.stderr b/tests/ui/unused_peekable.stderr index bd087f56e4ce..4794bfdc1ec7 100644 --- a/tests/ui/unused_peekable.stderr +++ b/tests/ui/unused_peekable.stderr @@ -32,15 +32,15 @@ LL | let peekable_from_fn = returns_peekable(); = help: consider removing the call to `peekable` error: `peek` never called on `Peekable` iterator - --> $DIR/unused_peekable.rs:32:13 + --> $DIR/unused_peekable.rs:37:9 | -LL | let mut peekable_using_iterator_method = std::iter::empty::().peekable(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let passed_along_ref = std::iter::empty::().peekable(); + | ^^^^^^^^^^^^^^^^ | = help: consider removing the call to `peekable` error: `peek` never called on `Peekable` iterator - --> $DIR/unused_peekable.rs:35:13 + --> $DIR/unused_peekable.rs:40:13 | LL | let mut peekable_in_for_loop = std::iter::empty::().peekable(); | ^^^^^^^^^^^^^^^^^^^^