Feature/unset tracing (#20)

* add field info tracing in kSet and initial logging for missing fields

* add unit tests and use settings inside of setValues

* add todo note

* actually add simple tests

* fix duplicate printing and make test config nested

* use plain config structs to avoid poluting registry

* add verbose failure to ci

* combine test to avoid platform-specific failure

.github/workflows/cmake_build.yml

@@ -57,4 +57,4 @@ jobs:
       shell: bash
       # Execute tests defined by the CMake configuration.
       # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
-      run: ctest -C ${{env.BUILD_TYPE}}
+      run: ctest -C ${{env.BUILD_TYPE}} --rerun-failed --output-on-failure

.gitignore

@@ -64,3 +64,5 @@ CATKIN_IGNORE
 # Local utilities
 Notes.txt
 tmp/
+
+compile_commands.json

config_utilities/include/config_utilities/formatting/asl.h

@@ -56,6 +56,7 @@ class AslFormatter : public Formatter {
 
  protected:
   std::string formatErrorsImpl(const MetaData& data, const std::string& what, const Severity severity) override;
+  std::string formatMissingImpl(const MetaData& data, const std::string& what, const Severity severity) override;
   std::string formatConfigImpl(const MetaData& data) override;
   std::string formatConfigsImpl(const std::vector<MetaData>& data) override;
 
@@ -70,6 +71,7 @@ class AslFormatter : public Formatter {
 
   // Helper functions.
   std::string formatErrorsRecursive(const MetaData& data, const std::string& sev, const size_t length);
+  std::string formatMissingRecursive(const MetaData& data, const std::string& sev, const size_t length);
   std::string formatChecksInternal(const MetaData& data, const std::string& sev, const size_t length);
   std::string formatErrorsInternal(const MetaData& data, const std::string& sev, const size_t length);
   std::string toStringInternal(const MetaData& data, size_t indent) const;

config_utilities/include/config_utilities/internal/formatter.h

@@ -63,6 +63,11 @@ class Formatter {
                                   const std::string& what = "",
                                   const Severity severity = Severity::kWarning);
 
+  // Format all missing fields in the meta data into the display string.
+  static std::string formatMissing(const MetaData& data,
+                                   const std::string& what = "",
+                                   const Severity severity = Severity::kWarning);
+
   // Format the content of a single config the display string.
   static std::string formatConfig(const MetaData& data);
 
@@ -74,10 +79,12 @@ class Formatter {
 
  protected:
   virtual std::string formatErrorsImpl(const MetaData& data, const std::string& what, const Severity severity);
+  virtual std::string formatMissingImpl(const MetaData& data, const std::string& what, const Severity severity);
   virtual std::string formatConfigImpl(const MetaData& data);
   virtual std::string formatConfigsImpl(const std::vector<MetaData>& data);
 
  private:
+  std::string getUnspecifiedString() const;
   inline static Formatter::Ptr formatter_ = std::make_shared<Formatter>();
 };
 

config_utilities/include/config_utilities/internal/meta_data.h

@@ -63,6 +63,9 @@ struct FieldInfo {
 
   // Whether the field corresponds to its default value. Only queried if Settings().indicate_default_values is true.
   bool is_default = false;
+
+  // Whether or not the field was parsed
+  bool was_parsed = false;
 };
 
 // Struct to issue warnings. Currently used for parsing errors but can be extended to other warnings in the future.
@@ -127,6 +130,9 @@ struct MetaData {
   // Utility to look up if there's any error messages in the data or its sub-configs.
   bool hasErrors() const;
 
+  // Utility to look up if any fields were not parsed
+  bool hasMissing() const;
+
   // Utility function so not every class needs to write their own recursion.
   void performOnAll(const std::function<void(MetaData&)>& func);
   void performOnAll(const std::function<void(const MetaData&)>& func) const;

config_utilities/include/config_utilities/internal/visitor.h

@@ -65,7 +65,8 @@ struct Visitor {
                             const YAML::Node& node,
                             const bool print_warnings = true,
                             const std::string& name_space = "",
-                            const std::string& field_name = "");
+                            const std::string& field_name = "",
+                            const bool print_missing = true);
 
   // Get the data stored in the config.
   template <typename ConfigT>

config_utilities/include/config_utilities/internal/visitor_impl.hpp

@@ -60,13 +60,19 @@ MetaData Visitor::setValues(ConfigT& config,
                             const YAML::Node& node,
                             const bool print_warnings,
                             const std::string& name_space,
-                            const std::string& field_name) {
+                            const std::string& field_name,
+                            const bool print_missing) {
   Visitor visitor(Mode::kSet, name_space, field_name);
   visitor.data.data.reset(node);
   ::config::declare_config(config);
   if (print_warnings && visitor.data.hasErrors()) {
     Logger::logWarning(Formatter::formatErrors(visitor.data, "Errors parsing config", Severity::kWarning));
   }
+
+  if (print_missing && Settings::instance().print_missing && visitor.data.hasMissing()) {
+    Logger::logWarning(Formatter::formatMissing(visitor.data, "Missing fields from config", Severity::kWarning));
+  }
+
   return visitor.data;
 }
 
@@ -110,7 +116,7 @@ MetaData Visitor::subVisit(ConfigT& config,
       data = getValues(config, print_warnings, name_space, field_name);
       break;
     case Visitor::Mode::kSet:
-      data = setValues(config, current_visitor.data.data, print_warnings, name_space, field_name);
+      data = setValues(config, current_visitor.data.data, print_warnings, name_space, field_name, false);
       break;
     case Visitor::Mode::kCheck:
       data = getChecks(config, field_name);
@@ -124,24 +130,28 @@ MetaData Visitor::subVisit(ConfigT& config,
 // Visit a non-config field.
 template <typename T, typename std::enable_if<!isConfig<T>(), bool>::type>
 void Visitor::visitField(T& field, const std::string& field_name, const std::string& unit) {
-  Visitor& visitor = Visitor::instance();
+  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;
-    YamlParser::fromYaml(visitor.data.data, field_name, field, visitor.name_space, error);
+    info.was_parsed = YamlParser::fromYaml(visitor.data.data, field_name, field, visitor.name_space, error);
     if (!error.empty()) {
       visitor.data.errors.emplace_back(new Warning(field_name, error));
     }
   }
 
   if (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetDefaults) {
-    FieldInfo& info = visitor.data.field_infos.emplace_back();
     std::string error;
     YAML::Node node = YamlParser::toYaml(field_name, field, 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));
-    info.name = field_name;
-    info.unit = unit;
     if (!error.empty()) {
       visitor.data.errors.emplace_back(new Warning(field_name, error));
     }
@@ -151,13 +161,20 @@ 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>
 void Visitor::visitField(T& field, const std::string& field_name, const std::string& unit) {
-  Visitor& visitor = Visitor::instance();
+  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.
 
-    YamlParser::fromYaml(visitor.data.data, field_name, intermediate, visitor.name_space, error);
+    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();
@@ -170,7 +187,6 @@ void Visitor::visitField(T& field, const std::string& field_name, const std::str
   }
 
   if (visitor.mode == Visitor::Mode::kGet || visitor.mode == Visitor::Mode::kGetDefaults) {
-    FieldInfo& info = visitor.data.field_infos.emplace_back();
     std::string error;
     const auto intermediate = Conversion::toIntermediate(field, error);
     if (!error.empty()) {
@@ -181,8 +197,6 @@ void Visitor::visitField(T& field, const std::string& field_name, const std::str
     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));
-    info.name = field_name;
-    info.unit = unit;
     if (!error.empty()) {
       visitor.data.errors.emplace_back(new Warning(field_name, error));
     }
@@ -225,7 +239,7 @@ void Visitor::visitField(std::vector<ConfigT>& config, const std::string& field_
     size_t index = 0;
     for (const auto& node : nodes) {
       ConfigT& sub_config = config.emplace_back();
-      visitor.data.sub_configs.emplace_back(setValues(sub_config, node, false, "", field_name));
+      visitor.data.sub_configs.emplace_back(setValues(sub_config, node, false, "", field_name, false));
       visitor.data.sub_configs.back().array_config_index = index++;
     }
   }
@@ -269,7 +283,7 @@ void Visitor::visitField(std::map<K, ConfigT>& config, const std::string& field_
     for (auto&& [key, node] : nodes) {
       auto iter = config.emplace(YAML::Node(key).template as<K>(), ConfigT()).first;
       auto& sub_config = iter->second;
-      visitor.data.sub_configs.emplace_back(setValues(sub_config, node, false, "", field_name));
+      visitor.data.sub_configs.emplace_back(setValues(sub_config, node, false, "", field_name, false));
       visitor.data.sub_configs.back().map_config_key = key;
     }
   }

config_utilities/include/config_utilities/parsing/yaml.h

@@ -62,7 +62,7 @@ namespace config {
 template <typename ConfigT>
 ConfigT fromYaml(const YAML::Node& node, const std::string& name_space = "") {
   ConfigT config;
-  internal::Visitor::setValues(config, internal::lookupNamespace(node, name_space));
+  internal::Visitor::setValues(config, internal::lookupNamespace(node, name_space), true);
   return config;
 }
 
@@ -200,9 +200,9 @@ std::unique_ptr<BaseT> createFromYamlFileWithNamespace(const std::string& file_n
  * @brief Update the config with the values in a YAML node.
  * @note This function will update the field and check the validity of the config afterwards. If the config is invalid,
  * the field will be reset to its original value.
-  * @param config The config to update.
-  * @param node The node containing the field(s) to update.
-  * @param name_space Optionally specify a name space to create the config from. Separate names with slashes '/'.
+ * @param config The config to update.
+ * @param node The node containing the field(s) to update.
+ * @param name_space Optionally specify a name space to create the config from. Separate names with slashes '/'.
  */
 template <typename ConfigT>
 bool updateFromYaml(ConfigT& config, const YAML::Node& node, const std::string& name_space = "") {

config_utilities/include/config_utilities/settings.h

@@ -75,6 +75,9 @@ struct Settings {
   // If true, attempts to print floats and float-like fields with default stream precision
   bool reformat_floats = true;
 
+  // If true, prints fields that had no value present when being parsed
+  bool print_missing = false;
+
   /* Factory settings */
   // The factory will look for this param to deduce the type of the object to be created.
   std::string factory_type_param_name = "type";

config_utilities/src/asl_formatter.cpp

@@ -94,6 +94,65 @@ std::string AslFormatter::formatErrorsRecursive(const MetaData& data, const std:
   return result;
 }
 
+std::string AslFormatter::formatMissingImpl(const MetaData& data, const std::string& what, const Severity severity) {
+  const std::string sev = severityToString(severity) + ": ";
+  const size_t print_width = Settings::instance().print_width;
+  is_first_divider_ = true;
+  name_prefix_ = "";
+
+  // Header line.
+  std::string result = what + " '" + resolveConfigName(data) + "':\n" +
+                       internal::printCenter(resolveConfigName(data), print_width, '=') + "\n";
+
+  // Format all checks and errors.
+  result += formatMissingRecursive(data, sev, print_width);
+  return result + std::string(print_width, '=');
+}
+
+std::string AslFormatter::formatMissingRecursive(const MetaData& data, const std::string& sev, const size_t length) {
+  const std::string name_prefix_before = name_prefix_;
+  if (Settings::instance().inline_subconfig_field_names) {
+    if (!data.field_name.empty()) {
+      // TOOD(nathan) refactor to put in metadata
+      name_prefix_ += data.field_name;
+      if (data.array_config_index >= 0) {
+        name_prefix_ += "[" + std::to_string(data.array_config_index) + "]";
+      } else if (data.map_config_key) {
+        name_prefix_ += "[" + *data.map_config_key + "]";
+      }
+      name_prefix_ += ".";
+    }
+  }
+
+  std::string result;
+  for (const auto& field_info : data.field_infos) {
+    if (field_info.was_parsed) {
+      continue;
+    }
+
+    const std::string rendered_name = "'" + name_prefix_ + field_info.name + "'";
+    const auto msg = sev + "Missing field " + rendered_name + "!";
+    result.append(wrapString(msg, length, sev.length(), false) + "\n");
+  }
+
+  // Add more dividers if necessary.
+  if (!Settings::instance().inline_subconfig_field_names && !result.empty()) {
+    if (is_first_divider_) {
+      is_first_divider_ = false;
+    } else {
+      result = internal::printCenter(resolveConfigName(data), Settings::instance().print_width, '-') + "\n" + result;
+    }
+  }
+
+  for (const auto& sub_data : data.sub_configs) {
+    // TODO(nathan) think about indenting and actually showing subconfig structure
+    result += formatMissingRecursive(sub_data, sev, length);
+  }
+
+  name_prefix_ = name_prefix_before;
+  return result;
+}
+
 std::string AslFormatter::formatChecksInternal(const MetaData& data, const std::string& sev, const size_t length) {
   std::string result;
   for (const auto& check : data.checks) {

config_utilities/src/formatter.cpp

@@ -41,6 +41,10 @@ std::string Formatter::formatErrors(const MetaData& data, const std::string& wha
   return formatter_->formatErrorsImpl(data, what, severity);
 }
 
+std::string Formatter::formatMissing(const MetaData& data, const std::string& what, const Severity severity) {
+  return formatter_->formatMissingImpl(data, what, severity);
+}
+
 std::string Formatter::formatConfig(const MetaData& data) { return formatter_->formatConfigImpl(data); }
 
 std::string Formatter::formatConfigs(const std::vector<MetaData>& data) { return formatter_->formatConfigsImpl(data); }
@@ -52,14 +56,18 @@ void Formatter::setFormatter(Formatter::Ptr formatter) {
 }
 
 std::string Formatter::formatErrorsImpl(const MetaData& data, const std::string& what, const Severity severity) {
-  return "No format specified. Specify a format by including one of 'config_utilities/formatters/<preferred_style>.h'.";
+  return getUnspecifiedString();
 }
 
-std::string Formatter::formatConfigImpl(const MetaData& data) {
-  return "No format specified. Specify a format by including one of 'config_utilities/formatters/<preferred_style>.h'.";
+std::string Formatter::formatMissingImpl(const MetaData& data, const std::string& what, const Severity severity) {
+  return getUnspecifiedString();
 }
 
-std::string Formatter::formatConfigsImpl(const std::vector<MetaData>& data) {
+std::string Formatter::formatConfigImpl(const MetaData& data) { return getUnspecifiedString(); }
+
+std::string Formatter::formatConfigsImpl(const std::vector<MetaData>& data) { return getUnspecifiedString(); }
+
+std::string Formatter::getUnspecifiedString() const {
   return "No format specified. Specify a format by including one of 'config_utilities/formatters/<preferred_style>.h'.";
 }
 

config_utilities/src/meta_data.cpp

@@ -41,14 +41,32 @@ bool MetaData::hasErrors() const {
   if (!errors.empty()) {
     return true;
   }
-  for (const MetaData& sub_config : sub_configs) {
+  for (const auto& sub_config : sub_configs) {
     if (sub_config.hasErrors()) {
       return true;
     }
   }
   return false;
 }
 
+bool MetaData::hasMissing() const {
+  // first check all current fields
+  for (const auto& field_info : field_infos) {
+    if (!field_info.was_parsed) {
+      return true;
+    }
+  }
+
+  // next check all subconfigs recursively
+  for (const auto& sub_config : sub_configs) {
+    if (sub_config.hasMissing()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 void MetaData::performOnAll(const std::function<void(MetaData&)>& func) {
   func(*this);
   for (MetaData& sub_config : sub_configs) {

config_utilities/test/CMakeLists.txt

@@ -14,6 +14,7 @@ add_executable(
   tests/enums.cpp
   tests/factory.cpp
   tests/inheritance.cpp
+  tests/missing_fields.cpp
   tests/namespacing.cpp
   tests/path.cpp
   tests/string_utils.cpp