Skip to content

Commit

Permalink
[yaml] Minor tweaks to internal::Node API (RobotLocomotion#15934)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jwnimmer-tri
jwnimmer-tri authored Oct 18, 2021
1 parent 93782ea commit 4c61b32
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 56 deletions.
73 changes: 50 additions & 23 deletions common/yaml/test/yaml_node_test.cc
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "drake/common/yaml/yaml_node.h"

#include <fmt/format.h>
#include <fmt/ostream.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>

#include "drake/common/test_utilities/expect_throws_message.h"
Expand All @@ -18,17 +20,22 @@ static void PrintTo(const NodeType& node_type, std::ostream* os) {

namespace {

// Boilerplate for std::visit.
template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; };
template<class... Ts> overloaded(Ts...) -> overloaded<Ts...>;
// Check that the tag constants exist.
GTEST_TEST(YamlNodeTest, TagConstants) {
EXPECT_GT(Node::kTagFloat.size(), 0);
EXPECT_GT(Node::kTagInt.size(), 0);
EXPECT_GT(Node::kTagNull.size(), 0);
EXPECT_GT(Node::kTagStr.size(), 0);
}

// Check the default constructor.
GTEST_TEST(YamlNodeTest, DefaultConstructor) {
Node dut;
// Check the null-returning factory function.
GTEST_TEST(YamlNodeTest, MakeNull) {
Node dut = Node::MakeNull();
EXPECT_EQ(dut.GetType(), NodeType::kScalar);
EXPECT_TRUE(dut.IsScalar());
EXPECT_TRUE(dut.IsEmptyScalar());
EXPECT_EQ(dut.GetScalar(), "");
EXPECT_EQ(dut.GetScalar(), "null");
EXPECT_EQ(dut.GetTag(), Node::kTagNull);

EXPECT_TRUE(Node::MakeNull() == Node::MakeNull());
}

// Parameterize the remainder of the tests across the three possible types.
Expand Down Expand Up @@ -90,7 +97,6 @@ TEST_P(YamlNodeParamaterizedTest, GetType) {
EXPECT_EQ(dut.GetType(), GetExpectedType());
EXPECT_EQ(dut.GetTypeString(), GetExpectedTypeString());
EXPECT_EQ(dut.IsScalar(), GetExpectedType() == NodeType::kScalar);
EXPECT_EQ(dut.IsEmptyScalar(), GetExpectedType() == NodeType::kScalar);
EXPECT_EQ(dut.IsSequence(), GetExpectedType() == NodeType::kSequence);
EXPECT_EQ(dut.IsMapping(), GetExpectedType() == NodeType::kMapping);
}
Expand Down Expand Up @@ -127,7 +133,7 @@ TEST_P(YamlNodeParamaterizedTest, EfficientMoveConstructor) {
// Ditto per the prior test case.
TEST_P(YamlNodeParamaterizedTest, EfficientMoveAssignment) {
Node dut = MakeNonEmptyDut();
Node foo;
Node foo = Node::MakeNull();

auto guard = std::make_unique<test::LimitMalloc>();
foo = std::move(dut);
Expand Down Expand Up @@ -164,7 +170,6 @@ TEST_P(YamlNodeParamaterizedTest, ScalarOps) {
Node dut2 = MakeNonEmptyDut();
EXPECT_FALSE(dut == dut2);
EXPECT_EQ(dut2.GetScalar(), "foo");
EXPECT_FALSE(dut2.IsEmptyScalar());
} else {
DRAKE_EXPECT_THROWS_MESSAGE(
dut.GetScalar(), GetExpectedCannot("GetScalar"));
Expand All @@ -184,7 +189,7 @@ TEST_P(YamlNodeParamaterizedTest, SequenceOps) {
DRAKE_EXPECT_THROWS_MESSAGE(
dut.GetSequence(), GetExpectedCannot("GetSequence"));
DRAKE_EXPECT_THROWS_MESSAGE(
dut.Add(Node{}), GetExpectedCannot("Add"));
dut.Add(Node::MakeNull()), GetExpectedCannot("Add"));
}
}

Expand All @@ -210,7 +215,7 @@ TEST_P(YamlNodeParamaterizedTest, MappingOps) {
DRAKE_EXPECT_THROWS_MESSAGE(
dut.GetMapping(), GetExpectedCannot("GetMapping"));
DRAKE_EXPECT_THROWS_MESSAGE(
dut.Add("key", Node{}), GetExpectedCannot("Add"));
dut.Add("key", Node::MakeNull()), GetExpectedCannot("Add"));
DRAKE_EXPECT_THROWS_MESSAGE(
dut.At("key"), GetExpectedCannot("At"));
DRAKE_EXPECT_THROWS_MESSAGE(
Expand All @@ -227,12 +232,12 @@ struct VisitorThatCopies {
sequence = data.sequence;
}
void operator()(const Node::MappingData& data) {
map = data.map;
mapping = data.mapping;
}

std::optional<std::string> scalar;
std::optional<std::vector<Node>> sequence;
std::optional<std::map<std::string, Node>> map;
std::optional<std::map<std::string, Node>> mapping;
};

// Check visiting.
Expand All @@ -242,22 +247,25 @@ TEST_P(YamlNodeParamaterizedTest, Visiting) {
dut.Visit(visitor);
switch (GetExpectedType()) {
case NodeType::kScalar: {
EXPECT_EQ(visitor.scalar, std::optional<std::string>{"foo"});
const std::string empty;
EXPECT_EQ(visitor.scalar.value_or(empty), "foo");
EXPECT_FALSE(visitor.sequence.has_value());
EXPECT_FALSE(visitor.map.has_value());
EXPECT_FALSE(visitor.mapping.has_value());
return;
}
case NodeType::kSequence: {
ASSERT_EQ(visitor.sequence.value_or(std::vector<Node>{}).size(), 1);
const std::vector<Node> empty;
ASSERT_EQ(visitor.sequence.value_or(empty).size(), 1);
EXPECT_EQ(visitor.sequence->front().GetScalar(), "item");
EXPECT_FALSE(visitor.scalar.has_value());
EXPECT_FALSE(visitor.map.has_value());
EXPECT_FALSE(visitor.mapping.has_value());
return;
}
case NodeType::kMapping: {
ASSERT_EQ(visitor.map.value_or(std::map<std::string, Node>{}).size(), 1);
EXPECT_EQ(visitor.map->begin()->first, "key");
EXPECT_EQ(visitor.map->at("key").GetScalar(), "value");
const std::map<std::string, Node> empty;
ASSERT_EQ(visitor.mapping.value_or(empty).size(), 1);
EXPECT_EQ(visitor.mapping->begin()->first, "key");
EXPECT_EQ(visitor.mapping->at("key").GetScalar(), "value");
EXPECT_FALSE(visitor.scalar.has_value());
EXPECT_FALSE(visitor.sequence.has_value());
return;
Expand All @@ -266,6 +274,25 @@ TEST_P(YamlNodeParamaterizedTest, Visiting) {
DRAKE_UNREACHABLE();
}

// Check debug printing using operator<<.
TEST_P(YamlNodeParamaterizedTest, ToString) {
Node dut = MakeNonEmptyDut();

// Confirm that the code is being run by checking for a notable
// substring within the printed result.
std::string needle;
switch (GetExpectedType()) {
case NodeType::kScalar: { needle = "foo"; break; }
case NodeType::kSequence: { needle = "item"; break; }
case NodeType::kMapping: { needle = "key"; break; }
}
EXPECT_THAT(fmt::format("{}", dut), testing::HasSubstr(needle));

// Confirm that tags are represented somehow.
dut.SetTag("MyTag");
EXPECT_THAT(fmt::format("{}", dut), testing::HasSubstr("MyTag"));
}

INSTANTIATE_TEST_SUITE_P(Suite, YamlNodeParamaterizedTest, testing::Values(
Param(NodeType::kScalar, "Scalar"),
Param(NodeType::kSequence, "Sequence"),
Expand Down
60 changes: 51 additions & 9 deletions common/yaml/yaml_node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ template <typename T> std::string_view GetNiceVariantName(const T&) {

} // namespace

Node::Node() = default;

Node Node::MakeScalar(std::string value) {
Node result;
result.data_ = ScalarData{std::move(value)};
Expand All @@ -47,6 +49,13 @@ Node Node::MakeMapping() {
return result;
}

Node Node::MakeNull() {
Node result;
result.data_ = ScalarData{"null"};
result.tag_ = kTagNull;
return result;
}

NodeType Node::GetType() const {
return std::visit(overloaded{
[](const ScalarData&) { return NodeType::kScalar; },
Expand Down Expand Up @@ -80,10 +89,6 @@ bool Node::IsScalar() const {
return std::holds_alternative<ScalarData>(data_);
}

bool Node::IsEmptyScalar() const {
return IsScalar() && GetScalar().empty();
}

bool Node::IsSequence() const {
return std::holds_alternative<SequenceData>(data_);
}
Expand All @@ -105,7 +110,7 @@ bool operator==(const Node::SequenceData& a, const Node::SequenceData& b) {
}

bool operator==(const Node::MappingData& a, const Node::MappingData& b) {
return a.map == b.map;
return a.mapping == b.mapping;
}

const std::string& Node::GetTag() const {
Expand Down Expand Up @@ -155,7 +160,7 @@ void Node::Add(Node value) {
const std::map<std::string, Node>& Node::GetMapping() const {
return std::visit(overloaded{
[](const MappingData& data) -> const std::map<std::string, Node>& {
return data.map;
return data.mapping;
},
[](auto&& data) -> const std::map<std::string, Node>& {
throw std::logic_error(fmt::format(
Expand All @@ -167,7 +172,8 @@ const std::map<std::string, Node>& Node::GetMapping() const {
void Node::Add(std::string key, Node value) {
return std::visit(overloaded{
[&key, &value](MappingData& data) -> void {
const auto result = data.map.insert({std::move(key), std::move(value)});
const auto result = data.mapping.insert({
std::move(key), std::move(value)});
const bool inserted = result.second;
if (!inserted) {
// Our 'key' argument is now empty (because it has been moved-from), so
Expand All @@ -187,7 +193,7 @@ void Node::Add(std::string key, Node value) {
Node& Node::At(std::string_view key) {
return std::visit(overloaded{
[key](MappingData& data) -> Node& {
return data.map.at(std::string{key});
return data.mapping.at(std::string{key});
},
[](auto&& data) -> Node& {
throw std::logic_error(fmt::format(
Expand All @@ -199,7 +205,7 @@ Node& Node::At(std::string_view key) {
void Node::Remove(std::string_view key) {
return std::visit(overloaded{
[key](MappingData& data) -> void {
auto erased = data.map.erase(std::string{key});
auto erased = data.mapping.erase(std::string{key});
if (!erased) {
throw std::logic_error(fmt::format(
"No such key '{}' during Node::Remove(key)", key));
Expand All @@ -212,6 +218,42 @@ void Node::Remove(std::string_view key) {
}, data_);
}

std::ostream& operator<<(std::ostream& os, const Node& node) {
if (!node.GetTag().empty()) {
os << "!<" << node.GetTag() << "> ";
}
node.Visit(overloaded{
[&](const Node::ScalarData& data) {
os << '"' << data.scalar << '"';
},
[&](const Node::SequenceData& data) {
os << "[";
bool first = true;
for (const auto& child : data.sequence) {
if (!first) {
os << ", ";
}
first = false;
os << child;
}
os << "]";
},
[&](const Node::MappingData& data) {
os << "{";
bool first = true;
for (const auto& [key, child] : data.mapping) {
if (!first) {
os << ", ";
}
first = false;
os << '"' << key << '"' << ": " << child;
}
os << "}";
},
});
return os;
}

} // namespace internal
} // namespace yaml
} // namespace drake
34 changes: 26 additions & 8 deletions common/yaml/yaml_node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ enum class NodeType {
//
// Note that even though YAML in general allows the keys of a mapping to be
// any type of node, in our implementation we limit keys to be only strings,
// for better compaibility with other serialization formats such as JSON.
// for better compatibility with other serialization formats such as JSON.
kMapping,
};

Expand All @@ -63,7 +63,7 @@ Refer to https://yaml.org/spec/1.2.2/#nodes for details.
Note that even though YAML in general allows the keys of a mapping to be
any type of node, in our implementation we limit keys to be only strings,
for better compaibility with other serialization formats such as JSON.
for better compatibility with other serialization formats such as JSON.
Each node may also have a tag. By default (i.e., at construction time),
the tag will be empty. Use GetTag() and SetTag() to query and adjust it.
Expand All @@ -86,8 +86,10 @@ class Node final {
/* Returns an empty Mapping node. */
static Node MakeMapping();

/* Creates an empty Scalar node. */
Node() = default;
/* Returns a null Scalar node.
The returned node's tag is kTagNull and value is "null".
Refer to https://yaml.org/spec/1.2.2/#null for details. */
static Node MakeNull();

/* Returns type of stored value. */
NodeType GetType() const;
Expand All @@ -103,9 +105,6 @@ class Node final {
/* Returns true iff this Node's type is Scalar. */
bool IsScalar() const;

/* Returns true iff this Node's type is Scalar and the value is empty. */
bool IsEmptyScalar() const;

/* Returns true iff this Node's type is Sequence. */
bool IsSequence() const;

Expand All @@ -126,6 +125,18 @@ class Node final {
type nor value. The caller is responsible for providing a valid tag. */
void SetTag(std::string);

// https://yaml.org/spec/1.2.2/#floating-point
static constexpr std::string_view kTagFloat{"tag:yaml.org,2002:float"};

// https://yaml.org/spec/1.2.2/#integer
static constexpr std::string_view kTagInt{"tag:yaml.org,2002:int"};

// https://yaml.org/spec/1.2.2/#null
static constexpr std::string_view kTagNull{"tag:yaml.org,2002:null"};

// https://yaml.org/spec/1.2.2/#generic-string
static constexpr std::string_view kTagStr{"tag:yaml.org,2002:str"};

// @name Scalar-only Functions
// These functions may only be called when IsScalar() is true;
// otherwise, they will throw an exception.
Expand Down Expand Up @@ -209,12 +220,19 @@ class Node final {
struct MappingData final {
// Even though YAML mappings are notionally unordered, we use an ordered
// map here to ensure program determinism.
std::map<std::string, Node> map;
std::map<std::string, Node> mapping;

friend bool operator==(const MappingData&, const MappingData&);
};

/* Displays the given node using flow style. Intended only for debugging,
not serialization. */
friend std::ostream& operator<<(std::ostream&, const Node&);

private:
/* No-op for use only by the public "Make..." functions. */
Node();

using Variant = std::variant<ScalarData, SequenceData, MappingData>;

std::string tag_;
Expand Down
Loading

0 comments on commit 4c61b32

Please sign in to comment.