rust: clippy pass

This removes some justified warnings and one hard error that, while technically
not a bug, was an ugly oversight on my part.

Differential Revision: https://phab.mercurial-scm.org/D9094

rust/hg-core/src/dirstate/dirstate_tree/iter.rs

@@ -1,392 +1,392 b''
 // iter.rs
 //
 // Copyright 2020, Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 use super::node::{Node, NodeKind};
 use super::tree::Tree;
 use crate::dirstate::dirstate_tree::node::Directory;
 use crate::dirstate::status::Dispatch;
 use crate::utils::hg_path::{hg_path_to_path_buf, HgPath, HgPathBuf};
 use crate::DirstateEntry;
 use std::borrow::Cow;
 use std::collections::VecDeque;
 use std::iter::{FromIterator, FusedIterator};
 use std::path::PathBuf;
 
 impl FromIterator<(HgPathBuf, DirstateEntry)> for Tree {
     fn from_iter<T: IntoIterator<Item = (HgPathBuf, DirstateEntry)>>(
         iter: T,
     ) -> Self {
         let mut tree = Self::new();
         for (path, entry) in iter {
             tree.insert(path, entry);
         }
         tree
     }
 }
 
 /// Iterator of all entries in the dirstate tree.
 ///
 /// It has no particular ordering.
 pub struct Iter<'a> {
     to_visit: VecDeque<(Cow<'a, [u8]>, &'a Node)>,
 }
 
 impl<'a> Iter<'a> {
     pub fn new(node: &'a Node) -> Iter<'a> {
         let mut to_visit = VecDeque::new();
         to_visit.push_back((Cow::Borrowed(&b""[..]), node));
         Self { to_visit }
     }
 }
 
 impl<'a> Iterator for Iter<'a> {
     type Item = (HgPathBuf, DirstateEntry);
 
     fn next(&mut self) -> Option<Self::Item> {
         while let Some((base_path, node)) = self.to_visit.pop_front() {
             match &node.kind {
                 NodeKind::Directory(dir) => {
                     add_children_to_visit(
                         &mut self.to_visit,
                         &base_path,
                         &dir,
                     );
                     if let Some(file) = &dir.was_file {
                         return Some((
                             HgPathBuf::from_bytes(&base_path),
                             file.entry,
                         ));
                     }
                 }
                 NodeKind::File(file) => {
                     if let Some(dir) = &file.was_directory {
                         add_children_to_visit(
                             &mut self.to_visit,
                             &base_path,
                             &dir,
                         );
                     }
                     return Some((
                         HgPathBuf::from_bytes(&base_path),
                         file.entry,
                     ));
                 }
             }
         }
         None
     }
 }
 
 impl<'a> FusedIterator for Iter<'a> {}
 
 /// Iterator of all entries in the dirstate tree, with a special filesystem
 /// handling for the directories containing said entries.
 ///
 /// It checks every directory on-disk to see if it has become a symlink, to
 /// prevent a potential security issue.
 /// Using this information, it may dispatch `status` information early: it
 /// returns canonical paths along with `Shortcut`s, which are either a
 /// `DirstateEntry` or a `Dispatch`, if the fate of said path has already been
 /// determined.
 ///
 /// Like `Iter`, it has no particular ordering.
 pub struct FsIter<'a> {
     root_dir: PathBuf,
     to_visit: VecDeque<(Cow<'a, [u8]>, &'a Node)>,
     shortcuts: VecDeque<(HgPathBuf, StatusShortcut)>,
 }
 
 impl<'a> FsIter<'a> {
     pub fn new(node: &'a Node, root_dir: PathBuf) -> FsIter<'a> {
         let mut to_visit = VecDeque::new();
         to_visit.push_back((Cow::Borrowed(&b""[..]), node));
         Self {
             root_dir,
             to_visit,
             shortcuts: Default::default(),
         }
     }
 
     /// Mercurial tracks symlinks but *not* what they point to.
     /// If a directory is moved and symlinked:
     ///
     /// ```bash
     /// $ mkdir foo
     /// $ touch foo/a
     /// $ # commit...
     /// $ mv foo bar
     /// $ ln -s bar foo
     /// ```
     /// We need to dispatch the new symlink as `Unknown` and all the
     /// descendents of the directory it replace as `Deleted`.
     fn dispatch_symlinked_directory(
         &mut self,
         path: impl AsRef<HgPath>,
         node: &Node,
     ) {
         let path = path.as_ref();
         self.shortcuts.push_back((
             path.to_owned(),
             StatusShortcut::Dispatch(Dispatch::Unknown),
         ));
         for (file, _) in node.iter() {
             self.shortcuts.push_back((
                 path.join(&file),
                 StatusShortcut::Dispatch(Dispatch::Deleted),
             ));
         }
     }
 
     /// Returns `true` if the canonical `path` of a directory corresponds to a
     /// symlink on disk. It means it was moved and symlinked after the last
     /// dirstate update.
     ///
     /// # Special cases
     ///
     /// Returns `false` for the repository root.
     /// Returns `false` on io error, error handling is outside of the iterator.
     fn directory_became_symlink(&mut self, path: &HgPath) -> bool {
         if path.is_empty() {
             return false;
         }
         let filename_as_path = match hg_path_to_path_buf(&path) {
             Ok(p) => p,
             _ => return false,
         };
         let meta = self.root_dir.join(filename_as_path).symlink_metadata();
         match meta {
             Ok(ref m) if m.file_type().is_symlink() => true,
-            _ => return false,
+            _ => false,
         }
     }
 }
 
 /// Returned by `FsIter`, since the `Dispatch` of any given entry may already
 /// be determined during the iteration. This is necessary for performance
 /// reasons, since hierarchical information is needed to `Dispatch` an entire
 /// subtree efficiently.
 #[derive(Debug, Copy, Clone)]
 pub enum StatusShortcut {
     /// A entry in the dirstate for further inspection
     Entry(DirstateEntry),
     /// The result of the status of the corresponding file
     Dispatch(Dispatch),
 }
 
 impl<'a> Iterator for FsIter<'a> {
     type Item = (HgPathBuf, StatusShortcut);
 
     fn next(&mut self) -> Option<Self::Item> {
         // If any paths have already been `Dispatch`-ed, return them
-        while let Some(res) = self.shortcuts.pop_front() {
+        if let Some(res) = self.shortcuts.pop_front() {
             return Some(res);
         }
 
         while let Some((base_path, node)) = self.to_visit.pop_front() {
             match &node.kind {
                 NodeKind::Directory(dir) => {
                     let canonical_path = HgPath::new(&base_path);
                     if self.directory_became_symlink(canonical_path) {
                         // Potential security issue, don't do a normal
                         // traversal, force the results.
                         self.dispatch_symlinked_directory(
                             canonical_path,
                             &node,
                         );
                         continue;
                     }
                     add_children_to_visit(
                         &mut self.to_visit,
                         &base_path,
                         &dir,
                     );
                     if let Some(file) = &dir.was_file {
                         return Some((
                             HgPathBuf::from_bytes(&base_path),
                             StatusShortcut::Entry(file.entry),
                         ));
                     }
                 }
                 NodeKind::File(file) => {
                     if let Some(dir) = &file.was_directory {
                         add_children_to_visit(
                             &mut self.to_visit,
                             &base_path,
                             &dir,
                         );
                     }
                     return Some((
                         HgPathBuf::from_bytes(&base_path),
                         StatusShortcut::Entry(file.entry),
                     ));
                 }
             }
         }
 
         None
     }
 }
 
 impl<'a> FusedIterator for FsIter<'a> {}
 
 fn join_path<'a, 'b>(path: &'a [u8], other: &'b [u8]) -> Cow<'b, [u8]> {
     if path.is_empty() {
         other.into()
     } else {
         [path, &b"/"[..], other].concat().into()
     }
 }
 
 /// Adds all children of a given directory `dir` to the visit queue `to_visit`
 /// prefixed by a `base_path`.
 fn add_children_to_visit<'a>(
     to_visit: &mut VecDeque<(Cow<'a, [u8]>, &'a Node)>,
     base_path: &[u8],
     dir: &'a Directory,
 ) {
     to_visit.extend(dir.children.iter().map(|(path, child)| {
         let full_path = join_path(&base_path, &path);
-        (Cow::from(full_path), child)
+        (full_path, child)
     }));
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::utils::hg_path::HgPath;
     use crate::{EntryState, FastHashMap};
     use std::collections::HashSet;
 
     #[test]
     fn test_iteration() {
         let mut tree = Tree::new();
 
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"foo/bar"),
                 DirstateEntry {
                     state: EntryState::Merged,
                     mode: 41,
                     mtime: 42,
                     size: 43,
                 }
             ),
             None
         );
 
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"foo2"),
                 DirstateEntry {
                     state: EntryState::Merged,
                     mode: 40,
                     mtime: 41,
                     size: 42,
                 }
             ),
             None
         );
 
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"foo/baz"),
                 DirstateEntry {
                     state: EntryState::Normal,
                     mode: 0,
                     mtime: 0,
                     size: 0,
                 }
             ),
             None
         );
 
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"foo/bap/nested"),
                 DirstateEntry {
                     state: EntryState::Normal,
                     mode: 0,
                     mtime: 0,
                     size: 0,
                 }
             ),
             None
         );
 
         assert_eq!(tree.len(), 4);
 
         let results: HashSet<_> =
             tree.iter().map(|(c, _)| c.to_owned()).collect();
         dbg!(&results);
         assert!(results.contains(HgPath::new(b"foo2")));
         assert!(results.contains(HgPath::new(b"foo/bar")));
         assert!(results.contains(HgPath::new(b"foo/baz")));
         assert!(results.contains(HgPath::new(b"foo/bap/nested")));
 
         let mut iter = tree.iter();
         assert!(iter.next().is_some());
         assert!(iter.next().is_some());
         assert!(iter.next().is_some());
         assert!(iter.next().is_some());
         assert_eq!(None, iter.next());
         assert_eq!(None, iter.next());
         drop(iter);
 
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"foo/bap/nested/a"),
                 DirstateEntry {
                     state: EntryState::Normal,
                     mode: 0,
                     mtime: 0,
                     size: 0,
                 }
             ),
             None
         );
 
         let results: FastHashMap<_, _> = tree.iter().collect();
         assert!(results.contains_key(HgPath::new(b"foo2")));
         assert!(results.contains_key(HgPath::new(b"foo/bar")));
         assert!(results.contains_key(HgPath::new(b"foo/baz")));
         // Is a dir but `was_file`, so it's listed as a removed file
         assert!(results.contains_key(HgPath::new(b"foo/bap/nested")));
         assert!(results.contains_key(HgPath::new(b"foo/bap/nested/a")));
 
         // insert removed file (now directory) after nested file
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"a/a"),
                 DirstateEntry {
                     state: EntryState::Normal,
                     mode: 0,
                     mtime: 0,
                     size: 0,
                 }
             ),
             None
         );
 
         // `insert` returns `None` for a directory
         assert_eq!(
             tree.insert(
                 HgPathBuf::from_bytes(b"a"),
                 DirstateEntry {
                     state: EntryState::Removed,
                     mode: 0,
                     mtime: 0,
                     size: 0,
                 }
             ),
             None
         );
 
         let results: FastHashMap<_, _> = tree.iter().collect();
         assert!(results.contains_key(HgPath::new(b"a")));
         assert!(results.contains_key(HgPath::new(b"a/a")));
     }
 }

