Skip to content

Commit f354ddc

Browse files
jviottijviotti
authored
Fix unnecessary_allof_wrapper_modern conflict on canonicalise (#1877)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 3cf52e7 commit f354ddc

7 files changed

+424
-0
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@ class UnnecessaryAllOfWrapperDraft final : public SchemaTransformRule {
2828

2929
for (const auto &entry : schema.at("allOf").as_array()) {
3030
if (entry.is_object()) {
31+
// It is dangerous to extract type-specific keywords from a schema that
32+
// declares a type into another schema that also declares a type if
33+
// the types are different. As we might lead to those type-keywords
34+
// getting incorrectly removed if they don't apply to the target type
35+
if (schema.defines("type") && entry.defines("type") &&
36+
// TODO: Ideally we also check for intersection of types in type
37+
// arrays or whether one is contained in the other
38+
schema.at("type") != entry.at("type")) {
39+
continue;
40+
}
41+
3142
for (const auto &subentry : entry.as_object()) {
3243
if (subentry.first != "$ref" && !schema.defines(subentry.first)) {
3344
return true;

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,27 @@ class UnnecessaryAllOfWrapperModern final : public SchemaTransformRule {
2626
return false;
2727
}
2828

29+
const auto has_validation{contains_any(
30+
vocabularies,
31+
{"https://json-schema.org/draft/2020-12/vocab/validation",
32+
"https://json-schema.org/draft/2019-09/vocab/validation"})};
33+
2934
for (const auto &entry : schema.at("allOf").as_array()) {
3035
if (entry.is_object()) {
36+
// It is dangerous to extract type-specific keywords from a schema that
37+
// declares a type into another schema that also declares a type if
38+
// the types are different. As we might lead to those type-keywords
39+
// getting incorrectly removed if they don't apply to the target type
40+
if (has_validation && schema.defines("type") && entry.defines("type") &&
41+
// TODO: Ideally we also check for intersection of types in type
42+
// arrays or whether one is contained in the other
43+
schema.at("type") != entry.at("type")) {
44+
continue;
45+
}
46+
3147
for (const auto &subentry : entry.as_object()) {
48+
// TODO: Have another rule that removes a keyword if its exactly
49+
// equal to an instance of the same keyword outside the wrapper
3250
if (!schema.defines(subentry.first)) {
3351
return true;
3452
}

‎test/alterschema/alterschema_lint_2019_09_test.cc

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,85 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_1) {
657657
EXPECT_EQ(document, expected);
658658
}
659659

660+
TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_2) {
661+
auto document = sourcemeta::core::parse_json(R"JSON({
662+
"$schema": "https://json-schema.org/draft/2019-09/schema",
663+
"allOf": [
664+
{ "type": "number" },
665+
{ "type": "string" },
666+
{ "type": "number" }
667+
]
668+
})JSON");
669+
670+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
671+
672+
const auto expected = sourcemeta::core::parse_json(R"JSON({
673+
"$schema": "https://json-schema.org/draft/2019-09/schema",
674+
"type": "number",
675+
"multipleOf": 1,
676+
"allOf": [
677+
{ "type": "string", "minLength": 0 }
678+
]
679+
})JSON");
680+
681+
EXPECT_EQ(document, expected);
682+
}
683+
684+
TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_3) {
685+
auto document = sourcemeta::core::parse_json(R"JSON({
686+
"$schema": "https://json-schema.org/draft/2019-09/schema",
687+
"allOf": [
688+
{ "type": "number" },
689+
{ "type": "number" },
690+
{ "type": "string" }
691+
]
692+
})JSON");
693+
694+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
695+
696+
const auto expected = sourcemeta::core::parse_json(R"JSON({
697+
"$schema": "https://json-schema.org/draft/2019-09/schema",
698+
"type": "number",
699+
"multipleOf": 1,
700+
"allOf": [
701+
{ "type": "string", "minLength": 0 }
702+
]
703+
})JSON");
704+
705+
EXPECT_EQ(document, expected);
706+
}
707+
708+
TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_4) {
709+
auto document = sourcemeta::core::parse_json(R"JSON({
710+
"$schema": "https://json-schema.org/draft/2019-09/schema",
711+
"allOf": [
712+
{ "type": "number" },
713+
{ "type": "string" },
714+
{ "type": "number" },
715+
{ "type": "number" },
716+
{ "type": "number" },
717+
{ "type": "string" },
718+
{ "type": "string" },
719+
{ "type": "string" },
720+
{ "type": "number" },
721+
{ "type": "number" }
722+
]
723+
})JSON");
724+
725+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
726+
727+
const auto expected = sourcemeta::core::parse_json(R"JSON({
728+
"$schema": "https://json-schema.org/draft/2019-09/schema",
729+
"type": "number",
730+
"multipleOf": 1,
731+
"allOf": [
732+
{ "type": "string", "minLength": 0 }
733+
]
734+
})JSON");
735+
736+
EXPECT_EQ(document, expected);
737+
}
738+
660739
TEST(AlterSchema_lint_2019_09, duplicate_anyof_branches_1) {
661740
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
662741
"$schema": "https://json-schema.org/draft/2019-09/schema",

‎test/alterschema/alterschema_lint_2020_12_test.cc

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,85 @@ TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_1) {
676676
EXPECT_EQ(document, expected);
677677
}
678678

