Detect invalid yaml. (#3239)

Previous code could fail silently, only caught with yaml output
mismatches. This uses ErrorOr to be explicit about the error detection;
recovery isn't supported because we should only print valid yaml (in
tests, at least).

Also moves the test helpers from //toolchain/base to
//toolchain/testing.

Switches tree_test to use the matchers so that, on mismatch, gtest
prints a path to the mismatch (the straight value means it was just
printing "not equal").

Addresses
https://github.com/carbon-language/carbon-lang/pull/3217#discussion_r1323586836

toolchain/base/BUILD

@@ -22,15 +22,3 @@ cc_library(
         "@llvm-project//llvm:Support",
     ],
 )
-
-cc_library(
-    name = "yaml_test_helpers",
-    testonly = 1,
-    srcs = ["yaml_test_helpers.cpp"],
-    hdrs = ["yaml_test_helpers.h"],
-    deps = [
-        "//common:ostream",
-        "@com_google_googletest//:gtest",
-        "@llvm-project//llvm:Support",
-    ],
-)

toolchain/driver/BUILD

@@ -45,9 +45,9 @@ cc_test(
         ":driver",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
-        "//toolchain/base:yaml_test_helpers",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/lex:tokenized_buffer_test_helpers",
+        "//toolchain/testing:yaml_test_helpers",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Object",
         "@llvm-project//llvm:Support",

toolchain/driver/driver_test.cpp

@@ -15,10 +15,12 @@
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "testing/base/test_raw_ostream.h"
+#include "toolchain/testing/yaml_test_helpers.h"
 
 namespace Carbon::Testing {
 namespace {
 
+using ::testing::_;
 using ::testing::ContainsRegex;
 using ::testing::HasSubstr;
 using ::testing::StrEq;
@@ -125,7 +127,8 @@ TEST_F(DriverTest, DumpTokens) {
       driver_.RunCommand({"compile", "--phase=lex", "--dump-tokens", file}));
   EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
   // Verify there is output without examining it.
-  EXPECT_FALSE(test_output_stream_.TakeStr().empty());
+  EXPECT_THAT(Yaml::Value::FromText(test_output_stream_.TakeStr()),
+              Yaml::IsYaml(_));
 }
 
 TEST_F(DriverTest, DumpParseTree) {
@@ -134,7 +137,8 @@ TEST_F(DriverTest, DumpParseTree) {
       {"compile", "--phase=parse", "--dump-parse-tree", file}));
   EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
   // Verify there is output without examining it.
-  EXPECT_FALSE(test_output_stream_.TakeStr().empty());
+  EXPECT_THAT(Yaml::Value::FromText(test_output_stream_.TakeStr()),
+              Yaml::IsYaml(_));
 }
 
 TEST_F(DriverTest, StdoutOutput) {

toolchain/lex/BUILD

@@ -215,9 +215,9 @@ cc_test(
         ":tokenized_buffer_test_helpers",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
-        "//toolchain/base:yaml_test_helpers",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/diagnostics:mocks",
+        "//toolchain/testing:yaml_test_helpers",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Support",
     ],

toolchain/lex/tokenized_buffer_test.cpp

@@ -12,10 +12,10 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "testing/base/test_raw_ostream.h"
-#include "toolchain/base/yaml_test_helpers.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/diagnostics/mocks.h"
 #include "toolchain/lex/tokenized_buffer_test_helpers.h"
+#include "toolchain/testing/yaml_test_helpers.h"
 
 namespace Carbon::Testing {
 namespace {
@@ -27,6 +27,7 @@ using ::testing::_;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::HasSubstr;
+using ::testing::Pair;
 
 class LexerTest : public ::testing::Test {
  protected:
@@ -1003,36 +1004,29 @@ TEST_F(LexerTest, PrintingAsYaml) {
 
   EXPECT_THAT(
       Yaml::Value::FromText(print_stream.TakeStr()),
-      ElementsAre(Yaml::SequenceValue{Yaml::MappingValue{
-          {"filename", Yaml::ScalarValue{source_storage_.front().filename()}},
-          {"tokens", Yaml::SequenceValue{
-                         Yaml::MappingValue{{"index", "0"},
-                                            {"kind", "Semi"},
-                                            {"line", "2"},
-                                            {"column", "2"},
-                                            {"indent", "2"},
-                                            {"spelling", ";"},
-                                            {"has_trailing_space", "true"}},
-                         Yaml::MappingValue{{"index", "1"},
-                                            {"kind", "Semi"},
-                                            {"line", "5"},
-                                            {"column", "1"},
-                                            {"indent", "1"},
-                                            {"spelling", ";"},
-                                            {"has_trailing_space", "true"}},
-                         Yaml::MappingValue{{"index", "2"},
-                                            {"kind", "Semi"},
-                                            {"line", "5"},
-                                            {"column", "3"},
-                                            {"indent", "1"},
-                                            {"spelling", ";"},
-                                            {"has_trailing_space", "true"}},
-                         Yaml::MappingValue{{"index", "3"},
-                                            {"kind", "EndOfFile"},
-                                            {"line", "15"},
-                                            {"column", "1"},
-                                            {"indent", "1"},
-                                            {"spelling", ""}}}}}}));
+      IsYaml(ElementsAre(Yaml::Sequence(ElementsAre(Yaml::Mapping(ElementsAre(
+          Pair("filename", source_storage_.front().filename().str()),
+          Pair("tokens",
+               Yaml::Sequence(ElementsAre(
+                   Yaml::Mapping(ElementsAre(
+                       Pair("index", "0"), Pair("kind", "Semi"),
+                       Pair("line", "2"), Pair("column", "2"),
+                       Pair("indent", "2"), Pair("spelling", ";"),
+                       Pair("has_trailing_space", "true"))),
+                   Yaml::Mapping(
+                       ElementsAre(Pair("index", "1"), Pair("kind", "Semi"),
+                                   Pair("line", "5"), Pair("column", "1"),
+                                   Pair("indent", "1"), Pair("spelling", ";"),
+                                   Pair("has_trailing_space", "true"))),
+                   Yaml::Mapping(
+                       ElementsAre(Pair("index", "2"), Pair("kind", "Semi"),
+                                   Pair("line", "5"), Pair("column", "3"),
+                                   Pair("indent", "1"), Pair("spelling", ";"),
+                                   Pair("has_trailing_space", "true"))),
+                   Yaml::Mapping(ElementsAre(
+                       Pair("index", "3"), Pair("kind", "EndOfFile"),
+                       Pair("line", "15"), Pair("column", "1"),
+                       Pair("indent", "1"), Pair("spelling", "")))))))))))));
 }
 
 }  // namespace

toolchain/parse/BUILD

@@ -70,10 +70,10 @@ cc_test(
         "//common:ostream",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
-        "//toolchain/base:yaml_test_helpers",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/diagnostics:mocks",
         "//toolchain/lex:tokenized_buffer",
+        "//toolchain/testing:yaml_test_helpers",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Support",
     ],

toolchain/parse/tree_test.cpp

@@ -10,16 +10,17 @@
 #include <forward_list>
 
 #include "testing/base/test_raw_ostream.h"
-#include "toolchain/base/yaml_test_helpers.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/diagnostics/mocks.h"
 #include "toolchain/lex/tokenized_buffer.h"
+#include "toolchain/testing/yaml_test_helpers.h"
 
 namespace Carbon::Testing {
 namespace {
 
 using Parse::Tree;
 using ::testing::ElementsAre;
+using ::testing::Pair;
 
 class TreeTest : public ::testing::Test {
  protected:
@@ -56,26 +57,23 @@ TEST_F(TreeTest, PrintPostorderAsYAML) {
   TestRawOstream print_stream;
   tree.Print(print_stream);
 
-  auto file = Yaml::SequenceValue{
-      Yaml::MappingValue{{"kind", "FunctionIntroducer"}, {"text", "fn"}},
-      Yaml::MappingValue{{"kind", "Name"}, {"text", "F"}},
-      Yaml::MappingValue{{"kind", "ParameterListStart"}, {"text", "("}},
-      Yaml::MappingValue{
-          {"kind", "ParameterList"}, {"text", ")"}, {"subtree_size", "2"}},
-      Yaml::MappingValue{{"kind", "FunctionDeclaration"},
-                         {"text", ";"},
-                         {"subtree_size", "5"}},
-      Yaml::MappingValue{{"kind", "FileEnd"}, {"text", ""}},
-  };
-
-  auto root = Yaml::SequenceValue{
-      Yaml::MappingValue{
-          {"filename", "test.carbon"},
-          {"parse_tree", file},
-      },
-  };
-
-  EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(root));
+  auto file = Yaml::Sequence(ElementsAre(
+      Yaml::Mapping(
+          ElementsAre(Pair("kind", "FunctionIntroducer"), Pair("text", "fn"))),
+      Yaml::Mapping(ElementsAre(Pair("kind", "Name"), Pair("text", "F"))),
+      Yaml::Mapping(
+          ElementsAre(Pair("kind", "ParameterListStart"), Pair("text", "("))),
+      Yaml::Mapping(ElementsAre(Pair("kind", "ParameterList"),
+                                Pair("text", ")"), Pair("subtree_size", "2"))),
+      Yaml::Mapping(ElementsAre(Pair("kind", "FunctionDeclaration"),
+                                Pair("text", ";"), Pair("subtree_size", "5"))),
+      Yaml::Mapping(ElementsAre(Pair("kind", "FileEnd"), Pair("text", "")))));
+
+  auto root = Yaml::Sequence(ElementsAre(Yaml::Mapping(
+      ElementsAre(Pair("filename", "test.carbon"), Pair("parse_tree", file)))));
+
+  EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()),
+              IsYaml(ElementsAre(root)));
 }
 
 TEST_F(TreeTest, PrintPreorderAsYAML) {
@@ -85,40 +83,34 @@ TEST_F(TreeTest, PrintPreorderAsYAML) {
   TestRawOstream print_stream;
   tree.Print(print_stream, /*preorder=*/true);
 
-  auto parameter_list = Yaml::SequenceValue{
-      Yaml::MappingValue{
-          {"node_index", "2"}, {"kind", "ParameterListStart"}, {"text", "("}},
-  };
-
-  auto function_decl = Yaml::SequenceValue{
-      Yaml::MappingValue{
-          {"node_index", "0"}, {"kind", "FunctionIntroducer"}, {"text", "fn"}},
-      Yaml::MappingValue{{"node_index", "1"}, {"kind", "Name"}, {"text", "F"}},
-      Yaml::MappingValue{{"node_index", "3"},
-                         {"kind", "ParameterList"},
-                         {"text", ")"},
-                         {"subtree_size", "2"},
-                         {"children", parameter_list}},
-  };
-
-  auto file = Yaml::SequenceValue{
-      Yaml::MappingValue{{"node_index", "4"},
-                         {"kind", "FunctionDeclaration"},
-                         {"text", ";"},
-                         {"subtree_size", "5"},
-                         {"children", function_decl}},
-      Yaml::MappingValue{
-          {"node_index", "5"}, {"kind", "FileEnd"}, {"text", ""}},
-  };
-
-  auto root = Yaml::SequenceValue{
-      Yaml::MappingValue{
-          {"filename", "test.carbon"},
-          {"parse_tree", file},
-      },
-  };
-
-  EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(root));
+  auto parameter_list = Yaml::Sequence(ElementsAre(Yaml::Mapping(
+      ElementsAre(Pair("node_index", "2"), Pair("kind", "ParameterListStart"),
+                  Pair("text", "(")))));
+
+  auto function_decl = Yaml::Sequence(ElementsAre(
+      Yaml::Mapping(ElementsAre(Pair("node_index", "0"),
+                                Pair("kind", "FunctionIntroducer"),
+                                Pair("text", "fn"))),
+      Yaml::Mapping(ElementsAre(Pair("node_index", "1"), Pair("kind", "Name"),
+                                Pair("text", "F"))),
+      Yaml::Mapping(ElementsAre(Pair("node_index", "3"),
+                                Pair("kind", "ParameterList"),
+                                Pair("text", ")"), Pair("subtree_size", "2"),
+                                Pair("children", parameter_list)))));
+
+  auto file = Yaml::Sequence(ElementsAre(
+      Yaml::Mapping(ElementsAre(Pair("node_index", "4"),
+                                Pair("kind", "FunctionDeclaration"),
+                                Pair("text", ";"), Pair("subtree_size", "5"),
+                                Pair("children", function_decl))),
+      Yaml::Mapping(ElementsAre(Pair("node_index", "5"),
+                                Pair("kind", "FileEnd"), Pair("text", "")))));
+
+  auto root = Yaml::Sequence(ElementsAre(Yaml::Mapping(
+      ElementsAre(Pair("filename", "test.carbon"), Pair("parse_tree", file)))));
+
+  EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()),
+              IsYaml(ElementsAre(root)));
 }
 
 TEST_F(TreeTest, HighRecursion) {

toolchain/sem_ir/BUILD

@@ -74,8 +74,8 @@ cc_test(
         "//common:ostream",
         "//testing/base:gtest_main",
         "//testing/base:test_raw_ostream",
-        "//toolchain/base:yaml_test_helpers",
         "//toolchain/driver",
+        "//toolchain/testing:yaml_test_helpers",
         "@com_google_googletest//:gtest",
         "@llvm-project//llvm:Support",
     ],

toolchain/sem_ir/file_test.cpp

@@ -9,8 +9,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "testing/base/test_raw_ostream.h"
-#include "toolchain/base/yaml_test_helpers.h"
 #include "toolchain/driver/driver.h"
+#include "toolchain/testing/yaml_test_helpers.h"
 
 namespace Carbon::Testing {
 namespace {
@@ -72,7 +72,8 @@ TEST(SemIRTest, YAML) {
   auto root = Yaml::Sequence(ElementsAre(Yaml::Mapping(
       ElementsAre(Pair("filename", "test.carbon"), Pair("sem_ir", file)))));
 
-  EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(root));
+  EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()),
+              IsYaml(ElementsAre(root)));
 }
 
 }  // namespace

