Skip to content

Commit 4e376a9

Browse files
authored
Implement a linter rule for simpler property names (#2139)
Signed-off-by: Juan Cruz Viotti <[email protected]>
1 parent ef1081e commit 4e376a9

14 files changed

+782
-19
lines changed

config.cmake.in

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ foreach(component ${SOURCEMETA_CORE_COMPONENTS})
9292
find_dependency(mpdecimal CONFIG)
9393
include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_numeric.cmake")
9494
include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_io.cmake")
95+
find_dependency(PCRE2 CONFIG)
96+
include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_regex.cmake")
9597
include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_json.cmake")
9698
include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_jsonpointer.cmake")
9799
include("${CMAKE_CURRENT_LIST_DIR}/sourcemeta_core_jsonschema.cmake")

src/core/jsonschema/include/sourcemeta/core/jsonschema_transform.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,11 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaTransformer {
251251
const SchemaTransformRule::Result &)>;
252252

253253
/// Apply the bundle of rules to a schema
254-
auto apply(JSON &schema, const SchemaWalker &walker,
255-
const SchemaResolver &resolver, const Callback &callback,
256-
const std::optional<JSON::String> &default_dialect = std::nullopt,
257-
const std::optional<JSON::String> &default_id = std::nullopt) const
254+
[[nodiscard]] auto
255+
apply(JSON &schema, const SchemaWalker &walker,
256+
const SchemaResolver &resolver, const Callback &callback,
257+
const std::optional<JSON::String> &default_dialect = std::nullopt,
258+
const std::optional<JSON::String> &default_id = std::nullopt) const
258259
-> std::pair<bool, std::uint8_t>;
259260

260261
/// Report back the rules from the bundle that need to be applied to a schema

src/extension/alterschema/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ sourcemeta_library(NAMESPACE sourcemeta PROJECT core NAME alterschema
5151
common/non_applicable_type_specific_keywords.h
5252
common/not_false.h
5353
common/required_properties_in_properties.h
54+
common/simple_properties_identifiers.h
5455
common/single_type_array.h
5556
common/then_empty.h
5657
common/then_without_if.h
@@ -96,3 +97,5 @@ endif()
9697

9798
target_link_libraries(sourcemeta_core_alterschema PUBLIC
9899
sourcemeta::core::jsonschema)
100+
target_link_libraries(sourcemeta_core_alterschema PRIVATE
101+
sourcemeta::core::regex)

src/extension/alterschema/alterschema.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <sourcemeta/core/alterschema.h>
2+
#include <sourcemeta/core/regex.h>
23

34
// For built-in rules
45
#include <algorithm> // std::sort, std::unique
@@ -78,6 +79,7 @@ inline auto APPLIES_TO_POINTERS(std::vector<Pointer> &&keywords)
7879
#include "common/not_false.h"
7980
#include "common/orphan_definitions.h"
8081
#include "common/required_properties_in_properties.h"
82+
#include "common/simple_properties_identifiers.h"
8183
#include "common/single_type_array.h"
8284
#include "common/then_empty.h"
8385
#include "common/then_without_if.h"
@@ -166,6 +168,7 @@ auto add(SchemaTransformer &bundle, const AlterSchemaMode mode) -> void {
166168
bundle.add<UnknownKeywordsPrefix>();
167169
bundle.add<UnknownLocalRef>();
168170
bundle.add<RequiredPropertiesInProperties>();
171+
bundle.add<SimplePropertiesIdentifiers>();
169172
bundle.add<OrphanDefinitions>();
170173

171174
if (mode == AlterSchemaMode::Canonicalizer) {
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
class SimplePropertiesIdentifiers final : public SchemaTransformRule {
2+
public:
3+
SimplePropertiesIdentifiers()
4+
// Inspired by
5+
// https://json-structure.github.io/core/draft-vasters-json-structure-core.html#section-3.6
6+
: SchemaTransformRule{
7+
"simple_properties_identifiers",
8+
"Set `properties` to identifier names that can be easily mapped to "
9+
"programming languages (matching [A-Za-z_][A-Za-z0-9_]*)"} {};
10+
11+
[[nodiscard]] auto
12+
condition(const sourcemeta::core::JSON &schema,
13+
const sourcemeta::core::JSON &root,
14+
const sourcemeta::core::Vocabularies &vocabularies,
15+
const sourcemeta::core::SchemaFrame &frame,
16+
const sourcemeta::core::SchemaFrame::Location &location,
17+
const sourcemeta::core::SchemaWalker &,
18+
const sourcemeta::core::SchemaResolver &) const
19+
-> sourcemeta::core::SchemaTransformRule::Result override {
20+
ONLY_CONTINUE_IF(vocabularies.contains_any(
21+
{Vocabularies::Known::JSON_Schema_2020_12_Applicator,
22+
Vocabularies::Known::JSON_Schema_2019_09_Applicator,
23+
Vocabularies::Known::JSON_Schema_Draft_7,
24+
Vocabularies::Known::JSON_Schema_Draft_6,
25+
Vocabularies::Known::JSON_Schema_Draft_4,
26+
Vocabularies::Known::JSON_Schema_Draft_3,
27+
Vocabularies::Known::JSON_Schema_Draft_2,
28+
Vocabularies::Known::JSON_Schema_Draft_2_Hyper,
29+
Vocabularies::Known::JSON_Schema_Draft_1,
30+
Vocabularies::Known::JSON_Schema_Draft_1_Hyper}));
31+
ONLY_CONTINUE_IF(schema.is_object() && schema.defines("properties") &&
32+
schema.at("properties").is_object() &&
33+
!schema.at("properties").empty());
34+
35+
if (vocabularies.contains_any(
36+
{Vocabularies::Known::JSON_Schema_2020_12_Core,
37+
Vocabularies::Known::JSON_Schema_2019_09_Core})) {
38+
// Skip meta-schemas with `$vocabulary` (2019-09+)
39+
// We check the current schema resource (not root) to handle bundled
40+
// schemas
41+
const auto base_location{frame.traverse(location.base)};
42+
if (base_location.has_value()) {
43+
const auto &resource{get(root, base_location->get().pointer)};
44+
ONLY_CONTINUE_IF(!resource.is_object() ||
45+
!resource.defines("$vocabulary"));
46+
}
47+
} else {
48+
// Skip pre-vocabulary meta-schemas
49+
ONLY_CONTINUE_IF(location.base != location.dialect &&
50+
(location.base + "#") != location.dialect);
51+
}
52+
53+
std::vector<Pointer> offenders;
54+
for (const auto &entry : schema.at("properties").as_object()) {
55+
static const Regex IDENTIFIER_PATTERN{
56+
to_regex("^[A-Za-z_][A-Za-z0-9_]*$").value()};
57+
if (!matches(IDENTIFIER_PATTERN, entry.first)) {
58+
offenders.push_back(Pointer{"properties", entry.first});
59+
}
60+
}
61+
62+
ONLY_CONTINUE_IF(!offenders.empty());
63+
return APPLIES_TO_POINTERS(std::move(offenders));
64+
}
65+
};

test/alterschema/alterschema_canonicalize_2020_12_test.cc

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,3 +1322,48 @@ TEST(AlterSchema_canonicalize_2020_12,
13221322

13231323
EXPECT_EQ(document, expected);
13241324
}
1325+
1326+
TEST(AlterSchema_canonicalize_2020_12,
1327+
simple_properties_identifiers_skip_metaschema) {
1328+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
1329+
"$schema": "https://json-schema.org/draft/2020-12/schema",
1330+
"$id": "https://example.com/my-metaschema",
1331+
"$vocabulary": {
1332+
"https://json-schema.org/draft/2020-12/vocab/core": true,
1333+
"https://json-schema.org/draft/2020-12/vocab/applicator": true,
1334+
"https://json-schema.org/draft/2020-12/vocab/validation": true
1335+
},
1336+
"type": "object",
1337+
"minProperties": 0,
1338+
"properties": {
1339+
"foo-bar": { "type": "string", "minLength": 0 }
1340+
}
1341+
})JSON");
1342+
1343+
CANONICALIZE_WITHOUT_FIX(document, result, traces);
1344+
1345+
EXPECT_TRUE(result.first);
1346+
EXPECT_EQ(traces.size(), 0);
1347+
}
1348+
1349+
TEST(AlterSchema_canonicalize_2020_12,
1350+
simple_properties_identifiers_applies_non_meta) {
1351+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
1352+
"$schema": "https://json-schema.org/draft/2020-12/schema",
1353+
"$id": "https://example.com/my-schema",
1354+
"type": "object",
1355+
"minProperties": 0,
1356+
"properties": {
1357+
"foo-bar": { "type": "string", "minLength": 0 }
1358+
}
1359+
})JSON");
1360+
1361+
CANONICALIZE_WITHOUT_FIX(document, result, traces);
1362+
1363+
EXPECT_FALSE(result.first);
1364+
EXPECT_EQ(traces.size(), 1);
1365+
EXPECT_LINT_TRACE(traces, 0, "", "simple_properties_identifiers",
1366+
"Set `properties` to identifier names that can be easily "
1367+
"mapped to programming languages (matching "
1368+
"[A-Za-z_][A-Za-z0-9_]*)");
1369+
}

