Auto merge of #6134 - Eh2406:dead-code, r=alexcrichton

remove dead code, use impl Iterator instead of custom types, cargo fmt

This is a mishmash of things. `Graph::sort` was unused so I removed it. There are a bunch of custom types that allow us to return iterators that can now be replaced with `impl Iterator`. jetbrains wanted to reorder some impls to match the definition. fmt also made some changes to the files I opened.

Author: bors

src/cargo/core/resolver/mod.rs

@@ -68,7 +68,7 @@ use self::types::{RcVecIter, RegistryQueryer, RemainingDeps, ResolverProgress};
 
 pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
 pub use self::encode::{Metadata, WorkspaceResolve};
-pub use self::resolve::{Deps, DepsNotReplaced, Resolve};
+pub use self::resolve::Resolve;
 pub use self::types::Method;
 
 mod conflict_cache;

src/cargo/core/resolver/resolve.rs

@@ -5,9 +5,8 @@ use std::iter::FromIterator;
 use url::Url;
 
 use core::{Dependency, PackageId, PackageIdSpec, Summary, Target};
-use util::Graph;
 use util::errors::CargoResult;
-use util::graph::{Edges, Nodes};
+use util::Graph;
 
 use super::encode::Metadata;
 
@@ -162,21 +161,18 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         Ok(())
     }
 
-    pub fn iter(&self) -> Nodes<PackageId, Vec<Dependency>> {
+    pub fn iter(&self) -> impl Iterator<Item = &PackageId> {
         self.graph.iter()
     }
 
-    pub fn deps(&self, pkg: &PackageId) -> Deps {
-        Deps {
-            edges: self.graph.edges(pkg),
-            resolve: self,
-        }
+    pub fn deps(&self, pkg: &PackageId) -> impl Iterator<Item = (&PackageId, &[Dependency])> {
+        self.graph
+            .edges(pkg)
+            .map(move |(id, deps)| (self.replacement(id).unwrap_or(id), deps.as_slice()))
     }
 
-    pub fn deps_not_replaced(&self, pkg: &PackageId) -> DepsNotReplaced {
-        DepsNotReplaced {
-            edges: self.graph.edges(pkg),
-        }
+    pub fn deps_not_replaced(&self, pkg: &PackageId) -> impl Iterator<Item = &PackageId> {
+        self.graph.edges(pkg).map(|(id, _)| id)
     }
 
     pub fn replacement(&self, pkg: &PackageId) -> Option<&PackageId> {
@@ -226,16 +222,22 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         };
 
         let crate_name = to_target.crate_name();
-        let mut names = deps.iter()
-            .map(|d| d.explicit_name_in_toml().map(|s| s.as_str()).unwrap_or(&crate_name));
+        let mut names = deps.iter().map(|d| {
+            d.explicit_name_in_toml()
+                .map(|s| s.as_str())
+                .unwrap_or(&crate_name)
+        });
         let name = names.next().unwrap_or(&crate_name);
         for n in names {
             if n == name {
-                continue
+                continue;
             }
-            bail!("multiple dependencies listed for the same crate must \
-                   all have the same name, but the dependency on `{}` \
-                   is listed as having different names", to);
+            bail!(
+                "multiple dependencies listed for the same crate must \
+                 all have the same name, but the dependency on `{}` \
+                 is listed as having different names",
+                to
+            );
         }
         Ok(name.to_string())
     }
@@ -272,48 +274,3 @@ impl fmt::Debug for Resolve {
         write!(fmt, "}}")
     }
 }
-
-pub struct Deps<'a> {
-    edges: Option<Edges<'a, PackageId, Vec<Dependency>>>,
-    resolve: &'a Resolve,
-}
-
-impl<'a> Iterator for Deps<'a> {
-    type Item = (&'a PackageId, &'a [Dependency]);
-
-    fn next(&mut self) -> Option<(&'a PackageId, &'a [Dependency])> {
-        let (id, deps) = self.edges.as_mut()?.next()?;
-        let id_ret = self.resolve.replacement(id).unwrap_or(id);
-        Some((id_ret, deps))
-    }
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        // Note: Edges is actually a std::collections::hash_set::Iter, which
-        // is an ExactSizeIterator.
-        let len = self.edges.as_ref().map(ExactSizeIterator::len).unwrap_or(0);
-        (len, Some(len))
-    }
-}
-
-impl<'a> ExactSizeIterator for Deps<'a> {}
-
-pub struct DepsNotReplaced<'a> {
-    edges: Option<Edges<'a, PackageId, Vec<Dependency>>>,
-}
-
-impl<'a> Iterator for DepsNotReplaced<'a> {
-    type Item = &'a PackageId;
-
-    fn next(&mut self) -> Option<&'a PackageId> {
-        Some(self.edges.as_mut()?.next()?.0)
-    }
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        // Note: Edges is actually a std::collections::hash_set::Iter, which
-        // is an ExactSizeIterator.
-        let len = self.edges.as_ref().map(ExactSizeIterator::len).unwrap_or(0);
-        (len, Some(len))
-    }
-}
-
-impl<'a> ExactSizeIterator for DepsNotReplaced<'a> {}

