From 68e90eacbbf1479fed903171c0d93a6bbde017fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Sun, 26 Mar 2023 20:25:38 +0200 Subject: [PATCH 1/2] lint: Fix all issues in preparation for enabling clippy::if_not_else --- src/uu/base32/src/base_common.rs | 24 ++-- src/uu/chmod/src/chmod.rs | 12 +- src/uu/chown/src/chown.rs | 12 +- src/uu/date/src/date.rs | 6 +- src/uu/dd/src/dd.rs | 12 +- src/uu/df/src/filesystem.rs | 6 +- src/uu/dirname/src/dirname.rs | 6 +- src/uu/env/src/env.rs | 8 +- src/uu/expr/src/syntax_tree.rs | 8 +- src/uu/factor/src/numeric/montgomery.rs | 12 +- src/uu/fmt/src/parasplit.rs | 6 +- src/uu/hostname/src/hostname.rs | 6 +- src/uu/id/src/id.rs | 6 +- src/uu/install/src/install.rs | 12 +- src/uu/ls/src/ls.rs | 14 +- src/uu/numfmt/src/format.rs | 2 +- src/uu/numfmt/src/options.rs | 12 +- src/uu/od/src/od.rs | 12 +- src/uu/pinky/src/pinky.rs | 18 +-- src/uu/rm/src/rm.rs | 6 +- src/uu/sort/src/merge.rs | 6 +- src/uu/sort/src/numeric_str_cmp.rs | 12 +- src/uu/sort/src/sort.rs | 32 ++--- src/uu/split/src/number.rs | 6 +- src/uu/stat/src/stat.rs | 128 +++++++++--------- src/uu/tail/src/chunks.rs | 8 +- src/uu/tail/src/tail.rs | 16 +-- src/uu/tee/src/tee.rs | 6 +- src/uu/users/src/users.rs | 6 +- src/uu/who/src/who.rs | 16 +-- src/uucore/src/lib/features/encoding.rs | 6 +- src/uucore/src/lib/features/entries.rs | 6 +- src/uucore/src/lib/features/fs.rs | 2 + src/uucore/src/lib/features/fsext.rs | 12 +- src/uucore/src/lib/features/perms.rs | 6 +- src/uucore/src/lib/features/process.rs | 12 +- src/uucore/src/lib/features/tokenize/sub.rs | 54 ++++---- .../lib/features/tokenize/unescaped_text.rs | 6 +- src/uucore/src/lib/features/utmpx.rs | 6 +- src/uucore/src/lib/mods/quoting_style.rs | 6 +- src/uucore/src/lib/parser/parse_size.rs | 6 +- 41 files changed, 278 insertions(+), 280 deletions(-) diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index f68aa6b86..78698e8b9 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -160,18 +160,7 @@ pub fn handle_input( data = data.line_wrap(wrap); } - if !decode { - match data.encode() { - Ok(s) => { - wrap_print(&data, &s); - Ok(()) - } - Err(_) => Err(USimpleError::new( - 1, - "error: invalid input (length must be multiple of 4 characters)", - )), - } - } else { + if decode { match data.decode() { Ok(s) => { // Silent the warning as we want to the error message @@ -184,5 +173,16 @@ pub fn handle_input( } Err(_) => Err(USimpleError::new(1, "error: invalid input")), } + } else { + match data.encode() { + Ok(s) => { + wrap_print(&data, &s); + Ok(()) + } + Err(_) => Err(USimpleError::new( + 1, + "error: invalid input (length must be multiple of 4 characters)", + )), + } } } diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 14d761cf9..a79886037 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -274,10 +274,10 @@ impl Chmoder { ) )); } - if !self.recursive { - r = self.chmod_file(file).and(r); - } else { + if self.recursive { r = self.walk_dir(file); + } else { + r = self.chmod_file(file).and(r); } } r @@ -360,10 +360,10 @@ impl Chmoder { naively_expected_new_mode = naive_mode; } Err(f) => { - if !self.quiet { - return Err(USimpleError::new(1, f)); - } else { + if self.quiet { return Err(ExitCode::new(1)); + } else { + return Err(USimpleError::new(1, f)); } } } diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 3faecd571..2d810564e 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -198,7 +198,9 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { let user = args.next().unwrap_or(""); let group = args.next().unwrap_or(""); - let uid = if !user.is_empty() { + let uid = if user.is_empty() { + None + } else { Some(match Passwd::locate(user) { Ok(u) => u.uid, // We have been able to get the uid Err(_) => @@ -225,10 +227,10 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { } } }) - } else { - None }; - let gid = if !group.is_empty() { + let gid = if group.is_empty() { + None + } else { Some(match Group::locate(group) { Ok(g) => g.gid, Err(_) => match group.parse() { @@ -241,8 +243,6 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { } }, }) - } else { - None }; if user.chars().next().map(char::is_numeric).unwrap_or(false) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 8ff6283c5..667eafe81 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -414,10 +414,10 @@ fn set_system_datetime(date: DateTime) -> UResult<()> { let result = unsafe { clock_settime(CLOCK_REALTIME, ×pec) }; - if result != 0 { - Err(std::io::Error::last_os_error().map_err_context(|| "cannot set date".to_string())) - } else { + if result == 0 { Ok(()) + } else { + Err(std::io::Error::last_os_error().map_err_context(|| "cannot set date".to_string())) } } diff --git a/src/uu/dd/src/dd.rs b/src/uu/dd/src/dd.rs index 00be1558c..6f75f4707 100644 --- a/src/uu/dd/src/dd.rs +++ b/src/uu/dd/src/dd.rs @@ -265,10 +265,10 @@ fn make_linux_iflags(iflags: &IFlags) -> Option { flag |= libc::O_SYNC; } - if flag != 0 { - Some(flag) - } else { + if flag == 0 { None + } else { + Some(flag) } } @@ -784,10 +784,10 @@ fn make_linux_oflags(oflags: &OFlags) -> Option { flag |= libc::O_SYNC; } - if flag != 0 { - Some(flag) - } else { + if flag == 0 { None + } else { + Some(flag) } } diff --git a/src/uu/df/src/filesystem.rs b/src/uu/df/src/filesystem.rs index aac63ac9d..461815e67 100644 --- a/src/uu/df/src/filesystem.rs +++ b/src/uu/df/src/filesystem.rs @@ -83,9 +83,7 @@ where impl Filesystem { // TODO: resolve uuid in `mount_info.dev_name` if exists pub(crate) fn new(mount_info: MountInfo, file: Option) -> Option { - let _stat_path = if !mount_info.mount_dir.is_empty() { - mount_info.mount_dir.clone() - } else { + let _stat_path = if mount_info.mount_dir.is_empty() { #[cfg(unix)] { mount_info.dev_name.clone() @@ -95,6 +93,8 @@ impl Filesystem { // On windows, we expect the volume id mount_info.dev_id.clone() } + } else { + mount_info.mount_dir.clone() }; #[cfg(unix)] let usage = FsUsage::new(statfs(_stat_path).ok()?); diff --git a/src/uu/dirname/src/dirname.rs b/src/uu/dirname/src/dirname.rs index 687aaa550..cecd6aec8 100644 --- a/src/uu/dirname/src/dirname.rs +++ b/src/uu/dirname/src/dirname.rs @@ -38,7 +38,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|s| s.to_owned()) .collect(); - if !dirnames.is_empty() { + if dirnames.is_empty() { + return Err(UUsageError::new(1, "missing operand")); + } else { for path in &dirnames { let p = Path::new(path); match p.parent() { @@ -59,8 +61,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } print!("{separator}"); } - } else { - return Err(UUsageError::new(1, "missing operand")); } Ok(()) diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index a29ee3b0f..0ef191b8d 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -299,7 +299,10 @@ fn run_env(args: impl uucore::Args) -> UResult<()> { env::set_var(name, val); } - if !opts.program.is_empty() { + if opts.program.is_empty() { + // no program provided, so just dump all env vars to stdout + print_env(opts.null); + } else { // we need to execute a command let (prog, args) = build_command(&mut opts.program); @@ -344,9 +347,6 @@ fn run_env(args: impl uucore::Args) -> UResult<()> { Err(_) => return Err(126.into()), Ok(_) => (), } - } else { - // no program provided, so just dump all env vars to stdout - print_env(opts.null); } Ok(()) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 936760f27..1d2ddbced 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -185,14 +185,14 @@ pub fn tokens_to_ast( maybe_dump_rpn(&out_stack); let result = ast_from_rpn(&mut out_stack); - if !out_stack.is_empty() { + if out_stack.is_empty() { + maybe_dump_ast(&result); + result + } else { Err( "syntax error (first RPN token does not represent the root of the expression AST)" .to_owned(), ) - } else { - maybe_dump_ast(&result); - result } }) } diff --git a/src/uu/factor/src/numeric/montgomery.rs b/src/uu/factor/src/numeric/montgomery.rs index dd4523022..d411d0d41 100644 --- a/src/uu/factor/src/numeric/montgomery.rs +++ b/src/uu/factor/src/numeric/montgomery.rs @@ -82,10 +82,10 @@ impl Montgomery { // (x + n*m) / R // in case of overflow, this is (2¹²⁸ + xnm)/2⁶⁴ - n = xnm/2⁶⁴ + (2⁶⁴ - n) let y = T::from_double_width(xnm >> t_bits) - + if !overflow { - T::zero() - } else { + + if overflow { n.wrapping_neg() + } else { + T::zero() }; if y >= *n { @@ -132,10 +132,10 @@ impl Arithmetic for Montgomery { let (r, overflow) = a.overflowing_add(&b); // In case of overflow, a+b = 2⁶⁴ + r = (2⁶⁴ - n) + r (working mod n) - let r = if !overflow { - r - } else { + let r = if overflow { r + self.n.wrapping_neg() + } else { + r }; // Normalize to [0; n[ diff --git a/src/uu/fmt/src/parasplit.rs b/src/uu/fmt/src/parasplit.rs index c60be0a47..d2e3d292d 100644 --- a/src/uu/fmt/src/parasplit.rs +++ b/src/uu/fmt/src/parasplit.rs @@ -580,11 +580,11 @@ impl<'a> Iterator for WordSplit<'a> { // points to whitespace character OR end of string let mut word_nchars = 0; self.position = match self.string[word_start..].find(|x: char| { - if !x.is_whitespace() { + if x.is_whitespace() { + true + } else { word_nchars += char_width(x); false - } else { - true } }) { None => self.length, diff --git a/src/uu/hostname/src/hostname.rs b/src/uu/hostname/src/hostname.rs index 0e959b471..83a22a82f 100644 --- a/src/uu/hostname/src/hostname.rs +++ b/src/uu/hostname/src/hostname.rs @@ -41,10 +41,10 @@ mod wsa { let mut data = std::mem::MaybeUninit::::uninit(); WSAStartup(0x0202, data.as_mut_ptr()) }; - if err != 0 { - Err(io::Error::from_raw_os_error(err)) - } else { + if err == 0 { Ok(WsaHandle(())) + } else { + Err(io::Error::from_raw_os_error(err)) } } diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 4a80099ad..7a8e40059 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -203,9 +203,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } for i in 0..=users.len() { - let possible_pw = if !state.user_specified { - None - } else { + let possible_pw = if state.user_specified { match Passwd::locate(users[i].as_str()) { Ok(p) => Some(p), Err(_) => { @@ -218,6 +216,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } } + } else { + None }; // GNU's `id` does not support the flags: -p/-P/-A. diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 98fdb47d6..039aa1b15 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -399,13 +399,13 @@ fn behavior(matches: &ArgMatches) -> UResult { .unwrap_or("") .to_string(); - let owner_id = if !owner.is_empty() { + let owner_id = if owner.is_empty() { + None + } else { match usr2uid(&owner) { Ok(u) => Some(u), Err(_) => return Err(InstallError::InvalidUser(owner.clone()).into()), } - } else { - None }; let group = matches @@ -414,13 +414,13 @@ fn behavior(matches: &ArgMatches) -> UResult { .unwrap_or("") .to_string(); - let group_id = if !group.is_empty() { + let group_id = if group.is_empty() { + None + } else { match grp2gid(&group) { Ok(g) => Some(g), Err(_) => return Err(InstallError::InvalidGroup(group.clone()).into()), } - } else { - None }; Ok(Behavior { diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index c5d811834..309ae1a76 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -2008,16 +2008,16 @@ fn enter_directory( continue; } Ok(rd) => { - if !listed_ancestors + if listed_ancestors .insert(FileInformation::from_path(&e.p_buf, e.must_dereference)?) { - out.flush()?; - show!(LsError::AlreadyListedError(e.p_buf.clone())); - } else { writeln!(out, "\n{}:", e.p_buf.display())?; enter_directory(e, rd, config, out, listed_ancestors)?; listed_ancestors .remove(&FileInformation::from_path(&e.p_buf, e.must_dereference)?); + } else { + out.flush()?; + show!(LsError::AlreadyListedError(e.p_buf.clone())); } } } @@ -2867,10 +2867,10 @@ fn display_file_name( // to get correct alignment from later calls to`display_grid()`. if config.context { if let Some(pad_count) = prefix_context { - let security_context = if !matches!(config.format, Format::Commas) { - pad_left(&path.security_context, pad_count) - } else { + let security_context = if matches!(config.format, Format::Commas) { path.security_context.to_owned() + } else { + pad_left(&path.security_context, pad_count) }; name = format!("{security_context} {name}"); width += security_context.len() + 1; diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index 3e20d00ee..eb75f7554 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -55,7 +55,7 @@ impl<'a> Iterator for WhitespaceSplitter<'a> { .unwrap_or(field.len()), ); - self.s = if !rest.is_empty() { Some(rest) } else { None }; + self.s = if rest.is_empty() { None } else { Some(rest) }; Some((prefix, field)) } diff --git a/src/uu/numfmt/src/options.rs b/src/uu/numfmt/src/options.rs index 79d7461ae..131ad378b 100644 --- a/src/uu/numfmt/src/options.rs +++ b/src/uu/numfmt/src/options.rs @@ -190,14 +190,12 @@ impl FromStr for FormatOptions { } } - if !precision.is_empty() { - if let Ok(p) = precision.parse() { - options.precision = Some(p); - } else { - return Err(format!("invalid precision in format '{s}'")); - } - } else { + if precision.is_empty() { options.precision = Some(0); + } else if let Ok(p) = precision.parse() { + options.precision = Some(p); + } else { + return Err(format!("invalid precision in format '{s}'")); } } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 5e8f8814d..09765ed2b 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -171,12 +171,7 @@ impl OdOptions { None => Radix::Octal, Some(s) => { let st = s.as_bytes(); - if st.len() != 1 { - return Err(USimpleError::new( - 1, - "Radix must be one of [d, o, n, x]".to_string(), - )); - } else { + if st.len() == 1 { let radix: char = *(st .first() .expect("byte string of length 1 lacks a 0th elem")) @@ -193,6 +188,11 @@ impl OdOptions { )) } } + } else { + return Err(USimpleError::new( + 1, + "Radix must be one of [d, o, n, x]".to_string(), + )); } } }; diff --git a/src/uu/pinky/src/pinky.rs b/src/uu/pinky/src/pinky.rs index 7e21fba9e..18f72638b 100644 --- a/src/uu/pinky/src/pinky.rs +++ b/src/uu/pinky/src/pinky.rs @@ -218,10 +218,10 @@ impl Capitalize for str { fn capitalize(&self) -> String { self.char_indices() .fold(String::with_capacity(self.len()), |mut acc, x| { - if x.0 != 0 { - acc.push(x.1); - } else { + if x.0 == 0 { acc.push(x.1.to_ascii_uppercase()); + } else { + acc.push(x.1); } acc }) @@ -281,10 +281,10 @@ impl Pinky { match pts_path.metadata() { #[allow(clippy::unnecessary_cast)] Ok(meta) => { - mesg = if meta.mode() & S_IWGRP as u32 != 0 { - ' ' - } else { + mesg = if meta.mode() & S_IWGRP as u32 == 0 { '*' + } else { + ' ' }; last_change = meta.atime(); } @@ -312,10 +312,10 @@ impl Pinky { print!(" {}{:<8.*}", mesg, utmpx::UT_LINESIZE, ut.tty_device()); if self.include_idle { - if last_change != 0 { - print!(" {:<6}", idle_string(last_change)); - } else { + if last_change == 0 { print!(" {:<6}", "?????"); + } else { + print!(" {:<6}", idle_string(last_change)); } } diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 05d03cee2..02aa96e24 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -257,14 +257,14 @@ fn remove(files: &[String], options: &Options) -> bool { // TODO: When the error is not about missing files // (e.g., permission), even rm -f should fail with // outputting the error, but there's no easy eay. - if !options.force { + if options.force { + false + } else { show_error!( "cannot remove {}: No such file or directory", filename.quote() ); true - } else { - false } } } diff --git a/src/uu/sort/src/merge.rs b/src/uu/sort/src/merge.rs index a7b4417e3..f6da0ee32 100644 --- a/src/uu/sort/src/merge.rs +++ b/src/uu/sort/src/merge.rs @@ -362,16 +362,16 @@ impl<'a> Compare for FileComparator<'a> { // Wait for the child to exit and check its exit code. fn check_child_success(mut child: Child, program: &str) -> UResult<()> { - if !matches!( + if matches!( child.wait().map(|e| e.code()), Ok(Some(0)) | Ok(None) | Err(_) ) { + Ok(()) + } else { Err(SortError::CompressProgTerminatedAbnormally { prog: program.to_owned(), } .into()) - } else { - Ok(()) } } diff --git a/src/uu/sort/src/numeric_str_cmp.rs b/src/uu/sort/src/numeric_str_cmp.rs index 70f8c2cd8..21d733877 100644 --- a/src/uu/sort/src/numeric_str_cmp.rs +++ b/src/uu/sort/src/numeric_str_cmp.rs @@ -198,15 +198,13 @@ pub fn human_numeric_str_cmp( let a_unit = get_unit(a.chars().next_back()); let b_unit = get_unit(b.chars().next_back()); let ordering = a_unit.cmp(&b_unit); - if ordering != Ordering::Equal { - if a_info.sign == Sign::Negative { - ordering.reverse() - } else { - ordering - } - } else { + if ordering == Ordering::Equal { // 3. Number numeric_str_cmp((a, a_info), (b, b_info)) + } else if a_info.sign == Sign::Negative { + ordering.reverse() + } else { + ordering } } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index a681026f6..5b309e413 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -185,7 +185,9 @@ impl Display for SortError { line, silent, } => { - if !silent { + if *silent { + Ok(()) + } else { write!( f, "{}:{}: disorder: {}", @@ -193,8 +195,6 @@ impl Display for SortError { line_number, line ) - } else { - Ok(()) } } Self::OpenFailed { path, error } => { @@ -582,7 +582,15 @@ impl<'a> Line<'a> { selection.start += num_range.start; selection.end = selection.start + num_range.len(); - if num_range != (0..0) { + if num_range == (0..0) { + // This was not a valid number. + // Report no match at the first non-whitespace character. + let leading_whitespace = self.line[selection.clone()] + .find(|c: char| !c.is_whitespace()) + .unwrap_or(0); + selection.start += leading_whitespace; + selection.end += leading_whitespace; + } else { // include a trailing si unit if selector.settings.mode == SortMode::HumanNumeric && self.line[selection.end..initial_selection.end] @@ -597,14 +605,6 @@ impl<'a> Line<'a> { { selection.start -= 1; } - } else { - // This was not a valid number. - // Report no match at the first non-whitespace character. - let leading_whitespace = self.line[selection.clone()] - .find(|c: char| !c.is_whitespace()) - .unwrap_or(0); - selection.start += leading_whitespace; - selection.end += leading_whitespace; } } SortMode::GeneralNumeric => { @@ -1572,10 +1572,7 @@ fn compare_by<'a>( let mut num_info_index = 0; let mut parsed_float_index = 0; for selector in &global_settings.selectors { - let (a_str, b_str) = if !selector.needs_selection { - // We can select the whole line. - (a.line, b.line) - } else { + let (a_str, b_str) = if selector.needs_selection { let selections = ( a_line_data.selections [a.index * global_settings.precomputed.selections_per_line + selection_index], @@ -1584,6 +1581,9 @@ fn compare_by<'a>( ); selection_index += 1; selections + } else { + // We can select the whole line. + (a.line, b.line) }; let settings = &selector.settings; diff --git a/src/uu/split/src/number.rs b/src/uu/split/src/number.rs index 778e24f7c..567526538 100644 --- a/src/uu/split/src/number.rs +++ b/src/uu/split/src/number.rs @@ -200,10 +200,10 @@ impl FixedWidthNumber { break; } } - if suffix_start != 0 { - Err(Overflow) - } else { + if suffix_start == 0 { Ok(Self { radix, digits }) + } else { + Err(Overflow) } } diff --git a/src/uu/stat/src/stat.rs b/src/uu/stat/src/stat.rs index fbbed8c21..586803db4 100644 --- a/src/uu/stat/src/stat.rs +++ b/src/uu/stat/src/stat.rs @@ -375,9 +375,7 @@ impl Stater { }); } '\\' => { - if !use_printf { - tokens.push(Token::Char('\\')); - } else { + if use_printf { i += 1; if i >= bound { show_warning!("backslash at end of format"); @@ -414,6 +412,8 @@ impl Stater { tokens.push(Token::Char(c)); } } + } else { + tokens.push(Token::Char('\\')); } } @@ -519,7 +519,67 @@ impl Stater { OsString::from(file) }; - if !self.show_fs { + if self.show_fs { + #[cfg(unix)] + let p = file.as_bytes(); + #[cfg(not(unix))] + let p = file.into_string().unwrap(); + match statfs(p) { + Ok(meta) => { + let tokens = &self.default_tokens; + + for t in tokens.iter() { + match *t { + Token::Char(c) => print!("{c}"), + Token::Directive { + flag, + width, + precision, + format, + } => { + let output = match format { + // free blocks available to non-superuser + 'a' => OutputType::Unsigned(meta.avail_blocks()), + // total data blocks in file system + 'b' => OutputType::Unsigned(meta.total_blocks()), + // total file nodes in file system + 'c' => OutputType::Unsigned(meta.total_file_nodes()), + // free file nodes in file system + 'd' => OutputType::Unsigned(meta.free_file_nodes()), + // free blocks in file system + 'f' => OutputType::Unsigned(meta.free_blocks()), + // file system ID in hex + 'i' => OutputType::UnsignedHex(meta.fsid()), + // maximum length of filenames + 'l' => OutputType::Unsigned(meta.namelen()), + // file name + 'n' => OutputType::Str(display_name.to_string()), + // block size (for faster transfers) + 's' => OutputType::Unsigned(meta.io_size()), + // fundamental block size (for block counts) + 'S' => OutputType::Integer(meta.block_size()), + // file system type in hex + 't' => OutputType::UnsignedHex(meta.fs_type() as u64), + // file system type in human readable form + 'T' => OutputType::Str(pretty_fstype(meta.fs_type()).into()), + _ => OutputType::Unknown, + }; + + print_it(&output, flag, width, precision); + } + } + } + } + Err(e) => { + show_error!( + "cannot read file system information for {}: {}", + display_name.quote(), + e + ); + return 1; + } + } + } else { let result = if self.follow || stdin_is_fifo && display_name == "-" { fs::metadata(&file) } else { @@ -660,66 +720,6 @@ impl Stater { return 1; } } - } else { - #[cfg(unix)] - let p = file.as_bytes(); - #[cfg(not(unix))] - let p = file.into_string().unwrap(); - match statfs(p) { - Ok(meta) => { - let tokens = &self.default_tokens; - - for t in tokens.iter() { - match *t { - Token::Char(c) => print!("{c}"), - Token::Directive { - flag, - width, - precision, - format, - } => { - let output = match format { - // free blocks available to non-superuser - 'a' => OutputType::Unsigned(meta.avail_blocks()), - // total data blocks in file system - 'b' => OutputType::Unsigned(meta.total_blocks()), - // total file nodes in file system - 'c' => OutputType::Unsigned(meta.total_file_nodes()), - // free file nodes in file system - 'd' => OutputType::Unsigned(meta.free_file_nodes()), - // free blocks in file system - 'f' => OutputType::Unsigned(meta.free_blocks()), - // file system ID in hex - 'i' => OutputType::UnsignedHex(meta.fsid()), - // maximum length of filenames - 'l' => OutputType::Unsigned(meta.namelen()), - // file name - 'n' => OutputType::Str(display_name.to_string()), - // block size (for faster transfers) - 's' => OutputType::Unsigned(meta.io_size()), - // fundamental block size (for block counts) - 'S' => OutputType::Integer(meta.block_size()), - // file system type in hex - 't' => OutputType::UnsignedHex(meta.fs_type() as u64), - // file system type in human readable form - 'T' => OutputType::Str(pretty_fstype(meta.fs_type()).into()), - _ => OutputType::Unknown, - }; - - print_it(&output, flag, width, precision); - } - } - } - } - Err(e) => { - show_error!( - "cannot read file system information for {}: {}", - display_name.quote(), - e - ); - return 1; - } - } } 0 } diff --git a/src/uu/tail/src/chunks.rs b/src/uu/tail/src/chunks.rs index 4b934c1d7..3aa380e20 100644 --- a/src/uu/tail/src/chunks.rs +++ b/src/uu/tail/src/chunks.rs @@ -579,16 +579,16 @@ impl LinesChunkBuffer { } } - if !&self.chunks.is_empty() { + if self.chunks.is_empty() { + // chunks is empty when a file is empty so quitting early here + return Ok(()); + } else { let length = &self.chunks.len(); let last = &mut self.chunks[length - 1]; if !last.get_buffer().ends_with(&[self.delimiter]) { last.lines += 1; self.lines += 1; } - } else { - // chunks is empty when a file is empty so quitting early here - return Ok(()); } // skip unnecessary chunks and save the first chunk which may hold some lines we have to diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index 9e3273638..e07616c6f 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -125,10 +125,10 @@ fn tail_file( show_error!("error reading '{}': {}", input.display_name, err_msg); if settings.follow.is_some() { - let msg = if !settings.retry { - "; giving up on this name" - } else { + let msg = if settings.retry { "" + } else { + "; giving up on this name" }; show_error!( "{}: cannot follow end of this type of file{}", @@ -215,11 +215,7 @@ fn tail_stdin( // pipe None => { header_printer.print_input(input); - if !paths::stdin_is_bad_fd() { - let mut reader = BufReader::new(stdin()); - unbounded_tail(&mut reader, settings)?; - observer.add_stdin(input.display_name.as_str(), Some(Box::new(reader)), true)?; - } else { + if paths::stdin_is_bad_fd() { set_exit_code(1); show_error!( "cannot fstat {}: {}", @@ -233,6 +229,10 @@ fn tail_stdin( text::BAD_FD ); } + } else { + let mut reader = BufReader::new(stdin()); + unbounded_tail(&mut reader, settings)?; + observer.add_stdin(input.display_name.as_str(), Some(Box::new(reader)), true)?; } } }; diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index f8e6ebe41..5c388dd0e 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -261,11 +261,11 @@ fn process_error( Err(f) } Some(OutputErrorMode::ExitNoPipe) => { - if f.kind() != ErrorKind::BrokenPipe { + if f.kind() == ErrorKind::BrokenPipe { + Ok(()) + } else { show_error!("{}: {}", writer.name.maybe_quote(), f); Err(f) - } else { - Ok(()) } } } diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index 05d656576..6a5e54f99 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -41,10 +41,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(AsRef::as_ref).collect()) .unwrap_or_default(); - let filename = if !files.is_empty() { - files[0] - } else { + let filename = if files.is_empty() { utmpx::DEFAULT_FILE.as_ref() + } else { + files[0] }; let mut users = Utmpx::iter_all_records_from(filename) diff --git a/src/uu/who/src/who.rs b/src/uu/who/src/who.rs index 26248b12b..fbfff80d7 100644 --- a/src/uu/who/src/who.rs +++ b/src/uu/who/src/who.rs @@ -316,13 +316,13 @@ fn time_string(ut: &Utmpx) -> String { fn current_tty() -> String { unsafe { let res = ttyname(STDIN_FILENO); - if !res.is_null() { + if res.is_null() { + String::new() + } else { CStr::from_ptr(res as *const _) .to_string_lossy() .trim_start_matches("/dev/") .to_owned() - } else { - String::new() } } } @@ -402,7 +402,7 @@ impl Who { &time_string(ut), "", "", - if !last.is_control() { &comment } else { "" }, + if last.is_control() { "" } else { &comment }, "", ); } @@ -482,7 +482,7 @@ impl Who { let iwgrp = S_IWGRP; #[cfg(any(target_os = "android", target_os = "freebsd", target_vendor = "apple"))] let iwgrp = S_IWGRP as u32; - mesg = if meta.mode() & iwgrp != 0 { '+' } else { '-' }; + mesg = if meta.mode() & iwgrp == 0 { '-' } else { '+' }; last_change = meta.atime(); } _ => { @@ -491,10 +491,10 @@ impl Who { } } - let idle = if last_change != 0 { - idle_string(last_change, 0) - } else { + let idle = if last_change == 0 { " ?".into() + } else { + idle_string(last_change, 0) }; let s = if self.do_lookup { diff --git a/src/uucore/src/lib/features/encoding.rs b/src/uucore/src/lib/features/encoding.rs index db4c4c635..a42044eea 100644 --- a/src/uucore/src/lib/features/encoding.rs +++ b/src/uucore/src/lib/features/encoding.rs @@ -67,10 +67,10 @@ pub fn encode(f: Format, input: &[u8]) -> Result { Z85 => { // According to the spec we should not accept inputs whose len is not a multiple of 4. // However, the z85 crate implements a padded encoding and accepts such inputs. We have to manually check for them. - if input.len() % 4 != 0 { - return Err(EncodeError::Z85InputLenNotMultipleOf4); - } else { + if input.len() % 4 == 0 { z85::encode(input) + } else { + return Err(EncodeError::Z85InputLenNotMultipleOf4); } } }) diff --git a/src/uucore/src/lib/features/entries.rs b/src/uucore/src/lib/features/entries.rs index 561de888b..1e8f2417a 100644 --- a/src/uucore/src/lib/features/entries.rs +++ b/src/uucore/src/lib/features/entries.rs @@ -168,10 +168,10 @@ pub struct Passwd { /// SAFETY: ptr must point to a valid C string. /// Returns None if ptr is null. unsafe fn cstr2string(ptr: *const c_char) -> Option { - if !ptr.is_null() { - Some(CStr::from_ptr(ptr).to_string_lossy().into_owned()) - } else { + if ptr.is_null() { None + } else { + Some(CStr::from_ptr(ptr).to_string_lossy().into_owned()) } } diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 43c21aa8d..b82eba94a 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -455,6 +455,8 @@ pub fn display_permissions(metadata: &fs::Metadata, display_file_type: bool) -> display_permissions_unix(mode, display_file_type) } +// The logic below is more readable written this way. +#[allow(clippy::if_not_else)] #[cfg(unix)] /// Display the permissions of a file on a unix like system pub fn display_permissions_unix(mode: mode_t, display_file_type: bool) -> String { diff --git a/src/uucore/src/lib/features/fsext.rs b/src/uucore/src/lib/features/fsext.rs index 927ae0f7a..e7d5d66bf 100644 --- a/src/uucore/src/lib/features/fsext.rs +++ b/src/uucore/src/lib/features/fsext.rs @@ -295,10 +295,10 @@ impl MountInfo { fs_type_buf.len() as u32, ) }; - let fs_type = if 0 != success { - Some(LPWSTR2String(&fs_type_buf)) - } else { + let fs_type = if 0 == success { None + } else { + Some(LPWSTR2String(&fs_type_buf)) }; let mut mn_info = Self { dev_id: volume_name, @@ -862,10 +862,10 @@ pub fn pretty_time(sec: i64, nsec: i64) -> String { pub fn pretty_filetype<'a>(mode: mode_t, size: u64) -> &'a str { match mode & S_IFMT { S_IFREG => { - if size != 0 { - "regular file" - } else { + if size == 0 { "regular empty file" + } else { + "regular file" } } S_IFDIR => "directory", diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 07203a318..110313642 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -263,10 +263,10 @@ impl ChownExecutor { 0 }; - if !self.recursive { - ret - } else { + if self.recursive { ret | self.dive_into(&root) + } else { + ret } } diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index b8b2f178f..4a52f0fc4 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -58,10 +58,10 @@ pub trait ChildExt { impl ChildExt for Child { fn send_signal(&mut self, signal: usize) -> io::Result<()> { - if unsafe { libc::kill(self.id() as pid_t, signal as i32) } != 0 { - Err(io::Error::last_os_error()) - } else { + if unsafe { libc::kill(self.id() as pid_t, signal as i32) } == 0 { Ok(()) + } else { + Err(io::Error::last_os_error()) } } @@ -70,10 +70,10 @@ impl ChildExt for Child { if unsafe { libc::signal(signal as i32, libc::SIG_IGN) } != 0 { return Err(io::Error::last_os_error()); } - if unsafe { libc::kill(0, signal as i32) } != 0 { - Err(io::Error::last_os_error()) - } else { + if unsafe { libc::kill(0, signal as i32) } == 0 { Ok(()) + } else { + Err(io::Error::last_os_error()) } } diff --git a/src/uucore/src/lib/features/tokenize/sub.rs b/src/uucore/src/lib/features/tokenize/sub.rs index 9312040e3..af13fb83f 100644 --- a/src/uucore/src/lib/features/tokenize/sub.rs +++ b/src/uucore/src/lib/features/tokenize/sub.rs @@ -197,30 +197,7 @@ impl SubParser { self.text_so_far.push(ch); match ch { '-' | '*' | '0'..='9' => { - if !self.past_decimal { - if self.min_width_is_asterisk || self.specifiers_found { - return Err(SubError::InvalidSpec(self.text_so_far.clone()).into()); - } - if self.min_width_tmp.is_none() { - self.min_width_tmp = Some(String::new()); - } - match self.min_width_tmp.as_mut() { - Some(x) => { - if (ch == '-' || ch == '*') && !x.is_empty() { - return Err( - SubError::InvalidSpec(self.text_so_far.clone()).into() - ); - } - if ch == '*' { - self.min_width_is_asterisk = true; - } - x.push(ch); - } - None => { - panic!("should be unreachable"); - } - } - } else { + if self.past_decimal { // second field should never have a // negative value if self.second_field_is_asterisk || ch == '-' || self.specifiers_found { @@ -245,13 +222,36 @@ impl SubParser { panic!("should be unreachable"); } } + } else { + if self.min_width_is_asterisk || self.specifiers_found { + return Err(SubError::InvalidSpec(self.text_so_far.clone()).into()); + } + if self.min_width_tmp.is_none() { + self.min_width_tmp = Some(String::new()); + } + match self.min_width_tmp.as_mut() { + Some(x) => { + if (ch == '-' || ch == '*') && !x.is_empty() { + return Err( + SubError::InvalidSpec(self.text_so_far.clone()).into() + ); + } + if ch == '*' { + self.min_width_is_asterisk = true; + } + x.push(ch); + } + None => { + panic!("should be unreachable"); + } + } } } '.' => { - if !self.past_decimal { - self.past_decimal = true; - } else { + if self.past_decimal { return Err(SubError::InvalidSpec(self.text_so_far.clone()).into()); + } else { + self.past_decimal = true; } } x if legal_fields.binary_search(&x).is_ok() => { diff --git a/src/uucore/src/lib/features/tokenize/unescaped_text.rs b/src/uucore/src/lib/features/tokenize/unescaped_text.rs index e659b11b5..b4f632901 100644 --- a/src/uucore/src/lib/features/tokenize/unescaped_text.rs +++ b/src/uucore/src/lib/features/tokenize/unescaped_text.rs @@ -135,13 +135,13 @@ impl UnescapedText { } _ => {} } - if !ignore { + if ignore { + byte_vec.push(ch as u8); + } else { let val = (Self::base_to_u32(min_len, max_len, base, it) % 256) as u8; byte_vec.push(val); let bvec = [val]; flush_bytes(writer, &bvec); - } else { - byte_vec.push(ch as u8); } } e => { diff --git a/src/uucore/src/lib/features/utmpx.rs b/src/uucore/src/lib/features/utmpx.rs index 599a02778..8ac41014b 100644 --- a/src/uucore/src/lib/features/utmpx.rs +++ b/src/uucore/src/lib/features/utmpx.rs @@ -336,7 +336,9 @@ impl Iterator for UtmpxIter { fn next(&mut self) -> Option { unsafe { let res = getutxent(); - if !res.is_null() { + if res.is_null() { + None + } else { // The data behind this pointer will be replaced by the next // call to getutxent(), so we have to read it now. // All the strings live inline in the struct as arrays, which @@ -344,8 +346,6 @@ impl Iterator for UtmpxIter { Some(Utmpx { inner: ptr::read(res as *const _), }) - } else { - None } } } diff --git a/src/uucore/src/lib/mods/quoting_style.rs b/src/uucore/src/lib/mods/quoting_style.rs index 3c8bf686a..a6efb2898 100644 --- a/src/uucore/src/lib/mods/quoting_style.rs +++ b/src/uucore/src/lib/mods/quoting_style.rs @@ -259,13 +259,13 @@ fn shell_with_escape(name: &str, quotes: Quotes) -> (String, bool) { pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String { match style { QuotingStyle::Literal { show_control } => { - if !show_control { + if *show_control { + name.to_string_lossy().into_owned() + } else { name.to_string_lossy() .chars() .flat_map(|c| EscapedChar::new_literal(c).hide_control()) .collect() - } else { - name.to_string_lossy().into_owned() } } QuotingStyle::C { quotes } => { diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index d1e571e06..60209d849 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -70,13 +70,13 @@ impl<'parser> Parser<'parser> { // Get the numeric part of the size argument. For example, if the // argument is "123K", then the numeric part is "123". let numeric_string: String = size.chars().take_while(|c| c.is_ascii_digit()).collect(); - let number: u64 = if !numeric_string.is_empty() { + let number: u64 = if numeric_string.is_empty() { + 1 + } else { match numeric_string.parse() { Ok(n) => n, Err(_) => return Err(ParseSizeError::parse_failure(size)), } - } else { - 1 }; // Get the alphabetic units part of the size argument and compute From b056e29b06e5f8934794bd6b328a627688734587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Sun, 26 Mar 2023 21:34:43 +0200 Subject: [PATCH 2/2] lint: Enable clippy::if_not_else rule --- .cargo/config | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.cargo/config b/.cargo/config index 7724c1b54..a4dc4b423 100644 --- a/.cargo/config +++ b/.cargo/config @@ -3,10 +3,11 @@ linker = "x86_64-unknown-redox-gcc" [target.'cfg(feature = "cargo-clippy")'] rustflags = [ - "-Wclippy::use_self", - "-Wclippy::needless_pass_by_value", - "-Wclippy::semicolon_if_nothing_returned", - "-Wclippy::single_char_pattern", - "-Wclippy::explicit_iter_loop", + "-Wclippy::use_self", + "-Wclippy::needless_pass_by_value", + "-Wclippy::semicolon_if_nothing_returned", + "-Wclippy::single_char_pattern", + "-Wclippy::explicit_iter_loop", + "-Wclippy::if_not_else", ]