fix(generator): impl stub one field per line to fix E0063 missing fields#292
fix(generator): impl stub one field per line to fix E0063 missing fields#292
Conversation
- impl_controller_stub.rs.txt: remove Tera whitespace stripping so each response field is on its own line; all-on-one-line let first // comment hide rest of fields and caused missing-field compile errors - Resolve response schema before extract_fields for full Response literal - Add decimal/money lint rule in lint_property; tests
📝 WalkthroughWalkthroughThe pull request introduces OpenAPI schema $ref resolution during stub generation to properly handle referenced components, adds linter validation to warn about monetary properties lacking decimal format specifications, improves workspace dependency filtering in Cargo manifest generation, and includes formatting adjustments across templates and tests. Changes
Sequence DiagramsequenceDiagram
participant Gen as Stub Generator
participant Loader as Route/Spec Loader
participant Resolver as Reference Resolver
participant Extractor as Field Extractor
Gen->>Loader: Load routes
Gen->>Loader: Load OpenAPI spec
Loader-->>Gen: Return spec
loop For each route
Gen->>Resolver: Resolve $ref in response schema
alt Schema contains $ref
Resolver->>Resolver: Lookup component definition
Resolver-->>Gen: Return resolved schema
else Direct schema
Resolver-->>Gen: Return schema as-is
end
Gen->>Extractor: Extract response fields from resolved schema
Extractor-->>Gen: Return complete field set
Gen->>Gen: Generate stub code with fields
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fe7fdfba8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (name, spec) in &config.dependencies { | ||
| if use_workspace_deps && !workspace_deps.is_empty() && !workspace_deps.contains(name) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When generating inside a workspace that has [workspace.dependencies], this new filter skips any config dependency not listed there. That silently removes valid per-crate dependencies (e.g., a crates.io version specified in brrtrouter-dependencies.toml), so the generated Cargo.toml can miss crates that the generated code expects and fail to compile. Consider keeping those deps with their explicit spec (or at least surfacing an error) instead of continue-skipping them.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| for param in &route.parameters { | ||
| request_fields.push(parameter_to_field(param)); | ||
| } | ||
| let response_fields = route |
There was a problem hiding this comment.
Sync path missing $ref resolution for response schema
Medium Severity
The $ref resolution fix for response schemas was only applied to the non-sync path but not to the sync path. When using --sync mode, if the response schema is a bare $ref (like {"$ref": "#/components/schemas/Response"}), extract_fields will return an empty vector because a $ref object has no properties field. This causes synced stubs to lose their response fields.
Additional Locations (1)
| deps_config.as_ref(), | ||
| Some(&detected_conditional_deps), | ||
| )?; | ||
| } |
There was a problem hiding this comment.
Redundant conditional branch adds unnecessary complexity
Low Severity
The new conditional if deps_config.is_none() && version.is_none() creates two code paths that produce the same result. The write_cargo_toml function is just a thin wrapper that calls write_cargo_toml_with_options with default values. Additionally, when this branch executes, use_workspace_deps gets re-computed inside write_cargo_toml even though it was already computed at line 396-397.
![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
🤖 Fix all issues with AI agents
In `@src/generator/project/generate.rs`:
- Around line 682-697: The sync branch is still building response_fields from
the raw schema (producing empty fields for $ref); update the sync path to reuse
the already-parsed OpenApiV3Spec (the spec variable created from spec_path) and
the same $ref-resolution logic used for the non-sync branch when constructing
response_fields. Concretely, in the sync branch code that builds response_fields
(the branch handling --sync), replace the raw-schema inspection with the
resolver/path-following logic that uses spec (and any helper used elsewhere to
expand $ref) so $ref-resolved response fields are produced; apply the same
change to the other sync block later as mentioned (the second occurrence
building response_fields).
| // Load spec and routes | ||
| let spec_str = spec_path | ||
| .to_str() | ||
| .ok_or_else(|| anyhow::anyhow!("Invalid UTF-8 in spec path"))?; | ||
| let (routes, _slug) = load_spec(spec_str)?; | ||
|
|
||
| // Load OpenAPI spec again for $ref resolution when building response fields (ensures full Response in stubs) | ||
| let spec: OpenApiV3Spec = if spec_path | ||
| .extension() | ||
| .map(|e| e == "yaml" || e == "yml") | ||
| .unwrap_or(false) | ||
| { | ||
| serde_yaml::from_str(&std::fs::read_to_string(spec_path)?)? | ||
| } else { | ||
| serde_json::from_str(&std::fs::read_to_string(spec_path)?)? | ||
| }; |
There was a problem hiding this comment.
Sync mode still misses $ref-resolved response fields.
--sync still builds response_fields from the raw response schema, so $ref responses can produce empty fields and keep the E0063 error. Please reuse the new $ref resolution logic for the sync branch as well (around Line 773).
🛠️ Suggested fix for the sync path
- let response_fields = route
- .response_schema
- .as_ref()
- .map_or(vec![], extract_fields);
+ let response_schema_value: serde_json::Value = route
+ .response_schema
+ .as_ref()
+ .and_then(|v| {
+ if let Some(ref_path) = v.get("$ref").and_then(|r| r.as_str()) {
+ resolve_schema_ref(&spec, ref_path)
+ .and_then(|s| serde_json::to_value(s).ok())
+ } else {
+ Some(v.clone())
+ }
+ })
+ .unwrap_or_default();
+ let response_fields = extract_fields(&response_schema_value);Also applies to: 825-838
🤖 Prompt for AI Agents
In `@src/generator/project/generate.rs` around lines 682 - 697, The sync branch is
still building response_fields from the raw schema (producing empty fields for
$ref); update the sync path to reuse the already-parsed OpenApiV3Spec (the spec
variable created from spec_path) and the same $ref-resolution logic used for the
non-sync branch when constructing response_fields. Concretely, in the sync
branch code that builds response_fields (the branch handling --sync), replace
the raw-schema inspection with the resolver/path-following logic that uses spec
(and any helper used elsewhere to expand $ref) so $ref-resolved response fields
are produced; apply the same change to the other sync block later as mentioned
(the second occurrence building response_fields).
1 participant
Note
Medium Risk
Touches core code generation (impl stubs, Cargo.toml emission) and adds new lint warnings; low blast radius but could change generated output and CI expectations for existing specs.
Overview
Impl stub generation now resolves bare
$refresponse schemas beforeextract_fields, ensuring generatedResponse { ... }literals include all properties and don’t trigger missing-field compile errors. Theimpl_controller_stub.rs.txttemplate stops whitespace-stripping around the response field loop so each field renders on its own line (preventing a comment from swallowing subsequent fields).Project generation now writes a minimal
Cargo.tomlwhen neither--versionnor a deps config is provided, and when using workspace deps it filters configured dependencies to only those present in[workspace.dependencies]. The linter adds a new warning (money_type_without_decimal_format) for money-liketype: numberproperties missingformat: decimal|money, with accompanying tests.Written by Cursor Bugbot for commit 7fe7fdf. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.