Skip to content

Commit 94ff49b

Browse files
jviottijviotti
authored
Implement re-referencing for "move" lint rules (#2144)
Signed-off-by: Juan Cruz Viotti <[email protected]>
1 parent 33f8e4a commit 94ff49b

File tree

10 files changed

+390
-30
lines changed

10 files changed

+390
-30
lines changed

‎src/core/json/include/sourcemeta/core/json_hash.h‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
#ifndef SOURCEMETA_CORE_JSON_HASH_H_
22
#define SOURCEMETA_CORE_JSON_HASH_H_
33

4-
#include <cassert> // assert
5-
#include <cstdint> // std::uint64_t
6-
#include <cstring> // std::memcpy
4+
#include <cassert> // assert
5+
#include <cstdint> // std::uint64_t
6+
#include <cstring> // std::memcpy
7+
#include <functional> // std::reference_wrapper
78

89
namespace sourcemeta::core {
910

1011
/// @ingroup json
1112
template <typename T> struct HashJSON {
1213
using hash_type = std::uint64_t;
14+
1315
inline auto operator()(const T &value) const noexcept -> hash_type {
14-
return value.fast_hash();
16+
if constexpr (requires { value.get().fast_hash(); }) {
17+
return value.get().fast_hash();
18+
} else {
19+
return value.fast_hash();
20+
}
1521
}
1622

1723
[[nodiscard]]
@@ -145,6 +151,21 @@ template <typename T> struct PropertyHashJSON {
145151
}
146152
};
147153

154+
/// @ingroup json
155+
/// Until C++26, `std::reference_wrapper` does not overload `operator==`,
156+
/// so we need custom comparisons for use in i.e. `unordered_set`
157+
/// See
158+
/// https://en.cppreference.com/w/cpp/utility/functional/reference_wrapper/operator_cmp.html
159+
template <typename T> struct EqualJSON {
160+
inline auto operator()(const T &left, const T &right) const -> bool {
161+
if constexpr (requires { left.get() == right.get(); }) {
162+
return left.get() == right.get();
163+
} else {
164+
return left == right;
165+
}
166+
}
167+
};
168+
148169
} // namespace sourcemeta::core
149170

150171
#endif

‎src/extension/alterschema/common/duplicate_allof_branches.h‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,44 @@ class DuplicateAllOfBranches final : public SchemaTransformRule {
3131
}
3232

3333
auto transform(JSON &schema, const Result &) const -> void override {
34-
auto collection = schema.at("allOf");
35-
std::sort(collection.as_array().begin(), collection.as_array().end());
36-
auto last =
37-
std::unique(collection.as_array().begin(), collection.as_array().end());
38-
collection.erase(last, collection.as_array().end());
39-
schema.at("allOf").into(std::move(collection));
34+
this->index_mapping_.clear();
35+
const auto &original{schema.at("allOf")};
36+
37+
std::unordered_map<std::reference_wrapper<const JSON>, std::size_t,
38+
HashJSON<std::reference_wrapper<const JSON>>,
39+
EqualJSON<std::reference_wrapper<const JSON>>>
40+
seen;
41+
auto result{JSON::make_array()};
42+
43+
for (std::size_t index = 0; index < original.size(); ++index) {
44+
const auto &value{original.at(index)};
45+
const auto match{seen.find(std::cref(value))};
46+
47+
if (match == seen.end()) {
48+
this->index_mapping_[index] = seen.size();
49+
seen.emplace(std::cref(value), seen.size());
50+
result.push_back(value);
51+
} else {
52+
this->index_mapping_[index] = match->second;
53+
}
54+
}
55+
56+
schema.assign("allOf", std::move(result));
4057
}
58+
59+
[[nodiscard]] auto rereference(const std::string &, const Pointer &,
60+
const Pointer &target,
61+
const Pointer &current) const
62+
-> Pointer override {
63+
const auto allof_prefix{current.concat({"allOf"})};
64+
const auto relative{target.resolve_from(allof_prefix)};
65+
const auto old_index{relative.at(0).to_index()};
66+
const auto new_index{this->index_mapping_.at(old_index)};
67+
const Pointer old_prefix{allof_prefix.concat({old_index})};
68+
const Pointer new_prefix{allof_prefix.concat({new_index})};
69+
return target.rebase(old_prefix, new_prefix);
70+
}
71+
72+
private:
73+
mutable std::unordered_map<std::size_t, std::size_t> index_mapping_;
4174
};

‎src/extension/alterschema/common/duplicate_anyof_branches.h‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,44 @@ class DuplicateAnyOfBranches final : public SchemaTransformRule {
3131
}
3232

