De-duplicate anyOf/allOf

Signed-off-by: Juan Cruz Viotti <[email protected]>

DEPENDENCIES

@@ -1,4 +1,4 @@
 vendorpull https://github.com/sourcemeta/vendorpull dea311b5bfb53b6926a4140267959ae334d3ecf4
 noa https://github.com/sourcemeta/noa 7e26abce7a4e31e86a16ef2851702a56773ca527
-jsontoolkit https://github.com/sourcemeta/jsontoolkit 6c78f131029c647be79cc5da5874fa8d50415cde
+jsontoolkit https://github.com/sourcemeta/jsontoolkit a0aafd3f62c03514db9b71b1c89f2ed69669b130
 googletest https://github.com/google/googletest a7f443b80b105f940225332ed3c31f2790092f47

src/linter/CMakeLists.txt

@@ -20,6 +20,8 @@ noa_library(NAMESPACE sourcemeta PROJECT alterschema NAME linter
     redundant/content_media_type_without_encoding.h
     redundant/content_schema_default.h
     redundant/content_schema_without_media_type.h
+    redundant/duplicate_allof_branches.h
+    redundant/duplicate_anyof_branches.h
     redundant/else_without_if.h
     redundant/items_array_default.h
     redundant/items_schema_default.h

src/linter/linter.cc

@@ -3,8 +3,8 @@
 #include <cassert> // assert
 
 // For built-in rules
-#include <algorithm> // std::any_of
-#include <iterator>  // std::cbegin, std::cend
+#include <algorithm>
+#include <iterator>
 namespace sourcemeta::alterschema {
 template <typename T>
 auto contains_any(const T &container, const T &values) -> bool {
@@ -29,6 +29,8 @@ auto contains_any(const T &container, const T &values) -> bool {
 #include "redundant/content_media_type_without_encoding.h"
 #include "redundant/content_schema_default.h"
 #include "redundant/content_schema_without_media_type.h"
+#include "redundant/duplicate_allof_branches.h"
+#include "redundant/duplicate_anyof_branches.h"
 #include "redundant/else_without_if.h"
 #include "redundant/items_array_default.h"
 #include "redundant/items_schema_default.h"
@@ -62,6 +64,8 @@ auto add(Bundle &bundle, const LinterCategory category) -> void {
       bundle.add<ContentMediaTypeWithoutEncoding>();
       bundle.add<ContentSchemaDefault>();
       bundle.add<ContentSchemaWithoutMediaType>();
+      bundle.add<DuplicateAllOfBranches>();
+      bundle.add<DuplicateAnyOfBranches>();
       bundle.add<ElseWithoutIf>();
       bundle.add<ItemsArrayDefault>();
       bundle.add<ItemsSchemaDefault>();

src/linter/redundant/duplicate_allof_branches.h

@@ -0,0 +1,33 @@
+class DuplicateAllOfBranches final : public Rule {
+public:
+  DuplicateAllOfBranches()
+      : Rule{"duplicate_allof_branches",
+             "Setting duplicate subschemas in `allOf` is redundant, as it "
+             "produces "
+             "unnecessary additional validation that is guaranteed to not "
+             "affect the validation result"} {};
+
+  [[nodiscard]] auto
+  condition(const sourcemeta::jsontoolkit::JSON &schema, const std::string &,
+            const std::set<std::string> &vocabularies,
+            const sourcemeta::jsontoolkit::Pointer &) const -> bool override {
+    return contains_any(
+               vocabularies,
+               {"https://json-schema.org/draft/2020-12/vocab/applicator",
+                "https://json-schema.org/draft/2019-09/vocab/applicator",
+                "http://json-schema.org/draft-07/schema#",
+                "http://json-schema.org/draft-06/schema#",
+                "http://json-schema.org/draft-04/schema#"}) &&
+           schema.is_object() && schema.defines("allOf") &&
+           schema.at("allOf").is_array() && !schema.at("allOf").unique();
+  }
+
+  auto transform(Transformer &transformer) const -> void override {
+    auto collection = transformer.schema().at("allOf");
+    std::sort(collection.as_array().begin(), collection.as_array().end());
+    auto last =
+        std::unique(collection.as_array().begin(), collection.as_array().end());
+    collection.erase(last, collection.as_array().end());
+    transformer.replace({"allOf"}, std::move(collection));
+  }
+};

src/linter/redundant/duplicate_anyof_branches.h

@@ -0,0 +1,33 @@
+class DuplicateAnyOfBranches final : public Rule {
+public:
+  DuplicateAnyOfBranches()
+      : Rule{"duplicate_anyof_branches",
+             "Setting duplicate subschemas in `anyOf` is redundant, as it "
+             "produces "
+             "unnecessary additional validation that is guaranteed to not "
+             "affect the validation result"} {};
+
+  [[nodiscard]] auto
+  condition(const sourcemeta::jsontoolkit::JSON &schema, const std::string &,
+            const std::set<std::string> &vocabularies,
+            const sourcemeta::jsontoolkit::Pointer &) const -> bool override {
+    return contains_any(
+               vocabularies,
+               {"https://json-schema.org/draft/2020-12/vocab/applicator",
+                "https://json-schema.org/draft/2019-09/vocab/applicator",
+                "http://json-schema.org/draft-07/schema#",
+                "http://json-schema.org/draft-06/schema#",
+                "http://json-schema.org/draft-04/schema#"}) &&
+           schema.is_object() && schema.defines("anyOf") &&
+           schema.at("anyOf").is_array() && !schema.at("anyOf").unique();
+  }
+
+  auto transform(Transformer &transformer) const -> void override {
+    auto collection = transformer.schema().at("anyOf");
+    std::sort(collection.as_array().begin(), collection.as_array().end());
+    auto last =
+        std::unique(collection.as_array().begin(), collection.as_array().end());
+    collection.erase(last, collection.as_array().end());
+    transformer.replace({"anyOf"}, std::move(collection));
+  }
+};

test/linter/2019_09_test.cc

@@ -700,3 +700,39 @@ TEST(Lint_2019_09, exclusive_minimum_number_and_minimum_3) {
 
   EXPECT_EQ(document, expected);
 }
+
+TEST(Lint_2019_09, duplicate_allof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2019-09/schema",
+    "allOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2019-09/schema",
+    "allOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}
+
+TEST(Lint_2019_09, duplicate_anyof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2019-09/schema",
+    "anyOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2019-09/schema",
+    "anyOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}

test/linter/2020_12_test.cc

@@ -683,3 +683,39 @@ TEST(Lint_2020_12, exclusive_minimum_number_and_minimum_3) {
 
   EXPECT_EQ(document, expected);
 }
+
+TEST(Lint_2020_12, duplicate_allof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2020-12/schema",
+    "allOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2020-12/schema",
+    "allOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}
+
+TEST(Lint_2020_12, duplicate_anyof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2020-12/schema",
+    "anyOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "https://json-schema.org/draft/2020-12/schema",
+    "anyOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}

test/linter/draft4_test.cc

@@ -197,3 +197,39 @@ TEST(Lint_draft4, duplicate_required_values_1) {
 
   EXPECT_EQ(document, expected);
 }
+
+TEST(Lint_draft4, duplicate_allof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "allOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "allOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}
+
+TEST(Lint_draft4, duplicate_anyof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "anyOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "anyOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}

test/linter/draft6_test.cc

@@ -348,3 +348,39 @@ TEST(Lint_draft6, exclusive_minimum_number_and_minimum_3) {
 
   EXPECT_EQ(document, expected);
 }
+
+TEST(Lint_draft6, duplicate_allof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-06/schema#",
+    "allOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-06/schema#",
+    "allOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}
+
+TEST(Lint_draft6, duplicate_anyof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-06/schema#",
+    "anyOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-06/schema#",
+    "anyOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}

test/linter/draft7_test.cc

@@ -439,3 +439,39 @@ TEST(Lint_draft7, exclusive_minimum_number_and_minimum_3) {
 
   EXPECT_EQ(document, expected);
 }
+
+TEST(Lint_draft7, duplicate_allof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "allOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "allOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}
+
+TEST(Lint_draft7, duplicate_anyof_branches_1) {
+  sourcemeta::jsontoolkit::JSON document =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "anyOf": [ { "type": "string" }, { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  LINT_AND_FIX(document);
+
+  const sourcemeta::jsontoolkit::JSON expected =
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-07/schema#",
+    "anyOf": [ { "type": "integer" }, { "type": "string" } ]
+  })JSON");
+
+  EXPECT_EQ(document, expected);
+}