feat: replace #[schema] with vendored JsonSchema derive#30
Merged
Conversation
Vendor schemars_derive into rovo-macros with the default crate path changed to ::rovo::schemars. This makes #[derive(JsonSchema)] work out of the box when imported from rovo — no helper attributes or special import patterns required. Remove the #[schema] attribute macro as it is no longer needed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #30 +/- ##
==========================================
- Coverage 92.44% 92.43% -0.01%
==========================================
Files 17 17
Lines 5519 5529 +10
==========================================
+ Hits 5102 5111 +9
- Misses 417 418 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReplaced the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Use a proxy struct approach: our JsonSchema derive creates a hidden copy of the type, applies schemars' original derive with the crate path set to ::rovo::schemars, then generates a thin delegation impl for the original type. This makes #[derive(JsonSchema)] work out of the box when imported from rovo — no helper attributes or special import patterns required. Remove the #[schema] attribute macro as it is no longer needed.
…to feat/auto-schema-derive # Conflicts: # rovo-macros/src/lib.rs # src/lib.rs
![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 (2)
rovo-macros/src/json_schema/attr/doc.rs (1)
1-5: Unused imports present.
format_ident,quote_spanned, andTokenare imported but not used in this file. While the parent module suppresses unused import warnings, cleaning these up would improve clarity.♻️ Suggested cleanup
use proc_macro2::TokenStream; use quote::TokenStreamExt; -use quote::{format_ident, quote, quote_spanned}; +use quote::quote; use syn::Attribute; -use syn::{parse_quote, Token}; +use syn::parse_quote;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rovo-macros/src/json_schema/attr/doc.rs` around lines 1 - 5, Remove the unused imports to clean up the module: delete format_ident, quote_spanned, and Token from the use statements (they are currently imported in the top of the file alongside TokenStream, TokenStreamExt, quote, and parse_quote) so only actually used symbols remain; update the use lines in doc.rs to import only proc_macro2::TokenStream, quote::TokenStreamExt, quote, and syn::{Attribute, parse_quote} (or equivalent) to eliminate the unused-imports while preserving existing functionality.rovo-macros/src/lib.rs (1)
19-27: Broad lint suppressions for vendored module.The lint suppressions are reasonable for vendored code from
schemars_derive. Consider adding a brief comment explaining these are suppressed because the code is vendored and minimally modified.📝 Suggested documentation
+// Vendored from schemars_derive - lint warnings suppressed to minimize divergence from upstream. #[allow( clippy::all, clippy::nursery, clippy::pedantic, missing_docs, rust_2018_idioms, unused_imports )] mod json_schema;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rovo-macros/src/lib.rs` around lines 19 - 27, Add a brief explanatory comment above the existing #[allow(...)] attribute for the vendored mod json_schema explaining that these broad lint suppressions are intentional because the code is vendored from schemars_derive and only minimally modified; update the comment to reference the vendored source (schemars_derive) and state that clippy/idiom warnings are suppressed to avoid noisy lints, leaving the #[allow(...)] attribute and mod json_schema unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rovo-macros/src/json_schema/attr/schemars_to_serde.rs`:
- Around line 76-87: The code currently keys override tracking by only the
top-level meta name (keyword) so a schemars attribute like
#[schemars(rename(serialize = "..."))] will suppress an unrelated serde nested
half (rename(deserialize=...)). Change the keying to include nested subkeys for
paired items (e.g., "rename.serialize" vs "rename.deserialize") instead of just
"rename": when iterating get_meta_items(attrs, "schemars", ctxt) inside the
block that builds serde_meta and schemars_meta_names, detect nested meta-list
entries (rename/rename_all with named sub-entries) and derive a more specific
identifier (use the top-level ident plus the nested ident like
"rename.serialize" or "rename_all.case") to store in schemars_meta_names; update
the same logic in the other similar block (lines ~94-103) so that only the exact
nested subkey suppresses the serde item, allowing the untouched half (e.g.,
deserialize) to be preserved and merged.
---
Nitpick comments:
In `@rovo-macros/src/json_schema/attr/doc.rs`:
- Around line 1-5: Remove the unused imports to clean up the module: delete
format_ident, quote_spanned, and Token from the use statements (they are
currently imported in the top of the file alongside TokenStream, TokenStreamExt,
quote, and parse_quote) so only actually used symbols remain; update the use
lines in doc.rs to import only proc_macro2::TokenStream, quote::TokenStreamExt,
quote, and syn::{Attribute, parse_quote} (or equivalent) to eliminate the
unused-imports while preserving existing functionality.
In `@rovo-macros/src/lib.rs`:
- Around line 19-27: Add a brief explanatory comment above the existing
#[allow(...)] attribute for the vendored mod json_schema explaining that these
broad lint suppressions are intentional because the code is vendored from
schemars_derive and only minimally modified; update the comment to reference the
vendored source (schemars_derive) and state that clippy/idiom warnings are
suppressed to avoid noisy lints, leaving the #[allow(...)] attribute and mod
json_schema unchanged.
🪄 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
Run ID: 3ccd311d-b9e6-41d6-ad02-788277d26a77
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/hello_world/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
examples/hello_world/src/main.rsrovo-macros/Cargo.tomlrovo-macros/src/json_schema/ast/from_serde.rsrovo-macros/src/json_schema/ast/mod.rsrovo-macros/src/json_schema/attr/doc.rsrovo-macros/src/json_schema/attr/mod.rsrovo-macros/src/json_schema/attr/parse_meta.rsrovo-macros/src/json_schema/attr/schemars_to_serde.rsrovo-macros/src/json_schema/attr/validation.rsrovo-macros/src/json_schema/idents.rsrovo-macros/src/json_schema/mod.rsrovo-macros/src/json_schema/schema_exprs.rsrovo-macros/src/lib.rssrc/lib.rstests/schema_macro.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rovo-macros/src/lib.rs (1)
495-513: Consider using syn's meta parsing for more robust attribute detection.The current token iteration approach works for the standard
#[schemars(crate = "...")]syntax, but it's slightly fragile—crateis a reserved keyword, and flat token iteration may not handle all edge cases (e.g., nested attribute structures).Using
syn::Attribute::parse_nested_metawould be more robust:let has_crate_attr = hidden.attrs.iter().any(|attr| { if !attr.path().is_ident("schemars") { return false; } let mut found = false; let _ = attr.parse_nested_meta(|meta| { if meta.path.is_ident("crate") { found = true; } Ok(()) }); found });That said, the current implementation should work fine for typical usage patterns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rovo-macros/src/lib.rs` around lines 495 - 513, The current has_crate_attr detection (iterating hidden.attrs and inspecting attr.meta tokens) is brittle; replace it by using syn's nested meta parsing: keep the attr.path().is_ident("schemars") check, then call attr.parse_nested_meta(|meta| { if meta.path.is_ident("crate") { found = true } Ok(()) }) (or equivalent using Attribute::parse_nested_meta) to set found and return it as has_crate_attr, and leave crate_attr construction (quote! { #[schemars(crate = "::rovo::schemars")] }) unchanged; update the has_crate_attr variable initialization to use this parse_nested_meta-based detection for robust attribute handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rovo-macros/src/lib.rs`:
- Around line 515-534: The delegation impl is missing the JsonSchema bound for
generic parameters, causing compilation when the hidden helper type requires
bounds; update the generated where clause used with
impl_generics/ty_generics/where_clause so it includes a predicate that the
hidden helper type implements ::rovo::schemars::JsonSchema (e.g. add a
where-clause entry like `#hidden_name `#ty_generics` :
::rovo::schemars::JsonSchema` or merge that predicate into the existing
where_clause) before emitting the impl for `::rovo::schemars::JsonSchema` for
`#original_name`, ensuring the delegation calls to `<#hidden_name `#ty_generics`
as ::rovo::schemars::JsonSchema>::...` compile for generic types.
---
Nitpick comments:
In `@rovo-macros/src/lib.rs`:
- Around line 495-513: The current has_crate_attr detection (iterating
hidden.attrs and inspecting attr.meta tokens) is brittle; replace it by using
syn's nested meta parsing: keep the attr.path().is_ident("schemars") check, then
call attr.parse_nested_meta(|meta| { if meta.path.is_ident("crate") { found =
true } Ok(()) }) (or equivalent using Attribute::parse_nested_meta) to set found
and return it as has_crate_attr, and leave crate_attr construction (quote! {
#[schemars(crate = "::rovo::schemars")] }) unchanged; update the has_crate_attr
variable initialization to use this parse_nested_meta-based detection for robust
attribute handling.
🪄 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
Run ID: 0b9f595f-46f0-42bd-9c5b-0b5784dabee2
📒 Files selected for processing (2)
rovo-macros/src/lib.rssrc/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib.rs
The proxy derive's delegation impl was missing a where clause requiring
the hidden type to implement JsonSchema. This caused compile errors for
generic types like `struct Wrapper<T> { inner: T }`.
Add a test covering generic types.
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.
Vendor schemars_derive into rovo-macros with the default crate path changed to ::rovo::schemars. This makes #[derive(JsonSchema)] work out of the box when imported from rovo — no helper attributes or special import patterns required.
Remove the #[schema] attribute macro as it is no longer needed.
Closes #28
Summary by CodeRabbit
Breaking Changes
New Features
Tests