Include support for schemars #666
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #666 +/- ##
==========================================
+ Coverage 64.97% 65.26% +0.29%
==========================================
Files 36 38 +2
Lines 2187 2240 +53
==========================================
+ Hits 1421 1462 +41
- Misses 766 778 +12 ☔ View full report in Codecov by Sentry. |
|
Thank you for picking up that PR :) Overall, this already looks good. Some more tests would be good. I will need some more time to look at the PR, but I can't give you a timeline right now. If I see it correctly, the code is even supposed to handle conditional implementations of Do you have any suggestion how best to test the new schema implementations? There are two basic tests types I can think of: 1) Generate a schema for a type and verify that it is as expected (like the current test) and 2) generate a schema and serialize a value to JSON, then validate the JSON using the schema. |
|
No rush on the PR review :) I figured that this might take a while. I have added tests for the handling of conditional As for testing, I see three different kinds of tests that can be done. You've already got the first two
I would lean towards a mix of 2 and 3 when writing tests. For more complicated impls I think it should mostly be type 3 tests. I do think that it is reasonable to skip the test cases when the Ultimately the decision on what should have tests is up to you :) Now it looks like I need to figure out the right dependency version to add to get a |
I agree with this assessment. I think type 1 and 2 work for more basic implementations that basically just forward the definition to another type. You can quickly check that Your type 3 is good. I forgot that there are cases that do not delegate to another schema. For these more complex implementations, it makes sense to have positive and negative cases. So far, I have used
I would not totally skip tests for them, but they can be reduced. For example, you could have one test with If the current MSRV is making trouble for you, feel free to bump it. |
This commit adds support for emitting
#[schemars(with = "::serde_with::Schema<T, ...>")]
field annotations when any of the following are true:
- there is a #[derive(JsonSchema)] attribute, or,
- the user explicitly requests it via #[serde_as(schemars)]
This requires a bunch of extra work to properly scan for a few different
possible paths to JsonSchema (see the added docs for details) and also
to properly handle the case where the derive is within a #[cfg_attr]. eg
#[cfg_attr(feature = "blah", derive(schemars::JsonSchema))]
While rustc will evaluate the cfgs for attributes declared by derive
macros unfortunately it does not do the same for attribute macros so we
need to do it ourselves.
The code here started from jonasbb#623 as a base and all changes have been made
on top of that.
Co-Authored-By: Jonas Bushart <jonas@bushart.org>
This commit adds two main new types to the crate: - trait JsonSchemaFor<T> - this is the main trait needed to make things work. It is meant to be an analogue of the SerializeAs and DeserializeAs traits but for the JsonSchema trait. - struct Schema<T, TA> - this implements JsonSchema if TA: JsonSchemaFor<T>. It is automatically used by the #[serde_as] macro if it finds a #[derive(JsonSchema)] annotation. It then adds a bunch of JsonSchemaFor<T> impls which forward things through the relevant JsonSchema impl. By combining these two types we can use them to unwrap into the types making up the #[serde_as(as = "T")] annotation. As an example, suppose we have #[serde_as(as = "Option<Same>")] field: Option<u32> the type that will get emitted into the #[schemars(with = ...)] annotation is Schema<Option<u32>, Option<Same>>. To get the actual schema we use the following impls: impl JsonSchema for Schema<Option<u32>, Option<Same>> -> forwards to impl JsonSchemaFor<Option<u32>> for Option<Same> -> forwards to impl JsonSchema for Option<Schema<u32, Same>> -> which has its own custom impl This design has the unfortunate consequence that we need to do a lot of forwarding trait impls but I have been unable to figure out a design which automatically makes Schema<T, T> work for any T: JsonSchema while still allowing #[serde_as(as = "Option<Same>")] attributes to generate the correct schema.
These are enough to show the basic workings and to add some basic test cases verifying that everything fits together properly.
This tests that the various different use cases work properly and produce the expected schema.
Better to remove a public trait if it is not needed. This simplifies the public API at the expense of making the internal implementations a bit more verbose.
It was not being used by the test so there's no point in having it right now.
It has no members so there's no point in adding it to the public API.
Since this is done via a compile_fail I've needed to introduce the trybuild crate.
As it turns out, rustc's "trait unimplemented" error messages have not stayed perfectly stable across the last 10 versions. This is likely an issue that would keep happening with trybuild which would make it somewhat unsuitable. With some thinking I was able to get the same test coverage without requiring a compile error.
|
I've gone and added snapshot tests that cover most of the forwarding impls for std types. These are done through a macro so it should be pretty easy to write more snapshot tests in the future. The MSRV is no issue. Generating the lockfile was inconvenient since I had to use the latest rust nightly for the msrv-aware resolver but once it is generated then everything works as normal. |
I saw the existing PR for this over in #623 and figure that since I'm interested in using
schemarsandserde_withtogether then I might as well push this over the finish line.What's been kept the same as #623:
Schema<T, TA>JsonSchemaforSchema<T, TA>#[serde_as]macro can add#[schemars(with = "Schema<T, ...>")]attributes to fields annotated with#[serde_as(as = ...)]What's new here:
JsonSchemaimpls for various std wrapper types.JsonSchemaforSchema<T, Same>andSchema<T, DisplayFromStr>#[serde_as]will now automatically detect a#[derive(JsonSchema)]attribute and use that to determine if it should add#[schemars(with = ...)]to fields. This only happens if theschemars_0_8feature is enabled and can be overridden by doing#[serde_as(schemars = true/false)].I have a branch where I'm working to implement
JsonSchemafor all the types in this crate but figured it would be best to make multiple (still large) PRs instead of one enormous one.