toolchain/testing/BUILD

@@ -2,6 +2,7 @@
 # Exceptions. See /LICENSE for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
+load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
 load("//testing/file_test:rules.bzl", "file_test")
 
 package(default_visibility = ["//visibility:public"])
@@ -23,3 +24,27 @@ file_test(
         "@llvm-project//llvm:Support",
     ],
 )
+
+cc_library(
+    name = "yaml_test_helpers",
+    testonly = 1,
+    srcs = ["yaml_test_helpers.cpp"],
+    hdrs = ["yaml_test_helpers.h"],
+    deps = [
+        "//common:error",
+        "//common:ostream",
+        "@com_google_googletest//:gtest",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_test(
+    name = "yaml_test_helpers_test",
+    size = "small",
+    srcs = ["yaml_test_helpers_test.cpp"],
+    deps = [
+        ":yaml_test_helpers",
+        "//testing/base:gtest_main",
+        "@com_google_googletest//:gtest",
+    ],
+)

toolchain/base/yaml_test_helpers.cpp → toolchain/testing/yaml_test_helpers.cpp

@@ -2,7 +2,7 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#include "toolchain/base/yaml_test_helpers.h"
+#include "toolchain/testing/yaml_test_helpers.h"
 
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/YAMLParser.h"
@@ -10,9 +10,7 @@
 namespace Carbon::Testing::Yaml {
 
 static auto Parse(llvm::yaml::Node* node) -> Value {
-  if (!node) {
-    return Value{ErrorValue()};
-  }
+  CARBON_CHECK(node != nullptr);
 
   // getType returns an unsigned int which should map to the enum.
   switch (static_cast<llvm::yaml::Node::NodeKind>(node->getType())) {
@@ -57,13 +55,25 @@ static auto Parse(llvm::yaml::Node* node) -> Value {
   llvm_unreachable("unknown yaml node kind");
 }
 
-auto Value::FromText(llvm::StringRef text) -> SequenceValue {
+auto Value::FromText(llvm::StringRef text) -> ErrorOr<SequenceValue> {
   llvm::SourceMgr sm;
+  std::optional<std::string> error_message;
+  sm.setDiagHandler(
+      [](const llvm::SMDiagnostic& diag, void* context) -> void {
+        auto* error_message = static_cast<std::optional<std::string>*>(context);
+        *error_message = std::string();
+        llvm::raw_string_ostream stream(**error_message);
+        diag.print(/*ProgName=*/nullptr, stream, /*ShowColors=*/false);
+      },
+      &error_message);
   llvm::yaml::Stream yaml_stream(text, sm);
 
   SequenceValue result;
   for (llvm::yaml::Document& document : yaml_stream) {
     result.push_back(Parse(document.getRoot()));
+    if (error_message) {
+      return Error(*error_message);
+    }
   }
   return result;
 }
@@ -74,7 +84,6 @@ auto operator<<(std::ostream& os, const Value& v) -> std::ostream& {
   struct Printer {
     auto operator()(NullValue /*v*/) -> void { out << "Yaml::NullValue()"; }
     auto operator()(AliasValue /*v*/) -> void { out << "Yaml::AliasValue()"; }
-    auto operator()(ErrorValue /*v*/) -> void { out << "Yaml::ErrorValue()"; }
     auto operator()(const ScalarValue& v) -> void { out << std::quoted(v); }
     auto operator()(const MappingValue& v) -> void {
       out << "Yaml::MappingValue{";

toolchain/base/yaml_test_helpers.h → toolchain/testing/yaml_test_helpers.h

@@ -27,33 +27,33 @@
 //     // Exact values can be matched by constructing the desired value.
 //     EXPECT_THAT(
 //         yaml,
-//         ElementsAre(
+//         Yaml::IsYaml(ElementsAre(
 //             Yaml::MappingValue{
 //                 {"fruits", Yaml::SequenceValue{"apple", "orange", "pear"}}},
 //             Yaml::SequenceValue{Yaml::MappingValue{
 //                 {Yaml::SequenceValue{Yaml::MappingValue{{"foo", "bar"}}},
-//                  "baz"}}}));
+//                  "baz"}}})));
 //
 //     // Properties can be checked using Yaml::Mapping or Yaml::Sequence to
 //     // adapt regular gmock container matchers.
 //     EXPECT_THAT(
 //         yaml,
-//         Contains(Yaml::Mapping(
-//             Contains(Pair("fruits", Yaml::Sequence(Contains("orange")))))));
+//         Yaml::IsYaml(Contains(Yaml::Mapping(
+//             Contains(Pair("fruits", Yaml::Sequence(Contains("orange"))))))));
 //
 // On match failure, Yaml::Values are printed as C++ code that can be used to
 // recreate the value, for easy copy-pasting into test expectations.
 
-#ifndef CARBON_TOOLCHAIN_BASE_YAML_TEST_HELPERS_H_
-#define CARBON_TOOLCHAIN_BASE_YAML_TEST_HELPERS_H_
+#ifndef CARBON_TOOLCHAIN_TESTING_YAML_TEST_HELPERS_H_
+#define CARBON_TOOLCHAIN_TESTING_YAML_TEST_HELPERS_H_
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 #include <iostream>
-#include <sstream>
 #include <variant>
 
+#include "common/error.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace Carbon::Testing::Yaml {
@@ -75,38 +75,50 @@ using ScalarValue = std::string;
 using MappingValue = std::vector<std::pair<Value, Value>>;
 using SequenceValue = std::vector<Value>;
 struct AliasValue : EmptyComparable {};
-struct ErrorValue : EmptyComparable {};
 
 // A thin wrapper around a variant of possible YAML value types. This type
 // intentionally provides no additional encapsulation or invariants beyond
 // those of the variant.
 struct Value : std::variant<NullValue, ScalarValue, MappingValue, SequenceValue,
-                            AliasValue, ErrorValue> {
+                            AliasValue> {
   using variant::variant;
 
   // Prints the Value in the form of code to recreate the value.
   friend auto operator<<(std::ostream& os, const Value& v) -> std::ostream&;
 
   // Parses a sequence of YAML documents from the given YAML text.
-  static auto FromText(llvm::StringRef text) -> SequenceValue;
+  static auto FromText(llvm::StringRef text) -> ErrorOr<SequenceValue>;
 };
 
-template <typename T>
-auto DescribeMatcher(::testing::Matcher<T> matcher) -> std::string {
-  std::ostringstream out;
-  matcher.DescribeTo(&out);
-  return out.str();
+// Used to examine the results of Value::FromText.
+// NOLINTNEXTLINE: Expands from GoogleTest.
+MATCHER_P(IsYaml, matcher,
+          "is yaml root sequence that " +
+              ::testing::DescribeMatcher<SequenceValue>(matcher)) {
+  const ErrorOr<SequenceValue>& yaml = arg;
+  const ::testing::Matcher<SequenceValue>& typed_matcher = matcher;
+  if (yaml.ok()) {
+    // It's hard to intercept printing of the ErrorOr value, so just print it
+    // here.
+    *result_listener << "\n  which is: " << *yaml << "\n  ";
+    return typed_matcher.MatchAndExplain(*yaml, result_listener);
+  }
+
+  *result_listener << "\n  with the error: " << yaml.error() << "\n  ";
+  return false;
 }
 
 // Match a Value that is a MappingValue.
-// Same as testing::VariantWith<MappingValue>(contents).
+// Similar to testing::VariantWith<MappingValue>(matcher), but with better
+// descriptions.
 // NOLINTNEXTLINE: Expands from GoogleTest.
-MATCHER_P(Mapping, contents,
-          "is mapping that " + DescribeMatcher<MappingValue>(contents)) {
-  ::testing::Matcher<MappingValue> contents_matcher = contents;
-
-  if (auto* map = std::get_if<MappingValue>(&arg)) {
-    return contents_matcher.MatchAndExplain(*map, result_listener);
+MATCHER_P(Mapping, matcher,
+          "is mapping that " +
+              ::testing::DescribeMatcher<MappingValue>(matcher)) {
+  const Value& val = arg;
+  const ::testing::Matcher<MappingValue>& typed_matcher = matcher;
+  if (const auto* map = std::get_if<MappingValue>(&val)) {
+    return typed_matcher.MatchAndExplain(*map, result_listener);
   }
 
   *result_listener << "which is not a mapping";
@@ -114,14 +126,16 @@ MATCHER_P(Mapping, contents,
 }
 
 // Match a Value that is a SequenceValue.
-// Same as testing::VariantWith<SequenceValue>(contents).
+// Similar to testing::VariantWith<SequenceValue>(matcher), but with better
+// descriptions.
 // NOLINTNEXTLINE: Expands from GoogleTest.
-MATCHER_P(Sequence, contents,
-          "is mapping that " + DescribeMatcher<SequenceValue>(contents)) {
-  ::testing::Matcher<SequenceValue> contents_matcher = contents;
-
-  if (auto* map = std::get_if<SequenceValue>(&arg)) {
-    return contents_matcher.MatchAndExplain(*map, result_listener);
+MATCHER_P(Sequence, matcher,
+          "is sequence that " +
+              ::testing::DescribeMatcher<SequenceValue>(matcher)) {
+  const Value& val = arg;
+  const ::testing::Matcher<SequenceValue>& typed_matcher = matcher;
+  if (const auto* map = std::get_if<SequenceValue>(&val)) {
+    return typed_matcher.MatchAndExplain(*map, result_listener);
   }
 
   *result_listener << "which is not a sequence";
@@ -129,14 +143,16 @@ MATCHER_P(Sequence, contents,
 }
 
 // Match a Value that is a ScalarValue.
-// Same as testing::VariantWith<ScalarValue>(contents).
+// Similar to testing::VariantWith<ScalarValue>(matcher), but with better
+// descriptions.
 // NOLINTNEXTLINE: Expands from GoogleTest.
-MATCHER_P(Scalar, value,
-          "has scalar value " + DescribeMatcher<std::string>(value)) {
-  ::testing::Matcher<ScalarValue> value_matcher = value;
-
-  if (auto* map = std::get_if<ScalarValue>(&arg)) {
-    return value_matcher.MatchAndExplain(*map, result_listener);
+MATCHER_P(Scalar, matcher,
+          "has scalar value " +
+              ::testing::DescribeMatcher<ScalarValue>(matcher)) {
+  const Value& val = arg;
+  const ::testing::Matcher<ScalarValue>& typed_matcher = matcher;
+  if (const auto* map = std::get_if<ScalarValue>(&val)) {
+    return typed_matcher.MatchAndExplain(*map, result_listener);
   }
 
   *result_listener << "which is not a scalar";
@@ -145,4 +161,4 @@ MATCHER_P(Scalar, value,
 
 }  // namespace Carbon::Testing::Yaml
 
-#endif  // CARBON_TOOLCHAIN_BASE_YAML_TEST_HELPERS_H_
+#endif  // CARBON_TOOLCHAIN_TESTING_YAML_TEST_HELPERS_H_

toolchain/testing/yaml_test_helpers_test.cpp

@@ -0,0 +1,32 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "toolchain/testing/yaml_test_helpers.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace Carbon::Testing {
+namespace {
+
+using ::testing::_;
+using ::testing::ElementsAre;
+using ::testing::Not;
+
+TEST(YamlTestHelpersTest, ValidYaml) {
+  EXPECT_THAT(
+      Yaml::Value::FromText("[foo, bar]"),
+      Yaml::IsYaml(ElementsAre(Yaml::Sequence(ElementsAre("foo", "bar")))));
+}
+
+TEST(YamlTestHelpersTest, InvalidYaml) {
+  auto result = Yaml::Value::FromText("- foo\nbar");
+  // Make sure we've constructed invalid YAML.
+  EXPECT_FALSE(result.ok());
+  // Make sure the matcher detects the invalid YAML.
+  EXPECT_THAT(result, Not(Yaml::IsYaml(_)));
+}
+
+}  // namespace
+}  // namespace Carbon::Testing