Skip to content

Commit

Permalink
refactor: apply Clippy suggestions (#52)
Browse files Browse the repository at this point in the history 
* refactor: apply trivial Clippy suggestions

* refactor: cargo clippy --fix

* refactor: rewrite if chain using cmp() and match

* feat: derive Default for OfflineDependencyProvider
  • Loading branch information
@aleksator
aleksator committed Oct 25, 2020
1 parent 25b8651 commit cbc6d4d
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 42 deletions.
10 changes: 5 additions & 5 deletions src/internal/memory.rs
View file
Expand Up @@ -56,23 +56,23 @@ impl<P: Package, V: Version> Memory<P, V> {
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.
fn add_derivation(&mut self, package: P, term: Term<V>) {
let pa = self
.assignments
.entry(package)
.or_insert(PackageAssignments::new());
.or_insert_with(PackageAssignments::new);
pa.derivations_not_intersected_yet.push(term);
}

Expand Down
38 changes: 23 additions & 15 deletions src/range.rs
View file
Expand Up @@ -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))]
Expand Down Expand Up @@ -183,34 +185,38 @@ impl<V: Version> Range<V> {
}

// 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))) => {
Expand Down Expand Up @@ -291,13 +297,15 @@ fn interval_to_string<V: Version>(interval: &Interval<V>) -> 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<Value = Range<NumberVersion>> {
prop::collection::vec(any::<u32>(), 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);
Expand All @@ -312,7 +320,7 @@ pub mod tests {
}

fn version_strat() -> impl Strategy<Value = NumberVersion> {
any::<u32>().prop_map(|x| NumberVersion(x))
any::<u32>().prop_map(NumberVersion)
}

proptest! {
Expand Down
14 changes: 7 additions & 7 deletions src/report.rs
View file
Expand Up @@ -82,14 +82,14 @@ impl<P: Package, V: Version> DerivationTree<P, V> {
*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();
Expand Down Expand Up @@ -200,12 +200,12 @@ impl DefaultStringReporter {

fn build_recursive<P: Package, V: Version>(&mut self, derived: &Derived<P, V>) {
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<P: Package, V: Version>(&mut self, current: &Derived<P, V>) {
Expand Down Expand Up @@ -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<usize>) -> Option<usize> {
Expand Down
17 changes: 8 additions & 9 deletions src/solver.rs
View file
Expand Up @@ -98,12 +98,11 @@ pub fn resolve<P: Package, V: Version>(
// 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,
};
Expand Down Expand Up @@ -163,9 +162,9 @@ pub fn resolve<P: Package, V: Version>(
{
// 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);
}
Expand Down Expand Up @@ -198,7 +197,7 @@ pub trait DependencyProvider<P: Package, V: Version> {
}

/// 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<P: Package, V: Version + Hash> {
Expand Down Expand Up @@ -270,7 +269,7 @@ impl<P: Package, V: Version + Hash> DependencyProvider<P, V> for OfflineDependen
Ok(self
.versions(package)
.map(|v| v.into_iter().rev().collect())
.unwrap_or(Vec::new()))
.unwrap_or_default())
}

fn get_dependencies(
Expand Down
6 changes: 3 additions & 3 deletions src/term.rs
View file
Expand Up @@ -146,7 +146,7 @@ impl<'a, V: 'a + Version> Term<V> {
/// Otherwise the relation is inconclusive.
pub(crate) fn relation_with(&self, other_terms_intersection: &Term<V>) -> 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
Expand Down Expand Up @@ -183,8 +183,8 @@ pub mod tests {

pub fn strategy() -> impl Strategy<Value = Term<NumberVersion>> {
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),
]
}

Expand Down
6 changes: 3 additions & 3 deletions tests/proptest.rs
View file
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit cbc6d4d

Please sign in to comment.