Skip to content

chore: address a few clippy lints that break API #7697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvestre merged 1 commit into from
Apr 15, 2025
Merged

chore: address a few clippy lints that break API #7697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .clippy.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 changes: 13 additions & 15 deletions src/uu/cp/src/cp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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 '{}'",
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)?;
}

Expand Down Expand Up @@ -2081,7 +2079,7 @@ fn handle_copy_mode(
CopyMode::Update => {
if dest.exists() {
match options.update {
UpdateMode::ReplaceAll => {
UpdateMode::All => {
copy_helper(
source,
dest,
Expand All @@ -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()?;
Expand Down Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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(());
Expand Down
10 changes: 5 additions & 5 deletions src/uu/ln/src/ln.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
23 changes: 10 additions & 13 deletions src/uu/mv/src/mv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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!(
Expand All @@ -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());
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
}
Expand Down
1 change: 1 addition & 0 deletions src/uu/stty/src/stty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 24 additions & 24 deletions src/uucore/src/lib/features/backup_control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
/// }
/// ```
///
Expand Down Expand Up @@ -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.
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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.
}
Expand All @@ -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)),
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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) };
}

Expand All @@ -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) };
}

Expand Down Expand Up @@ -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) };
}

Expand Down
Loading
Loading