Merge pull request #4647 from kamilogorek/if-not-else-lint

lint: Enable clippy::if_not_else and fix all lint issues

.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",
 ]
 

src/uu/base32/src/base_common.rs

@@ -160,18 +160,7 @@ pub fn handle_input<R: Read>(
         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<R: Read>(
             }
             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)",
+            )),
+        }
     }
 }

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));
                             }
                         }
                     }

src/uu/chown/src/chown.rs

@@ -198,7 +198,9 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option<u32>, Option<u32>)> {
     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<u32>, Option<u32>)> {
                 }
             }
         })
-    } 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<u32>, Option<u32>)> {
                 }
             },
         })
-    } else {
-        None
     };
 
     if user.chars().next().map(char::is_numeric).unwrap_or(false)

src/uu/date/src/date.rs

@@ -414,10 +414,10 @@ fn set_system_datetime(date: DateTime<Utc>) -> UResult<()> {
 
     let result = unsafe { clock_settime(CLOCK_REALTIME, &timespec) };
 
-    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()))
     }
 }
 

src/uu/dd/src/dd.rs

@@ -265,10 +265,10 @@ fn make_linux_iflags(iflags: &IFlags) -> Option<libc::c_int> {
         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<libc::c_int> {
         flag |= libc::O_SYNC;
     }
 
-    if flag != 0 {
-        Some(flag)
-    } else {
+    if flag == 0 {
         None
+    } else {
+        Some(flag)
     }
 }
 

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<String>) -> Option<Self> {
-        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()?);

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(())

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(())

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

src/uu/factor/src/numeric/montgomery.rs

@@ -82,10 +82,10 @@ impl<T: DoubleInt> Montgomery<T> {
         // (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<T: DoubleInt> Arithmetic for Montgomery<T> {
         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[

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,

src/uu/hostname/src/hostname.rs

@@ -41,10 +41,10 @@ mod wsa {
             let mut data = std::mem::MaybeUninit::<WSADATA>::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))
         }
     }
 

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.

src/uu/install/src/install.rs

@@ -399,13 +399,13 @@ fn behavior(matches: &ArgMatches) -> UResult<Behavior> {
         .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<Behavior> {
         .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 {

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;

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

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}'"));
             }
         }
 

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(),
+                    ));
                 }
             }
         };

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));
             }
         }