mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-24 05:33:27 +00:00
Auto merge of #9258 - Serial-ATA:unused-peekable, r=Alexendoo
Add [`unused_peekable`] lint changelog: Add [`unused_peekable`] lint closes: #854
This commit is contained in:
commit
2091142f5d
10 changed files with 444 additions and 1 deletions
|
@ -4152,6 +4152,7 @@ Released 2018-09-13
|
|||
[`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect
|
||||
[`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount
|
||||
[`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label
|
||||
[`unused_peekable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_peekable
|
||||
[`unused_rounding`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_rounding
|
||||
[`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self
|
||||
[`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit
|
||||
|
|
|
@ -339,6 +339,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
|
|||
LintId::of(unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS),
|
||||
LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
|
||||
LintId::of(unused_io_amount::UNUSED_IO_AMOUNT),
|
||||
LintId::of(unused_peekable::UNUSED_PEEKABLE),
|
||||
LintId::of(unused_unit::UNUSED_UNIT),
|
||||
LintId::of(unwrap::PANICKING_UNWRAP),
|
||||
LintId::of(unwrap::UNNECESSARY_UNWRAP),
|
||||
|
|
|
@ -575,6 +575,7 @@ store.register_lints(&[
|
|||
unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
|
||||
unused_async::UNUSED_ASYNC,
|
||||
unused_io_amount::UNUSED_IO_AMOUNT,
|
||||
unused_peekable::UNUSED_PEEKABLE,
|
||||
unused_rounding::UNUSED_ROUNDING,
|
||||
unused_self::UNUSED_SELF,
|
||||
unused_unit::UNUSED_UNIT,
|
||||
|
|
|
@ -32,5 +32,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
|
|||
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
|
||||
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
|
||||
LintId::of(swap_ptr_to_ref::SWAP_PTR_TO_REF),
|
||||
LintId::of(unused_peekable::UNUSED_PEEKABLE),
|
||||
LintId::of(write::POSITIONAL_NAMED_FORMAT_PARAMETERS),
|
||||
])
|
||||
|
|
|
@ -381,6 +381,7 @@ mod unnested_or_patterns;
|
|||
mod unsafe_removed_from_name;
|
||||
mod unused_async;
|
||||
mod unused_io_amount;
|
||||
mod unused_peekable;
|
||||
mod unused_rounding;
|
||||
mod unused_self;
|
||||
mod unused_unit;
|
||||
|
@ -894,6 +895,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
|
||||
store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
|
||||
store.register_late_pass(|| Box::new(manual_empty_string_creations::ManualEmptyStringCreations));
|
||||
store.register_late_pass(|| Box::new(unused_peekable::UnusedPeekable));
|
||||
// add lints here, do not remove this comment, it's used in `new_lint`
|
||||
}
|
||||
|
||||
|
|
225
clippy_lints/src/unused_peekable.rs
Normal file
225
clippy_lints/src/unused_peekable.rs
Normal file
|
@ -0,0 +1,225 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_help;
|
||||
use clippy_utils::ty::{match_type, peel_mid_ty_refs_is_mutable};
|
||||
use clippy_utils::{fn_def_id, is_trait_method, 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_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::sym;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
/// Checks for the creation of a `peekable` iterator that is never `.peek()`ed
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
/// Creating a peekable iterator without using any of its methods is likely a mistake,
|
||||
/// or just a leftover after a refactor.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// let collection = vec![1, 2, 3];
|
||||
/// let iter = collection.iter().peekable();
|
||||
///
|
||||
/// for item in iter {
|
||||
/// // ...
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// let collection = vec![1, 2, 3];
|
||||
/// let iter = collection.iter();
|
||||
///
|
||||
/// for item in iter {
|
||||
/// // ...
|
||||
/// }
|
||||
/// ```
|
||||
#[clippy::version = "1.64.0"]
|
||||
pub UNUSED_PEEKABLE,
|
||||
suspicious,
|
||||
"creating a peekable iterator without using any of its methods"
|
||||
}
|
||||
|
||||
declare_lint_pass!(UnusedPeekable => [UNUSED_PEEKABLE]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for UnusedPeekable {
|
||||
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
|
||||
// Don't lint `Peekable`s returned from a block
|
||||
if let Some(expr) = block.expr
|
||||
&& let Some(ty) = cx.typeck_results().expr_ty_opt(peel_ref_operators(cx, expr))
|
||||
&& match_type(cx, ty, &paths::PEEKABLE)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
for (idx, stmt) in block.stmts.iter().enumerate() {
|
||||
if !stmt.span.from_expansion()
|
||||
&& let StmtKind::Local(local) = stmt.kind
|
||||
&& let PatKind::Binding(_, binding, ident, _) = local.pat.kind
|
||||
&& let Some(init) = local.init
|
||||
&& !init.span.from_expansion()
|
||||
&& let Some(ty) = cx.typeck_results().expr_ty_opt(init)
|
||||
&& let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty)
|
||||
&& match_type(cx, ty, &paths::PEEKABLE)
|
||||
{
|
||||
let mut vis = PeekableVisitor::new(cx, binding);
|
||||
|
||||
if idx + 1 == block.stmts.len() && block.expr.is_none() {
|
||||
return;
|
||||
}
|
||||
|
||||
for stmt in &block.stmts[idx..] {
|
||||
vis.visit_stmt(stmt);
|
||||
}
|
||||
|
||||
if let Some(expr) = block.expr {
|
||||
vis.visit_expr(expr);
|
||||
}
|
||||
|
||||
if !vis.found_peek_call {
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
UNUSED_PEEKABLE,
|
||||
ident.span,
|
||||
"`peek` never called on `Peekable` iterator",
|
||||
None,
|
||||
"consider removing the call to `peekable`"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct PeekableVisitor<'a, 'tcx> {
|
||||
cx: &'a LateContext<'tcx>,
|
||||
expected_hir_id: HirId,
|
||||
found_peek_call: bool,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> {
|
||||
fn new(cx: &'a LateContext<'tcx>, expected_hir_id: HirId) -> Self {
|
||||
Self {
|
||||
cx,
|
||||
expected_hir_id,
|
||||
found_peek_call: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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 {
|
||||
Node::Expr(expr) => {
|
||||
match expr.kind {
|
||||
// some_function(peekable)
|
||||
//
|
||||
// If the Peekable is passed to a function, stop
|
||||
ExprKind::Call(_, args) => {
|
||||
if let Some(func_did) = fn_def_id(self.cx, expr)
|
||||
&& let Ok(into_iter_did) = self
|
||||
.cx
|
||||
.tcx
|
||||
.lang_items()
|
||||
.require(LangItem::IntoIterIntoIter)
|
||||
&& func_did == into_iter_did
|
||||
{
|
||||
// Probably a for loop desugar, stop searching
|
||||
return;
|
||||
}
|
||||
|
||||
if args.iter().any(|arg| {
|
||||
matches!(arg.kind, ExprKind::Path(_)) && arg_is_mut_peekable(self.cx, arg)
|
||||
}) {
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
}
|
||||
},
|
||||
// Catch anything taking a Peekable mutably
|
||||
ExprKind::MethodCall(
|
||||
PathSegment {
|
||||
ident: method_name_ident,
|
||||
..
|
||||
},
|
||||
[self_arg, remaining_args @ ..],
|
||||
_,
|
||||
) => {
|
||||
let method_name = method_name_ident.name.as_str();
|
||||
|
||||
// `Peekable` methods
|
||||
if matches!(method_name, "peek" | "peek_mut" | "next_if" | "next_if_eq")
|
||||
&& arg_is_mut_peekable(self.cx, self_arg)
|
||||
{
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
}
|
||||
|
||||
// foo.some_method() excluding Iterator methods
|
||||
if remaining_args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg))
|
||||
&& !is_trait_method(self.cx, expr, sym::Iterator)
|
||||
{
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
}
|
||||
|
||||
// foo.by_ref(), keep checking for `peek`
|
||||
if method_name == "by_ref" {
|
||||
continue;
|
||||
}
|
||||
|
||||
return;
|
||||
},
|
||||
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 arg_is_mut_peekable(self.cx, init) {
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
}
|
||||
|
||||
break;
|
||||
},
|
||||
Node::Stmt(stmt) => match stmt.kind {
|
||||
StmtKind::Expr(_) | StmtKind::Semi(_) => {},
|
||||
_ => {
|
||||
self.found_peek_call = true;
|
||||
return;
|
||||
},
|
||||
},
|
||||
Node::Block(_) => {},
|
||||
_ => {
|
||||
break;
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
}
|
|
@ -95,6 +95,7 @@ pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwL
|
|||
pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 3] = ["lock_api", "rwlock", "RwLockWriteGuard"];
|
||||
pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
|
||||
pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
|
||||
pub const PEEKABLE: [&str; 5] = ["core", "iter", "adapters", "peekable", "Peekable"];
|
||||
pub const PERMISSIONS: [&str; 3] = ["std", "fs", "Permissions"];
|
||||
#[cfg_attr(not(unix), allow(clippy::invalid_paths))]
|
||||
pub const PERMISSIONS_FROM_MODE: [&str; 6] = ["std", "os", "unix", "fs", "PermissionsExt", "from_mode"];
|
||||
|
|
|
@ -410,7 +410,7 @@ pub fn peel_mid_ty_refs(ty: Ty<'_>) -> (Ty<'_>, usize) {
|
|||
peel(ty, 0)
|
||||
}
|
||||
|
||||
/// Peels off all references on the type.Returns the underlying type, the number of references
|
||||
/// Peels off all references on the type. Returns the underlying type, the number of references
|
||||
/// removed, and whether the pointer is ultimately mutable or not.
|
||||
pub fn peel_mid_ty_refs_is_mutable(ty: Ty<'_>) -> (Ty<'_>, usize, Mutability) {
|
||||
fn f(ty: Ty<'_>, count: usize, mutability: Mutability) -> (Ty<'_>, usize, Mutability) {
|
||||
|
|
144
tests/ui/unused_peekable.rs
Normal file
144
tests/ui/unused_peekable.rs
Normal file
|
@ -0,0 +1,144 @@
|
|||
#![warn(clippy::unused_peekable)]
|
||||
#![allow(clippy::no_effect)]
|
||||
|
||||
use std::iter::Empty;
|
||||
use std::iter::Peekable;
|
||||
|
||||
fn main() {
|
||||
invalid();
|
||||
valid();
|
||||
}
|
||||
|
||||
#[allow(clippy::unused_unit)]
|
||||
fn invalid() {
|
||||
let peekable = std::iter::empty::<u32>().peekable();
|
||||
|
||||
// Only lint `new_local`
|
||||
let old_local = std::iter::empty::<u32>().peekable();
|
||||
let new_local = old_local;
|
||||
|
||||
// Behind mut ref
|
||||
let mut by_mut_ref_test = std::iter::empty::<u32>().peekable();
|
||||
let by_mut_ref = &mut by_mut_ref_test;
|
||||
|
||||
// Explicitly returns `Peekable`
|
||||
fn returns_peekable() -> Peekable<Empty<u32>> {
|
||||
std::iter::empty().peekable()
|
||||
}
|
||||
|
||||
let peekable_from_fn = returns_peekable();
|
||||
|
||||
// Using a method not exclusive to `Peekable`
|
||||
let mut peekable_using_iterator_method = std::iter::empty::<u32>().peekable();
|
||||
peekable_using_iterator_method.next();
|
||||
|
||||
// Passed by ref to another function
|
||||
fn takes_ref(_peek: &Peekable<Empty<u32>>) {}
|
||||
let passed_along_ref = std::iter::empty::<u32>().peekable();
|
||||
takes_ref(&passed_along_ref);
|
||||
|
||||
// `by_ref` without `peek`
|
||||
let mut by_ref_test = std::iter::empty::<u32>().peekable();
|
||||
let _by_ref = by_ref_test.by_ref();
|
||||
|
||||
let mut peekable_in_for_loop = std::iter::empty::<u32>().peekable();
|
||||
for x in peekable_in_for_loop {}
|
||||
}
|
||||
|
||||
fn valid() {
|
||||
fn takes_peekable(_peek: Peekable<Empty<u32>>) {}
|
||||
|
||||
// Passed to another function
|
||||
let passed_along = std::iter::empty::<u32>().peekable();
|
||||
takes_peekable(passed_along);
|
||||
|
||||
// Passed to another method
|
||||
struct PeekableConsumer;
|
||||
impl PeekableConsumer {
|
||||
fn consume(&self, _: Peekable<Empty<u32>>) {}
|
||||
fn consume_mut_ref(&self, _: &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);
|
||||
|
||||
// `peek` called in another block
|
||||
let mut peekable_in_block = std::iter::empty::<u32>().peekable();
|
||||
{
|
||||
peekable_in_block.peek();
|
||||
}
|
||||
|
||||
// Check the other `Peekable` methods :)
|
||||
{
|
||||
let mut peekable_with_peek_mut = std::iter::empty::<u32>().peekable();
|
||||
peekable_with_peek_mut.peek_mut();
|
||||
|
||||
let mut peekable_with_next_if = std::iter::empty::<u32>().peekable();
|
||||
peekable_with_next_if.next_if(|_| true);
|
||||
|
||||
let mut peekable_with_next_if_eq = std::iter::empty::<u32>().peekable();
|
||||
peekable_with_next_if_eq.next_if_eq(&3);
|
||||
}
|
||||
|
||||
let mut peekable_in_closure = std::iter::empty::<u32>().peekable();
|
||||
let call_peek = |p: &mut Peekable<Empty<u32>>| {
|
||||
p.peek();
|
||||
};
|
||||
call_peek(&mut peekable_in_closure);
|
||||
|
||||
// From a macro
|
||||
macro_rules! make_me_a_peekable_please {
|
||||
() => {
|
||||
std::iter::empty::<u32>().peekable()
|
||||
};
|
||||
}
|
||||
|
||||
let _unsuspecting_macro_user = make_me_a_peekable_please!();
|
||||
|
||||
// Generic Iterator returned
|
||||
fn return_an_iter() -> impl Iterator<Item = u32> {
|
||||
std::iter::empty::<u32>().peekable()
|
||||
}
|
||||
|
||||
let _unsuspecting_user = return_an_iter();
|
||||
|
||||
// Call `peek` in a macro
|
||||
macro_rules! peek_iter {
|
||||
($iter:ident) => {
|
||||
$iter.peek();
|
||||
};
|
||||
}
|
||||
|
||||
let mut peek_in_macro = std::iter::empty::<u32>().peekable();
|
||||
peek_iter!(peek_in_macro);
|
||||
|
||||
// Behind mut ref
|
||||
let mut by_mut_ref_test = std::iter::empty::<u32>().peekable();
|
||||
let by_mut_ref = &mut by_mut_ref_test;
|
||||
by_mut_ref.peek();
|
||||
|
||||
// Behind ref
|
||||
let mut by_ref_test = std::iter::empty::<u32>().peekable();
|
||||
let by_ref = &by_ref_test;
|
||||
by_ref_test.peek();
|
||||
|
||||
// In struct
|
||||
struct PeekableWrapper {
|
||||
f: Peekable<Empty<u32>>,
|
||||
}
|
||||
|
||||
let struct_test = std::iter::empty::<u32>().peekable();
|
||||
PeekableWrapper { f: struct_test };
|
||||
|
||||
// `by_ref` before `peek`
|
||||
let mut by_ref_test = std::iter::empty::<u32>().peekable();
|
||||
let peeked_val = by_ref_test.by_ref().peek();
|
||||
|
||||
// `peek` called in another block as the last expression
|
||||
let mut peekable_last_expr = std::iter::empty::<u32>().peekable();
|
||||
{
|
||||
peekable_last_expr.peek();
|
||||
}
|
||||
}
|
67
tests/ui/unused_peekable.stderr
Normal file
67
tests/ui/unused_peekable.stderr
Normal file
|
@ -0,0 +1,67 @@
|
|||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:14:9
|
||||
|
|
||||
LL | let peekable = std::iter::empty::<u32>().peekable();
|
||||
| ^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::unused-peekable` implied by `-D warnings`
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:18:9
|
||||
|
|
||||
LL | let new_local = old_local;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:22:9
|
||||
|
|
||||
LL | let by_mut_ref = &mut by_mut_ref_test;
|
||||
| ^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:29:9
|
||||
|
|
||||
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
|
||||
|
|
||||
LL | let mut peekable_using_iterator_method = std::iter::empty::<u32>().peekable();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:37:9
|
||||
|
|
||||
LL | let passed_along_ref = std::iter::empty::<u32>().peekable();
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:42:9
|
||||
|
|
||||
LL | let _by_ref = by_ref_test.by_ref();
|
||||
| ^^^^^^^
|
||||
|
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: `peek` never called on `Peekable` iterator
|
||||
--> $DIR/unused_peekable.rs:44:13
|
||||
|
|
||||
LL | let mut peekable_in_for_loop = std::iter::empty::<u32>().peekable();
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing the call to `peekable`
|
||||
|
||||
error: aborting due to 8 previous errors
|
||||
|
Loading…
Reference in a new issue