Merge pull request #9337 from blyxxyz/lossy-string-cleanup

Remove lossy OS string conversions

Author: sylvestre

src/uu/base32/src/base_common.rs

@@ -54,7 +54,7 @@ impl Config {
                 if let Some(extra_op) = values.next() {
                     return Err(UUsageError::new(
                         BASE_CMD_PARSE_ERROR,
-                        translate!("base-common-extra-operand", "operand" => extra_op.to_string_lossy().quote()),
+                        translate!("base-common-extra-operand", "operand" => extra_op.quote()),
                     ));
                 }
 

src/uu/chcon/src/chcon.rs

@@ -760,7 +760,7 @@ fn root_dev_ino_warn(dir_name: &Path) {
     } else {
         show_warning!(
             "{}",
-            translate!("chcon-warning-dangerous-recursive-dir", "dir" => dir_name.to_string_lossy(), "option" => options::preserve_root::NO_PRESERVE_ROOT)
+            translate!("chcon-warning-dangerous-recursive-dir", "dir" => dir_name.quote(), "option" => options::preserve_root::NO_PRESERVE_ROOT)
         );
     }
 }
@@ -782,7 +782,7 @@ fn cycle_warning_required(fts_options: c_int, entry: &fts::EntryRef) -> bool {
 fn emit_cycle_warning(file_name: &Path) {
     show_warning!(
         "{}",
-        translate!("chcon-warning-circular-directory", "file" => file_name.to_string_lossy())
+        translate!("chcon-warning-circular-directory", "file" => file_name.quote())
     );
 }
 

src/uu/chmod/locales/en-US.ftl

@@ -9,7 +9,7 @@ chmod-error-dangling-symlink = cannot operate on dangling symlink {$file}
 chmod-error-no-such-file = cannot access {$file}: No such file or directory
 chmod-error-preserve-root = it is dangerous to operate recursively on {$file}
   chmod: use --no-preserve-root to override this failsafe
-chmod-error-permission-denied = {$file}: Permission denied
+chmod-error-permission-denied = cannot access {$file}: Permission denied
 chmod-error-new-permissions = {$file}: new permissions are {$actual}, not {$expected}
 chmod-error-missing-operand = missing operand
 

src/uu/chmod/locales/fr-FR.ftl

@@ -21,7 +21,7 @@ chmod-error-dangling-symlink = impossible d'opérer sur le lien symbolique pendo
 chmod-error-no-such-file = impossible d'accéder à {$file} : Aucun fichier ou répertoire de ce type
 chmod-error-preserve-root = il est dangereux d'opérer récursivement sur {$file}
   chmod: utiliser --no-preserve-root pour outrepasser cette protection
-chmod-error-permission-denied = {$file} : Permission refusée
+chmod-error-permission-denied = impossible d'accéder à {$file} : Permission refusée
 chmod-error-new-permissions = {$file} : les nouvelles permissions sont {$actual}, pas {$expected}
 chmod-error-missing-operand = opérande manquant
 

src/uu/chmod/src/chmod.rs

@@ -9,7 +9,7 @@ use clap::{Arg, ArgAction, Command};
 use std::ffi::OsString;
 use std::fs;
 use std::os::unix::fs::{MetadataExt, PermissionsExt};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use thiserror::Error;
 use uucore::display::Quotable;
 use uucore::error::{ExitCode, UError, UResult, USimpleError, UUsageError, set_exit_code};
@@ -27,17 +27,17 @@ use uucore::translate;
 #[derive(Debug, Error)]
 enum ChmodError {
     #[error("{}", translate!("chmod-error-cannot-stat", "file" => _0.quote()))]
-    CannotStat(String),
+    CannotStat(PathBuf),
     #[error("{}", translate!("chmod-error-dangling-symlink", "file" => _0.quote()))]
-    DanglingSymlink(String),
+    DanglingSymlink(PathBuf),
     #[error("{}", translate!("chmod-error-no-such-file", "file" => _0.quote()))]
-    NoSuchFile(String),
+    NoSuchFile(PathBuf),
     #[error("{}", translate!("chmod-error-preserve-root", "file" => _0.quote()))]
-    PreserveRoot(String),
+    PreserveRoot(PathBuf),
     #[error("{}", translate!("chmod-error-permission-denied", "file" => _0.quote()))]
-    PermissionDenied(String),
-    #[error("{}", translate!("chmod-error-new-permissions", "file" => _0.clone(), "actual" => _1.clone(), "expected" => _2.clone()))]
-    NewPermissions(String, String, String),
+    PermissionDenied(PathBuf),
+    #[error("{}", translate!("chmod-error-new-permissions", "file" => _0.maybe_quote(), "actual" => _1.clone(), "expected" => _2.clone()))]
+    NewPermissions(PathBuf, String, String),
 }
 
 impl UError for ChmodError {}
@@ -123,7 +123,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         Some(fref) => match fs::metadata(fref) {
             Ok(meta) => Some(meta.mode() & 0o7777),
             Err(_) => {
-                return Err(ChmodError::CannotStat(fref.to_string_lossy().to_string()).into());
+                return Err(ChmodError::CannotStat(fref.into()).into());
             }
         },
         None => None,
@@ -384,22 +384,18 @@ impl Chmoder {
                     }
 
                     if !self.quiet {
-                        show!(ChmodError::DanglingSymlink(
-                            filename.to_string_lossy().to_string()
-                        ));
+                        show!(ChmodError::DanglingSymlink(filename.into()));
                         set_exit_code(1);
                     }
 
                     if self.verbose {
                         println!(
                             "{}",
-                            translate!("chmod-verbose-failed-dangling", "file" => filename.to_string_lossy().quote())
+                            translate!("chmod-verbose-failed-dangling", "file" => filename.quote())
                         );
                     }
                 } else if !self.quiet {
-                    show!(ChmodError::NoSuchFile(
-                        filename.to_string_lossy().to_string()
-                    ));
+                    show!(ChmodError::NoSuchFile(filename.into()));
                 }
                 // GNU exits with exit code 1 even if -q or --quiet are passed
                 // So we set the exit code, because it hasn't been set yet if `self.quiet` is true.
