Merge topic 'improve-cmJSONHelpers'

17de44e99b cmJSONHelpers.h: Add some empty lines to split code blocks
0b334e5bfb cmJSONHelpers.h: Add generic predicate checking helper
503a73b183 cmJSONHelpers.h: Use `map::emplace()` instead of `operator[]`
e7dcd51a61 cmJSONHelpers.h: Remove useless `cmStrCat()` call
5096ea7a92 cmJSONHelpers.h: Optimize of adding `Member`s to a vector
f4a2070731 cmJSONHelpers.h: Remove redundant `;`
23be530275 cmJSONHelpers.h: Move variable declaation close to first usage

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Acked-by: Martin Duffy <martin.duffy@kitware.com>
Merge-request: !9731

6 files changed, 49 insertions, 26 deletions

diff --git a/Source/cmCMakePresetsErrors.cxx b/Source/cmCMakePresetsErrors.cxx
index 5919512..ab0afed 100644
--- a/Source/cmCMakePresetsErrors.cxx
+++ b/Source/cmCMakePresetsErrors.cxx
@@ -238,6 +238,16 @@ void TRACE_UNSUPPORTED(cmJSONState* state)
   state->AddError("File version must be 7 or higher for trace preset support");
 }
 
+JsonErrors::ErrorGenerator UNRECOGNIZED_VERSION_RANGE(int min, int max)
+{
+  return [min, max](const Json::Value* value, cmJSONState* state) -> void {
+    state->AddErrorAtValue(cmStrCat("Unrecognized \"version\" ",
+                                    value->asString(), ": must be >=", min,
+                                    " and <=", max),
+                           value);
+  };
+}
+
 JsonErrors::ErrorGenerator UNRECOGNIZED_CMAKE_VERSION(
   const std::string& version, int current, int required)
 {
diff --git a/Source/cmCMakePresetsErrors.h b/Source/cmCMakePresetsErrors.h
index dda9274..22830d2 100644
--- a/Source/cmCMakePresetsErrors.h
+++ b/Source/cmCMakePresetsErrors.h
@@ -94,6 +94,8 @@ void CTEST_JUNIT_UNSUPPORTED(cmJSONState* state);
 
 void TRACE_UNSUPPORTED(cmJSONState* state);
 
+JsonErrors::ErrorGenerator UNRECOGNIZED_VERSION_RANGE(int min, int max);
+
 JsonErrors::ErrorGenerator UNRECOGNIZED_CMAKE_VERSION(
   const std::string& version, int current, int required);
 
diff --git a/Source/cmCMakePresetsGraphReadJSON.cxx b/Source/cmCMakePresetsGraphReadJSON.cxx
index df6482d..18ae9af 100644
--- a/Source/cmCMakePresetsGraphReadJSON.cxx
+++ b/Source/cmCMakePresetsGraphReadJSON.cxx
@@ -256,9 +256,14 @@ auto const VersionIntHelper =
 auto const VersionHelper = JSONHelperBuilder::Required<int>(
   cmCMakePresetsErrors::NO_VERSION, VersionIntHelper);
 
+auto const VersionRangeHelper = JSONHelperBuilder::Checked<int>(
+  cmCMakePresetsErrors::UNRECOGNIZED_VERSION_RANGE(MIN_VERSION, MAX_VERSION),
+  VersionHelper,
+  [](const int v) -> bool { return v >= MIN_VERSION && v <= MAX_VERSION; });
+
 auto const RootVersionHelper =
   JSONHelperBuilder::Object<int>(cmCMakePresetsErrors::INVALID_ROOT_OBJECT)
-    .Bind("version"_s, VersionHelper, false);
+    .Bind("version"_s, VersionRangeHelper, false);
 
 auto const CMakeVersionUIntHelper =
   JSONHelperBuilder::UInt(cmCMakePresetsErrors::INVALID_VERSION);
@@ -481,11 +486,6 @@ bool cmCMakePresetsGraph::ReadJSONFile(const std::string& filename,
   if ((result = RootVersionHelper(v, &root, &parseState)) != true) {
     return result;
   }
-  if (v < MIN_VERSION || v > MAX_VERSION) {
-    cmCMakePresetsErrors::UNRECOGNIZED_VERSION(&root["version"],
-                                               &this->parseState);
-    return false;
-  }
 
   // Support for build and test presets added in version 2.
   if (v < 2) {
diff --git a/Source/cmJSONHelpers.h b/Source/cmJSONHelpers.h
index 368a0df..0bb32f4 100644
--- a/Source/cmJSONHelpers.h
+++ b/Source/cmJSONHelpers.h
@@ -34,9 +34,11 @@ enum ObjectError
   ExtraField,
   MissingRequired
 };
+
 using ErrorGenerator = std::function<void(const Json::Value*, cmJSONState*)>;
 using ObjectErrorGenerator =
   std::function<ErrorGenerator(ObjectError, const Json::Value::Members&)>;
+
 ErrorGenerator EXPECTED_TYPE(const std::string& type);
 
 void INVALID_STRING(const Json::Value* value, cmJSONState* state);
@@ -104,7 +106,6 @@ struct cmJSONHelperBuilder
     bool operator()(T& out, const Json::Value* value, cmJSONState* state) const
     {
       Json::Value::Members extraFields;
-      bool success = true;
       if (!value && this->AnyRequired) {
         Error(JsonErrors::ObjectError::RequiredMissing, extraFields)(value,
                                                                      state);
@@ -125,6 +126,7 @@ struct cmJSONHelperBuilder
           extraFields.end());
       }
 
+      bool success = true;
       for (auto const& m : this->Members) {
         std::string name(m.Name.data(), m.Name.size());
         state->push_stack(name, value);
@@ -159,6 +161,12 @@ struct cmJSONHelperBuilder
                                               cmJSONState* state)>;
     struct Member
     {
+      Member(cm::string_view name, MemberFunction func, bool required)
+        : Name{ name }
+        , Function{ std::move(func) }
+        , Required{ required }
+      {
+      }
       cm::string_view Name;
       MemberFunction Function;
       bool Required;
@@ -171,14 +179,8 @@ struct cmJSONHelperBuilder
     Object& BindPrivate(const cm::string_view& name, MemberFunction&& func,
                         bool required)
     {
-      Member m;
-      m.Name = name;
-      m.Function = std::move(func);
-      m.Required = required;
-      this->Members.push_back(std::move(m));
-      if (required) {
-        this->AnyRequired = true;
-      }
+      this->Members.emplace_back(name, std::move(func), required);
+      this->AnyRequired = this->AnyRequired || required;
       return *this;
     }
   };
@@ -195,7 +197,6 @@ struct cmJSONHelperBuilder
       }
       if (!value->isString()) {
         error(value, state);
-        ;
         return false;
       }
       out = value->asString();
@@ -220,7 +221,6 @@ struct cmJSONHelperBuilder
       }
       if (!value->isInt()) {
         error(value, state);
-        ;
         return false;
       }
       out = value->asInt();
