Skip to content

Commit

Permalink
mv: clippy lint errors
Browse files Browse the repository at this point in the history
  • Loading branch information
matrixhead committed Oct 5, 2024
1 parent a24aca1 commit 7c32f06
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 37 deletions.
4 changes: 3 additions & 1 deletion src/uu/cp/src/cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ use platform::copy_on_write;
use uucore::display::Quotable;
use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError};
use uucore::fs::{
are_hardlinks_to_same_file, canonicalize, disk_usage, get_filename, is_symlink_loop, path_ends_with_terminator, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode
are_hardlinks_to_same_file, canonicalize, disk_usage, get_filename, is_symlink_loop,
path_ends_with_terminator, paths_refer_to_same_file, FileInformation, MissingHandling,
ResolveMode,
};
use uucore::{backup_control, update_control};
// These are exposed for projects (e.g. nushell) that want to create an `Options` value, which
Expand Down
30 changes: 17 additions & 13 deletions src/uu/mv/src/mv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
use std::collections::HashSet;
use std::env;
use std::ffi::OsString;
use std::fs::{self, FileType};
use std::fs;
use std::io;
#[cfg(unix)]
use std::os::unix;
Expand Down Expand Up @@ -105,6 +105,8 @@ pub enum OverwriteMode {
///'-f' '--force' overwrite without prompt
Force,
}

/// a context for handling verbose output during file operations.
struct VerboseContext<'a> {
backup: Option<&'a Path>,
pb: Option<&'a MultiProgress>,
Expand Down Expand Up @@ -170,6 +172,7 @@ impl<'a> VerboseContext<'a> {
self.hide_pb_and_print(&message);
}
}

