From b5ed4a4acfff35d9503085048d367b43fd4b5c4c Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Fri, 11 Apr 2025 00:23:55 -0400 Subject: [PATCH] chore: simplify return in multi-branch It is usually easier to reason about the code if the return is used in one place rather than in every branch of the match or if statements. --- src/uu/chmod/src/chmod.rs | 12 +++++------ src/uu/cp/src/copydir.rs | 8 ++++---- src/uu/cp/src/cp.rs | 6 +++--- src/uu/df/src/blocks.rs | 6 +----- src/uu/env/src/env.rs | 28 ++++++++++++++------------ src/uu/expand/src/expand.rs | 10 ++++----- src/uu/fmt/src/fmt.rs | 10 ++++----- src/uu/id/src/id.rs | 10 ++++----- src/uu/readlink/src/readlink.rs | 10 ++++----- src/uu/tail/src/follow/files.rs | 7 ++++--- src/uu/unexpand/src/unexpand.rs | 14 ++++++------- src/uucore/src/lib/features/signals.rs | 6 +----- 12 files changed, 61 insertions(+), 66 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index e9defffd9..dfe304859 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -357,24 +357,24 @@ impl Chmoder { Ok(meta) => meta.mode() & 0o7777, Err(err) => { // Handle dangling symlinks or other errors - if file.is_symlink() && !self.dereference { + return if file.is_symlink() && !self.dereference { if self.verbose { println!( "neither symbolic link {} nor referent has been changed", file.quote() ); } - return Ok(()); // Skip dangling symlinks + 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 - return Err(USimpleError::new( + Err(USimpleError::new( 1, format!("{}: Permission denied", file.quote()), - )); + )) } else { - return Err(USimpleError::new(1, format!("{}: {err}", file.quote()))); - } + Err(USimpleError::new(1, format!("{}: {err}", file.quote()))) + }; } }; diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 40b5456db..567cca1d5 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -251,15 +251,15 @@ fn copy_direntry( && !ends_with_slash_dot(&source_absolute) && !local_to_target.exists() { - if target_is_file { - return Err("cannot overwrite non-directory with directory".into()); + return if target_is_file { + Err("cannot overwrite non-directory with directory".into()) } else { build_dir(&local_to_target, false, options, Some(&source_absolute))?; if options.verbose { println!("{}", context_for(&source_relative, &local_to_target)); } - return Ok(()); - } + Ok(()) + }; } // If the source is not a directory, then we need to copy the file. diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 4c294ba16..a884f114d 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -2451,10 +2451,10 @@ fn handle_no_preserve_mode(options: &Options, org_mode: u32) -> u32 { { const MODE_RW_UGO: u32 = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; const S_IRWXUGO: u32 = S_IRWXU | S_IRWXG | S_IRWXO; - if is_explicit_no_preserve_mode { - return MODE_RW_UGO; + return if is_explicit_no_preserve_mode { + MODE_RW_UGO } else { - return org_mode & S_IRWXUGO; + org_mode & S_IRWXUGO }; } diff --git a/src/uu/df/src/blocks.rs b/src/uu/df/src/blocks.rs index 6d9e24001..26b763cac 100644 --- a/src/uu/df/src/blocks.rs +++ b/src/uu/df/src/blocks.rs @@ -184,11 +184,7 @@ pub(crate) fn read_block_size(matches: &ArgMatches) -> Result Option { for env_var in ["DF_BLOCK_SIZE", "BLOCK_SIZE", "BLOCKSIZE"] { if let Ok(env_size) = env::var(env_var) { - if let Ok(size) = parse_size_u64(&env_size) { - return Some(size); - } else { - return None; - } + return parse_size_u64(&env_size).ok(); } } diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index f17d70ed6..92288c4a8 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -581,19 +581,21 @@ impl EnvAppData { } return Err(exit.code().unwrap().into()); } - Err(ref err) => match err.kind() { - io::ErrorKind::NotFound | io::ErrorKind::InvalidInput => { - return Err(self.make_error_no_such_file_or_dir(prog.deref())); - } - io::ErrorKind::PermissionDenied => { - uucore::show_error!("{}: Permission denied", prog.quote()); - return Err(126.into()); - } - _ => { - uucore::show_error!("unknown error: {err:?}"); - return Err(126.into()); - } - }, + Err(ref err) => { + return match err.kind() { + io::ErrorKind::NotFound | io::ErrorKind::InvalidInput => { + Err(self.make_error_no_such_file_or_dir(prog.deref())) + } + io::ErrorKind::PermissionDenied => { + uucore::show_error!("{}: Permission denied", prog.quote()); + Err(126.into()) + } + _ => { + uucore::show_error!("unknown error: {err:?}"); + Err(126.into()) + } + }; + } Ok(_) => (), } Ok(()) diff --git a/src/uu/expand/src/expand.rs b/src/uu/expand/src/expand.rs index 1e29f5aac..a334bff3d 100644 --- a/src/uu/expand/src/expand.rs +++ b/src/uu/expand/src/expand.rs @@ -145,14 +145,14 @@ fn tabstops_parse(s: &str) -> Result<(RemainingMode, Vec), ParseError> { } let s = s.trim_start_matches(char::is_numeric); - if s.starts_with('/') || s.starts_with('+') { - return Err(ParseError::SpecifierNotAtStartOfNumber( + return if s.starts_with('/') || s.starts_with('+') { + Err(ParseError::SpecifierNotAtStartOfNumber( s[0..1].to_string(), s.to_string(), - )); + )) } else { - return Err(ParseError::InvalidCharacter(s.to_string())); - } + Err(ParseError::InvalidCharacter(s.to_string())) + }; } } } diff --git a/src/uu/fmt/src/fmt.rs b/src/uu/fmt/src/fmt.rs index 2fdc17746..497718f28 100644 --- a/src/uu/fmt/src/fmt.rs +++ b/src/uu/fmt/src/fmt.rs @@ -270,14 +270,14 @@ fn extract_files(matches: &ArgMatches) -> UResult> { fn extract_width(matches: &ArgMatches) -> UResult> { let width_opt = matches.get_one::(options::WIDTH); if let Some(width_str) = width_opt { - if let Ok(width) = width_str.parse::() { - return Ok(Some(width)); + return if let Ok(width) = width_str.parse::() { + Ok(Some(width)) } else { - return Err(USimpleError::new( + Err(USimpleError::new( 1, format!("invalid width: {}", width_str.quote()), - )); - } + )) + }; } if let Some(1) = matches.index_of(options::FILES_OR_WIDTH) { diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 37bb208ac..9fc446e46 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -176,7 +176,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let line_ending = LineEnding::from_zero_flag(state.zflag); if state.cflag { - if state.selinux_supported { + return if state.selinux_supported { // print SElinux context and exit #[cfg(all(any(target_os = "linux", target_os = "android"), feature = "selinux"))] if let Ok(context) = selinux::SecurityContext::current(false) { @@ -186,13 +186,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // print error because `cflag` was explicitly requested return Err(USimpleError::new(1, "can't get process context")); } - return Ok(()); + Ok(()) } else { - return Err(USimpleError::new( + Err(USimpleError::new( 1, "--context (-Z) works only on an SELinux-enabled kernel", - )); - } + )) + }; } for i in 0..=users.len() { diff --git a/src/uu/readlink/src/readlink.rs b/src/uu/readlink/src/readlink.rs index 86eca253a..211422e03 100644 --- a/src/uu/readlink/src/readlink.rs +++ b/src/uu/readlink/src/readlink.rs @@ -84,15 +84,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { show(&path, line_ending).map_err_context(String::new)?; } Err(err) => { - if verbose { - return Err(USimpleError::new( + return if verbose { + Err(USimpleError::new( 1, err.map_err_context(move || f.maybe_quote().to_string()) .to_string(), - )); + )) } else { - return Err(1.into()); - } + Err(1.into()) + }; } } } diff --git a/src/uu/tail/src/follow/files.rs b/src/uu/tail/src/follow/files.rs index 509d84d84..0fcf90e2a 100644 --- a/src/uu/tail/src/follow/files.rs +++ b/src/uu/tail/src/follow/files.rs @@ -162,12 +162,13 @@ impl FileHandling { pub fn needs_header(&self, path: &Path, verbose: bool) -> bool { if verbose { if let Some(ref last) = self.last { - return !last.eq(&path); + !last.eq(&path) } else { - return true; + true } + } else { + false } - false } } diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs index 5b66b4daa..7d12daf0c 100644 --- a/src/uu/unexpand/src/unexpand.rs +++ b/src/uu/unexpand/src/unexpand.rs @@ -44,14 +44,14 @@ fn tabstops_parse(s: &str) -> Result, ParseError> { for word in words { match word.parse::() { Ok(num) => nums.push(num), - Err(e) => match e.kind() { - IntErrorKind::PosOverflow => return Err(ParseError::TabSizeTooLarge), - _ => { - return Err(ParseError::InvalidCharacter( + Err(e) => { + return match e.kind() { + IntErrorKind::PosOverflow => Err(ParseError::TabSizeTooLarge), + _ => Err(ParseError::InvalidCharacter( word.trim_start_matches(char::is_numeric).to_string(), - )); - } - }, + )), + }; + } } } diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 713aef002..4e7fe81c9 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -350,11 +350,7 @@ pub static ALL_SIGNALS: [&str; 37] = [ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { let signal_name_upcase = signal_name_or_value.to_uppercase(); if let Ok(value) = signal_name_upcase.parse() { - if is_signal(value) { - return Some(value); - } else { - return None; - } + return if is_signal(value) { Some(value) } else { None }; } let signal_name = signal_name_upcase.trim_start_matches("SIG");