Merge pull request #7525 from sylvestre/thiserror4

Move more programs to thiserror

Cargo.lock

@@ -2850,6 +2850,7 @@ dependencies = [
  "file_diff",
  "filetime",
  "libc",
+ "thiserror 2.0.12",
  "uucore",
 ]
 
@@ -2859,6 +2860,7 @@ version = "0.0.30"
 dependencies = [
  "clap",
  "memchr",
+ "thiserror 2.0.12",
  "uucore",
 ]
 
@@ -2884,6 +2886,7 @@ name = "uu_ln"
 version = "0.0.30"
 dependencies = [
  "clap",
+ "thiserror 2.0.12",
  "uucore",
 ]
 
@@ -2909,6 +2912,7 @@ dependencies = [
  "number_prefix",
  "selinux",
  "terminal_size",
+ "thiserror 2.0.12",
  "uucore",
  "uutils_term_grid",
 ]

src/uu/install/Cargo.toml

@@ -21,6 +21,7 @@ clap = { workspace = true }
 filetime = { workspace = true }
 file_diff = { workspace = true }
 libc = { workspace = true }
+thiserror = { workspace = true }
 uucore = { workspace = true, features = [
   "backup-control",
   "buf-copy",

src/uu/install/src/install.rs

@@ -10,22 +10,22 @@ mod mode;
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use file_diff::diff;
 use filetime::{set_file_times, FileTime};
-use std::error::Error;
+use std::fmt::Debug;
-use std::fmt::{Debug, Display};
 use std::fs::File;
 use std::fs::{self, metadata};
 use std::path::{Path, PathBuf, MAIN_SEPARATOR};
 use std::process;
+use thiserror::Error;
 use uucore::backup_control::{self, BackupMode};
 use uucore::buf_copy::copy_stream;
 use uucore::display::Quotable;
 use uucore::entries::{grp2gid, usr2uid};
-use uucore::error::{FromIo, UError, UIoError, UResult, UUsageError};
+use uucore::error::{FromIo, UError, UResult, UUsageError};
 use uucore::fs::dir_strip_dot_for_creation;
 use uucore::mode::get_umask;
 use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel};
 use uucore::process::{getegid, geteuid};
-use uucore::{format_usage, help_about, help_usage, show, show_error, show_if_err, uio_error};
+use uucore::{format_usage, help_about, help_usage, show, show_error, show_if_err};
 
 #[cfg(unix)]
 use std::os::unix::fs::{FileTypeExt, MetadataExt};
@@ -52,22 +52,51 @@ pub struct Behavior {
     target_dir: Option<String>,
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 enum InstallError {
+    #[error("Unimplemented feature: {0}")]
     Unimplemented(String),
-    DirNeedsArg(),
+
-    CreateDirFailed(PathBuf, std::io::Error),
+    #[error("{} with -d requires at least one argument.", uucore::util_name())]
+    DirNeedsArg,
+
+    #[error("failed to create {0}")]
+    CreateDirFailed(PathBuf, #[source] std::io::Error),
+
+    #[error("failed to chmod {}", .0.quote())]
     ChmodFailed(PathBuf),
+
+    #[error("failed to chown {}: {}", .0.quote(), .1)]
     ChownFailed(PathBuf, String),
+
+    #[error("invalid target {}: No such file or directory", .0.quote())]
     InvalidTarget(PathBuf),
+
+    #[error("target {} is not a directory", .0.quote())]
     TargetDirIsntDir(PathBuf),
-    BackupFailed(PathBuf, PathBuf, std::io::Error),
+
-    InstallFailed(PathBuf, PathBuf, std::io::Error),
+    #[error("cannot backup {0} to {1}")]
+    BackupFailed(PathBuf, PathBuf, #[source] std::io::Error),
+
+    #[error("cannot install {0} to {1}")]
+    InstallFailed(PathBuf, PathBuf, #[source] std::io::Error),
+
+    #[error("strip program failed: {0}")]
     StripProgramFailed(String),
-    MetadataFailed(std::io::Error),
+
+    #[error("metadata error")]
+    MetadataFailed(#[source] std::io::Error),
+
+    #[error("invalid user: {}", .0.quote())]
     InvalidUser(String),
+
+    #[error("invalid group: {}", .0.quote())]
     InvalidGroup(String),
+
+    #[error("omitting directory {}", .0.quote())]
     OmittingDirectory(PathBuf),
+
+    #[error("failed to access {}: Not a directory", .0.quote())]
     NotADirectory(PathBuf),
 }
 
@@ -84,52 +113,6 @@ impl UError for InstallError {
     }
 }
 
-impl Error for InstallError {}
-
-impl Display for InstallError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::Unimplemented(opt) => write!(f, "Unimplemented feature: {opt}"),
-            Self::DirNeedsArg() => {
-                write!(
-                    f,
-                    "{} with -d requires at least one argument.",
-                    uucore::util_name()
-                )
-            }
-            Self::CreateDirFailed(dir, e) => {
-                Display::fmt(&uio_error!(e, "failed to create {}", dir.quote()), f)
-            }
-            Self::ChmodFailed(file) => write!(f, "failed to chmod {}", file.quote()),
-            Self::ChownFailed(file, msg) => write!(f, "failed to chown {}: {}", file.quote(), msg),
-            Self::InvalidTarget(target) => write!(
-                f,
-                "invalid target {}: No such file or directory",
-                target.quote()
-            ),
-            Self::TargetDirIsntDir(target) => {
-                write!(f, "target {} is not a directory", target.quote())
-            }
-            Self::BackupFailed(from, to, e) => Display::fmt(
-                &uio_error!(e, "cannot backup {} to {}", from.quote(), to.quote()),
-                f,
-            ),
-            Self::InstallFailed(from, to, e) => Display::fmt(
-                &uio_error!(e, "cannot install {} to {}", from.quote(), to.quote()),
-                f,
-            ),
-            Self::StripProgramFailed(msg) => write!(f, "strip program failed: {msg}"),
-            Self::MetadataFailed(e) => Display::fmt(&uio_error!(e, ""), f),
-            Self::InvalidUser(user) => write!(f, "invalid user: {}", user.quote()),
-            Self::InvalidGroup(group) => write!(f, "invalid group: {}", group.quote()),
-            Self::OmittingDirectory(dir) => write!(f, "omitting directory {}", dir.quote()),
-            Self::NotADirectory(dir) => {
-                write!(f, "failed to access {}: Not a directory", dir.quote())
-            }
-        }
-    }
-}
-
 #[derive(Clone, Eq, PartialEq)]
 pub enum MainFunction {
     /// Create directories
@@ -456,7 +439,7 @@ fn behavior(matches: &ArgMatches) -> UResult<Behavior> {
 ///
 fn directory(paths: &[String], b: &Behavior) -> UResult<()> {
     if paths.is_empty() {
-        Err(InstallError::DirNeedsArg().into())
+        Err(InstallError::DirNeedsArg.into())
     } else {
         for path in paths.iter().map(Path::new) {
             // if the path already exist, don't try to create it again

src/uu/join/Cargo.toml

@@ -20,6 +20,7 @@ path = "src/join.rs"
 clap = { workspace = true }
 uucore = { workspace = true }
 memchr = { workspace = true }
+thiserror = { workspace = true }
 
 [[bin]]
 name = "join"

src/uu/join/src/join.rs

@@ -9,14 +9,13 @@ use clap::builder::ValueParser;
 use clap::{Arg, ArgAction, Command};
 use memchr::{memchr_iter, memmem::Finder, Memchr3};
 use std::cmp::Ordering;
-use std::error::Error;
 use std::ffi::OsString;
-use std::fmt::Display;
 use std::fs::File;
 use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Split, Stdin, Write};
 use std::num::IntErrorKind;
 #[cfg(unix)]
 use std::os::unix::ffi::OsStrExt;
+use thiserror::Error;
 use uucore::display::Quotable;
 use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError};
 use uucore::line_ending::LineEnding;
@@ -25,35 +24,22 @@ use uucore::{format_usage, help_about, help_usage};
 const ABOUT: &str = help_about!("join.md");
 const USAGE: &str = help_usage!("join.md");
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
 enum JoinError {
-    IOError(std::io::Error),
+    #[error("io error: {0}")]
+    IOError(#[from] std::io::Error),
+
+    #[error("{0}")]
     UnorderedInput(String),
 }
 
+// If you still need the UError implementation for compatibility:
 impl UError for JoinError {
     fn code(&self) -> i32 {
         1
     }
 }
 
-impl Error for JoinError {}
-
-impl Display for JoinError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::IOError(e) => write!(f, "io error: {e}"),
-            Self::UnorderedInput(e) => f.write_str(e),
-        }
-    }
-}
-
-impl From<std::io::Error> for JoinError {
-    fn from(error: std::io::Error) -> Self {
-        Self::IOError(error)
-    }
-}
-
 #[derive(Copy, Clone, PartialEq)]
 enum FileNum {
     File1,

src/uu/ln/Cargo.toml

@@ -19,6 +19,7 @@ path = "src/ln.rs"
 [dependencies]
 clap = { workspace = true }
 uucore = { workspace = true, features = ["backup-control", "fs"] }
+thiserror = { workspace = true }
 
 [[bin]]
 name = "ln"

src/uu/ln/src/ln.rs

@@ -13,10 +13,9 @@ use uucore::{format_usage, help_about, help_section, help_usage, prompt_yes, sho
 
 use std::borrow::Cow;
 use std::collections::HashSet;
-use std::error::Error;
 use std::ffi::OsString;
-use std::fmt::Display;
 use std::fs;
+use thiserror::Error;
 
 #[cfg(any(unix, target_os = "redox"))]
 use std::os::unix::fs::symlink;
@@ -46,37 +45,24 @@ pub enum OverwriteMode {
     Force,
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 enum LnError {
+    #[error("target {} is not a directory", _0.quote())]
     TargetIsDirectory(PathBuf),
+
+    #[error("")]
     SomeLinksFailed,
+
+    #[error("{} and {} are the same file", _0.quote(), _1.quote())]
     SameFile(PathBuf, PathBuf),
+
+    #[error("missing destination file operand after {}", _0.quote())]
     MissingDestination(PathBuf),
-    ExtraOperand(OsString),
-}
 
-impl Display for LnError {
+    #[error("extra operand {}\nTry '{} --help' for more information.", 
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    format!("{:?}", _0).trim_matches('"'), _1)]
-        match self {
+    ExtraOperand(OsString, String),
-            Self::TargetIsDirectory(s) => write!(f, "target {} is not a directory", s.quote()),
-            Self::SameFile(s, d) => {
-                write!(f, "{} and {} are the same file", s.quote(), d.quote())
 }
-            Self::SomeLinksFailed => Ok(()),
-            Self::MissingDestination(s) => {
-                write!(f, "missing destination file operand after {}", s.quote())
-            }
-            Self::ExtraOperand(s) => write!(
-                f,
-                "extra operand {}\nTry '{} --help' for more information.",
-                s.quote(),
-                uucore::execution_phrase()
-            ),
-        }
-    }
-}
-
-impl Error for LnError {}
 
 impl UError for LnError {
     fn code(&self) -> i32 {
@@ -284,7 +270,11 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> {
         return Err(LnError::MissingDestination(files[0].clone()).into());
     }
     if files.len() > 2 {
-        return Err(LnError::ExtraOperand(files[2].clone().into()).into());
+        return Err(LnError::ExtraOperand(
+            files[2].clone().into(),
+            uucore::execution_phrase().to_string(),
+        )
+        .into());
     }
     assert!(!files.is_empty());
 

src/uu/ls/Cargo.toml

@@ -26,6 +26,7 @@ lscolors = { workspace = true }
 number_prefix = { workspace = true }
 selinux = { workspace = true, optional = true }
 terminal_size = { workspace = true }
+thiserror = { workspace = true }
 uucore = { workspace = true, features = [
   "colors",
   "custom-tz-fmt",

src/uu/ls/src/ls.rs

@@ -10,9 +10,8 @@ use std::os::windows::fs::MetadataExt;
 use std::{cell::OnceCell, num::IntErrorKind};
 use std::{
     cmp::Reverse,
-    error::Error,
     ffi::{OsStr, OsString},
-    fmt::{Display, Write as FmtWrite},
+    fmt::Write as FmtWrite,
     fs::{self, DirEntry, FileType, Metadata, ReadDir},
     io::{stdout, BufWriter, ErrorKind, Stdout, Write},
     path::{Path, PathBuf},
@@ -35,7 +34,7 @@ use clap::{
 use glob::{MatchOptions, Pattern};
 use lscolors::LsColors;
 use term_grid::{Direction, Filling, Grid, GridOptions};
-
+use thiserror::Error;
 use uucore::error::USimpleError;
 use uucore::format::human::{human_readable, SizeFormat};
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
@@ -175,14 +174,41 @@ const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512;
 const DEFAULT_BLOCK_SIZE: u64 = 1024;
 const DEFAULT_FILE_SIZE_BLOCK_SIZE: u64 = 1;
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 enum LsError {
+    #[error("invalid line width: '{0}'")]
     InvalidLineWidth(String),
-    IOError(std::io::Error),
+
-    IOErrorContext(std::io::Error, PathBuf, bool),
+    #[error("general io error: {0}")]
+    IOError(#[from] std::io::Error),
+
+    #[error("{}", match .1.kind() {
+        ErrorKind::NotFound => format!("cannot access '{}': No such file or directory", .0.to_string_lossy()),
+        ErrorKind::PermissionDenied => match .1.raw_os_error().unwrap_or(1) {
+            1 => format!("cannot access '{}': Operation not permitted", .0.to_string_lossy()),
+            _ => if .0.is_dir() {
+                format!("cannot open directory '{}': Permission denied", .0.to_string_lossy())
+            } else {
+                format!("cannot open file '{}': Permission denied", .0.to_string_lossy())
+            },
+        },
+        _ => match .1.raw_os_error().unwrap_or(1) {
+            9 => format!("cannot open directory '{}': Bad file descriptor", .0.to_string_lossy()),
+            _ => format!("unknown io error: '{:?}', '{:?}'", .0.to_string_lossy(), .1),
+        },
+    })]
+    IOErrorContext(PathBuf, std::io::Error, bool),
+
+    #[error("invalid --block-size argument '{0}'")]
     BlockSizeParseError(String),
+
+    #[error("--dired and --zero are incompatible")]
     DiredAndZeroAreIncompatible,
+
+    #[error("{}: not listing already-listed directory", .0.to_string_lossy())]
     AlreadyListedError(PathBuf),
+
+    #[error("invalid --time-style argument {}\nPossible values are: {:?}\n\nFor more information try --help", .0.quote(), .1)]
     TimeStyleParseError(String, Vec<String>),
 }
 
@@ -201,100 +227,6 @@ impl UError for LsError {
     }
 }
 
-impl Error for LsError {}
-
-impl Display for LsError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::BlockSizeParseError(s) => {
-                write!(f, "invalid --block-size argument {}", s.quote())
-            }
-            Self::DiredAndZeroAreIncompatible => {
-                write!(f, "--dired and --zero are incompatible")
-            }
-            Self::TimeStyleParseError(s, possible_time_styles) => {
-                write!(
-                    f,
-                    "invalid --time-style argument {}\nPossible values are: {:?}\n\nFor more information try --help",
-                    s.quote(),
-                    possible_time_styles
-                )
-            }
-            Self::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()),
-            Self::IOError(e) => write!(f, "general io error: {e}"),
-            Self::IOErrorContext(e, p, _) => {
-                let error_kind = e.kind();
-                let errno = e.raw_os_error().unwrap_or(1i32);
-
-                match error_kind {
-                    // No such file or directory
-                    ErrorKind::NotFound => {
-                        write!(
-                            f,
-                            "cannot access '{}': No such file or directory",
-                            p.to_string_lossy(),
-                        )
-                    }
-                    // Permission denied and Operation not permitted
-                    ErrorKind::PermissionDenied =>
-                    {
-                        #[allow(clippy::wildcard_in_or_patterns)]
-                        match errno {
-                            1i32 => {
-                                write!(
-                                    f,
-                                    "cannot access '{}': Operation not permitted",
-                                    p.to_string_lossy(),
-                                )
-                            }
-                            13i32 | _ => {
-                                if p.is_dir() {
-                                    write!(
-                                        f,
-                                        "cannot open directory '{}': Permission denied",
-                                        p.to_string_lossy(),
-                                    )
-                                } else {
-                                    write!(
-                                        f,
-                                        "cannot open file '{}': Permission denied",
-                                        p.to_string_lossy(),
-                                    )
-                                }
-                            }
-                        }
-                    }
-                    _ => match errno {
-                        9i32 => {
-                            // only should ever occur on a read_dir on a bad fd
-                            write!(
-                                f,
-                                "cannot open directory '{}': Bad file descriptor",
-                                p.to_string_lossy(),
-                            )
-                        }
-                        _ => {
-                            write!(
-                                f,
-                                "unknown io error: '{:?}', '{:?}'",
-                                p.to_string_lossy(),
-                                e
-                            )
-                        }
-                    },
-                }
-            }
-            Self::AlreadyListedError(path) => {
-                write!(
-                    f,
-                    "{}: not listing already-listed directory",
-                    path.to_string_lossy()
-                )
-            }
-        }
-    }
-}
-
 #[derive(PartialEq, Eq, Debug)]
 pub enum Format {
     Columns,
@@ -2054,8 +1986,8 @@ impl PathData {
                             }
                         }
                         show!(LsError::IOErrorContext(
-                            err,
                             self.p_buf.clone(),
+                            err,
                             self.command_line
                         ));
                         None
@@ -2161,8 +2093,8 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> {
                 // flush stdout buffer before the error to preserve formatting and order
                 out.flush()?;
                 show!(LsError::IOErrorContext(
-                    err,
                     path_data.p_buf.clone(),
+                    err,
                     path_data.command_line
                 ));
                 continue;
@@ -2396,8 +2328,8 @@ fn enter_directory(
                 Err(err) => {
                     out.flush()?;
                     show!(LsError::IOErrorContext(
-                        err,
                         e.p_buf.clone(),
+                        err,
                         e.command_line
                     ));
                     continue;
@@ -3365,7 +3297,7 @@ fn display_item_name(
                 }
             }
             Err(err) => {
-                show!(LsError::IOErrorContext(err, path.p_buf.clone(), false));
+                show!(LsError::IOErrorContext(path.p_buf.clone(), err, false));
             }
         }
     }
@@ -3448,7 +3380,7 @@ fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) -
             Err(err) => {
                 // The Path couldn't be dereferenced, so return early and set exit code 1
                 // to indicate a minor error
-                show!(LsError::IOErrorContext(err, p_buf.to_path_buf(), false));
+                show!(LsError::IOErrorContext(p_buf.to_path_buf(), err, false));
                 return substitute_string;
             }
             Ok(_md) => (),