const ABOUT: &str = help_about!("mv.md");
const USAGE: &str = help_usage!("mv.md");
const AFTER_HELP: &str = help_section!("after help", "mv.md");
Expand Down Expand Up @@ -684,7 +687,7 @@ fn rename_with_fallback(
from: &Path,
to: &Path,
multi_progress: Option<&MultiProgress>,
vebose_context: Option<&VerboseContext<'_>>,
verbose_context: Option<&VerboseContext<'_>>,
) -> io::Result<()> {
if fs::rename(from, to).is_err() {
// Get metadata without following symlinks
Expand All @@ -693,7 +696,7 @@ fn rename_with_fallback(

if file_type.is_symlink() {
rename_symlink_fallback(from, to)?;
if let Some(vc) = vebose_context {
if let Some(vc) = verbose_context {
vc.print_move_file(from, to);
}
} else if file_type.is_dir() {
Expand All @@ -709,8 +712,6 @@ fn rename_with_fallback(
// If finding the total size fails for whatever reason,
// the progress bar wont be shown for this file / dir.
// (Move will probably fail due to permission error later?)
// let total_size = disk_usage(&[from],true).ok();
// let total_size = None;
let mut progress_bar = None;
if let Some(multi_progress) = multi_progress {
if let Ok(total_size) = disk_usage(&[from], true) {
Expand All @@ -725,7 +726,7 @@ fn rename_with_fallback(
}

#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
let result = move_dir(from, to, progress_bar.as_ref(), vebose_context);
let result = move_dir(from, to, progress_bar.as_ref(), verbose_context);

if let Err(err) = result {
return match err {
Expand All @@ -737,7 +738,7 @@ fn rename_with_fallback(
"Permission denied",
)),
MvError::NotAllFilesMoved => {
Err(io::Error::new(io::ErrorKind::Other, format!("")))
Err(io::Error::new(io::ErrorKind::Other, String::new()))
}
_ => Err(io::Error::new(io::ErrorKind::Other, format!("{err:?}"))),
};
Expand All @@ -759,25 +760,25 @@ fn rename_with_fallback(
#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
fs::copy(from, to)
.map(|v| {
if let Some(vc) = vebose_context {
if let Some(vc) = verbose_context {
vc.print_copy_file(from, to, true);
}
v
})
.and_then(|_| fsxattr::copy_xattrs(&from, &to))?;
#[cfg(any(target_os = "macos", target_os = "redox", not(unix)))]
fs::copy(from, to).map(|v| {
if let Some(vc) = vebose_context {
if let Some(vc) = verbose_context {
vc.print_copy_file(from, to, true);
}
v
})?;
fs::remove_file(from)?;
if let Some(vc) = vebose_context {
if let Some(vc) = verbose_context {
vc.remove_file(from);
}
}
} else if let Some(vb) = vebose_context {
} else if let Some(vb) = verbose_context {
vb.print_move_file(from, to);
}
Ok(())
Expand Down Expand Up @@ -839,7 +840,7 @@ fn move_dir(
// The return value that represents the number of bytes copied.
let mut result: u64 = 0;
let mut error_occurred = false;
let mut moved_entries: Vec<(PathBuf, FileType, PathBuf, Option<fs::Metadata>, usize)> = vec![];
let mut moved_entries: Vec<(PathBuf, fs::FileType, PathBuf, Option<fs::Metadata>, usize)> = vec![];
for dir_entry_result in WalkDir::new(from) {
match dir_entry_result {
Ok(dir_entry) => {
Expand All @@ -853,6 +854,9 @@ fn move_dir(
let res = fs_extra::dir::create(&dir_entry_to, false);
if let Err(err) = res {
if let FsXErrorKind::NotFound = err.kind {
// This error would be thrown in the first iteration
// if the destination parent directory doesn't
// exist.
return Err(err.into());
}
error_occurred = true;
Expand Down Expand Up @@ -1010,7 +1014,7 @@ fn copy_metadata(src: &Path, dest: &Path, src_metadata: &fs::Metadata) -> io::Re
level: VerbosityLevel::Silent,
},
)
.map_err(|err| io::Error::other(err))?;
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
}

Ok(())
Expand Down
40 changes: 26 additions & 14 deletions src/uucore/src/lib/features/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ fn disk_usage_directory<P: AsRef<Path>>(p: P) -> IOResult<u64> {
for entry in fs::read_dir(p)? {
let entry = entry?;
if entry.file_type()?.is_dir() {
total += disk_usage_directory(&entry.path())?;
total += disk_usage_directory(entry.path())?;
} else {
total += entry.metadata()?.len();
}
Expand Down Expand Up @@ -1091,20 +1091,23 @@ mod tests {
#[test]
fn calculates_total_size_of_single_file() {
let dir = tempdir().unwrap();
let file_path1 = dir.path().join("file1.txt");
let file_path1 = dir.path().join("file1.txt");

let mut file1 = fs::File::create(&file_path1).unwrap();
writeln!(file1, "Hello, world!").unwrap();

let paths = vec![file_path1];

let actual_len = file1.metadata().expect("couldn't get metadata for file1").len();

let actual_len = file1
.metadata()
.expect("couldn't get metadata for file1")
.len();

let total_size = disk_usage(&paths, false).unwrap();

assert_eq!(total_size, actual_len);
}
#[test]
#[test]
fn calculates_total_size_of_files() {
let dir = tempdir().unwrap();
let file_path1 = dir.path().join("file1.txt");
Expand All @@ -1118,15 +1121,21 @@ mod tests {

let paths = vec![file_path1, file_path2];

let file_1_len = file1.metadata().expect("couldn't get metadata for file1").len();
let file_2_len = file2.metadata().expect("couldn't get metadata for file1").len();
let actual_len = file_1_len+file_2_len;

let file_1_len = file1
.metadata()
.expect("couldn't get metadata for file1")
.len();
let file_2_len = file2
.metadata()
.expect("couldn't get metadata for file1")
.len();
let actual_len = file_1_len + file_2_len;

let total_size = disk_usage(&paths, false).unwrap();
assert_eq!(total_size, actual_len);

assert_eq!(total_size, actual_len);
}
#[test]
#[test]
fn test_disk_usage_recursive() {
let dir = tempdir().unwrap();
let file_path = dir.path().join("file1.txt");
Expand All @@ -1142,6 +1151,9 @@ mod tests {
let paths: Vec<PathBuf> = vec![dir.path().to_path_buf()];
let total_size = disk_usage(&paths, true).unwrap();

assert_eq!(total_size, file_path.metadata().unwrap().len() + subfile_path.metadata().unwrap().len());
assert_eq!(
total_size,
file_path.metadata().unwrap().len() + subfile_path.metadata().unwrap().len()
);
}
}
18 changes: 9 additions & 9 deletions tests/by-util/test_mv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1926,7 +1926,7 @@ mod inter_partition_copying {
let at = &scene.fixtures;
at.mkdir_all("dir");
let file = at.make_file("dir/file");
at.set_mode("dir/file", 0o100700);
at.set_mode("dir/file", 0o100_700);

// Set xattrs for the source file
let test_attr = "user.test_attr";
Expand Down Expand Up @@ -1965,7 +1965,7 @@ mod inter_partition_copying {
.metadata()
.expect("couldn't get metadata of dest file");
let mode = dest_metadata.mode();
assert_eq!(mode, 0o100700, "permission doesn't match");
assert_eq!(mode, 0o100_700, "permission doesn't match");
assert_eq!(
dest_metadata
.modified()
Expand All @@ -1980,7 +1980,7 @@ mod inter_partition_copying {
);

// Verify that the xattrs were copied
let retrieved_xattrs = retrieve_xattrs(&other_fs_tempdir.path().join("dir/file")).unwrap();
let retrieved_xattrs = retrieve_xattrs(other_fs_tempdir.path().join("dir/file")).unwrap();
assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str()));
assert_eq!(
retrieved_xattrs
Expand Down Expand Up @@ -2053,7 +2053,7 @@ mod inter_partition_copying {
);

// Verify that the xattrs were copied
let retrieved_xattrs = retrieve_xattrs(&other_fs_tempdir.path().join("dir")).unwrap();
let retrieved_xattrs = retrieve_xattrs(other_fs_tempdir.path().join("dir")).unwrap();
assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str()));
assert_eq!(
retrieved_xattrs
Expand Down Expand Up @@ -2086,7 +2086,7 @@ mod inter_partition_copying {
//make sure mv doesn't print errors for the parent directories
.stderr_does_not_contain("'a/aa'");
assert!(at.file_exists("a/aa/aa1"));
assert!(other_fs_tempdir.path().join("a/aa/aa1").exists())
assert!(other_fs_tempdir.path().join("a/aa/aa1").exists());
}

#[test]
Expand Down Expand Up @@ -2116,7 +2116,7 @@ mod inter_partition_copying {
assert!(at.file_exists("a/aa/aa1"));
assert!(at.file_exists("a/aa/aa2"));
assert!(at.dir_exists("a/aa/aaa"));
assert!(other_fs_tempdir.path().join("a/aa/aa1").exists())
assert!(other_fs_tempdir.path().join("a/aa/aa1").exists());
}

#[test]
Expand All @@ -2142,7 +2142,7 @@ mod inter_partition_copying {
assert!(at.file_exists("a/aa/aa1"));
// file that doesn't belong to the branch that error occured didn't got removed
assert!(!at.file_exists("a/a1"));
assert!(other_fs_tempdir.path().join("a/aa/aa1").exists())
assert!(other_fs_tempdir.path().join("a/aa/aa1").exists());
}

#[test]
Expand All @@ -2153,7 +2153,7 @@ mod inter_partition_copying {
at.mkdir_all("a/ab/");
at.write("a/aa/aa1", "filecontents");
at.write("a/ab/ab1", "filecontents");

//remove write permssion for the subdir
at.set_mode("a/aa/", 0o555);
// create a folder in another partition.
Expand All @@ -2170,7 +2170,7 @@ mod inter_partition_copying {
assert!(at.file_exists("a/aa/aa1"));
// folder that doesn't belong to the branch that error occured didn't got removed
assert!(!at.dir_exists("a/ab"));
assert!(other_fs_tempdir.path().join("a/ab/ab1").exists())
assert!(other_fs_tempdir.path().join("a/ab/ab1").exists());
}
}

Expand Down

0 comments on commit 7c32f06

Please sign in to comment.