Skip to content

Commit f3f86d8

Browse files
committed
Move iter_once and iter_empty to methods as a late pass
This enables more thorough checking of types to avoid triggering on custom Some and None enum variants
1 parent 332e031 commit f3f86d8

File tree

10 files changed

+245
-170
lines changed

10 files changed

+245
-170
lines changed

clippy_lints/src/iter_once_empty.rs

Lines changed: 0 additions & 164 deletions
This file was deleted.

clippy_lints/src/lib.register_lints.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,6 @@ store.register_lints(&[
200200
invalid_utf8_in_unchecked::INVALID_UTF8_IN_UNCHECKED,
201201
items_after_statements::ITEMS_AFTER_STATEMENTS,
202202
iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR,
203-
iter_once_empty::ITER_EMPTY,
204-
iter_once_empty::ITER_ONCE,
205203
large_const_arrays::LARGE_CONST_ARRAYS,
206204
large_enum_variant::LARGE_ENUM_VARIANT,
207205
large_include_file::LARGE_INCLUDE_FILE,
@@ -314,9 +312,11 @@ store.register_lints(&[
314312
methods::ITERATOR_STEP_BY_ZERO,
315313
methods::ITER_CLONED_COLLECT,
316314
methods::ITER_COUNT,
315+
methods::ITER_EMPTY,
317316
methods::ITER_NEXT_SLICE,
318317
methods::ITER_NTH,
319318
methods::ITER_NTH_ZERO,
319+
methods::ITER_ONCE,
320320
methods::ITER_OVEREAGER_CLONED,
321321
methods::ITER_SKIP_NEXT,
322322
methods::ITER_WITH_DRAIN,

clippy_lints/src/lib.register_pedantic.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
4141
LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS),
4242
LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS),
4343
LintId::of(iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR),
44-
LintId::of(iter_once_empty::ITER_EMPTY),
45-
LintId::of(iter_once_empty::ITER_ONCE),
4644
LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS),
4745
LintId::of(let_underscore::LET_UNDERSCORE_DROP),
4846
LintId::of(literal_representation::LARGE_DIGIT_GROUPS),

clippy_lints/src/lib.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,6 @@ mod invalid_upcast_comparisons;
258258
mod invalid_utf8_in_unchecked;
259259
mod items_after_statements;
260260
mod iter_not_returning_iterator;
261-
mod iter_once_empty;
262261
mod large_const_arrays;
263262
mod large_enum_variant;
264263
mod large_include_file;
@@ -932,7 +931,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
932931
store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
933932
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
934933
store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
935-
store.register_early_pass(|| Box::new(iter_once_empty::IterOnceEmpty));
936934
// add lints here, do not remove this comment, it's used in `new_lint`
937935
}
938936

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::is_lang_ctor;
3+
use clippy_utils::source::snippet;
4+
5+
use rustc_errors::Applicability;
6+
use rustc_hir::LangItem::{OptionNone, OptionSome};
7+
use rustc_hir::{Expr, ExprKind};
8+
use rustc_lint::LateContext;
9+
10+
use super::{ITER_EMPTY, ITER_ONCE};
11+
12+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, method_name: &str, recv: &Expr<'_>) {
13+
let item = match &recv.kind {
14+
ExprKind::Array(v) if v.len() <= 1 => v.first(),
15+
ExprKind::Path(p) => {
16+
if is_lang_ctor(cx, p, OptionNone) {
17+
None
18+
} else {
19+
return;
20+
}
21+
},
22+
ExprKind::Call(f, some_args) if some_args.len() == 1 => {
23+
if let ExprKind::Path(p) = &f.kind {
24+
if is_lang_ctor(cx, p, OptionSome) {
25+
Some(&some_args[0])
26+
} else {
27+
return;
28+
}
29+
} else {
30+
return;
31+
}
32+
},
33+
_ => return,
34+
};
35+
36+
if let Some(i) = item {
37+
let (sugg, msg) = match method_name {
38+
"iter" => (
39+
format!("std::iter::once(&{})", snippet(cx, i.span, "...")),
40+
"this `iter` call can be replaced with std::iter::once",
41+
),
42+
"iter_mut" => (
43+
format!("std::iter::once(&mut {})", snippet(cx, i.span, "...")),
44+
"this `iter_mut` call can be replaced with std::iter::once",
45+
),
46+
"into_iter" => (
47+
format!("std::iter::once({})", snippet(cx, i.span, "...")),
48+
"this `into_iter` call can be replaced with std::iter::once",
49+
),
50+
_ => return,
51+
};
52+
span_lint_and_sugg(cx, ITER_ONCE, expr.span, msg, "try", sugg, Applicability::Unspecified);
53+
} else {
54+
let msg = match method_name {
55+
"iter" => "this `iter call` can be replaced with std::iter::empty",
56+
"iter_mut" => "this `iter_mut` call can be replaced with std::iter::empty",
57+
"into_iter" => "this `into_iter` call can be replaced with std::iter::empty",
58+
_ => return,
59+
};
60+
span_lint_and_sugg(
61+
cx,
62+
ITER_EMPTY,
63+
expr.span,
64+
msg,
65+
"try",
66+
"std::iter::empty()".to_string(),
67+
Applicability::Unspecified,
68+
);
69+
}
70+
}

