From 11cd0b1bbf61cf19a9b3c384748bc7c45214679b Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 13 Apr 2025 03:49:08 +0200 Subject: [PATCH 1/3] replace Error::new(ErrorKind::Other, _) as suggested by clippy See also: https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error --- src/uu/comm/src/comm.rs | 2 +- src/uu/du/src/du.rs | 25 +++++++--------- src/uu/mv/src/mv.rs | 13 ++++----- src/uu/split/src/platform/unix.rs | 14 ++------- src/uu/split/src/platform/windows.rs | 16 ++--------- src/uu/split/src/split.rs | 37 ++++++++++++------------ src/uu/touch/src/touch.rs | 2 +- src/uu/wc/src/wc.rs | 3 +- src/uucore/src/lib/features/checksum.rs | 15 +++++----- tests/uutests/src/lib/util.rs | 38 ++++++++++++------------- 10 files changed, 68 insertions(+), 97 deletions(-) diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index 6df8b1301..11752c331 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -267,7 +267,7 @@ fn open_file(name: &str, line_ending: LineEnding) -> io::Result { Ok(LineReader::new(Input::Stdin(stdin()), line_ending)) } else { if metadata(name)?.is_dir() { - return Err(io::Error::new(io::ErrorKind::Other, "Is a directory")); + return Err(io::Error::other("Is a directory")); } let f = File::open(name)?; Ok(LineReader::new( diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index f2d2f3aa9..e99574085 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -580,20 +580,18 @@ fn read_files_from(file_name: &str) -> Result, std::io::Error> { // First, check if the file_name is a directory let path = PathBuf::from(file_name); if path.is_dir() { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("{file_name}: read error: Is a directory"), - )); + return Err(std::io::Error::other(format!( + "{file_name}: read error: Is a directory" + ))); } // Attempt to open the file and handle the error if it does not exist match File::open(file_name) { Ok(file) => Box::new(BufReader::new(file)), Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("cannot open '{file_name}' for reading: No such file or directory"), - )); + return Err(std::io::Error::other(format!( + "cannot open '{file_name}' for reading: No such file or directory" + ))); } Err(e) => return Err(e), } @@ -637,13 +635,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let files = if let Some(file_from) = matches.get_one::(options::FILES0_FROM) { if file_from == "-" && matches.get_one::(options::FILE).is_some() { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "extra operand {}\nfile operands cannot be combined with --files0-from", - matches.get_one::(options::FILE).unwrap().quote() - ), - ) + return Err(std::io::Error::other(format!( + "extra operand {}\nfile operands cannot be combined with --files0-from", + matches.get_one::(options::FILE).unwrap().quote() + )) .into()); } diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index d4260c074..50a8a4c5f 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -370,7 +370,7 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()> OverwriteMode::NoClobber => return Ok(()), OverwriteMode::Interactive => { if !prompt_yes!("overwrite {}? ", target.quote()) { - return Err(io::Error::new(io::ErrorKind::Other, "").into()); + return Err(io::Error::other("").into()); } } OverwriteMode::Force => {} @@ -609,7 +609,7 @@ fn rename( if opts.update == UpdateMode::ReplaceNoneFail { let err_msg = format!("not replacing {}", to.quote()); - return Err(io::Error::new(io::ErrorKind::Other, err_msg)); + return Err(io::Error::other(err_msg)); } match opts.overwrite { @@ -621,7 +621,7 @@ fn rename( } OverwriteMode::Interactive => { if !prompt_yes!("overwrite {}?", to.quote()) { - return Err(io::Error::new(io::ErrorKind::Other, "")); + return Err(io::Error::other("")); } } OverwriteMode::Force => {} @@ -640,7 +640,7 @@ fn rename( if is_empty_dir(to) { fs::remove_dir(to)?; } else { - return Err(io::Error::new(io::ErrorKind::Other, "Directory not empty")); + return Err(io::Error::other("Directory not empty")); } } } @@ -756,7 +756,7 @@ fn rename_with_fallback( io::ErrorKind::PermissionDenied, "Permission denied", )), - _ => Err(io::Error::new(io::ErrorKind::Other, format!("{err:?}"))), + _ => Err(io::Error::other(format!("{err:?}"))), }; } } else { @@ -811,8 +811,7 @@ fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { } #[cfg(not(any(windows, unix)))] { - return Err(io::Error::new( - io::ErrorKind::Other, + return Err(io::Error::other( "your operating system does not support symlinks", )); } diff --git a/src/uu/split/src/platform/unix.rs b/src/uu/split/src/platform/unix.rs index 023653d5a..446d8d667 100644 --- a/src/uu/split/src/platform/unix.rs +++ b/src/uu/split/src/platform/unix.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. use std::env; use std::io::Write; -use std::io::{BufWriter, Error, ErrorKind, Result}; +use std::io::{BufWriter, Error, Result}; use std::path::Path; use std::process::{Child, Command, Stdio}; use uucore::error::USimpleError; @@ -134,22 +134,14 @@ pub fn instantiate_current_writer( .create(true) .truncate(true) .open(Path::new(&filename)) - .map_err(|_| { - Error::new( - ErrorKind::Other, - format!("unable to open '{filename}'; aborting"), - ) - })? + .map_err(|_| Error::other(format!("unable to open '{filename}'; aborting")))? } else { // re-open file that we previously created to append to it std::fs::OpenOptions::new() .append(true) .open(Path::new(&filename)) .map_err(|_| { - Error::new( - ErrorKind::Other, - format!("unable to re-open '{filename}'; aborting"), - ) + Error::other(format!("unable to re-open '{filename}'; aborting")) })? }; Ok(BufWriter::new(Box::new(file) as Box)) diff --git a/src/uu/split/src/platform/windows.rs b/src/uu/split/src/platform/windows.rs index 1566e5773..576fa43c4 100644 --- a/src/uu/split/src/platform/windows.rs +++ b/src/uu/split/src/platform/windows.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. use std::io::Write; -use std::io::{BufWriter, Error, ErrorKind, Result}; +use std::io::{BufWriter, Error, Result}; use std::path::Path; use uucore::fs; @@ -23,23 +23,13 @@ pub fn instantiate_current_writer( .create(true) .truncate(true) .open(Path::new(&filename)) - .map_err(|_| { - Error::new( - ErrorKind::Other, - format!("unable to open '{filename}'; aborting"), - ) - })? + .map_err(|_| Error::other(format!("unable to open '{filename}'; aborting")))? } else { // re-open file that we previously created to append to it std::fs::OpenOptions::new() .append(true) .open(Path::new(&filename)) - .map_err(|_| { - Error::new( - ErrorKind::Other, - format!("unable to re-open '{filename}'; aborting"), - ) - })? + .map_err(|_| Error::other(format!("unable to re-open '{filename}'; aborting")))? }; Ok(BufWriter::new(Box::new(file) as Box)) } diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 3fc637253..79aea3e15 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -535,10 +535,9 @@ impl Settings { is_new: bool, ) -> io::Result>> { if platform::paths_refer_to_same_file(&self.input, filename) { - return Err(io::Error::new( - ErrorKind::Other, - format!("'{filename}' would overwrite input; aborting"), - )); + return Err(io::Error::other(format!( + "'{filename}' would overwrite input; aborting" + ))); } platform::instantiate_current_writer(self.filter.as_deref(), filename, is_new) @@ -638,10 +637,9 @@ where } else if input == "-" { // STDIN stream that did not fit all content into a buffer // Most likely continuous/infinite input stream - return Err(io::Error::new( - ErrorKind::Other, - format!("{input}: cannot determine input size"), - )); + return Err(io::Error::other(format!( + "{input}: cannot determine input size" + ))); } else { // Could be that file size is larger than set read limit // Get the file size from filesystem metadata @@ -664,10 +662,9 @@ where // Give up and return an error // TODO It might be possible to do more here // to address all possible file types and edge cases - return Err(io::Error::new( - ErrorKind::Other, - format!("{input}: cannot determine file size"), - )); + return Err(io::Error::other(format!( + "{input}: cannot determine file size" + ))); } } } @@ -750,9 +747,10 @@ impl Write for ByteChunkWriter<'_> { self.num_bytes_remaining_in_current_chunk = self.chunk_size; // Allocate the new file, since at this point we know there are bytes to be written to it. - let filename = self.filename_iterator.next().ok_or_else(|| { - io::Error::new(ErrorKind::Other, "output file suffixes exhausted") - })?; + let filename = self + .filename_iterator + .next() + .ok_or_else(|| io::Error::other("output file suffixes exhausted"))?; if self.settings.verbose { println!("creating file {}", filename.quote()); } @@ -871,9 +869,10 @@ impl Write for LineChunkWriter<'_> { // corresponding writer. if self.num_lines_remaining_in_current_chunk == 0 { self.num_chunks_written += 1; - let filename = self.filename_iterator.next().ok_or_else(|| { - io::Error::new(ErrorKind::Other, "output file suffixes exhausted") - })?; + let filename = self + .filename_iterator + .next() + .ok_or_else(|| io::Error::other("output file suffixes exhausted"))?; if self.settings.verbose { println!("creating file {}", filename.quote()); } @@ -948,7 +947,7 @@ impl ManageOutFiles for OutFiles { // This object is responsible for creating the filename for each chunk let mut filename_iterator: FilenameIterator<'_> = FilenameIterator::new(&settings.prefix, &settings.suffix) - .map_err(|e| io::Error::new(ErrorKind::Other, format!("{e}")))?; + .map_err(|e| io::Error::other(format!("{e}")))?; let mut out_files: Self = Self::new(); for _ in 0..num_files { let filename = filename_iterator diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index bd68d17b8..6749933f0 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -477,7 +477,7 @@ fn touch_file( false }; if is_directory { - let custom_err = Error::new(ErrorKind::Other, "No such file or directory"); + let custom_err = Error::other("No such file or directory"); return Err( custom_err.map_err_context(|| format!("cannot touch {}", filename.quote())) ); diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index f260341e6..47abfe210 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -794,8 +794,7 @@ fn files0_iter<'a>( // ...Windows does not, we must go through Strings. #[cfg(not(unix))] { - let s = String::from_utf8(p) - .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; + let s = String::from_utf8(p).map_err(io::Error::other)?; Ok(Input::Path(PathBuf::from(s).into())) } } diff --git a/src/uucore/src/lib/features/checksum.rs b/src/uucore/src/lib/features/checksum.rs index 7d05e48fa..2718f0e28 100644 --- a/src/uucore/src/lib/features/checksum.rs +++ b/src/uucore/src/lib/features/checksum.rs @@ -780,19 +780,18 @@ fn get_input_file(filename: &OsStr) -> UResult> { match File::open(filename) { Ok(f) => { if f.metadata()?.is_dir() { - Err(io::Error::new( - io::ErrorKind::Other, - format!("{}: Is a directory", filename.to_string_lossy()), + Err( + io::Error::other(format!("{}: Is a directory", filename.to_string_lossy())) + .into(), ) - .into()) } else { Ok(Box::new(f)) } } - Err(_) => Err(io::Error::new( - io::ErrorKind::Other, - format!("{}: No such file or directory", filename.to_string_lossy()), - ) + Err(_) => Err(io::Error::other(format!( + "{}: No such file or directory", + filename.to_string_lossy() + )) .into()), } } diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 55c3e3a1c..964b24e86 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -2286,10 +2286,10 @@ impl UChild { if start.elapsed() < timeout { self.delay(10); } else { - return Err(io::Error::new( - io::ErrorKind::Other, - format!("kill: Timeout of '{}s' reached", timeout.as_secs_f64()), - )); + return Err(io::Error::other(format!( + "kill: Timeout of '{}s' reached", + timeout.as_secs_f64() + ))); } hint::spin_loop(); } @@ -2354,10 +2354,10 @@ impl UChild { if start.elapsed() < timeout { self.delay(10); } else { - return Err(io::Error::new( - io::ErrorKind::Other, - format!("kill: Timeout of '{}s' reached", timeout.as_secs_f64()), - )); + return Err(io::Error::other(format!( + "kill: Timeout of '{}s' reached", + timeout.as_secs_f64() + ))); } hint::spin_loop(); } @@ -2446,10 +2446,10 @@ impl UChild { handle.join().unwrap().unwrap(); result } - Err(RecvTimeoutError::Timeout) => Err(io::Error::new( - io::ErrorKind::Other, - format!("wait: Timeout of '{}s' reached", timeout.as_secs_f64()), - )), + Err(RecvTimeoutError::Timeout) => Err(io::Error::other(format!( + "wait: Timeout of '{}s' reached", + timeout.as_secs_f64() + ))), Err(RecvTimeoutError::Disconnected) => { handle.join().expect("Panic caused disconnect").unwrap(); panic!("Error receiving from waiting thread because of unexpected disconnect"); @@ -2691,10 +2691,9 @@ impl UChild { .name("pipe_in".to_string()) .spawn( move || match writer.write_all(&content).and_then(|()| writer.flush()) { - Err(error) if !ignore_stdin_write_error => Err(io::Error::new( - io::ErrorKind::Other, - format!("failed to write to stdin of child: {error}"), - )), + Err(error) if !ignore_stdin_write_error => Err(io::Error::other(format!( + "failed to write to stdin of child: {error}" + ))), Ok(()) | Err(_) => Ok(()), }, ) @@ -2736,10 +2735,9 @@ impl UChild { let mut writer = self.access_stdin_as_writer(); match writer.write_all(&data.into()).and_then(|()| writer.flush()) { - Err(error) if !ignore_stdin_write_error => Err(io::Error::new( - io::ErrorKind::Other, - format!("failed to write to stdin of child: {error}"), - )), + Err(error) if !ignore_stdin_write_error => Err(io::Error::other(format!( + "failed to write to stdin of child: {error}" + ))), Ok(()) | Err(_) => Ok(()), } } From fc3f7eaa48b7e12646098df1d9691356cb20a095 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 13 Apr 2025 03:54:20 +0200 Subject: [PATCH 2/3] replace let_return as suggested by clippy See also: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return --- src/uu/echo/src/echo.rs | 26 +++++++++++--------------- src/uu/env/src/native_int_str.rs | 3 +-- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 7ce2fa9ad..a59ba86d6 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -168,21 +168,17 @@ fn execute( } fn bytes_from_os_string(input: &OsStr) -> Option<&[u8]> { - let option = { - #[cfg(target_family = "unix")] - { - use std::os::unix::ffi::OsStrExt; + #[cfg(target_family = "unix")] + { + use std::os::unix::ffi::OsStrExt; - Some(input.as_bytes()) - } + Some(input.as_bytes()) + } - #[cfg(not(target_family = "unix"))] - { - // TODO - // Verify that this works correctly on these platforms - input.to_str().map(|st| st.as_bytes()) - } - }; - - option + #[cfg(not(target_family = "unix"))] + { + // TODO + // Verify that this works correctly on these platforms + input.to_str().map(|st| st.as_bytes()) + } } diff --git a/src/uu/env/src/native_int_str.rs b/src/uu/env/src/native_int_str.rs index 06252b325..856948fc1 100644 --- a/src/uu/env/src/native_int_str.rs +++ b/src/uu/env/src/native_int_str.rs @@ -283,8 +283,7 @@ impl<'a> NativeStr<'a> { match &self.native { Cow::Borrowed(b) => { let slice = f_borrow(b); - let os_str = slice.map(|x| from_native_int_representation(Cow::Borrowed(x))); - os_str + slice.map(|x| from_native_int_representation(Cow::Borrowed(x))) } Cow::Owned(o) => { let slice = f_owned(o); From 875770f5d17d3430f07cc842be0200e9281d263e Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 13 Apr 2025 04:05:56 +0200 Subject: [PATCH 3/3] apply simple clippy nightly suggestions See also: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_some https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq --- src/uu/cp/src/copydir.rs | 2 +- src/uu/fmt/src/linebreak.rs | 4 +--- src/uu/unexpand/src/unexpand.rs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 551ec0374..ba58ecbac 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -82,7 +82,7 @@ fn get_local_to_root_parent( /// Given an iterator, return all its items except the last. fn skip_last(mut iter: impl Iterator) -> impl Iterator { let last = iter.next(); - iter.scan(last, |state, item| std::mem::replace(state, Some(item))) + iter.scan(last, |state, item| state.replace(item)) } /// Paths that are invariant throughout the traversal when copying a directory. diff --git a/src/uu/fmt/src/linebreak.rs b/src/uu/fmt/src/linebreak.rs index 72f954914..6c34b0808 100644 --- a/src/uu/fmt/src/linebreak.rs +++ b/src/uu/fmt/src/linebreak.rs @@ -164,9 +164,7 @@ fn break_knuth_plass<'a, T: Clone + Iterator>>( // We find identical breakpoints here by comparing addresses of the references. // This is OK because the backing vector is not mutating once we are linebreaking. - let winfo_ptr = winfo as *const _; - let next_break_ptr = next_break as *const _; - if winfo_ptr == next_break_ptr { + if std::ptr::eq(winfo, next_break) { // OK, we found the matching word if break_before { write_newline(args.indent_str, args.ostream)?; diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs index 7d12daf0c..fb17b971d 100644 --- a/src/uu/unexpand/src/unexpand.rs +++ b/src/uu/unexpand/src/unexpand.rs @@ -55,7 +55,7 @@ fn tabstops_parse(s: &str) -> Result, ParseError> { } } - if nums.iter().any(|&n| n == 0) { + if nums.contains(&0) { return Err(ParseError::TabSizeCannotBeZero); }