PR 155 - arithmetic expressions

crates/deno_task_shell/src/grammar.pest

@@ -248,27 +248,33 @@ bitwise_or = { "|" }
 logical_and = { "&&" }
 logical_or = { "||" }
 
+unary_plus = { "+" }
+unary_minus = { "-" }
+logical_not = { "!" }
+bitwise_not = { "~" }
+increment = { "++" }
+decrement = { "--" }
+
 unary_arithmetic_expr = !{
-    (unary_arithmetic_op | post_arithmetic_op) ~ (parentheses_expr | VARIABLE | NUMBER) |
-    (parentheses_expr | VARIABLE | NUMBER) ~ post_arithmetic_op
+    unary_pre_arithmetic_expr | unary_post_arithmetic_expr
 }
 
-unary_arithmetic_op = _{
-    unary_plus | unary_minus | logical_not | bitwise_not
+unary_pre_arithmetic_expr = !{
+    pre_arithmetic_op ~ (parentheses_expr | VARIABLE | NUMBER)
 }
 
-unary_plus = { "+" }
-unary_minus = { "-" }
-logical_not = { "!" }
-bitwise_not = { "~" }
+unary_post_arithmetic_expr = !{
+    (parentheses_expr | VARIABLE | NUMBER) ~ post_arithmetic_op
+}
+
+pre_arithmetic_op= !{
+    increment | decrement | unary_plus | unary_minus | logical_not | bitwise_not
+}
 
 post_arithmetic_op = !{
     increment | decrement
 }
 
-increment = { "++" }
-decrement = { "--" }
-
 assignment_operator = _{
     assign | multiply_assign | divide_assign | modulo_assign | add_assign | subtract_assign |
     left_shift_assign | right_shift_assign | bitwise_and_assign | bitwise_xor_assign | bitwise_or_assign

crates/deno_task_shell/src/parser.rs

@@ -484,11 +484,6 @@ pub enum ArithmeticPart {
         operator: UnaryArithmeticOp,
         operand: Box<ArithmeticPart>,
     },
-    #[error("Invalid post arithmetic expression")]
-    PostArithmeticExpr {
-        operand: Box<ArithmeticPart>,
-        operator: PostArithmeticOp,
-    },
     #[error("Invalid variable")]
     Variable(String),
     #[error("Invalid number")]
@@ -534,7 +529,9 @@ pub enum AssignmentOp {
 #[cfg_attr(feature = "serialization", derive(serde::Serialize))]
 #[cfg_attr(feature = "serialization", serde(rename_all = "camelCase"))]
 #[derive(Debug, Clone, PartialEq, Eq, Copy)]
-pub enum UnaryArithmeticOp {
+pub enum PreArithmeticOp {
+    Increment,  // ++
+    Decrement,  // --
     Plus,       // +
     Minus,      // -
     LogicalNot, // !
@@ -543,12 +540,20 @@ pub enum UnaryArithmeticOp {
 
 #[cfg_attr(feature = "serialization", derive(serde::Serialize))]
 #[cfg_attr(feature = "serialization", serde(rename_all = "camelCase"))]
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Copy)]
 pub enum PostArithmeticOp {
     Increment, // ++
     Decrement, // --
 }
 
+#[cfg_attr(feature = "serialization", derive(serde::Serialize))]
+#[cfg_attr(feature = "serialization", serde(rename_all = "camelCase"))]
+#[derive(Debug, Clone, PartialEq, Eq, Copy)]
+pub enum UnaryArithmeticOp {
+    Pre(PreArithmeticOp),
+    Post(PostArithmeticOp),
+}
+
 #[cfg_attr(feature = "serialization", derive(serde::Serialize))]
 #[cfg_attr(
     feature = "serialization",
@@ -1708,56 +1713,151 @@ fn parse_arithmetic_expr(pair: Pair<Rule>) -> Result<ArithmeticPart> {
 
 fn parse_unary_arithmetic_expr(pair: Pair<Rule>) -> Result<ArithmeticPart> {
     let mut inner = pair.into_inner();
-    let first = inner.next().unwrap();
+    let first = inner
+        .next()
+        .ok_or_else(|| miette!("Expected unary operator"))?;
+
+    match first.as_rule() {
+        Rule::unary_pre_arithmetic_expr => unary_pre_arithmetic_expr(first),
+        Rule::unary_post_arithmetic_expr => unary_post_arithmetic_expr(first),
+        _ => Err(miette!(
+            "Unexpected rule in unary arithmetic expression: {:?}",
+            first.as_rule()
+        )),
+    }
+}
+
+fn unary_pre_arithmetic_expr(pair: Pair<Rule>) -> Result<ArithmeticPart> {
+    let mut inner = pair.into_inner();
+    let first = inner
+        .next()
+        .ok_or_else(|| miette!("Expected unary pre operator"))?;
+    let second = inner.next().ok_or_else(|| miette!("Expected operand"))?;
+    let operand = match second.as_rule() {
+        Rule::parentheses_expr => {
+            let inner = second
+                .into_inner()
+                .next()
+                .ok_or_else(|| miette!("Expected expression in parentheses"))?;
+            let parts = parse_arithmetic_sequence(inner)?;
+            Ok(ArithmeticPart::ParenthesesExpr(Box::new(Arithmetic {
+                parts,
+            })))
+        }
+        Rule::VARIABLE => {
+            Ok(ArithmeticPart::Variable(second.as_str().to_string()))
+        }
+        Rule::NUMBER => Ok(ArithmeticPart::Number(second.as_str().to_string())),
+        _ => Err(miette!(
+            "Unexpected rule in arithmetic expression: {:?}",
+            second.as_rule()
+        )),
+    }?;
 
     match first.as_rule() {
-        Rule::unary_arithmetic_op => {
-            let op = parse_unary_arithmetic_op(first)?;
-            let operand = parse_arithmetic_expr(inner.next().unwrap())?;
+        Rule::pre_arithmetic_op => {
+            let op = parse_pre_arithmetic_op(first)?;
+            // Validate that increment/decrement are only applied to variables or expressions
+            if matches!(
+                op,
+                PreArithmeticOp::Increment | PreArithmeticOp::Decrement
+            ) && matches!(operand, ArithmeticPart::Number(_))
+            {
+                return Err(miette!(
+            "Increment/decrement operators cannot be applied to literal numbers"
+          ));
+            }
             Ok(ArithmeticPart::UnaryArithmeticExpr {
-                operator: op,
+                operator: UnaryArithmeticOp::Pre(op),
                 operand: Box::new(operand),
             })
         }
         Rule::post_arithmetic_op => {
-            let operand = parse_arithmetic_expr(inner.next().unwrap())?;
             let op = parse_post_arithmetic_op(first)?;
-            Ok(ArithmeticPart::PostArithmeticExpr {
+            Ok(ArithmeticPart::UnaryArithmeticExpr {
+                operator: UnaryArithmeticOp::Post(op),
                 operand: Box::new(operand),
-                operator: op,
             })
         }
-        _ => {
-            let operand = parse_arithmetic_expr(first)?;
-            let op = parse_post_arithmetic_op(inner.next().unwrap())?;
-            Ok(ArithmeticPart::PostArithmeticExpr {
-                operand: Box::new(operand),
-                operator: op,
-            })
+        _ => Err(miette!(
+            "Unexpected rule in unary arithmetic operator: {:?}",
+            first.as_rule()
+        )),
+    }
+}
+
+fn unary_post_arithmetic_expr(pair: Pair<Rule>) -> Result<ArithmeticPart> {
+    let mut inner = pair.into_inner();
+    let first = inner
+        .next()
+        .ok_or_else(|| miette!("Expected unary post operator"))?;
+    let second = inner.next().ok_or_else(|| miette!("Expected operand"))?;
+
+    let operand = match first.as_rule() {
+        Rule::parentheses_expr => {
+            let inner = first
+                .into_inner()
+                .next()
+                .ok_or_else(|| miette!("Expected expression in parentheses"))?;
+            let parts = parse_arithmetic_sequence(inner)?;
+            Ok(ArithmeticPart::ParenthesesExpr(Box::new(Arithmetic {
+                parts,
+            })))
+        }
+        Rule::VARIABLE => {
+            Ok(ArithmeticPart::Variable(first.as_str().to_string()))
         }
+        Rule::NUMBER => Ok(ArithmeticPart::Number(first.as_str().to_string())),
+        _ => Err(miette!(
+            "Unexpected rule in arithmetic expression: {:?}",
+            first.as_rule()
+        )),
+    }?;
+
+    // Validate that increment/decrement are only applied to variables or expressions
+    if matches!(operand, ArithmeticPart::Number(_)) {
+        return Err(miette!(
+      "Increment/decrement operators cannot be applied to literal numbers"
+    ));
     }
+
+    let op = parse_post_arithmetic_op(second)?;
+    Ok(ArithmeticPart::UnaryArithmeticExpr {
+        operator: UnaryArithmeticOp::Post(op),
+        operand: Box::new(operand),
+    })
 }
 
-fn parse_unary_arithmetic_op(pair: Pair<Rule>) -> Result<UnaryArithmeticOp> {
-    match pair.as_str() {
-        "+" => Ok(UnaryArithmeticOp::Plus),
-        "-" => Ok(UnaryArithmeticOp::Minus),
-        "!" => Ok(UnaryArithmeticOp::LogicalNot),
-        "~" => Ok(UnaryArithmeticOp::BitwiseNot),
+fn parse_pre_arithmetic_op(pair: Pair<Rule>) -> Result<PreArithmeticOp> {
+    let first = pair
+        .into_inner()
+        .next()
+        .ok_or_else(|| miette!("Expected increment or decrement operator"))?;
+    match first.as_rule() {
+        Rule::increment => Ok(PreArithmeticOp::Increment),
+        Rule::decrement => Ok(PreArithmeticOp::Decrement),
+        Rule::unary_plus => Ok(PreArithmeticOp::Plus),
+        Rule::unary_minus => Ok(PreArithmeticOp::Minus),
+        Rule::logical_not => Ok(PreArithmeticOp::LogicalNot),
+        Rule::bitwise_not => Ok(PreArithmeticOp::BitwiseNot),
         _ => Err(miette!(
-            "Invalid unary arithmetic operator: {}",
-            pair.as_str()
+            "Unexpected rule in pre arithmetic operator: {:?}",
+            first.as_rule()
         )),
     }
 }
 
 fn parse_post_arithmetic_op(pair: Pair<Rule>) -> Result<PostArithmeticOp> {
-    match pair.as_str() {
-        "++" => Ok(PostArithmeticOp::Increment),
-        "--" => Ok(PostArithmeticOp::Decrement),
+    let first = pair
+        .into_inner()
+        .next()
+        .ok_or_else(|| miette!("Expected increment or decrement operator"))?;
+    match first.as_rule() {
+        Rule::increment => Ok(PostArithmeticOp::Increment),
+        Rule::decrement => Ok(PostArithmeticOp::Decrement),
         _ => Err(miette!(
-            "Invalid post arithmetic operator: {}",
-            pair.as_str()
+            "Unexpected rule in post arithmetic operator: {:?}",
+            first.as_rule()
         )),
     }
 }

crates/deno_task_shell/src/shell/execute.rs

@@ -15,55 +15,21 @@ use thiserror::Error;
 use tokio::task::JoinHandle;
 use tokio_util::sync::CancellationToken;
 
-use crate::parser::AssignmentOp;
-use crate::parser::BinaryOp;
-use crate::parser::CaseClause;
-use crate::parser::Condition;
-use crate::parser::ConditionInner;
-use crate::parser::ElsePart;
-use crate::parser::ForLoop;
-use crate::parser::IoFile;
-use crate::parser::RedirectOpInput;
-use crate::parser::RedirectOpOutput;
-use crate::parser::UnaryOp;
-use crate::parser::VariableModifier;
-use crate::parser::WhileLoop;
-use crate::shell::commands::ShellCommand;
-use crate::shell::commands::ShellCommandContext;
-use crate::shell::types::pipe;
-use crate::shell::types::ArithmeticResult;
-use crate::shell::types::ArithmeticValue;
-use crate::shell::types::EnvChange;
-use crate::shell::types::ExecuteResult;
-use crate::shell::types::FutureExecuteResult;
-use crate::shell::types::ShellPipeReader;
-use crate::shell::types::ShellPipeWriter;
-use crate::shell::types::ShellState;
+use crate::parser::{
+    Arithmetic, ArithmeticPart, AssignmentOp, BinaryArithmeticOp, BinaryOp,
+    CaseClause, Command, CommandInner, Condition, ConditionInner, ElsePart,
+    ForLoop, IfClause, IoFile, PipeSequence, PipeSequenceOperator, Pipeline,
+    PipelineInner, Redirect, RedirectFd, RedirectOp, RedirectOpInput,
+    RedirectOpOutput, Sequence, SequentialList, SimpleCommand,
+    UnaryArithmeticOp, UnaryOp, VariableModifier, WhileLoop, Word, WordPart,
+};
+use crate::shell::commands::{ShellCommand, ShellCommandContext};
 use crate::shell::types::TextPart::Text as OtherText;
-
-use crate::parser::Arithmetic;
-use crate::parser::ArithmeticPart;
-use crate::parser::BinaryArithmeticOp;
-use crate::parser::Command;
-use crate::parser::CommandInner;
-use crate::parser::IfClause;
-use crate::parser::PipeSequence;
-use crate::parser::PipeSequenceOperator;
-use crate::parser::Pipeline;
-use crate::parser::PipelineInner;
-use crate::parser::Redirect;
-use crate::parser::RedirectFd;
-use crate::parser::RedirectOp;
-use crate::parser::Sequence;
-use crate::parser::SequentialList;
-use crate::parser::SimpleCommand;
-use crate::parser::UnaryArithmeticOp;
-use crate::parser::Word;
-use crate::parser::WordPart;
-use crate::shell::types::Text;
-use crate::shell::types::TextPart;
-use crate::shell::types::WordPartsResult;
-use crate::shell::types::WordResult;
+use crate::shell::types::{
+    pipe, ArithmeticResult, ArithmeticValue, EnvChange, ExecuteResult,
+    FutureExecuteResult, ShellPipeReader, ShellPipeWriter, ShellState, Text,
+    TextPart, WordPartsResult, WordResult,
+};
 
 use super::command::execute_unresolved_command_name;
 use super::command::UnresolvedCommandName;
@@ -1027,12 +993,7 @@ async fn evaluate_arithmetic_part(
         ArithmeticPart::UnaryArithmeticExpr { operator, operand } => {
             let val =
                 Box::pin(evaluate_arithmetic_part(operand, state)).await?;
-            apply_unary_op(*operator, val)
-        }
-        ArithmeticPart::PostArithmeticExpr { operand, .. } => {
-            let val =
-                Box::pin(evaluate_arithmetic_part(operand, state)).await?;
-            Ok(val)
+            apply_unary_op(state, *operator, val, operand)
         }
         ArithmeticPart::Variable(name) => state
             .get_var(name)
@@ -1120,19 +1081,15 @@ fn apply_conditional_binary_op(
 }
 
 fn apply_unary_op(
+    state: &mut ShellState,
     op: UnaryArithmeticOp,
     val: ArithmeticResult,
+    operand: &ArithmeticPart,
 ) -> Result<ArithmeticResult, Error> {
-    match op {
-        UnaryArithmeticOp::Plus => Ok(val),
-        UnaryArithmeticOp::Minus => val.checked_neg(),
-        UnaryArithmeticOp::LogicalNot => Ok(if val.is_zero() {
-            ArithmeticResult::new(ArithmeticValue::Integer(1))
-        } else {
-            ArithmeticResult::new(ArithmeticValue::Integer(0))
-        }),
-        UnaryArithmeticOp::BitwiseNot => val.checked_not(),
-    }
+    let result = val.unary_op(operand, op)?;
+    let result_clone = result.clone();
+    state.apply_changes(&result_clone.changes);
+    Ok(result)
 }
 
 async fn execute_pipe_sequence(