Switch back to additionalProperties for simple cases

GREsau · Aug 22, 2024 · 82ca2db · 82ca2db
1 parent 2fad83b
commit 82ca2db
Show file tree

Hide file tree

Showing 23 changed files with 80 additions and 84 deletions.
diff --git a/schemars/src/_private.rs b/schemars/src/_private.rs
@@ -18,6 +18,8 @@ pub fn json_schema_for_flatten<T: ?Sized + JsonSchema>(
     }
 
     SemiRecursiveTransform(|schema: &mut Schema| {
+        // Always allow aditional/unevaluated properties, because the outer struct determines
+        // whether it denies unknown fields.
         if let Some(obj) = schema.as_object_mut() {
             if obj.get("additionalProperties").and_then(Value::as_bool) == Some(false) {
                 obj.remove("additionalProperties");
@@ -214,13 +216,6 @@ pub fn flatten(schema: &mut Schema, other: Schema) {
                             }
                         }
                     }
-                    "additionalProperties" | "unevaluatedProperties" => {
-                        // Even if an outer type has `deny_unknown_fields`, unknown fields
-                        // may be accepted by the flattened type
-                        if occupied.get() == &Value::Bool(false) {
-                            *occupied.into_mut() = value2;
-                        }
-                    }
                     "oneOf" | "anyOf" => {
                         // `OccupiedEntry` currently has no `.remove_entry()` method :(
                         let key = occupied.key().clone();
@@ -245,16 +240,49 @@ pub fn flatten(schema: &mut Schema, other: Schema) {
     match other.try_to_object() {
         Err(false) => {}
         Err(true) => {
-            schema
-                .ensure_object()
-                .insert("unevaluatedProperties".to_owned(), true.into());
+            if let Some(obj) = schema.as_object_mut() {
+                if !obj.contains_key("additionalProperties")
+                    && !obj.contains_key("unevaluatedProperties")
+                {
+                    let key = if contains_immediate_subschema(obj) {
+                        "unevaluatedProperties"
+                    } else {
+                        "additionalProperties"
+                    };
+                    obj.insert(key.to_owned(), true.into());
+                }
+            }
         }
-        Ok(obj2) => {
+        Ok(mut obj2) => {
             let obj1 = schema.ensure_object();
 
+            // For complex merges, replace `additionalProperties` with `unevaluatedProperties`
+            // which usually "works out better".
+            normalise_additional_unevaluated_properties(obj1, &obj2);
+            normalise_additional_unevaluated_properties(&mut obj2, obj1);
+
             for (key, value2) in obj2 {
                 flatten_property(obj1, key, value2);
             }
         }
     }
 }
+
+fn normalise_additional_unevaluated_properties(
+    schema_obj1: &mut Map<String, Value>,
+    schema_obj2: &Map<String, Value>,
+) {
+    if schema_obj1.contains_key("additionalProperties")
+        && (schema_obj2.contains_key("unevaluatedProperties")
+            || contains_immediate_subschema(schema_obj2))
+    {
+        let ap = schema_obj1.remove("additionalProperties");
+        schema_obj1.insert("unevaluatedProperties".to_owned(), ap.into());
+    }
+}
+
+fn contains_immediate_subschema(schema_obj: &Map<String, Value>) -> bool {
+    ["if", "then", "else", "allOf", "anyOf", "oneOf", "$ref"]
+        .into_iter()
+        .any(|k| schema_obj.contains_key(k))
+}
diff --git a/schemars/src/json_schema_impls/maps.rs b/schemars/src/json_schema_impls/maps.rs
@@ -21,7 +21,7 @@ macro_rules! map_impl {
             fn json_schema(generator: &mut SchemaGenerator) -> Schema {
                 json_schema!({
                     "type": "object",
-                    "unevaluatedProperties": generator.subschema_for::<V>(),
+                    "additionalProperties": generator.subschema_for::<V>(),
                 })
             }
         }

diff --git a/schemars/tests/enum_deny_unknown_fields.rs b/schemars/tests/enum_deny_unknown_fields.rs
@@ -17,8 +17,8 @@ struct Struct {
     bar: bool,
 }
 
-// Outer container should always have unevaluatedProperties: false
-// `Struct` variant should have unevaluatedProperties: false
+// Outer container should always have additionalProperties: false
+// `Struct` variant should have additionalProperties: false
 #[allow(dead_code)]
 #[derive(JsonSchema)]
 #[schemars(rename_all = "camelCase", deny_unknown_fields)]
@@ -42,7 +42,7 @@ fn enum_external_tag() -> TestResult {
     test_default_generated_schema::<External>("enum-external-duf")
 }
 
-// Only `Struct` variant should have unevaluatedProperties: false
+// Only `Struct` variant should have additionalProperties: false
 #[allow(dead_code)]
 #[derive(JsonSchema)]
 #[schemars(tag = "typeProperty", deny_unknown_fields)]
@@ -65,7 +65,7 @@ fn enum_internal_tag() -> TestResult {
     test_default_generated_schema::<Internal>("enum-internal-duf")
 }
 
-// Only `Struct` variant should have unevaluatedProperties: false
+// Only `Struct` variant should have additionalProperties: false
 #[allow(dead_code)]
 #[derive(JsonSchema)]
 #[schemars(untagged, deny_unknown_fields)]
@@ -88,7 +88,7 @@ fn enum_untagged() -> TestResult {
     test_default_generated_schema::<Untagged>("enum-untagged-duf")
 }
 
-// Outer container and `Struct` variant should have unevaluatedProperties: false
+// Outer container and `Struct` variant should have additionalProperties: false
 #[allow(dead_code)]
 #[derive(JsonSchema)]
 #[schemars(tag = "t", content = "c", deny_unknown_fields)]

diff --git a/schemars/tests/expected/enum-adjacent-tagged-duf.json b/schemars/tests/expected/enum-adjacent-tagged-duf.json
@@ -15,7 +15,7 @@
       "required": [
         "t"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -28,7 +28,7 @@
         },
         "c": {
           "type": "object",
-          "unevaluatedProperties": {
+          "additionalProperties": {
             "type": "string"
           }
         }
@@ -37,7 +37,7 @@
         "t",
         "c"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -56,7 +56,7 @@
         "t",
         "c"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -75,7 +75,7 @@
         "t",
         "c"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -97,7 +97,7 @@
               "type": "boolean"
             }
           },
-          "unevaluatedProperties": false,
+          "additionalProperties": false,
           "required": [
             "foo",
             "bar"
@@ -108,7 +108,7 @@
         "t",
         "c"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -138,7 +138,7 @@
         "t",
         "c"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -153,7 +153,7 @@
       "required": [
         "t"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     },
     {
       "type": "object",
@@ -173,7 +173,7 @@
         "t",
         "c"
       ],
-      "unevaluatedProperties": false
+      "additionalProperties": false
     }
   ],
   "$defs": {

diff --git a/schemars/tests/expected/enum-adjacent-tagged.json b/schemars/tests/expected/enum-adjacent-tagged.json
@@ -27,7 +27,7 @@
         },
         "c": {
           "type": "object",
-          "unevaluatedProperties": {
+          "additionalProperties": {
             "type": "string"
           }
         }

diff --git a/schemars/tests/expected/enum-external-duf.json b/schemars/tests/expected/enum-external-duf.json
@@ -14,7 +14,7 @@
       "properties": {
         "stringMap": {
           "type": "object",
-          "unevaluatedProperties": {
+          "additionalProperties": {
             "type": "string"
           }
         }
@@ -62,7 +62,7 @@
               "type": "boolean"
             }
           },
-          "unevaluatedProperties": false,
+          "additionalProperties": false,
           "required": [
             "foo",
             "bar"

diff --git a/schemars/tests/expected/enum-external.json b/schemars/tests/expected/enum-external.json
@@ -14,7 +14,7 @@
       "properties": {
         "stringMap": {
           "type": "object",
-          "unevaluatedProperties": {
+          "additionalProperties": {
             "type": "string"
           }
         }

diff --git a/schemars/tests/expected/enum-internal-duf.json b/schemars/tests/expected/enum-internal-duf.json
@@ -23,7 +23,7 @@
           "const": "StringMap"
         }
       },
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "string"
       },
       "required": [
@@ -79,7 +79,7 @@
           "const": "Struct"
         }
       },
-      "unevaluatedProperties": false,
+      "additionalProperties": false,
       "required": [
         "typeProperty",
         "foo",

diff --git a/schemars/tests/expected/enum-internal.json b/schemars/tests/expected/enum-internal.json
@@ -22,7 +22,7 @@
           "const": "StringMap"
         }
       },
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "string"
       },
       "required": [

diff --git a/schemars/tests/expected/enum-untagged-duf.json b/schemars/tests/expected/enum-untagged-duf.json
@@ -7,7 +7,7 @@
     },
     {
       "type": "object",
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "string"
       }
     },
