diff --git a/src/internal/memory.rs b/src/internal/memory.rs index 07597340..122e4221 100644 --- a/src/internal/memory.rs +++ b/src/internal/memory.rs @@ -56,15 +56,15 @@ impl Memory { let pa = self .assignments .entry(package) - .or_insert(PackageAssignments::new()); + .or_insert_with(PackageAssignments::new); pa.decision = Some((version.clone(), Term::exact(version))); } /// Remove a decision from a Memory. pub fn remove_decision(&mut self, package: &P) { - self.assignments - .get_mut(package) - .map(|pa| pa.decision = None); + if let Some(pa) = self.assignments.get_mut(package) { + pa.decision = None; + } } /// Add a derivation to a Memory. @@ -72,7 +72,7 @@ impl Memory { let pa = self .assignments .entry(package) - .or_insert(PackageAssignments::new()); + .or_insert_with(PackageAssignments::new); pa.derivations_not_intersected_yet.push(term); } diff --git a/src/range.rs b/src/range.rs index aaeaa55f..25d9403c 100644 --- a/src/range.rs +++ b/src/range.rs @@ -14,9 +14,11 @@ //! - [strictly_lower_than(v)](Range::strictly_lower_than): the set defined by `versions < v` //! - [between(v1, v2)](Range::between): the set defined by `v1 <= versions < v2` -use crate::version::Version; +use std::cmp::Ordering; use std::fmt; +use crate::version::Version; + /// A Range is a set of versions. #[derive(Debug, Clone, Eq, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -183,34 +185,38 @@ impl Range { } // Right contains an infinite interval: - (Some((l1, Some(l2))), Some((r1, None))) => { - if l2 < r1 { + (Some((l1, Some(l2))), Some((r1, None))) => match l2.cmp(r1) { + Ordering::Less => { left = left_iter.next(); - } else if l2 == r1 { + } + Ordering::Equal => { segments.extend(left_iter.cloned()); break; - } else { + } + Ordering::Greater => { let start = l1.max(r1).to_owned(); segments.push((start, Some(l2.to_owned()))); segments.extend(left_iter.cloned()); break; } - } + }, // Left contains an infinite interval: - (Some((l1, None)), Some((r1, Some(r2)))) => { - if r2 < l1 { + (Some((l1, None)), Some((r1, Some(r2)))) => match r2.cmp(l1) { + Ordering::Less => { right = right_iter.next(); - } else if r2 == l1 { + } + Ordering::Equal => { segments.extend(right_iter.cloned()); break; - } else { + } + Ordering::Greater => { let start = l1.max(r1).to_owned(); segments.push((start, Some(r2.to_owned()))); segments.extend(right_iter.cloned()); break; } - } + }, // Both sides contain an infinite interval: (Some((l1, None)), Some((r1, None))) => { @@ -291,13 +297,15 @@ fn interval_to_string(interval: &Interval) -> String { #[cfg(test)] pub mod tests { - use super::*; - use crate::version::NumberVersion; use proptest::prelude::*; + use crate::version::NumberVersion; + + use super::*; + pub fn strategy() -> impl Strategy> { prop::collection::vec(any::(), 0..10).prop_map(|mut vec| { - vec.sort(); + vec.sort_unstable(); vec.dedup(); let mut pair_iter = vec.chunks_exact(2); let mut segments = Vec::with_capacity(vec.len() / 2 + 1); @@ -312,7 +320,7 @@ pub mod tests { } fn version_strat() -> impl Strategy { - any::().prop_map(|x| NumberVersion(x)) + any::().prop_map(NumberVersion) } proptest! { diff --git a/src/report.rs b/src/report.rs index ba120284..7b8688ea 100644 --- a/src/report.rs +++ b/src/report.rs @@ -82,14 +82,14 @@ impl DerivationTree { *self = cause2 .clone() .merge_noversion(p.to_owned(), r.to_owned()) - .unwrap_or(self.to_owned()); + .unwrap_or_else(|| self.to_owned()); } (ref mut cause1, DerivationTree::External(External::NoVersion(p, r))) => { cause1.collapse_noversion(); *self = cause1 .clone() .merge_noversion(p.to_owned(), r.to_owned()) - .unwrap_or(self.to_owned()); + .unwrap_or_else(|| self.to_owned()); } _ => { derived.cause1.collapse_noversion(); @@ -200,12 +200,12 @@ impl DefaultStringReporter { fn build_recursive(&mut self, derived: &Derived) { self.build_recursive_helper(derived); - derived.shared_id.map(|id| { + if let Some(id) = derived.shared_id { if self.shared_with_ref.get(&id) == None { self.add_line_ref(); self.shared_with_ref.insert(id, self.ref_count); } - }); + }; } fn build_recursive_helper(&mut self, current: &Derived) { @@ -460,9 +460,9 @@ impl DefaultStringReporter { fn add_line_ref(&mut self) { let new_count = self.ref_count + 1; self.ref_count = new_count; - self.lines - .last_mut() - .map(|line| *line = format!("{} ({})", line, new_count)); + if let Some(line) = self.lines.last_mut() { + *line = format!("{} ({})", line, new_count); + } } fn line_ref_of(&self, shared_id: Option) -> Option { diff --git a/src/solver.rs b/src/solver.rs index bc487201..ad074df9 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -98,12 +98,11 @@ pub fn resolve( // Pick the next package. let (p, term) = match state.partial_solution.pick_package(dependency_provider)? { None => { - return state - .partial_solution - .extract_solution() - .ok_or(PubGrubError::Failure( + return state.partial_solution.extract_solution().ok_or_else(|| { + PubGrubError::Failure( "How did we end up with no package to choose but no solution?".into(), - )) + ) + }) } Some(x) => x, }; @@ -163,9 +162,9 @@ pub fn resolve( { // For a dependency incompatibility to be terminal, // it can only mean that root depend on not root? - Err(PubGrubError::Failure( + return Err(PubGrubError::Failure( "Root package depends on itself at a different version?".into(), - ))?; + )); } state.partial_solution.add_version(p, v, &dep_incompats); } @@ -198,7 +197,7 @@ pub trait DependencyProvider { } /// A basic implementation of [DependencyProvider]. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "serde", serde(transparent))] pub struct OfflineDependencyProvider { @@ -270,7 +269,7 @@ impl DependencyProvider for OfflineDependen Ok(self .versions(package) .map(|v| v.into_iter().rev().collect()) - .unwrap_or(Vec::new())) + .unwrap_or_default()) } fn get_dependencies( diff --git a/src/term.rs b/src/term.rs index d20503e1..ed8b5da4 100644 --- a/src/term.rs +++ b/src/term.rs @@ -146,7 +146,7 @@ impl<'a, V: 'a + Version> Term { /// Otherwise the relation is inconclusive. pub(crate) fn relation_with(&self, other_terms_intersection: &Term) -> Relation { let full_intersection = self.intersection(other_terms_intersection.as_ref()); - if &full_intersection == other_terms_intersection.as_ref() { + if &full_intersection == other_terms_intersection { Relation::Satisfied } else if full_intersection == Self::empty() { Relation::Contradicted @@ -183,8 +183,8 @@ pub mod tests { pub fn strategy() -> impl Strategy> { prop_oneof![ - crate::range::tests::strategy().prop_map(|range| Term::Positive(range)), - crate::range::tests::strategy().prop_map(|range| Term::Negative(range)), + crate::range::tests::strategy().prop_map(Term::Positive), + crate::range::tests::strategy().prop_map(Term::Negative), ] } diff --git a/tests/proptest.rs b/tests/proptest.rs index a11730b2..bf85f933 100644 --- a/tests/proptest.rs +++ b/tests/proptest.rs @@ -326,8 +326,8 @@ proptest! { (Ok(l), Ok(r)) => assert_eq!(l, r), (Err(PubGrubError::NoSolution(derivation_l)), Err(PubGrubError::NoSolution(derivation_r))) => { prop_assert_eq!( - format!("{}", DefaultStringReporter::report(&derivation_l)), - format!("{}", DefaultStringReporter::report(&derivation_r)) + DefaultStringReporter::report(&derivation_l).to_string(), + DefaultStringReporter::report(&derivation_r).to_string() )}, _ => panic!("not the same result") } @@ -366,7 +366,7 @@ proptest! { let version = version_idx.get(&versions); let dependencys: Vec<_> = dependency_provider.get_dependencies(package, version).unwrap().unwrap().into_iter().collect(); if !dependencys.is_empty() { - let dependency = dep_idx.get(&dependencys).0.clone(); + let dependency = dep_idx.get(&dependencys).0; removed_provider.add_dependencies( **package, *version,