-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(cheatcodes): implement new cheatcode to check if a string contains another string #9085
feat(cheatcodes): implement new cheatcode to check if a string contains another string #9085
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.
I just realized that we don't even have vm.contains
so this would just by assert(vm.contains())
wdyt @grandizzy
yep, agree |
Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
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.
Thanks! LGTM
Personally don't have a strong preference for renaming assertContains
-> contains
as proposed by others but wouldn't mind it either. Without the assert*
prefix would be more clear to users that they can use it in branches and it doesn't throw a hard error that they might expect when using assertions.
Sure, let me rename the method from |
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
@zerosnacks PR is ready for another review :) |
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.
Lgtm!
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.
Thanks @leovct! LGTM
@zerosnacks @leovct @grandizzy thank you so much for getting this over the line and closing out my older PR! Cheers! |
- add `delegatecall` flag to `prank` cheatcodes (foundry-rs/foundry#8863) - support EIP-7702 Delegations (`create/sign/attachDelegation`) (foundry-rs/foundry#9236) - add `contains` to check if a string contains another string (foundry-rs/foundry#9085)
…ns another string (foundry-rs#9085) * feat: implement new cheatcode to check if a string contains another string * chore: make clippy and rustfmt happy * chore: vm.contains should return a boolean * Update testdata/cheats/Vm.sol Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> * Update crates/cheatcodes/spec/src/vm.rs Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> * chore: update `cheatcodes.json` * chore: update var names * chore: rename to `vm.contains` * Update crates/cheatcodes/spec/src/vm.rs Co-authored-by: Matt Solomon <matt@mattsolomon.dev> * Update crates/cheatcodes/spec/src/vm.rs Co-authored-by: Matt Solomon <matt@mattsolomon.dev> * chore: address PR comments --------- Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
- add `delegatecall` flag to `prank` cheatcodes (foundry-rs/foundry#8863) - support EIP-7702 Delegations (`create/sign/attachDelegation`) (foundry-rs/foundry#9236) - add `contains` to check if a string contains another string (foundry-rs/foundry#9085)
Motivation
Closes #4859
Solution
Simple check if a string contains another string.
As a follow-up, we could implement other cheatcodes to check if an array contains an element?