Skip to content
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

Add support for context receivers @PropertySpec and fix issues with annotations #1247

Conversation

seriouslyhypersonic
Copy link
Contributor

@seriouslyhypersonic seriouslyhypersonic commented May 7, 2022

Summary

As discussed in #1244, here is a PR to add support for context receivers @PropertySpec.

Proposed Changes

  1. Make contextReceivers available in PropertySpec builder
  2. Throw exceptions when: property is mutable or immutable without a getter
  3. Update FunSpec to throw exception when a context receiver is added to an accessor
  4. Fix order in which context receivers and annotations are emitted: context receivers must come first

seriouslyhypersonic added 2 commits May 7, 2022 09:44
Prevent context receivers on accessors
Add FunSpec tests:
 - Annotated function with context receiver
 - Accessor with context receiver
Add context receivers to PropertySpec
Add PropertySpec tests:
 - Var with context receiver
 - Val without getter with context receiver
 - Val with context receiver
 - Annotated val with context receiver
Update tests for vars with context receivers without custom accessors
Add test for var with context receivers and custom accessors
Fix code style
Copy link
Collaborator

@Egorand Egorand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Added a single suggestion, otherwise good to merge - thanks!

Co-authored-by: Egor Andreevich <github@egorand.dev>
@seriouslyhypersonic
Copy link
Contributor Author

@Egorand will the snapshot repository be updated with this? I really need this fix to publish my library ☺️

@Egorand
Copy link
Collaborator

Egorand commented May 9, 2022

Yes! As soon as it's merged CI will publish a fresh snapshot.

@seriouslyhypersonic
Copy link
Contributor Author

@Egorand Please recheck as I just accepted your suggestion earlier today and had to update the tests right now. Thank you!

@Egorand
Copy link
Collaborator

Egorand commented May 9, 2022

Merging, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants