Skip to content

Commit 2b28626

Browse files
jviottijviotti
authored
Provide better error messages for SchemaRule instances (#631)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 34d4922 commit 2b28626

2 files changed

Lines changed: 94 additions & 24 deletions

File tree

‎src/linter/schema.cc‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,19 @@ auto SchemaRule::condition(const sourcemeta::core::JSON &schema,
7373
return false;
7474
}
7575

76-
std::ostringstream message;
76+
std::vector<sourcemeta::core::Pointer> locations;
7777
for (const auto &entry : output) {
78-
message << entry.message << "\n";
79-
message << " at instance location \"";
80-
sourcemeta::core::stringify(entry.instance_location, message);
81-
message << "\"\n";
82-
message << " at evaluate path \"";
83-
sourcemeta::core::stringify(entry.evaluate_path, message);
84-
message << "\"\n";
78+
if (!entry.instance_location.empty()) {
79+
locations.push_back(
80+
sourcemeta::core::to_pointer(entry.instance_location));
81+
}
8582
}
8683

87-
return {{}, std::move(message).str()};
84+
if (output.cbegin() != output.cend()) {
85+
return {std::move(locations), std::string{output.cbegin()->message}};
86+
} else {
87+
return {std::move(locations)};
88+
}
8889
}
8990

9091
} // namespace sourcemeta::blaze

‎test/linter/linter_schema_test.cc‎

Lines changed: 84 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,7 @@ TEST(Linter, schema_rule_fail_root_and_nested_subschema) {
8686
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
8787
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
8888
"The value was expected to be an object that defines the property "
89-
"\"type\"\n"
90-
" at instance location \"\"\n"
91-
" at evaluate path \"/required\"\n");
89+
"\"type\"");
9290
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
9391
EXPECT_FALSE(std::get<4>(entries.at(0)));
9492