test/alterschema/alterschema_canonicalize_draft7_test.cc

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,3 +569,43 @@ TEST(AlterSchema_canonicalize_draft7, min_properties_implicit_2) {
569569

570570
EXPECT_EQ(document, expected);
571571
}
572+
573+
TEST(AlterSchema_canonicalize_draft7,
574+
simple_properties_identifiers_skip_metaschema) {
575+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
576+
"$schema": "http://json-schema.org/draft-07/schema#",
577+
"$id": "http://json-schema.org/draft-07/schema#",
578+
"type": "object",
579+
"minProperties": 0,
580+
"properties": {
581+
"foo-bar": { "type": "string", "minLength": 0 }
582+
}
583+
})JSON");
584+
585+
CANONICALIZE_WITHOUT_FIX(document, result, traces);
586+
587+
EXPECT_TRUE(result.first);
588+
EXPECT_EQ(traces.size(), 0);
589+
}
590+
591+
TEST(AlterSchema_canonicalize_draft7,
592+
simple_properties_identifiers_applies_non_meta) {
593+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
594+
"$schema": "http://json-schema.org/draft-07/schema#",
595+
"$id": "https://example.com/my-schema",
596+
"type": "object",
597+
"minProperties": 0,
598+
"properties": {
599+
"foo-bar": { "type": "string", "minLength": 0 }
600+
}
601+
})JSON");
602+
603+
CANONICALIZE_WITHOUT_FIX(document, result, traces);
604+
605+
EXPECT_FALSE(result.first);
606+
EXPECT_EQ(traces.size(), 1);
607+
EXPECT_LINT_TRACE(traces, 0, "", "simple_properties_identifiers",
608+
"Set `properties` to identifier names that can be easily "
609+
"mapped to programming languages (matching "
610+
"[A-Za-z_][A-Za-z0-9_]*)");
611+
}

