feat(sdk): add Rust code generation#669
Open
gaojunran wants to merge 6 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds complete Rust SDK generation support to the ChangesRust SDK Generation
Sequence Diagram(s)sequenceDiagram
participant CliArg as CLI --language
participant Dispatch as generate dispatcher
participant RustGen as rust::generate()
participant TypesGen as types::render()
participant ClientGen as client::render()
participant RuntimeConst as RUNTIME_RS constant
participant Output as SdkOutput
CliArg->>Dispatch: SdkLanguage::Rust + SdkOptions
Dispatch->>RustGen: call rust::generate(spec, opts)
RustGen->>TypesGen: render(spec, pkg_name)
TypesGen->>Output: types.rs string
RustGen->>ClientGen: render(spec, pkg_name)
ClientGen->>Output: client.rs string
RustGen->>RuntimeConst: read RUNTIME_RS
RuntimeConst->>Output: runtime.rs string
RustGen->>Output: format lib.rs with re-exports
Output->>Output: finalize SdkOutput
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #669 +/- ##
==========================================
+ Coverage 79.31% 81.16% +1.85%
==========================================
Files 56 59 +3
Lines 10487 11779 +1292
Branches 10487 11779 +1292
==========================================
+ Hits 8318 9561 +1243
- Misses 1409 1445 +36
- Partials 760 773 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Greptile SummaryThis PR adds a full Rust SDK code-generator (
Confidence Score: 4/5Safe to merge after adding the cargo availability guard to the compile test. The core generators are sound and all previously flagged structural bugs are fixed. The compile test will panic rather than skip gracefully when cargo is not installed, which could block CI on non-Rust environments. lib/tests/sdk_compile.rs needs the tool_exists("cargo") guard before invoking cargo check. Important Files Changed
Reviews (7): Last reviewed commit: "fix(sdk): remove unused choice type impo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…type name mismatch, borrow-after-move
…structor borrow-after-move Flags are now inserted before the '--' separator instead of after it, ensuring CLI parsers correctly recognize them as flags rather than positional arguments. This fix applies to all three SDK generators (Rust, Python, TypeScript). Also fixes the non-root constructor to emit 'runner,' after 'runner.clone()' calls, preventing borrow-after-move compile errors for nested subcommands.
- Use let cmd_args (no mut) when command has no args and no flags
- Only generate {Name}Flags struct when command has its own flags;
commands that only inherit global flags use GlobalFlags directly
…rved keywords - Filter out choice enum names from client.rs imports since they are only referenced in types.rs, not in client.rs - Add missing Rust reserved keywords to sanitize_rs_ident: priv, do, try, become, final, typeof, unsized, virtual - Remove 'default' which is not a reserved keyword in Rust
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/tests/sdk_compile.rs (1)
144-188: ⚡ Quick winAdd cargo availability check for consistency with Python test.
The Rust compilation test should verify
cargois available before running, matching the pattern used in the Python test (Line 95). Without this check, the test will panic with a confusing error ifcargois not installed.🔧 Proposed fix to add cargo availability check
#[test] fn test_rust_sdk_compiles() { + if !tool_exists("cargo") { + eprintln!("Skipping Rust SDK compilation test - cargo not found"); + return; + } + let spec = full_spec();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/tests/sdk_compile.rs` around lines 144 - 188, The test_rust_sdk_compiles test should check that the "cargo" binary is available before invoking Command::new("cargo") to avoid a confusing panic when cargo is missing; add an availability check (e.g. call Command::new("cargo").arg("--version").output() or .status() and test for Err or non-success) immediately before the existing Command::new("cargo") invocation and if cargo is not found, print/elog a short message and return/skip the test instead of continuing to run cargo check; update the test_rust_sdk_compiles function (which uses usage::sdk::generate and later builds the Command) to perform this pre-check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/sdk/rust/types.rs`:
- Around line 461-463: escape_rs_string currently only handles backslash and
double-quote and fails to escape control/non-printable characters; change its
implementation to build a fully escaped string by iterating chars and using each
char's escape_default (e.g. s.chars().flat_map(|c|
c.escape_default()).collect::<String>()) and then ensure double quotes are
escaped (e.g. replace '"' with r#"\""#) so that control characters like \n, \r,
\t, \0 and other non-printables are emitted as valid Rust escapes when used by
the generator (function escape_rs_string, which is used to embed spec metadata
such as spec.about).
---
Nitpick comments:
In `@lib/tests/sdk_compile.rs`:
- Around line 144-188: The test_rust_sdk_compiles test should check that the
"cargo" binary is available before invoking Command::new("cargo") to avoid a
confusing panic when cargo is missing; add an availability check (e.g. call
Command::new("cargo").arg("--version").output() or .status() and test for Err or
non-success) immediately before the existing Command::new("cargo") invocation
and if cargo is not found, print/elog a short message and return/skip the test
instead of continuing to run cargo check; update the test_rust_sdk_compiles
function (which uses usage::sdk::generate and later builds the Command) to
perform this pre-check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 97b0195e-3daf-425e-8d7c-c48d4598f82b
⛔ Files ignored due to path filters (56)
lib/src/sdk/python/snapshots/usage__sdk__python__tests__python_client.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_client_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_count_flag_build.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_deep_nesting.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_double_dash_automatic.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_exec_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flag_edge_cases-2.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_flags_only_subcommand.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_full_feature_client.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_global_flags_flags_only.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_multiple_aliases.snapis excluded by!**/*.snaplib/src/sdk/python/snapshots/usage__sdk__python__tests__python_negate_flag_build.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_arg_defaults.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_boolean_flag_default_false.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_choice_collision.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_client.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_config_all_optional.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_config_boolean_default_false.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_config_props.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_config_string_with_default.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_count_flag_build.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_deep_nesting.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_double_dash_automatic.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_example_without_lang.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_flag_edge_cases-2.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_flag_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_flag_with_choices.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_flag_with_env.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_flags_only_subcommand.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_full_feature_client.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_full_feature_types.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_global_flags_flags_only.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_global_repeatable_flags.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_hyphenated_subcommands.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_lib.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_minimal.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_multiple_aliases.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_negate_flag_build.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_optional_arg_empty_flags.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_optional_variadic_arg.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_package_name_override.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_required_flag_type.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_runtime.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_types.snapis excluded by!**/*.snaplib/src/sdk/rust/snapshots/usage__sdk__rust__tests__rust_var_value_flag_with_default.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__deep_nesting.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__flags_only_subcommand.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__full_feature_client.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_client.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_client_edge_cases.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_count_flag_build.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_double_dash_automatic.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_flag_edge_cases-2.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_global_flags_flags_only.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_multiple_aliases.snapis excluded by!**/*.snaplib/src/sdk/typescript/snapshots/usage__sdk__typescript__types__tests__typescript_negate_flag_build.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
cli/assets/fig.tscli/src/cli/generate/sdk.rscli/usage.usage.kdldocs/cli/reference/commands.jsondocs/cli/reference/generate/sdk.mddocs/cli/sdk.mdlib/src/sdk/mod.rslib/src/sdk/python/mod.rslib/src/sdk/rust/client.rslib/src/sdk/rust/mod.rslib/src/sdk/rust/runtime.rslib/src/sdk/rust/types.rslib/src/sdk/typescript/wrappers.rslib/tests/sdk_compile.rs
Comment on lines
+461
to
+463
| pub fn escape_rs_string(s: &str) -> String { | ||
| s.replace('\\', r"\\").replace('"', r#"\""#) | ||
| } |
There was a problem hiding this comment.
Incomplete string escaping will generate invalid Rust code.
escape_rs_string only escapes backslash and double-quote, but does not handle control characters like newlines (\n), tabs (\t), or carriage returns (\r). When this function is used to embed spec metadata (lines 35, 41, 47) or other string content into generated Rust string literals, any control characters in the source will produce syntactically invalid Rust.
For example, if spec.about contains "A tool\nfor testing", the generated code would be:
pub const ABOUT: &str = "A tool
for testing"; // ❌ syntax error🔧 Proposed fix to escape control characters
pub fn escape_rs_string(s: &str) -> String {
- s.replace('\\', r"\\").replace('"', r#"\""#)
+ s.replace('\\', r"\\")
+ .replace('"', r#"\""#)
+ .replace('\n', r"\n")
+ .replace('\r', r"\r")
+ .replace('\t', r"\t")
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/sdk/rust/types.rs` around lines 461 - 463, escape_rs_string currently
only handles backslash and double-quote and fails to escape
control/non-printable characters; change its implementation to build a fully
escaped string by iterating chars and using each char's escape_default (e.g.
s.chars().flat_map(|c| c.escape_default()).collect::<String>()) and then ensure
double quotes are escaped (e.g. replace '"' with r#"\""#) so that control
characters like \n, \r, \t, \0 and other non-printables are emitted as valid
Rust escapes when used by the generator (function escape_rs_string, which is
used to embed spec metadata such as spec.about).
Contributor
Author
|
@jdx ready for review! remaining comments are for add cargo cli existence check in ci and I don't think it's needed lol. and coverage workflow fails strangely and may need a check |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
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
1 participant
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.
Summary by CodeRabbit
Release Notes
New Features
--language rustwith thegenerate sdkcommand).Documentation