@@ -412,7 +408,7 @@ impl Chmoder {
                 continue;
             }
             if self.recursive && self.preserve_root && file == Path::new("/") {
-                return Err(ChmodError::PreserveRoot("/".to_string()).into());
+                return Err(ChmodError::PreserveRoot("/".into()).into());
             }
             if self.recursive {
                 r = self.walk_dir_with_context(file, true);
@@ -474,10 +470,7 @@ impl Chmoder {
                 Err(err) => {
                     // Handle permission denied errors with proper file path context
                     if err.kind() == std::io::ErrorKind::PermissionDenied {
-                        r = r.and(Err(ChmodError::PermissionDenied(
-                            file_path.to_string_lossy().to_string(),
-                        )
-                        .into()));
+                        r = r.and(Err(ChmodError::PermissionDenied(file_path.into()).into()));
                     } else {
                         r = r.and(Err(err.into()));
                     }
@@ -504,7 +497,7 @@ impl Chmoder {
                 // Handle permission denied with proper file path context
                 let e = dir_meta.unwrap_err();
                 let error = if e.kind() == std::io::ErrorKind::PermissionDenied {
-                    ChmodError::PermissionDenied(entry_path.to_string_lossy().to_string()).into()
+                    ChmodError::PermissionDenied(entry_path).into()
                 } else {
                     e.into()
                 };
@@ -530,10 +523,7 @@ impl Chmoder {
                         }
                         Err(err) => {
                             let error = if err.kind() == std::io::ErrorKind::PermissionDenied {
-                                ChmodError::PermissionDenied(
-                                    entry_path.to_string_lossy().to_string(),
-                                )
-                                .into()
+                                ChmodError::PermissionDenied(entry_path).into()
                             } else {
                                 err.into()
                             };
@@ -599,9 +589,7 @@ impl Chmoder {
                     new_mode
                 );
             }
-            return Err(
-                ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()).into(),
-            );
+            return Err(ChmodError::PermissionDenied(file_path.into()).into());
         }
 
         // Report the change using the helper method
@@ -638,11 +626,9 @@ impl Chmoder {
                     }
                     Ok(()) // Skip dangling symlinks
                 } else if err.kind() == std::io::ErrorKind::PermissionDenied {
-                    // These two filenames would normally be conditionally
-                    // quoted, but GNU's tests expect them to always be quoted
-                    Err(ChmodError::PermissionDenied(file.to_string_lossy().to_string()).into())
+                    Err(ChmodError::PermissionDenied(file.into()).into())
                 } else {
-                    Err(ChmodError::CannotStat(file.to_string_lossy().to_string()).into())
+                    Err(ChmodError::CannotStat(file.into()).into())
                 };
             }
         };
