Skip to content

Commit 5ad13b6

Browse files
Serial-ATASerial-ATA
committed
Better handle method/function calls
1 parent 55e6fac commit 5ad13b6

File tree

3 files changed

+52
-36
lines changed

3 files changed

+52
-36
lines changed

‎clippy_lints/src/unused_peekable.rs

Lines changed: 31 additions & 32 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,35 @@ 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| {
145+
matches!(arg.kind, ExprKind::Path(_)) && arg_is_mut_peekable(self.cx, arg)
146+
}) {
147+
self.found_peek_call = true;
148+
return;
153149
}
154150
},
155-
// Peekable::peek()
156-
ExprKind::MethodCall(PathSegment { ident: method_name, .. }, [arg, ..], _) => {
151+
// Catch anything taking a Peekable mutably
152+
ExprKind::MethodCall(_, [arg, ..], _) => {
157153
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)
154+
155+
if let ExprKind::Path(_) = arg.kind
156+
&& arg_is_mut_peekable(self.cx, arg)
167157
{
168158
self.found_peek_call = true;
169159
return;
170160
}
171161
},
172-
// Don't bother if moved into a struct
173-
ExprKind::Struct(..) => {
162+
ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => {
163+
},
164+
ExprKind::AddrOf(_, Mutability::Not, _) => return,
165+
_ => {
174166
self.found_peek_call = true;
175167
return;
176168
},
177-
_ => {},
178169
}
179170
},
180171
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-
{
172+
if arg_is_mut_peekable(self.cx, init) {
185173
self.found_peek_call = true;
186174
return;
187175
}
@@ -206,3 +194,14 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
206194
walk_expr(self, ex);
207195
}
208196
}
197+
198+
fn arg_is_mut_peekable(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
199+
if let Some(ty) = cx.typeck_results().expr_ty_opt(arg)
200+
&& let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty)
201+
&& match_type(cx, ty, &paths::PEEKABLE)
202+
{
203+
true
204+
} else {
205+
false
206+
}
207+
}

‎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)