implemented parenthesis in expr #34
Conversation
WalkthroughThis pull request introduces a new production rule to handle parenthesized expressions in the grammar. The changes span grammar updates in Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Test
participant Parser as NumscriptParser
participant Listener as NumscriptListener
Client->>Parser: Request to parse expression e.g. set_tx_meta("k1", 1 + (2 - 3))
Parser->>Parser: Recognize '(' and initiate ParenthesizedExprContext
Parser->>Listener: Call EnterParenthesizedExpr
Parser->>Parser: Parse inner expression (2 - 3)
Parser->>Listener: Call ExitParenthesizedExpr
Parser->>Client: Return parsed expression tree
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
==========================================
- Coverage 63.27% 63.14% -0.14%
==========================================
Files 29 29
Lines 6462 6520 +58
==========================================
+ Hits 4089 4117 +28
- Misses 2191 2217 +26
- Partials 182 186 +4 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/parser/parser_test.go (1)
340-346: Add more test cases for comprehensive coverage.While the current test case verifies basic parenthesized expression parsing, consider adding more test cases to cover:
- Nested parentheses:
((1 + 2) - (3 + 4))- Mixed operators:
(1 + 2) - (3 + 4)- Edge cases:
(1),((1)),(1 + (2 + 3))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
internal/parser/__snapshots__/parser_fault_tolerance_test.snapis excluded by!**/*.snap,!**/*.snapinternal/parser/__snapshots__/parser_test.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (7)
Numscript.g4(1 hunks)internal/parser/antlr/Numscript.interp(1 hunks)internal/parser/antlr/numscript_base_listener.go(1 hunks)internal/parser/antlr/numscript_listener.go(2 hunks)internal/parser/antlr/numscript_parser.go(48 hunks)internal/parser/parser.go(1 hunks)internal/parser/parser_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (32)
internal/parser/antlr/numscript_parser.go (24)
57-57: No concerns about the updated serialized ATN segment.
These changes appear to be auto-generated from the grammar updates.
62-68: No concerns about the updated serialized ATN data.
Again, these lines appear auto-generated.
69-69: No concerns.
This line also appears auto-generated from grammar updates.
74-74: No concerns.
Automatically generated content.
80-81: No concerns.
Auto-generated changes for parser initialization.
90-90: No concerns.
Likewise auto-generated line for the parser.
96-96: No concerns.
Continues the auto-generated changes.
99-99: No concerns.
Auto-generated.
103-105: No concerns.
Parser synchronization steps—auto-generated.
107-107: No concerns.
Auto-generated code.
108-110: No concerns.
Continues standard parser logic.
121-128: No concerns.
These lines define portion/variable syntax, appear to be standard auto-generated code.
140-140: No concerns.
Remains part of the parser's internal data table.
148-148: No concerns.
Automatic table updates.
157-157: No concerns.
Auto-generated indexing for parser data.
166-166: No concerns.
Standard parser code, auto-generated.
177-177: No concerns.
Maintains the parser simulator state.
185-185: No concerns.
Still auto-generated logic.
192-192: No concerns.
Reflects standard parser logic.
199-199: No concerns.
No functional risk identified.
218-218: No concerns.
Continues auto-generated steps.
819-819: No concerns.
Routine parser logic.
909-961: Implementation ofParenthesizedExprContext
These lines correctly introduce a new parse context for parenthesized expressions. The structure appears coherent, with proper getters and rule overrides.
1186-1206: Grammar extension to handle parentheses invalueExpr
This switch case cleanly inserts a new branch for matchingLPARENS ... RPARENS. The code checks out logically and helps parse nested expressions.Numscript.g4 (1)
60-61: LGTM! The grammar changes correctly implement parenthesized expressions.The changes look good:
- The infix expression rule correctly uses left recursion for operator precedence.
- The new parenthesized expression rule properly allows wrapping any
valueExprin parentheses, enabling expression grouping.internal/parser/antlr/Numscript.interp (2)
104-104: LGTM! Auto-generated changes.The changes to the ATN array are auto-generated by ANTLR and correctly reflect the grammar updates.
1-104: Skip review of auto-generated file.This is an auto-generated file by ANTLR and should not be modified manually. The changes in the
atnarray are a result of grammar modifications inNumscript.g4.internal/parser/antlr/numscript_listener.go (2)
35-37: LGTM! The listener interface is correctly extended.The new methods for handling parenthesized expressions follow the established pattern and are properly documented.
Also applies to: 158-160
1-257: Skip review of auto-generated file.This is an auto-generated file by ANTLR and should not be modified manually. The new methods
EnterParenthesizedExprandExitParenthesizedExprare correctly generated based on the grammar modifications.internal/parser/antlr/numscript_base_listener.go (1)
72-76: LGTM!The new methods for handling parenthesized expressions are correctly implemented following the base listener pattern.
internal/parser/parser.go (1)
307-309: LGTM!The implementation correctly handles parenthesized expressions by recursively parsing the inner expression. This approach preserves the expression's semantics and follows the established pattern for handling other expression types.
internal/parser/parser_test.go (1)
340-346: LGTM! Test case verifies parentheses support.The test case is well-structured and effectively verifies the parsing of expressions with parentheses, ensuring correct operator precedence.
This PR allows numscript to use parenthesis in expressions
E.g.
$x + ($y - $z)That's a new (backwards compatible) functionality compared to numscript machine implementation and it's not under a feature flag.
The change only involves the grammar, without introducing new AST nodes