Skip to content

Commit 85b02e0

Browse files
jviottijviotti
authored
Expand SchemaTransformRule::Result into a full struct (#1957)
Signed-off-by: Juan Cruz Viotti <[email protected]>
1 parent 40e1142 commit 85b02e0

File tree

4 files changed

+47
-59
lines changed

4 files changed

+47
-59
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include <string> // std::string
2121
#include <string_view> // std::string_view
2222
#include <utility> // std::move, std::forward, std::pair
23-
#include <variant> // std::variant
2423
#include <vector> // std::vector
2524

2625
namespace sourcemeta::core {
@@ -83,7 +82,15 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaTransformRule {
8382
[[nodiscard]] auto message() const -> const std::string &;
8483

8584
/// The result of evaluating a rule
86-
using Result = std::variant<bool, std::string>;
85+
struct Result {
86+
Result(const bool applies_) : applies{applies_} {}
87+
Result(std::string description_)
88+
: applies{true}, description{std::move(description_)} {}
89+
Result(const char *description_)
90+
: applies{true}, description{description_} {}
91+
bool applies;
92+
std::string description{""};
93+
};
8794

8895
/// Apply the rule to a schema
8996
auto apply(JSON &schema, const JSON &root, const Vocabularies &vocabularies,
@@ -224,10 +231,11 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaTransformer {
224231
/// - The JSON Pointer to the given subschema
225232
/// - The name of the rule
226233
/// - The message of the rule
234+
/// - The rule evaluation result
227235
/// - The longer description of the rule (if any)
228-
using Callback =
229-
std::function<void(const Pointer &, const std::string_view,
230-
const std::string_view, const std::string_view)>;
236+
using Callback = std::function<void(const Pointer &, const std::string_view,
237+
const std::string_view,
238+
const SchemaTransformRule::Result &)>;
231239

232240
/// Apply the bundle of rules to a schema
233241
auto apply(JSON &schema, const SchemaWalker &walker,

‎src/core/jsonschema/transformer.cc

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,6 @@
99

1010
namespace {
1111

12-
auto is_true(const sourcemeta::core::SchemaTransformRule::Result &result)
13-
-> bool {
14-
switch (result.index()) {
15-
case 0:
16-
assert(std::holds_alternative<bool>(result));
17-
return *std::get_if<bool>(&result);
18-
default:
19-
assert(std::holds_alternative<std::string>(result));
20-
return true;
21-
}
22-
}
23-
2412
auto calculate_health_percentage(const std::size_t subschemas,
2513
const std::size_t failed_subschemas)
2614
-> std::uint8_t {
@@ -71,7 +59,7 @@ auto SchemaTransformRule::apply(JSON &schema, const JSON &root,
7159
-> std::pair<bool, Result> {
7260
auto outcome{this->condition(schema, root, vocabularies, frame, location,
7361
walker, resolver)};
74-
if (!is_true(outcome)) {
62+
if (!outcome.applies) {
7563
return {true, std::move(outcome)};
7664
}
7765

@@ -83,8 +71,9 @@ auto SchemaTransformRule::apply(JSON &schema, const JSON &root,
8371

8472
// The condition must always be false after applying the
8573
// transformation in order to avoid infinite loops
86-
if (is_true(this->condition(schema, root, vocabularies, frame, location,
87-
walker, resolver))) {
74+
if (this->condition(schema, root, vocabularies, frame, location, walker,
75+
resolver)
76+
.applies) {
8877
// TODO: Throw a better custom error that also highlights the schema
8978
// location
9079
std::ostringstream error;
@@ -133,21 +122,9 @@ auto SchemaTransformer::check(
133122
for (const auto &[name, rule] : this->rules) {
134123
const auto outcome{rule->check(current, schema, current_vocabularies,
135124
walker, resolver, frame, entry.second)};
136-
switch (outcome.index()) {
137-
case 0:
138-
assert(std::holds_alternative<bool>(outcome));
139-
if (*std::get_if<bool>(&outcome)) {
140-
subresult = false;
141-
callback(entry.second.pointer, name, rule->message(), "");
142-
}
143-
144-
break;
145-
default:
146-
assert(std::holds_alternative<std::string>(outcome));
147-
subresult = false;
148-
callback(entry.second.pointer, name, rule->message(),
149-
*std::get_if<std::string>(&outcome));
150-
break;
125+
if (outcome.applies) {
126+
subresult = false;
127+
callback(entry.second.pointer, name, rule->message(), outcome);
151128
}
152129
}
153130

@@ -191,13 +168,11 @@ auto SchemaTransformer::apply(
191168
entry.second)};
192169
// This means the rule is fixable
193170
if (subresult.first) {
194-
applied = is_true(subresult.second) || applied;
171+
applied = subresult.second.applies || applied;
195172
} else {
196173
result = false;
197174
callback(entry.second.pointer, name, rule->message(),
198-
subresult.second.index() == 0
199-
? ""
200-
: *std::get_if<std::string>(&subresult.second));
175+
subresult.second);
201176
}
202177

203178
if (!applied) {

‎test/alterschema/alterschema_test_utils.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
#define LINT_WITHOUT_FIX_FOR_READABILITY(document, result, traces) \
1111
std::vector<std::tuple<sourcemeta::core::Pointer, std::string, std::string, \
12-
std::string>> \
12+
sourcemeta::core::SchemaTransformRule::Result>> \
1313
traces; \
1414
sourcemeta::core::SchemaTransformer bundle; \
1515
sourcemeta::core::add(bundle, \
@@ -18,16 +18,18 @@
1818
bundle.check(document, sourcemeta::core::schema_official_walker, \
1919
sourcemeta::core::schema_official_resolver, \
2020
[&traces](const auto &pointer, const auto &name, \
21-
const auto &message, const auto &description) { \
22-
traces.emplace_back(pointer, name, message, description); \
21+
const auto &message, const auto &outcome) { \
22+
traces.emplace_back(pointer, name, message, outcome); \
2323
});
2424

25-
#define EXPECT_LINT_TRACE(traces, index, pointer, name, message, description) \
25+
#define EXPECT_LINT_TRACE(traces, index, pointer, name, message, \
26+
expected_description) \
2627
EXPECT_EQ(sourcemeta::core::to_string(std::get<0>((traces).at(index))), \
2728
(pointer)); \
2829
EXPECT_EQ(std::get<1>((traces).at(index)), (name)); \
2930
EXPECT_EQ(std::get<2>((traces).at(index)), (message)); \
30-
EXPECT_EQ(std::get<3>((traces).at(index)), (description));
31+
EXPECT_EQ(std::get<3>((traces).at(index)).description, \
32+
(expected_description));
3133

3234
#define LINT_AND_FIX_FOR_READABILITY(traces) \
3335
sourcemeta::core::SchemaTransformer bundle; \

‎test/jsonschema/jsonschema_transformer_test.cc

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,20 @@
1010

1111
#include "jsonschema_transform_rules.h"
1212

13-
static auto transformer_callback_noop(const sourcemeta::core::Pointer &,
14-
const std::string_view,
15-
const std::string_view,
16-
const std::string_view) -> void {}
13+
static auto
14+
transformer_callback_noop(const sourcemeta::core::Pointer &,
15+
const std::string_view, const std::string_view,
16+
const sourcemeta::core::SchemaTransformRule::Result &)
17+
-> void {}
1718

1819
using TestTransformTraces =
1920
std::vector<std::tuple<sourcemeta::core::Pointer, std::string, std::string,
20-
std::string>>;
21+
sourcemeta::core::SchemaTransformRule::Result>>;
2122

2223
static auto transformer_callback_trace(TestTransformTraces &traces) -> auto {
2324
return [&traces](const auto &pointer, const auto &name, const auto &message,
24-
const auto &description) {
25-
traces.emplace_back(pointer, name, message, description);
25+
const auto &result) {
26+
traces.emplace_back(pointer, name, message, result);
2627
};
2728
}
2829

@@ -482,13 +483,14 @@ TEST(JSONSchema_transformer, check_top_level) {
482483
EXPECT_EQ(std::get<0>(entries.at(0)), sourcemeta::core::Pointer{});
483484
EXPECT_EQ(std::get<1>(entries.at(0)), "example_rule_1");
484485
EXPECT_EQ(std::get<2>(entries.at(0)), "Keyword foo is not permitted");
485-
EXPECT_EQ(std::get<3>(entries.at(0)), "This is a message from the rule");
486+
EXPECT_EQ(std::get<3>(entries.at(0)).description,
487+
"This is a message from the rule");
486488

487489
EXPECT_EQ(std::get<0>(entries.at(1)),
488490
sourcemeta::core::Pointer({"properties", "xxx"}));
489491
EXPECT_EQ(std::get<1>(entries.at(1)), "example_rule_2");
490492
EXPECT_EQ(std::get<2>(entries.at(1)), "Keyword bar is not permitted");
491-
EXPECT_EQ(std::get<3>(entries.at(1)), "");
493+
EXPECT_EQ(std::get<3>(entries.at(1)).description, "");
492494
}
493495

494496
TEST(JSONSchema_transformer, check_no_match) {
@@ -552,7 +554,7 @@ TEST(JSONSchema_transformer, check_partial_match) {
552554
sourcemeta::core::Pointer({"properties", "bar"}));
553555
EXPECT_EQ(std::get<1>(entries.at(0)), "example_rule_1");
554556
EXPECT_EQ(std::get<2>(entries.at(0)), "Keyword foo is not permitted");
555-
EXPECT_EQ(std::get<3>(entries.at(0)), "");
557+
EXPECT_EQ(std::get<3>(entries.at(0)).description, "");
556558
}
557559

558560
TEST(JSONSchema_transformer, check_empty) {
@@ -621,13 +623,13 @@ TEST(JSONSchema_transformer, check_with_default_dialect) {
621623
EXPECT_EQ(std::get<0>(entries.at(0)), sourcemeta::core::Pointer{});
622624
EXPECT_EQ(std::get<1>(entries.at(0)), "example_rule_1");
623625
EXPECT_EQ(std::get<2>(entries.at(0)), "Keyword foo is not permitted");
624-
EXPECT_EQ(std::get<3>(entries.at(0)), "");
626+
EXPECT_EQ(std::get<3>(entries.at(0)).description, "");
625627

626628
EXPECT_EQ(std::get<0>(entries.at(1)),
627629
sourcemeta::core::Pointer({"properties", "xxx"}));
628630
EXPECT_EQ(std::get<1>(entries.at(1)), "example_rule_2");
629631
EXPECT_EQ(std::get<2>(entries.at(1)), "Keyword bar is not permitted");
630-
EXPECT_EQ(std::get<3>(entries.at(1)), "");
632+
EXPECT_EQ(std::get<3>(entries.at(1)).description, "");
631633
}
632634

633635
TEST(JSONSchema_transformer, remove_rule_by_name) {
@@ -687,7 +689,7 @@ TEST(JSONSchema_transformer, unfixable_apply_without_description) {
687689
EXPECT_EQ(std::get<0>(entries.at(0)), sourcemeta::core::Pointer{});
688690
EXPECT_EQ(std::get<1>(entries.at(0)), "example_rule_unfixable_1");
689691
EXPECT_EQ(std::get<2>(entries.at(0)), "An example rule that cannot be fixed");
690-
EXPECT_EQ(std::get<3>(entries.at(0)), "");
692+
EXPECT_EQ(std::get<3>(entries.at(0)).description, "");
691693

692694
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
693695
"$schema": "https://json-schema.org/draft/2020-12/schema",
@@ -723,7 +725,8 @@ TEST(JSONSchema_transformer, unfixable_apply_with_description) {
723725
EXPECT_EQ(std::get<1>(entries.at(0)),
724726
"example_rule_unfixable_with_description_1");
725727
EXPECT_EQ(std::get<2>(entries.at(0)), "An example rule that cannot be fixed");
726-
EXPECT_EQ(std::get<3>(entries.at(0)), "The subschema cannot define foo");
728+
EXPECT_EQ(std::get<3>(entries.at(0)).description,
729+
"The subschema cannot define foo");
727730

728731
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
729732
"$schema": "https://json-schema.org/draft/2020-12/schema",
@@ -758,7 +761,7 @@ TEST(JSONSchema_transformer, unfixable_check) {
758761
EXPECT_EQ(std::get<0>(entries.at(0)), sourcemeta::core::Pointer{});
759762
EXPECT_EQ(std::get<1>(entries.at(0)), "example_rule_unfixable_1");
760763
EXPECT_EQ(std::get<2>(entries.at(0)), "An example rule that cannot be fixed");
761-
EXPECT_EQ(std::get<3>(entries.at(0)), "");
764+
EXPECT_EQ(std::get<3>(entries.at(0)).description, "");
762765
}
763766

764767
TEST(JSONSchema_transformer, rereference_not_hit) {

0 commit comments

Comments
 (0)