Skip to content

Commit 4b3327d

Browse files
nyuriknyurik
committed
chore: address a few clippy lints that break API
* 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
1 parent a067b58 commit 4b3327d

File tree

9 files changed

+79
-82
lines changed

9 files changed

+79
-82
lines changed

‎.clippy.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
avoid-breaking-exported-api = false
2+
check-private-items = true
13
cognitive-complexity-threshold = 24
24
missing-docs-in-crate-items = true
3-
check-private-items = true

‎src/uu/cp/src/cp.rs‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
771771
} else if let Ok(mut matches) = matches {
772772
let options = Options::from_matches(&matches)?;
773773

774-
if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::NoBackup {
774+
if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::None {
775775
return Err(UUsageError::new(
776776
EXIT_ERR,
777777
"options --backup and --no-clobber are mutually exclusive",
@@ -997,7 +997,7 @@ impl Options {
997997
};
998998
let update_mode = update_control::determine_update_mode(matches);
999999

1000-
if backup_mode != BackupMode::NoBackup
1000+
if backup_mode != BackupMode::None
10011001
&& matches
10021002
.get_one::<String>(update_control::arguments::OPT_UPDATE)
10031003
.is_some_and(|v| v == "none" || v == "none-fail")
@@ -1331,7 +1331,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
13311331

13321332
for source in sources {
13331333
let normalized_source = normalize_path(source);
1334-
if options.backup == BackupMode::NoBackup && seen_sources.contains(&normalized_source) {
1334+
if options.backup == BackupMode::None && seen_sources.contains(&normalized_source) {
13351335
let file_type = if source.symlink_metadata()?.file_type().is_dir() {
13361336
"directory"
13371337
} else {
@@ -1353,9 +1353,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult
13531353
|| matches!(options.copy_mode, CopyMode::SymLink)
13541354
{
13551355
// There is already a file and it isn't a symlink (managed in a different place)
1356-
if copied_destinations.contains(&dest)
1357-
&& options.backup != BackupMode::NumberedBackup
1358-
{
1356+
if copied_destinations.contains(&dest) && options.backup != BackupMode::Numbered {
13591357
// If the target file was already created in this cp call, do not overwrite
13601358
return Err(Error::Error(format!(
13611359
"will not overwrite just-created '{}' with '{}'",
@@ -1788,7 +1786,7 @@ fn is_forbidden_to_copy_to_same_file(
17881786
if !paths_refer_to_same_file(source, dest, dereference_to_compare) {
17891787
return false;
17901788
}
1791-
if options.backup != BackupMode::NoBackup {
1789+
if options.backup != BackupMode::None {
17921790
if options.force() && !source_is_symlink {
17931791
return false;
17941792
}
@@ -1828,14 +1826,14 @@ fn handle_existing_dest(
18281826
return Err(format!("{} and {} are the same file", source.quote(), dest.quote()).into());
18291827
}
18301828

1831-
if options.update == UpdateMode::ReplaceNone {
1829+
if options.update == UpdateMode::None {
18321830
if options.debug {
18331831
println!("skipped {}", dest.quote());
18341832
}
18351833
return Err(Error::Skipped(false));
18361834
}
18371835

1838-
if options.update != UpdateMode::ReplaceIfOlder {
1836+
if options.update != UpdateMode::IfOlder {
18391837
options.overwrite.verify(dest, options.debug)?;
18401838
}
18411839

@@ -2081,7 +2079,7 @@ fn handle_copy_mode(
20812079
CopyMode::Update => {
20822080
if dest.exists() {
20832081
match options.update {
2084-
UpdateMode::ReplaceAll => {
2082+
UpdateMode::All => {
20852083
copy_helper(
20862084
source,
20872085
dest,
@@ -2094,17 +2092,17 @@ fn handle_copy_mode(
20942092
source_is_stream,
20952093
)?;
20962094
}
2097-
UpdateMode::ReplaceNone => {
2095+
UpdateMode::None => {
20982096
if options.debug {
20992097
println!("skipped {}", dest.quote());
21002098
}
21012099

21022100
return Ok(PerformedAction::Skipped);
21032101
}
2104-
UpdateMode::ReplaceNoneFail => {
2102+
UpdateMode::NoneFail => {
21052103
return Err(Error::Error(format!("not replacing '{}'", dest.display())));
21062104
}
2107-
UpdateMode::ReplaceIfOlder => {
2105+
UpdateMode::IfOlder => {
21082106
let dest_metadata = fs::symlink_metadata(dest)?;
21092107

21102108
let src_time = source_metadata.modified()?;
@@ -2261,7 +2259,7 @@ fn copy_file(
22612259
options.overwrite,
22622260
OverwriteMode::Clobber(ClobberMode::RemoveDestination)
22632261
)
2264-
&& options.backup == BackupMode::NoBackup
2262+
&& options.backup == BackupMode::None
22652263
{
22662264
fs::remove_file(dest)?;
22672265
}
@@ -2292,7 +2290,7 @@ fn copy_file(
22922290
if !options.dereference {
22932291
return Ok(());
22942292
}
2295-
} else if options.backup != BackupMode::NoBackup && !dest_is_symlink {
2293+
} else if options.backup != BackupMode::None && !dest_is_symlink {
22962294
if source == dest {
22972295
if !options.force() {
22982296
return Ok(());

‎src/uu/ln/src/ln.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,12 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
376376

377377
if dst.is_symlink() || dst.exists() {
378378
backup_path = match settings.backup {
379-
BackupMode::NoBackup => None,
380-
BackupMode::SimpleBackup => Some(simple_backup_path(dst, &settings.suffix)),
381-
BackupMode::NumberedBackup => Some(numbered_backup_path(dst)),
382-
BackupMode::ExistingBackup => Some(existing_backup_path(dst, &settings.suffix)),
379+
BackupMode::None => None,
380+
BackupMode::Simple => Some(simple_backup_path(dst, &settings.suffix)),
381+
BackupMode::Numbered => Some(numbered_backup_path(dst)),
382+
BackupMode::Existing => Some(existing_backup_path(dst, &settings.suffix)),
383383
};
384-
if settings.backup == BackupMode::ExistingBackup && !settings.symbolic {
384+
if settings.backup == BackupMode::Existing && !settings.symbolic {
385385
// when ln --backup f f, it should detect that it is the same file
386386
if paths_refer_to_same_file(src, dst, true) {
387387
return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into());

‎src/uu/mv/src/mv.rs‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
157157
let backup_mode = backup_control::determine_backup_mode(&matches)?;
158158
let update_mode = update_control::determine_update_mode(&matches);
159159

160-
if backup_mode != BackupMode::NoBackup
160+
if backup_mode != BackupMode::None
161161
&& (overwrite_mode == OverwriteMode::NoClobber
162-
|| update_mode == UpdateMode::ReplaceNone
163-
|| update_mode == UpdateMode::ReplaceNoneFail)
162+
|| update_mode == UpdateMode::None
163+
|| update_mode == UpdateMode::NoneFail)
164164
{
165165
return Err(UUsageError::new(
166166
1,
@@ -319,9 +319,7 @@ fn parse_paths(files: &[OsString], opts: &Options) -> Vec<PathBuf> {
319319
}
320320

321321
fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()> {
322-
if opts.backup == BackupMode::SimpleBackup
323-
&& source_is_target_backup(source, target, &opts.suffix)
324-
{
322+
if opts.backup == BackupMode::Simple && source_is_target_backup(source, target, &opts.suffix) {
325323
return Err(io::Error::new(
326324
io::ErrorKind::NotFound,
327325
format!(
@@ -346,7 +344,7 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()>
346344
if path_ends_with_terminator(target)
347345
&& (!target_is_dir && !source_is_dir)
348346
&& !opts.no_target_dir
349-
&& opts.update != UpdateMode::ReplaceIfOlder
347+
&& opts.update != UpdateMode::IfOlder
350348
{
351349
return Err(MvError::FailedToAccessNotADirectory(target.quote().to_string()).into());
352350
}
@@ -428,7 +426,7 @@ fn assert_not_same_file(
428426
let same_file = (canonicalized_source.eq(&canonicalized_target)
429427
|| are_hardlinks_to_same_file(source, target)
430428
|| are_hardlinks_or_one_way_symlink_to_same_file(source, target))
431-
&& opts.backup == BackupMode::NoBackup;
429+
&& opts.backup == BackupMode::None;
432430

433431
// get the expected target path to show in errors
434432
// 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)
539537
}
540538
};
541539

542-
if moved_destinations.contains(&targetpath) && options.backup != BackupMode::NumberedBackup
543-
{
540+
if moved_destinations.contains(&targetpath) && options.backup != BackupMode::Numbered {
544541
// If the target file was already created in this mv call, do not overwrite
545542
show!(USimpleError::new(
546543
1,
@@ -594,20 +591,20 @@ fn rename(
594591
let mut backup_path = None;
595592

596593
if to.exists() {
597-
if opts.update == UpdateMode::ReplaceNone {
594+
if opts.update == UpdateMode::None {
598595
if opts.debug {
599596
println!("skipped {}", to.quote());
600597
}
601598
return Ok(());
602599
}
603600

604-
if (opts.update == UpdateMode::ReplaceIfOlder)
601+
if (opts.update == UpdateMode::IfOlder)
605602
&& fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()?
606603
{
607604
return Ok(());
608605
}
609606

610-
if opts.update == UpdateMode::ReplaceNoneFail {
607+
if opts.update == UpdateMode::NoneFail {
611608
let err_msg = format!("not replacing {}", to.quote());
612609
return Err(io::Error::other(err_msg));
613610
}

‎src/uu/stty/src/stty.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const SUMMARY: &str = help_about!("stty.md");
4040
#[derive(Clone, Copy, Debug)]
4141
pub struct Flag<T> {
4242
name: &'static str,
43+
#[expect(clippy::struct_field_names)]
4344
flag: T,
4445
show: bool,
4546
sane: bool,

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

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,13 @@ pub const DEFAULT_BACKUP_SUFFIX: &str = "~";
123123
pub enum BackupMode {
124124
/// Argument 'none', 'off'
125125
#[default]
126-
NoBackup,
126+
None,
127127
/// Argument 'simple', 'never'
128-
SimpleBackup,
128+
Simple,
129129
/// Argument 'numbered', 't'
130-
NumberedBackup,
130+
Numbered,
131131
/// Argument 'existing', 'nil'
132-
ExistingBackup,
132+
Existing,
133133
}
134134

135135
/// Backup error types.
@@ -303,7 +303,7 @@ pub fn determine_backup_suffix(matches: &ArgMatches) -> String {
303303
/// ]);
304304
///
305305
/// let backup_mode = backup_control::determine_backup_mode(&matches).unwrap();
306-
/// assert_eq!(backup_mode, BackupMode::NumberedBackup)
306+
/// assert_eq!(backup_mode, BackupMode::Numbered)
307307
/// }
308308
/// ```
309309
///
@@ -348,7 +348,7 @@ pub fn determine_backup_mode(matches: &ArgMatches) -> UResult<BackupMode> {
348348
match_method(&method, "$VERSION_CONTROL")
349349
} else {
350350
// Default if no argument is provided to '--backup'
351-
Ok(BackupMode::ExistingBackup)
351+
Ok(BackupMode::Existing)
352352
}
353353
} else if matches.get_flag(arguments::OPT_BACKUP_NO_ARG) {
354354
// 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> {
357357
if let Ok(method) = env::var("VERSION_CONTROL") {
358358
match_method(&method, "$VERSION_CONTROL")
359359
} else {
360-
Ok(BackupMode::ExistingBackup)
360+
Ok(BackupMode::Existing)
361361
}
362362
} else {
363363
// No option was present at all
364-
Ok(BackupMode::NoBackup)
364+
Ok(BackupMode::None)
365365
}
366366
}
367367

@@ -391,10 +391,10 @@ fn match_method(method: &str, origin: &str) -> UResult<BackupMode> {
391391
.collect();
392392
if matches.len() == 1 {
393393
match *matches[0] {
394-
"simple" | "never" => Ok(BackupMode::SimpleBackup),
395-
"numbered" | "t" => Ok(BackupMode::NumberedBackup),
396-
"existing" | "nil" => Ok(BackupMode::ExistingBackup),
397-
"none" | "off" => Ok(BackupMode::NoBackup),
394+
"simple" | "never" => Ok(BackupMode::Simple),
395+
"numbered" | "t" => Ok(BackupMode::Numbered),
396+
"existing" | "nil" => Ok(BackupMode::Existing),
397+
"none" | "off" => Ok(BackupMode::None),
398398
_ => unreachable!(), // cannot happen as we must have exactly one match
399399
// from the list above.
400400
}
@@ -411,10 +411,10 @@ pub fn get_backup_path(
411411
suffix: &str,
412412
) -> Option<PathBuf> {
413413
match backup_mode {
414-
BackupMode::NoBackup => None,
415-
BackupMode::SimpleBackup => Some(simple_backup_path(backup_path, suffix)),
416-
BackupMode::NumberedBackup => Some(numbered_backup_path(backup_path)),
417-
BackupMode::ExistingBackup => Some(existing_backup_path(backup_path, suffix)),
414+
BackupMode::None => None,
415+
BackupMode::Simple => Some(simple_backup_path(backup_path, suffix)),
416+
BackupMode::Numbered => Some(numbered_backup_path(backup_path)),
417+
BackupMode::Existing => Some(existing_backup_path(backup_path, suffix)),
418418
}
419419
}
420420

@@ -511,7 +511,7 @@ mod tests {
511511

512512
let result = determine_backup_mode(&matches).unwrap();
513513

514-
assert_eq!(result, BackupMode::ExistingBackup);
514+
assert_eq!(result, BackupMode::Existing);
515515
}
516516

517517
// --backup takes precedence over -b
@@ -522,7 +522,7 @@ mod tests {
522522

523523
let result = determine_backup_mode(&matches).unwrap();
524524

525-
assert_eq!(result, BackupMode::NoBackup);
525+
assert_eq!(result, BackupMode::None);
526526
}
527527

528528
// --backup can be passed without an argument
@@ -533,7 +533,7 @@ mod tests {
533533

534534
let result = determine_backup_mode(&matches).unwrap();
535535

536-
assert_eq!(result, BackupMode::ExistingBackup);
536+
assert_eq!(result, BackupMode::Existing);
537537
}
538538

539539
// --backup can be passed with an argument only
@@ -544,7 +544,7 @@ mod tests {
544544

545545
let result = determine_backup_mode(&matches).unwrap();
546546

547-
assert_eq!(result, BackupMode::SimpleBackup);
547+
assert_eq!(result, BackupMode::Simple);
548548
}
549549

550550
// --backup errors on invalid argument
@@ -581,7 +581,7 @@ mod tests {
581581

582582
let result = determine_backup_mode(&matches).unwrap();
583583

584-
assert_eq!(result, BackupMode::SimpleBackup);
584+
assert_eq!(result, BackupMode::Simple);
585585
}
586586

587587
// -b doesn't ignores the "VERSION_CONTROL" environment variable
@@ -593,7 +593,7 @@ mod tests {
593593

594594
let result = determine_backup_mode(&matches).unwrap();
595595

596-
assert_eq!(result, BackupMode::NumberedBackup);
596+
assert_eq!(result, BackupMode::Numbered);
597597
unsafe { env::remove_var(ENV_VERSION_CONTROL) };
598598
}
599599

@@ -606,7 +606,7 @@ mod tests {
606606

607607
let result = determine_backup_mode(&matches).unwrap();
608608

609-
assert_eq!(result, BackupMode::NoBackup);
609+
assert_eq!(result, BackupMode::None);
610610
unsafe { env::remove_var(ENV_VERSION_CONTROL) };
611611
}
612612

@@ -649,7 +649,7 @@ mod tests {
649649

650650
let result = determine_backup_mode(&matches).unwrap();
651651

652-
assert_eq!(result, BackupMode::SimpleBackup);
652+
assert_eq!(result, BackupMode::Simple);
653653
unsafe { env::remove_var(ENV_VERSION_CONTROL) };
654654
}
655655

0 commit comments

Comments
 (0)