@@ -245,7 +245,6 @@ struct cmJSONHelperBuilder
       }
       if (!value->isUInt()) {
         error(value, state);
-        ;
         return false;
       }
       out = value->asUInt();
@@ -270,7 +269,6 @@ struct cmJSONHelperBuilder
       }
       if (!value->isBool()) {
         error(value, state);
-        ;
         return false;
       }
       out = value->asBool();
@@ -332,19 +330,18 @@ struct cmJSONHelperBuilder
     return [error, func, filter](std::map<std::string, T>& out,
                                  const Json::Value* value,
                                  cmJSONState* state) -> bool {
-      bool success = true;
       if (!value) {
         out.clear();
         return true;
       }
       if (!value->isObject()) {
         error(value, state);
-        ;
         return false;
       }
       out.clear();
+      bool success = true;
       for (auto const& key : value->getMemberNames()) {
-        state->push_stack(cmStrCat(key, ""), &(*value)[key]);
+        state->push_stack(key, &(*value)[key]);
         if (!filter(key)) {
           state->pop_stack();
           continue;
@@ -353,7 +350,7 @@ struct cmJSONHelperBuilder
         if (!func(t, &(*value)[key], state)) {
           success = false;
         }
-        out[key] = std::move(t);
+        out.emplace(key, std::move(t));
         state->pop_stack();
       }
       return success;
@@ -390,10 +387,24 @@ struct cmJSONHelperBuilder
                          cmJSONState* state) -> bool {
       if (!value) {
         error(value, state);
-        ;
         return false;
       }
       return func(out, value, state);
     };
   }
+
+  template <typename T, typename F, typename P>
+  static cmJSONHelper<T> Checked(const JsonErrors::ErrorGenerator& error,
+                                 F func, P predicate)
+  {
+    return [error, func, predicate](T& out, const Json::Value* value,
+                                    cmJSONState* state) -> bool {
+      bool result = func(out, value, state);
+      if (result && !predicate(out)) {
+        error(value, state);
+        result = false;
+      }
+      return result;
+    };
+  }
 };
diff --git a/Tests/RunCMake/CMakePresets/HighVersion-stderr.txt b/Tests/RunCMake/CMakePresets/HighVersion-stderr.txt
index 598478f..f8454b9 100644
--- a/Tests/RunCMake/CMakePresets/HighVersion-stderr.txt
+++ b/Tests/RunCMake/CMakePresets/HighVersion-stderr.txt
@@ -1,5 +1,5 @@
 ^CMake Error: Could not read presets from [^
 ]*/Tests/RunCMake/CMakePresets/HighVersion:
-Error: @2,14: Unrecognized "version" field
+Error: @2,14: Unrecognized "version" 1000: must be >=1 and <=10
   "version": 1000,
              \^$
diff --git a/Tests/RunCMake/CMakePresets/LowVersion-stderr.txt b/Tests/RunCMake/CMakePresets/LowVersion-stderr.txt
index e933100..f4f65f9 100644
--- a/Tests/RunCMake/CMakePresets/LowVersion-stderr.txt
+++ b/Tests/RunCMake/CMakePresets/LowVersion-stderr.txt
@@ -1,5 +1,5 @@
 ^CMake Error: Could not read presets from [^
 ]*/Tests/RunCMake/CMakePresets/LowVersion:
-Error: @2,14: Unrecognized "version" field
+Error: @2,14: Unrecognized "version" 0: must be >=1 and <=10
   "version": 0,
              \^