@@ -28,7 +28,7 @@
           "type": "boolean"
         }
       },
-      "unevaluatedProperties": false,
+      "additionalProperties": false,
       "required": [
         "foo",
         "bar"

diff --git a/schemars/tests/expected/enum-untagged.json b/schemars/tests/expected/enum-untagged.json
@@ -7,7 +7,7 @@
     },
     {
       "type": "object",
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "string"
       }
     },

diff --git a/schemars/tests/expected/flattened_value.json b/schemars/tests/expected/flattened_value.json
@@ -10,5 +10,5 @@
   "required": [
     "flag"
   ],
-  "unevaluatedProperties": true
+  "additionalProperties": true
 }
diff --git a/schemars/tests/expected/indexmap.json b/schemars/tests/expected/indexmap.json
@@ -5,7 +5,7 @@
   "properties": {
     "map": {
       "type": "object",
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "boolean"
       }
     },

diff --git a/schemars/tests/expected/remote_derive_generic.json b/schemars/tests/expected/remote_derive_generic.json
@@ -14,7 +14,7 @@
     },
     "fake_map": {
       "type": "object",
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "array",
         "uniqueItems": true,
         "items": {

diff --git a/schemars/tests/expected/schema_settings-2019_09.json b/schemars/tests/expected/schema_settings-2019_09.json
@@ -13,7 +13,7 @@
     },
     "values": {
       "type": "object",
-      "unevaluatedProperties": true
+      "additionalProperties": true
     },
     "value": true,
     "inner": {

diff --git a/schemars/tests/expected/schema_settings-2020_12.json b/schemars/tests/expected/schema_settings-2020_12.json
@@ -13,7 +13,7 @@
     },
     "values": {
       "type": "object",
-      "unevaluatedProperties": true
+      "additionalProperties": true
     },
     "value": true,
     "inner": {

diff --git a/schemars/tests/expected/struct-normal-additional-properties.json b/schemars/tests/expected/struct-normal-additional-properties.json
@@ -17,7 +17,7 @@
       ]
     }
   },
