fix: address clippy warnings for CI build

- Add #[allow(clippy::too_many_lines)] to write_command
- Use is_some_and instead of map().unwrap_or(false)
- Merge identical match arms for redirect types
- Pass ListOp by value instead of reference
- Use &Command instead of &Box<Command> in write_if
- Fix uninlined format args in tests
- Make get_redirects const fn
- Merge match arms in tests

Author: cv

src/to_bash.rs

@@ -29,6 +29,7 @@ pub fn to_bash(cmd: &Command) -> String {
 }
 
 /// Write a command to the output string
+#[allow(clippy::too_many_lines)]
 fn write_command(cmd: &Command, out: &mut String) {
     match cmd {
         Command::Simple {
@@ -47,7 +48,7 @@ fn write_command(cmd: &Command, out: &mut String) {
         Command::List {
             op, left, right, ..
         } => {
-            write_list(op, left, right, out);
+            write_list(*op, left, right, out);
         }
         Command::For {
             variable,
@@ -81,7 +82,13 @@ fn write_command(cmd: &Command, out: &mut String) {
             redirects,
             ..
         } => {
-            write_if(condition, then_branch, else_branch.as_ref(), redirects, out);
+            write_if(
+                condition,
+                then_branch,
+                else_branch.as_deref(),
+                redirects,
+                out,
+            );
         }
         Command::Case {
             word,
@@ -143,15 +150,12 @@ fn write_simple(
 ) {
     // Check if the command is a builtin that takes assignments as arguments
     // (like local, export, declare, readonly, typeset)
-    let is_assignment_builtin = words
-        .first()
-        .map(|w| {
-            matches!(
-                w.word.as_str(),
-                "local" | "export" | "declare" | "readonly" | "typeset"
-            )
-        })
-        .unwrap_or(false);
+    let is_assignment_builtin = words.first().is_some_and(|w| {
+        matches!(
+            w.word.as_str(),
+            "local" | "export" | "declare" | "readonly" | "typeset"
+        )
+    });
 
     if is_assignment_builtin {
         // For assignment builtins: cmd assignments...
@@ -279,12 +283,10 @@ fn write_redirect(redirect: &Redirect, out: &mut String) {
         RedirectType::HereString => out.push_str("<<<"),
         RedirectType::InputOutput => out.push_str("<>"),
         RedirectType::Clobber => out.push_str(">|"),
-        RedirectType::DupInput => out.push_str("<&"),
-        RedirectType::DupOutput => out.push_str(">&"),
+        RedirectType::DupInput | RedirectType::MoveInput => out.push_str("<&"),
+        RedirectType::DupOutput | RedirectType::MoveOutput => out.push_str(">&"),
         RedirectType::ErrAndOut => out.push_str("&>"),
         RedirectType::AppendErrAndOut => out.push_str("&>>"),
-        RedirectType::MoveInput => out.push_str("<&"),
-        RedirectType::MoveOutput => out.push_str(">&"),
         RedirectType::HereDoc | RedirectType::Close => unreachable!(), // Handled above
     }
 
@@ -320,7 +322,7 @@ fn write_pipeline(commands: &[Command], negated: bool, out: &mut String) {
 }
 
 /// Write a list (cmd1 && cmd2, cmd1 || cmd2, etc.)
-fn write_list(op: &ListOp, left: &Command, right: &Command, out: &mut String) {
+fn write_list(op: ListOp, left: &Command, right: &Command, out: &mut String) {
     write_command(left, out);
 
     match op {
@@ -381,7 +383,7 @@ fn write_until(test: &Command, body: &Command, redirects: &[Redirect], out: &mut
 fn write_if(
     condition: &Command,
     then_branch: &Command,
-    else_branch: Option<&Box<Command>>,
+    else_branch: Option<&Command>,
     redirects: &[Redirect],
     out: &mut String,
 ) {
@@ -397,7 +399,7 @@ fn write_if(
             then_branch: elif_then,
             else_branch: elif_else,
             ..
-        } = else_cmd.as_ref()
+        } = else_cmd
         {
             out.push_str("; elif ");
             write_command(elif_cond, out);
@@ -619,15 +621,15 @@ mod tests {
         init();
     }
 
-    /// Helper to test round-trip: parse -> to_bash -> parse -> compare structure
+    /// Helper to test round-trip: parse -> `to_bash` -> parse -> compare structure
     fn assert_round_trip(script: &str) {
         setup();
-        let ast1 = parse(script).expect(&format!("Failed to parse original: {}", script));
+        let ast1 =
+            parse(script).unwrap_or_else(|e| panic!("Failed to parse original: {script}: {e}"));
         let regenerated = to_bash(&ast1);
-        let ast2 = parse(&regenerated).expect(&format!(
-            "Failed to parse regenerated script: {}\nOriginal: {}",
-            regenerated, script
-        ));
+        let ast2 = parse(&regenerated).unwrap_or_else(|e| {
+            panic!("Failed to parse regenerated script: {regenerated}\nOriginal: {script}: {e}")
+        });
 
         // Compare JSON representations (ignoring line numbers)
         let json1 = serde_json::to_string(&ast1).unwrap();
@@ -639,8 +641,7 @@ mod tests {
 
         assert_eq!(
             json1_no_lines, json2_no_lines,
-            "AST mismatch!\nOriginal: {}\nRegenerated: {}\nAST1: {}\nAST2: {}",
-            script, regenerated, json1, json2
+            "AST mismatch!\nOriginal: {script}\nRegenerated: {regenerated}\nAST1: {json1}\nAST2: {json2}"
         );
     }
 
@@ -651,8 +652,8 @@ mod tests {
         let mut chars = json.chars().peekable();
 
         while let Some(c) = chars.next() {
+            result.push(c);
             if c == '"' {
-                result.push(c);
                 // Read the key
                 let mut key = String::new();
                 while let Some(&nc) = chars.peek() {
@@ -682,8 +683,6 @@ mod tests {
                     let line_len = "\"line\"".len();
                     result.truncate(result.len() - line_len);
                 }
-            } else {
-                result.push(c);
             }
         }
 

tests/compound_redirects.rs

@@ -11,21 +11,21 @@ fn setup() {
 
 fn parse_ok(script: &str) -> Command {
     setup();
-    parse(script).unwrap_or_else(|e| panic!("Failed to parse {:?}: {}", script, e))
+    parse(script).unwrap_or_else(|e| panic!("Failed to parse {script:?}: {e}"))
 }
 
 /// Helper to check if a command has redirects
-fn get_redirects(cmd: &Command) -> Option<&Vec<Redirect>> {
+const fn get_redirects(cmd: &Command) -> Option<&Vec<Redirect>> {
     match cmd {
-        Command::While { redirects, .. } => Some(redirects),
-        Command::Until { redirects, .. } => Some(redirects),
-        Command::For { redirects, .. } => Some(redirects),
-        Command::If { redirects, .. } => Some(redirects),
-        Command::Case { redirects, .. } => Some(redirects),
-        Command::Select { redirects, .. } => Some(redirects),
-        Command::Group { redirects, .. } => Some(redirects),
-        Command::Subshell { redirects, .. } => Some(redirects),
-        Command::Simple { redirects, .. } => Some(redirects),
+        Command::While { redirects, .. }
+        | Command::Until { redirects, .. }
+        | Command::For { redirects, .. }
+        | Command::If { redirects, .. }
+        | Command::Case { redirects, .. }
+        | Command::Select { redirects, .. }
+        | Command::Group { redirects, .. }
+        | Command::Subshell { redirects, .. }
+        | Command::Simple { redirects, .. } => Some(redirects),
         _ => None,
     }
 }
@@ -34,12 +34,7 @@ fn get_redirects(cmd: &Command) -> Option<&Vec<Redirect>> {
 fn test_while_with_input_redirect() {
     let cmd = parse_ok("while read line; do echo $line; done < input.txt");
     let redirects = get_redirects(&cmd).expect("While should have redirects field");
-    assert_eq!(
-        redirects.len(),
-        1,
-        "Expected 1 redirect, got {:?}",
-        redirects
-    );
+    assert_eq!(redirects.len(), 1, "Expected 1 redirect, got {redirects:?}");
 }
 
 #[test]
@@ -107,8 +102,7 @@ fn test_while_redirect_roundtrip() {
     let regenerated = to_bash(&ast);
     assert!(
         regenerated.contains("< input.txt") || regenerated.contains("<input.txt"),
-        "Regenerated script should contain redirect: {}",
-        regenerated
+        "Regenerated script should contain redirect: {regenerated}"
     );
 }
 
@@ -120,8 +114,7 @@ fn test_for_redirect_roundtrip() {
     let regenerated = to_bash(&ast);
     assert!(
         regenerated.contains("> output.txt") || regenerated.contains(">output.txt"),
-        "Regenerated script should contain redirect: {}",
-        regenerated
+        "Regenerated script should contain redirect: {regenerated}"
     );
 }
 
@@ -133,8 +126,7 @@ fn test_group_redirect_roundtrip() {
     let regenerated = to_bash(&ast);
     assert!(
         regenerated.contains("> file.txt") || regenerated.contains(">file.txt"),
-        "Regenerated script should contain redirect: {}",
-        regenerated
+        "Regenerated script should contain redirect: {regenerated}"
     );
 }
 
@@ -178,8 +170,7 @@ fn test_negated_roundtrip() {
     let regenerated = to_bash(&ast);
     assert!(
         regenerated.starts_with("! "),
-        "Regenerated should start with '! ': {}",
-        regenerated
+        "Regenerated should start with '! ': {regenerated}"
     );
 }
 
@@ -191,7 +182,6 @@ fn test_negated_pipeline_roundtrip() {
     let regenerated = to_bash(&ast);
     assert!(
         regenerated.starts_with("! "),
-        "Regenerated should start with '! ': {}",
-        regenerated
+        "Regenerated should start with '! ': {regenerated}"
     );
 }