clippy_lints/src/methods/mod.rs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ mod iter_count;
3333
mod iter_next_slice;
3434
mod iter_nth;
3535
mod iter_nth_zero;
36+
mod iter_once_empty;
3637
mod iter_overeager_cloned;
3738
mod iter_skip_next;
3839
mod iter_with_drain;
@@ -2304,6 +2305,83 @@ declare_clippy_lint! {
23042305
more clearly with `if .. else ..`"
23052306
}
23062307

2308+
declare_clippy_lint! {
2309+
/// ### What it does
2310+
///
2311+
/// Checks for usage of:
2312+
///
2313+
/// - `[foo].iter()`
2314+
/// - `[foo].iter_mut()`
2315+
/// - `[foo].into_iter()`
2316+
/// - `Some(foo).iter()`
2317+
/// - `Some(foo).iter_mut()`
2318+
/// - `Some(foo).into_iter()`
2319+
///
2320+
/// ### Why is this bad?
2321+
///
2322+
/// It is simpler to use the once function from the standard library:
2323+
///
2324+
/// ### Example
2325+
///
2326+
/// ```rust
2327+
/// let a = [123].iter();
2328+
/// let b = Some(123).into_iter();
2329+
/// ```
2330+
/// Use instead:
2331+
/// ```rust
2332+
/// use std::iter;
2333+
/// let a = iter::once(&123);
2334+
/// let b = iter::once(123);
2335+
/// ```
2336+
///
2337+
/// ### Known problems
2338+
///
2339+
/// The type of the resulting iterator might become incompatible with its usage
2340+
#[clippy::version = "1.64.0"]
2341+
pub ITER_ONCE,
2342+
nursery,
2343+
"Iterator for array of length 1"
2344+
}
2345+
2346+
declare_clippy_lint! {
2347+
/// ### What it does
2348+
///
2349+
/// Checks for usage of:
2350+
///
2351+
/// - `[].iter()`
2352+
/// - `[].iter_mut()`
2353+
/// - `[].into_iter()`
2354+
/// - `None.iter()`
2355+
/// - `None.iter_mut()`
2356+
/// - `None.into_iter()`
2357+
///
2358+
/// ### Why is this bad?
2359+
///
2360+
/// It is simpler to use the empty function from the standard library:
2361+
///
2362+
/// ### Example
2363+
///
2364+
/// ```rust
2365+
/// use std::{slice, option};
2366+
/// let a: slice::Iter<i32> = [].iter();
2367+
/// let f: option::IntoIter<i32> = None.into_iter();
2368+
/// ```
2369+
/// Use instead:
2370+
/// ```rust
2371+
/// use std::iter;
2372+
/// let a: iter::Empty<i32> = iter::empty();
2373+
/// let b: iter::Empty<i32> = iter::empty();
2374+
/// ```
2375+
///
2376+
/// ### Known problems
2377+
///
2378+
/// The type of the resulting iterator might become incompatible with its usage
2379+
#[clippy::version = "1.64.0"]
2380+
pub ITER_EMPTY,
2381+
nursery,
2382+
"Iterator for empty array"
2383+
}
2384+
23072385
pub struct Methods {
23082386
avoid_breaking_exported_api: bool,
23092387
msrv: Option<RustcVersion>,
@@ -2406,6 +2484,8 @@ impl_lint_pass!(Methods => [
24062484
NEEDLESS_OPTION_TAKE,
24072485
NO_EFFECT_REPLACE,
24082486
OBFUSCATED_IF_ELSE,
2487+
ITER_ONCE,
2488+
ITER_EMPTY
24092489
]);
24102490

24112491
/// Extracts a method call name, args, and `Span` of the method name.
@@ -2708,6 +2788,7 @@ impl Methods {
27082788
("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, self.msrv),
27092789
("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
27102790
("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
2791+
("iter" | "iter_mut" | "into_iter", []) => iter_once_empty::check(cx, expr, name, recv),
27112792
("join", [join_arg]) => {
27122793
if let Some(("collect", _, span)) = method_call(recv) {
27132794
unnecessary_join::check(cx, expr, recv, join_arg, span);

0 commit comments

Comments
 (0)