Skip to content

Commit d3040c2

Browse files
committed
fix: Align schema compatibility with JSON Schema semantics
- compare const, enum, and numeric constraints by accepted values - handle exact mixed numeric equality and signed-zero bounds - apply the validator's default dialect to unevaluated properties
1 parent 6a8d744 commit d3040c2

10 files changed

Lines changed: 873 additions & 162 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ chrono = "0.4"
183183

184184
# JSON Schema validation
185185
jsonschema = { version = "0.40", default-features = false }
186+
# Exact comparison between differently typed numbers, as used by `jsonschema`.
187+
num-cmp = "0.1"
186188

187189
# JSON Schema generation
188190
schemars = { version = "1.2", features = ["uuid1"] }

gts-dylint/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gts-macros/src/lib.rs

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,6 +1587,15 @@ pub fn struct_to_gts_schema(attr: TokenStream, item: TokenStream) -> TokenStream
15871587
// `properties` declared in the same schema object, so closing a branch
15881588
// would reject the properties its sibling branches declare. Schemars
15891589
// already closes the branches of an externally tagged enum itself.
1590+
// * a `definitions` entry a combinator branch resolves to, because such an
1591+
// entry *is* the branch and closing it would reject the sibling branches'
1592+
// properties just the same. Reachability is computed first, over both the
1593+
// property subschemas and `definitions` itself, and is followed through
1594+
// chains of aliasing definitions (a top-level `$ref`). The granularity is
1595+
// the whole entry, so a definition used both as a combinator branch and as
1596+
// an ordinary property schema stays open everywhere: keeping the
1597+
// composition satisfiable wins over closing the ordinary use, which merely
1598+
// forfeits in-place evolution for that one level.
15901599
// * the generic extension field, which this macro replaces with a bare
15911600
// `{"type": "object"}` before this pass runs and which sec 4.4.1 requires
15921601
// to stay open so derived types can extend it.
@@ -1616,6 +1625,82 @@ pub fn struct_to_gts_schema(attr: TokenStream, item: TokenStream) -> TokenStream
16161625
];
16171626
const SCHEMA_LIST: &[&str] = &["allOf", "anyOf", "oneOf", "prefixItems"];
16181627

1628+
fn local_definition_name(reference: &str) -> Option<String> {
1629+
reference
1630+
.strip_prefix("#/definitions/")
1631+
.or_else(|| reference.strip_prefix("#/$defs/"))
1632+
.and_then(|name| name.split('/').next())
1633+
.map(|name| name.replace("~1", "/").replace("~0", "~"))
1634+
}
1635+
1636+
fn collect_combinator_definition_refs(
1637+
value: &serde_json::Value,
1638+
is_combinator_branch: bool,
1639+
referenced: &mut ::std::collections::HashSet<String>,
1640+
) {
1641+
let Some(object) = value.as_object() else {
1642+
return;
1643+
};
1644+
1645+
if is_combinator_branch
1646+
&& let Some(name) = object
1647+
.get("$ref")
1648+
.and_then(serde_json::Value::as_str)
1649+
.and_then(local_definition_name)
1650+
{
1651+
referenced.insert(name);
1652+
}
1653+
1654+
for (keyword, nested) in object {
1655+
let branch = COMBINATORS.contains(&keyword.as_str());
1656+
if SINGLE_SCHEMA.contains(&keyword.as_str()) {
1657+
collect_combinator_definition_refs(nested, branch, referenced);
1658+
} else if SCHEMA_MAP.contains(&keyword.as_str()) {
1659+
collect_combinator_definition_refs_map(nested, branch, referenced);
1660+
} else if SCHEMA_LIST.contains(&keyword.as_str()) {
1661+
collect_combinator_definition_refs_list(nested, branch, referenced);
1662+
} else if keyword == "items" {
1663+
if nested.is_array() {
1664+
collect_combinator_definition_refs_list(nested, branch, referenced);
1665+
} else {
1666+
collect_combinator_definition_refs(nested, branch, referenced);
1667+
}
1668+
}
1669+
}
1670+
}
1671+
1672+
fn collect_combinator_definition_refs_map(
1673+
value: &serde_json::Value,
1674+
is_combinator_branch: bool,
1675+
referenced: &mut ::std::collections::HashSet<String>,
1676+
) {
1677+
if let Some(object) = value.as_object() {
1678+
for nested in object.values() {
1679+
collect_combinator_definition_refs(
1680+
nested,
1681+
is_combinator_branch,
1682+
referenced,
1683+
);
1684+
}
1685+
}
1686+
}
1687+
1688+
fn collect_combinator_definition_refs_list(
1689+
value: &serde_json::Value,
1690+
is_combinator_branch: bool,
1691+
referenced: &mut ::std::collections::HashSet<String>,
1692+
) {
1693+
if let Some(values) = value.as_array() {
1694+
for nested in values {
1695+
collect_combinator_definition_refs(
1696+
nested,
1697+
is_combinator_branch,
1698+
referenced,
1699+
);
1700+
}
1701+
}
1702+
}
1703+
16191704
fn close_schema(value: &mut serde_json::Value, is_combinator_branch: bool) {
16201705
let Some(object) = value.as_object_mut() else {
16211706
return;
@@ -1674,9 +1759,51 @@ pub fn struct_to_gts_schema(attr: TokenStream, item: TokenStream) -> TokenStream
16741759
}
16751760
}
16761761

