feat(shield): add arithmetic operations #330
Conversation
WalkthroughWalkthroughThe changes introduce basic arithmetic operators ( Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
api/shield/token/token.pulsar.gois excluded by!api/**shield/token/token.pb.gois excluded by!**/*.pb.go
Files selected for processing (7)
- proto/shield/token/token.proto (1 hunks)
- shield/internal/evaluator/evaluator.go (1 hunks)
- shield/internal/evaluator/evaluator_test.go (1 hunks)
- shield/internal/lexer/lexer.go (1 hunks)
- shield/internal/lexer/lexer_test.go (2 hunks)
- shield/internal/parser/parser.go (2 hunks)
- shield/internal/parser/parser_test.go (1 hunks)
Additional Context Used
Path-based Instructions (6)
shield/internal/lexer/lexer_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"shield/internal/evaluator/evaluator_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"shield/internal/lexer/lexer.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.shield/internal/parser/parser_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"shield/internal/evaluator/evaluator.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.shield/internal/parser/parser.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (10)
proto/shield/token/token.proto (1)
27-30: The addition of arithmetic operation types in the enum is correctly implemented.shield/internal/lexer/lexer_test.go (2)
10-10: The test input string has been correctly updated to include arithmetic operators.
30-33: The test cases for the new arithmetic tokens are correctly added and follow the expected structure.shield/internal/evaluator/evaluator_test.go (1)
42-61: The new test functionTestArithmeticIntegeris well-structured and covers a variety of arithmetic operations, ensuring that the basic arithmetic functionality works as expected.shield/internal/lexer/lexer.go (1)
52-59: TheNextTokenmethod has been correctly extended to handle the new arithmetic operators. Each case correctly creates a new token with the appropriate type.shield/internal/parser/parser_test.go (1)
137-152: The test cases for arithmetic operations inTestParserare correctly structured to ensure the parser handles operator precedence as expected.shield/internal/evaluator/evaluator.go (1)
88-96: TheevalInfixExpressionfunction has been correctly updated to handle arithmetic operations between integers. Each case correctly computes the result based on the operator.shield/internal/parser/parser.go (3)
17-18: The addition ofADD_SUBandMUL_DIVconstants is correctly implemented to handle the precedence of new arithmetic operations.
25-28: The update to theprecedencesmap correctly associates the new arithmetic tokens with their respective precedence levels.
64-67: The registration of infix parse functions for arithmetic operations in theNewmethod is correctly implemented.
|
I'll merge comparison operators from |
b12508b to
51dabed
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
api/shield/token/token.pulsar.gois excluded by!api/**shield/token/token.pb.gois excluded by!**/*.pb.go
Files selected for processing (7)
- proto/shield/token/token.proto (1 hunks)
- shield/internal/evaluator/evaluator.go (1 hunks)
- shield/internal/evaluator/evaluator_test.go (2 hunks)
- shield/internal/lexer/lexer.go (1 hunks)
- shield/internal/lexer/lexer_test.go (2 hunks)
- shield/internal/parser/parser.go (3 hunks)
- shield/internal/parser/parser_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (7)
- proto/shield/token/token.proto
- shield/internal/evaluator/evaluator.go
- shield/internal/evaluator/evaluator_test.go
- shield/internal/lexer/lexer.go
- shield/internal/lexer/lexer_test.go
- shield/internal/parser/parser.go
- shield/internal/parser/parser_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
CHANGELOG.md (4)
Line range hint
106-106: Correct grammatical number for better readability.The phrase "queries performance" should be "query performance" to correctly match the singular form of "performance".
- ...nd `x/warden`'s store design to improve queries performance: * (x/intent) [#117](ht... + ...nd `x/warden`'s store design to improve query performance: * (x/intent) [#117](ht...
Line range hint
109-109: Add missing article for grammatical correctness.Consider adding "the" before "owner" to complete the phrase.
- ...otocol/wardenprotocol/pull/122) Improve `SpacesByOwner` query by adding a new "ow... + ...otocol/wardenprotocol/pull/122) Improve `SpacesByOwner` query by adding the new "ow...
Line range hint
57-57: Correct indentation for list items.The indentation for list items should be consistent throughout the document. Here, it should be reduced to 2 spaces instead of 4.
- * (x/intent) [#117](https://github.com/warden-protocol/wardenprotocol/pull/117) Pin the intent definition when an Action is created instead of just referencing it. + * (x/intent) [#117](https://github.com/warden-protocol/wardenprotocol/pull/117) Pin the intent definition when an Action is created instead of just referencing it.Also applies to: 70-70, 82-82, 83-83, 107-107, 108-108, 109-109, 114-114, 117-117
Line range hint
41-41: Remove excessive blank lines for cleaner formatting.Multiple consecutive blank lines should be reduced to a single blank line to maintain a clean and professional format in the document.
- - +Also applies to: 128-128, 133-133, 134-134
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional Context Used
LanguageTool (2)
CHANGELOG.md (2)
Near line 106: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...ndx/warden's store design to improve queries performance: * (x/intent) [#117](ht...
Rule ID: AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER
Near line 109: You might be missing the article “the” here.
Context: ...otocol/wardenprotocol/pull/122) ImproveSpacesByOwnerquery by adding a new "ow...
Rule ID: AI_EN_LECTOR_MISSING_DETERMINER_THE
Markdownlint (14)
CHANGELOG.md (14)
57: Expected: 2; Actual: 4
Unordered list indentation
70: Expected: 2; Actual: 4
Unordered list indentation
82: Expected: 2; Actual: 4
Unordered list indentation
83: Expected: 2; Actual: 4
Unordered list indentation
107: Expected: 2; Actual: 4
Unordered list indentation
108: Expected: 2; Actual: 4
Unordered list indentation
109: Expected: 2; Actual: 4
Unordered list indentation
114: Expected: 2; Actual: 4
Unordered list indentation
117: Expected: 2; Actual: 4
Unordered list indentation
41: Expected: 1; Actual: 2
Multiple consecutive blank lines
128: Expected: 1; Actual: 2
Multiple consecutive blank lines
133: Expected: 1; Actual: 2
Multiple consecutive blank lines
134: Expected: 1; Actual: 3
Multiple consecutive blank lines
114: null
Bare URL used
Path-based Instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
| @@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| * (shield) Add new operators: `<`, `>`, `<=`, `>=`, `==`, `!=` for comparing integers to each others. | |||
| * (shield) Add support for `string` objects | |||
| * (shield) Add `contains(elem, array)` builtin function for checking if `elem` is contained in the `array` | |||
| * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic | |||
There was a problem hiding this comment.
Ensure consistent punctuation in list items.
Consider adding a period at the end of the entry for consistency with other entries in the list.
- * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic
+ * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic | |
| * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic. |
add tests for arithmetic and comparison combinations update CHANGELOG
7790b64 to
9b8a9bc
Compare
done as requested |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
CHANGELOG.md (3)
Line range hint
57-57: Correct the indentation of unordered list items.- * (wardend) analyzers smart contract: a way for 3rd party builders to provide metadata to shield's intents during new signature requests + * (wardend) analyzers smart contract: a way for 3rd party builders to provide metadata to shield's intents during new signature requests - * can be used to pass an Ethereum unsigned transaction, and will return the correct DataForSigning + * can be used to pass an Ethereum unsigned transaction, and will return the correct DataForSigningAlso applies to: 70-70, 82-82, 83-83, 107-107, 108-108, 109-109, 114-114, 117-117
Line range hint
41-41: Reduce multiple consecutive blank lines to a single blank line.- +Also applies to: 128-128, 133-133, 134-134
Line range hint
114-114: Avoid using bare URLs. Provide a descriptive link text.- * (cosmwasm) [#156](https://github.com/warden-protocol/wardenprotocol/pull/156) Add support for smart contracts by integrating CosmWasm + * (cosmwasm) [#156](Add support for smart contracts by integrating CosmWasm)[https://github.com/warden-protocol/wardenprotocol/pull/156]
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
api/shield/token/token.pulsar.gois excluded by!api/**shield/token/token.pb.gois excluded by!**/*.pb.gowarden/x/warden/types/v1beta2/signature.pb.gois excluded by!**/*.pb.go
Files selected for processing (8)
- CHANGELOG.md (1 hunks)
- proto/shield/token/token.proto (1 hunks)
- shield/internal/evaluator/evaluator.go (1 hunks)
- shield/internal/evaluator/evaluator_test.go (2 hunks)
- shield/internal/lexer/lexer.go (1 hunks)
- shield/internal/lexer/lexer_test.go (2 hunks)
- shield/internal/parser/parser.go (3 hunks)
- shield/internal/parser/parser_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (7)
- proto/shield/token/token.proto
- shield/internal/evaluator/evaluator.go
- shield/internal/evaluator/evaluator_test.go
- shield/internal/lexer/lexer.go
- shield/internal/lexer/lexer_test.go
- shield/internal/parser/parser.go
- shield/internal/parser/parser_test.go
Additional context used
Path-based instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
CHANGELOG.md
57-57: Expected: 2; Actual: 4
Unordered list indentation
70-70: Expected: 2; Actual: 4
Unordered list indentation
82-82: Expected: 2; Actual: 4
Unordered list indentation
83-83: Expected: 2; Actual: 4
Unordered list indentation
107-107: Expected: 2; Actual: 4
Unordered list indentation
108-108: Expected: 2; Actual: 4
Unordered list indentation
109-109: Expected: 2; Actual: 4
Unordered list indentation
114-114: Expected: 2; Actual: 4
Unordered list indentation
117-117: Expected: 2; Actual: 4
Unordered list indentation
41-41: Expected: 1; Actual: 2
Multiple consecutive blank lines
128-128: Expected: 1; Actual: 2
Multiple consecutive blank lines
133-133: Expected: 1; Actual: 2
Multiple consecutive blank lines
134-134: Expected: 1; Actual: 3
Multiple consecutive blank lines
114-114: null
Bare URL used
Additional comments not posted (1)
CHANGELOG.md (1)
50-50: Ensure consistent punctuation in list items.Consider adding a period at the end of the entry for consistency with other entries in the list.
- * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic + * (shield) Add `+`, `-`, `*`, `/` math operators for basic integer arithmetic.
Add basic arithmetic operations (closes #260)
No dealing with overflows and parsing negative numbers
Summary by CodeRabbit
New Features
+,-,*,/) on integers.Bug Fixes
Tests
Documentation