Fixed error handling during loading of generation configuration

Fixed incorrect error message
Made other error messages more descriptive
Made error handling more consistent
Report all errors at once rather than having to run multiple times to fix each error
Put error checking all in one place
Check for empty values more consistently

Also fixed types of abbreviations to elements in this structure to be const references
Also fixed type of config_path to be std::string

Author: NathanJPhillips

src/java-class-info/generation_configuration.cpp

@@ -2,20 +2,67 @@
 
 #include "generation_configuration.h"
 #include <boost/assign.hpp>
+#include <boost/filesystem.hpp>
 #include <list>
+#include <sstream>
 #include <util/json.h>
 
 generation_configurationt::generation_configurationt(const jsont &json)
 {
   detected_entry_points_path =
     json[json_namest::detected_entry_points_path].value;
+  for(const jsont &element : json[json_namest::pattern_group].array)
+  {
+    pattern_group.emplace_back(element);
+  }
+  if(!pattern_group.empty())
+  {
+    if(detected_entry_points_path.empty())
+    {
+      std::stringstream error;
+      error << "The configuration file contained pattern groups but not a "
+            << json_namest::detected_entry_points_path << " element.";
+      errors.push_back(error.str());
+    }
+    else if(boost::filesystem::is_directory(detected_entry_points_path))
+    {
+      std::stringstream error;
+      error << "The output path '" << detected_entry_points_path
+            << "' references an existing directory.";
+      errors.push_back(error.str());
+    }
+  }
+
   collected_classes_info_path =
     json[json_namest::collected_classes_info_path].value;
-  collected_classes_path = json[json_namest::collected_classes_path].value;
-  di_configuration_path = json[json_namest::di_configuration_path].value;
+  if(collected_classes_info_path.empty())
+  {
+    std::stringstream error;
+    error << "The configuration file did not contain a "
+          << json_namest::collected_classes_info_path << " element.";
+  }
+  else if(boost::filesystem::is_directory(collected_classes_info_path))
+  {
+    std::stringstream error;
+    error << "The output path '" << collected_classes_info_path
+          << "' references an existing directory.";
+  }
 
-  for(const jsont &element : json[json_namest::pattern_group].array)
+  collected_classes_path = json[json_namest::collected_classes_path].value;
+  if(collected_classes_path.empty())
   {
-    pattern_group.emplace_back(element);
+    std::stringstream error;
+    error << "The configuration file did not contain a "
+          << json_namest::collected_classes_path << " element.";
+    errors.push_back(error.str());
+  }
+  else if(!boost::filesystem::is_regular_file(collected_classes_path))
+  {
+    std::stringstream error;
+    error << "The input file '" << collected_classes_path
+          << "' does not exist.";
+    errors.push_back(error.str());
   }
+
+  di_configuration_path = json[json_namest::di_configuration_path].value;
 }

src/java-class-info/generation_configuration.h

@@ -4,6 +4,7 @@
 #define SECURITY_SCANNER_GENERATION_CONFIGURATIONT_H
 
 #include "pattern_group.h"
+#include <optional.h>
 
 /// Object representation of the main configuration file used to drive
 /// java_class-info.
@@ -18,6 +19,13 @@ class generation_configurationt
   std::string detected_entry_points_path;
   std::string di_configuration_path;
 
+  optionalt<std::vector<std::string>> get_parse_errors()
+  {
+    if(errors.empty())
+      return {};
+    return errors;
+  }
+
 private:
   class json_namest
   {
@@ -31,6 +39,8 @@ class generation_configurationt
       "detectedEntryPointsPath";
     static constexpr const char *di_configuration_path = "diConfigurationPath";
   };
+
+  std::vector<std::string> errors;
 };
 
 #endif //SECURITY_SCANNER_GENERATION_CONFIGURATIONT_H

src/java-class-info/java_class_info.cpp

@@ -18,6 +18,7 @@
 #include <util/file_util.h>
 #include <util/irep_ids.h>
 #include <util/parse_options.h>
+#include <util/string_join.h>
 #include <util/ui_message.h>
 #include <utility>
 #include <utils.h>
@@ -304,30 +305,37 @@ int cmdline_optionst::doit()
   }
   if(!cmdline.isset("configuration-path"))
   {
-    msg.error() << "Please set --configuration-path to a valid configuration "
-                   "file."
+    msg.error() << "--configuration-path wasn't provided. "
+                   "Use it to provide a valid configuration file."
                 << messaget::eom;
     return -1;
   }
 
-  boost::filesystem::path config_path = cmdline.get_value("configuration-path");
+  std::string config_path = cmdline.get_value("configuration-path");
+  if(config_path.empty())
+  {
+    msg.error() << "--configuration-path was empty. "
+                   "Use it to provide a valid configuration file."
+                << messaget::eom;
+    return -1;
+  }
   jsont config_json;
-  if(parse_json(config_path.string(), ui_message_handler, config_json))
+  if(parse_json(config_path, ui_message_handler, config_json))
   {
     msg.error() << "ERROR: The input file '" << config_path
                 << "' is not a valid JSON file." << messaget::eom;
     return -3;
   }
 
   generation_configurationt config = generation_configurationt(config_json);
-  const std::string in_json_pathname = config.collected_classes_path;
-  if(!boost::filesystem::is_regular_file(in_json_pathname))
+  optionalt<std::vector<std::string>> parse_errors = config.get_parse_errors();
+  if(parse_errors)
   {
-    msg.error()
-      << "ERROR: The input file '" << in_json_pathname
-      << "' does not exist." << messaget::eom;
+    msg.error() << "ERROR: " << join(*parse_errors, "\n") << messaget::eom;
     return -2;
   }
+
+  const std::string &in_json_pathname = config.collected_classes_path;
   jsont cfg;
   if(parse_json(in_json_pathname, ui_message_handler, cfg))
   {
@@ -345,21 +353,9 @@ int cmdline_optionst::doit()
       << "class files)." << messaget::eom;
     return -4;
   }
-  if(config.collected_classes_info_path.empty())
-  {
-    msg.status() << "ERROR: collected_classes_info_path is empty.";
-    return -5;
-  }
-  const std::string out_json_pathname = config.collected_classes_info_path;
-  if(boost::filesystem::is_directory(out_json_pathname))
-  {
-    msg.error()
-      << "ERROR: The output path '" << out_json_pathname
-      << "' references an existing directory." << messaget::eom;
-    return -6;
-  }
+  const std::string &out_json_pathname = config.collected_classes_info_path;
 
-  const std::string di_out_path = config.di_configuration_path;
+  const std::string &di_out_path = config.di_configuration_path;
   bool enable_di_scan = !di_out_path.empty();
   json_objectt results;
   json_arrayt errors;
@@ -605,21 +601,7 @@ int cmdline_optionst::doit()
 
   if(ep_generator.enabled())
   {
-    const std::string detected_ep_path = config.detected_entry_points_path;
-
-    if(detected_ep_path.empty())
-    {
-      msg.status() << "ERROR: collected_classes_info_path is empty.";
-      return -9;
-    }
-    if(boost::filesystem::is_directory(detected_ep_path))
-    {
-      msg.error()
-        << "ERROR: The output path '" << detected_ep_path
-        << "' references an existing directory." << messaget::eom;
-      return -10;
-    }
-
+    const std::string &detected_ep_path = config.detected_entry_points_path;
     std::ofstream epp_file(detected_ep_path);
     if(!epp_file)
     {