-
Notifications
You must be signed in to change notification settings - Fork 97
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
Simple Stubbing with Contracts #2746
Simple Stubbing with Contracts #2746
Conversation
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.
It looks good so far! I still need to review the proc_macro logic.
tests/expected/function-contract/missing_contract_for_replace.expected
Outdated
Show resolved
Hide resolved
Co-authored-by: Celina G. Val <celinval@amazon.com>
tests/expected/function-contract/missing_contract_for_check.expected
Outdated
Show resolved
Hide resolved
I've made a bit of a major change and added a "module-level" contract documentation as I would appreciate any feedback you ahve on this new "module-level" documentation. I think at this point it doesn't have to be perfect as it will expand with future contract PRs anyway but whether it's good in broad strokes. Also if you have the time, download it and build the docs for the This now offers also the possibility of grouping all contract attributes in this module. I haven't done that here, I just reexport them, but I could if people want. |
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.
I'm impressed with all this macro magic. Thanks!
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.
Amazing! Thanks @JustusAdam
Resolving the last conversation (the obscure paragraph) was quite tricky and lead to a major expansion in the docs. You may want to give them a read and see if they make sense @celinval and also it now has the diagram you wanted @feliperodri. You may want to read the rendered version which you can get with |
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.
Thank you, Justus! Only small nitpicks (feel free to merge) and the state machine looks very clear!
tests/expected/function-contract/missing_contract_for_replace.expected
Outdated
Show resolved
Hide resolved
tests/expected/function-contract/missing_contract_for_check.expected
Outdated
Show resolved
Hide resolved
Co-authored-by: Felipe R. Monteiro <rms.felipe@gmail.com>
…ani into simple-contract-replacement
Co-authored-by: Felipe R. Monteiro <rms.felipe@gmail.com>
Description of changes:
Expands the contract features introduced in #2655 by completing the checking with stubbing/replacement capabilities.
Resolved issues:
Resolves #2621
Related RFC: 0009 Function Contracts
Tracking issue: #2652
Optional #ISSUE-NUMBER.
Call-outs:
This makes the internal state machine in the macro expansion more explicit. Previously it was a single, long function it is now a struct and an enum that hold the state and the generation is split into multiple methods on those. While technically a refactoring change to some degree it really only made sense to do this now, as now there are two dimensions along which the macro codegen operates.
requires
/ensures
). This one was present before also.Testing:
How is this change tested? New
expected
test casesIs this a refactor change? No
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.