This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9094

rust: clippy pass
ClosedPublic

Authored by Alphare on Sep 28 2020, 8:24 AM.

Details

Reviewers
pulkit
marmoute
Group Reviewers
hg-reviewers
Commits
rHG2a169781556e: rust: clippy pass
Summary

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

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Sep 28 2020, 8:24 AM
Alphare added a comment.Sep 28 2020, 8:25 AM

Just to clarify, this fixes a hard error *in clippy*, not in the actual rust compiler. It just so happens that clippy is a very useful tool and I plan on maybe adding it to our CI to better catch stuff like this.

pulkit accepted this revision.Sep 30 2020, 8:51 AM
This revision is now accepted and ready to land.Sep 30 2020, 8:51 AM
marmoute accepted this revision.Sep 30 2020, 9:57 AM
Alphare added a commit: rHG2a169781556e: rust: clippy pass.Sep 30 2020, 10:17 AM
Closed by commit rHG2a169781556e: rust: clippy pass (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 22879

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

} }
let filename_as_path = match hg_path_to_path_buf(&path) { let filename_as_path = match hg_path_to_path_buf(&path) {
Ok(p) => p, Ok(p) => p,
_ => return false, _ => return false,
}; };
let meta = self.root_dir.join(filename_as_path).symlink_metadata(); let meta = self.root_dir.join(filename_as_path).symlink_metadata();
match meta { match meta {
Ok(ref m) if m.file_type().is_symlink() => true, 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 /// Returned by `FsIter`, since the `Dispatch` of any given entry may already
/// be determined during the iteration. This is necessary for performance /// be determined during the iteration. This is necessary for performance
/// reasons, since hierarchical information is needed to `Dispatch` an entire /// reasons, since hierarchical information is needed to `Dispatch` an entire
/// subtree efficiently. /// subtree efficiently.
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum StatusShortcut { pub enum StatusShortcut {
/// A entry in the dirstate for further inspection /// A entry in the dirstate for further inspection
Entry(DirstateEntry), Entry(DirstateEntry),
/// The result of the status of the corresponding file /// The result of the status of the corresponding file
Dispatch(Dispatch), Dispatch(Dispatch),
} }
impl<'a> Iterator for FsIter<'a> { impl<'a> Iterator for FsIter<'a> {
type Item = (HgPathBuf, StatusShortcut); type Item = (HgPathBuf, StatusShortcut);
fn next(&mut self) -> Option<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
// If any paths have already been `Dispatch`-ed, return them // 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); return Some(res);
} }
while let Some((base_path, node)) = self.to_visit.pop_front() { while let Some((base_path, node)) = self.to_visit.pop_front() {
match &node.kind { match &node.kind {
NodeKind::Directory(dir) => { NodeKind::Directory(dir) => {
let canonical_path = HgPath::new(&base_path); let canonical_path = HgPath::new(&base_path);
if self.directory_became_symlink(canonical_path) { if self.directory_became_symlink(canonical_path) {
/// prefixed by a `base_path`. /// prefixed by a `base_path`.
fn add_children_to_visit<'a>( fn add_children_to_visit<'a>(
to_visit: &mut VecDeque<(Cow<'a, [u8]>, &'a Node)>, to_visit: &mut VecDeque<(Cow<'a, [u8]>, &'a Node)>,
base_path: &[u8], base_path: &[u8],
dir: &'a Directory, dir: &'a Directory,
) { ) {
to_visit.extend(dir.children.iter().map(|(path, child)| { to_visit.extend(dir.children.iter().map(|(path, child)| {
let full_path = join_path(&base_path, &path); let full_path = join_path(&base_path, &path);
(Cow::from(full_path), child) (full_path, child)
})); }));
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::{EntryState, FastHashMap}; use crate::{EntryState, FastHashMap};

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

) )
} }
} }
} }
} }
match &mut self.kind { match &mut self.kind {
NodeKind::Directory(directory) => { NodeKind::Directory(directory) => {
return Node::insert_in_directory( Node::insert_in_directory(
directory, new_entry, head, tail, directory, new_entry, head, tail,
); )
} }
NodeKind::File(_) => { NodeKind::File(_) => {
unreachable!("The file case has already been handled") unreachable!("The file case has already been handled")
} }
} }
} }
/// The current file still exists and is not marked as `Removed`. /// The current file still exists and is not marked as `Removed`.
match &mut self.kind { match &mut self.kind {
NodeKind::Directory(d) => { NodeKind::Directory(d) => {
if let Some(child) = d.children.get_mut(head) { if let Some(child) = d.children.get_mut(head) {
let mut res = child.remove(tail); let mut res = child.remove(tail);
if res.cleanup { if res.cleanup {
d.children.remove(head); d.children.remove(head);
} }
res.cleanup = res.cleanup =
d.children.len() == 0 && d.was_file.is_none(); d.children.is_empty() && d.was_file.is_none();
res res
} else { } else {
empty_result empty_result
} }
} }
NodeKind::File(f) => { NodeKind::File(f) => {
if let Some(d) = &mut f.was_directory { if let Some(d) = &mut f.was_directory {
if let Some(child) = d.children.get_mut(head) { if let Some(child) = d.children.get_mut(head) {
let RemoveResult { cleanup, old_entry } = let RemoveResult { cleanup, old_entry } =
child.remove(tail); child.remove(tail);
if cleanup { if cleanup {
d.children.remove(head); 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; f.was_directory = None;
} }
return RemoveResult { return RemoveResult {
cleanup: false, cleanup: false,
old_entry, old_entry,
}; };
} }
d.children.remove(head); d.children.remove(head);
} }
Some(dir) => { Some(dir) => {
node.kind = NodeKind::Directory(*dir.clone()); node.kind = NodeKind::Directory(*dir.clone());
} }
} }
RemoveResult { RemoveResult {
cleanup: cleanup, cleanup,
old_entry: Some(entry), old_entry: Some(entry),
} }
} }
}; };
} }
RemoveResult::default() RemoveResult::default()
} }