@@ -672,7 +658,7 @@ impl Chmoder {
                 // if a permission would have been removed if umask was 0, but it wasn't because umask was not 0, print an error and fail
                 if (new_mode & !naively_expected_new_mode) != 0 {
                     return Err(ChmodError::NewPermissions(
-                        file.to_string_lossy().to_string(),
+                        file.into(),
                         display_permissions_unix(new_mode as mode_t, false),
                         display_permissions_unix(naively_expected_new_mode as mode_t, false),
                     )

src/uu/chroot/src/chroot.rs

@@ -183,7 +183,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     }
 
     if !options.newroot.is_dir() {
-        return Err(ChrootError::NoSuchDirectory(format!("{}", options.newroot.display())).into());
+        return Err(ChrootError::NoSuchDirectory(options.newroot).into());
     }
 
     let commands = match matches.get_many::<String>(options::COMMAND) {
@@ -429,7 +429,7 @@ fn enter_chroot(root: &Path, skip_chdir: bool) -> UResult<()> {
     let err = unsafe {
         chroot(
             CString::new(root.as_os_str().as_bytes().to_vec())
-                .map_err(|e| ChrootError::CannotEnter("root".to_string(), e.into()))?
+                .map_err(|e| ChrootError::CannotEnter("root".into(), e.into()))?
                 .as_bytes_with_nul()
                 .as_ptr()
                 .cast(),
@@ -442,6 +442,6 @@ fn enter_chroot(root: &Path, skip_chdir: bool) -> UResult<()> {
         }
         Ok(())
     } else {
-        Err(ChrootError::CannotEnter(format!("{}", root.display()), Error::last_os_error()).into())
+        Err(ChrootError::CannotEnter(root.into(), Error::last_os_error()).into())
     }
 }

src/uu/chroot/src/error.rs

@@ -5,6 +5,7 @@
 // spell-checker:ignore NEWROOT Userspec userspec
 //! Errors returned by chroot.
 use std::io::Error;
+use std::path::PathBuf;
 use thiserror::Error;
 use uucore::display::Quotable;
 use uucore::error::UError;
@@ -16,7 +17,7 @@ use uucore::translate;
 pub enum ChrootError {
     /// Failed to enter the specified directory.
     #[error("{}", translate!("chroot-error-cannot-enter", "dir" => _0.quote(), "err" => _1))]
-    CannotEnter(String, #[source] Error),
+    CannotEnter(PathBuf, #[source] Error),
 
     /// Failed to execute the specified command.
     #[error("{}", translate!("chroot-error-command-failed", "cmd" => _0.quote(), "err" => _1))]
@@ -52,7 +53,7 @@ pub enum ChrootError {
 
     /// The given directory does not exist.
     #[error("{}", translate!("chroot-error-no-such-directory", "dir" => _0.quote()))]
-    NoSuchDirectory(String),
+    NoSuchDirectory(PathBuf),
 
     /// The call to `setgid()` failed.
     #[error("{}", translate!("chroot-error-set-gid-failed", "gid" => _0, "err" => _1))]

src/uu/comm/src/comm.rs

@@ -10,6 +10,7 @@ use std::ffi::OsString;
 use std::fs::{File, metadata};
 use std::io::{self, BufRead, BufReader, Read, StdinLock, stdin};
 use std::path::Path;
+use uucore::display::Quotable;
 use uucore::error::{FromIo, UResult, USimpleError};
 use uucore::format_usage;
 use uucore::fs::paths_refer_to_same_file;
@@ -310,9 +311,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let filename1 = matches.get_one::<OsString>(options::FILE_1).unwrap();
     let filename2 = matches.get_one::<OsString>(options::FILE_2).unwrap();
     let mut f1 = open_file(filename1, line_ending)
-        .map_err_context(|| filename1.to_string_lossy().to_string())?;
+        .map_err_context(|| filename1.maybe_quote().to_string())?;
     let mut f2 = open_file(filename2, line_ending)
-        .map_err_context(|| filename2.to_string_lossy().to_string())?;
+        .map_err_context(|| filename2.maybe_quote().to_string())?;
 
     // Due to default_value(), there must be at least one value here, thus unwrap() must not panic.
     let all_delimiters = matches

src/uu/cp/src/cp.rs

@@ -1739,13 +1739,13 @@ pub(crate) fn copy_attributes(
             if let Some(context) = context {
                 if let Err(e) = context.set_for_path(dest, false, false) {
                     return Err(CpError::Error(
-                        translate!("cp-error-selinux-set-context", "path" => dest.display(), "error" => e),
+                        translate!("cp-error-selinux-set-context", "path" => dest.quote(), "error" => e),
                     ));
                 }
             }
         } else {
             return Err(CpError::Error(
-                translate!("cp-error-selinux-get-context", "path" => source.display()),
+                translate!("cp-error-selinux-get-context", "path" => source.quote()),
             ));
         }
         Ok(())

src/uu/cp/src/platform/macos.rs

@@ -10,6 +10,7 @@ use std::os::unix::fs::OpenOptionsExt;
 use std::path::Path;
 
 use uucore::buf_copy;
+use uucore::display::Quotable;
 use uucore::translate;
 
 use uucore::mode::get_umask;
@@ -86,7 +87,7 @@ pub(crate) fn copy_on_write(
         // support COW).
         match reflink_mode {
             ReflinkMode::Always => {
-                return Err(translate!("cp-error-failed-to-clone", "source" => source.display(), "dest" => dest.display(), "error" => error)
+                return Err(translate!("cp-error-failed-to-clone", "source" => source.quote(), "dest" => dest.quote(), "error" => error)
                 .into());
             }
             _ => {