Skip to content

Commit d28e72b

Browse files
committed
Better handle method/function calls
1 parent 55e6fac commit d28e72b

File tree

3 files changed

+49
-37
lines changed

3 files changed

+49
-37
lines changed

clippy_lints/src/unused_peekable.rs

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@ use clippy_utils::{fn_def_id, path_to_local_id, paths, peel_ref_operators};
44
use rustc_ast::Mutability;
55
use rustc_hir::intravisit::{walk_expr, Visitor};
66
use rustc_hir::lang_items::LangItem;
7-
use rustc_hir::{Block, Expr, ExprKind, HirId, Local, Node, PatKind, PathSegment, StmtKind};
7+
use rustc_hir::{Block, Expr, ExprKind, HirId, Local, Node, PatKind, StmtKind};
88
use rustc_lint::{LateContext, LateLintPass};
9-
use rustc_middle::ty::Ty;
109
use rustc_session::{declare_tool_lint, impl_lint_pass};
1110

1211
declare_clippy_lint! {
@@ -117,6 +116,10 @@ impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> {
117116

118117
impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
119118
fn visit_expr(&mut self, ex: &'_ Expr<'_>) {
119+
if self.found_peek_call {
120+
return;
121+
}
122+
120123
if path_to_local_id(ex, self.expected_hir_id) {
121124
for (_, node) in self.cx.tcx.hir().parent_iter(ex.hir_id) {
122125
match node {
@@ -138,50 +141,31 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
138141
return;
139142
}
140143

141-
for arg in args.iter().map(|arg| peel_ref_operators(self.cx, arg)) {
142-
if let ExprKind::Path(_) = arg.kind
143-
&& let Some(ty) = self
144-
.cx
145-
.typeck_results()
146-
.expr_ty_opt(arg)
147-
.map(Ty::peel_refs)
148-
&& match_type(self.cx, ty, &paths::PEEKABLE)
149-
{
150-
self.found_peek_call = true;
151-
return;
152-
}
144+
if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) {
145+
self.found_peek_call = true;
146+
return;
153147
}
154148
},
155-
// Peekable::peek()
156-
ExprKind::MethodCall(PathSegment { ident: method_name, .. }, [arg, ..], _) => {
149+
// Catch anything taking a Peekable mutably
150+
ExprKind::MethodCall(_, [arg, ..], _) => {
157151
let arg = peel_ref_operators(self.cx, arg);
158-
let method_name = method_name.name.as_str();
159-
160-
if (method_name == "peek"
161-
|| method_name == "peek_mut"
162-
|| method_name == "next_if"
163-
|| method_name == "next_if_eq")
164-
&& let ExprKind::Path(_) = arg.kind
165-
&& let Some(ty) = self.cx.typeck_results().expr_ty_opt(arg).map(Ty::peel_refs)
166-
&& match_type(self.cx, ty, &paths::PEEKABLE)
167-
{
152+
153+
if arg_is_mut_peekable(self.cx, arg) {
168154
self.found_peek_call = true;
169155
return;
170156
}
171157
},
172-
// Don't bother if moved into a struct
173-
ExprKind::Struct(..) => {
158+
ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => {
159+
},
160+
ExprKind::AddrOf(_, Mutability::Not, _) => return,
161+
_ => {
174162
self.found_peek_call = true;
175163
return;
176164
},
177-
_ => {},
178165
}
179166
},
180167
Node::Local(Local { init: Some(init), .. }) => {
181-
if let Some(ty) = self.cx.typeck_results().expr_ty_opt(init)
182-
&& let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty)
183-
&& match_type(self.cx, ty, &paths::PEEKABLE)
184-
{
168+
if arg_is_mut_peekable(self.cx, init) {
185169
self.found_peek_call = true;
186170
return;
187171
}
@@ -206,3 +190,14 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
206190
walk_expr(self, ex);
207191
}
208192
}
193+
194+
fn arg_is_mut_peekable(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
195+
if let Some(ty) = cx.typeck_results().expr_ty_opt(arg)
196+
&& let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty)
197+
&& match_type(cx, ty, &paths::PEEKABLE)
198+
{
199+
true
200+
} else {
201+
false
202+
}
203+
}

tests/ui/unused_peekable.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ fn invalid() {
3232
let mut peekable_using_iterator_method = std::iter::empty::<u32>().peekable();
3333
peekable_using_iterator_method.next();
3434

35+
// Passed by ref to another function
36+
fn takes_ref(_peek: &Peekable<Empty<u32>>) {}
37+
let passed_along_ref = std::iter::empty::<u32>().peekable();
38+
takes_ref(&passed_along_ref);
39+
3540
let mut peekable_in_for_loop = std::iter::empty::<u32>().peekable();
3641
for x in peekable_in_for_loop {}
3742
}
@@ -43,6 +48,18 @@ fn valid() {
4348
let passed_along = std::iter::empty::<u32>().peekable();
4449
takes_peekable(passed_along);
4550

51+
// Passed to another method
52+
struct PeekableConsumer;
53+
impl PeekableConsumer {
54+
fn consume(&self, _: Peekable<Empty<u32>>) {}
55+
fn consume_mut_ref(&self, _: &mut Peekable<Empty<u32>>) {}
56+
}
57+
58+
let peekable_consumer = PeekableConsumer;
59+
let mut passed_along_to_method = std::iter::empty::<u32>().peekable();
60+
peekable_consumer.consume_mut_ref(&mut passed_along_to_method);
61+
peekable_consumer.consume(passed_along_to_method);
62+
4663
// `peek` called in another block
4764
let mut peekable_in_block = std::iter::empty::<u32>().peekable();
4865
{

tests/ui/unused_peekable.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ LL | let peekable_from_fn = returns_peekable();
3232
= help: consider removing the call to `peekable`
3333

3434
error: `peek` never called on `Peekable` iterator
35-
--> $DIR/unused_peekable.rs:32:13
35+
--> $DIR/unused_peekable.rs:37:9
3636
|
37-
LL | let mut peekable_using_iterator_method = std::iter::empty::<u32>().peekable();
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
37+
LL | let passed_along_ref = std::iter::empty::<u32>().peekable();
38+
| ^^^^^^^^^^^^^^^^
3939
|
4040
= help: consider removing the call to `peekable`
4141

4242
error: `peek` never called on `Peekable` iterator
43-
--> $DIR/unused_peekable.rs:35:13
43+
--> $DIR/unused_peekable.rs:40:13
4444
|
4545
LL | let mut peekable_in_for_loop = std::iter::empty::<u32>().peekable();
4646
| ^^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)