Suggest into_iter() over drain(..)

Add doc

Add description

iter_with_drain dogfood

Disable emiting on struct field.

Fix clippy

Add eq_path for SpanlessEq

Fix tests

Better error message

Fix doc test

Fix version

Apply suggestions
This commit is contained in:
Liu Dingming
2022-02-13 03:14:04 +08:00
parent 6e211eac7c
commit 6cc2eeaa56
16 changed files with 276 additions and 14 deletions

View File

@@ -164,6 +164,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::ITER_NTH_ZERO),
LintId::of(methods::ITER_OVEREAGER_CLONED),
LintId::of(methods::ITER_SKIP_NEXT),
LintId::of(methods::ITER_WITH_DRAIN),
LintId::of(methods::MANUAL_FILTER_MAP),
LintId::of(methods::MANUAL_FIND_MAP),
LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),

View File

@@ -299,6 +299,7 @@ store.register_lints(&[
methods::ITER_NTH_ZERO,
methods::ITER_OVEREAGER_CLONED,
methods::ITER_SKIP_NEXT,
methods::ITER_WITH_DRAIN,
methods::MANUAL_FILTER_MAP,
methods::MANUAL_FIND_MAP,
methods::MANUAL_SATURATING_ARITHMETIC,

View File

@@ -16,6 +16,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
LintId::of(methods::EXTEND_WITH_DRAIN),
LintId::of(methods::ITER_NTH),
LintId::of(methods::ITER_OVEREAGER_CLONED),
LintId::of(methods::ITER_WITH_DRAIN),
LintId::of(methods::MANUAL_STR_REPEAT),
LintId::of(methods::OR_FUN_CALL),
LintId::of(methods::SINGLE_CHAR_PATTERN),

View File

@@ -0,0 +1,72 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_integer_const;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{
higher::{self, Range},
SpanlessEq,
};
use rustc_ast::ast::RangeLimits;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::LateContext;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::Span;
use super::ITER_WITH_DRAIN;
const DRAIN_TYPES: &[Symbol] = &[sym::Vec, sym::VecDeque];
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: &Expr<'_>) {
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
if let Some(drained_type) = DRAIN_TYPES.iter().find(|&&sym| is_type_diagnostic_item(cx, ty, sym)) {
// Refuse to emit `into_iter` suggestion on draining struct fields due
// to the strong possibility of processing unmovable field.
if let ExprKind::Field(..) = recv.kind {
return;
}
if let Some(range) = higher::Range::hir(arg) {
let left_full = match range {
Range { start: Some(start), .. } if is_integer_const(cx, start, 0) => true,
Range { start: None, .. } => true,
_ => false,
};
let full = left_full
&& match range {
Range {
end: Some(end),
limits: RangeLimits::HalfOpen,
..
} => {
// `x.drain(..x.len())` call
if_chain! {
if let ExprKind::MethodCall(len_path, len_args, _) = end.kind;
if len_path.ident.name == sym::len && len_args.len() == 1;
if let ExprKind::Path(QPath::Resolved(_, drain_path)) = recv.kind;
if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_args[0].kind;
if SpanlessEq::new(cx).eq_path(drain_path, len_path);
then { true }
else { false }
}
},
Range {
end: None,
limits: RangeLimits::HalfOpen,
..
} => true,
_ => false,
};
if full {
span_lint_and_sugg(
cx,
ITER_WITH_DRAIN,
span.with_hi(expr.span.hi()),
&format!("`drain(..)` used on a `{}`", drained_type),
"try this",
"into_iter()".to_string(),
Applicability::MaybeIncorrect,
);
}
}
}
}

View File

@@ -32,6 +32,7 @@ mod iter_nth;
mod iter_nth_zero;
mod iter_overeager_cloned;
mod iter_skip_next;
mod iter_with_drain;
mod iterator_step_by_zero;
mod manual_saturating_arithmetic;
mod manual_str_repeat;
@@ -1118,6 +1119,31 @@ declare_clippy_lint! {
"using `.skip(x).next()` on an iterator"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for use of `.drain(..)` on `Vec` and `VecDeque` for iteration.
///
/// ### Why is this bad?
/// `.into_iter()` is simpler with better performance.
///
/// ### Example
/// ```rust
/// # use std::collections::HashSet;
/// let mut foo = vec![0, 1, 2, 3];
/// let bar: HashSet<usize> = foo.drain(..).collect();
/// ```
/// Use instead:
/// ```rust
/// # use std::collections::HashSet;
/// let foo = vec![0, 1, 2, 3];
/// let bar: HashSet<usize> = foo.into_iter().collect();
/// ```
#[clippy::version = "1.61.0"]
pub ITER_WITH_DRAIN,
perf,
"replace `.drain(..)` with `.into_iter()`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for use of `.get().unwrap()` (or
@@ -2047,6 +2073,7 @@ impl_lint_pass!(Methods => [
GET_UNWRAP,
STRING_EXTEND_CHARS,
ITER_CLONED_COLLECT,
ITER_WITH_DRAIN,
USELESS_ASREF,
UNNECESSARY_FOLD,
UNNECESSARY_FILTER_MAP,
@@ -2327,6 +2354,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg),
_ => {},
},
("drain", [arg]) => {
iter_with_drain::check(cx, expr, recv, span, arg);
},
("expect", [_]) => match method_call(recv) {
Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv),
_ => expect_used::check(cx, expr, recv),

View File

@@ -399,9 +399,9 @@ fn if_statment_binops(kind: &ExprKind) -> Option<Vec<BinaryOp<'_>>> {
fn append_opt_vecs<A>(target_opt: Option<Vec<A>>, source_opt: Option<Vec<A>>) -> Option<Vec<A>> {
match (target_opt, source_opt) {
(Some(mut target), Some(mut source)) => {
(Some(mut target), Some(source)) => {
target.reserve(source.len());
for op in source.drain(..) {
for op in source {
target.push(op);
}
Some(target)
@@ -436,9 +436,9 @@ fn chained_binops_helper<'expr>(left_outer: &'expr Expr, right_outer: &'expr Exp
chained_binops_helper(left_left, left_right),
chained_binops_helper(right_left, right_right),
) {
(Some(mut left_ops), Some(mut right_ops)) => {
(Some(mut left_ops), Some(right_ops)) => {
left_ops.reserve(right_ops.len());
for op in right_ops.drain(..) {
for op in right_ops {
left_ops.push(op);
}
Some(left_ops)

View File

@@ -473,7 +473,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector {
/// ```
fn check_expr(&mut self, cx: &LateContext<'hir>, expr: &'hir hir::Expr<'_>) {
if let Some(args) = match_lint_emission(cx, expr) {
let mut emission_info = extract_emission_info(cx, args);
let emission_info = extract_emission_info(cx, args);
if emission_info.is_empty() {
// See:
// - src/misc.rs:734:9
@@ -483,7 +483,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector {
return;
}
for (lint_name, applicability, is_multi_part) in emission_info.drain(..) {
for (lint_name, applicability, is_multi_part) in emission_info {
let app_info = self.applicability_info.entry(lint_name).or_default();
app_info.applicability = applicability;
app_info.is_multi_part_suggestion = is_multi_part;
@@ -693,7 +693,7 @@ fn extract_emission_info<'hir>(
}
lints
.drain(..)
.into_iter()
.map(|lint_name| (lint_name, applicability, multi_part))
.collect()
}