-
-
Notifications
You must be signed in to change notification settings - Fork 836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update lark grammar to handle imported types in array/tuple/map def #4213
base: master
Are you sure you want to change the base?
Conversation
…ple/map definitions
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4213 +/- ##
=======================================
Coverage 92.12% 92.12%
=======================================
Files 119 119
Lines 16967 16967
Branches 2872 2872
=======================================
Hits 15631 15631
Misses 918 918
Partials 418 418 ☔ View full report in Codecov by Sentry. |
// NOTE: Map takes a basic type and maps to another type (can be non-basic, including maps) | ||
_MAP: "HashMap" | ||
map_def: _MAP "[" ( NAME | array_def ) "," type "]" | ||
map_def: _MAP "[" ( NAME | imported_type | array_def ) "," type "]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this correct as mappings, arrays or structs are not supported as key types?
Can we please add a test for the imported types? |
@benber86 is this still needed? |
What I did
Current lark grammar can't parse types imported from modules in array/dynarray/tuples/hashmap definitions.
For instance:
How I did it
Added
imported_type
to the array/dynarray/tuple/map def in grammarHow to verify it
Commit message
Update lark grammar to handle imported types in array/tuples definition
Description for the changelog
Cute Animal Picture