mirrors/coreutils
1
0
Fork 0
mirror of https://github.com/uutils/coreutils.git synced 2025-07-07 21:45:01 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

chore: address a few clippy lints that break API

Browse source 
* Disabled `avoid-breaking-exported-api` and sorted items in Clippy.toml
* Renamed `BSD` -> `Bsd`, `SYSV` -> `SysV`, and `CRC` -> `Crc` to match Rust naming rules
* Renamed items in `BackupMode` and `UpdateMode` because they repeated the same word in every item - making it redundant and harder to read
This commit is contained in:
Yuri Astrakhan 2025-04-08 12:43:23 -04:00
parent a067b58707
commit 61fafe9bda
9 changed files with 79 additions and 82 deletions
Download patch file Download diff file Expand all files Collapse all files

3
.clippy.toml
View file

@ -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

28
src/uu/cp/src/cp.rs
View file

@ -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)
&& options.backup != BackupMode::NumberedBackup
{
if copied_destinations.contains(&dest) && options.backup != BackupMode::Numbered {
// 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(());

10
src/uu/ln/src/ln.rs
View file

@ -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::SimpleBackup => Some(simple_backup_path(dst, &settings.suffix)),
BackupMode::NumberedBackup => Some(numbered_backup_path(dst)),
BackupMode::ExistingBackup => Some(existing_backup_path(dst, &settings.suffix)),
BackupMode::None => None,
BackupMode::Simple => Some(simple_backup_path(dst, &settings.suffix)),
BackupMode::Numbered => Some(numbered_backup_path(dst)),
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());

23
src/uu/mv/src/mv.rs
View file

@ -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::ReplaceNoneFail)
|| update_mode == UpdateMode::None
|| 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
&& source_is_target_backup(source, target, &opts.suffix)
{
if opts.backup == BackupMode::Simple && 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));
}

1
src/uu/stty/src/stty.rs
View file

@ -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,

48
src/uucore/src/lib/features/backup_control.rs
View file

@ -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),
"numbered" | "t" => Ok(BackupMode::NumberedBackup),
"existing" | "nil" => Ok(BackupMode::ExistingBackup),
"none" | "off" => Ok(BackupMode::NoBackup),
"simple" | "never" => Ok(BackupMode::Simple),
"numbered" | "t" => Ok(BackupMode::Numbered),
"existing" | "nil" => Ok(BackupMode::Existing),
"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::SimpleBackup => Some(simple_backup_path(backup_path, suffix)),
BackupMode::NumberedBackup => Some(numbered_backup_path(backup_path)),
BackupMode::ExistingBackup => Some(existing_backup_path(backup_path, suffix)),
BackupMode::None => None,
BackupMode::Simple => Some(simple_backup_path(backup_path, suffix)),
BackupMode::Numbered => Some(numbered_backup_path(backup_path)),
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) };
}

10
src/uucore/src/lib/features/checksum.rs
View file

@ -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,
Sha3_256, Sha3_384, Sha3_512, Sha224, Sha256, Sha384, Sha512, Shake128, Shake256, Sm3,
Blake2b, Blake3, Bsd, CRC32B, Crc, Digest, DigestWriter, Md5, Sha1, Sha3_224, Sha3_256,
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 {

14
src/uucore/src/lib/features/sum.rs
View file

@ -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 }
}

24
src/uucore/src/lib/features/update_control.rs
View file

@ -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,
ReplaceNoneFail,
IfOlder,
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,
"none" => UpdateMode::ReplaceNone,
"older" => UpdateMode::ReplaceIfOlder,
"none-fail" => UpdateMode::ReplaceNoneFail,
"all" => UpdateMode::All,
"none" => UpdateMode::None,
"older" => UpdateMode::IfOlder,
"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
}
}