test/alterschema/alterschema_lint_2019_09_test.cc

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3896,3 +3896,104 @@ TEST(AlterSchema_lint_2019_09,
38963896

38973897
EXPECT_EQ(document, expected);
38983898
}
3899+
3900+
TEST(AlterSchema_lint_2019_09, simple_properties_identifiers_skip_metaschema) {
3901+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
3902+
"$schema": "https://json-schema.org/draft/2019-09/schema",
3903+
"$id": "https://example.com/my-metaschema",
3904+
"title": "My meta-schema",
3905+
"description": "A custom meta-schema",
3906+
"examples": [ {} ],
3907+
"$vocabulary": {
3908+
"https://json-schema.org/draft/2019-09/vocab/core": true,
3909+
"https://json-schema.org/draft/2019-09/vocab/applicator": true
3910+
},
3911+
"properties": {
3912+
"foo-bar": { "type": "string" }
3913+
}
3914+
})JSON");
3915+
3916+
LINT_WITHOUT_FIX(document, result, traces);
3917+
3918+
EXPECT_TRUE(result.first);
3919+
EXPECT_EQ(traces.size(), 0);
3920+
}
3921+
3922+
TEST(AlterSchema_lint_2019_09,
3923+
simple_properties_identifiers_skip_metaschema_nested) {
3924+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
3925+
"$schema": "https://json-schema.org/draft/2019-09/schema",
3926+
"$id": "https://example.com/my-metaschema",
3927+
"title": "My meta-schema",
3928+
"description": "A custom meta-schema",
3929+
"examples": [ {} ],
3930+
"$vocabulary": {
3931+
"https://json-schema.org/draft/2019-09/vocab/core": true,
3932+
"https://json-schema.org/draft/2019-09/vocab/applicator": true
3933+
},
3934+
"properties": {
3935+
"valid": {
3936+
"properties": {
3937+
"also-invalid": { "type": "number" }
3938+
}
3939+
}
3940+
}
3941+
})JSON");
3942+
3943+
LINT_WITHOUT_FIX(document, result, traces);
3944+
3945+
EXPECT_TRUE(result.first);
3946+
EXPECT_EQ(traces.size(), 0);
3947+
}
3948+
3949+
TEST(AlterSchema_lint_2019_09, simple_properties_identifiers_applies_non_meta) {
3950+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
3951+
"$schema": "https://json-schema.org/draft/2019-09/schema",
3952+
"$id": "https://example.com/my-schema",
3953+
"title": "Test",
3954+
"description": "A test schema",
3955+
"examples": [ {} ],
3956+
"properties": {
3957+
"foo-bar": { "type": "string" }
3958+
}
3959+
})JSON");
3960+
3961+
LINT_WITHOUT_FIX(document, result, traces);
3962+
3963+
EXPECT_FALSE(result.first);
3964+
EXPECT_EQ(traces.size(), 1);
3965+
EXPECT_LINT_TRACE(traces, 0, "", "simple_properties_identifiers",
3966+
"Set `properties` to identifier names that can be easily "
3967+
"mapped to programming languages (matching "
3968+
"[A-Za-z_][A-Za-z0-9_]*)");
3969+
}
3970+
3971+
TEST(AlterSchema_lint_2019_09,
3972+
simple_properties_identifiers_skip_bundled_metaschema) {
3973+
const sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
3974+
"$schema": "https://json-schema.org/draft/2019-09/schema",
3975+
"$id": "https://example.com/main",
3976+
"title": "Main schema",
3977+
"description": "A schema that bundles a meta-schema",
3978+
"examples": [ {} ],
3979+
"$ref": "https://example.com/my-metaschema",
3980+
"$defs": {
3981+
"metaschema": {
3982+
"$schema": "https://json-schema.org/draft/2019-09/schema",
3983+
"$id": "https://example.com/my-metaschema",
3984+
"$vocabulary": {
3985+
"https://json-schema.org/draft/2019-09/vocab/core": true,
3986+
"https://json-schema.org/draft/2019-09/vocab/applicator": true
3987+
},
3988+
"properties": {
3989+
"foo-bar": { "type": "string" }
3990+
}
3991+
}
3992+
}
3993+
})JSON");
3994+
3995+
LINT_WITHOUT_FIX(document, result, traces);
3996+
3997+
EXPECT_TRUE(result.first);
3998+
EXPECT_EQ(traces.size(), 0);
3999+
}

0 commit comments

Comments
 (0)