@@ -99,9 +97,7 @@ TEST(Linter, schema_rule_fail_root_and_nested_subschema) {
9997
EXPECT_TRUE(std::get<3>(entries.at(1)).description.has_value());
10098
EXPECT_EQ(std::get<3>(entries.at(1)).description.value(),
10199
"The value was expected to be an object that defines the property "
102-
"\"type\"\n"
103-
" at instance location \"\"\n"
104-
" at evaluate path \"/required\"\n");
100+
"\"type\"");
105101
EXPECT_EQ(std::get<3>(entries.at(1)).locations.size(), 0);
106102
EXPECT_FALSE(std::get<4>(entries.at(1)));
107103
}
@@ -177,7 +173,9 @@ TEST(Linter, schema_rule_no_description_fails) {
177173
EXPECT_EQ(std::get<1>(entries.at(0)), "test/no_description");
178174
EXPECT_EQ(std::get<2>(entries.at(0)), "");
179175
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
180-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
176+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
177+
"The value was expected to be an object that defines the property "
178+
"\"type\"");
181179
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
182180
EXPECT_FALSE(std::get<4>(entries.at(0)));
183181
}
@@ -230,7 +228,9 @@ TEST(Linter, schema_rule_nested_property_fails) {
230228
EXPECT_EQ(std::get<2>(entries.at(0)),
231229
"Subschemas must be objects with a type keyword");
232230
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
233-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
231+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
232+
"The value was expected to be an object that defines the property "
233+
"\"type\"");
234234
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
235235
EXPECT_FALSE(std::get<4>(entries.at(0)));
236236
}
@@ -561,7 +561,9 @@ TEST(Linter, schema_rule_non_string_description_integer) {
561561
EXPECT_EQ(std::get<1>(entries.at(0)), "test/integer_desc");
562562
EXPECT_EQ(std::get<2>(entries.at(0)), "42");
563563
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
564-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
564+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
565+
"The value was expected to be an object that defines the property "
566+
"\"type\"");
565567
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
566568
EXPECT_FALSE(std::get<4>(entries.at(0)));
567569
}
@@ -604,7 +606,9 @@ TEST(Linter, schema_rule_non_string_description_boolean) {
604606
EXPECT_EQ(std::get<1>(entries.at(0)), "test/bool_desc");
605607
EXPECT_EQ(std::get<2>(entries.at(0)), "true");
606608
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
607-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
609+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
610+
"The value was expected to be an object that defines the property "
611+
"\"type\"");
608612
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
609613
EXPECT_FALSE(std::get<4>(entries.at(0)));
610614
}
@@ -647,7 +651,9 @@ TEST(Linter, schema_rule_non_string_description_null) {
647651
EXPECT_EQ(std::get<1>(entries.at(0)), "test/null_desc");
648652
EXPECT_EQ(std::get<2>(entries.at(0)), "null");
649653
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
650-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
654+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
655+
"The value was expected to be an object that defines the property "
656+
"\"type\"");
651657
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
652658
EXPECT_FALSE(std::get<4>(entries.at(0)));
653659
}
@@ -696,7 +702,9 @@ TEST(Linter, schema_rule_with_default_dialect_no_schema_keyword) {
696702
EXPECT_EQ(std::get<1>(entries.at(0)), "test/require_type_no_schema");
697703
EXPECT_EQ(std::get<2>(entries.at(0)), "Every subschema must define a type");
698704
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
699-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
705+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
706+
"The value was expected to be an object that defines the property "
707+
"\"type\"");
700708
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
701709
EXPECT_FALSE(std::get<4>(entries.at(0)));
702710
}
@@ -746,7 +754,9 @@ TEST(Linter, schema_rule_with_default_dialect_and_schema_keyword) {
746754
EXPECT_EQ(std::get<1>(entries.at(0)), "test/require_type_with_schema");
747755
EXPECT_EQ(std::get<2>(entries.at(0)), "Every subschema must define a type");
748756
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
749-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
757+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
758+
"The value was expected to be an object that defines the property "
759+
"\"type\"");
750760
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
751761
EXPECT_FALSE(std::get<4>(entries.at(0)));
752762
}
@@ -809,7 +819,9 @@ TEST(Linter, schema_rule_multiple_rules_in_bundle) {
809819
EXPECT_EQ(std::get<1>(entries.at(0)), "test/require_type");
810820
EXPECT_EQ(std::get<2>(entries.at(0)), "Every subschema must define a type");
811821
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
812-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
822+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
823+
"The value was expected to be an object that defines the property "
824+
"\"type\"");
813825
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
814826
EXPECT_FALSE(std::get<4>(entries.at(0)));
815827
}
@@ -855,7 +867,9 @@ TEST(Linter, schema_rule_boolean_true_schema_conforms) {
855867
EXPECT_EQ(std::get<1>(entries.at(0)), "test/must_be_object");
856868
EXPECT_EQ(std::get<2>(entries.at(0)), "Every subschema must be an object");
857869
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
858-
EXPECT_FALSE(std::get<3>(entries.at(0)).description.value().empty());
870+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
871+
"The value was expected to be of type object but it was of type "
872+
"boolean");
859873
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 0);
860874
EXPECT_FALSE(std::get<4>(entries.at(0)));
861875
}
@@ -919,3 +933,58 @@ TEST(Linter, schema_rule_title_array_throws) {
919933
sourcemeta::blaze::default_schema_compiler),
920934
sourcemeta::blaze::LinterInvalidNameError);
921935
}
936+
937+
TEST(Linter, schema_rule_non_empty_instance_location) {
938+
const auto rule_schema{sourcemeta::core::parse_json(R"JSON({
939+
"$schema": "https://json-schema.org/draft/2020-12/schema",
940+
"title": "test/foo_must_be_string",
941+
"description": "The foo property must be a string",
942+
"type": "object",
943+
"properties": {
944+
"foo": { "type": "string" }
945+
}
946+
})JSON")};
947+
948+
sourcemeta::core::SchemaTransformer bundle;
949+
bundle.add<sourcemeta::blaze::SchemaRule>(
950+
rule_schema, sourcemeta::core::schema_walker,
951+
sourcemeta::core::schema_resolver,
952+
sourcemeta::blaze::default_schema_compiler);
953+
954+
const auto schema{sourcemeta::core::parse_json(R"JSON({
955+
"$schema": "https://json-schema.org/draft/2020-12/schema",
956+
"type": "object",
957+
"properties": {
958+
"bar": {
959+
"foo": 42
960+
}
961+
}
962+
})JSON")};
963+
964+
std::vector<std::tuple<sourcemeta::core::Pointer, std::string, std::string,
965+
sourcemeta::core::SchemaTransformRule::Result, bool>>
966+
entries;
967+
const auto result = bundle.check(
968+
schema, sourcemeta::core::schema_walker,
969+
sourcemeta::core::schema_resolver,
970+
[&entries](const auto &pointer, const auto &name, const auto &message,
971+
const auto &outcome, const auto mutable_) {
972+
entries.emplace_back(pointer, name, message, outcome, mutable_);
973+
});
974+
975+
EXPECT_FALSE(result.first);
976+
EXPECT_EQ(entries.size(), 1);
977+
978+
EXPECT_EQ(std::get<0>(entries.at(0)),
979+
sourcemeta::core::Pointer({"properties", "bar"}));
980+
EXPECT_EQ(std::get<1>(entries.at(0)), "test/foo_must_be_string");
981+
EXPECT_EQ(std::get<2>(entries.at(0)), "The foo property must be a string");
982+
EXPECT_TRUE(std::get<3>(entries.at(0)).description.has_value());
983+
EXPECT_EQ(std::get<3>(entries.at(0)).description.value(),
984+
"The value was expected to be of type string but it was of type "
985+
"integer");
986+
EXPECT_EQ(std::get<3>(entries.at(0)).locations.size(), 1);
987+
EXPECT_EQ(std::get<3>(entries.at(0)).locations.at(0),
988+
sourcemeta::core::Pointer({"foo"}));
989+
EXPECT_FALSE(std::get<4>(entries.at(0)));
990+
}

0 commit comments

Comments
 (0)