From 73cd95465e85f878fc5c30a79ce76a9fb59d690c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Sat, 16 Jul 2022 10:22:43 +0200 Subject: [PATCH] Add iter_once and iter_empty lints --- CHANGELOG.md | 2 + clippy_lints/src/iter_once_empty.rs | 164 ++++++++++++++++++++++ clippy_lints/src/lib.register_lints.rs | 2 + clippy_lints/src/lib.register_nursery.rs | 2 + clippy_lints/src/lib.register_pedantic.rs | 2 + clippy_lints/src/lib.rs | 2 + clippy_utils/src/lib.rs | 1 + tests/ui/iter_empty.fixed | 32 +++++ tests/ui/iter_empty.rs | 32 +++++ tests/ui/iter_empty.stderr | 40 ++++++ tests/ui/iter_once.fixed | 32 +++++ tests/ui/iter_once.rs | 32 +++++ tests/ui/iter_once.stderr | 40 ++++++ 13 files changed, 383 insertions(+) create mode 100644 clippy_lints/src/iter_once_empty.rs create mode 100644 tests/ui/iter_empty.fixed create mode 100644 tests/ui/iter_empty.rs create mode 100644 tests/ui/iter_empty.stderr create mode 100644 tests/ui/iter_once.fixed create mode 100644 tests/ui/iter_once.rs create mode 100644 tests/ui/iter_once.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ea7e76ee..94f71eba0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3651,11 +3651,13 @@ Released 2018-09-13 [`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count +[`iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_empty [`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop [`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice [`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_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_once [`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 diff --git a/clippy_lints/src/iter_once_empty.rs b/clippy_lints/src/iter_once_empty.rs new file mode 100644 index 000000000..098960011 --- /dev/null +++ b/clippy_lints/src/iter_once_empty.rs @@ -0,0 +1,164 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use rustc_ast::ast::{Expr, ExprKind}; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for usage of: + /// + /// - `[foo].iter()` + /// - `[foo].iter_mut()` + /// - `[foo].into_iter()` + /// - `Some(foo).iter()` + /// - `Some(foo).iter_mut()` + /// - `Some(foo).into_iter()` + /// + /// ### 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_ONCE, + nursery, + "Iterator for array of length 1" +} + +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for usage of: + /// + /// - `[].iter()` + /// - `[].iter_mut()` + /// - `[].into_iter()` + /// - `None.iter()` + /// - `None.iter_mut()` + /// - `None.into_iter()` + /// + /// ### 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 = [].iter(); + /// let f: option::IntoIter = None.into_iter(); + /// ``` + /// Use instead: + /// ```rust + /// use std::iter; + /// let a: iter::Empty = iter::empty(); + /// let b: iter::Empty = iter::empty(); + /// ``` + /// + /// ### Known problems + /// + /// The type of the resulting iterator might become incompatible with its usage + #[clippy::version = "1.64.0"] + pub ITER_EMPTY, + nursery, + "Iterator for empty array" +} + +declare_lint_pass!(IterOnceEmpty => [ITER_ONCE, ITER_EMPTY]); + +impl EarlyLintPass for IterOnceEmpty { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { + if expr.span.from_expansion() { + // Don't lint match expressions present in + // macro_rules! block + return; + } + + let (method_name, args) = if let ExprKind::MethodCall(seg, args, _) = &expr.kind { + (seg.ident.as_str(), args) + } else { + return; + }; + let arg = if args.len() == 1 { + &args[0] + } else { + return; + }; + + let item = match &arg.kind { + ExprKind::Array(v) if v.len() <= 1 => v.first(), + ExprKind::Path(None, p) => { + if p.segments.len() == 1 && p.segments[0].ident.name == rustc_span::sym::None { + None + } else { + return; + } + }, + ExprKind::Call(f, some_args) if some_args.len() == 1 => { + if let ExprKind::Path(None, p) = &f.kind { + if p.segments.len() == 1 && p.segments[0].ident.name == rustc_span::sym::Some { + Some(&some_args[0]) + } else { + return; + } + } else { + return; + } + }, + _ => return, + }; + + if let Some(i) = item { + let (sugg, msg) = match method_name { + "iter" => ( + format!("std::iter::once(&{})", snippet(cx, i.span, "...")), + "this `iter` call can be replaced with std::iter::once", + ), + "iter_mut" => ( + format!("std::iter::once(&mut {})", snippet(cx, i.span, "...")), + "this `iter_mut` call can be replaced with std::iter::once", + ), + "into_iter" => ( + format!("std::iter::once({})", snippet(cx, i.span, "...")), + "this `into_iter` call can be replaced with std::iter::once", + ), + _ => return, + }; + span_lint_and_sugg(cx, ITER_ONCE, expr.span, msg, "try", sugg, Applicability::Unspecified); + } else { + let msg = match method_name { + "iter" => "this `iter call` can be replaced with std::iter::empty", + "iter_mut" => "this `iter_mut` call can be replaced with std::iter::empty", + "into_iter" => "this `into_iter` call can be replaced with std::iter::empty", + _ => return, + }; + span_lint_and_sugg( + cx, + ITER_EMPTY, + expr.span, + msg, + "try", + "std::iter::empty()".to_string(), + Applicability::Unspecified, + ); + } + } +} diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index d83508a2d..49fc3a895 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -200,6 +200,8 @@ store.register_lints(&[ invalid_utf8_in_unchecked::INVALID_UTF8_IN_UNCHECKED, items_after_statements::ITEMS_AFTER_STATEMENTS, iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR, + iter_once_empty::ITER_EMPTY, + iter_once_empty::ITER_ONCE, large_const_arrays::LARGE_CONST_ARRAYS, large_enum_variant::LARGE_ENUM_VARIANT, large_include_file::LARGE_INCLUDE_FILE, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index 7d731d52d..9953aca43 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -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_EMPTY), + LintId::of(methods::ITER_ONCE), 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), diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 4250ee055..6063fb00a 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -41,6 +41,8 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS), LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS), LintId::of(iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR), + LintId::of(iter_once_empty::ITER_EMPTY), + LintId::of(iter_once_empty::ITER_ONCE), LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS), LintId::of(let_underscore::LET_UNDERSCORE_DROP), LintId::of(literal_representation::LARGE_DIGIT_GROUPS), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 88c1a727f..78279db9a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -258,6 +258,7 @@ mod invalid_upcast_comparisons; mod invalid_utf8_in_unchecked; mod items_after_statements; mod iter_not_returning_iterator; +mod iter_once_empty; mod large_const_arrays; mod large_enum_variant; mod large_include_file; @@ -931,6 +932,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked)); store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default())); store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed)); + store.register_early_pass(|| Box::new(iter_once_empty::IterOnceEmpty)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index dcfc03475..9c58978a0 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -484,6 +484,7 @@ pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Res { } fn find_primitive<'tcx>(tcx: TyCtxt<'tcx>, name: &str) -> impl Iterator + 'tcx { let single = |ty| tcx.incoherent_impls(ty).iter().copied(); + #[allow(clippy::iter_empty)] let empty = || [].iter().copied(); match name { "bool" => single(BoolSimplifiedType), diff --git a/tests/ui/iter_empty.fixed b/tests/ui/iter_empty.fixed new file mode 100644 index 000000000..fb7118f0d --- /dev/null +++ b/tests/ui/iter_empty.fixed @@ -0,0 +1,32 @@ +// run-rustfix +#![warn(clippy::iter_empty)] +#![allow(clippy::iter_next_slice, clippy::redundant_clone)] + +fn array() { + assert_eq!(std::iter::empty().next(), Option::::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::::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 = None.clone(); + let _: [String; 0] = [].clone(); +} + +macro_rules! in_macros { + () => { + assert_eq!([].into_iter().next(), Option::::None); + assert_eq!([].iter_mut().next(), Option::<&mut i32>::None); + assert_eq!([].iter().next(), Option::<&i32>::None); + assert_eq!(None.into_iter().next(), Option::::None); + assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None); + assert_eq!(None.iter().next(), Option::<&i32>::None); + }; +} + +fn main() { + array(); + in_macros!(); +} diff --git a/tests/ui/iter_empty.rs b/tests/ui/iter_empty.rs new file mode 100644 index 000000000..bb192fe16 --- /dev/null +++ b/tests/ui/iter_empty.rs @@ -0,0 +1,32 @@ +// run-rustfix +#![warn(clippy::iter_empty)] +#![allow(clippy::iter_next_slice, clippy::redundant_clone)] + +fn array() { + assert_eq!([].into_iter().next(), Option::::None); + assert_eq!([].iter_mut().next(), Option::<&mut i32>::None); + assert_eq!([].iter().next(), Option::<&i32>::None); + assert_eq!(None.into_iter().next(), Option::::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 = None.clone(); + let _: [String; 0] = [].clone(); +} + +macro_rules! in_macros { + () => { + assert_eq!([].into_iter().next(), Option::::None); + assert_eq!([].iter_mut().next(), Option::<&mut i32>::None); + assert_eq!([].iter().next(), Option::<&i32>::None); + assert_eq!(None.into_iter().next(), Option::::None); + assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None); + assert_eq!(None.iter().next(), Option::<&i32>::None); + }; +} + +fn main() { + array(); + in_macros!(); +} diff --git a/tests/ui/iter_empty.stderr b/tests/ui/iter_empty.stderr new file mode 100644 index 000000000..f4f06e93b --- /dev/null +++ b/tests/ui/iter_empty.stderr @@ -0,0 +1,40 @@ +error: this `into_iter` call can be replaced with std::iter::empty + --> $DIR/iter_empty.rs:6:16 + | +LL | assert_eq!([].into_iter().next(), Option::::None); + | ^^^^^^^^^^^^^^ help: try: `std::iter::empty()` + | + = note: `-D clippy::iter-empty` implied by `-D warnings` + +error: this `iter_mut` call can be replaced with std::iter::empty + --> $DIR/iter_empty.rs:7:16 + | +LL | assert_eq!([].iter_mut().next(), Option::<&mut i32>::None); + | ^^^^^^^^^^^^^ help: try: `std::iter::empty()` + +error: this `iter call` can be replaced with std::iter::empty + --> $DIR/iter_empty.rs:8:16 + | +LL | assert_eq!([].iter().next(), Option::<&i32>::None); + | ^^^^^^^^^ help: try: `std::iter::empty()` + +error: this `into_iter` call can be replaced with std::iter::empty + --> $DIR/iter_empty.rs:9:16 + | +LL | assert_eq!(None.into_iter().next(), Option::::None); + | ^^^^^^^^^^^^^^^^ help: try: `std::iter::empty()` + +error: this `iter_mut` call can be replaced with std::iter::empty + --> $DIR/iter_empty.rs:10:16 + | +LL | assert_eq!(None.iter_mut().next(), Option::<&mut i32>::None); + | ^^^^^^^^^^^^^^^ help: try: `std::iter::empty()` + +error: this `iter call` can be replaced with std::iter::empty + --> $DIR/iter_empty.rs:11:16 + | +LL | assert_eq!(None.iter().next(), Option::<&i32>::None); + | ^^^^^^^^^^^ help: try: `std::iter::empty()` + +error: aborting due to 6 previous errors + diff --git a/tests/ui/iter_once.fixed b/tests/ui/iter_once.fixed new file mode 100644 index 000000000..7247e34d7 --- /dev/null +++ b/tests/ui/iter_once.fixed @@ -0,0 +1,32 @@ +// run-rustfix +#![warn(clippy::iter_once)] +#![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 = Some("test".to_string()).clone(); + let _: [String; 1] = ["test".to_string()].clone(); +} + +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)); + }; +} + +fn main() { + array(); + in_macros!(); +} diff --git a/tests/ui/iter_once.rs b/tests/ui/iter_once.rs new file mode 100644 index 000000000..3a2b9c95c --- /dev/null +++ b/tests/ui/iter_once.rs @@ -0,0 +1,32 @@ +// run-rustfix +#![warn(clippy::iter_once)] +#![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 = Some("test".to_string()).clone(); + let _: [String; 1] = ["test".to_string()].clone(); +} + +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)); + }; +} + +fn main() { + array(); + in_macros!(); +} diff --git a/tests/ui/iter_once.stderr b/tests/ui/iter_once.stderr new file mode 100644 index 000000000..d9e8f96f7 --- /dev/null +++ b/tests/ui/iter_once.stderr @@ -0,0 +1,40 @@ +error: this `into_iter` call can be replaced with std::iter::once + --> $DIR/iter_once.rs:6:16 + | +LL | assert_eq!([123].into_iter().next(), Some(123)); + | ^^^^^^^^^^^^^^^^^ help: try: `std::iter::once(123)` + | + = note: `-D clippy::iter-once` implied by `-D warnings` + +error: this `iter_mut` call can be replaced with std::iter::once + --> $DIR/iter_once.rs:7:16 + | +LL | assert_eq!([123].iter_mut().next(), Some(&mut 123)); + | ^^^^^^^^^^^^^^^^ help: try: `std::iter::once(&mut 123)` + +error: this `iter` call can be replaced with std::iter::once + --> $DIR/iter_once.rs:8:16 + | +LL | assert_eq!([123].iter().next(), Some(&123)); + | ^^^^^^^^^^^^ help: try: `std::iter::once(&123)` + +error: this `into_iter` call can be replaced with std::iter::once + --> $DIR/iter_once.rs:9:16 + | +LL | assert_eq!(Some(123).into_iter().next(), Some(123)); + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::once(123)` + +error: this `iter_mut` call can be replaced with std::iter::once + --> $DIR/iter_once.rs:10:16 + | +LL | assert_eq!(Some(123).iter_mut().next(), Some(&mut 123)); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `std::iter::once(&mut 123)` + +error: this `iter` call can be replaced with std::iter::once + --> $DIR/iter_once.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 +