Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions typify-impl/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1461,6 +1461,15 @@ impl TypeSpace {
metadata: &'a Option<Box<Metadata>>,
subschemas: &'a [Schema],
) -> Result<(TypeEntry, &'a Option<Box<Metadata>>)> {
// An empty `anyOf` cannot match any instance.
if subschemas.is_empty() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think an empty anyOf is a valid input for what it's worth.

Is this related to the fix?

let type_name = match get_type_name(&type_name, metadata) {
Some(name) => Name::Required(name),
None => type_name,
};
return self.convert_never(type_name, original_schema);
}

// Rust can emit "anyOf":[{"$ref":"#/definitions/C"},{"type":"null"}
// for Option. We match this here because the mutual exclusion check
// below may fail for cases such as Option<T> where T is defined to be,
Expand Down Expand Up @@ -2294,6 +2303,26 @@ mod tests {
}
}

#[test]
fn test_empty_any_of() {
let schema_json = r#"
{
"title": "EmptyAnyOf",
"anyOf": []
}
"#;

let schema: RootSchema = serde_json::from_str(schema_json).unwrap();

let mut type_space = TypeSpace::default();
let type_id = type_space.add_type(&schema.schema.into()).unwrap();

match &type_space.id_to_entry[&type_id].details {
super::TypeEntryDetails::Enum(details) => assert!(details.variants.is_empty()),
details => panic!("empty anyOf should be uninhabited, got {details:?}"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "uninhabited" is a term we use typically. Usually something like "unsatisfiable"

}
}

#[test]
fn test_overridden_conversion() {
let schema_json = r#"
Expand Down
19 changes: 18 additions & 1 deletion typify-impl/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ pub(crate) fn all_mutually_exclusive(
definitions: &BTreeMap<RefKey, Schema>,
) -> bool {
let len = subschemas.len();
// With fewer than two subschemas there are no pairs to compare, so the
// schemas are vacuously mutually exclusive. This also avoids underflow
// in `len - 1` below when `subschemas` is empty.
Comment on lines +51 to +53

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// With fewer than two subschemas there are no pairs to compare, so the
// schemas are vacuously mutually exclusive. This also avoids underflow
// in `len - 1` below when `subschemas` is empty.
// With fewer than two subschemas, this is a degenerate case where the lone schema is mutually exclusive with everything else (which happens to be nothing).

And please line wrap

if len < 2 {
return true;
}
// Consider all pairs
(0..len - 1)
.flat_map(|ii| (ii + 1..len).map(move |jj| (ii, jj)))
Expand Down Expand Up @@ -1001,7 +1007,10 @@ mod tests {
};

use crate::{
util::{decode_segment, sanitize, schemas_mutually_exclusive, Case, ReorderedInstanceType},
util::{
all_mutually_exclusive, decode_segment, sanitize, schemas_mutually_exclusive, Case,
ReorderedInstanceType,
},
Name,
};

Expand Down Expand Up @@ -1107,6 +1116,14 @@ mod tests {
assert!(schemas_mutually_exclusive(&b, &a, &BTreeMap::new()));
}

#[test]
fn test_all_mutually_exclusive_empty() {
// An empty slice of subschemas has no pairs to compare, so it's
// vacuously true. This should not panic (previously `len - 1`
// underflowed for an empty slice).
Comment on lines +1121 to +1123

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// An empty slice of subschemas has no pairs to compare, so it's
// vacuously true. This should not panic (previously `len - 1`
// underflowed for an empty slice).

assert!(all_mutually_exclusive(&[], &BTreeMap::new()));
}

#[test]
fn test_decode_segment() {
assert_eq!(decode_segment("foo~1bar"), "foo/bar");
Expand Down