Skip to content

Commit f00f364

Browse files
jviottijviotti
authored
Generalise how to extract any keywords from allOf (#1830)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent bcb19ff commit f00f364

10 files changed

+174
-117
lines changed

‎src/extension/alterschema/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ sourcemeta_library(NAMESPACE sourcemeta PROJECT core NAME alterschema
4848
linter/content_media_type_without_encoding.h
4949
linter/content_schema_without_media_type.h
5050
linter/non_applicable_type_specific_keywords.h
51-
linter/unnecessary_allof_ref_wrapper.h
51+
linter/unnecessary_allof_wrapper_modern.h
52+
linter/unnecessary_allof_wrapper_draft.h
5253
linter/duplicate_allof_branches.h
5354
linter/duplicate_anyof_branches.h
5455
linter/else_without_if.h

‎src/extension/alterschema/alterschema.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ contains_any(const Vocabularies &container,
6969
#include "linter/then_without_if.h"
7070
#include "linter/unevaluated_items_default.h"
7171
#include "linter/unevaluated_properties_default.h"
72-
#include "linter/unnecessary_allof_ref_wrapper.h"
72+
#include "linter/unnecessary_allof_wrapper_draft.h"
73+
#include "linter/unnecessary_allof_wrapper_modern.h"
7374
#include "linter/unsatisfiable_max_contains.h"
7475
#include "linter/unsatisfiable_min_properties.h"
7576
} // namespace sourcemeta::core
@@ -83,7 +84,8 @@ auto add(SchemaTransformer &bundle, const AlterSchemaMode mode)
8384
bundle.add<ContentMediaTypeWithoutEncoding>();
8485
bundle.add<ContentSchemaWithoutMediaType>();
8586
bundle.add<NonApplicableTypeSpecificKeywords>();
86-
bundle.add<UnnecessaryAllOfRefWrapper>();
87+
bundle.add<UnnecessaryAllOfWrapperModern>();
88+
bundle.add<UnnecessaryAllOfWrapperDraft>();
8789
bundle.add<DuplicateAllOfBranches>();
8890
bundle.add<DuplicateAnyOfBranches>();
8991
bundle.add<ElseWithoutIf>();

‎src/extension/alterschema/linter/unnecessary_allof_ref_wrapper.h

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎src/extension/alterschema/linter/unnecessary_allof_wrapper_draft.h

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
class UnnecessaryAllOfWrapperDraft final : public SchemaTransformRule {
2+
public:
3+
UnnecessaryAllOfWrapperDraft()
4+
: SchemaTransformRule{"unnecessary_allof_wrapper_draft",
5+
"Wrapping any keyword other than `$ref` in `allOf` "
6+
"is unnecessary"} {};
7+
8+
[[nodiscard]] auto
9+
condition(const sourcemeta::core::JSON &schema,
10+
const sourcemeta::core::JSON &,
11+
const sourcemeta::core::Vocabularies &vocabularies,
12+
const sourcemeta::core::SchemaFrame &,
13+
const sourcemeta::core::SchemaFrame::Location &,
14+
const sourcemeta::core::SchemaWalker &,
15+
const sourcemeta::core::SchemaResolver &) const
16+
-> sourcemeta::core::SchemaTransformRule::Result override {
17+
if (!contains_any(vocabularies,
18+
{"http://json-schema.org/draft-07/schema#",
19+
"http://json-schema.org/draft-06/schema#",
20+
"http://json-schema.org/draft-04/schema#"})) {
21+
return false;
22+
}
23+
24+
if (!schema.is_object() || !schema.defines("allOf") ||
25+
!schema.at("allOf").is_array()) {
26+
return false;
27+
}
28+
29+
for (const auto &entry : schema.at("allOf").as_array()) {
30+
if (entry.is_object()) {
31+
for (const auto &subentry : entry.as_object()) {
32+
if (subentry.first != "$ref" && !schema.defines(subentry.first)) {
33+
return true;
34+
}
35+
}
36+
}
37+
}
38+
39+
return false;
40+
}
41+
42+
auto transform(JSON &schema) const -> void override {
43+
for (auto &entry : schema.at("allOf").as_array()) {
44+
if (entry.is_object()) {
45+
std::vector<JSON::String> blacklist;
46+
for (auto &subentry : entry.as_object()) {
47+
if (subentry.first != "$ref" && !schema.defines(subentry.first)) {
48+
blacklist.push_back(subentry.first);
49+
}
50+
}
51+
52+
for (auto &property : blacklist) {
53+
schema.try_assign_before(property, entry.at(property), "allOf");
54+
entry.erase(property);
55+
}
56+
}
57+
}
58+
59+
schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema);
60+
61+
if (schema.at("allOf").empty()) {
62+
schema.erase("allOf");
63+
}
64+
}
65+
};

‎src/extension/alterschema/linter/unnecessary_allof_wrapper_modern.h

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
class UnnecessaryAllOfWrapperModern final : public SchemaTransformRule {
2+
public:
3+
UnnecessaryAllOfWrapperModern()
4+
: SchemaTransformRule{"unnecessary_allof_wrapper_modern",
5+
"Wrapping any keyword in `allOf` is unnecessary"} {
6+
};
7+
8+
[[nodiscard]] auto
9+
condition(const sourcemeta::core::JSON &schema,
10+
const sourcemeta::core::JSON &,
11+
const sourcemeta::core::Vocabularies &vocabularies,
12+
const sourcemeta::core::SchemaFrame &,
13+
const sourcemeta::core::SchemaFrame::Location &,
14+
const sourcemeta::core::SchemaWalker &,
15+
const sourcemeta::core::SchemaResolver &) const
16+
-> sourcemeta::core::SchemaTransformRule::Result override {
17+
if (!contains_any(
18+
vocabularies,
19+
{"https://json-schema.org/draft/2020-12/vocab/applicator",
20+
"https://json-schema.org/draft/2019-09/vocab/applicator"})) {
21+
return false;
22+
}
23+
24+
if (!schema.is_object() || !schema.defines("allOf") ||
25+
!schema.at("allOf").is_array()) {
26+
return false;
27+
}
28+
29+
for (const auto &entry : schema.at("allOf").as_array()) {
30+
if (entry.is_object()) {
31+
for (const auto &subentry : entry.as_object()) {
32+
if (!schema.defines(subentry.first)) {
33+
return true;
34+
}
35+
}
36+
}
37+
}
38+
39+
return false;
40+
}
41+
42+
auto transform(JSON &schema) const -> void override {
43+
for (auto &entry : schema.at("allOf").as_array()) {
44+
if (entry.is_object()) {
45+
std::vector<JSON::String> blacklist;
46+
for (auto &subentry : entry.as_object()) {
47+
if (!schema.defines(subentry.first)) {
48+
blacklist.push_back(subentry.first);
49+
}
50+
}
51+
52+
for (auto &property : blacklist) {
53+
schema.try_assign_before(property, entry.at(property), "allOf");
54+
entry.erase(property);
55+
}
56+
}
57+
}
58+
59+
schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema);
60+
61+
if (schema.at("allOf").empty()) {
62+
schema.erase("allOf");
63+
}
64+
}
65+
};