src/cargo/core/source/mod.rs

@@ -1,4 +1,4 @@
-use std::collections::hash_map::{HashMap, IterMut, Values};
+use std::collections::hash_map::HashMap;
 use std::fmt;
 
 use core::{Dependency, Package, PackageId, Summary};
@@ -51,8 +51,7 @@ pub trait Source {
     /// version specified.
     fn download(&mut self, package: &PackageId) -> CargoResult<MaybePackage>;
 
-    fn finish_download(&mut self, package: &PackageId, contents: Vec<u8>)
-        -> CargoResult<Package>;
+    fn finish_download(&mut self, package: &PackageId, contents: Vec<u8>) -> CargoResult<Package>;
 
     /// Generates a unique string which represents the fingerprint of the
     /// current state of the source.
@@ -88,13 +87,20 @@ pub trait Source {
 
 pub enum MaybePackage {
     Ready(Package),
-    Download {
-        url: String,
-        descriptor: String,
-    }
+    Download { url: String, descriptor: String },
 }
 
 impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
+    /// Forwards to `Source::source_id`
+    fn source_id(&self) -> &SourceId {
+        (**self).source_id()
+    }
+
+    /// Forwards to `Source::replaced_source_id`
+    fn replaced_source_id(&self) -> &SourceId {
+        (**self).replaced_source_id()
+    }
+
     /// Forwards to `Source::supports_checksums`
     fn supports_checksums(&self) -> bool {
         (**self).supports_checksums()
@@ -115,16 +121,6 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
         (**self).fuzzy_query(dep, f)
     }
 
-    /// Forwards to `Source::source_id`
-    fn source_id(&self) -> &SourceId {
-        (**self).source_id()
-    }
-
-    /// Forwards to `Source::replaced_source_id`
-    fn replaced_source_id(&self) -> &SourceId {
-        (**self).replaced_source_id()
-    }
-
     /// Forwards to `Source::update`
     fn update(&mut self) -> CargoResult<()> {
         (**self).update()
@@ -159,6 +155,14 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
 }
 
 impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T {
+    fn source_id(&self) -> &SourceId {
+        (**self).source_id()
+    }
+
+    fn replaced_source_id(&self) -> &SourceId {
+        (**self).replaced_source_id()
+    }
+
     fn supports_checksums(&self) -> bool {
         (**self).supports_checksums()
     }
@@ -175,14 +179,6 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T {
         (**self).fuzzy_query(dep, f)
     }
 
-    fn source_id(&self) -> &SourceId {
-        (**self).source_id()
-    }
-
-    fn replaced_source_id(&self) -> &SourceId {
-        (**self).replaced_source_id()
-    }
-
     fn update(&mut self) -> CargoResult<()> {
         (**self).update()
     }
@@ -226,14 +222,6 @@ impl<'src> fmt::Debug for SourceMap<'src> {
     }
 }
 
-/// A `std::collection::hash_map::Values` for `SourceMap`
-pub type Sources<'a, 'src> = Values<'a, SourceId, Box<Source + 'src>>;
-
-/// A `std::collection::hash_map::IterMut` for `SourceMap`
-pub struct SourcesMut<'a, 'src: 'a> {
-    inner: IterMut<'a, SourceId, Box<Source + 'src>>,
-}
-
 impl<'src> SourceMap<'src> {
     /// Create an empty map
     pub fn new() -> SourceMap<'src> {
@@ -288,21 +276,14 @@ impl<'src> SourceMap<'src> {
     }
 
     /// Like `HashMap::values`
-    pub fn sources<'a>(&'a self) -> Sources<'a, 'src> {
+    pub fn sources<'a>(&'a self) -> impl Iterator<Item = &'a Box<Source + 'src>> {
         self.map.values()
     }
 
     /// Like `HashMap::iter_mut`
-    pub fn sources_mut<'a>(&'a mut self) -> SourcesMut<'a, 'src> {
-        SourcesMut {
-            inner: self.map.iter_mut(),
-        }
-    }
-}
-
-impl<'a, 'src> Iterator for SourcesMut<'a, 'src> {
-    type Item = (&'a SourceId, &'a mut (Source + 'src));
-    fn next(&mut self) -> Option<(&'a SourceId, &'a mut (Source + 'src))> {
-        self.inner.next().map(|(a, b)| (a, &mut **b))
+    pub fn sources_mut<'a>(
+        &'a mut self,
+    ) -> impl Iterator<Item = (&'a SourceId, &'a mut (Source + 'src))> {
+        self.map.iter_mut().map(|(a, b)| (a, &mut **b))
     }
 }

src/cargo/util/graph.rs

@@ -1,19 +1,11 @@
+use std::collections::hash_map::HashMap;
 use std::fmt;
 use std::hash::Hash;
-use std::collections::hash_map::{HashMap, Iter, Keys};
 
 pub struct Graph<N, E> {
     nodes: HashMap<N, HashMap<N, E>>,
 }
 
-enum Mark {
-    InProgress,
-    Done,
-}
-
-pub type Nodes<'a, N, E> = Keys<'a, N, HashMap<N, E>>;
-pub type Edges<'a, N, E> = Iter<'a, N, E>;
-
 impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
     pub fn new() -> Graph<N, E> {
         Graph {
@@ -37,37 +29,11 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
         self.nodes.get(from)?.get(to)
     }
 
-    pub fn edges(&self, from: &N) -> Option<Edges<N, E>> {
-        self.nodes.get(from).map(|set| set.iter())
-    }
-
-    pub fn sort(&self) -> Option<Vec<N>> {
-        let mut ret = Vec::new();
-        let mut marks = HashMap::new();
-
-        for node in self.nodes.keys() {
-            self.visit(node, &mut ret, &mut marks);
-        }
-
-        Some(ret)
-    }
-
-    fn visit(&self, node: &N, dst: &mut Vec<N>, marks: &mut HashMap<N, Mark>) {
-        if marks.contains_key(node) {
-            return;
-        }
-
-        marks.insert(node.clone(), Mark::InProgress);
-
-        for child in self.nodes[node].keys() {
-            self.visit(child, dst, marks);
-        }
-
-        dst.push(node.clone());
-        marks.insert(node.clone(), Mark::Done);
+    pub fn edges(&self, from: &N) -> impl Iterator<Item = (&N, &E)> {
+        self.nodes.get(from).into_iter().flat_map(|x| x.iter())
     }
 
-    pub fn iter(&self) -> Nodes<N, E> {
+    pub fn iter(&self) -> impl Iterator<Item = &N> {
         self.nodes.keys()
     }
 
@@ -81,7 +47,7 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
         let first_pkg_depending_on = |pkg: &N, res: &[&N]| {
             self.nodes
                 .iter()
-                .filter(|&(_node, adjacent)| adjacent.contains_key(pkg))
+                .filter(|&(_, adjacent)| adjacent.contains_key(pkg))
                 // Note that we can have "cycles" introduced through dev-dependency
                 // edges, so make sure we don't loop infinitely.
                 .find(|&(node, _)| !res.contains(&node))

src/cargo/util/paths.rs

@@ -3,8 +3,8 @@ use std::ffi::{OsStr, OsString};
 use std::fs::{self, File, OpenOptions};
 use std::io;
 use std::io::prelude::*;
-use std::path::{Component, Path, PathBuf};
 use std::iter;
+use std::path::{Component, Path, PathBuf};
 
 use filetime::FileTime;
 
@@ -127,8 +127,7 @@ pub fn read_bytes(path: &Path) -> CargoResult<Vec<u8>> {
         }
         f.read_to_end(&mut ret)?;
         Ok(ret)
-    })()
-        .chain_err(|| format!("failed to read `{}`", path.display()))?;
+    })().chain_err(|| format!("failed to read `{}`", path.display()))?;
     Ok(res)
 }
 
@@ -137,8 +136,7 @@ pub fn write(path: &Path, contents: &[u8]) -> CargoResult<()> {
         let mut f = File::create(path)?;
         f.write_all(contents)?;
         Ok(())
-    })()
-        .chain_err(|| format!("failed to write `{}`", path.display()))?;
+    })().chain_err(|| format!("failed to write `{}`", path.display()))?;
     Ok(())
 }
 
