From 2a38090c95cc6b84f32e1bd6cb39f2727d2b4c1c Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Wed, 15 Jul 2026 19:06:26 -0300 Subject: [PATCH 1/3] Fix `SimpleOutput` leaking annotations of failed branches Signed-off-by: Juan Cruz Viotti --- .../include/sourcemeta/blaze/output_simple.h | 22 +- src/output/output_simple.cc | 126 ++++++-- test/output/output_jsonld_test.cc | 91 ++++++ test/output/output_simple_test.cc | 281 ++++++++++++++++++ 4 files changed, 482 insertions(+), 38 deletions(-) diff --git a/src/output/include/sourcemeta/blaze/output_simple.h b/src/output/include/sourcemeta/blaze/output_simple.h index 071cef4a6..3d1a522a2 100644 --- a/src/output/include/sourcemeta/blaze/output_simple.h +++ b/src/output/include/sourcemeta/blaze/output_simple.h @@ -11,12 +11,11 @@ #include +#include // std::size_t #include // std::reference_wrapper // TODO(C++23): Consider std::flat_map/std::flat_set when available in libc++ -#include // std::map -#include // std::ostream #include // std::string -#include // std::pair +#include // std::move #include // std::vector namespace sourcemeta::blaze { @@ -131,16 +130,19 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput { #if defined(_MSC_VER) #pragma warning(disable : 4251) #endif + /// An in-flight branching keyword, along with the number of annotations + /// collected before it started and the error traces it buffers + struct MaskEntry { + sourcemeta::core::WeakPointer evaluate_path; + sourcemeta::core::WeakPointer instance_location; + std::size_t annotations_mark; + std::vector buffered_traces; + }; + const sourcemeta::core::JSON &instance_; const sourcemeta::core::WeakPointer base_; container_type output; - std::vector< - std::pair> - mask; - std::map< - std::pair, - std::vector> - masked_traces; + std::vector mask; std::vector annotations_; #if defined(_MSC_VER) #pragma warning(default : 4251) diff --git a/src/output/output_simple.cc b/src/output/output_simple.cc index 4bf975fca..2ca2fcf86 100644 --- a/src/output/output_simple.cc +++ b/src/output/output_simple.cc @@ -2,11 +2,32 @@ #include -#include // std::any_of, std::sort +#include // std::any_of, std::equal, std::sort #include // assert -#include // std::back_inserter, std::make_move_iterator +#include // std::back_inserter, std::make_move_iterator, std::next +#include // std::views::reverse #include // std::move +namespace { + +auto shares_prefix(const sourcemeta::core::WeakPointer &pointer, + const sourcemeta::core::WeakPointer &other, + const std::size_t prefix_size) -> bool { + return pointer.size() >= prefix_size && other.size() >= prefix_size && + std::equal(pointer.cbegin(), + std::next(pointer.cbegin(), + static_cast(prefix_size)), + other.cbegin()); +} + +auto starts_with_strict(const sourcemeta::core::WeakPointer &pointer, + const sourcemeta::core::WeakPointer &other) -> bool { + return pointer.size() > other.size() && + shares_prefix(pointer, other, other.size()); +} + +} // namespace + namespace sourcemeta::blaze { SimpleOutput::SimpleOutput(const sourcemeta::core::JSON &instance, @@ -46,13 +67,14 @@ auto SimpleOutput::operator()( const auto &keyword{evaluate_path.back().to_property()}; if (keyword == "anyOf" || keyword == "oneOf" || keyword == "not" || keyword == "if" || keyword == "contains") { - this->mask.emplace_back(evaluate_path, instance_location); + this->mask.push_back({.evaluate_path = evaluate_path, + .instance_location = instance_location, + .annotations_mark = this->annotations_.size(), + .buffered_traces = {}}); } } else if (type == EvaluationType::Post && !this->mask.empty() && - this->mask.back().first == evaluate_path && - this->mask.back().second == instance_location) { - const auto mask_key{std::make_pair(evaluate_path, instance_location)}; - this->masked_traces.erase(mask_key); + this->mask.back().evaluate_path == evaluate_path && + this->mask.back().instance_location == instance_location) { this->mask.pop_back(); } @@ -83,27 +105,28 @@ auto SimpleOutput::operator()( // To ease the output if (keyword == "anyOf" || keyword == "oneOf" || keyword == "not" || keyword == "if" || keyword == "contains") { - this->mask.emplace_back(evaluate_path, instance_location); + this->mask.push_back({.evaluate_path = evaluate_path, + .instance_location = instance_location, + .annotations_mark = this->annotations_.size(), + .buffered_traces = {}}); } } else if (type == EvaluationType::Post) { - const auto mask_key{std::make_pair(evaluate_path, instance_location)}; - const auto mask_it{std::ranges::find(this->mask, mask_key)}; + const auto mask_it{std::ranges::find_if( + this->mask, [&](const MaskEntry &mask_entry) -> bool { + return mask_entry.evaluate_path == evaluate_path && + mask_entry.instance_location == instance_location; + })}; if (mask_it != this->mask.end()) { // Present unexpected traces only when needed if (!result && keyword != "not" && keyword != "if") { - auto buffered{this->masked_traces.find(mask_key)}; - if (buffered != this->masked_traces.end()) { #ifdef __cpp_lib_containers_ranges - this->output.append_range(std::move(buffered->second)); + this->output.append_range(std::move(mask_it->buffered_traces)); #else - this->output.insert(this->output.end(), - std::make_move_iterator(buffered->second.begin()), - std::make_move_iterator(buffered->second.end())); + this->output.insert( + this->output.end(), + std::make_move_iterator(mask_it->buffered_traces.begin()), + std::make_move_iterator(mask_it->buffered_traces.end())); #endif - this->masked_traces.erase(buffered); - } - } else { - this->masked_traces.erase(mask_key); } this->mask.erase(mask_it); @@ -115,19 +138,66 @@ auto SimpleOutput::operator()( } if (type == EvaluationType::Post && !this->annotations_.empty()) { - std::erase_if( - this->annotations_, [&](const AnnotationEntry &entry) -> bool { - return entry.evaluate_path.starts_with_initial(evaluate_path) && - entry.instance_location == instance_location; - }); + // A failure inside a branching keyword discards every annotation that + // the failing unit collected, including the ones at instance locations + // below the unit's own instance location. The unit is the disjunction + // branch for `anyOf` and `oneOf`, the application of the subschema to + // the array element for `contains`, and the entire subschema for `if` + // and `not`. Outside of branching keywords, annotations from successful + // sibling subschemas are reported on a best-effort basis, so only + // annotations at the exact instance location of the failure are + // discarded + const MaskEntry *unit{nullptr}; + for (const auto &mask_entry : std::views::reverse(this->mask)) { + if (starts_with_strict(evaluate_path, mask_entry.evaluate_path) && + instance_location.starts_with(mask_entry.instance_location)) { + unit = &mask_entry; + break; + } + } + + if (unit) { + const auto &mask_keyword{unit->evaluate_path.back().to_property()}; + const auto evaluate_prefix_size{ + unit->evaluate_path.size() + + ((mask_keyword == "anyOf" || mask_keyword == "oneOf") ? 1 : 0)}; + const auto instance_prefix_size{unit->instance_location.size() + + (mask_keyword == "contains" ? 1 : 0)}; + // Every annotation the failing unit collected, including the ones + // that the equality check below would match, sits at or beyond the + // mark the unit's branching keyword recorded, so older entries do + // not need to be scanned at all + assert(unit->annotations_mark <= this->annotations_.size()); + const auto from{this->annotations_.begin() + + static_cast(unit->annotations_mark)}; + this->annotations_.erase( + std::remove_if( + from, this->annotations_.end(), + [&](const AnnotationEntry &entry) -> bool { + return (entry.evaluate_path.starts_with_initial( + evaluate_path) && + entry.instance_location == instance_location) || + (shares_prefix(entry.evaluate_path, evaluate_path, + evaluate_prefix_size) && + shares_prefix(entry.instance_location, + instance_location, instance_prefix_size)); + }), + this->annotations_.end()); + } else { + std::erase_if( + this->annotations_, [&](const AnnotationEntry &entry) -> bool { + return entry.evaluate_path.starts_with_initial(evaluate_path) && + entry.instance_location == instance_location; + }); + } } if (keyword == "if") { return; } else { - for (const auto &mask_entry : this->mask) { - if (evaluate_path.starts_with(mask_entry.first)) { - this->masked_traces[mask_entry].push_back( + for (auto &mask_entry : this->mask) { + if (evaluate_path.starts_with(mask_entry.evaluate_path)) { + mask_entry.buffered_traces.push_back( {.message = describe(result, step, evaluate_path, instance_location, this->instance_, annotation), .instance_location = instance_location, diff --git a/test/output/output_jsonld_test.cc b/test/output/output_jsonld_test.cc index 6ef0db15d..3e1fa6b5b 100644 --- a/test/output/output_jsonld_test.cc +++ b/test/output/output_jsonld_test.cc @@ -391,6 +391,97 @@ TEST(JSONLD_oneof_only_matching_branch_survives) { EXPECT_JSON_LD_VALUE(schema, instance, expected); } +TEST(JSONLD_anyof_failed_branch_nested_annotations_dropped) { + const auto schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "payment": { + "x-jsonld-id": "https://schema.org/payment", + "anyOf": [ + { + "properties": { + "amount": { + "x-jsonld-id": "https://schema.org/wrongPredicate", + "x-jsonld-datatype": "http://www.w3.org/2001/XMLSchema#date" + }, + "kind": { + "type": "number", + "minimum": 5, + "maximum": 10, + "multipleOf": 2 + } + } + }, + { + "properties": { + "amount": { "x-jsonld-id": "https://schema.org/rightPredicate" } + } + } + ] + } + } + })JSON")}; + + const auto instance{sourcemeta::core::parse_json( + R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")}; + + const auto expected{sourcemeta::core::parse_json(R"JSON([ + { + "https://schema.org/payment": [ + { + "https://schema.org/rightPredicate": [ { "@value": "hi" } ] + } + ] + } + ])JSON")}; + + EXPECT_JSON_LD_VALUE(schema, instance, expected); +} + +TEST(JSONLD_anyof_failed_branch_nested_annotations_dropped_cheap_sibling) { + const auto schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "payment": { + "x-jsonld-id": "https://schema.org/payment", + "anyOf": [ + { + "properties": { + "amount": { + "x-jsonld-id": "https://schema.org/wrongPredicate", + "x-jsonld-datatype": "http://www.w3.org/2001/XMLSchema#date" + }, + "kind": { "const": 4 } + } + }, + { + "properties": { + "amount": { "x-jsonld-id": "https://schema.org/rightPredicate" } + } + } + ] + } + } + })JSON")}; + + const auto instance{sourcemeta::core::parse_json( + R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")}; + + const auto expected{sourcemeta::core::parse_json(R"JSON([ + { + "https://schema.org/payment": [ + { + "https://schema.org/rightPredicate": [ { "@value": "hi" } ] + } + ] + } + ])JSON")}; + + EXPECT_JSON_LD_VALUE(schema, instance, expected); +} + TEST(JSONLD_standalone_node_without_edge) { const auto schema{sourcemeta::core::parse_json(R"JSON({ "$schema": "https://json-schema.org/draft/2020-12/schema", diff --git a/test/output/output_simple_test.cc b/test/output/output_simple_test.cc index 4f77924a0..cdb0cad8c 100644 --- a/test/output/output_simple_test.cc +++ b/test/output/output_simple_test.cc @@ -1651,3 +1651,284 @@ TEST(annotations_failure_1) { "The value was expected to be of type string but it was of type integer"); EXPECT_ANNOTATION_COUNT(output, 0); } + +TEST(annotations_success_anyof_failed_branch_nested_property_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "payment": { + "anyOf": [ + { + "properties": { + "amount": { "title": "Wrong" }, + "kind": { + "type": "number", + "minimum": 5, + "maximum": 10, + "multipleOf": 2 + } + } + }, + { + "properties": { + "amount": { "title": "Right" } + } + } + ] + } + } + })JSON")}; + + const auto schema_template{ + sourcemeta::blaze::compile(schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::Exhaustive)}; + + const sourcemeta::core::JSON instance{sourcemeta::core::parse_json( + R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + std::vector traces{output.cbegin(), + output.cend()}; + EXPECT_TRUE(traces.empty()); + + EXPECT_ANNOTATION_COUNT(output, 3); + + EXPECT_ANNOTATION(output, 0, "/payment/amount", + "/properties/payment/anyOf/1/properties/amount/title", + "#/properties/payment/anyOf/1/properties/amount/title", + sourcemeta::core::JSON{"Right"}); + EXPECT_ANNOTATION(output, 1, "/payment", + "/properties/payment/anyOf/1/properties", + "#/properties/payment/anyOf/1/properties", + sourcemeta::core::JSON{"amount"}); + EXPECT_ANNOTATION(output, 2, "", "/properties", "#/properties", + sourcemeta::core::JSON{"payment"}); +} + +TEST(annotations_success_anyof_failed_branch_nested_property_2) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "payment": { + "anyOf": [ + { + "properties": { + "amount": { "title": "Wrong" }, + "kind": { "const": 4 } + } + }, + { + "properties": { + "amount": { "title": "Right" } + } + } + ] + } + } + })JSON")}; + + const auto schema_template{ + sourcemeta::blaze::compile(schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::Exhaustive)}; + + const sourcemeta::core::JSON instance{sourcemeta::core::parse_json( + R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + std::vector traces{output.cbegin(), + output.cend()}; + EXPECT_TRUE(traces.empty()); + + EXPECT_ANNOTATION_COUNT(output, 3); + + EXPECT_ANNOTATION(output, 0, "/payment/amount", + "/properties/payment/anyOf/1/properties/amount/title", + "#/properties/payment/anyOf/1/properties/amount/title", + sourcemeta::core::JSON{"Right"}); + EXPECT_ANNOTATION(output, 1, "/payment", + "/properties/payment/anyOf/1/properties", + "#/properties/payment/anyOf/1/properties", + sourcemeta::core::JSON{"amount"}); + EXPECT_ANNOTATION(output, 2, "", "/properties", "#/properties", + sourcemeta::core::JSON{"payment"}); +} + +TEST(annotations_success_whitelist_anyof_failed_branch_nested_property) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "payment": { + "anyOf": [ + { + "properties": { + "amount": { "x-custom": "Wrong" }, + "kind": { + "type": "number", + "minimum": 5, + "maximum": 10, + "multipleOf": 2 + } + } + }, + { + "properties": { + "amount": { "x-custom": "Right" } + } + } + ] + } + } + })JSON")}; + + sourcemeta::blaze::Tweaks tweaks; + tweaks.annotations = + std::unordered_set{"x-custom"}; + const auto schema_template{sourcemeta::blaze::compile( + schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::FastValidation, "", "", "", tweaks)}; + + const sourcemeta::core::JSON instance{sourcemeta::core::parse_json( + R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + std::vector traces{output.cbegin(), + output.cend()}; + EXPECT_TRUE(traces.empty()); + + EXPECT_ANNOTATION_COUNT(output, 1); + + EXPECT_ANNOTATION(output, 0, "/payment/amount", + "/properties/payment/anyOf/1/properties/amount/x-custom", + "#/properties/payment/anyOf/1/properties/amount/x-custom", + sourcemeta::core::JSON{"Right"}); +} + +TEST(annotations_success_oneof_failed_branch_nested_property_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "payment": { + "oneOf": [ + { + "properties": { + "amount": { "title": "Wrong" }, + "kind": { + "type": "number", + "minimum": 5, + "maximum": 10, + "multipleOf": 2 + } + } + }, + { + "properties": { + "amount": { "title": "Right" }, + "kind": { "const": 999 } + } + } + ] + } + } + })JSON")}; + + const auto schema_template{ + sourcemeta::blaze::compile(schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::Exhaustive)}; + + const sourcemeta::core::JSON instance{sourcemeta::core::parse_json( + R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + std::vector traces{output.cbegin(), + output.cend()}; + EXPECT_TRUE(traces.empty()); + + EXPECT_ANNOTATION_COUNT(output, 4); + + EXPECT_ANNOTATION(output, 0, "/payment", + "/properties/payment/oneOf/1/properties", + "#/properties/payment/oneOf/1/properties", + sourcemeta::core::JSON{"kind"}); + EXPECT_ANNOTATION(output, 1, "/payment/amount", + "/properties/payment/oneOf/1/properties/amount/title", + "#/properties/payment/oneOf/1/properties/amount/title", + sourcemeta::core::JSON{"Right"}); + EXPECT_ANNOTATION(output, 2, "/payment", + "/properties/payment/oneOf/1/properties", + "#/properties/payment/oneOf/1/properties", + sourcemeta::core::JSON{"amount"}); + EXPECT_ANNOTATION(output, 3, "", "/properties", "#/properties", + sourcemeta::core::JSON{"payment"}); +} + +TEST(annotations_success_if_else_failed_branch_nested_property_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "if": { + "properties": { + "amount": { "title": "Wrong" }, + "kind": { + "type": "number", + "minimum": 5, + "maximum": 10, + "multipleOf": 2 + } + } + }, + "else": { + "properties": { + "amount": { "title": "Right" } + } + } + })JSON")}; + + const auto schema_template{ + sourcemeta::blaze::compile(schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::Exhaustive)}; + + const sourcemeta::core::JSON instance{sourcemeta::core::parse_json( + R"JSON({ "amount": "hi", "kind": 999 })JSON")}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + std::vector traces{output.cbegin(), + output.cend()}; + EXPECT_TRUE(traces.empty()); + + EXPECT_ANNOTATION_COUNT(output, 2); + + EXPECT_ANNOTATION(output, 0, "/amount", "/else/properties/amount/title", + "#/else/properties/amount/title", + sourcemeta::core::JSON{"Right"}); + EXPECT_ANNOTATION(output, 1, "", "/else/properties", "#/else/properties", + sourcemeta::core::JSON{"amount"}); +} From 5cf3f79278ed70f1e70343bb693e57f49c6ec98b Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 16 Jul 2026 10:00:48 -0300 Subject: [PATCH 2/3] More correctness Signed-off-by: Juan Cruz Viotti --- .../include/sourcemeta/blaze/output_simple.h | 9 ++ src/output/output_simple.cc | 79 +++++++----- test/output/output_simple_test.cc | 122 +++++++++++++++++- 3 files changed, 172 insertions(+), 38 deletions(-) diff --git a/src/output/include/sourcemeta/blaze/output_simple.h b/src/output/include/sourcemeta/blaze/output_simple.h index 3d1a522a2..d441aa7ec 100644 --- a/src/output/include/sourcemeta/blaze/output_simple.h +++ b/src/output/include/sourcemeta/blaze/output_simple.h @@ -12,6 +12,7 @@ #include #include // std::size_t +#include // std::uint8_t #include // std::reference_wrapper // TODO(C++23): Consider std::flat_map/std::flat_set when available in libc++ #include // std::string @@ -130,11 +131,19 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput { #if defined(_MSC_VER) #pragma warning(disable : 4251) #endif + /// How much of a branching instruction fails as a unit when one of its + /// subinstructions fails + enum class MaskKind : std::uint8_t { None, Disjunction, Element, Subschema }; + + /// Classify an instruction according to how it absorbs failures + static auto mask_kind(const Instruction &step) noexcept -> MaskKind; + /// An in-flight branching keyword, along with the number of annotations /// collected before it started and the error traces it buffers struct MaskEntry { sourcemeta::core::WeakPointer evaluate_path; sourcemeta::core::WeakPointer instance_location; + MaskKind kind; std::size_t annotations_mark; std::vector buffered_traces; }; diff --git a/src/output/output_simple.cc b/src/output/output_simple.cc index 2ca2fcf86..34056bf94 100644 --- a/src/output/output_simple.cc +++ b/src/output/output_simple.cc @@ -2,7 +2,7 @@ #include -#include // std::any_of, std::equal, std::sort +#include // std::equal, std::min, std::remove_if #include // assert #include // std::back_inserter, std::make_move_iterator, std::next #include // std::views::reverse @@ -20,12 +20,6 @@ auto shares_prefix(const sourcemeta::core::WeakPointer &pointer, other.cbegin()); } -auto starts_with_strict(const sourcemeta::core::WeakPointer &pointer, - const sourcemeta::core::WeakPointer &other) -> bool { - return pointer.size() > other.size() && - shares_prefix(pointer, other, other.size()); -} - } // namespace namespace sourcemeta::blaze { @@ -34,6 +28,22 @@ SimpleOutput::SimpleOutput(const sourcemeta::core::JSON &instance, sourcemeta::core::WeakPointer base) : instance_{instance}, base_{std::move(base)} {} +auto SimpleOutput::mask_kind(const Instruction &step) noexcept -> MaskKind { + switch (step.type) { + case InstructionIndex::LogicalOr: + case InstructionIndex::LogicalXor: + return MaskKind::Disjunction; + case InstructionIndex::LoopContains: + return MaskKind::Element; + case InstructionIndex::LogicalCondition: + case InstructionIndex::LogicalNot: + case InstructionIndex::LogicalNotEvaluate: + return MaskKind::Subschema; + default: + return MaskKind::None; + } +} + auto SimpleOutput::begin() const -> const_iterator { return this->output.begin(); } @@ -58,17 +68,15 @@ auto SimpleOutput::operator()( return; } - assert(evaluate_path.back().is_property()); - // Fast path: passing non-annotation instructions that are not // closing a mask entry can be skipped entirely if (result && !is_annotation(step.type)) { if (type == EvaluationType::Pre) { - const auto &keyword{evaluate_path.back().to_property()}; - if (keyword == "anyOf" || keyword == "oneOf" || keyword == "not" || - keyword == "if" || keyword == "contains") { + const auto kind{mask_kind(step)}; + if (kind != MaskKind::None) { this->mask.push_back({.evaluate_path = evaluate_path, .instance_location = instance_location, + .kind = kind, .annotations_mark = this->annotations_.size(), .buffered_traces = {}}); } @@ -98,15 +106,14 @@ auto SimpleOutput::operator()( return; } - const auto &keyword{evaluate_path.back().to_property()}; - if (type == EvaluationType::Pre) { assert(result); // To ease the output - if (keyword == "anyOf" || keyword == "oneOf" || keyword == "not" || - keyword == "if" || keyword == "contains") { + const auto kind{mask_kind(step)}; + if (kind != MaskKind::None) { this->mask.push_back({.evaluate_path = evaluate_path, .instance_location = instance_location, + .kind = kind, .annotations_mark = this->annotations_.size(), .buffered_traces = {}}); } @@ -118,7 +125,7 @@ auto SimpleOutput::operator()( })}; if (mask_it != this->mask.end()) { // Present unexpected traces only when needed - if (!result && keyword != "not" && keyword != "if") { + if (!result && mask_it->kind != MaskKind::Subschema) { #ifdef __cpp_lib_containers_ranges this->output.append_range(std::move(mask_it->buffered_traces)); #else @@ -143,13 +150,12 @@ auto SimpleOutput::operator()( // below the unit's own instance location. The unit is the disjunction // branch for `anyOf` and `oneOf`, the application of the subschema to // the array element for `contains`, and the entire subschema for `if` - // and `not`. Outside of branching keywords, annotations from successful - // sibling subschemas are reported on a best-effort basis, so only - // annotations at the exact instance location of the failure are - // discarded + // and negations. A failure outside of every branching keyword cannot + // be absorbed, so the overall result is bound to be false, in which + // case no annotations may be reported at all const MaskEntry *unit{nullptr}; for (const auto &mask_entry : std::views::reverse(this->mask)) { - if (starts_with_strict(evaluate_path, mask_entry.evaluate_path) && + if (evaluate_path.starts_with(mask_entry.evaluate_path) && instance_location.starts_with(mask_entry.instance_location)) { unit = &mask_entry; break; @@ -157,12 +163,21 @@ auto SimpleOutput::operator()( } if (unit) { - const auto &mask_keyword{unit->evaluate_path.back().to_property()}; - const auto evaluate_prefix_size{ - unit->evaluate_path.size() + - ((mask_keyword == "anyOf" || mask_keyword == "oneOf") ? 1 : 0)}; - const auto instance_prefix_size{unit->instance_location.size() + - (mask_keyword == "contains" ? 1 : 0)}; + auto evaluate_prefix_size{unit->evaluate_path.size()}; + auto instance_prefix_size{unit->instance_location.size()}; + switch (unit->kind) { + case MaskKind::Disjunction: + evaluate_prefix_size = + std::min(evaluate_prefix_size + 1, evaluate_path.size()); + break; + case MaskKind::Element: + instance_prefix_size = + std::min(instance_prefix_size + 1, instance_location.size()); + break; + default: + break; + } + // Every annotation the failing unit collected, including the ones // that the equality check below would match, sits at or beyond the // mark the unit's branching keyword recorded, so older entries do @@ -184,15 +199,11 @@ auto SimpleOutput::operator()( }), this->annotations_.end()); } else { - std::erase_if( - this->annotations_, [&](const AnnotationEntry &entry) -> bool { - return entry.evaluate_path.starts_with_initial(evaluate_path) && - entry.instance_location == instance_location; - }); + this->annotations_.clear(); } } - if (keyword == "if") { + if (step.type == InstructionIndex::LogicalCondition) { return; } else { for (auto &mask_entry : this->mask) { diff --git a/test/output/output_simple_test.cc b/test/output/output_simple_test.cc index cdb0cad8c..e1b6bd424 100644 --- a/test/output/output_simple_test.cc +++ b/test/output/output_simple_test.cc @@ -1025,10 +1025,7 @@ TEST(annotations_failure_2) { "properties " "subschemas"); - EXPECT_ANNOTATION_COUNT(output, 1); - - EXPECT_ANNOTATION(output, 0, "/bar", "/properties/bar/title", - "#/properties/bar/title", sourcemeta::core::JSON{"Bar"}); + EXPECT_ANNOTATION_COUNT(output, 0); } TEST(annotations_success_oneof_1) { @@ -1932,3 +1929,120 @@ TEST(annotations_success_if_else_failed_branch_nested_property_1) { EXPECT_ANNOTATION(output, 1, "", "/else/properties", "#/else/properties", sourcemeta::core::JSON{"amount"}); } + +TEST(annotations_failure_multiple_locations_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "foo": { "title": "Foo" }, + "zzz": { "type": "string" } + } + })JSON")}; + + const auto schema_template{ + sourcemeta::blaze::compile(schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::Exhaustive)}; + + const sourcemeta::core::JSON instance{ + sourcemeta::core::parse_json(R"JSON({ "foo": 1, "zzz": 2 })JSON")}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_FALSE(result); + + EXPECT_ANNOTATION_COUNT(output, 0); +} + +TEST(annotations_success_whitelist_disallow_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-03/schema#", + "x-custom": "Kept", + "disallow": [ { "type": "integer" } ] + })JSON")}; + + sourcemeta::blaze::Tweaks tweaks; + tweaks.annotations = + std::unordered_set{"x-custom"}; + const auto schema_template{sourcemeta::blaze::compile( + schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::FastValidation, "", "", "", tweaks)}; + + const sourcemeta::core::JSON instance{"hello"}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + + EXPECT_ANNOTATION_COUNT(output, 0); +} + +TEST(annotations_success_whitelist_disallow_cross_dialect_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "x-custom": "Kept", + "$ref": "#/$defs/legacy", + "$defs": { + "legacy": { + "$schema": "http://json-schema.org/draft-03/schema#", + "disallow": [ { "type": "integer" } ] + } + } + })JSON")}; + + sourcemeta::blaze::Tweaks tweaks; + tweaks.annotations = + std::unordered_set{"x-custom"}; + const auto schema_template{sourcemeta::blaze::compile( + schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::FastValidation, "", "", "", tweaks)}; + + const sourcemeta::core::JSON instance{"hello"}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_TRUE(result); + + EXPECT_ANNOTATION_COUNT(output, 1); + + EXPECT_ANNOTATION(output, 0, "", "/x-custom", "#/x-custom", + sourcemeta::core::JSON{"Kept"}); +} + +TEST(annotations_failure_whitelist_disallow_1) { + const sourcemeta::core::JSON schema{sourcemeta::core::parse_json(R"JSON({ + "$schema": "http://json-schema.org/draft-03/schema#", + "x-custom": "Gone", + "disallow": [ { "type": "integer" } ] + })JSON")}; + + sourcemeta::blaze::Tweaks tweaks; + tweaks.annotations = + std::unordered_set{"x-custom"}; + const auto schema_template{sourcemeta::blaze::compile( + schema, sourcemeta::blaze::schema_walker, + sourcemeta::blaze::schema_resolver, + sourcemeta::blaze::default_schema_compiler, + sourcemeta::blaze::Mode::FastValidation, "", "", "", tweaks)}; + + const sourcemeta::core::JSON instance{5}; + + sourcemeta::blaze::SimpleOutput output{instance}; + sourcemeta::blaze::Evaluator evaluator; + const auto result{ + evaluator.validate(schema_template, instance, std::ref(output))}; + EXPECT_FALSE(result); + + EXPECT_ANNOTATION_COUNT(output, 0); +} From e5c5c9a6ad5f25142ec6b7b839b1c1c4d2f22375 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 16 Jul 2026 10:18:24 -0300 Subject: [PATCH 3/3] Core Signed-off-by: Juan Cruz Viotti --- DEPENDENCIES | 2 +- src/output/output_simple.cc | 26 +++-------- .../sourcemeta/core/jsonpointer_pointer.h | 43 ++++++++++++++++++- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/DEPENDENCIES b/DEPENDENCIES index 4f98c23bb..0b53f83ad 100644 --- a/DEPENDENCIES +++ b/DEPENDENCIES @@ -1,5 +1,5 @@ vendorpull https://github.com/sourcemeta/vendorpull 1dcbac42809cf87cb5b045106b863e17ad84ba02 -core https://github.com/sourcemeta/core 2060a4f64a51bd277c4da802321f20e06089c145 +core https://github.com/sourcemeta/core 966869b3659c20fc8a1089999a02d80a61b2b6d1 jsonschema-test-suite https://github.com/json-schema-org/JSON-Schema-Test-Suite 1acd90e53554fa24d2529b49fd7d50bab18f8b7e jsonschema-2020-12 https://github.com/json-schema-org/json-schema-spec 769daad75a9553562333a8937a187741cb708c72 jsonschema-2019-09 https://github.com/json-schema-org/json-schema-spec 41014ea723120ce70b314d72f863c6929d9f3cfd diff --git a/src/output/output_simple.cc b/src/output/output_simple.cc index 34056bf94..30e51c2ec 100644 --- a/src/output/output_simple.cc +++ b/src/output/output_simple.cc @@ -2,26 +2,12 @@ #include -#include // std::equal, std::min, std::remove_if +#include // std::min, std::remove_if #include // assert -#include // std::back_inserter, std::make_move_iterator, std::next +#include // std::make_move_iterator #include // std::views::reverse #include // std::move -namespace { - -auto shares_prefix(const sourcemeta::core::WeakPointer &pointer, - const sourcemeta::core::WeakPointer &other, - const std::size_t prefix_size) -> bool { - return pointer.size() >= prefix_size && other.size() >= prefix_size && - std::equal(pointer.cbegin(), - std::next(pointer.cbegin(), - static_cast(prefix_size)), - other.cbegin()); -} - -} // namespace - namespace sourcemeta::blaze { SimpleOutput::SimpleOutput(const sourcemeta::core::JSON &instance, @@ -192,10 +178,10 @@ auto SimpleOutput::operator()( return (entry.evaluate_path.starts_with_initial( evaluate_path) && entry.instance_location == instance_location) || - (shares_prefix(entry.evaluate_path, evaluate_path, - evaluate_prefix_size) && - shares_prefix(entry.instance_location, - instance_location, instance_prefix_size)); + (entry.evaluate_path.shares_prefix( + evaluate_path, evaluate_prefix_size) && + entry.instance_location.shares_prefix( + instance_location, instance_prefix_size)); }), this->annotations_.end()); } else { diff --git a/vendor/core/src/core/jsonpointer/include/sourcemeta/core/jsonpointer_pointer.h b/vendor/core/src/core/jsonpointer/include/sourcemeta/core/jsonpointer_pointer.h index 2a77f7192..c616e2e64 100644 --- a/vendor/core/src/core/jsonpointer/include/sourcemeta/core/jsonpointer_pointer.h +++ b/vendor/core/src/core/jsonpointer/include/sourcemeta/core/jsonpointer_pointer.h @@ -8,7 +8,7 @@ #include // std::size_t #include // std::reference_wrapper #include // std::initializer_list -#include // std::advance, std::back_inserter +#include // std::advance, std::back_inserter, std::next #include // std::optional #include // std::ranges::subrange #include // std::is_same_v, std::decay_t @@ -663,6 +663,47 @@ template class GenericPointer { this->data[prefix_size + 1].to_property() == tail_right; } + /// Check whether two JSON Pointers are equal up to a given number of + /// leading tokens. For example: + /// + /// ```cpp + /// #include + /// #include + /// + /// const sourcemeta::core::Pointer pointer{"foo", "bar", "baz"}; + /// const sourcemeta::core::Pointer other{"foo", "bar", "qux"}; + /// assert(pointer.shares_prefix(other, 2)); + /// assert(!pointer.shares_prefix(other, 3)); + /// ``` + [[nodiscard]] auto shares_prefix(const GenericPointer &other, + const size_type prefix_size) const -> bool { + return this->data.size() >= prefix_size && + other.data.size() >= prefix_size && + std::equal(this->data.cbegin(), + std::next(this->data.cbegin(), + static_cast(prefix_size)), + other.data.cbegin()); + } + + /// Check whether a JSON Pointer starts with another JSON Pointer without + /// the two being equal. For example: + /// + /// ```cpp + /// #include + /// #include + /// + /// const sourcemeta::core::Pointer pointer{"foo", "bar", "baz"}; + /// const sourcemeta::core::Pointer prefix{"foo", "bar"}; + /// assert(pointer.starts_with_strict(prefix)); + /// assert(!pointer.starts_with_strict(pointer)); + /// ``` + [[nodiscard]] auto + starts_with_strict(const GenericPointer &other) const + -> bool { + return this->data.size() > other.data.size() && + this->shares_prefix(other, other.data.size()); + } + /// Check whether a JSON Pointer starts with the initial part of another JSON /// Pointer. For example: ///