1762+
let mut combinator_definitions = ::std::collections::HashSet::new();
1763+
collect_combinator_definition_refs_map(
1764+
&properties,
1765+
false,
1766+
&mut combinator_definitions,
1767+
);
1768+
if let Some(definitions) = definitions.as_ref() {
1769+
collect_combinator_definition_refs_map(
1770+
definitions,
1771+
false,
1772+
&mut combinator_definitions,
1773+
);
1774+
}
1775+
1776+
// A definition whose top level is a bare `$ref` only aliases another
1777+
// one - Schemars emits that for a newtype struct carrying no doc
1778+
// comment - so the alias target is what actually contributes the
1779+
// branch's content model and has to stay open as well. Chase the
1780+
// alias chain to a fixed point; a name is enqueued only when it was
1781+
// newly inserted, so the walk terminates even on a `$ref` cycle.
1782+
let mut alias_queue: Vec<String> =
1783+
combinator_definitions.iter().cloned().collect();
1784+
while let Some(name) = alias_queue.pop() {
1785+
let alias = definitions
1786+
.as_ref()
1787+
.and_then(serde_json::Value::as_object)
1788+
.and_then(|object| object.get(&name))
1789+
.and_then(|definition| definition.get("$ref"))
1790+
.and_then(serde_json::Value::as_str)
1791+
.and_then(local_definition_name);
1792+
if let Some(alias) = alias
1793+
&& combinator_definitions.insert(alias.clone())
1794+
{
1795+
alias_queue.push(alias);
1796+
}
1797+
}
1798+
16771799
close_schema_map(&mut properties, false);
1678-
if let Some(definitions) = definitions.as_mut() {
1679-
close_schema_map(definitions, false);
1800+
if let Some(definitions_object) = definitions
1801+
.as_mut()
1802+
.and_then(serde_json::Value::as_object_mut)
1803+
{
1804+
for (name, definition) in definitions_object {
1805+
close_schema(definition, combinator_definitions.contains(name));
1806+
}
16801807
}
16811808
}
16821809
};

gts-macros/tests/inheritance_tests.rs

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,95 @@ pub struct SchemaWithNestedContactV1 {
9797
pub contact: NestedContact,
9898
}
9999

