fix(rust_spec): Add doc strings to generated error enum and variants - #1689
fix(rust_spec): Add doc strings to generated error enum and variants#1689willemneal wants to merge 1 commit into
Conversation
Given that the comments exist in the spec it is useful for DevX to see these comments when working with generated contracts and understanding their errors.
There was a problem hiding this comment.
Pull request overview
This PR adds documentation strings to generated error enums and their variants in the Soroban Rust SDK's spec code generation.
Changes:
- Modified
generate_error_enumfunction to include doc attributes for error enums and their variants - Added test coverage for the new documentation generation functionality
There was a problem hiding this comment.
See:
rs-soroban-sdk/soroban-spec-rust/src/types.rs
Lines 10 to 13 in 1d0fb59
Making it as safe as possible to contractimport! an untrusted contract is the tradeoff. It's a tradeoff with a cost for sure, but I think the tradeoff is more valuable.
|
Do you mean as doc tests? Could we not ban code blocks? |
I don't think there's a way to ban code blocks within rust tests. The code generator would have to sanitize them away, or mark code blocks as non-executable, and guard against injection attacks. |
I meant that we just don't even generate the doc string if there is a code block. Since they aren't necessary for errors anyway. |
That's still doing sanitizing with the same challenges and risks, just moving it to another spot. And that'd put the protection on the generation side, when the protection needs to be on the importer side. Otherwise anyone could circumvent the protection by simply crafting their own docs that contained a doctest.
This issue applies to doc strings across all entries in the spec, not just errors. |
What
Insert in doc strings into generated error types and each of their variants
Why
This way IDEs and generated rust code in generally will add the provided documentation to make it easier for devs to understand what they mean.
Known limitations
N/A