Merge pull request #7697 from nyurik/names

chore: address a few clippy lints that break API

.clippy.toml

@@ -1,3 +1,4 @@
+avoid-breaking-exported-api = false
+check-private-items = true
 cognitive-complexity-threshold = 24
 missing-docs-in-crate-items = true
-check-private-items = true

src/uu/cp/src/cp.rs

@@ -771,7 +771,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     } else if let Ok(mut matches) = matches {
         let options = Options::from_matches(&matches)?;
 
-        if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::NoBackup {
+        if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::None {
             return Err(UUsageError::new(
                 EXIT_ERR,
                 "options --backup and --no-clobber are mutually exclusive",
@@ -997,7 +997,7 @@ impl Options {
         };
         let update_mode = update_control::determine_update_mode(matches);
 
-        if backup_mode != BackupMode::NoBackup
+        if backup_mode != BackupMode::None
             && matches
                 .get_one::<String>(update_control::arguments::OPT_UPDATE)
                 .is_some_and(|v| v == "none" || v == "none-fail")
@@ -1331,7 +1331,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
 
     for source in sources {
         let normalized_source = normalize_path(source);
-        if options.backup == BackupMode::NoBackup && seen_sources.contains(&normalized_source) {
+        if options.backup == BackupMode::None && seen_sources.contains(&normalized_source) {
             let file_type = if source.symlink_metadata()?.file_type().is_dir() {
                 "directory"
             } else {
@@ -1353,9 +1353,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
                 || matches!(options.copy_mode, CopyMode::SymLink)
             {
                 // There is already a file and it isn't a symlink (managed in a different place)
-                if copied_destinations.contains(&dest)
+                if copied_destinations.contains(&dest) && options.backup != BackupMode::Numbered {
-                    && options.backup != BackupMode::NumberedBackup
-                {
                     // If the target file was already created in this cp call, do not overwrite
                     return Err(Error::Error(format!(
                         "will not overwrite just-created '{}' with '{}'",
@@ -1788,7 +1786,7 @@ fn is_forbidden_to_copy_to_same_file(
     if !paths_refer_to_same_file(source, dest, dereference_to_compare) {
         return false;
     }
-    if options.backup != BackupMode::NoBackup {
+    if options.backup != BackupMode::None {
         if options.force() && !source_is_symlink {
             return false;
         }
@@ -1828,14 +1826,14 @@ fn handle_existing_dest(
         return Err(format!("{} and {} are the same file", source.quote(), dest.quote()).into());
     }
 
-    if options.update == UpdateMode::ReplaceNone {
+    if options.update == UpdateMode::None {
         if options.debug {
             println!("skipped {}", dest.quote());
         }
         return Err(Error::Skipped(false));
     }
 
-    if options.update != UpdateMode::ReplaceIfOlder {
+    if options.update != UpdateMode::IfOlder {
         options.overwrite.verify(dest, options.debug)?;
     }
 
@@ -2081,7 +2079,7 @@ fn handle_copy_mode(
         CopyMode::Update => {
             if dest.exists() {
                 match options.update {
-                    UpdateMode::ReplaceAll => {
+                    UpdateMode::All => {
                         copy_helper(
                             source,
                             dest,
@@ -2094,17 +2092,17 @@ fn handle_copy_mode(
                             source_is_stream,
                         )?;
                     }
-                    UpdateMode::ReplaceNone => {
+                    UpdateMode::None => {
                         if options.debug {
                             println!("skipped {}", dest.quote());
                         }
 
                         return Ok(PerformedAction::Skipped);
                     }
-                    UpdateMode::ReplaceNoneFail => {
+                    UpdateMode::NoneFail => {
                         return Err(Error::Error(format!("not replacing '{}'", dest.display())));
                     }
-                    UpdateMode::ReplaceIfOlder => {
+                    UpdateMode::IfOlder => {
                         let dest_metadata = fs::symlink_metadata(dest)?;
 
                         let src_time = source_metadata.modified()?;
@@ -2261,7 +2259,7 @@ fn copy_file(
                 options.overwrite,
                 OverwriteMode::Clobber(ClobberMode::RemoveDestination)
             )
-            && options.backup == BackupMode::NoBackup
+            && options.backup == BackupMode::None
         {
             fs::remove_file(dest)?;
         }
@@ -2292,7 +2290,7 @@ fn copy_file(
                 if !options.dereference {
                     return Ok(());
                 }
-            } else if options.backup != BackupMode::NoBackup && !dest_is_symlink {
+            } else if options.backup != BackupMode::None && !dest_is_symlink {
                 if source == dest {
                     if !options.force() {
                         return Ok(());

src/uu/ln/src/ln.rs

@@ -376,12 +376,12 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
 
     if dst.is_symlink() || dst.exists() {
         backup_path = match settings.backup {
-            BackupMode::NoBackup => None,
+            BackupMode::None => None,
-            BackupMode::SimpleBackup => Some(simple_backup_path(dst, &settings.suffix)),
+            BackupMode::Simple => Some(simple_backup_path(dst, &settings.suffix)),
-            BackupMode::NumberedBackup => Some(numbered_backup_path(dst)),
+            BackupMode::Numbered => Some(numbered_backup_path(dst)),
-            BackupMode::ExistingBackup => Some(existing_backup_path(dst, &settings.suffix)),
+            BackupMode::Existing => Some(existing_backup_path(dst, &settings.suffix)),
         };
-        if settings.backup == BackupMode::ExistingBackup && !settings.symbolic {
+        if settings.backup == BackupMode::Existing && !settings.symbolic {
             // when ln --backup f f, it should detect that it is the same file
             if paths_refer_to_same_file(src, dst, true) {
                 return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into());

src/uu/mv/src/mv.rs

@@ -157,10 +157,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let backup_mode = backup_control::determine_backup_mode(&matches)?;
     let update_mode = update_control::determine_update_mode(&matches);
 
-    if backup_mode != BackupMode::NoBackup
+    if backup_mode != BackupMode::None
         && (overwrite_mode == OverwriteMode::NoClobber
-            || update_mode == UpdateMode::ReplaceNone
+            || update_mode == UpdateMode::None
-            || update_mode == UpdateMode::ReplaceNoneFail)
+            || update_mode == UpdateMode::NoneFail)
     {
         return Err(UUsageError::new(
             1,
@@ -319,9 +319,7 @@ fn parse_paths(files: &[OsString], opts: &Options) -> Vec<PathBuf> {
 }
 
 fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()> {
-    if opts.backup == BackupMode::SimpleBackup
+    if opts.backup == BackupMode::Simple && source_is_target_backup(source, target, &opts.suffix) {
-        && source_is_target_backup(source, target, &opts.suffix)
-    {
         return Err(io::Error::new(
             io::ErrorKind::NotFound,
             format!(
@@ -346,7 +344,7 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()>
     if path_ends_with_terminator(target)
         && (!target_is_dir && !source_is_dir)
         && !opts.no_target_dir
-        && opts.update != UpdateMode::ReplaceIfOlder
+        && opts.update != UpdateMode::IfOlder
     {
         return Err(MvError::FailedToAccessNotADirectory(target.quote().to_string()).into());
     }
@@ -428,7 +426,7 @@ fn assert_not_same_file(
     let same_file = (canonicalized_source.eq(&canonicalized_target)
         || are_hardlinks_to_same_file(source, target)
         || are_hardlinks_or_one_way_symlink_to_same_file(source, target))
-        && opts.backup == BackupMode::NoBackup;
+        && opts.backup == BackupMode::None;
 
     // get the expected target path to show in errors
     // this is based on the argument and not canonicalized
@@ -539,8 +537,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, options: &Options)
             }
         };
 
-        if moved_destinations.contains(&targetpath) && options.backup != BackupMode::NumberedBackup
+        if moved_destinations.contains(&targetpath) && options.backup != BackupMode::Numbered {
-        {
             // If the target file was already created in this mv call, do not overwrite
             show!(USimpleError::new(
                 1,
@@ -594,20 +591,20 @@ fn rename(
     let mut backup_path = None;
 
     if to.exists() {
-        if opts.update == UpdateMode::ReplaceNone {
+        if opts.update == UpdateMode::None {
             if opts.debug {
                 println!("skipped {}", to.quote());
             }
             return Ok(());
         }
 
-        if (opts.update == UpdateMode::ReplaceIfOlder)
+        if (opts.update == UpdateMode::IfOlder)
             && fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()?
         {
             return Ok(());
         }
 
-        if opts.update == UpdateMode::ReplaceNoneFail {
+        if opts.update == UpdateMode::NoneFail {
             let err_msg = format!("not replacing {}", to.quote());
             return Err(io::Error::other(err_msg));
         }

src/uu/stty/src/stty.rs

@@ -40,6 +40,7 @@ const SUMMARY: &str = help_about!("stty.md");
 #[derive(Clone, Copy, Debug)]
 pub struct Flag<T> {
     name: &'static str,
+    #[expect(clippy::struct_field_names)]
     flag: T,
     show: bool,
     sane: bool,

src/uucore/src/lib/features/backup_control.rs

@@ -123,13 +123,13 @@ pub const DEFAULT_BACKUP_SUFFIX: &str = "~";
 pub enum BackupMode {
     /// Argument 'none', 'off'
     #[default]
-    NoBackup,
+    None,
     /// Argument 'simple', 'never'
-    SimpleBackup,
+    Simple,
     /// Argument 'numbered', 't'
-    NumberedBackup,
+    Numbered,
     /// Argument 'existing', 'nil'
-    ExistingBackup,
+    Existing,
 }
 
 /// Backup error types.
@@ -303,7 +303,7 @@ pub fn determine_backup_suffix(matches: &ArgMatches) -> String {
 ///         ]);
 ///
 ///     let backup_mode = backup_control::determine_backup_mode(&matches).unwrap();
-///     assert_eq!(backup_mode, BackupMode::NumberedBackup)
+///     assert_eq!(backup_mode, BackupMode::Numbered)
 /// }
 /// ```
 ///
@@ -348,7 +348,7 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult<BackupMode> {
             match_method(&method, "$VERSION_CONTROL")
         } else {
             // Default if no argument is provided to '--backup'
-            Ok(BackupMode::ExistingBackup)
+            Ok(BackupMode::Existing)
         }
     } else if matches.get_flag(arguments::OPT_BACKUP_NO_ARG) {
         // the short form of this option, -b does not accept any argument.
@@ -357,11 +357,11 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult<BackupMode> {
         if let Ok(method) = env::var("VERSION_CONTROL") {
             match_method(&method, "$VERSION_CONTROL")
         } else {
-            Ok(BackupMode::ExistingBackup)
+            Ok(BackupMode::Existing)
         }
     } else {
         // No option was present at all
-        Ok(BackupMode::NoBackup)
+        Ok(BackupMode::None)
     }
 }
 
@@ -391,10 +391,10 @@ fn match_method(method: &str, origin: &str) -> UResult<BackupMode> {
         .collect();
     if matches.len() == 1 {
         match *matches[0] {
-            "simple" | "never" => Ok(BackupMode::SimpleBackup),
+            "simple" | "never" => Ok(BackupMode::Simple),
-            "numbered" | "t" => Ok(BackupMode::NumberedBackup),
+            "numbered" | "t" => Ok(BackupMode::Numbered),
-            "existing" | "nil" => Ok(BackupMode::ExistingBackup),
+            "existing" | "nil" => Ok(BackupMode::Existing),
-            "none" | "off" => Ok(BackupMode::NoBackup),
+            "none" | "off" => Ok(BackupMode::None),
             _ => unreachable!(), // cannot happen as we must have exactly one match
                                  // from the list above.
         }
@@ -411,10 +411,10 @@ pub fn get_backup_path(
     suffix: &str,
 ) -> Option<PathBuf> {
     match backup_mode {
-        BackupMode::NoBackup => None,
+        BackupMode::None => None,
-        BackupMode::SimpleBackup => Some(simple_backup_path(backup_path, suffix)),
+        BackupMode::Simple => Some(simple_backup_path(backup_path, suffix)),
-        BackupMode::NumberedBackup => Some(numbered_backup_path(backup_path)),
+        BackupMode::Numbered => Some(numbered_backup_path(backup_path)),
-        BackupMode::ExistingBackup => Some(existing_backup_path(backup_path, suffix)),
+        BackupMode::Existing => Some(existing_backup_path(backup_path, suffix)),
     }
 }
 
@@ -511,7 +511,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::ExistingBackup);
+        assert_eq!(result, BackupMode::Existing);
     }
 
     // --backup takes precedence over -b
@@ -522,7 +522,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::NoBackup);
+        assert_eq!(result, BackupMode::None);
     }
 
     // --backup can be passed without an argument
@@ -533,7 +533,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::ExistingBackup);
+        assert_eq!(result, BackupMode::Existing);
     }
 
     // --backup can be passed with an argument only
@@ -544,7 +544,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::SimpleBackup);
+        assert_eq!(result, BackupMode::Simple);
     }
 
     // --backup errors on invalid argument
@@ -581,7 +581,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::SimpleBackup);
+        assert_eq!(result, BackupMode::Simple);
     }
 
     // -b doesn't ignores the "VERSION_CONTROL" environment variable
@@ -593,7 +593,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::NumberedBackup);
+        assert_eq!(result, BackupMode::Numbered);
         unsafe { env::remove_var(ENV_VERSION_CONTROL) };
     }
 
@@ -606,7 +606,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::NoBackup);
+        assert_eq!(result, BackupMode::None);
         unsafe { env::remove_var(ENV_VERSION_CONTROL) };
     }
 
@@ -649,7 +649,7 @@ mod tests {
 
         let result = determine_backup_mode(&matches).unwrap();
 
-        assert_eq!(result, BackupMode::SimpleBackup);
+        assert_eq!(result, BackupMode::Simple);
         unsafe { env::remove_var(ENV_VERSION_CONTROL) };
     }
 

src/uucore/src/lib/features/checksum.rs

@@ -20,8 +20,8 @@ use crate::{
     error::{FromIo, UError, UResult, USimpleError},
     os_str_as_bytes, os_str_from_bytes, read_os_string_lines, show, show_error, show_warning_caps,
     sum::{
-        BSD, Blake2b, Blake3, CRC, CRC32B, Digest, DigestWriter, Md5, SYSV, Sha1, Sha3_224,
+        Blake2b, Blake3, Bsd, CRC32B, Crc, Digest, DigestWriter, Md5, Sha1, Sha3_224, Sha3_256,
-        Sha3_256, Sha3_384, Sha3_512, Sha224, Sha256, Sha384, Sha512, Shake128, Shake256, Sm3,
+        Sha3_384, Sha3_512, Sha224, Sha256, Sha384, Sha512, Shake128, Shake256, Sm3, SysV,
     },
     util_name,
 };
@@ -364,17 +364,17 @@ pub fn detect_algo(algo: &str, length: Option<usize>) -> UResult<HashAlgorithm>
     match algo {
         ALGORITHM_OPTIONS_SYSV => Ok(HashAlgorithm {
             name: ALGORITHM_OPTIONS_SYSV,
-            create_fn: Box::new(|| Box::new(SYSV::new())),
+            create_fn: Box::new(|| Box::new(SysV::new())),
             bits: 512,
         }),
         ALGORITHM_OPTIONS_BSD => Ok(HashAlgorithm {
             name: ALGORITHM_OPTIONS_BSD,
-            create_fn: Box::new(|| Box::new(BSD::new())),
+            create_fn: Box::new(|| Box::new(Bsd::new())),
             bits: 1024,
         }),
         ALGORITHM_OPTIONS_CRC => Ok(HashAlgorithm {
             name: ALGORITHM_OPTIONS_CRC,
-            create_fn: Box::new(|| Box::new(CRC::new())),
+            create_fn: Box::new(|| Box::new(Crc::new())),
             bits: 256,
         }),
         ALGORITHM_OPTIONS_CRC32B => Ok(HashAlgorithm {

src/uucore/src/lib/features/sum.rs

@@ -125,12 +125,12 @@ impl Digest for Sm3 {
 // NOTE: CRC_TABLE_LEN *must* be <= 256 as we cast 0..CRC_TABLE_LEN to u8
 const CRC_TABLE_LEN: usize = 256;
 
-pub struct CRC {
+pub struct Crc {
     state: u32,
     size: usize,
     crc_table: [u32; CRC_TABLE_LEN],
 }
-impl CRC {
+impl Crc {
     fn generate_crc_table() -> [u32; CRC_TABLE_LEN] {
         let mut table = [0; CRC_TABLE_LEN];
 
@@ -166,7 +166,7 @@ impl CRC {
     }
 }
 
-impl Digest for CRC {
+impl Digest for Crc {
     fn new() -> Self {
         Self {
             state: 0,
@@ -238,10 +238,10 @@ impl Digest for CRC32B {
     }
 }
 
-pub struct BSD {
+pub struct Bsd {
     state: u16,
 }
-impl Digest for BSD {
+impl Digest for Bsd {
     fn new() -> Self {
         Self { state: 0 }
     }
@@ -272,10 +272,10 @@ impl Digest for BSD {
     }
 }
 
-pub struct SYSV {
+pub struct SysV {
     state: u32,
 }
-impl Digest for SYSV {
+impl Digest for SysV {
     fn new() -> Self {
         Self { state: 0 }
     }

src/uucore/src/lib/features/update_control.rs

@@ -39,7 +39,7 @@
 //!     let update_mode = update_control::determine_update_mode(&matches);
 //!
 //!     // handle cases
-//!     if update_mode == UpdateMode::ReplaceIfOlder {
+//!     if update_mode == UpdateMode::IfOlder {
 //!         // do
 //!     } else {
 //!         unreachable!()
@@ -53,14 +53,14 @@ use clap::ArgMatches;
 pub enum UpdateMode {
     /// --update=`all`, ``
     #[default]
-    ReplaceAll,
+    All,
     /// --update=`none`
-    ReplaceNone,
+    None,
 
     /// --update=`older`
     /// -u
-    ReplaceIfOlder,
+    IfOlder,
-    ReplaceNoneFail,
+    NoneFail,
 }
 
 pub mod arguments {
@@ -124,22 +124,22 @@ pub mod arguments {
 ///         ]);
 ///
 ///     let update_mode = update_control::determine_update_mode(&matches);
-///     assert_eq!(update_mode, UpdateMode::ReplaceAll)
+///     assert_eq!(update_mode, UpdateMode::All)
 /// }
 pub fn determine_update_mode(matches: &ArgMatches) -> UpdateMode {
     if let Some(mode) = matches.get_one::<String>(arguments::OPT_UPDATE) {
         match mode.as_str() {
-            "all" => UpdateMode::ReplaceAll,
+            "all" => UpdateMode::All,
-            "none" => UpdateMode::ReplaceNone,
+            "none" => UpdateMode::None,
-            "older" => UpdateMode::ReplaceIfOlder,
+            "older" => UpdateMode::IfOlder,
-            "none-fail" => UpdateMode::ReplaceNoneFail,
+            "none-fail" => UpdateMode::NoneFail,
             _ => unreachable!("other args restricted by clap"),
         }
     } else if matches.get_flag(arguments::OPT_UPDATE_NO_ARG) {
         // short form of this option is equivalent to using --update=older
-        UpdateMode::ReplaceIfOlder
+        UpdateMode::IfOlder
     } else {
         // no option was present
-        UpdateMode::ReplaceAll
+        UpdateMode::All
     }
 }