Skip to content

Commit aaa8cea

Browse files
Serial-ATASerial-ATA
committed
Better handle method/function calls
1 parent 5986708 commit aaa8cea

File tree

4 files changed

+76
-36
lines changed

4 files changed

+76
-36
lines changed

‎clippy_lints/src/lib.register_suspicious.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
3232
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
3333
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
3434
LintId::of(swap_ptr_to_ref::SWAP_PTR_TO_REF),
35-
LintId::of(write::POSITIONAL_NAMED_FORMAT_PARAMETERS),
3635
LintId::of(unused_peekable::UNUSED_PEEKABLE),
36+
LintId::of(write::POSITIONAL_NAMED_FORMAT_PARAMETERS),
3737
])

‎clippy_lints/src/unused_peekable.rs

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::ty::{match_type, peel_mid_ty_refs_is_mutable};
3-
use clippy_utils::{fn_def_id, path_to_local_id, paths, peel_ref_operators};
3+
use clippy_utils::{fn_def_id, is_trait_method, 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;
77
use rustc_hir::{Block, Expr, ExprKind, HirId, Local, Node, PatKind, PathSegment, StmtKind};
88
use rustc_lint::{LateContext, LateLintPass};
9-
use rustc_middle::ty::Ty;
109
use rustc_session::{declare_tool_lint, impl_lint_pass};
10+
use rustc_span::sym;
1111

1212
declare_clippy_lint! {
1313
/// ### What it does
@@ -117,6 +117,10 @@ impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> {
117117

118118
impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
119119
fn visit_expr(&mut self, ex: &'_ Expr<'_>) {
120+
if self.found_peek_call {
121+
return;
122+
}
123+
120124
if path_to_local_id(ex, self.expected_hir_id) {
121125
for (_, node) in self.cx.tcx.hir().parent_iter(ex.hir_id) {
122126
match node {
@@ -138,50 +142,50 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
138142
return;
139143
}
140144

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-
}
145+
if args.iter().any(|arg| {
146+
matches!(arg.kind, ExprKind::Path(_)) && arg_is_mut_peekable(self.cx, arg)
147+
}) {
148+
self.found_peek_call = true;
149+
return;
153150
}
154151
},
155-
// Peekable::peek()
156-
ExprKind::MethodCall(PathSegment { ident: method_name, .. }, [arg, ..], _) => {
157-
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)
152+
// Catch anything taking a Peekable mutably
153+
ExprKind::MethodCall(
154+
PathSegment { ident: method_name, .. },
155+
[self_arg, remaining_args @ ..],
156+
_,
157+
) => {
158+
// `Peekable` methods
159+
if matches!(
160+
method_name.name.as_str(),
161+
"peek" | "peek_mut" | "next_if" | "next_if_eq"
162+
) && arg_is_mut_peekable(self.cx, self_arg)
163+
{
164+
self.found_peek_call = true;
165+
return;
166+
}
167+
168+
// foo.some_method() excluding Iterator methods
169+
if remaining_args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg))
170+
&& !is_trait_method(self.cx, expr, sym::Iterator)
167171
{
168172
self.found_peek_call = true;
169173
return;
170174
}
175+
176+
return;
171177
},
172-
// Don't bother if moved into a struct
173-
ExprKind::Struct(..) => {
178+
ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => {
179+
},
180+
ExprKind::AddrOf(_, Mutability::Not, _) => return,
181+
_ => {
174182
self.found_peek_call = true;
175183
return;
176184
},
177-
_ => {},
178185
}
179186
},
180187
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-
{
188+
if arg_is_mut_peekable(self.cx, init) {
185189
self.found_peek_call = true;
186190
return;
187191
}
@@ -206,3 +210,14 @@ impl<'tcx> Visitor<'_> for PeekableVisitor<'_, 'tcx> {
206210
walk_expr(self, ex);
207211
}
208212
}
213+
214+
fn arg_is_mut_peekable(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool {
215+
if let Some(ty) = cx.typeck_results().expr_ty_opt(arg)
216+
&& let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty)
217+
&& match_type(cx, ty, &paths::PEEKABLE)
218+
{
219+
true
220+
} else {
221+
false
222+
}
223+
}

‎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: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,20 @@ LL | let mut peekable_using_iterator_method = std::iter::empty::<u32>().peek
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:37:9
44+
|
45+
LL | let passed_along_ref = std::iter::empty::<u32>().peekable();
46+
| ^^^^^^^^^^^^^^^^
47+
|
48+
= help: consider removing the call to `peekable`
49+
50+
error: `peek` never called on `Peekable` iterator
51+
--> $DIR/unused_peekable.rs:40:13
4452
|
4553
LL | let mut peekable_in_for_loop = std::iter::empty::<u32>().peekable();
4654
| ^^^^^^^^^^^^^^^^^^^^
4755
|
4856
= help: consider removing the call to `peekable`
4957

50-
error: aborting due to 6 previous errors
58+
error: aborting due to 7 previous errors
5159

0 commit comments

Comments
 (0)