@@ -158,8 +156,7 @@ pub fn write_if_changed<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) ->
             f.write_all(contents)?;
         }
         Ok(())
-    })()
-        .chain_err(|| format!("failed to write `{}`", path.as_ref().display()))?;
+    })().chain_err(|| format!("failed to write `{}`", path.as_ref().display()))?;
     Ok(())
 }
 
@@ -173,8 +170,7 @@ pub fn append(path: &Path, contents: &[u8]) -> CargoResult<()> {
 
         f.write_all(contents)?;
         Ok(())
-    })()
-        .chain_err(|| format!("failed to write `{}`", path.display()))?;
+    })().chain_err(|| format!("failed to write `{}`", path.display()))?;
     Ok(())
 }
 
@@ -198,8 +194,8 @@ pub fn path2bytes(path: &Path) -> CargoResult<&[u8]> {
 
 #[cfg(unix)]
 pub fn bytes2path(bytes: &[u8]) -> CargoResult<PathBuf> {
-    use std::os::unix::prelude::*;
     use std::ffi::OsStr;
+    use std::os::unix::prelude::*;
     Ok(PathBuf::from(OsStr::from_bytes(bytes)))
 }
 #[cfg(windows)]
@@ -258,7 +254,8 @@ fn _remove_dir_all(p: &Path) -> CargoResult<()> {
     if p.symlink_metadata()?.file_type().is_symlink() {
         return remove_file(p);
     }
-    let entries = p.read_dir()
+    let entries = p
+        .read_dir()
         .chain_err(|| format!("failed to read directory `{}`", p.display()))?;
     for entry in entries {
         let entry = entry?;