Revise unnecessary_allof_wrapper rules to only cover $ref (#2049)

See: sourcemeta/jsonschema#536
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

Author: jviotti

src/extension/alterschema/CMakeLists.txt

@@ -50,9 +50,8 @@ sourcemeta_library(NAMESPACE sourcemeta PROJECT core NAME alterschema
     linter/content_schema_without_media_type.h
     linter/additional_items_with_schema_items.h
     linter/non_applicable_type_specific_keywords.h
-    linter/unnecessary_allof_wrapper_modern.h
-    linter/unnecessary_allof_wrapper_draft.h
-    linter/unnecessary_allof_wrapper_properties.h
+    linter/unnecessary_allof_ref_wrapper_modern.h
+    linter/unnecessary_allof_ref_wrapper_draft.h
     linter/duplicate_allof_branches.h
     linter/duplicate_anyof_branches.h
     linter/else_without_if.h

src/extension/alterschema/alterschema.cc

@@ -98,9 +98,8 @@ inline auto APPLIES_TO_POINTERS(std::vector<Pointer> &&keywords)
 #include "linter/unevaluated_items_default.h"
 #include "linter/unevaluated_properties_default.h"
 #include "linter/unknown_keywords_prefix.h"
-#include "linter/unnecessary_allof_wrapper_draft.h"
-#include "linter/unnecessary_allof_wrapper_modern.h"
-#include "linter/unnecessary_allof_wrapper_properties.h"
+#include "linter/unnecessary_allof_ref_wrapper_draft.h"
+#include "linter/unnecessary_allof_ref_wrapper_modern.h"
 #include "linter/unsatisfiable_max_contains.h"
 #include "linter/unsatisfiable_min_properties.h"
 
@@ -118,9 +117,8 @@ auto add(SchemaTransformer &bundle, const AlterSchemaMode mode) -> void {
   bundle.add<ContentSchemaWithoutMediaType>();
   bundle.add<DraftOfficialDialectWithoutEmptyFragment>();
   bundle.add<NonApplicableTypeSpecificKeywords>();
-  bundle.add<UnnecessaryAllOfWrapperModern>();
-  bundle.add<UnnecessaryAllOfWrapperDraft>();
-  bundle.add<UnnecessaryAllOfWrapperProperties>();
+  bundle.add<UnnecessaryAllOfRefWrapperModern>();
+  bundle.add<UnnecessaryAllOfRefWrapperDraft>();
   bundle.add<DuplicateAllOfBranches>();
   bundle.add<DuplicateAnyOfBranches>();
   bundle.add<ElseWithoutIf>();

src/extension/alterschema/linter/unnecessary_allof_ref_wrapper_draft.h

@@ -0,0 +1,42 @@
+class UnnecessaryAllOfRefWrapperDraft final : public SchemaTransformRule {
+public:
+  UnnecessaryAllOfRefWrapperDraft()
+      : SchemaTransformRule{"unnecessary_allof_ref_wrapper_draft",
+                            "Wrapping `$ref` in `allOf` is only necessary if "
+                            "there are other sibling keywords"} {};
+
+  [[nodiscard]] auto
+  condition(const sourcemeta::core::JSON &schema,
+            const sourcemeta::core::JSON &,
+            const sourcemeta::core::Vocabularies &vocabularies,
+            const sourcemeta::core::SchemaFrame &,
+            const sourcemeta::core::SchemaFrame::Location &,
+            const sourcemeta::core::SchemaWalker &,
+            const sourcemeta::core::SchemaResolver &) const
+      -> sourcemeta::core::SchemaTransformRule::Result override {
+    ONLY_CONTINUE_IF(contains_any(vocabularies,
+                                  {"http://json-schema.org/draft-07/schema#",
+                                   "http://json-schema.org/draft-06/schema#",
+                                   "http://json-schema.org/draft-04/schema#"}));
+    ONLY_CONTINUE_IF(schema.is_object() && schema.size() == 1 &&
+                     schema.defines("allOf") && schema.at("allOf").is_array());
+
+    const auto &all_of{schema.at("allOf")};
+
+    // In Draft 7 and older, `$ref` overrides sibling keywords, so we can only
+    // elevate it if it is the only keyword of the only branch, and the outer
+    // subschema only declares `allOf`
+    ONLY_CONTINUE_IF(all_of.size() == 1);
+    const auto &entry{all_of.at(0)};
+    ONLY_CONTINUE_IF(entry.is_object());
+    ONLY_CONTINUE_IF(entry.size() == 1 && entry.defines("$ref"));
+
+    return APPLIES_TO_POINTERS({{"allOf", 0, "$ref"}});
+  }
+
+  auto transform(JSON &schema, const Result &) const -> void override {
+    auto value{schema.at("allOf").at(0).at("$ref")};
+    schema.at("allOf").into(std::move(value));
+    schema.rename("allOf", "$ref");
+  }
+};

src/extension/alterschema/linter/unnecessary_allof_ref_wrapper_modern.h

@@ -0,0 +1,69 @@
+class UnnecessaryAllOfRefWrapperModern final : public SchemaTransformRule {
+public:
+  UnnecessaryAllOfRefWrapperModern()
+      : SchemaTransformRule{"unnecessary_allof_ref_wrapper_modern",
+                            "Wrapping `$ref` in `allOf` was only necessary in "
+                            "JSON Schema Draft 7 and older"} {};
+
+  [[nodiscard]] auto
+  condition(const sourcemeta::core::JSON &schema,
+            const sourcemeta::core::JSON &,
+            const sourcemeta::core::Vocabularies &vocabularies,
+            const sourcemeta::core::SchemaFrame &,
+            const sourcemeta::core::SchemaFrame::Location &,
+            const sourcemeta::core::SchemaWalker &,
+            const sourcemeta::core::SchemaResolver &) const
+      -> sourcemeta::core::SchemaTransformRule::Result override {
+    ONLY_CONTINUE_IF(contains_any(
+        vocabularies,
+        {"https://json-schema.org/draft/2020-12/vocab/applicator",
+         "https://json-schema.org/draft/2019-09/vocab/applicator"}));
+    ONLY_CONTINUE_IF(schema.is_object() && schema.defines("allOf") &&
+                     schema.at("allOf").is_array());
+
+    const auto &all_of{schema.at("allOf")};
+
+    // Don't do anything if there is more than one branch and ALL branches
+    // define `$ref` (a common multiple composition pattern)
+    ONLY_CONTINUE_IF(
+        !(all_of.size() > 1 &&
+          std::ranges::all_of(all_of.as_array(), [](const auto &entry) {
+            return entry.is_object() && entry.defines("$ref");
+          })));
+
+    std::vector<Pointer> locations;
+    for (std::size_t index = 0; index < all_of.size(); index++) {
+      const auto &entry{all_of.at(index)};
+      if (entry.is_object() && entry.defines("$ref") &&
+          // We cannot safely elevate a reference on a subschema with its own
+          // base URI
+          // TODO: In theory we can if the URI is absolute
+          !entry.defines("$id") && !schema.defines("$ref")) {
+        locations.push_back(Pointer{"allOf", index, "$ref"});
+      }
+    }
+
+    ONLY_CONTINUE_IF(!locations.empty());
+    return APPLIES_TO_POINTERS(std::move(locations));
+  }
+
+  auto transform(JSON &schema, const Result &result) const -> void override {
+    for (const auto &location : result.locations) {
+      assert(location.size() == 3);
+      const auto allof_index{location.at(1).to_index()};
+      const auto &keyword{location.at(2).to_property()};
+
+      if (!schema.defines(keyword)) {
+        schema.try_assign_before(
+            keyword, schema.at("allOf").at(allof_index).at(keyword), "allOf");
+        schema.at("allOf").at(allof_index).erase(keyword);
+      }
+    }
+
+    schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema);
+
+    if (schema.at("allOf").empty()) {
+      schema.erase("allOf");
+    }
+  }
+};