Skip to content

Commit 80de55d

Browse files
jviottijviotti
authored
Implement a linting rule to merge up properties within allOf (#1838)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent b655f9e commit 80de55d

8 files changed

+441
-0
lines changed

‎src/extension/alterschema/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ sourcemeta_library(NAMESPACE sourcemeta PROJECT core NAME alterschema
5050
linter/non_applicable_type_specific_keywords.h
5151
linter/unnecessary_allof_wrapper_modern.h
5252
linter/unnecessary_allof_wrapper_draft.h
53+
linter/unnecessary_allof_wrapper_properties.h
5354
linter/duplicate_allof_branches.h
5455
linter/duplicate_anyof_branches.h
5556
linter/else_without_if.h

‎src/extension/alterschema/alterschema.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ contains_any(const Vocabularies &container,
7171
#include "linter/unevaluated_properties_default.h"
7272
#include "linter/unnecessary_allof_wrapper_draft.h"
7373
#include "linter/unnecessary_allof_wrapper_modern.h"
74+
#include "linter/unnecessary_allof_wrapper_properties.h"
7475
#include "linter/unsatisfiable_max_contains.h"
7576
#include "linter/unsatisfiable_min_properties.h"
7677
} // namespace sourcemeta::core
@@ -86,6 +87,7 @@ auto add(SchemaTransformer &bundle, const AlterSchemaMode mode)
8687
bundle.add<NonApplicableTypeSpecificKeywords>();
8788
bundle.add<UnnecessaryAllOfWrapperModern>();
8889
bundle.add<UnnecessaryAllOfWrapperDraft>();
90+
bundle.add<UnnecessaryAllOfWrapperProperties>();
8991
bundle.add<DuplicateAllOfBranches>();
9092
bundle.add<DuplicateAnyOfBranches>();
9193
bundle.add<ElseWithoutIf>();

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
class UnnecessaryAllOfWrapperProperties final : public SchemaTransformRule {
2+
public:
3+
UnnecessaryAllOfWrapperProperties()
4+
: SchemaTransformRule{
5+
"unnecessary_allof_wrapper_properties",
6+
"Avoid unnecessarily wrapping object `properties` in `allOf`"} {};
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+
// TODO: Make this work on older dialects. Right we can't do that
20+
// safely for Draft 7 and earlier if `properties` is a sibling of
21+
// `$ref`
22+
{"https://json-schema.org/draft/2020-12/vocab/applicator",
23+
"https://json-schema.org/draft/2019-09/vocab/applicator"}) &&
24+
schema.is_object() && schema.defines("allOf") &&
25+
schema.at("allOf").is_array() && schema.defines("properties") &&
26+
schema.at("properties").is_object()) {
27+
for (const auto &entry : schema.at("allOf").as_array()) {
28+
if (entry.is_object() && entry.defines("properties") &&
29+
entry.at("properties").is_object()) {
30+
for (const auto &subentry : entry.at("properties").as_object()) {
31+
if (!schema.at("properties").defines(subentry.first)) {
32+
return true;
33+
}
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() && entry.defines("properties")) {
45+
std::vector<JSON::String> blacklist;
46+
for (const auto &subentry : entry.at("properties").as_object()) {
47+
if (!schema.at("properties").defines(subentry.first)) {
48+
blacklist.push_back(subentry.first);
49+
}
50+
}
51+
52+
for (auto &property : blacklist) {
53+
schema.at("properties")
54+
.assign(property, entry.at("properties").at(property));
55+
entry.at("properties").erase(property);
56+
}
57+
58+
if (entry.at("properties").empty()) {
59+
entry.erase("properties");
60+
}
61+
}
62+
}
63+
64+
schema.at("allOf").erase_if(sourcemeta::core::is_empty_schema);
65+
66+
if (schema.at("allOf").empty()) {
67+
schema.erase("allOf");
68+
}
69+
}
70+
};

‎test/alterschema/alterschema_lint_2019_09_test.cc

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,3 +1749,133 @@ TEST(AlterSchema_lint_2019_09, multiple_of_default_no_change_1) {
17491749

17501750
EXPECT_EQ(document, expected);
17511751
}
1752+
1753+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_1) {
1754+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
1755+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1756+
"properties": {
1757+
"foo": { "type": "string" }
1758+
},
1759+
"allOf": [
1760+
{
1761+
"properties": {
1762+
"bar": { "type": "string" }
1763+
}
1764+
}
1765+
]
1766+
})JSON");
1767+
1768+
LINT_AND_FIX_FOR_READABILITY(document);
1769+
1770+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
1771+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1772+
"properties": {
1773+
"foo": { "type": "string" },
1774+
"bar": { "type": "string" }
1775+
}
1776+
})JSON");
1777+
1778+
EXPECT_EQ(document, expected);
1779+
}
1780+
1781+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_2) {
1782+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
1783+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1784+
"properties": {
1785+
"foo": { "type": "string" }
1786+
},
1787+
"allOf": [
1788+
{
1789+
"properties": {
1790+
"foo": { "minLength": 3 }
1791+
}
1792+
}
1793+
]
1794+
})JSON");
1795+
1796+
LINT_AND_FIX_FOR_READABILITY(document);
1797+
1798+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
1799+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1800+
"properties": {
1801+
"foo": { "type": "string" }
1802+
},
1803+
"allOf": [
1804+
{
1805+
"properties": {
1806+
"foo": { "minLength": 3 }
1807+
}
1808+
}
1809+
]
1810+
})JSON");
1811+
1812+
EXPECT_EQ(document, expected);
1813+
}
1814+
1815+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_3) {
1816+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
1817+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1818+
"properties": {
1819+
"foo": { "type": "string" }
1820+
},
1821+
"allOf": [
1822+
{
1823+
"properties": {
1824+
"bar": { "minLength": 3 },
1825+
"baz": { "maxLength": 5 }
1826+
}
1827+
},
1828+
{ "type": "object" },
1829+
{
1830+
"properties": {
1831+
"qux": { "pattern": "^f" }
1832+
}
1833+
}
1834+
]
1835+
})JSON");
1836+
1837+
LINT_AND_FIX_FOR_READABILITY(document);
1838+
1839+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
1840+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1841+
"type": "object",
1842+
"properties": {
1843+
"foo": { "type": "string" },
1844+
"bar": { "minLength": 3 },
1845+
"baz": { "maxLength": 5 },
1846+
"qux": { "pattern": "^f" }
1847+
}
1848+
})JSON");
1849+
1850+
EXPECT_EQ(document, expected);
1851+
}
1852+
1853+
TEST(AlterSchema_lint_2019_09, unnecessary_allof_wrapper_properties_4) {
1854+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
1855+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1856+
"properties": {
1857+
"foo": { "type": "string" }
1858+
},
1859+
"allOf": [
1860+
{
1861+
"$ref": "https://example.com",
1862+
"properties": {
1863+
"bar": { "pattern": "^f" }
1864+
}
1865+
}
1866+
]
1867+
})JSON");
1868+
1869+
LINT_AND_FIX_FOR_READABILITY(document);
1870+
1871+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
1872+
"$schema": "https://json-schema.org/draft/2019-09/schema",
1873+
"$ref": "https://example.com",
1874+
"properties": {
1875+
"foo": { "type": "string" },
1876+
"bar": { "pattern": "^f" }
1877+
}
1878+
})JSON");
1879+
1880+
EXPECT_EQ(document, expected);
1881+
}

