reuse visitField for conversions on intermediate types (#52)

* reuse visitField for conversions on intermediate types

* add new name function and remove manual namespacing

Author: nathanhhughes

config_utilities/include/config_utilities/config.h

@@ -53,6 +53,17 @@ namespace config {
  */
 inline void name(const std::string& name) { internal::Visitor::visitName(name); }
 
+/**
+ * @brief Set the name of a config from the template parameter type.
+ * @tparam T type to get the name from
+ *
+ * This is for cases where you may not have a static config struct name (i.e., with a templated config struct)
+ */
+template <typename T>
+void name() {
+  internal::Visitor::visitName(internal::typeName<T>());
+}
+
 /**
  * @brief Declare string-named fields of the config. This string will be used to get the configs field values during
  * creation, and for checking of validity.

config_utilities/include/config_utilities/internal/visitor.h

@@ -104,8 +104,8 @@ struct Visitor {
   template <typename T, typename std::enable_if<!isConfig<T>(), bool>::type = true>
   static void visitField(T& field, const std::string& field_name, const std::string& unit);
 
-  // Non-config types with a conversion.
-  template <typename Conversion, typename T, typename std::enable_if<!isConfig<T>(), bool>::type = true>
+  // Types with a extra conversion.
+  template <typename Conversion, typename T>
   static void visitField(T& field, const std::string& field_name, const std::string& unit);
 
   // Single config types.
@@ -166,11 +166,16 @@ struct Visitor {
   template <typename ConfigT, typename std::enable_if<is_virtual_config<ConfigT>::value, bool>::type = true>
   static MetaData getDefaults(const ConfigT& config);
 
-  // Dispatch getting field input info from conversions.
-  template <typename Conversion, typename std::enable_if<!hasFieldInputInfo<Conversion>(), bool>::type = true>
-  static FieldInputInfo::Ptr getFieldInputInfo();
-  template <typename Conversion, typename std::enable_if<hasFieldInputInfo<Conversion>(), bool>::type = true>
-  static FieldInputInfo::Ptr getFieldInputInfo();
+  // Dispatch populating field input info from conversions.
+  template <typename Conversion,
+            typename ConfigT,
+            typename std::enable_if<!hasFieldInputInfo<Conversion>() || isConfig<ConfigT>(), bool>::type = true>
+  static void getFieldInputInfo(const std::string& field_name);
+
+  template <typename Conversion,
+            typename ConfigT,
+            typename std::enable_if<hasFieldInputInfo<Conversion>() && !isConfig<ConfigT>(), bool>::type = true>
+  static void getFieldInputInfo(const std::string& field_name);
 
   // Computes the default values for all fields in the meta data. This assumes that the meta data is already created,
   // and the meta data was created from ConfigT.

config_utilities/include/config_utilities/internal/visitor_impl.hpp

@@ -188,26 +188,16 @@ void Visitor::visitField(T& field, const std::string& field_name, const std::str
 }
 
 // Visits a non-config field with conversion.
-template <typename Conversion, typename T, typename std::enable_if<!isConfig<T>(), bool>::type>
+template <typename Conversion, typename T>
 void Visitor::visitField(T& field, const std::string& field_name, const std::string& unit) {
   auto& visitor = Visitor::instance();
 
-  // record the field that we visited without storing the value
-  // kGet and kGetDefaults will populate the value field
-  auto& info = visitor.data.field_infos.emplace_back();
-  info.name = field_name;
-  info.unit = unit;
-
   if (visitor.mode == Visitor::Mode::kSet) {
     std::string error;
     auto intermediate = Conversion::toIntermediate(field, error);
     error.clear();  // We don't care about setting up the intermediate just to get data.
 
-    info.was_parsed = YamlParser::fromYaml(visitor.data.data, field_name, intermediate, visitor.name_space, error);
-    if (!error.empty()) {
-      visitor.data.errors.emplace_back(new Warning(field_name, error));
-      error.clear();
-    }
+    Visitor::visitField(intermediate, field_name, unit);
 
     Conversion::fromIntermediate(intermediate, field, error);
     if (!error.empty()) {
@@ -218,23 +208,17 @@ void Visitor::visitField(T& field, const std::string& field_name, const std::str
   if (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetDefaults ||
       visitor.mode == Visitor::Mode::kGetInfo) {
     std::string error;
-    const auto intermediate = Conversion::toIntermediate(field, error);
+    auto intermediate = Conversion::toIntermediate(field, error);
     if (!error.empty()) {
       visitor.data.errors.emplace_back(new Warning(field_name, error));
       error.clear();
     }
-    YAML::Node node = YamlParser::toYaml(field_name, intermediate, visitor.name_space, error);
-    mergeYamlNodes(visitor.data.data, node);
-    // This stores a reference to the node in the data.
-    info.value = lookupNamespace(node, joinNamespace(visitor.name_space, field_name));
-    if (!error.empty()) {
-      visitor.data.errors.emplace_back(new Warning(field_name, error));
-    }
+
+    Visitor::visitField(intermediate, field_name, unit);
 
     // Get type information if requested.
     if (visitor.mode == Visitor::Mode::kGetInfo) {
-      auto input_info = Visitor::getFieldInputInfo<Conversion>();
-      info.input_info = FieldInputInfo::merge(input_info, info.input_info);
+      Visitor::getFieldInputInfo<Conversion, T>(field_name);
     }
   }
 }
@@ -268,7 +252,7 @@ void Visitor::visitField(std::vector<ConfigT>& config, const std::string& field_
   }
 
   if (visitor.mode == Visitor::Mode::kSet) {
-    const auto array_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name;
+    const auto array_ns = joinNamespace(visitor.name_space, field_name);
     const auto subnode = lookupNamespace(visitor.data.data, array_ns);
     if (!subnode) {
       return;  // don't override the field if not present
@@ -346,7 +330,7 @@ void Visitor::visitField(OrderedMap<K, ConfigT>& config, const std::string& fiel
   }
 
   if (visitor.mode == Visitor::Mode::kSet) {
-    const auto map_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name;
+    const auto map_ns = joinNamespace(visitor.name_space, field_name);
     const auto subnode = lookupNamespace(visitor.data.data, map_ns);
     if (!subnode) {
       return;  // don't override the field if not present
@@ -470,14 +454,41 @@ void Visitor::getDefaultValues(const ConfigT& config, MetaData& data) {
   }
 }
 
-template <typename Conversion, typename std::enable_if<!hasFieldInputInfo<Conversion>(), bool>::type>
-FieldInputInfo::Ptr Visitor::getFieldInputInfo() {
-  return nullptr;
+// intentional no-op
+template <typename Conversion,
+          typename ConfigT,
+          typename std::enable_if<!hasFieldInputInfo<Conversion>() || isConfig<ConfigT>(), bool>::type>
+void Visitor::getFieldInputInfo(const std::string& field_name) {
+  static_assert(!isConfig<ConfigT>() || !hasFieldInputInfo<Conversion>(),
+                "Config types (with declare_config) cannot have field input information!");
+
+  if (isConfig<ConfigT>()) {
+    return;  // don't touch field info fields
+  }
+
+  auto& visitor = Visitor::instance();
+  if (visitor.data.field_infos.empty()) {
+    visitor.data.errors.emplace_back(
+        new Warning(field_name, "Invalid parsing state! Field info should already exist!"));
+  } else {
+    visitor.data.field_infos.back().input_info.reset();  // clear field input info for underlying intermediate type
+  }
 }
 
-template <typename Conversion, typename std::enable_if<hasFieldInputInfo<Conversion>(), bool>::type>
-FieldInputInfo::Ptr Visitor::getFieldInputInfo() {
-  return Conversion::getFieldInputInfo();
+template <typename Conversion,
+          typename ConfigT,
+          typename std::enable_if<hasFieldInputInfo<Conversion>() && !isConfig<ConfigT>(), bool>::type>
+void Visitor::getFieldInputInfo(const std::string& field_name) {
+  auto input_info = Conversion::getFieldInputInfo();
+
+  auto& visitor = Visitor::instance();
+  if (visitor.data.field_infos.empty()) {
+    visitor.data.errors.emplace_back(
+        new Warning(field_name, "Invalid parsing state! Field info should already exist!"));
+  } else {
+    auto& info = visitor.data.field_infos.back();
+    info.input_info = FieldInputInfo::merge(input_info, info.input_info);
+  }
 }
 
 }  // namespace config::internal

config_utilities/test/tests/conversions.cpp

@@ -83,6 +83,47 @@ struct TestConversionStruct {
   int test = 0;
 };
 
+struct IntermediateConfigConversionStruct {
+  struct Input {
+    int a = 4;
+    int b = 5;
+    int value() const { return a + b; }
+  };
+
+  struct Resolved {
+    std::vector<int> inputs;
+  };
+
+  struct Conversion {
+    static std::vector<Input> toIntermediate(const std::vector<int>& value, std::string& /* error */) {
+      std::vector<Input> to_return;
+      for (const auto total : value) {
+        to_return.push_back(Input{total, 0});
+      }
+
+      return to_return;
+    }
+
+    static void fromIntermediate(const std::vector<Input>& intermediate,
+                                 std::vector<int>& value,
+                                 std::string& /*error*/) {
+      value.clear();
+      for (const auto& input : intermediate) {
+        value.push_back(input.value());
+      }
+    }
+  };
+};
+
+void declare_config(IntermediateConfigConversionStruct::Input& config) {
+  field(config.a, "a");
+  field(config.b, "b");
+}
+
+void declare_config(IntermediateConfigConversionStruct::Resolved& config) {
+  field<IntermediateConfigConversionStruct::Conversion>(config.inputs, "inputs");
+}
+
 void declare_config(ConversionStruct& conf) {
   field<ThreadNumConversion>(conf.num_threads, "num_threads");
   field<CharConversion>(conf.some_character, "some_character");
@@ -206,4 +247,13 @@ TEST(Conversions, FieldInputInfo) {
   EXPECT_FALSE(data.field_infos[1].input_info);
 }
 
+TEST(Conversions, ConversionDeclareConfigDispatch) {
+  const std::string yaml_string = "inputs: [{a: 0, b: 1}, {a: 1, b: 1}, {a: 1, b: 2}, {a: 0, b: 4}]";
+  const auto node = YAML::Load(yaml_string);
+
+  std::vector<int> expected{1, 2, 3, 4};
+  const auto result = fromYaml<IntermediateConfigConversionStruct::Resolved>(node);
+  EXPECT_EQ(result.inputs, expected);
+}
+
 }  // namespace config::test