From db47f4d38aaf8a2a6d81281eac89dc3cacb7843f Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Tue, 12 Feb 2019 17:17:36 -0500
Subject: [PATCH] WIP clippy

---
 src/lib/builtins/exists.rs         |   5 +-
 src/lib/parser/shell_expand/mod.rs |  15 +--
 src/lib/shell/assignments.rs       |  18 +--
 src/lib/shell/flow.rs              |   2 +-
 src/lib/shell/pipe_exec/mod.rs     | 200 ++++++++++++++---------------
 src/lib/shell/variables/mod.rs     |   2 +-
 6 files changed, 111 insertions(+), 131 deletions(-)

diff --git a/src/lib/builtins/exists.rs b/src/lib/builtins/exists.rs
index 47db2dd3..5d0ee870 100644
--- a/src/lib/builtins/exists.rs
+++ b/src/lib/builtins/exists.rs
@@ -137,10 +137,7 @@ fn string_var_is_not_empty(stringvar: &str, shell: &Shell) -> bool {
 
 /// Returns true if a function with the given name is defined
 fn function_is_defined(function: &str, shell: &Shell) -> bool {
-    match shell.variables.get::<Function>(function) {
-        Some(_) => true,
-        None => false,
-    }
+    shell.variables.get::<Function>(function).is_some()
 }
 
 #[test]
diff --git a/src/lib/parser/shell_expand/mod.rs b/src/lib/parser/shell_expand/mod.rs
index a130a0f1..54c36e79 100644
--- a/src/lib/parser/shell_expand/mod.rs
+++ b/src/lib/parser/shell_expand/mod.rs
@@ -104,7 +104,7 @@ fn expand_brace<E: Expander>(
 ) {
     let mut temp = Vec::new();
     for word in nodes
-        .into_iter()
+        .iter()
         .flat_map(|node| expand_string_no_glob(node, expand_func, reverse_quoting))
     {
         match parse_range(&word) {
@@ -406,15 +406,12 @@ fn expand_braces<E: Expander>(
         .fold(types::Array::new(), |mut array, word| {
             if word.find('*').is_some() {
                 if let Ok(mut paths) = glob(&word) {
-                    match paths.next() {
-                        Some(path) => {
-                            if let Ok(path_buf) = path {
-                                array.push((*path_buf.to_string_lossy()).into());
-                            } else {
-                                array.push("".into());
-                            }
+                    if let Some(path) = paths.next() {
+                        if let Ok(path_buf) = path {
+                            array.push((*path_buf.to_string_lossy()).into());
+                        } else {
+                            array.push("".into());
                         }
-                        None => {}
                     }
                     for path in paths {
                         if let Ok(path_buf) = path {
diff --git a/src/lib/shell/assignments.rs b/src/lib/shell/assignments.rs
index 03effc7c..7c321f71 100644
--- a/src/lib/shell/assignments.rs
+++ b/src/lib/shell/assignments.rs
@@ -440,13 +440,8 @@ impl VariableStore for Shell {
         for action in actions_step2 {
             match action {
                 Ok(Action::UpdateArray(key, operator, ..)) => {
-                    if operator == Operator::OptionalEqual {
-                        match self.variables.get_ref(key.name) {
-                            Some(_) => {
-                                continue;
-                            }
-                            _ => (),
-                        };
+                    if operator == Operator::OptionalEqual && self.variables.get_ref(key.name).is_some() {
+                            continue;
                     }
                     match collected.remove(key.name) {
                         hmap @ Some(VariableType::HashMap(_)) => {
@@ -533,13 +528,8 @@ impl VariableStore for Shell {
                     }
                 }
                 Ok(Action::UpdateString(key, operator, ..)) => {
-                    if operator == Operator::OptionalEqual {
-                        match self.variables.get_ref(key.name) {
-                            Some(_) => {
-                                continue;
-                            }
-                            _ => (),
-                        };
+                    if operator == Operator::OptionalEqual && self.variables.get_ref(key.name).is_some() {
+                        continue;
                     }
                     match collected.remove(key.name) {
                         str_ @ Some(VariableType::Str(_)) => {
diff --git a/src/lib/shell/flow.rs b/src/lib/shell/flow.rs
index 4b043bae..14261b34 100644
--- a/src/lib/shell/flow.rs
+++ b/src/lib/shell/flow.rs
@@ -572,7 +572,7 @@ fn expand_pipeline(
                             }
                             // Append rest of the pipeline to the last pipeline in the
                             // alias.
-                            while let Some(item) = item_iter.next() {
+                            for item in item_iter {
                                 pline.items.push(item.clone());
                             }
                             // No error
diff --git a/src/lib/shell/pipe_exec/mod.rs b/src/lib/shell/pipe_exec/mod.rs
index 8e3e2ee4..aefab2f9 100644
--- a/src/lib/shell/pipe_exec/mod.rs
+++ b/src/lib/shell/pipe_exec/mod.rs
@@ -740,70 +740,99 @@ pub(crate) fn pipe(
     let mut commands = commands.into_iter().peekable();
     let mut ext_stdio_pipes: Option<Vec<File>> = None;
 
-    loop {
-        if let Some((mut parent, mut kind)) = commands.next() {
-            match kind {
-                JobKind::Pipe(mut mode) => {
-                    // We need to remember the commands as they own the file
-                    // descriptors that are created by sys::pipe.
-                    let remember: SmallVec<[RefinedJob; 16]> = SmallVec::new();
-                    let mut block_child = true;
-                    let (mut pgid, mut last_pid, mut current_pid) = (0, 0, 0);
-
-                    // Append jobs until all piped jobs are running
-                    while let Some((mut child, ckind)) = commands.next() {
-                        // If parent is a RefindJob::External, then we need to keep track of the
-                        // output pipes, so we can properly close them after the job has been
-                        // spawned.
-                        let is_external = if let JobVariant::External { .. } = parent.var {
-                            true
-                        } else {
-                            false
-                        };
-
-                        // TODO: Refactor this part
-                        // If we need to tee both stdout and stderr, we directly connect pipes to
-                        // the relevant sources in both of them.
-                        if let JobVariant::Tee {
-                            items: (Some(ref mut tee_out), Some(ref mut tee_err)),
-                            ..
-                        } = child.var
-                        {
-                            TeePipe::new(&mut parent, &mut ext_stdio_pipes, is_external)
-                                .connect(tee_out, tee_err);
-                        } else {
-                            // Pipe the previous command's stdin to this commands stdout/stderr.
-                            match sys::pipe2(sys::O_CLOEXEC) {
-                                Err(e) => pipe_fail(e),
-                                Ok((reader, writer)) => {
-                                    if is_external {
-                                        append_external_stdio_pipe(&mut ext_stdio_pipes, writer);
-                                    }
-                                    child.stdin(unsafe { File::from_raw_fd(reader) });
-                                    let writer = unsafe { File::from_raw_fd(writer) };
-                                    match mode {
-                                        RedirectFrom::Stderr => parent.stderr(writer),
-                                        RedirectFrom::Stdout => parent.stdout(writer),
-                                        RedirectFrom::Both => match writer.try_clone() {
-                                            Err(e) => {
-                                                eprintln!(
-                                                    "ion: failed to redirect stdout and stderr: {}",
-                                                    e
-                                                );
-                                            }
-                                            Ok(duped) => {
-                                                parent.stderr(writer);
-                                                parent.stdout(duped);
-                                            }
-                                        },
-                                    }
+    while let Some((mut parent, mut kind)) = commands.next() {
+        match kind {
+            JobKind::Pipe(mut mode) => {
+                // We need to remember the commands as they own the file
+                // descriptors that are created by sys::pipe.
+                let remember: SmallVec<[RefinedJob; 16]> = SmallVec::new();
+                let mut block_child = true;
+                let (mut pgid, mut last_pid, mut current_pid) = (0, 0, 0);
+
+                // Append jobs until all piped jobs are running
+                while let Some((mut child, ckind)) = commands.next() {
+                    // If parent is a RefindJob::External, then we need to keep track of the
+                    // output pipes, so we can properly close them after the job has been
+                    // spawned.
+                    let is_external = if let JobVariant::External { .. } = parent.var {
+                        true
+                    } else {
+                        false
+                    };
+
+                    // TODO: Refactor this part
+                    // If we need to tee both stdout and stderr, we directly connect pipes to
+                    // the relevant sources in both of them.
+                    if let JobVariant::Tee {
+                        items: (Some(ref mut tee_out), Some(ref mut tee_err)),
+                        ..
+                    } = child.var
+                    {
+                        TeePipe::new(&mut parent, &mut ext_stdio_pipes, is_external)
+                            .connect(tee_out, tee_err);
+                    } else {
+                        // Pipe the previous command's stdin to this commands stdout/stderr.
+                        match sys::pipe2(sys::O_CLOEXEC) {
+                            Err(e) => pipe_fail(e),
+                            Ok((reader, writer)) => {
+                                if is_external {
+                                    append_external_stdio_pipe(&mut ext_stdio_pipes, writer);
+                                }
+                                child.stdin(unsafe { File::from_raw_fd(reader) });
+                                let writer = unsafe { File::from_raw_fd(writer) };
+                                match mode {
+                                    RedirectFrom::Stderr => parent.stderr(writer),
+                                    RedirectFrom::Stdout => parent.stdout(writer),
+                                    RedirectFrom::Both => match writer.try_clone() {
+                                        Err(e) => {
+                                            eprintln!(
+                                                "ion: failed to redirect stdout and stderr: {}",
+                                                e
+                                            );
+                                        }
+                                        Ok(duped) => {
+                                            parent.stderr(writer);
+                                            parent.stdout(duped);
+                                        }
+                                    },
                                 }
                             }
                         }
+                    }
+
+                    match spawn_proc(
+                        shell,
+                        parent,
+                        kind,
+                        block_child,
+                        &mut last_pid,
+                        &mut current_pid,
+                        pgid,
+                    ) {
+                        SUCCESS => (),
+                        error_code => return error_code,
+                    }
+
+                    ext_stdio_pipes = None;
+
+                    if set_process_group(&mut pgid, current_pid)
+                        && foreground
+                        && !shell.is_library
+                    {
+                        let _ = sys::tcsetpgrp(0, pgid);
+                    }
 
+                    resume_prior_process(&mut last_pid, current_pid);
+
+                    if let JobKind::Pipe(m) = ckind {
+                        parent = child;
+                        mode = m;
+                    } else {
+                        kind = ckind;
+                        block_child = false;
                         match spawn_proc(
                             shell,
-                            parent,
+                            child,
                             kind,
                             block_child,
                             &mut last_pid,
@@ -814,57 +843,24 @@ pub(crate) fn pipe(
                             error_code => return error_code,
                         }
 
-                        ext_stdio_pipes = None;
-
-                        if set_process_group(&mut pgid, current_pid)
-                            && foreground
-                            && !shell.is_library
-                        {
-                            let _ = sys::tcsetpgrp(0, pgid);
-                        }
-
                         resume_prior_process(&mut last_pid, current_pid);
-
-                        if let JobKind::Pipe(m) = ckind {
-                            parent = child;
-                            mode = m;
-                        } else {
-                            kind = ckind;
-                            block_child = false;
-                            match spawn_proc(
-                                shell,
-                                child,
-                                kind,
-                                block_child,
-                                &mut last_pid,
-                                &mut current_pid,
-                                pgid,
-                            ) {
-                                SUCCESS => (),
-                                error_code => return error_code,
-                            }
-
-                            resume_prior_process(&mut last_pid, current_pid);
-                            break;
-                        }
+                        break;
                     }
+                }
 
-                    set_process_group(&mut pgid, current_pid);
+                set_process_group(&mut pgid, current_pid);
 
-                    previous_status = shell.wait(pgid, remember);
-                    if previous_status == TERMINATED {
-                        if let Err(why) = sys::killpg(pgid, sys::SIGTERM) {
-                            eprintln!("ion: failed to terminate foreground jobs: {}", why);
-                        }
-                        return previous_status;
+                previous_status = shell.wait(pgid, remember);
+                if previous_status == TERMINATED {
+                    if let Err(why) = sys::killpg(pgid, sys::SIGTERM) {
+                        eprintln!("ion: failed to terminate foreground jobs: {}", why);
                     }
-                }
-                _ => {
-                    previous_status = shell.exec_job(&mut parent, foreground);
+                    return previous_status;
                 }
             }
-        } else {
-            break;
+            _ => {
+                previous_status = shell.exec_job(&mut parent, foreground);
+            }
         }
     }
 
diff --git a/src/lib/shell/variables/mod.rs b/src/lib/shell/variables/mod.rs
index fa66eaed..8b7c125a 100644
--- a/src/lib/shell/variables/mod.rs
+++ b/src/lib/shell/variables/mod.rs
@@ -139,7 +139,7 @@ impl fmt::Display for VariableType {
             }
             VariableType::BTreeMap(ref map) => {
                 let mut format =
-                    map.into_iter()
+                    map.iter()
                         .fold(String::new(), |mut format, (_, var_type)| {
                             format.push_str(&format!("{}", var_type));
                             format.push(' ');
-- 
GitLab