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

native string utils cheatcodes #6891

Merged

Conversation

1010adigupta
Copy link
Contributor

@1010adigupta 1010adigupta commented Jan 24, 2024

Motivation

closes #6818

Solution

The following string utils cheatcodes have been created:

  • toLowercase
  • toUppercase
  • trim
  • replace
  • split

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

this looks good, but—could we add some tests for this?

@Evalir Evalir requested a review from klkvr January 24, 2024 17:27
Copy link
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

nice!

agree with @Evalir that this should have some tests but besides that lgtm

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is great!

would love a few sanity tests in the testdata/cheats folder

@mattsse mattsse added the A-cheatcodes Area: cheatcodes label Jan 24, 2024
@1010adigupta
Copy link
Contributor Author

okay working on tests now

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

sweet!

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice! ty

updated the spec

@@ -1077,6 +1077,22 @@ interface Vm {
#[cheatcode(group = String)]
function parseBool(string calldata stringifiedValue) external pure returns (bool parsedValue);

/// Converts the given `string` value to Lowercase.
#[cheatcode(group = String)]
function toLowercase(string calldata stringifiedValue) external pure returns (string memory stringifiedValue);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tested in forge-std to confirm, and this will error with identifier already declared in solidity since the stringifiedValue name is reused. Suggested var names for all sigs:

Suggested change
function toLowercase(string calldata stringifiedValue) external pure returns (string memory stringifiedValue);
function toLowercase(string calldata input) external pure returns (string memory output);

function trim(string calldata stringifiedValue) external pure returns (string memory stringifiedValue);
/// Replaces occurrences of `from` in the given `string` with `to`.
#[cheatcode(group = String)]
function replace(string calldata stringifiedValue, string calldata from, string calldata to) external pure returns (string memory stringifiedValue);
Copy link
Member

Choose a reason for hiding this comment

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

Same as https://github.com/foundry-rs/foundry/pull/6891/files#r1465480622
These are not stringifed values, but just strings, so s or str is more suitable

@1010adigupta
Copy link
Contributor Author

@mds1 ig it's corrected now, can you check

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

ty!

@1010adigupta
Copy link
Contributor Author

@mattsse ig this can be merged now

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

ty 🙏

@mattsse mattsse merged commit c61dc80 into foundry-rs:master Jan 25, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

native String utils cheatcodes
6 participants