Add printf, read, break, continue, command, function and type builtins (#280)

* Harden shell: eliminate panics, implement all file test operators, fix path resolution

- Replace panic!() in parser (for loop wordlist) and types (arithmetic) with
  proper error handling/fallback behavior
- Replace unreachable!() in parser (and_or lists, boolean operators), execute
  (assignment operators), and main (script/command dispatch) with descriptive errors
- Implement all 14 previously unimplemented file test operators: -b (block special),
  -c (char special), -g (setgid), -k (sticky bit), -p (named pipe), -s (size > 0),
  -t (terminal fd), -u (setuid), -G (owned by egid), -N (modified since read),
  -O (owned by euid), -S (socket), -R (nameref), and None (implicit -n)
- Add -e (file exists) and -t (terminal fd) to grammar and parser
- Fix file test path resolution to use shell CWD instead of process CWD
- Fix critical unwrap() calls in main.rs (current_dir), execute.rs (parse result),
  uname.rs, touch.rs (DST-ambiguous datetimes), set.rs, and mod.rs (clear command)
- Add comprehensive test suite for file test operators (3 new test functions)

* Add missing bash builtins: break, continue, :, printf, read, command, type

Implement 7 new shell builtins for improved bash compatibility:
- break/continue: Loop control flow via sentinel exit codes (-100/-101)
  that propagate through ExecuteResult::Exit and are caught by while/for loops
- : (colon): No-op command that always returns 0
- printf: Format string support with %s, %d, %o, %x, %f, %c, %b specifiers,
  width/precision/flags, and standard escape sequences
- read: Read input with -r (raw) and -p (prompt) flags, field splitting,
  default REPLY variable
- command: Run commands bypassing aliases, -v flag for path lookup
- type: Describe how a command name resolves (alias, builtin, or PATH)

All builtins include comprehensive tests.

* Add bash builtins: eval, source, shift, local, return, trap

Implement 6 more shell builtins for improved bash compatibility:
- eval: Parse and execute string arguments as shell commands
- source/.: Fix to use context pipes instead of real stdout/stderr,
  enabling proper pipe capture in tests and subshells. Read file,
  parse, and execute in current shell context with env change propagation
- shift: Shift positional parameters ($1, $2, ...) left by N
- local: Declare shell-scoped variables (foundation for future
  function support)
- return: Exit from sourced scripts with specified exit code via
  ExecuteResult::Exit propagation
- trap: Basic signal handling with -l (list signals), -p (print traps),
  and handler registration acceptance for compatibility

All builtins include comprehensive tests. Total: 46 tests passing.

https://claude.ai/code/session_01GTXgqqxHbE7ACysQDZ9cLU

* Implement shell function definitions, positional parameters, and brace groups

- Add function_definition parsing (foo() { ... }) with FunctionDefinition AST node
- Store functions in ShellState and execute via execute_shell_function
- Support positional parameters ($1-$N, $#, $@, $*) with save/restore across calls
- Fix grammar: UNQUOTED_ESCAPE_CHAR now excludes SPECIAL_PARAM so $1/$2/etc parse correctly
- Fix RETURN_EXIT_CODE sentinel handling at top level and in function scope
- Sync last_command_exit_code with shell var "?" for proper $? expansion
- Add brace group compound command support
- Add comprehensive tests for functions, positional params, return values, nested calls

* fix lint / fmt

* Add e2e test-data files for functions and builtins

- functions.sh: tests for function definitions, positional parameters
  ($1-$N, $#, $@), nested calls, variable scoping, and function overriding
- builtins.sh: tests for eval, shift, local, and brace groups

Author: wolfv

Cargo.lock

@@ -30,6 +30,9 @@ pest_ascii_tree = "0.1.0"
 miette = { version = "7.5.0", features = ["fancy"] }
 lazy_static = "1.5.0"
 
+[target.'cfg(unix)'.dependencies]
+libc = "0.2"
+
 [dev-dependencies]
 tempfile = "3.16.0"
 parking_lot = "0.12.3"

crates/deno_task_shell/Cargo.toml

@@ -77,9 +77,9 @@ FILE_NAME_PENDING_WORD = ${
     ))+
 }
 
-UNQUOTED_ESCAPE_CHAR = ${ ("\\" ~ "$" | "$" ~ !"(" ~  !"{" ~ !VARIABLE) | "\\" ~ (" " | "`" | "\"" | "(" | ")") }
-QUOTED_ESCAPE_CHAR = ${ "\\" ~ "$" | "$" ~ !"(" ~ !"{" ~ !(ASCII_DIGIT | VARIABLE) | "\\" ~ ("`" | "\"" | "(" | ")" | "'") }
-PARAMETER_ESCAPE_CHAR = ${ "\\" ~ "$" | "$" ~ !"(" ~ !"{" ~ !VARIABLE | "\\" ~ "}" }
+UNQUOTED_ESCAPE_CHAR = ${ ("\\" ~ "$" | "$" ~ !"(" ~  !"{" ~ !VARIABLE ~ !SPECIAL_PARAM) | "\\" ~ (" " | "`" | "\"" | "(" | ")") }
+QUOTED_ESCAPE_CHAR = ${ "\\" ~ "$" | "$" ~ !"(" ~ !"{" ~ !VARIABLE ~ !SPECIAL_PARAM | "\\" ~ ("`" | "\"" | "(" | ")" | "'") }
+PARAMETER_ESCAPE_CHAR = ${ "\\" ~ "$" | "$" ~ !"(" ~ !"{" ~ !VARIABLE ~ !SPECIAL_PARAM | "\\" ~ "}" }
 
 UNQUOTED_CHAR = ${ ("\\" ~ " ") | !("]]" | "[[" | "(" | ")" | "<" | ">" | "|" | "&" | ";" | "\"" | "'" | "$") ~ ANY }
 QUOTED_CHAR = ${ !"\"" ~ ANY }
@@ -192,8 +192,8 @@ pipe_sequence = !{ command ~ ((StdoutStderr | Stdout) ~ linebreak ~ pipe_sequenc
 
 command = !{
     compound_command ~ redirect_list? |
-    simple_command |
-    function_definition
+    function_definition |
+    simple_command
 }
 
 compound_command = {
@@ -347,8 +347,8 @@ unary_conditional_expression = !{
 }
 
 file_conditional_op = !{
-    "-a" | "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-h" | "-k" | 
-    "-p" | "-r" | "-s" | "-u" | "-w" | "-x" | "-G" | "-L" | 
+    "-a" | "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-h" | "-k" |
+    "-p" | "-r" | "-s" | "-t" | "-u" | "-w" | "-x" | "-G" | "-L" |
     "-N" | "-O" | "-S"
 }
 

crates/deno_task_shell/src/grammar.pest

@@ -170,6 +170,8 @@ pub enum CommandInner {
     Case(CaseClause),
     #[error("Invalid arithmetic expression")]
     ArithmeticExpression(Arithmetic),
+    #[error("Invalid function definition")]
+    Function(FunctionDefinition),
 }
 
 impl From<Command> for Sequence {
@@ -238,6 +240,15 @@ pub struct CaseClause {
     pub cases: Vec<(Vec<Word>, SequentialList)>,
 }
 
+#[cfg_attr(feature = "serialization", derive(serde::Serialize))]
+#[cfg_attr(feature = "serialization", serde(rename_all = "camelCase"))]
+#[derive(Debug, PartialEq, Eq, Clone, Error)]
+#[error("Invalid function definition")]
+pub struct FunctionDefinition {
+    pub name: String,
+    pub body: SequentialList,
+}
+
 #[cfg_attr(feature = "serialization", derive(serde::Serialize))]
 #[cfg_attr(feature = "serialization", serde(rename_all = "camelCase"))]
 #[derive(Debug, PartialEq, Eq, Clone, Error)]
@@ -661,11 +672,16 @@ pub fn parse(input: &str) -> Result<SequentialList> {
         miette::Error::new(e.into_miette()).context("Failed to parse input")
     })?;
 
-    parse_file(pairs.next().unwrap())
+    parse_file(pairs.next().ok_or_else(|| miette!("Empty parse result"))?)
 }
 
 fn parse_file(pairs: Pair<Rule>) -> Result<SequentialList> {
-    parse_complete_command(pairs.into_inner().next().unwrap())
+    parse_complete_command(
+        pairs
+            .into_inner()
+            .next()
+            .ok_or_else(|| miette!("Expected complete command"))?,
+    )
 }
 
 fn parse_complete_command(pair: Pair<Rule>) -> Result<SequentialList> {
@@ -779,11 +795,15 @@ fn parse_term(
 fn parse_and_or(pair: Pair<Rule>) -> Result<Sequence> {
     assert!(pair.as_rule() == Rule::and_or);
     let mut items = pair.into_inner();
-    let first_item = items.next().unwrap();
+    let first_item = items
+        .next()
+        .ok_or_else(|| miette!("Expected item in and_or list"))?;
     let mut current = match first_item.as_rule() {
         Rule::ASSIGNMENT_WORD => parse_shell_var(first_item)?,
         Rule::pipeline => parse_pipeline(first_item)?,
-        _ => unreachable!(),
+        rule => {
+            return Err(miette!("Unexpected rule in and_or list: {:?}", rule))
+        }
     };
 
     match items.next() {
@@ -796,10 +816,17 @@ fn parse_and_or(pair: Pair<Rule>) -> Result<Sequence> {
                 let op = match next_item.as_str() {
                     "&&" => BooleanListOperator::And,
                     "||" => BooleanListOperator::Or,
-                    _ => unreachable!(),
+                    other => {
+                        return Err(miette!(
+                            "Expected '&&' or '||', got '{}'",
+                            other
+                        ))
+                    }
                 };
 
-                let next_item = items.next().unwrap();
+                let next_item = items.next().ok_or_else(|| {
+                    miette!("Expected expression after boolean operator")
+                })?;
                 let next = parse_and_or(next_item)?;
                 current = Sequence::BooleanList(Box::new(BooleanList {
                     current,
@@ -910,9 +937,7 @@ fn parse_command(pair: Pair<Rule>) -> Result<Command> {
     match inner.as_rule() {
         Rule::simple_command => parse_simple_command(inner),
         Rule::compound_command => parse_compound_command(inner),
-        Rule::function_definition => {
-            Err(miette!("Function definitions are not supported yet"))
-        }
+        Rule::function_definition => parse_function_definition(inner),
         _ => Err(miette!("Unexpected rule in command: {:?}", inner.as_rule())),
     }
 }
@@ -1010,7 +1035,7 @@ fn parse_for_loop(pairs: Pair<Rule>) -> Result<ForLoop> {
 
     let wordlist = match inner.next() {
         Some(wordlist_pair) => parse_wordlist(wordlist_pair)?,
-        None => panic!("Expected wordlist in for loop"),
+        None => return Err(miette!("Expected wordlist in for loop")),
     };
 
     let body_pair = inner
@@ -1048,9 +1073,7 @@ fn parse_while_loop(pair: Pair<Rule>, is_until: bool) -> Result<WhileLoop> {
 fn parse_compound_command(pair: Pair<Rule>) -> Result<Command> {
     let inner = pair.into_inner().next().unwrap();
     match inner.as_rule() {
-        Rule::brace_group => {
-            Err(miette!("Unsupported compound command brace_group"))
-        }
+        Rule::brace_group => parse_brace_group(inner),
         Rule::subshell => parse_subshell(inner),
         Rule::for_clause => {
             let for_loop = parse_for_loop(inner);
@@ -1116,6 +1139,65 @@ fn parse_subshell(pair: Pair<Rule>) -> Result<Command> {
     }
 }
 
+fn parse_brace_group(pair: Pair<Rule>) -> Result<Command> {
+    let mut items = Vec::new();
+    for inner in pair.into_inner() {
+        if inner.as_rule() == Rule::compound_list {
+            parse_compound_list(inner, &mut items)?;
+        }
+    }
+    Ok(Command {
+        inner: CommandInner::Subshell(Box::new(SequentialList { items })),
+        redirect: None,
+    })
+}
+
+fn parse_function_definition(pair: Pair<Rule>) -> Result<Command> {
+    let mut inner = pair.into_inner();
+    let fname = inner
+        .next()
+        .ok_or_else(|| miette!("Expected function name"))?;
+    let name = fname.as_str().to_string();
+
+    // Skip linebreak tokens, find function_body
+    let function_body = inner
+        .find(|p| p.as_rule() == Rule::function_body)
+        .ok_or_else(|| miette!("Expected function body"))?;
+
+    // function_body = compound_command ~ redirect_list?
+    let compound_command = function_body
+        .into_inner()
+        .next()
+        .ok_or_else(|| miette!("Expected compound command in function body"))?;
+
+    // Parse the compound command (usually a brace_group)
+    let body_inner = compound_command.into_inner().next().unwrap();
+    let mut items = Vec::new();
+    match body_inner.as_rule() {
+        Rule::brace_group => {
+            for inner in body_inner.into_inner() {
+                if inner.as_rule() == Rule::compound_list {
+                    parse_compound_list(inner, &mut items)?;
+                }
+            }
+        }
+        _ => {
+            return Err(miette!(
+                "Unsupported function body type: {:?}",
+                body_inner.as_rule()
+            ));
+        }
+    }
+
+    Ok(Command {
+        inner: CommandInner::Function(FunctionDefinition {
+            name,
+            body: SequentialList { items },
+        }),
+        redirect: None,
+    })
+}
+
 fn parse_if_clause(pair: Pair<Rule>) -> Result<IfClause> {
     let mut inner = pair.into_inner();
     let condition = inner
@@ -1265,7 +1347,7 @@ fn parse_unary_conditional_expression(pair: Pair<Rule>) -> Result<Condition> {
             }
         },
         Rule::file_conditional_op => match operator.as_str() {
-            "-a" => UnaryOp::FileExists,
+            "-a" | "-e" => UnaryOp::FileExists,
             "-b" => UnaryOp::BlockSpecial,
             "-c" => UnaryOp::CharSpecial,
             "-d" => UnaryOp::Directory,
@@ -1276,6 +1358,7 @@ fn parse_unary_conditional_expression(pair: Pair<Rule>) -> Result<Condition> {
             "-p" => UnaryOp::NamedPipe,
             "-r" => UnaryOp::Readable,
             "-s" => UnaryOp::SizeNonZero,
+            "-t" => UnaryOp::TerminalFd,
             "-u" => UnaryOp::SetUserId,
             "-w" => UnaryOp::Writable,
             "-x" => UnaryOp::Executable,

crates/deno_task_shell/src/parser.rs

@@ -205,6 +205,7 @@ async fn parse_shebang_args(
         CommandInner::While(_) => return err_unsupported(text),
         CommandInner::ArithmeticExpression(_) => return err_unsupported(text),
         CommandInner::Case(_) => return err_unsupported(text),
+        CommandInner::Function(_) => return err_unsupported(text),
     };
     if !cmd.env_vars.is_empty() {
         return err_unsupported(text);

crates/deno_task_shell/src/shell/command.rs

@@ -0,0 +1,52 @@
+// Copyright 2018-2024 the Deno authors. MIT license.
+
+use futures::future::LocalBoxFuture;
+use futures::FutureExt;
+
+use crate::parser;
+use crate::shell::execute::execute_sequential_list;
+use crate::shell::execute::AsyncCommandBehavior;
+
+use super::ShellCommand;
+use super::ShellCommandContext;
+use crate::shell::types::ExecuteResult;
+
+pub struct EvalCommand;
+
+impl ShellCommand for EvalCommand {
+    fn execute(
+        &self,
+        context: ShellCommandContext,
+    ) -> LocalBoxFuture<'static, ExecuteResult> {
+        let input = context.args.join(" ");
+        if input.is_empty() {
+            return Box::pin(futures::future::ready(
+                ExecuteResult::from_exit_code(0),
+            ));
+        }
+
+        async move {
+            let parsed = match parser::parse(&input) {
+                Ok(list) => list,
+                Err(err) => {
+                    let _ = context
+                        .stderr
+                        .clone()
+                        .write_line(&format!("eval: {err}"));
+                    return ExecuteResult::Continue(2, Vec::new(), Vec::new());
+                }
+            };
+
+            execute_sequential_list(
+                parsed,
+                context.state,
+                context.stdin,
+                context.stdout,
+                context.stderr,
+                AsyncCommandBehavior::Wait,
+            )
+            .await
+        }
+        .boxed_local()
+    }
+}

crates/deno_task_shell/src/shell/commands/eval.rs

@@ -0,0 +1,80 @@
+// Copyright 2018-2024 the Deno authors. MIT license.
+
+use futures::future::LocalBoxFuture;
+
+use super::ShellCommand;
+use super::ShellCommandContext;
+use crate::shell::types::EnvChange;
+use crate::shell::types::ExecuteResult;
+
+/// The `local` builtin declares variables with local scope.
+/// Since function support is not yet implemented, this behaves like
+/// setting a shell variable (not exported). This matches bash behavior
+/// where `local` outside a function generates a warning but still works.
+pub struct LocalCommand;
+
+impl ShellCommand for LocalCommand {
+    fn execute(
+        &self,
+        context: ShellCommandContext,
+    ) -> LocalBoxFuture<'static, ExecuteResult> {
+        let mut changes = Vec::new();
+
+        for arg in &context.args {
+            if let Some(equals_index) = arg.find('=') {
+                let name = &arg[..equals_index];
+                let value = &arg[equals_index + 1..];
+
+                if !is_valid_var_name(name) {
+                    let _ = context.stderr.clone().write_line(&format!(
+                        "local: `{name}': not a valid identifier"
+                    ));
+                    return Box::pin(futures::future::ready(
+                        ExecuteResult::Continue(1, Vec::new(), Vec::new()),
+                    ));
+                }
+
+                changes.push(EnvChange::SetShellVar(
+                    name.to_string(),
+                    value.to_string(),
+                ));
+            } else {
+                // `local VAR` without assignment - declare it with empty value
+                if !is_valid_var_name(arg) {
+                    let _ = context.stderr.clone().write_line(&format!(
+                        "local: `{arg}': not a valid identifier"
+                    ));
+                    return Box::pin(futures::future::ready(
+                        ExecuteResult::Continue(1, Vec::new(), Vec::new()),
+                    ));
+                }
+
+                // Only set if not already defined
+                if context.state.get_var(arg).is_none() {
+                    changes.push(EnvChange::SetShellVar(
+                        arg.to_string(),
+                        String::new(),
+                    ));
+                }
+            }
+        }
+
+        Box::pin(futures::future::ready(ExecuteResult::Continue(
+            0,
+            changes,
+            Vec::new(),
+        )))
+    }
+}
+
+fn is_valid_var_name(name: &str) -> bool {
+    if name.is_empty() {
+        return false;
+    }
+    let mut chars = name.chars();
+    let first = chars.next().unwrap();
+    if !first.is_ascii_alphabetic() && first != '_' {
+        return false;
+    }
+    chars.all(|c| c.is_ascii_alphanumeric() || c == '_')
+}