Returning more descriptive errors for invalid enum defaults

[oatmealdealer]

When trying to generate a REST client crate from an OpenAPI spec, I ran into this error output from progenitor:

gen fail: TypeError(InvalidValue)
Error: generation experienced errors

After tracing the error to typify I realized the schema had some properties with invalid defaults. Here's a basic example:

{
    "$schema": "http://json-schema.org/draft-07/schema#",
    "definitions": {
        "Foo": {
            "title": "Foo",
            "type": "object",
            "properties": {
                "bar": {
                    "title": "Bar",
                    "enum": [
                        "Foo",
                        "Bar"
                    ],
                    "type": "string",
                    "default": "Baz"
                }
            },
            "additionalProperties": false
        }
    }
}

It was this match arm that ended up being the one for my case:

typify/typify-impl/src/defaults.rs

Lines 151 to 153 in d36f65e

	EnumTagType::External => {
	validate_default_for_external_enum(type_space, variants, default)
	.ok_or_else(|| Error::invalid_value())

This is a slightly cleaned up version of the change that gave me enough info to find the problem with my schema file:

EnumTagType::External => validate_default_for_external_enum(
    type_space, variants, default,
)
.ok_or_else(|| Error::InvalidSchema {
    type_name: self.name().map(String::to_owned),
    reason: format!(
        "Invalid default {}, must be one of: {:?}",
        default,
        variants
            .iter()
            .map(|variant| variant.name.to_owned())
            .collect::<Vec<String>>()
    ),
}),

Ideally the error message could just be a JSON path like mentioned here: #485 (comment). I assume that would require a more structural refactor, but I'd be interested in pushing errors like this in the intended direction either way.

[ahl]

Thanks for the great analysis. Improved errors are a work in progress.

[oatmealdealer]

Thanks for the great analysis. Improved errors are a work in progress.

Thanks for the response - I hadn't seen #579 when I posted this, and I realize after the fact that "make the errors better," let alone "provide an exact JSON path with every validation error," is a way bigger ask than one might think at first glance.

I'm guessing the challenge is that typify is built on top of schemars which is built on top of serde, and serde hasn't solved this problem itself yet: serde-rs/json#173.

It also doesn't seem like the original structure of the JSON document is explicitly preserved in memory anywhere by the time typify gets a hold of it:

typify/typify-macro/src/lib.rs

Lines 232 to 239 in da943a0

	let root_schema: schemars::schema::RootSchema =
	serde_json::from_reader(std::fs::File::open(&path).map_err(|e| {
	syn::Error::new(
	schema.span(),
	format!("couldn't read file {}: {}", schema.value(), e),
	)
	})?)
	.unwrap();

I assume this means you'd have to extend serde to run typify's validations during deserialization while the path to a given value in the document is still available. On the other hand, from what I understand JSON schemas can also be self-referential in a lot of ways, so I'd expect at least some validations to be impossible until the schema has been fully deserialized.

I'd be interested in helping with a solution if you think it's a problem that would benefit from contribution, but I can definitely see why that might not be the case. This isn't blocking me from using progenitor for the purpose I had in mind though - so no worries if this is something that's just going to take time to figure out.