-  "unevaluatedProperties": false,
+  "additionalProperties": false,
   "required": [
     "foo",
     "bar"

diff --git a/schemars/tests/expected/test_flattened_struct_deny_unknown_fields.json b/schemars/tests/expected/test_flattened_struct_deny_unknown_fields.json
@@ -42,7 +42,7 @@
           "type": "boolean"
         }
       },
-      "unevaluatedProperties": false,
+      "additionalProperties": false,
       "required": [
         "middle_field",
         "inner_field"

diff --git a/schemars/tests/expected/validate.json b/schemars/tests/expected/validate.json
@@ -68,7 +68,7 @@
     },
     "map_contains": {
       "type": "object",
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "null"
       },
       "required": [

diff --git a/schemars/tests/expected/validate_schemars_attrs.json b/schemars/tests/expected/validate_schemars_attrs.json
@@ -68,7 +68,7 @@
     },
     "map_contains": {
       "type": "object",
-      "unevaluatedProperties": {
+      "additionalProperties": {
         "type": "null"
       },
       "required": [
-Original file line number
+Diff line change
@@ Expand Up / @@ -17,7 +17,7 @@ @@
           ]
         }
       },
-      "unevaluatedProperties": false,
+      "additionalProperties": false,
       "required": [
         "foo",
         "bar"
@@ Expand Down @@