diff --git a/src/extension/alterschema/CMakeLists.txt b/src/extension/alterschema/CMakeLists.txt index 516f81304..31bffe794 100644 --- a/src/extension/alterschema/CMakeLists.txt +++ b/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 diff --git a/src/extension/alterschema/alterschema.cc b/src/extension/alterschema/alterschema.cc index 34010fa0d..9619120c5 100644 --- a/src/extension/alterschema/alterschema.cc +++ b/src/extension/alterschema/alterschema.cc @@ -98,9 +98,8 @@ inline auto APPLIES_TO_POINTERS(std::vector &&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(); bundle.add(); bundle.add(); - bundle.add(); - bundle.add(); - bundle.add(); + bundle.add(); + bundle.add(); bundle.add(); bundle.add(); bundle.add(); diff --git a/src/extension/alterschema/linter/unnecessary_allof_ref_wrapper_draft.h b/src/extension/alterschema/linter/unnecessary_allof_ref_wrapper_draft.h new file mode 100644 index 000000000..ee49de0ec --- /dev/null +++ b/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"); + } +}; diff --git a/src/extension/alterschema/linter/unnecessary_allof_ref_wrapper_modern.h b/src/extension/alterschema/linter/unnecessary_allof_ref_wrapper_modern.h new file mode 100644 index 000000000..3d72a1906 --- /dev/null +++ b/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 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"); + } + } +}; diff --git a/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h b/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h deleted file mode 100644 index 20b8ddeeb..000000000 --- a/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h +++ /dev/null @@ -1,96 +0,0 @@ -class UnnecessaryAllOfWrapperDraft final : public SchemaTransformRule { -public: - UnnecessaryAllOfWrapperDraft() - : SchemaTransformRule{ - "unnecessary_allof_wrapper_draft", - "Wrapping keywords other than `$ref` in `allOf` " - "is often unnecessary and may even introduce a minor " - "evaluation performance overhead"} {}; - - [[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.defines("allOf") && - schema.at("allOf").is_array()); - - std::vector locations; - const auto &all_of{schema.at("allOf")}; - bool same_keywords{all_of.size() > 1}; - for (std::size_t index = 0; index < all_of.size(); index++) { - const auto &entry{all_of.at(index)}; - if (!entry.is_object()) { - same_keywords = false; - continue; - } - - // It is dangerous to extract type-specific keywords from a schema that - // declares a type into another schema that also declares a type if - // the types are different. As we might lead to those type-keywords - // getting incorrectly removed if they don't apply to the target type - if (schema.defines("type") && entry.defines("type") && - // TODO: Ideally we also check for intersection of types in type - // arrays or whether one is contained in the other - schema.at("type") != entry.at("type")) { - same_keywords = false; - continue; - } - - for (const auto &subentry : entry.as_object()) { - // If we have any new keyword in a branch after the first, - // then we probably need to collapse - if (index > 0 && same_keywords) { - const auto &previous{all_of.at(index - 1)}; - if (previous.is_object()) { - for (const auto &other : previous.as_object()) { - if (!entry.defines(other.first)) { - same_keywords = false; - break; - } - } - } - } - - if (subentry.first != "$ref" && !schema.defines(subentry.first)) { - locations.push_back(Pointer{"allOf", index, subentry.first}); - } - } - } - - ONLY_CONTINUE_IF(!locations.empty() && !same_keywords); - return APPLIES_TO_POINTERS(std::move(locations)); - } - - auto transform(JSON &schema, const Result &) const -> void override { - for (auto &entry : schema.at("allOf").as_array()) { - if (entry.is_object()) { - std::vector blacklist; - for (auto &subentry : entry.as_object()) { - if (subentry.first != "$ref" && !schema.defines(subentry.first)) { - blacklist.push_back(subentry.first); - } - } - - for (auto &property : blacklist) { - schema.try_assign_before(property, entry.at(property), "allOf"); - entry.erase(property); - } - } - } - - schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema); - - if (schema.at("allOf").empty()) { - schema.erase("allOf"); - } - } -}; diff --git a/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h b/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h deleted file mode 100644 index 8eeb05594..000000000 --- a/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h +++ /dev/null @@ -1,103 +0,0 @@ -class UnnecessaryAllOfWrapperModern final : public SchemaTransformRule { -public: - UnnecessaryAllOfWrapperModern() - : SchemaTransformRule{ - "unnecessary_allof_wrapper_modern", - "Wrapping keywords in `allOf` is often unnecessary and may even " - "introduce a minor evaluation performance overhead"} {}; - - [[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 has_validation{contains_any( - vocabularies, - {"https://json-schema.org/draft/2020-12/vocab/validation", - "https://json-schema.org/draft/2019-09/vocab/validation"})}; - - std::vector locations; - const auto &all_of{schema.at("allOf")}; - std::unordered_set keywords; - bool same_keywords{all_of.size() > 1}; - for (std::size_t index = 0; index < all_of.size(); index++) { - const auto &entry{all_of.at(index)}; - if (!entry.is_object()) { - same_keywords = false; - continue; - } - - // It is dangerous to extract type-specific keywords from a schema that - // declares a type into another schema that also declares a type if - // the types are different. As we might lead to those type-keywords - // getting incorrectly removed if they don't apply to the target type - if (has_validation && schema.defines("type") && entry.defines("type") && - // TODO: Ideally we also check for intersection of types in type - // arrays or whether one is contained in the other - schema.at("type") != entry.at("type")) { - same_keywords = false; - continue; - } - - for (const auto &subentry : entry.as_object()) { - // If we have any new keyword in a branch after the first, - // then we probably need to collapse - if (index > 0 && same_keywords) { - const auto &previous{all_of.at(index - 1)}; - if (previous.is_object()) { - for (const auto &other : previous.as_object()) { - if (!entry.defines(other.first)) { - same_keywords = false; - break; - } - } - } - } - - // TODO: Have another rule that removes a keyword if its exactly - // equal to an instance of the same keyword outside the wrapper - if (!schema.defines(subentry.first)) { - locations.push_back(Pointer{"allOf", index, subentry.first}); - } - } - } - - ONLY_CONTINUE_IF(!locations.empty() && !same_keywords); - return APPLIES_TO_POINTERS(std::move(locations)); - } - - auto transform(JSON &schema, const Result &) const -> void override { - for (auto &entry : schema.at("allOf").as_array()) { - if (entry.is_object()) { - std::vector blacklist; - for (auto &subentry : entry.as_object()) { - if (!schema.defines(subentry.first)) { - blacklist.push_back(subentry.first); - } - } - - for (auto &property : blacklist) { - schema.try_assign_before(property, entry.at(property), "allOf"); - entry.erase(property); - } - } - } - - schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema); - - if (schema.at("allOf").empty()) { - schema.erase("allOf"); - } - } -}; diff --git a/src/extension/alterschema/linter/unnecessary_allof_wrapper_properties.h b/src/extension/alterschema/linter/unnecessary_allof_wrapper_properties.h deleted file mode 100644 index b57035e9d..000000000 --- a/src/extension/alterschema/linter/unnecessary_allof_wrapper_properties.h +++ /dev/null @@ -1,77 +0,0 @@ -class UnnecessaryAllOfWrapperProperties final : public SchemaTransformRule { -public: - UnnecessaryAllOfWrapperProperties() - : SchemaTransformRule{ - "unnecessary_allof_wrapper_properties", - "Avoid unnecessarily wrapping object `properties` in `allOf` as it " - "may introduce a minor evaluation performance overhead and even " - "confuse documentation generators"} {}; - - [[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, - // TODO: Make this work on older dialects. Right we can't do that - // safely for Draft 7 and earlier if `properties` is a sibling of - // `$ref` - {"https://json-schema.org/draft/2020-12/vocab/applicator", - "https://json-schema.org/draft/2019-09/vocab/applicator"}) && - schema.is_object() && schema.defines("allOf") && - schema.at("allOf").is_array() && schema.defines("properties") && - schema.at("properties").is_object()); - - std::size_t cursor{0}; - for (const auto &entry : schema.at("allOf").as_array()) { - if (entry.is_object() && entry.defines("properties") && - entry.at("properties").is_object()) { - for (const auto &subentry : entry.at("properties").as_object()) { - if (!schema.at("properties").defines(subentry.first)) { - return APPLIES_TO_POINTERS( - {{"allOf", cursor, "properties", subentry.first}}); - } - } - } - - cursor += 1; - } - - return false; - } - - auto transform(JSON &schema, const Result &) const -> void override { - for (auto &entry : schema.at("allOf").as_array()) { - if (entry.is_object() && entry.defines("properties")) { - std::vector blacklist; - for (const auto &subentry : entry.at("properties").as_object()) { - if (!schema.at("properties").defines(subentry.first)) { - blacklist.push_back(subentry.first); - } - } - - for (auto &property : blacklist) { - schema.at("properties") - .assign(property, entry.at("properties").at(property)); - entry.at("properties").erase(property); - } - - if (entry.at("properties").empty()) { - entry.erase("properties"); - } - } - } - - schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema); - - if (schema.at("allOf").empty()) { - schema.erase("allOf"); - } - } -}; diff --git a/test/alterschema/alterschema_lint_2019_09_test.cc b/test/alterschema/alterschema_lint_2019_09_test.cc index 68b408c9a..41e9f8db6 100644 --- a/test/alterschema/alterschema_lint_2019_09_test.cc +++ b/test/alterschema/alterschema_lint_2019_09_test.cc @@ -976,9 +976,8 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_2) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -1000,9 +999,8 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_3) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -1031,9 +1029,8 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_4) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -2089,8 +2086,8 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_4) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", "$ref": "https://example.com", - "type": "number", "allOf": [ + { "type": "number" }, { "type": "integer" } ] })JSON"); @@ -2114,7 +2111,9 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_5) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", "$ref": "https://example.com", - "type": "integer" + "allOf": [ + { "type": "integer" } + ] })JSON"); EXPECT_EQ(document, expected); @@ -2132,9 +2131,9 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_6) { EXPECT_FALSE(result.first); EXPECT_EQ(traces.size(), 1); - EXPECT_LINT_TRACE(traces, 0, "", "unnecessary_allof_wrapper_modern", - "Wrapping keywords in `allOf` is often unnecessary and may " - "even introduce a minor evaluation performance overhead"); + EXPECT_LINT_TRACE(traces, 0, "", "unnecessary_allof_ref_wrapper_modern", + "Wrapping `$ref` in `allOf` was only necessary in JSON " + "Schema Draft 7 and older"); } TEST(AlterSchema_lint_2019_09, multiple_of_default_1) { @@ -2227,9 +2226,15 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_1) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", "properties": { - "foo": { "type": "string" }, - "bar": { "type": "string" } - } + "foo": { "type": "string" } + }, + "allOf": [ + { + "properties": { + "bar": { "type": "string" } + } + } + ] })JSON"); EXPECT_EQ(document, expected); @@ -2295,13 +2300,23 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_3) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", - "type": "object", "properties": { - "foo": { "type": "string" }, - "bar": { "minLength": 3 }, - "baz": { "maxLength": 5 }, - "qux": { "pattern": "^f" } - } + "foo": { "type": "string" } + }, + "allOf": [ + { + "properties": { + "bar": { "minLength": 3 }, + "baz": { "maxLength": 5 } + } + }, + { "type": "object" }, + { + "properties": { + "qux": { "pattern": "^f" } + } + } + ] })JSON"); EXPECT_EQ(document, expected); @@ -2327,11 +2342,113 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_4) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", - "$ref": "https://example.com", "properties": { - "foo": { "type": "string" }, - "bar": { "pattern": "^f" } - } + "foo": { "type": "string" } + }, + "$ref": "https://example.com", + "allOf": [ + { + "properties": { + "bar": { "pattern": "^f" } + } + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2019_09, + unnecessary_allof_ref_wrapper_modern_elevation_single_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "$ref": "https://example.com" + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2019_09, + unnecessary_allof_ref_wrapper_modern_no_elevation_all_branches_have_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" }, + { "$ref": "https://example.com/baz" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" }, + { "$ref": "https://example.com/baz" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST( + AlterSchema_lint_2019_09, + unnecessary_allof_ref_wrapper_modern_elevation_mixed_branches_with_and_without_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "$ref": "https://example.com/foo", "type": "object" }, + { "properties": { "bar": { "type": "string" } } } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "$ref": "https://example.com/foo", + "allOf": [ + { "type": "object" }, + { "properties": { "bar": { "type": "string" } } } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2019_09, + unnecessary_allof_ref_wrapper_modern_no_elevation_ref_with_id) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] })JSON"); EXPECT_EQ(document, expected); diff --git a/test/alterschema/alterschema_lint_2020_12_test.cc b/test/alterschema/alterschema_lint_2020_12_test.cc index f8e7283af..f478bb48f 100644 --- a/test/alterschema/alterschema_lint_2020_12_test.cc +++ b/test/alterschema/alterschema_lint_2020_12_test.cc @@ -776,9 +776,8 @@ TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_2) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -800,9 +799,8 @@ TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_3) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -831,9 +829,8 @@ TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_4) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -2047,9 +2044,9 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_4) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "number", "$ref": "https://example.com", "allOf": [ + { "type": "number" }, { "type": "integer" } ] })JSON"); @@ -2073,7 +2070,9 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_5) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", "$ref": "https://example.com", - "type": "integer" + "allOf": [ + { "type": "integer" } + ] })JSON"); EXPECT_EQ(document, expected); @@ -2126,7 +2125,9 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_7) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", "$ref": "https://example.com", - "type": "string", + "allOf": [ + { "type": "string" } + ], "title": "Foo" })JSON"); @@ -2142,7 +2143,7 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_7) { EXPECT_EQ(keywords.size(), 4); EXPECT_EQ(keywords.at(0), "$schema"); EXPECT_EQ(keywords.at(1), "$ref"); - EXPECT_EQ(keywords.at(2), "type"); + EXPECT_EQ(keywords.at(2), "allOf"); EXPECT_EQ(keywords.at(3), "title"); } @@ -2159,8 +2160,10 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_8) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "string", "$ref": "https://example.com", + "allOf": [ + { "type": "string" } + ], "title": "Foo" })JSON"); @@ -2175,8 +2178,8 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_8) { EXPECT_EQ(keywords.size(), 4); EXPECT_EQ(keywords.at(0), "$schema"); - EXPECT_EQ(keywords.at(1), "type"); - EXPECT_EQ(keywords.at(2), "$ref"); + EXPECT_EQ(keywords.at(1), "$ref"); + EXPECT_EQ(keywords.at(2), "allOf"); EXPECT_EQ(keywords.at(3), "title"); } @@ -2192,9 +2195,9 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_9) { EXPECT_FALSE(result.first); EXPECT_EQ(traces.size(), 1); - EXPECT_LINT_TRACE(traces, 0, "", "unnecessary_allof_wrapper_modern", - "Wrapping keywords in `allOf` is often unnecessary and may " - "even introduce a minor evaluation performance overhead"); + EXPECT_LINT_TRACE(traces, 0, "", "unnecessary_allof_ref_wrapper_modern", + "Wrapping `$ref` in `allOf` was only necessary in JSON " + "Schema Draft 7 and older"); } TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_10) { @@ -2210,9 +2213,9 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_10) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "number", "$ref": "https://example.com", "allOf": [ + { "type": "number" }, { "type": "integer" } ] })JSON"); @@ -2242,6 +2245,94 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_11) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_12) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "allOf": [ + { "properties": { "foo": { "type": "string" } } } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "allOf": [ + { "properties": { "foo": { "type": "string" } } } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_13) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "allOf": [ + { "patternProperties": { "^f": { "type": "string" } } } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "allOf": [ + { "patternProperties": { "^f": { "type": "string" } } } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_14) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "items": false, + "allOf": [ + { "prefixItems": [ true, false ] } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "items": false, + "allOf": [ + { "prefixItems": [ true, false ] } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_15) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "items": false, + "allOf": [ + { "properties": { "foo": { "type": "string" } } } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "items": false, + "allOf": [ + { "properties": { "foo": { "type": "string" } } } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_2020_12, multiple_of_default_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", @@ -2332,9 +2423,15 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_1) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", "properties": { - "foo": { "type": "string" }, - "bar": { "type": "string" } - } + "foo": { "type": "string" } + }, + "allOf": [ + { + "properties": { + "bar": { "type": "string" } + } + } + ] })JSON"); EXPECT_EQ(document, expected); @@ -2400,13 +2497,23 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_3) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "type": "object", "properties": { - "foo": { "type": "string" }, - "bar": { "minLength": 3 }, - "baz": { "maxLength": 5 }, - "qux": { "pattern": "^f" } - } + "foo": { "type": "string" } + }, + "allOf": [ + { + "properties": { + "bar": { "minLength": 3 }, + "baz": { "maxLength": 5 } + } + }, + { "type": "object" }, + { + "properties": { + "qux": { "pattern": "^f" } + } + } + ] })JSON"); EXPECT_EQ(document, expected); @@ -2432,11 +2539,215 @@ TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_4) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "$ref": "https://example.com", "properties": { - "foo": { "type": "string" }, - "bar": { "pattern": "^f" } - } + "foo": { "type": "string" } + }, + "$ref": "https://example.com", + "allOf": [ + { + "properties": { + "bar": { "pattern": "^f" } + } + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_elevation_single_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "https://example.com" + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_elevation_multiple_refs) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" }, + { "$ref": "https://example.com/baz" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" }, + { "$ref": "https://example.com/baz" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST( + AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_elevation_mixed_branches_with_and_without_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "properties": { "bar": { "type": "string" } } }, + { "$ref": "https://example.com/baz", "type": "object" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "$ref": "https://example.com/foo", + "allOf": [ + { "properties": { "bar": { "type": "string" } } }, + { "$ref": "https://example.com/baz", "type": "object" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_no_elevation_ref_with_id) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_no_elevation_parent_has_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "https://example.com/main", + "allOf": [ + { "$ref": "https://example.com/foo" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "https://example.com/main", + "allOf": [ + { "$ref": "https://example.com/foo" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_no_elevation_all_branches_have_ref) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST( + AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_no_elevation_all_branches_have_ref_with_siblings) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com/foo", "type": "object" }, + { "$ref": "https://example.com/bar", "properties": { "x": { "type": "string" } } } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "$ref": "https://example.com/foo", "type": "object" }, + { "$ref": "https://example.com/bar", "properties": { "x": { "type": "string" } } } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, + unnecessary_allof_ref_wrapper_modern_elevation_ref_with_siblings) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { + "$ref": "https://example.com", + "type": "object", + "properties": { "foo": { "type": "string" } } + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$ref": "https://example.com", + "allOf": [ + { + "type": "object", + "properties": { "foo": { "type": "string" } } + } + ] })JSON"); EXPECT_EQ(document, expected); @@ -3253,10 +3564,10 @@ TEST(AlterSchema_lint_2020_12, required_properties_in_properties_11) { const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", - "properties": { "foo": true, "bar": true, "baz": true }, + "properties": { "foo": true, "bar": true }, "allOf": [ { "required": [ "foo" ] }, - { "required": [ "bar", "baz" ] } + { "required": [ "bar", "baz" ], "properties": { "baz": true } } ] })JSON"); diff --git a/test/alterschema/alterschema_lint_draft4_test.cc b/test/alterschema/alterschema_lint_draft4_test.cc index 0b9bbdf5c..fcaf1e5bc 100644 --- a/test/alterschema/alterschema_lint_draft4_test.cc +++ b/test/alterschema/alterschema_lint_draft4_test.cc @@ -455,9 +455,8 @@ TEST(AlterSchema_lint_draft4, duplicate_allof_branches_2) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-04/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -479,9 +478,8 @@ TEST(AlterSchema_lint_draft4, duplicate_allof_branches_3) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-04/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -510,9 +508,8 @@ TEST(AlterSchema_lint_draft4, duplicate_allof_branches_4) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-04/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -1199,21 +1196,23 @@ TEST(AlterSchema_lint_draft4, unnecessary_allof_wrapper_1) { } TEST(AlterSchema_lint_draft4, unnecessary_allof_wrapper_2) { - const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-04/schema#", "allOf": [ { "type": "string" } ] })JSON"); - LINT_WITHOUT_FIX_FOR_READABILITY(document, result, traces); + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "type": "string" } + ] + })JSON"); - EXPECT_FALSE(result.first); - EXPECT_EQ(traces.size(), 1); - EXPECT_LINT_TRACE( - traces, 0, "", "unnecessary_allof_wrapper_draft", - "Wrapping keywords other than `$ref` in `allOf` is often unnecessary and " - "may even introduce a minor evaluation performance overhead"); + EXPECT_EQ(document, expected); } TEST(AlterSchema_lint_draft4, unnecessary_allof_wrapper_3) { @@ -1394,6 +1393,151 @@ TEST(AlterSchema_lint_draft4, unnecessary_allof_wrapper_properties_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_draft4, + unnecessary_allof_ref_wrapper_draft_no_elevation_ref_with_id) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "id": "https://other.com" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "id": "https://other.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft4, unnecessary_allof_ref_wrapper_draft_elevation) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "additionalProperties": { + "allOf": [ + { "$ref": "https://example.com" } + ] + } + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "additionalProperties": { + "$ref": "https://example.com" + } + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft4, + unnecessary_allof_ref_wrapper_draft_no_elevation_with_schema) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft4, + unnecessary_allof_ref_wrapper_draft_no_elevation_with_sibling_keyword) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST( + AlterSchema_lint_draft4, + unnecessary_allof_ref_wrapper_draft_no_elevation_ref_with_sibling_keyword) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "type": "object" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { + "$ref": "https://example.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft4, + unnecessary_allof_ref_wrapper_draft_no_elevation_multiple_branches) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_draft4, draft_official_dialect_without_empty_fragment_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-04/schema", diff --git a/test/alterschema/alterschema_lint_draft6_test.cc b/test/alterschema/alterschema_lint_draft6_test.cc index 24ad905b4..70789ce01 100644 --- a/test/alterschema/alterschema_lint_draft6_test.cc +++ b/test/alterschema/alterschema_lint_draft6_test.cc @@ -643,9 +643,8 @@ TEST(AlterSchema_lint_draft6, duplicate_allof_branches_2) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-06/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -667,9 +666,8 @@ TEST(AlterSchema_lint_draft6, duplicate_allof_branches_3) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-06/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -698,9 +696,8 @@ TEST(AlterSchema_lint_draft6, duplicate_allof_branches_4) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-06/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -1550,21 +1547,23 @@ TEST(AlterSchema_lint_draft6, unnecessary_allof_wrapper_1) { } TEST(AlterSchema_lint_draft6, unnecessary_allof_wrapper_2) { - const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-06/schema#", "allOf": [ { "type": "string" } ] })JSON"); - LINT_WITHOUT_FIX_FOR_READABILITY(document, result, traces); + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "type": "string" } + ] + })JSON"); - EXPECT_FALSE(result.first); - EXPECT_EQ(traces.size(), 1); - EXPECT_LINT_TRACE( - traces, 0, "", "unnecessary_allof_wrapper_draft", - "Wrapping keywords other than `$ref` in `allOf` is often unnecessary and " - "may even introduce a minor evaluation performance overhead"); + EXPECT_EQ(document, expected); } TEST(AlterSchema_lint_draft6, unnecessary_allof_wrapper_3) { @@ -1656,6 +1655,151 @@ TEST(AlterSchema_lint_draft6, unnecessary_allof_wrapper_properties_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_draft6, unnecessary_allof_ref_wrapper_draft_elevation) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "object", + "additionalProperties": { + "allOf": [ + { "$ref": "https://example.com" } + ] + } + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "object", + "additionalProperties": { + "$ref": "https://example.com" + } + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft6, + unnecessary_allof_ref_wrapper_draft_no_elevation_with_schema) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft6, + unnecessary_allof_ref_wrapper_draft_no_elevation_with_sibling_keyword) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "object", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "object", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST( + AlterSchema_lint_draft6, + unnecessary_allof_ref_wrapper_draft_no_elevation_ref_with_sibling_keyword) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "type": "object" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { + "$ref": "https://example.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft6, + unnecessary_allof_ref_wrapper_draft_no_elevation_ref_with_id) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft6, + unnecessary_allof_ref_wrapper_draft_no_elevation_multiple_branches) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_draft6, draft_official_dialect_without_empty_fragment_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-06/schema", diff --git a/test/alterschema/alterschema_lint_draft7_test.cc b/test/alterschema/alterschema_lint_draft7_test.cc index 862aace83..139ec8b8d 100644 --- a/test/alterschema/alterschema_lint_draft7_test.cc +++ b/test/alterschema/alterschema_lint_draft7_test.cc @@ -741,9 +741,8 @@ TEST(AlterSchema_lint_draft7, duplicate_allof_branches_2) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-07/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -765,9 +764,8 @@ TEST(AlterSchema_lint_draft7, duplicate_allof_branches_3) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-07/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -796,9 +794,8 @@ TEST(AlterSchema_lint_draft7, duplicate_allof_branches_4) { const auto expected = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-07/schema#", - "type": "number", - "multipleOf": 1, "allOf": [ + { "type": "number", "multipleOf": 1 }, { "type": "string", "minLength": 0 } ] })JSON"); @@ -1741,21 +1738,23 @@ TEST(AlterSchema_lint_draft7, unnecessary_allof_wrapper_1) { } TEST(AlterSchema_lint_draft7, unnecessary_allof_wrapper_2) { - const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-07/schema#", "allOf": [ { "type": "string" } ] })JSON"); - LINT_WITHOUT_FIX_FOR_READABILITY(document, result, traces); + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "type": "string" } + ] + })JSON"); - EXPECT_FALSE(result.first); - EXPECT_EQ(traces.size(), 1); - EXPECT_LINT_TRACE( - traces, 0, "", "unnecessary_allof_wrapper_draft", - "Wrapping keywords other than `$ref` in `allOf` is often unnecessary and " - "may even introduce a minor evaluation performance overhead"); + EXPECT_EQ(document, expected); } TEST(AlterSchema_lint_draft7, unnecessary_allof_wrapper_3) { @@ -1851,6 +1850,151 @@ TEST(AlterSchema_lint_draft7, unnecessary_allof_wrapper_properties_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_draft7, unnecessary_allof_ref_wrapper_draft_elevation) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "additionalProperties": { + "allOf": [ + { "$ref": "https://example.com" } + ] + } + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "additionalProperties": { + "$ref": "https://example.com" + } + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft7, + unnecessary_allof_ref_wrapper_draft_no_elevation_with_schema) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft7, + unnecessary_allof_ref_wrapper_draft_no_elevation_with_sibling_keyword) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ], + "type": "object" + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "$ref": "https://example.com" } + ], + "type": "object" + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST( + AlterSchema_lint_draft7, + unnecessary_allof_ref_wrapper_draft_no_elevation_ref_with_sibling_keyword) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "type": "object" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { + "$ref": "https://example.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft7, + unnecessary_allof_ref_wrapper_draft_no_elevation_ref_with_id) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { + "$ref": "https://example.com", + "$id": "https://other.com" + } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft7, + unnecessary_allof_ref_wrapper_draft_no_elevation_multiple_branches) { + sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + LINT_AND_FIX_FOR_READABILITY(document); + + const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "$ref": "https://example.com/foo" }, + { "$ref": "https://example.com/bar" } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_draft7, draft_official_dialect_without_empty_fragment_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-07/schema",