[JSON] Add ObjectMapper::mapOptional to validate optional data.

[sammccall]

Summary

Currently the idiom for mapping optional fields is:

ObjectMapper O(Val, P);
if (!O.map("required1", Out.R1) || !O.map("required2", Out.R2))
  return false;
O.map("optional1", Out.O1); // ignore result
return true;

If optional1 is present but malformed, then we won't detect/report
that error. We may even leave Out in an incomplete state while returning true.
Instead, we'd often prefer to ignore optional1 if it is absent, but otherwise
behave just like map().

llvm/include/llvm/Support/JSON.h

 }
 
 /// Helper for mapping JSON objects onto protocol structs.
 ///
 /// Example:
 /// \code
 ///   bool fromJSON(const Value &E, MyStruct &R, Path P) {
 ///     ObjectMapper O(E, P);
-///     if (!O || !O.map("mandatory_field", R.MandatoryField))
-///       return false; // error details are already reported
-///     O.map("optional_field", R.OptionalField);
-///     return true;
+///     // When returning false, error details were already reported.
+///     return O && O.map("mandatory_field", R.MandatoryField) &&
+///         O.mapOptional("optional_field", R.OptionalField);
 ///   }
 /// \endcode
 class ObjectMapper {
 public:
   /// If O is not an object, this mapper is invalid and an error is reported.
   ObjectMapper(const Value &E, Path P) : O(E.getAsObject()), P(P) {
     if (!O)
       P.report("expected object");
public:
   template <typename T> bool map(StringLiteral Prop, llvm::Optional<T> &Out) {
     assert(*this && "Must check this is an object before calling map()");
     if (const Value *E = O->get(Prop))
       return fromJSON(*E, Out, P.field(Prop));
     Out = llvm::None;
     return true;
   }
 
+  /// Maps a property to a field, if it exists.
+  /// If the property exists and is invalid, reports an error.
+  /// If the property does not exist, Out is unchanged.
+  template <typename T> bool mapOptional(StringLiteral Prop, T &Out) {
+    assert(*this && "Must check this is an object before calling map()");
+    if (const Value *E = O->get(Prop))
+      return fromJSON(*E, Out, P.field(Prop));
+    return true;
+  }
+
 private:
   const Object *O;
   Path P;
 };
 
 /// Parses the provided JSON source, or returns a ParseError.
 /// The returned Value is self-contained and owns its strings (they do not refer
 /// to the original source).

llvm/unittests/Support/JSONTest.cpp

 }
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
                                      const CustomStruct &S) {
   return OS << "(" << S.S << ", " << (S.I ? std::to_string(*S.I) : "None")
             << ", " << S.B << ")";
 }
 bool fromJSON(const Value &E, CustomStruct &R, Path P) {
   ObjectMapper O(E, P);
-  if (!O || !O.map("str", R.S) || !O.map("int", R.I))
-    return false;
-  O.map("bool", R.B);
-  return true;
+  return O && O.map("str", R.S) && O.map("int", R.I) &&
+         O.mapOptional("bool", R.B);
 }
 
 static std::string errorContext(const Value &V, const Path::Root &R) {
   std::string Context;
   llvm::raw_string_ostream OS(Context);
   R.printErrorContext(V, OS);
   return OS.str();
 }
 
 TEST(JSONTest, Deserialize) {
   std::map<std::string, std::vector<CustomStruct>> R;
   CustomStruct ExpectedStruct = {"foo", 42, true};
   std::map<std::string, std::vector<CustomStruct>> Expected;
-  Value J = Object{
-      {"foo",
-       Array{
+  Value J = Object{{"foo", Array{
                                Object{
                                    {"str", "foo"},
                                    {"int", 42},
                                    {"bool", true},
                                    {"unknown", "ignored"},
                                },
                                Object{{"str", "bar"}},
-           Object{
-               {"str", "baz"}, {"bool", "string"}, // OK, deserialize ignores.
-           },
                            }}};
   Expected["foo"] = {
       CustomStruct("foo", 42, true),
       CustomStruct("bar", llvm::None, false),
-      CustomStruct("baz", llvm::None, false),
   };
   Path::Root Root("CustomStruct");
   ASSERT_TRUE(fromJSON(J, R, Root));
   EXPECT_EQ(R, Expected);
 
   (*J.getAsObject()->getArray("foo"))[0] = 123;
   ASSERT_FALSE(fromJSON(J, R, Root));
   EXPECT_EQ("expected object at CustomStruct.foo[0]",
             toString(Root.getError()));
   const char *ExpectedDump = R"({
   "foo": [
     /* error: expected object */
     123,
-    { ... },
     { ... }
   ]
 })";
   EXPECT_EQ(ExpectedDump, errorContext(J, Root));
 
   CustomStruct V;
   EXPECT_FALSE(fromJSON(nullptr, V, Root));
   EXPECT_EQ("expected object when parsing CustomStruct",
             toString(Root.getError()));
 
   EXPECT_FALSE(fromJSON(Object{}, V, Root));
   EXPECT_EQ("missing value at CustomStruct.str", toString(Root.getError()));
 
   EXPECT_FALSE(fromJSON(Object{{"str", 1}}, V, Root));
   EXPECT_EQ("expected string at CustomStruct.str", toString(Root.getError()));
 
   // Optional<T> must parse as the correct type if present.
   EXPECT_FALSE(fromJSON(Object{{"str", "1"}, {"int", "string"}}, V, Root));
   EXPECT_EQ("expected integer at CustomStruct.int", toString(Root.getError()));
+
+  // mapOptional must parse as the correct type if present.
+  EXPECT_FALSE(fromJSON(Object{{"str", "1"}, {"bool", "string"}}, V, Root));
+  EXPECT_EQ("expected boolean at CustomStruct.bool", toString(Root.getError()));
 }
 
 TEST(JSONTest, ParseDeserialize) {
   auto E = parse<std::vector<CustomStruct>>(R"json(
     [{"str": "foo", "int": 42}, {"int": 42}]
   )json");
   EXPECT_THAT_EXPECTED(E, FailedWithMessage("missing value at (root)[1].str"));