Auto merge of #9187 - sgued:iter-once, r=flip1995

Add lint recommending using `std::iter::once` and `std::iter::empty`

```
changelog: [`iter_once`]: add new lint
changelog: [`iter_empty`]: add new lint
```

fixes #9186

- \[ ] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

The lint doesn't really follow the naming conventions. I don't have any better idea so I'm open to suggestions.
This commit is contained in:
bors 2022-08-14 15:45:17 +00:00
commit 679fa9f2bf
16 changed files with 524 additions and 5 deletions

View file

@ -3802,6 +3802,8 @@ Released 2018-09-13
[`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator
[`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth
[`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
[`iter_on_empty_collections`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_on_empty_collections
[`iter_on_single_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_on_single_items
[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
[`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain

View file

@ -760,7 +760,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
.and_then(|subs| subs.get(1..))
{
Some(subs) => cx.tcx.mk_substs(subs.iter().copied()),
None => cx.tcx.mk_substs([].iter()),
None => cx.tcx.mk_substs(std::iter::empty::<ty::subst::GenericArg<'_>>()),
} && let impl_ty = if cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref() {
// Trait methods taking `&self`
sub_ty

View file

@ -516,7 +516,10 @@ fn param_env_for_derived_eq(tcx: TyCtxt<'_>, did: DefId, eq_trait_id: DefId) ->
tcx.mk_predicates(ty_predicates.iter().map(|&(p, _)| p).chain(
params.iter().filter(|&&(_, needs_eq)| needs_eq).map(|&(param, _)| {
tcx.mk_predicate(Binder::dummy(PredicateKind::Trait(TraitPredicate {
trait_ref: TraitRef::new(eq_trait_id, tcx.mk_substs([tcx.mk_param_from_def(param)].into_iter())),
trait_ref: TraitRef::new(
eq_trait_id,
tcx.mk_substs(std::iter::once(tcx.mk_param_from_def(param))),
),
constness: BoundConstness::NotConst,
polarity: ImplPolarity::Positive,
})))

View file

@ -315,6 +315,8 @@ store.register_lints(&[
methods::ITER_NEXT_SLICE,
methods::ITER_NTH,
methods::ITER_NTH_ZERO,
methods::ITER_ON_EMPTY_COLLECTIONS,
methods::ITER_ON_SINGLE_ITEMS,
methods::ITER_OVEREAGER_CLONED,
methods::ITER_SKIP_NEXT,
methods::ITER_WITH_DRAIN,

View file

@ -14,6 +14,8 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(index_refutable_slice::INDEX_REFUTABLE_SLICE),
LintId::of(let_if_seq::USELESS_LET_IF_SEQ),
LintId::of(matches::SIGNIFICANT_DROP_IN_SCRUTINEE),
LintId::of(methods::ITER_ON_EMPTY_COLLECTIONS),
LintId::of(methods::ITER_ON_SINGLE_ITEMS),
LintId::of(methods::ITER_WITH_DRAIN),
LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),

View file

@ -0,0 +1,107 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::{get_expr_use_or_unification_node, is_lang_ctor, is_no_std_crate};
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{Expr, ExprKind, Node};
use rustc_lint::LateContext;
use super::{ITER_ON_EMPTY_COLLECTIONS, ITER_ON_SINGLE_ITEMS};
enum IterType {
Iter,
IterMut,
IntoIter,
}
impl IterType {
fn ref_prefix(&self) -> &'static str {
match self {
Self::Iter => "&",
Self::IterMut => "&mut ",
Self::IntoIter => "",
}
}
}
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, method_name: &str, recv: &Expr<'_>) {
let item = match &recv.kind {
ExprKind::Array(v) if v.len() <= 1 => v.first(),
ExprKind::Path(p) => {
if is_lang_ctor(cx, p, OptionNone) {
None
} else {
return;
}
},
ExprKind::Call(f, some_args) if some_args.len() == 1 => {
if let ExprKind::Path(p) = &f.kind {
if is_lang_ctor(cx, p, OptionSome) {
Some(&some_args[0])
} else {
return;
}
} else {
return;
}
},
_ => return,
};
let iter_type = match method_name {
"iter" => IterType::Iter,
"iter_mut" => IterType::IterMut,
"into_iter" => IterType::IntoIter,
_ => return,
};
let is_unified = match get_expr_use_or_unification_node(cx.tcx, expr) {
Some((Node::Expr(parent), child_id)) => match parent.kind {
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id == child_id => false,
ExprKind::If(_, _, _)
| ExprKind::Match(_, _, _)
| ExprKind::Closure(_)
| ExprKind::Ret(_)
| ExprKind::Break(_, _) => true,
_ => false,
},
Some((Node::Stmt(_) | Node::Local(_), _)) => false,
_ => true,
};
if is_unified {
return;
}
if let Some(i) = item {
let sugg = format!(
"{}::iter::once({}{})",
if is_no_std_crate(cx) { "core" } else { "std" },
iter_type.ref_prefix(),
snippet(cx, i.span, "...")
);
span_lint_and_sugg(
cx,
ITER_ON_SINGLE_ITEMS,
expr.span,
&format!("`{method_name}` call on a collection with only one item"),
"try",
sugg,
Applicability::MaybeIncorrect,
);
} else {
span_lint_and_sugg(
cx,
ITER_ON_EMPTY_COLLECTIONS,
expr.span,
&format!("`{method_name}` call on an empty collection"),
"try",
if is_no_std_crate(cx) {
"core::iter::empty()".to_string()
} else {
"std::iter::empty()".to_string()
},
Applicability::MaybeIncorrect,
);
}
}

View file

@ -33,6 +33,7 @@ mod iter_count;
mod iter_next_slice;
mod iter_nth;
mod iter_nth_zero;
mod iter_on_single_or_empty_collections;
mod iter_overeager_cloned;
mod iter_skip_next;
mod iter_with_drain;
@ -2304,6 +2305,69 @@ declare_clippy_lint! {
more clearly with `if .. else ..`"
}
declare_clippy_lint! {
/// ### What it does
///
/// Checks for calls to `iter`, `iter_mut` or `into_iter` on collections containing a single item
///
/// ### Why is this bad?
///
/// It is simpler to use the once function from the standard library:
///
/// ### Example
///
/// ```rust
/// let a = [123].iter();
/// let b = Some(123).into_iter();
/// ```
/// Use instead:
/// ```rust
/// use std::iter;
/// let a = iter::once(&123);
/// let b = iter::once(123);
/// ```
///
/// ### Known problems
///
/// The type of the resulting iterator might become incompatible with its usage
#[clippy::version = "1.64.0"]
pub ITER_ON_SINGLE_ITEMS,
nursery,
"Iterator for array of length 1"
}
declare_clippy_lint! {
/// ### What it does
///
/// Checks for calls to `iter`, `iter_mut` or `into_iter` on empty collections
///
/// ### Why is this bad?
///
/// It is simpler to use the empty function from the standard library:
///
/// ### Example
///
/// ```rust
/// use std::{slice, option};
/// let a: slice::Iter<i32> = [].iter();
/// let f: option::IntoIter<i32> = None.into_iter();
/// ```
/// Use instead:
/// ```rust
/// use std::iter;
/// let a: iter::Empty<i32> = iter::empty();
/// let b: iter::Empty<i32> = iter::empty();
/// ```
///
/// ### Known problems
///
/// The type of the resulting iterator might become incompatible with its usage
#[clippy::version = "1.64.0"]
pub ITER_ON_EMPTY_COLLECTIONS,
nursery,
"Iterator for empty array"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
@ -2406,6 +2470,8 @@ impl_lint_pass!(Methods => [
NEEDLESS_OPTION_TAKE,
NO_EFFECT_REPLACE,
OBFUSCATED_IF_ELSE,
ITER_ON_SINGLE_ITEMS,
ITER_ON_EMPTY_COLLECTIONS
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -2708,6 +2774,9 @@ impl Methods {
("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, self.msrv),
("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
("iter" | "iter_mut" | "into_iter", []) => {
iter_on_single_or_empty_collections::check(cx, expr, name, recv);
},
("join", [join_arg]) => {
if let Some(("collect", _, span)) = method_call(recv) {
unnecessary_join::check(cx, expr, recv, join_arg, span);

View file

@ -11,6 +11,8 @@ use rustc_middle::ty::adjustment::{Adjust, AutoBorrow, AutoBorrowMutability};
use rustc_middle::ty::subst::GenericArg;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use std::iter;
declare_clippy_lint! {
/// ### What it does
/// Checks for redundant slicing expressions which use the full range, and
@ -134,7 +136,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
} else if let Some(target_id) = cx.tcx.lang_items().deref_target() {
if let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions(
cx.param_env,
cx.tcx.mk_projection(target_id, cx.tcx.mk_substs([GenericArg::from(indexed_ty)].into_iter())),
cx.tcx.mk_projection(target_id, cx.tcx.mk_substs(iter::once(GenericArg::from(indexed_ty)))),
) {
if deref_ty == expr_ty {
let snip = snippet_with_context(cx, indexed.span, ctxt, "..", &mut app).0;

View file

@ -350,7 +350,7 @@ define_Conf! {
/// Lint: DISALLOWED_SCRIPT_IDENTS.
///
/// The list of unicode scripts allowed to be used in the scope.
(allowed_scripts: Vec<String> = ["Latin"].iter().map(ToString::to_string).collect()),
(allowed_scripts: Vec<String> = vec!["Latin".to_string()]),
/// Lint: NON_SEND_FIELDS_IN_SEND_TY.
///
/// Whether to apply the raw pointer heuristic to determine if a type is `Send`.

View file

@ -797,7 +797,7 @@ fn get_lint_group_and_level_or_lint(
let result = cx.lint_store.check_lint_name(
lint_name,
Some(sym::clippy),
&[Ident::with_dummy_span(sym::clippy)].into_iter().collect(),
&std::iter::once(Ident::with_dummy_span(sym::clippy)).collect(),
);
if let CheckLintNameResult::Tool(Ok(lint_lst)) = result {
if let Some(group) = get_lint_group(cx, lint_lst[0]) {

View file

@ -0,0 +1,63 @@
// run-rustfix
#![warn(clippy::iter_on_empty_collections)]
#![allow(clippy::iter_next_slice, clippy::redundant_clone)]
fn array() {
assert_eq!(std::iter::empty().next(), Option::<i32>::None);
assert_eq!(std::iter::empty().next(), Option::<&mut i32>::None);
assert_eq!(std::iter::empty().next(), Option::<&i32>::None);
assert_eq!(std::iter::empty().next(), Option::<i32>::None);
assert_eq!(std::iter::empty().next(), Option::<&mut i32>::None);
assert_eq!(std::iter::empty().next(), Option::<&i32>::None);
// Don't trigger on non-iter methods
let _: Option<String> = None.clone();
let _: [String; 0] = [].clone();
// Don't trigger on match or if branches
let _ = match 123 {
123 => [].iter(),
_ => ["test"].iter(),
};
let _ = if false { ["test"].iter() } else { [].iter() };
}
macro_rules! in_macros {
() => {
assert_eq!([].into_iter().next(), Option::<i32>::None);
assert_eq!([].iter_mut().next(), Option::<&mut i32>::None);
assert_eq!([].iter().next(), Option::<&i32>::None);
assert_eq!(None.into_iter().next(), Option::<i32>::None);
assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None);
assert_eq!(None.iter().next(), Option::<&i32>::None);
};
}
// Don't trigger on a `None` that isn't std's option
mod custom_option {
#[allow(unused)]
enum CustomOption {
Some(i32),
None,
}
impl CustomOption {
fn iter(&self) {}
fn iter_mut(&mut self) {}
fn into_iter(self) {}
}
use CustomOption::*;
pub fn custom_option() {
None.iter();
None.iter_mut();
None.into_iter();
}
}
fn main() {
array();
custom_option::custom_option();
in_macros!();
}

View file

@ -0,0 +1,63 @@
// run-rustfix
#![warn(clippy::iter_on_empty_collections)]
#![allow(clippy::iter_next_slice, clippy::redundant_clone)]
fn array() {
assert_eq!([].into_iter().next(), Option::<i32>::None);
assert_eq!([].iter_mut().next(), Option::<&mut i32>::None);
assert_eq!([].iter().next(), Option::<&i32>::None);
assert_eq!(None.into_iter().next(), Option::<i32>::None);
assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None);
assert_eq!(None.iter().next(), Option::<&i32>::None);
// Don't trigger on non-iter methods
let _: Option<String> = None.clone();
let _: [String; 0] = [].clone();
// Don't trigger on match or if branches
let _ = match 123 {
123 => [].iter(),
_ => ["test"].iter(),
};
let _ = if false { ["test"].iter() } else { [].iter() };
}
macro_rules! in_macros {
() => {
assert_eq!([].into_iter().next(), Option::<i32>::None);
assert_eq!([].iter_mut().next(), Option::<&mut i32>::None);
assert_eq!([].iter().next(), Option::<&i32>::None);
assert_eq!(None.into_iter().next(), Option::<i32>::None);
assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None);
assert_eq!(None.iter().next(), Option::<&i32>::None);
};
}
// Don't trigger on a `None` that isn't std's option
mod custom_option {
#[allow(unused)]
enum CustomOption {
Some(i32),
None,
}
impl CustomOption {
fn iter(&self) {}
fn iter_mut(&mut self) {}
fn into_iter(self) {}
}
use CustomOption::*;
pub fn custom_option() {
None.iter();
None.iter_mut();
None.into_iter();
}
}
fn main() {
array();
custom_option::custom_option();
in_macros!();
}

View file

@ -0,0 +1,40 @@
error: `into_iter` call on an empty collection
--> $DIR/iter_on_empty_collections.rs:6:16
|
LL | assert_eq!([].into_iter().next(), Option::<i32>::None);
| ^^^^^^^^^^^^^^ help: try: `std::iter::empty()`
|
= note: `-D clippy::iter-on-empty-collections` implied by `-D warnings`
error: `iter_mut` call on an empty collection
--> $DIR/iter_on_empty_collections.rs:7:16
|
LL | assert_eq!([].iter_mut().next(), Option::<&mut i32>::None);
| ^^^^^^^^^^^^^ help: try: `std::iter::empty()`
error: `iter` call on an empty collection
--> $DIR/iter_on_empty_collections.rs:8:16
|
LL | assert_eq!([].iter().next(), Option::<&i32>::None);
| ^^^^^^^^^ help: try: `std::iter::empty()`
error: `into_iter` call on an empty collection
--> $DIR/iter_on_empty_collections.rs:9:16
|
LL | assert_eq!(None.into_iter().next(), Option::<i32>::None);
| ^^^^^^^^^^^^^^^^ help: try: `std::iter::empty()`
error: `iter_mut` call on an empty collection
--> $DIR/iter_on_empty_collections.rs:10:16
|
LL | assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None);
| ^^^^^^^^^^^^^^^ help: try: `std::iter::empty()`
error: `iter` call on an empty collection
--> $DIR/iter_on_empty_collections.rs:11:16
|
LL | assert_eq!(None.iter().next(), Option::<&i32>::None);
| ^^^^^^^^^^^ help: try: `std::iter::empty()`
error: aborting due to 6 previous errors

View file

@ -0,0 +1,63 @@
// run-rustfix
#![warn(clippy::iter_on_single_items)]
#![allow(clippy::iter_next_slice, clippy::redundant_clone)]
fn array() {
assert_eq!(std::iter::once(123).next(), Some(123));
assert_eq!(std::iter::once(&mut 123).next(), Some(&mut 123));
assert_eq!(std::iter::once(&123).next(), Some(&123));
assert_eq!(std::iter::once(123).next(), Some(123));
assert_eq!(std::iter::once(&mut 123).next(), Some(&mut 123));
assert_eq!(std::iter::once(&123).next(), Some(&123));
// Don't trigger on non-iter methods
let _: Option<String> = Some("test".to_string()).clone();
let _: [String; 1] = ["test".to_string()].clone();
// Don't trigger on match or if branches
let _ = match 123 {
123 => [].iter(),
_ => ["test"].iter(),
};
let _ = if false { ["test"].iter() } else { [].iter() };
}
macro_rules! in_macros {
() => {
assert_eq!([123].into_iter().next(), Some(123));
assert_eq!([123].iter_mut().next(), Some(&mut 123));
assert_eq!([123].iter().next(), Some(&123));
assert_eq!(Some(123).into_iter().next(), Some(123));
assert_eq!(Some(123).iter_mut().next(), Some(&mut 123));
assert_eq!(Some(123).iter().next(), Some(&123));
};
}
// Don't trigger on a `Some` that isn't std's option
mod custom_option {
#[allow(unused)]
enum CustomOption {
Some(i32),
None,
}
impl CustomOption {
fn iter(&self) {}
fn iter_mut(&mut self) {}
fn into_iter(self) {}
}
use CustomOption::*;
pub fn custom_option() {
Some(3).iter();
Some(3).iter_mut();
Some(3).into_iter();
}
}
fn main() {
array();
custom_option::custom_option();
in_macros!();
}

View file

@ -0,0 +1,63 @@
// run-rustfix
#![warn(clippy::iter_on_single_items)]
#![allow(clippy::iter_next_slice, clippy::redundant_clone)]
fn array() {
assert_eq!([123].into_iter().next(), Some(123));
assert_eq!([123].iter_mut().next(), Some(&mut 123));
assert_eq!([123].iter().next(), Some(&123));
assert_eq!(Some(123).into_iter().next(), Some(123));
assert_eq!(Some(123).iter_mut().next(), Some(&mut 123));
assert_eq!(Some(123).iter().next(), Some(&123));
// Don't trigger on non-iter methods
let _: Option<String> = Some("test".to_string()).clone();
let _: [String; 1] = ["test".to_string()].clone();
// Don't trigger on match or if branches
let _ = match 123 {
123 => [].iter(),
_ => ["test"].iter(),
};
let _ = if false { ["test"].iter() } else { [].iter() };
}
macro_rules! in_macros {
() => {
assert_eq!([123].into_iter().next(), Some(123));
assert_eq!([123].iter_mut().next(), Some(&mut 123));
assert_eq!([123].iter().next(), Some(&123));
assert_eq!(Some(123).into_iter().next(), Some(123));
assert_eq!(Some(123).iter_mut().next(), Some(&mut 123));
assert_eq!(Some(123).iter().next(), Some(&123));
};
}
// Don't trigger on a `Some` that isn't std's option
mod custom_option {
#[allow(unused)]
enum CustomOption {
Some(i32),
None,
}
impl CustomOption {
fn iter(&self) {}
fn iter_mut(&mut self) {}
fn into_iter(self) {}
}
use CustomOption::*;
pub fn custom_option() {
Some(3).iter();
Some(3).iter_mut();
Some(3).into_iter();
}
}
fn main() {
array();
custom_option::custom_option();
in_macros!();
}

View file

@ -0,0 +1,40 @@
error: `into_iter` call on a collection with only one item
--> $DIR/iter_on_single_items.rs:6:16
|
LL | assert_eq!([123].into_iter().next(), Some(123));
| ^^^^^^^^^^^^^^^^^ help: try: `std::iter::once(123)`
|
= note: `-D clippy::iter-on-single-items` implied by `-D warnings`
error: `iter_mut` call on a collection with only one item
--> $DIR/iter_on_single_items.rs:7:16
|
LL | assert_eq!([123].iter_mut().next(), Some(&mut 123));
| ^^^^^^^^^^^^^^^^ help: try: `std::iter::once(&mut 123)`
error: `iter` call on a collection with only one item
--> $DIR/iter_on_single_items.rs:8:16
|
LL | assert_eq!([123].iter().next(), Some(&123));
| ^^^^^^^^^^^^ help: try: `std::iter::once(&123)`
error: `into_iter` call on a collection with only one item
--> $DIR/iter_on_single_items.rs:9:16
|
LL | assert_eq!(Some(123).into_iter().next(), Some(123));
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::once(123)`
error: `iter_mut` call on a collection with only one item
--> $DIR/iter_on_single_items.rs:10:16
|
LL | assert_eq!(Some(123).iter_mut().next(), Some(&mut 123));
| ^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::once(&mut 123)`
error: `iter` call on a collection with only one item
--> $DIR/iter_on_single_items.rs:11:16
|
LL | assert_eq!(Some(123).iter().next(), Some(&123));
| ^^^^^^^^^^^^^^^^ help: try: `std::iter::once(&123)`
error: aborting due to 6 previous errors