feat[lang]: use keyword arguments for event instantiation#4257
Merged
charles-cooper merged 30 commits intovyperlang:masterfrom Oct 5, 2024
Merged
feat[lang]: use keyword arguments for event instantiation#4257charles-cooper merged 30 commits intovyperlang:masterfrom
charles-cooper merged 30 commits intovyperlang:masterfrom
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. |
tserg
reviewed
Sep 25, 2024
Contributor
tserg
left a comment
tserg
left a comment
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 <[email protected]>
Reduces duplication around EventT and StructT classes
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+16
to
+20
| from vyper.semantics.analysis.utils import ( | ||
| check_modifiability, | ||
| validate_expected_type, | ||
| validate_kwargs, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import
also update expected exception on some struct failures now that we're using a common helper function
Contributor
Author
|
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 |
Member
i wouldn't worry about it too much, the codecov app often fails to merge the coverage files properly |
Co-authored-by: Charles Cooper <[email protected]>
add __sub__ and __isum__ to OrderedSet
This was referenced Oct 5, 2024
pcaversaccio
added a commit
to pcaversaccio/snekmate
that referenced
this pull request
Oct 7, 2024
### 🕓 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 <[email protected]>
2 tasks
willbrown84
added a commit
to willbrown84/snekmate
that referenced
this pull request
Sep 23, 2025
### 🕓 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 <[email protected]>
void-rider5560p
added a commit
to void-rider5560p/snekmate
that referenced
this pull request
Sep 28, 2025
### 🕓 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 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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