From 05f0db3cd148695c7582fe388c1e591f6727ece2 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet <44676486+Philippe-Cholet@users.noreply.github.com> Date: Thu, 4 Jul 2024 12:43:03 +0200 Subject: [PATCH 1/5] Migrate to edition 2021 Edition 2021 requires rustc 1.56+ so it's possible. 1. `cargo fix --edition` 2. Field "edition" updated in "Cargo.toml". --- Cargo.toml | 2 +- tests/specializations.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1d8a68f83..f4baa4b77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ description = "Extra iterator adaptors, iterator methods, free functions, and ma keywords = ["iterator", "data-structure", "zip", "product"] categories = ["algorithms", "rust-patterns", "no-std", "no-std::no-alloc"] -edition = "2018" +edition = "2021" # When bumping, please resolve all `#[allow(clippy::*)]` that are newly resolvable. rust-version = "1.63.0" diff --git a/tests/specializations.rs b/tests/specializations.rs index 26d1f5367..3828cfcff 100644 --- a/tests/specializations.rs +++ b/tests/specializations.rs @@ -43,7 +43,7 @@ where I: Iterator + Clone, { macro_rules! check_specialized { - ($src:expr, |$it:pat| $closure:expr) => { + ($src:expr, |$it:pat_param| $closure:expr) => { // Many iterators special-case the first elements, so we test specializations for iterators that have already been advanced. let mut src = $src.clone(); for _ in 0..5 { @@ -101,7 +101,7 @@ where I: DoubleEndedIterator + Clone, { macro_rules! check_specialized { - ($src:expr, |$it:pat| $closure:expr) => { + ($src:expr, |$it:pat_param| $closure:expr) => { // Many iterators special-case the first elements, so we test specializations for iterators that have already been advanced. let mut src = $src.clone(); for step in 0..8 { @@ -501,7 +501,7 @@ quickcheck! { fn helper(it: impl DoubleEndedIterator> + Clone) { macro_rules! check_results_specialized { - ($src:expr, |$it:pat| $closure:expr) => { + ($src:expr, |$it:pat_param| $closure:expr) => { assert_eq!( itertools::process_results($src.clone(), |$it| $closure), itertools::process_results($src.clone(), |i| { From 971ec97f170dc022da921ed608fd26176488f270 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet <44676486+Philippe-Cholet@users.noreply.github.com> Date: Mon, 24 Jun 2024 08:41:10 +0200 Subject: [PATCH 2/5] Automate MSRV change in the crate doc I noted some months ago that the environment variable CARGO_PKG_RUST_VERSION was publicized in 1.63.0 and that if we do not go back before 1.63 then we can automate the MSRV there. Remain "ci.yml" but I don't see a way to automate it, even with `cargo metadata --no-deps --format-version=1 | jq -r '.packages[0].version'`. --- src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 36ddef6cc..a16ed5a9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,7 +50,9 @@ //! //! ## Rust Version //! -//! This version of itertools requires Rust 1.63.0 or later. +//! This version of itertools requires Rust +#![doc = env!("CARGO_PKG_RUST_VERSION")] +//! or later. #[cfg(not(feature = "use_std"))] extern crate core as std; From 93f4d39d12a0757a3e0deb45884943f4d76796de Mon Sep 17 00:00:00 2001 From: Philippe-Cholet <44676486+Philippe-Cholet@users.noreply.github.com> Date: Thu, 4 Jul 2024 13:40:26 +0200 Subject: [PATCH 3/5] Use `bool::then_some` --- benches/specializations.rs | 2 +- tests/laziness.rs | 8 +------- tests/quick.rs | 2 +- tests/specializations.rs | 4 ++-- tests/test_std.rs | 4 +--- 5 files changed, 6 insertions(+), 14 deletions(-) diff --git a/benches/specializations.rs b/benches/specializations.rs index 2b9eff6d1..8dfde2b60 100644 --- a/benches/specializations.rs +++ b/benches/specializations.rs @@ -678,7 +678,7 @@ bench_specializations! { .map(|x| if x % 2 == 1 { Err(x) } else { Ok(x) }) .collect_vec()); } - v.iter().copied().filter_map_ok(|x| if x % 3 == 0 { Some(x + 1) } else { None }) + v.iter().copied().filter_map_ok(|x| (x % 3 == 0).then_some(x + 1)) } flatten_ok { DoubleEndedIterator diff --git a/tests/laziness.rs b/tests/laziness.rs index dfeee68f8..ff6eec458 100644 --- a/tests/laziness.rs +++ b/tests/laziness.rs @@ -122,13 +122,7 @@ must_use_tests! { let _ = Panicking.map(Ok::).filter_ok(|x| x % 2 == 0); } filter_map_ok { - let _ = Panicking.map(Ok::).filter_map_ok(|x| { - if x % 2 == 0 { - Some(x + 1) - } else { - None - } - }); + let _ = Panicking.map(Ok::).filter_map_ok(|x| (x % 2 == 0).then_some(x + 1)); } flatten_ok { let _ = Panicking.map(|x| Ok::<_, ()>([x])).flatten_ok(); diff --git a/tests/quick.rs b/tests/quick.rs index 674848512..90fb5fe36 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -1894,7 +1894,7 @@ quickcheck! { fn fused_filter_map_ok(a: Iter) -> bool { is_fused(a.map(|x| if x % 2 == 0 {Ok(x)} else {Err(x)} ) - .filter_map_ok(|x| if x % 3 == 0 {Some(x / 3)} else {None}) + .filter_map_ok(|x| (x % 3 == 0).then_some(x / 3)) .fuse()) } diff --git a/tests/specializations.rs b/tests/specializations.rs index 3828cfcff..6839b490e 100644 --- a/tests/specializations.rs +++ b/tests/specializations.rs @@ -229,7 +229,7 @@ quickcheck! { } fn while_some(v: Vec) -> () { - test_specializations(&v.iter().map(|&x| if x < 100 { Some(2 * x) } else { None }).while_some()); + test_specializations(&v.iter().map(|&x| (x < 100).then_some(2 * x)).while_some()); } fn pad_using(v: Vec) -> () { @@ -480,7 +480,7 @@ quickcheck! { } fn filter_map_ok(v: Vec>) -> () { - let it = v.into_iter().filter_map_ok(|i| if i < 20 { Some(i * 2) } else { None }); + let it = v.into_iter().filter_map_ok(|i| (i < 20).then_some(i * 2)); test_specializations(&it); test_double_ended_specializations(&it); } diff --git a/tests/test_std.rs b/tests/test_std.rs index 0e4c11b80..bec5c0988 100644 --- a/tests/test_std.rs +++ b/tests/test_std.rs @@ -1477,9 +1477,7 @@ fn format() { #[test] fn while_some() { - let ns = (1..10) - .map(|x| if x % 5 != 0 { Some(x) } else { None }) - .while_some(); + let ns = (1..10).map(|x| (x % 5 != 0).then_some(x)).while_some(); it::assert_equal(ns, vec![1, 2, 3, 4]); } From 90a5afe7f4a93ca661d3c7a5371a813a28f0448c Mon Sep 17 00:00:00 2001 From: Philippe-Cholet <44676486+Philippe-Cholet@users.noreply.github.com> Date: Thu, 4 Jul 2024 13:41:19 +0200 Subject: [PATCH 4/5] Simplifications from clippy `clippy::uninlined_format_args` and 2 more. --- src/lib.rs | 8 +------- src/process_results_impl.rs | 3 +-- src/repeatn.rs | 2 +- tests/quick.rs | 7 +++---- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a16ed5a9c..eea15e350 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4776,13 +4776,7 @@ where (Some(a), Some(b)) => a == b, _ => false, }; - assert!( - equal, - "Failed assertion {a:?} == {b:?} for iteration {i}", - i = i, - a = a, - b = b - ); + assert!(equal, "Failed assertion {a:?} == {b:?} for iteration {i}"); i += 1; } } diff --git a/src/process_results_impl.rs b/src/process_results_impl.rs index 31389c5fd..ed1a2c038 100644 --- a/src/process_results_impl.rs +++ b/src/process_results_impl.rs @@ -62,8 +62,7 @@ where impl DoubleEndedIterator for ProcessResults<'_, I, E> where - I: Iterator>, - I: DoubleEndedIterator, + I: DoubleEndedIterator>, { fn next_back(&mut self) -> Option { let item = self.iter.next_back(); diff --git a/src/repeatn.rs b/src/repeatn.rs index d86ad9fac..a30f2b4ed 100644 --- a/src/repeatn.rs +++ b/src/repeatn.rs @@ -34,7 +34,7 @@ where fn next(&mut self) -> Option { if self.n > 1 { self.n -= 1; - self.elt.as_ref().cloned() + self.elt.clone() } else { self.n = 0; self.elt.take() diff --git a/tests/quick.rs b/tests/quick.rs index 90fb5fe36..f5c34e9b6 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -681,7 +681,7 @@ quickcheck! { assert_eq!(perm.len(), k); let all_items_valid = perm.iter().all(|p| vals.contains(p)); - assert!(all_items_valid, "perm contains value not from input: {:?}", perm); + assert!(all_items_valid, "perm contains value not from input: {perm:?}"); // Check that all perm items are distinct let distinct_len = { @@ -691,7 +691,7 @@ quickcheck! { assert_eq!(perm.len(), distinct_len); // Check that the perm is new - assert!(actual.insert(perm.clone()), "perm already encountered: {:?}", perm); + assert!(actual.insert(perm.clone()), "perm already encountered: {perm:?}"); } } @@ -717,8 +717,7 @@ quickcheck! { for next_perm in perms { assert!( next_perm > curr_perm, - "next perm isn't greater-than current; next_perm={:?} curr_perm={:?} n={}", - next_perm, curr_perm, n + "next perm isn't greater-than current; next_perm={next_perm:?} curr_perm={curr_perm:?} n={n}" ); curr_perm = next_perm; From fde1ad9bafbfb6281a8143a3d7aac9e5a8ea177b Mon Sep 17 00:00:00 2001 From: Philippe-Cholet <44676486+Philippe-Cholet@users.noreply.github.com> Date: Thu, 4 Jul 2024 13:58:03 +0200 Subject: [PATCH 5/5] `ControlFlow` instead of `Result` --- src/adaptors/mod.rs | 30 +++++++++++++----------------- src/lib.rs | 2 +- src/zip_longest.rs | 9 +++++---- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/adaptors/mod.rs b/src/adaptors/mod.rs index 8fca0cfc5..b85ac25ab 100644 --- a/src/adaptors/mod.rs +++ b/src/adaptors/mod.rs @@ -16,6 +16,7 @@ use crate::size_hint::{self, SizeHint}; use std::fmt; use std::iter::{Enumerate, FromIterator, Fuse, FusedIterator}; use std::marker::PhantomData; +use std::ops::ControlFlow; /// An iterator adaptor that alternates elements from two iterators until both /// run out. @@ -93,13 +94,13 @@ where let res = i.try_fold(init, |mut acc, x| { acc = f(acc, x); match j.next() { - Some(y) => Ok(f(acc, y)), - None => Err(acc), + Some(y) => ControlFlow::Continue(f(acc, y)), + None => ControlFlow::Break(acc), } }); match res { - Ok(acc) => j.fold(acc, f), - Err(acc) => i.fold(acc, f), + ControlFlow::Continue(acc) => j.fold(acc, f), + ControlFlow::Break(acc) => i.fold(acc, f), } } } @@ -216,14 +217,12 @@ where let res = i.try_fold(init, |mut acc, x| { acc = f(acc, x); match j.next() { - Some(y) => Ok(f(acc, y)), - None => Err(acc), + Some(y) => ControlFlow::Continue(f(acc, y)), + None => ControlFlow::Break(acc), } }); - match res { - Ok(val) => val, - Err(val) => val, - } + let (ControlFlow::Continue(val) | ControlFlow::Break(val)) = res; + val } } @@ -595,14 +594,11 @@ where F: FnMut(B, Self::Item) -> B, { let res = self.iter.try_fold(acc, |acc, item| match item { - Some(item) => Ok(f(acc, item)), - None => Err(acc), + Some(item) => ControlFlow::Continue(f(acc, item)), + None => ControlFlow::Break(acc), }); - - match res { - Ok(val) => val, - Err(val) => val, - } + let (ControlFlow::Continue(val) | ControlFlow::Break(val)) = res; + val } } diff --git a/src/lib.rs b/src/lib.rs index eea15e350..b2c00253f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2908,7 +2908,7 @@ pub trait Itertools: Iterator { Self: Sized, F: FnMut(B, Self::Item) -> FoldWhile, { - use Result::{Err as Break, Ok as Continue}; + use std::ops::ControlFlow::{Break, Continue}; let result = self.try_fold( init, diff --git a/src/zip_longest.rs b/src/zip_longest.rs index d4eb9a882..8b3a47c46 100644 --- a/src/zip_longest.rs +++ b/src/zip_longest.rs @@ -1,6 +1,7 @@ use super::size_hint; use std::cmp::Ordering::{Equal, Greater, Less}; use std::iter::{Fuse, FusedIterator}; +use std::ops::ControlFlow; use crate::either_or_both::EitherOrBoth; @@ -62,12 +63,12 @@ where { let Self { mut a, mut b } = self; let res = a.try_fold(init, |init, a| match b.next() { - Some(b) => Ok(f(init, EitherOrBoth::Both(a, b))), - None => Err(f(init, EitherOrBoth::Left(a))), + Some(b) => ControlFlow::Continue(f(init, EitherOrBoth::Both(a, b))), + None => ControlFlow::Break(f(init, EitherOrBoth::Left(a))), }); match res { - Ok(acc) => b.map(EitherOrBoth::Right).fold(acc, f), - Err(acc) => a.map(EitherOrBoth::Left).fold(acc, f), + ControlFlow::Continue(acc) => b.map(EitherOrBoth::Right).fold(acc, f), + ControlFlow::Break(acc) => a.map(EitherOrBoth::Left).fold(acc, f), } } }