Skip to content

Fix SimpleOutput leaking annotations of failed branches #926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jviotti merged 3 commits into from
Jul 16, 2026
Merged

Fix SimpleOutput leaking annotations of failed branches #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a38090
Fix `SimpleOutput` leaking annotations of failed branches
jviotti Jul 15, 2026
5cf3f79
More correctness
jviotti Jul 16, 2026
e5c5c9a
Core
jviotti Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPENDENCIES
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down
31 changes: 21 additions & 10 deletions src/output/include/sourcemeta/blaze/output_simple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@

#include <sourcemeta/blaze/evaluator.h>

#include <cstddef> // std::size_t
#include <cstdint> // std::uint8_t
#include <functional> // std::reference_wrapper
// TODO(C++23): Consider std::flat_map/std::flat_set when available in libc++
#include <map> // std::map
#include <ostream> // std::ostream
#include <string> // std::string
#include <utility> // std::pair
#include <utility> // std::move
#include <vector> // std::vector

namespace sourcemeta::blaze {
Expand Down Expand Up @@ -131,16 +131,27 @@ 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<Entry> buffered_traces;
};

const sourcemeta::core::JSON &instance_;
const sourcemeta::core::WeakPointer base_;
container_type output;
std::vector<
std::pair<sourcemeta::core::WeakPointer, sourcemeta::core::WeakPointer>>
mask;
std::map<
std::pair<sourcemeta::core::WeakPointer, sourcemeta::core::WeakPointer>,
std::vector<Entry>>
masked_traces;
std::vector<MaskEntry> mask;
std::vector<AnnotationEntry> annotations_;
#if defined(_MSC_VER)
#pragma warning(default : 4251)
Expand Down
145 changes: 106 additions & 39 deletions src/output/output_simple.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

#include <sourcemeta/blaze/foundation.h>

#include <algorithm> // std::any_of, std::sort
#include <algorithm> // std::min, std::remove_if
#include <cassert> // assert
#include <iterator> // std::back_inserter, std::make_move_iterator
#include <iterator> // std::make_move_iterator
#include <ranges> // std::views::reverse
#include <utility> // std::move

namespace sourcemeta::blaze {
Expand All @@ -13,6 +14,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();
}
Expand All @@ -37,22 +54,21 @@ 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") {
this->mask.emplace_back(evaluate_path, instance_location);
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 = {}});
}
} 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();
}

Expand All @@ -76,34 +92,34 @@ 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") {
this->mask.emplace_back(evaluate_path, instance_location);
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 = {}});
}
} 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()) {
if (!result && mask_it->kind != MaskKind::Subschema) {
#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);
Expand All @@ -115,19 +131,70 @@ 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 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 (evaluate_path.starts_with(mask_entry.evaluate_path) &&
instance_location.starts_with(mask_entry.instance_location)) {
unit = &mask_entry;
break;
}
}

if (unit) {
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
// not need to be scanned at all
assert(unit->annotations_mark <= this->annotations_.size());
const auto from{this->annotations_.begin() +
static_cast<std::ptrdiff_t>(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) ||
(entry.evaluate_path.shares_prefix(
evaluate_path, evaluate_prefix_size) &&
entry.instance_location.shares_prefix(
instance_location, instance_prefix_size));
}),
this->annotations_.end());
} else {
this->annotations_.clear();
}
}

if (keyword == "if") {
if (step.type == InstructionIndex::LogicalCondition) {
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,
Expand Down
91 changes: 91 additions & 0 deletions test/output/output_jsonld_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading