diff --git a/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h b/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h index d23257336..26b1fc60f 100644 --- a/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h +++ b/src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h @@ -28,6 +28,17 @@ class UnnecessaryAllOfWrapperDraft final : public SchemaTransformRule { for (const auto &entry : schema.at("allOf").as_array()) { if (entry.is_object()) { + // 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")) { + continue; + } + for (const auto &subentry : entry.as_object()) { if (subentry.first != "$ref" && !schema.defines(subentry.first)) { return true; diff --git a/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h b/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h index a7adc26f3..195eb68a7 100644 --- a/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h +++ b/src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h @@ -26,9 +26,27 @@ class UnnecessaryAllOfWrapperModern final : public SchemaTransformRule { return false; } + 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"})}; + for (const auto &entry : schema.at("allOf").as_array()) { if (entry.is_object()) { + // 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")) { + continue; + } + for (const auto &subentry : entry.as_object()) { + // 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)) { return true; } diff --git a/test/alterschema/alterschema_lint_2019_09_test.cc b/test/alterschema/alterschema_lint_2019_09_test.cc index d35ff664c..171d82775 100644 --- a/test/alterschema/alterschema_lint_2019_09_test.cc +++ b/test/alterschema/alterschema_lint_2019_09_test.cc @@ -657,6 +657,85 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_2) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_3) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "type": "number" }, + { "type": "number" }, + { "type": "string" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_4) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" }, + { "type": "number" }, + { "type": "string" }, + { "type": "string" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2019-09/schema", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_2019_09, duplicate_anyof_branches_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2019-09/schema", diff --git a/test/alterschema/alterschema_lint_2020_12_test.cc b/test/alterschema/alterschema_lint_2020_12_test.cc index c6e8d40a4..bbad7d002 100644 --- a/test/alterschema/alterschema_lint_2020_12_test.cc +++ b/test/alterschema/alterschema_lint_2020_12_test.cc @@ -676,6 +676,85 @@ TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_2) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_3) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "type": "number" }, + { "type": "number" }, + { "type": "string" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_4) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" }, + { "type": "number" }, + { "type": "string" }, + { "type": "string" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_2020_12, duplicate_anyof_branches_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", diff --git a/test/alterschema/alterschema_lint_draft4_test.cc b/test/alterschema/alterschema_lint_draft4_test.cc index bdeb5a6ba..2321c065b 100644 --- a/test/alterschema/alterschema_lint_draft4_test.cc +++ b/test/alterschema/alterschema_lint_draft4_test.cc @@ -193,6 +193,85 @@ TEST(AlterSchema_lint_draft4, duplicate_allof_branches_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_draft4, duplicate_allof_branches_2) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft4, duplicate_allof_branches_3) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "type": "number" }, + { "type": "number" }, + { "type": "string" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft4, duplicate_allof_branches_4) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" }, + { "type": "number" }, + { "type": "string" }, + { "type": "string" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_draft4, duplicate_anyof_branches_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 e17eb414b..71a90b24b 100644 --- a/test/alterschema/alterschema_lint_draft6_test.cc +++ b/test/alterschema/alterschema_lint_draft6_test.cc @@ -328,6 +328,85 @@ TEST(AlterSchema_lint_draft6, duplicate_allof_branches_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_draft6, duplicate_allof_branches_2) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft6, duplicate_allof_branches_3) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "type": "number" }, + { "type": "number" }, + { "type": "string" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft6, duplicate_allof_branches_4) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" }, + { "type": "number" }, + { "type": "string" }, + { "type": "string" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-06/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_draft6, duplicate_anyof_branches_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 054bab182..3a4268637 100644 --- a/test/alterschema/alterschema_lint_draft7_test.cc +++ b/test/alterschema/alterschema_lint_draft7_test.cc @@ -424,6 +424,85 @@ TEST(AlterSchema_lint_draft7, duplicate_allof_branches_1) { EXPECT_EQ(document, expected); } +TEST(AlterSchema_lint_draft7, duplicate_allof_branches_2) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft7, duplicate_allof_branches_3) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "type": "number" }, + { "type": "number" }, + { "type": "string" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + +TEST(AlterSchema_lint_draft7, duplicate_allof_branches_4) { + auto document = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "allOf": [ + { "type": "number" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" }, + { "type": "number" }, + { "type": "string" }, + { "type": "string" }, + { "type": "string" }, + { "type": "number" }, + { "type": "number" } + ] + })JSON"); + + LINT_AND_FIX_FOR_STATIC_ANALYSIS(document); + + const auto expected = sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "number", + "multipleOf": 1, + "allOf": [ + { "type": "string", "minLength": 0 } + ] + })JSON"); + + EXPECT_EQ(document, expected); +} + TEST(AlterSchema_lint_draft7, duplicate_anyof_branches_1) { sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ "$schema": "http://json-schema.org/draft-07/schema#",