‎test/alterschema/alterschema_lint_2019_09_test.cc

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,8 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_1) {
650650

651651
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
652652
"$schema": "https://json-schema.org/draft/2019-09/schema",
653-
"allOf": [ { "type": "integer" }, { "type": "string" } ]
653+
"type": "integer",
654+
"allOf": [ { "type": "string" } ]
654655
})JSON");
655656

656657
EXPECT_EQ(document, expected);
@@ -1571,7 +1572,7 @@ TEST(AlterSchema_lint_2019_09, equal_numeric_bounds_to_enum_2) {
15711572
EXPECT_EQ(document, expected);
15721573
}
15731574

1574-
TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_1) {
1575+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_1) {
15751576
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
15761577
"$schema": "https://json-schema.org/draft/2019-09/schema",
15771578
"allOf": [
@@ -1589,7 +1590,7 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_1) {
15891590
EXPECT_EQ(document, expected);
15901591
}
15911592

1592-
TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_2) {
1593+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_2) {
15931594
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
15941595
"$schema": "https://json-schema.org/draft/2019-09/schema",
15951596
"allOf": [
@@ -1602,16 +1603,16 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_2) {
16021603

16031604
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
16041605
"$schema": "https://json-schema.org/draft/2019-09/schema",
1606+
"$ref": "https://example.com/foo",
16051607
"allOf": [
1606-
{ "$ref": "https://example.com/foo" },
16071608
{ "$ref": "https://example.com/bar" }
16081609
]
16091610
})JSON");
16101611

16111612
EXPECT_EQ(document, expected);
16121613
}
16131614

1614-
TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_3) {
1615+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_3) {
16151616
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
16161617
"$schema": "https://json-schema.org/draft/2019-09/schema",
16171618
"$ref": "https://example.com/foo",
@@ -1633,7 +1634,7 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_3) {
16331634
EXPECT_EQ(document, expected);
16341635
}
16351636

1636-
TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_4) {
1637+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_4) {
16371638
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
16381639
"$schema": "https://json-schema.org/draft/2019-09/schema",
16391640
"allOf": [
@@ -1648,20 +1649,20 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_4) {
16481649
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
16491650
"$schema": "https://json-schema.org/draft/2019-09/schema",
16501651
"$ref": "https://example.com",
1652+
"type": "number",
16511653
"allOf": [
1652-
{ "type": "number" },
16531654
{ "type": "integer" }
16541655
]
16551656
})JSON");
16561657

16571658
EXPECT_EQ(document, expected);
16581659
}
16591660

1660-
TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_5) {
1661+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_5) {
16611662
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
16621663
"$schema": "https://json-schema.org/draft/2019-09/schema",
16631664
"allOf": [
1664-
{
1665+
{
16651666
"type": "integer",
16661667
"$ref": "https://example.com"
16671668
}
@@ -1673,9 +1674,7 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_5) {
16731674
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
16741675
"$schema": "https://json-schema.org/draft/2019-09/schema",
16751676
"$ref": "https://example.com",
1676-
"allOf": [
1677-
{ "type": "integer" }
1678-
]
1677+
"type": "integer"
16791678
})JSON");
16801679

16811680
EXPECT_EQ(document, expected);

0 commit comments

Comments
 (0)