‎test/alterschema/alterschema_lint_2020_12_test.cc

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,3 +2012,133 @@ TEST(AlterSchema_lint_2020_12, multiple_of_default_no_change_numeric_value) {
20122012

20132013
EXPECT_EQ(document, expected);
20142014
}
2015+
2016+
TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_1) {
2017+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
2018+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2019+
"properties": {
2020+
"foo": { "type": "string" }
2021+
},
2022+
"allOf": [
2023+
{
2024+
"properties": {
2025+
"bar": { "type": "string" }
2026+
}
2027+
}
2028+
]
2029+
})JSON");
2030+
2031+
LINT_AND_FIX_FOR_READABILITY(document);
2032+
2033+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
2034+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2035+
"properties": {
2036+
"foo": { "type": "string" },
2037+
"bar": { "type": "string" }
2038+
}
2039+
})JSON");
2040+
2041+
EXPECT_EQ(document, expected);
2042+
}
2043+
2044+
TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_2) {
2045+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
2046+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2047+
"properties": {
2048+
"foo": { "type": "string" }
2049+
},
2050+
"allOf": [
2051+
{
2052+
"properties": {
2053+
"foo": { "minLength": 3 }
2054+
}
2055+
}
2056+
]
2057+
})JSON");
2058+
2059+
LINT_AND_FIX_FOR_READABILITY(document);
2060+
2061+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
2062+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2063+
"properties": {
2064+
"foo": { "type": "string" }
2065+
},
2066+
"allOf": [
2067+
{
2068+
"properties": {
2069+
"foo": { "minLength": 3 }
2070+
}
2071+
}
2072+
]
2073+
})JSON");
2074+
2075+
EXPECT_EQ(document, expected);
2076+
}
2077+
2078+
TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_3) {
2079+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
2080+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2081+
"properties": {
2082+
"foo": { "type": "string" }
2083+
},
2084+
"allOf": [
2085+
{
2086+
"properties": {
2087+
"bar": { "minLength": 3 },
2088+
"baz": { "maxLength": 5 }
2089+
}
2090+
},
2091+
{ "type": "object" },
2092+
{
2093+
"properties": {
2094+
"qux": { "pattern": "^f" }
2095+
}
2096+
}
2097+
]
2098+
})JSON");
2099+
2100+
LINT_AND_FIX_FOR_READABILITY(document);
2101+
2102+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
2103+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2104+
"type": "object",
2105+
"properties": {
2106+
"foo": { "type": "string" },
2107+
"bar": { "minLength": 3 },
2108+
"baz": { "maxLength": 5 },
2109+
"qux": { "pattern": "^f" }
2110+
}
2111+
})JSON");
2112+
2113+
EXPECT_EQ(document, expected);
2114+
}
2115+
2116+
TEST(AlterSchema_lint_2020_12, unnecessary_allof_wrapper_properties_4) {
2117+
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
2118+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2119+
"properties": {
2120+
"foo": { "type": "string" }
2121+
},
2122+
"allOf": [
2123+
{
2124+
"$ref": "https://example.com",
2125+
"properties": {
2126+
"bar": { "pattern": "^f" }
2127+
}
2128+
}
2129+
]
2130+
})JSON");
2131+
2132+
LINT_AND_FIX_FOR_READABILITY(document);
2133+
2134+
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
2135+
"$schema": "https://json-schema.org/draft/2020-12/schema",
2136+
"$ref": "https://example.com",
2137+
"properties": {
2138+
"foo": { "type": "string" },
2139+
"bar": { "pattern": "^f" }
2140+
}
2141+
})JSON");
2142+
2143+
EXPECT_EQ(document, expected);
2144+
}

0 commit comments

Comments
 (0)