-
Notifications
You must be signed in to change notification settings - Fork 3
Introduces #[WPContextual] derive macro #53
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
Merged
Conversation
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
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.
This PR introduces a new crate
wp_derive
, which implements the#[WPContextual]
derive macro. This has been a big undertaking, mainly because splitting this PR into multiple parts wasn't a good option. Procedural macro code can be a bit hard to follow unless the reviewer can look up the documentation and tests. Furthermore, we are hoping that we won't need to touch this crate very often - if at all. So, it made sense to make this as production ready as possible.The best way to learn why we need this macro is to read the documentation which you can generate by running
cargo doc
. Thewp_derive
crate level documentation goes through everything in a beginner friendly way whereas the documentation for the#[WPContextual]
macro is more technically oriented.The main test coverage for this implementation uses
trybuild
which acts as a great documentation in itself, as the errors will be checked into the source control as separate files. With derive macros, it's much more important to have these integration tests than unit tests. Having said that, I felt some of the helper functions were a bit more complicated, so I've added some unit tests for some extra assurance.The macro has a decent error handling. Since it works with
rust-analyzer
, developers will get these errors as they type in their editor. The errors show exactly what is causing the issue - at least most of the time. There are some minor instances where the error span is not as good as I'd like, but in those cases I didn't see any practical benefit in introducing any extra complexity.I've documented the implementation quite a bit, but happy to try and improve that if anything is not clear.