100+
fn composed_contact_schema(generator: &mut schemars::SchemaGenerator) -> schemars::Schema {
101+
let contact = generator.subschema_for::<NestedContact>();
102+
serde_json::from_value(serde_json::json!({
103+
"allOf": [
104+
contact,
105+
{
106+
"type": "object",
107+
"properties": {
108+
"label": {"type": "string"}
109+
},
110+
"required": ["label"]
111+
}
112+
]
113+
}))
114+
.expect("test schema")
115+
}
116+
117+
#[struct_to_gts_schema(
118+
dir_path = "schemas",
119+
base = true,
120+
type_id = gts_id!("x.test.nested.composed_definition.v1~"),
121+
description = "Schema composing a definition with sibling properties",
122+
properties = "schema_type,composed"
123+
)]
124+
#[derive(Debug)]
125+
pub struct SchemaWithComposedDefinitionV1 {
126+
#[serde(rename = "type")]
127+
pub schema_type: GtsTypeId,
128+
#[schemars(schema_with = "composed_contact_schema")]
129+
pub composed: serde_json::Value,
130+
}
131+
132+
// Newtype structs without a doc comment: Schemars emits each definition as a
133+
// bare `{"$ref": ...}` alias, so the combinator branch only reaches
134+
// `NestedContact` through two hops of aliasing.
135+
#[derive(Debug, serde::Serialize, serde::Deserialize, schemars::JsonSchema)]
136+
pub struct ContactAlias(pub NestedContact);
137+
138+
#[derive(Debug, serde::Serialize, serde::Deserialize, schemars::JsonSchema)]
139+
pub struct ContactAliasAlias(pub ContactAlias);
140+
141+
fn composed_alias_schema(generator: &mut schemars::SchemaGenerator) -> schemars::Schema {
142+
let alias = generator.subschema_for::<ContactAliasAlias>();
143+
serde_json::from_value(serde_json::json!({
144+
"allOf": [
145+
alias,
146+
{
147+
"type": "object",
148+
"properties": {
149+
"label": {"type": "string"}
150+
},
151+
"required": ["label"]
152+
}
153+
]
154+
}))
155+
.expect("test schema")
156+
}
157+
158+
#[struct_to_gts_schema(
159+
dir_path = "schemas",
160+
base = true,
161+
type_id = gts_id!("x.test.nested.aliased_definition.v1~"),
162+
description = "Schema composing an aliased definition with sibling properties",
163+
properties = "schema_type,composed"
164+
)]
165+
#[derive(Debug)]
166+
pub struct SchemaWithAliasedDefinitionV1 {
167+
#[serde(rename = "type")]
168+
pub schema_type: GtsTypeId,
169+
#[schemars(schema_with = "composed_alias_schema")]
170+
pub composed: serde_json::Value,
171+
}
172+
173+
#[struct_to_gts_schema(
174+
dir_path = "schemas",
175+
base = true,
176+
type_id = gts_id!("x.test.nested.shared_definition.v1~"),
177+
description = "Schema using one definition as a combinator branch and as a property",
178+
properties = "schema_type,composed,plain"
179+
)]
180+
#[derive(Debug)]
181+
pub struct SchemaWithSharedDefinitionV1 {
182+
#[serde(rename = "type")]
183+
pub schema_type: GtsTypeId,
184+
#[schemars(schema_with = "composed_contact_schema")]
185+
pub composed: serde_json::Value,
186+
pub plain: NestedContact,
187+
}
188+
100189
#[derive(Debug, serde::Serialize, serde::Deserialize, schemars::JsonSchema)]
101190
#[schemars(extend("additionalProperties" = true))]
102191
pub struct OpenExtensionPoint {
@@ -466,6 +555,107 @@ mod tests {
466555
jsonschema::validator_for(&schema).expect("emitted Draft-07 schema must compile");
467556
}
468557

558+
#[test]
559+
fn test_definition_referenced_by_combinator_branch_stays_open() {
560+
let schema = SchemaWithComposedDefinitionV1::gts_schema_with_refs();
561+
assert_eq!(
562+
schema.pointer("/properties/composed/allOf/0/$ref"),
563+
Some(&serde_json::json!("#/definitions/NestedContact"))
564+
);
565+
assert!(
566+
schema
567+
.pointer("/definitions/NestedContact/additionalProperties")
568+
.is_none(),
569+
"a definition composed with sibling properties must stay open:\n{}",
570+
serde_json::to_string_pretty(&schema).unwrap()
571+
);
572+
573+
let validator =
574+
jsonschema::validator_for(&schema).expect("emitted Draft-07 schema must compile");
575+
let instance = serde_json::json!({
576+
"type": "gts.x.test.nested.composed_definition.v1~",
577+
"composed": {
578+
"email": "dev@example.com",
579+
"label": "primary"
580+
}
581+
});
582+
assert!(
583+
validator.is_valid(&instance),
584+
"combinator siblings should not be rejected by a closed definition"
585+
);
586+
}
587+
588+
/// The branch may reach its definition through a chain of aliasing
589+
/// definitions, which Schemars emits for newtype structs.
590+
#[test]
591+
fn test_definition_aliased_by_combinator_branch_stays_open() {
592+
let schema = SchemaWithAliasedDefinitionV1::gts_schema_with_refs();
593+
assert_eq!(
594+
schema.pointer("/definitions/ContactAlias/$ref"),
595+
Some(&serde_json::json!("#/definitions/NestedContact")),
596+
"test relies on Schemars emitting a bare $ref alias:\n{}",
597+
serde_json::to_string_pretty(&schema).unwrap()
598+
);
599+
assert!(
600+
schema
601+
.pointer("/definitions/NestedContact/additionalProperties")
602+
.is_none(),
603+
"a definition an alias chain composes with sibling properties must stay open:\n{}",
604+
serde_json::to_string_pretty(&schema).unwrap()
605+
);
606+
607+
let validator =
608+
jsonschema::validator_for(&schema).expect("emitted Draft-07 schema must compile");
609+
let instance = serde_json::json!({
610+
"type": "gts.x.test.nested.aliased_definition.v1~",
611+
"composed": {
612+
"email": "dev@example.com",
613+
"label": "primary"
614+
}
615+
});
616+
assert!(
617+
validator.is_valid(&instance),
618+
"combinator siblings should not be rejected through an alias chain"
619+
);
620+
}
621+
622+
/// Reachability is tracked per `definitions` entry, not per use site, so one
623+
/// composed use keeps the entry open for its ordinary uses too. That trades
624+
/// the in-place evolvability of the ordinary level for a satisfiable
625+
/// composition - see the pass documentation in `gts-macros/src/lib.rs`.
626+
#[test]
627+
fn test_shared_definition_stays_open_for_its_ordinary_use() {
628+
let schema = SchemaWithSharedDefinitionV1::gts_schema_with_refs();
629+
assert_eq!(
630+
schema.pointer("/properties/plain/$ref"),
631+
Some(&serde_json::json!("#/definitions/NestedContact"))
632+
);
633+
assert!(
634+
schema
635+
.pointer("/definitions/NestedContact/additionalProperties")
636+
.is_none(),
637+
"a definition shared with a combinator branch must stay open:\n{}",
638+
serde_json::to_string_pretty(&schema).unwrap()
639+
);
640+
641+
let validator =
642+
jsonschema::validator_for(&schema).expect("emitted Draft-07 schema must compile");
643+
let instance = serde_json::json!({
644+
"type": "gts.x.test.nested.shared_definition.v1~",
645+
"composed": {
646+
"email": "dev@example.com",
647+
"label": "primary"
648+
},
649+
"plain": {
650+
"email": "ops@example.com"
651+
}
652+
});
653+
assert!(
654+
validator.is_valid(&instance),
655+
"both uses of the shared definition must still accept valid instances"
656+
);
657+
}
658+
469659
/// Nested object levels are closed so that a later definition of the type
470660
/// can add an optional property backward compatibly (gts-spec sec 4.4-4.5),
471661
/// while the levels where closing would be wrong are left alone.

gts/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ serde.workspace = true
2121
serde_json.workspace = true
2222
thiserror.workspace = true
2323
jsonschema.workspace = true
24+
num-cmp.workspace = true
2425
schemars.workspace = true
2526
walkdir.workspace = true
2627
tracing.workspace = true

0 commit comments

Comments
 (0)