Implement parser for arithmetic expressions like a++
#155
Conversation
prsabahrami
left a comment
prsabahrami
left a comment
There was a problem hiding this comment.
Thanks a lot!
I also think cases such as echo $(++2) are being parsed and created a part of the AST. I think we can raise error for this while creating the AST.
| } | ||
|
|
||
|
|
||
| unary_arithmetic_op = _{ |
There was a problem hiding this comment.
Can you change unary_arithmetic_op to pre_arithmetic_op for better clarity?
crates/deno_task_shell/src/parser.rs
Outdated
| } | ||
| Rule::unary_pre_arithmetic_expr => unary_pre_arithmetic_expr(first), | ||
| Rule::unary_post_arithmetic_expr => unary_post_arithmetic_expr(first), | ||
| _ => { |
There was a problem hiding this comment.
Shouldn't this result in an error given that unary_arithmetic_expr can only be one of the unary_pre_arithmetic_expr or unary_post_arithmetic_expr?
crates/deno_task_shell/src/parser.rs
Outdated
|
|
||
| fn unary_pre_arithmetic_expr(pair: Pair<Rule>) -> Result<ArithmeticPart> { | ||
| let mut inner = pair.into_inner(); | ||
| let first = inner.next().unwrap(); |
There was a problem hiding this comment.
I think it's better to replace unwraps with proper error handling.
crates/deno_task_shell/src/parser.rs
Outdated
| fn parse_unary_arithmetic_op_type(pair: Pair<Rule>) -> Result<bool> { | ||
| match pair.as_rule() { | ||
| Rule::unary_arithmetic_op => Ok(true), | ||
| Rule::post_arithmetic_op => Ok(false), | ||
| _ => Err(miette!( | ||
| "Unexpected rule in unary arithmetic operator: {:?}", | ||
| pair.as_rule() |
There was a problem hiding this comment.
This feels fairly hacky to return true of false for the type. Maybe you can try replacing this with match statement in unary_pre_arithmetic_expr?
I believe in such examples, bash raises an error while compiling. I thnik it would be better to do the same here as well. |
Armd04
changed the title
a++a++
|
@Armd04 Can you please take care of the issues on Windows? |
Merge branch 'pr-155' to add support for unary arithmetic operators including increment (++) and decrement (--) in both prefix and postfix forms. Resolved conflicts in: - parser.rs: Unified UnaryArithmeticExpr structure with Pre/Post variants - execute.rs: Updated evaluation logic for new operator structure - types.rs: Added unary_op method for pre/post increment/decrement The implementation now supports expressions like: - ++a (pre-increment) - a++ (post-increment) - --a (pre-decrement) - a-- (post-decrement)
Fixed the following issues raised in code review: 1. Added validation to reject increment/decrement on literal numbers - Now correctly errors on expressions like `echo $(++2)` - Operators can only be applied to variables or parenthesized expressions 2. Replaced .unwrap() calls with proper error handling - Fixed unwrap() in unary_pre_arithmetic_expr (line 1738) - Fixed unwrap() in unary_post_arithmetic_expr (line 1783) - Used ok_or_else() with descriptive error messages 3. Fixed missing parse_variable_expansion function - Restored function that was accidentally removed during merge - Fixes compilation errors 4. Fixed stray conflict marker in types.rs - Removed leftover "<<<<<<< HEAD" marker All tests pass and manual testing confirms: - Post-increment (a++) returns original value, updates variable - Pre-increment (++a) returns new value, updates variable - Invalid expressions like ++2 correctly produce errors
2 participants
No description provided.