rust/hg-core/src/dirstate/dirstate_tree/node.rs

@@ -1,395 +1,395 b''
 // node.rs
 //
 // Copyright 2020, Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 use super::iter::Iter;
 use crate::utils::hg_path::HgPathBuf;
 use crate::{DirstateEntry, EntryState, FastHashMap};
 
 /// Represents a filesystem directory in the dirstate tree
 #[derive(Debug, Default, Clone, PartialEq)]
 pub struct Directory {
     /// Contains the old file information if it existed between changesets.
     /// Happens if a file `foo` is marked as removed, removed from the
     /// filesystem then a directory `foo` is created and at least one of its
     /// descendents is added to Mercurial.
     pub(super) was_file: Option<Box<File>>,
     pub(super) children: FastHashMap<Vec<u8>, Node>,
 }
 
 /// Represents a filesystem file (or symlink) in the dirstate tree
 #[derive(Debug, Clone, PartialEq)]
 pub struct File {
     /// Contains the old structure if it existed between changesets.
     /// Happens all descendents of `foo` marked as removed and removed from
     /// the filesystem, then a file `foo` is created and added to Mercurial.
     pub(super) was_directory: Option<Box<Directory>>,
     pub(super) entry: DirstateEntry,
 }
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum NodeKind {
     Directory(Directory),
     File(File),
 }
 
 #[derive(Debug, Default, Clone, PartialEq)]
 pub struct Node {
     pub kind: NodeKind,
 }
 
 impl Default for NodeKind {
     fn default() -> Self {
         NodeKind::Directory(Default::default())
     }
 }
 
 impl Node {
     pub fn insert(
         &mut self,
         path: &[u8],
         new_entry: DirstateEntry,
     ) -> InsertResult {
         let mut split = path.splitn(2, |&c| c == b'/');
         let head = split.next().unwrap_or(b"");
         let tail = split.next().unwrap_or(b"");
 
         if let NodeKind::File(file) = &mut self.kind {
             if tail.is_empty() && head.is_empty() {
                 // We're modifying the current file
                 let new = Self {
                     kind: NodeKind::File(File {
                         entry: new_entry,
                         ..file.clone()
                     }),
                 };
                 return InsertResult {
                     did_insert: false,
                     old_entry: Some(std::mem::replace(self, new)),
                 };
             } else {
                 match file.entry.state {
                     // Only replace the current file with a directory if it's
                     // marked as `Removed`
                     EntryState::Removed => {
                         self.kind = NodeKind::Directory(Directory {
                             was_file: Some(Box::from(file.clone())),
                             children: Default::default(),
                         })
                     }
                     _ => {
                         return Node::insert_in_file(
                             file, new_entry, head, tail,
                         )
                     }
                 }
             }
         }
 
         match &mut self.kind {
             NodeKind::Directory(directory) => {
-                return Node::insert_in_directory(
+                Node::insert_in_directory(
                     directory, new_entry, head, tail,
-                );
+                )
             }
             NodeKind::File(_) => {
                 unreachable!("The file case has already been handled")
             }
         }
     }
 
     /// The current file still exists and is not marked as `Removed`.
     /// Insert the entry in its `was_directory`.
     fn insert_in_file(
         file: &mut File,
         new_entry: DirstateEntry,
         head: &[u8],
         tail: &[u8],
     ) -> InsertResult {
         if let Some(d) = &mut file.was_directory {
             Node::insert_in_directory(d, new_entry, head, tail)
         } else {
             let mut dir = Directory {
                 was_file: None,
                 children: FastHashMap::default(),
             };
             let res =
                 Node::insert_in_directory(&mut dir, new_entry, head, tail);
             file.was_directory = Some(Box::new(dir));
             res
         }
     }
 
     /// Insert an entry in the subtree of `directory`
     fn insert_in_directory(
         directory: &mut Directory,
         new_entry: DirstateEntry,
         head: &[u8],
         tail: &[u8],
     ) -> InsertResult {
         let mut res = InsertResult::default();
 
         if let Some(node) = directory.children.get_mut(head) {
             // Node exists
             match &mut node.kind {
                 NodeKind::Directory(subdir) => {
                     if tail.is_empty() {
                         let becomes_file = Self {
                             kind: NodeKind::File(File {
                                 was_directory: Some(Box::from(subdir.clone())),
                                 entry: new_entry,
                             }),
                         };
                         let old_entry = directory
                             .children
                             .insert(head.to_owned(), becomes_file);
                         return InsertResult {
                             did_insert: true,
                             old_entry,
                         };
                     } else {
                         res = node.insert(tail, new_entry);
                     }
                 }
                 NodeKind::File(_) => {
                     res = node.insert(tail, new_entry);
                 }
             }
         } else if tail.is_empty() {
             // File does not already exist
             directory.children.insert(
                 head.to_owned(),
                 Self {
                     kind: NodeKind::File(File {
                         was_directory: None,
                         entry: new_entry,
                     }),
                 },
             );
             res.did_insert = true;
         } else {
             // Directory does not already exist
             let mut nested = Self {
                 kind: NodeKind::Directory(Directory {
                     was_file: None,
                     children: Default::default(),
                 }),
             };
             res = nested.insert(tail, new_entry);
             directory.children.insert(head.to_owned(), nested);
         }
         res
     }
 
     /// Removes an entry from the tree, returns a `RemoveResult`.
     pub fn remove(&mut self, path: &[u8]) -> RemoveResult {
         let empty_result = RemoveResult::default();
         if path.is_empty() {
             return empty_result;
         }
         let mut split = path.splitn(2, |&c| c == b'/');
         let head = split.next();
         let tail = split.next().unwrap_or(b"");
 
         let head = match head {
             None => {
                 return empty_result;
             }
             Some(h) => h,
         };
         if head == path {
             match &mut self.kind {
                 NodeKind::Directory(d) => {
                     return Node::remove_from_directory(head, d);
                 }
                 NodeKind::File(f) => {
                     if let Some(d) = &mut f.was_directory {
                         let RemoveResult { old_entry, .. } =
                             Node::remove_from_directory(head, d);
                         return RemoveResult {
                             cleanup: false,
                             old_entry,
                         };
                     }
                 }
             }
             empty_result
         } else {
             // Look into the dirs
             match &mut self.kind {
                 NodeKind::Directory(d) => {
                     if let Some(child) = d.children.get_mut(head) {
                         let mut res = child.remove(tail);
                         if res.cleanup {
                             d.children.remove(head);
                         }
                         res.cleanup =
-                            d.children.len() == 0 && d.was_file.is_none();
+                            d.children.is_empty() && d.was_file.is_none();
                         res
                     } else {
                         empty_result
                     }
                 }
                 NodeKind::File(f) => {
                     if let Some(d) = &mut f.was_directory {
                         if let Some(child) = d.children.get_mut(head) {
                             let RemoveResult { cleanup, old_entry } =
                                 child.remove(tail);
                             if cleanup {
                                 d.children.remove(head);
                             }
-                            if d.children.len() == 0 && d.was_file.is_none() {
+                            if d.children.is_empty() && d.was_file.is_none() {
                                 f.was_directory = None;
                             }
 
                             return RemoveResult {
                                 cleanup: false,
                                 old_entry,
                             };
                         }
                     }
                     empty_result
                 }
             }
         }
     }
 
     fn remove_from_directory(head: &[u8], d: &mut Directory) -> RemoveResult {
         if let Some(node) = d.children.get_mut(head) {
             return match &mut node.kind {
                 NodeKind::Directory(d) => {
                     if let Some(f) = &mut d.was_file {
                         let entry = f.entry;
                         d.was_file = None;
                         RemoveResult {
                             cleanup: false,
                             old_entry: Some(entry),
                         }
                     } else {
                         RemoveResult::default()
                     }
                 }
                 NodeKind::File(f) => {
                     let entry = f.entry;
                     let mut cleanup = false;
                     match &f.was_directory {
                         None => {
                             if d.children.len() == 1 {
                                 cleanup = true;
                             }
                             d.children.remove(head);
                         }
                         Some(dir) => {
                             node.kind = NodeKind::Directory(*dir.clone());
                         }
                     }
 
                     RemoveResult {
-                        cleanup: cleanup,
+                        cleanup,
                         old_entry: Some(entry),
                     }
                 }
             };
         }
         RemoveResult::default()
     }
 
     pub fn get(&self, path: &[u8]) -> Option<&Node> {
         if path.is_empty() {
             return Some(&self);
         }
         let mut split = path.splitn(2, |&c| c == b'/');
         let head = split.next();
         let tail = split.next().unwrap_or(b"");
 
         let head = match head {
             None => {
                 return Some(&self);
             }
             Some(h) => h,
         };
         match &self.kind {
             NodeKind::Directory(d) => {
                 if let Some(child) = d.children.get(head) {
                     return child.get(tail);
                 }
             }
             NodeKind::File(f) => {
                 if let Some(d) = &f.was_directory {
                     if let Some(child) = d.children.get(head) {
                         return child.get(tail);
                     }
                 }
             }
         }
 
         None
     }
 
     pub fn get_mut(&mut self, path: &[u8]) -> Option<&mut NodeKind> {
         if path.is_empty() {
             return Some(&mut self.kind);
         }
         let mut split = path.splitn(2, |&c| c == b'/');
         let head = split.next();
         let tail = split.next().unwrap_or(b"");
 
         let head = match head {
             None => {
                 return Some(&mut self.kind);
             }
             Some(h) => h,
         };
         match &mut self.kind {
             NodeKind::Directory(d) => {
                 if let Some(child) = d.children.get_mut(head) {
                     return child.get_mut(tail);
                 }
             }
             NodeKind::File(f) => {
                 if let Some(d) = &mut f.was_directory {
                     if let Some(child) = d.children.get_mut(head) {
                         return child.get_mut(tail);
                     }
                 }
             }
         }
 
         None
     }
 
     pub fn iter(&self) -> Iter {
         Iter::new(self)
     }
 }
 
 /// Information returned to the caller of an `insert` operation for integrity.
 #[derive(Debug, Default)]
 pub struct InsertResult {
     /// Whether the insertion resulted in an actual insertion and not an
     /// update
     pub(super) did_insert: bool,
     /// The entry that was replaced, if it exists
     pub(super) old_entry: Option<Node>,
 }
 
 /// Information returned to the caller of a `remove` operation integrity.
 #[derive(Debug, Default)]
 pub struct RemoveResult {
     /// If the caller needs to remove the current node
     pub(super) cleanup: bool,
     /// The entry that was replaced, if it exists
     pub(super) old_entry: Option<DirstateEntry>,
 }
 
 impl<'a> IntoIterator for &'a Node {
     type Item = (HgPathBuf, DirstateEntry);
     type IntoIter = Iter<'a>;
 
     fn into_iter(self) -> Self::IntoIter {
         self.iter()
     }
 }