3333
auto transform(JSON &schema, const Result &) const -> void override {
34-
auto collection = schema.at("anyOf");
35-
std::sort(collection.as_array().begin(), collection.as_array().end());
36-
auto last =
37-
std::unique(collection.as_array().begin(), collection.as_array().end());
38-
collection.erase(last, collection.as_array().end());
39-
schema.at("anyOf").into(std::move(collection));
34+
this->index_mapping_.clear();
35+
const auto &original{schema.at("anyOf")};
36+
37+
std::unordered_map<std::reference_wrapper<const JSON>, std::size_t,
38+
HashJSON<std::reference_wrapper<const JSON>>,
39+
EqualJSON<std::reference_wrapper<const JSON>>>
40+
seen;
41+
auto result{JSON::make_array()};
42+
43+
for (std::size_t index = 0; index < original.size(); ++index) {
44+
const auto &value{original.at(index)};
45+
const auto match{seen.find(std::cref(value))};
46+
47+
if (match == seen.end()) {
48+
this->index_mapping_[index] = seen.size();
49+
seen.emplace(std::cref(value), seen.size());
50+
result.push_back(value);
51+
} else {
52+
this->index_mapping_[index] = match->second;
53+
}
54+
}
55+
56+
schema.assign("anyOf", std::move(result));
4057
}
58+
59+
[[nodiscard]] auto rereference(const std::string &, const Pointer &,
60+
const Pointer &target,
61+
const Pointer &current) const
62+
-> Pointer override {
63+
const auto anyof_prefix{current.concat({"anyOf"})};
64+
const auto relative{target.resolve_from(anyof_prefix)};
65+
const auto old_index{relative.at(0).to_index()};
66+
const auto new_index{this->index_mapping_.at(old_index)};
67+
const Pointer old_prefix{anyof_prefix.concat({old_index})};
68+
const Pointer new_prefix{anyof_prefix.concat({new_index})};
69+
return target.rebase(old_prefix, new_prefix);
70+
}
71+
72+
private:
73+
mutable std::unordered_map<std::size_t, std::size_t> index_mapping_;
4174
};

‎src/extension/alterschema/common/unnecessary_allof_wrapper.h‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class UnnecessaryAllOfWrapper final : public SchemaTransformRule {
77

88
[[nodiscard]] auto
99
condition(const JSON &schema, const JSON &, const Vocabularies &vocabularies,
10-
const SchemaFrame &, const SchemaFrame::Location &,
10+
const SchemaFrame &frame, const SchemaFrame::Location &location,
1111
const SchemaWalker &walker, const SchemaResolver &) const
1212
-> SchemaTransformRule::Result override {
1313
ONLY_CONTINUE_IF(vocabularies.contains_any(
@@ -51,6 +51,13 @@ class UnnecessaryAllOfWrapper final : public SchemaTransformRule {
5151
continue;
5252
}
5353

54+
// Skip entries that have direct references pointing to them
55+
const auto entry_pointer{
56+
location.relative_pointer.concat({"allOf", index - 1})};
57+
if (!frame.references_to(entry_pointer).empty()) {
58+
continue;
59+
}
60+
5461
// Skip entries that define their own identity, as elevating keywords
5562
// from them could break references that target those anchors
5663
if (!this->is_anonymous(entry, vocabularies)) {
@@ -111,6 +118,19 @@ class UnnecessaryAllOfWrapper final : public SchemaTransformRule {
111118
}
112119
}
113120

121+
[[nodiscard]] auto rereference(const std::string &, const Pointer &,
122+
const Pointer &target,
123+
const Pointer &current) const
124+
-> Pointer override {
125+
// The rule moves keywords from /allOf/<index>/<keyword> to /<keyword>
126+
const auto allof_prefix{current.concat({"allOf"})};
127+
const auto relative{target.resolve_from(allof_prefix)};
128+
const auto &keyword{relative.at(1).to_property()};
129+
const Pointer old_prefix{allof_prefix.concat({relative.at(0), keyword})};
130+
const Pointer new_prefix{current.concat({keyword})};
131+
return target.rebase(old_prefix, new_prefix);
132+
}
133+
114134
private:
115135
// TODO: Ideally we this information from the frame out of the box
116136
[[nodiscard]] auto is_anonymous(const JSON &entry,

‎test/alterschema/alterschema_lint_2019_09_test.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,7 +1358,7 @@ TEST(AlterSchema_lint_2019_09, duplicate_allof_branches_1) {
13581358

13591359
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
13601360
"$schema": "https://json-schema.org/draft/2019-09/schema",
1361-
"type": "string",
1361+
"type": "integer",
13621362
"allOf": [ false ]
13631363
})JSON");
13641364

@@ -1375,7 +1375,7 @@ TEST(AlterSchema_lint_2019_09, duplicate_anyof_branches_1) {
13751375

13761376
const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
13771377
"$schema": "https://json-schema.org/draft/2019-09/schema",
1378-
"anyOf": [ { "type": "integer" }, { "type": "string" } ]
1378+
"anyOf": [ { "type": "string" }, { "type": "integer" } ]
13791379
})JSON");
13801380

13811381
EXPECT_EQ(document, expected);

0 commit comments

Comments
 (0)