679+
TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_2) {
680+
auto document = sourcemeta::core::parse_json(R"JSON({
681+
"$schema": "https://json-schema.org/draft/2020-12/schema",
682+
"allOf": [
683+
{ "type": "number" },
684+
{ "type": "string" },
685+
{ "type": "number" }
686+
]
687+
})JSON");
688+
689+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
690+
691+
const auto expected = sourcemeta::core::parse_json(R"JSON({
692+
"$schema": "https://json-schema.org/draft/2020-12/schema",
693+
"type": "number",
694+
"multipleOf": 1,
695+
"allOf": [
696+
{ "type": "string", "minLength": 0 }
697+
]
698+
})JSON");
699+
700+
EXPECT_EQ(document, expected);
701+
}
702+
703+
TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_3) {
704+
auto document = sourcemeta::core::parse_json(R"JSON({
705+
"$schema": "https://json-schema.org/draft/2020-12/schema",
706+
"allOf": [
707+
{ "type": "number" },
708+
{ "type": "number" },
709+
{ "type": "string" }
710+
]
711+
})JSON");
712+
713+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
714+
715+
const auto expected = sourcemeta::core::parse_json(R"JSON({
716+
"$schema": "https://json-schema.org/draft/2020-12/schema",
717+
"type": "number",
718+
"multipleOf": 1,
719+
"allOf": [
720+
{ "type": "string", "minLength": 0 }
721+
]
722+
})JSON");
723+
724+
EXPECT_EQ(document, expected);
725+
}
726+
727+
TEST(AlterSchema_lint_2020_12, duplicate_allof_branches_4) {
728+
auto document = sourcemeta::core::parse_json(R"JSON({
729+
"$schema": "https://json-schema.org/draft/2020-12/schema",
730+
"allOf": [
731+
{ "type": "number" },
732+
{ "type": "string" },
733+
{ "type": "number" },
734+
{ "type": "number" },
735+
{ "type": "number" },
736+
{ "type": "string" },
737+
{ "type": "string" },
738+
{ "type": "string" },
739+
{ "type": "number" },
740+
{ "type": "number" }
741+
]
742+
})JSON");
743+
744+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
745+
746+
const auto expected = sourcemeta::core::parse_json(R"JSON({
747+
"$schema": "https://json-schema.org/draft/2020-12/schema",
748+
"type": "number",
749+
"multipleOf": 1,
750+
"allOf": [
751+
{ "type": "string", "minLength": 0 }
752+
]
753+
})JSON");
754+
755+
EXPECT_EQ(document, expected);
756+
}
757+
679758
TEST(AlterSchema_lint_2020_12, duplicate_anyof_branches_1) {
680759
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
681760
"$schema": "https://json-schema.org/draft/2020-12/schema",

‎test/alterschema/alterschema_lint_draft4_test.cc

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,85 @@ TEST(AlterSchema_lint_draft4, duplicate_allof_branches_1) {
193193
EXPECT_EQ(document, expected);
194194
}
195195

196+
TEST(AlterSchema_lint_draft4, duplicate_allof_branches_2) {
197+
auto document = sourcemeta::core::parse_json(R"JSON({
198+
"$schema": "http://json-schema.org/draft-04/schema#",
199+
"allOf": [
200+
{ "type": "number" },
201+
{ "type": "string" },
202+
{ "type": "number" }
203+
]
204+
})JSON");
205+
206+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
207+
208+
const auto expected = sourcemeta::core::parse_json(R"JSON({
209+
"$schema": "http://json-schema.org/draft-04/schema#",
210+
"type": "number",
211+
"multipleOf": 1,
212+
"allOf": [
213+
{ "type": "string", "minLength": 0 }
214+
]
215+
})JSON");
216+
217+
EXPECT_EQ(document, expected);
218+
}
219+
220+
TEST(AlterSchema_lint_draft4, duplicate_allof_branches_3) {
221+
auto document = sourcemeta::core::parse_json(R"JSON({
222+
"$schema": "http://json-schema.org/draft-04/schema#",
223+
"allOf": [
224+
{ "type": "number" },
225+
{ "type": "number" },
226+
{ "type": "string" }
227+
]
228+
})JSON");
229+
230+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
231+
232+
const auto expected = sourcemeta::core::parse_json(R"JSON({
233+
"$schema": "http://json-schema.org/draft-04/schema#",
234+
"type": "number",
235+
"multipleOf": 1,
236+
"allOf": [
237+
{ "type": "string", "minLength": 0 }
238+
]
239+
})JSON");
240+
241+
EXPECT_EQ(document, expected);
242+
}
243+
244+
TEST(AlterSchema_lint_draft4, duplicate_allof_branches_4) {
245+
auto document = sourcemeta::core::parse_json(R"JSON({
246+
"$schema": "http://json-schema.org/draft-04/schema#",
247+
"allOf": [
248+
{ "type": "number" },
249+
{ "type": "string" },
250+
{ "type": "number" },
251+
{ "type": "number" },
252+
{ "type": "number" },
253+
{ "type": "string" },
254+
{ "type": "string" },
255+
{ "type": "string" },
256+
{ "type": "number" },
257+
{ "type": "number" }
258+
]
259+
})JSON");
260+
261+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
262+
263+
const auto expected = sourcemeta::core::parse_json(R"JSON({
264+
"$schema": "http://json-schema.org/draft-04/schema#",
265+
"type": "number",
266+
"multipleOf": 1,
267+
"allOf": [
268+
{ "type": "string", "minLength": 0 }
269+
]
270+
})JSON");
271+
272+
EXPECT_EQ(document, expected);
273+
}
274+
196275
TEST(AlterSchema_lint_draft4, duplicate_anyof_branches_1) {
197276
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
198277
"$schema": "http://json-schema.org/draft-04/schema#",

‎test/alterschema/alterschema_lint_draft6_test.cc

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,85 @@ TEST(AlterSchema_lint_draft6, duplicate_allof_branches_1) {
328328
EXPECT_EQ(document, expected);
329329
}
330330

331+
TEST(AlterSchema_lint_draft6, duplicate_allof_branches_2) {
332+
auto document = sourcemeta::core::parse_json(R"JSON({
333+
"$schema": "http://json-schema.org/draft-06/schema#",
334+
"allOf": [
335+
{ "type": "number" },
336+
{ "type": "string" },
337+
{ "type": "number" }
338+
]
339+
})JSON");
340+
341+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
342+
343+
const auto expected = sourcemeta::core::parse_json(R"JSON({
344+
"$schema": "http://json-schema.org/draft-06/schema#",
345+
"type": "number",
346+
"multipleOf": 1,
347+
"allOf": [
348+
{ "type": "string", "minLength": 0 }
349+
]
350+
})JSON");
351+
352+
EXPECT_EQ(document, expected);
353+
}
354+
355+
TEST(AlterSchema_lint_draft6, duplicate_allof_branches_3) {
356+
auto document = sourcemeta::core::parse_json(R"JSON({
357+
"$schema": "http://json-schema.org/draft-06/schema#",
358+
"allOf": [
359+
{ "type": "number" },
360+
{ "type": "number" },
361+
{ "type": "string" }
362+
]
363+
})JSON");
364+
365+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
366+
367+
const auto expected = sourcemeta::core::parse_json(R"JSON({
368+
"$schema": "http://json-schema.org/draft-06/schema#",
369+
"type": "number",
370+
"multipleOf": 1,
371+
"allOf": [
372+
{ "type": "string", "minLength": 0 }
373+
]
374+
})JSON");
375+
376+
EXPECT_EQ(document, expected);
377+
}
378+
379+
TEST(AlterSchema_lint_draft6, duplicate_allof_branches_4) {
380+
auto document = sourcemeta::core::parse_json(R"JSON({
381+
"$schema": "http://json-schema.org/draft-06/schema#",
382+
"allOf": [
383+
{ "type": "number" },
384+
{ "type": "string" },
385+
{ "type": "number" },
386+
{ "type": "number" },
387+
{ "type": "number" },
388+
{ "type": "string" },
389+
{ "type": "string" },
390+
{ "type": "string" },
391+
{ "type": "number" },
392+
{ "type": "number" }
393+
]
394+
})JSON");
395+
396+
LINT_AND_FIX_FOR_STATIC_ANALYSIS(document);
397+
398+
const auto expected = sourcemeta::core::parse_json(R"JSON({
399+
"$schema": "http://json-schema.org/draft-06/schema#",
400+
"type": "number",
401+
"multipleOf": 1,
402+
"allOf": [
403+
{ "type": "string", "minLength": 0 }
404+
]
405+
})JSON");
406+
407+
EXPECT_EQ(document, expected);
408+
}
409+
331410
TEST(AlterSchema_lint_draft6, duplicate_anyof_branches_1) {
332411
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
333412
"$schema": "http://json-schema.org/draft-06/schema#",

0 commit comments

Comments
 (0)