feat[lang]: use keyword arguments for event instantiation #4257
Conversation
Backwards compatible (with deprecation warning) with positional arguments
z80dev
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4257 +/- ##
==========================================
- Coverage 91.36% 90.18% -1.19%
==========================================
Files 108 108
Lines 15637 15653 +16
Branches 3440 3446 +6
==========================================
- Hits 14287 14116 -171
- Misses 920 1066 +146
- Partials 430 471 +41
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Left a comment, otherwise LGTM!
Handing it over to @charles-cooper
we don't need to set a parent node for the kw node's value
rather than converting positional arg ASTs to kwarg ASTs, we handle kwargs alongside the existing positional arg logic
Co-authored-by: Charles Cooper <cooper.charles.m@gmail.com>
Reduces duplication around EventT and StructT classes
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| from vyper.semantics.analysis.utils import ( | ||
| check_modifiability, | ||
| validate_expected_type, | ||
| validate_kwargs, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import Note
also update expected exception on some struct failures now that we're using a common helper function
|
merging in master dropped the coverage a good amount but from looking at the report I don't think its related to this PR, but will look into it |
i wouldn't worry about it too much, the codecov app often fails to merge the coverage files properly |
Co-authored-by: Charles Cooper <cooper.charles.m@gmail.com>
add __sub__ and __isum__ to OrderedSet
### 🕓 Changelog Vyper changed event instantiation from taking positional arguments to taking keyword arguments (see PR [#4257](vyperlang/vyper#4257)). This PR refactors all 🐍 snekmate event instantiations accordingly (resolves #277). Furthermore, we configure the tests to use the latest commit from the `master` branch of Vyper going forward, ensuring we are always testing against the most up-to-date version of the compiler. --------- Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
What I did
Closes #4249
Implement support for keyword arguments when calling event constructors
How I did it
Mainly updated code to check
node.keywordsinstead ofnode.argswhere dealing with events and the arguments passed to the constructor.During semantic analysis, if we detect an event instantiated with positional args, we log a warning. Semantic analysis is the first place we can deal with this problem, as during parsing we don't have access to info such as what the names of a specific events' arguments are.
This implementation is fully backwards compatible with the positional arguments syntax.
How to verify it
All unit tests have been updated to use keyword arguments for events, so passing tests give us a lot of confidence the change is correct.
In addition, a new unit test was added verifying that positional arguments still compile and work as expected.
Commit message
Description for the changelog
Use kwargs for event instantiation
Cute Animal Picture