From 2244ecc3f314cc9718dd1c2e4257af64ebcb6adf Mon Sep 17 00:00:00 2001
From: Henry Fredrick Schreiner <henry.fredrick.schreiner@cern.ch>
Date: Fri, 29 Nov 2019 09:37:13 -0500
Subject: [PATCH] Adding and fixing Weffc++

---
 CMakeLists.txt               |  7 +++++
 include/CLI/App.hpp          | 53 +++++++++++++++++++-----------------
 include/CLI/ConfigFwd.hpp    |  8 +++---
 include/CLI/FormatterFwd.hpp |  2 +-
 include/CLI/Option.hpp       | 35 +++++++++++++-----------
 include/CLI/Validators.hpp   |  2 +-
 tests/HelpTest.cpp           |  4 +--
 tests/NewParseTest.cpp       | 13 +++++----
 tests/OptionGroupTest.cpp    | 24 +++++++++-------
 tests/SubcommandTest.cpp     | 24 ++++++++++------
 tests/app_helper.hpp         |  4 +--
 11 files changed, 101 insertions(+), 75 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ad574a21..24b49266 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -55,6 +55,13 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
         endif()
     else()
         target_compile_options(CLI11_warnings INTERFACE -Wall -Wextra -pedantic -Wshadow)
+        if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+            if(NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.9)
+                # GCC 4.8 has a false positive
+                # Other compilers ignore this flag
+                target_compile_options(CLI11_warnings INTERFACE -Weffc++)
+            endif()
+        endif()
         if(CLI11_WARNINGS_AS_ERRORS)
             target_compile_options(CLI11_warnings INTERFACE -Werror)
         endif()
diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 704cd594..0bbc6e78 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -65,10 +65,10 @@ class App {
     ///@{
 
     /// Subcommand name or program name (from parser if name is empty)
-    std::string name_;
+    std::string name_{};
 
     /// Description of the current program/subcommand
-    std::string description_;
+    std::string description_{};
 
     /// If true, allow extra arguments (ie, don't throw an error). INHERITABLE
     bool allow_extras_{false};
@@ -96,32 +96,32 @@ class App {
     bool immediate_callback_{false};
 
     /// This is a function that runs prior to the start of parsing
-    std::function<void(size_t)> pre_parse_callback_;
+    std::function<void(size_t)> pre_parse_callback_{};
 
     /// This is a function that runs when parsing has finished.
-    std::function<void()> parse_complete_callback_;
+    std::function<void()> parse_complete_callback_{};
     /// This is a function that runs when all processing has completed
-    std::function<void()> final_callback_;
+    std::function<void()> final_callback_{};
 
     ///@}
     /// @name Options
     ///@{
 
     /// The default values for options, customizable and changeable INHERITABLE
-    OptionDefaults option_defaults_;
+    OptionDefaults option_defaults_{};
 
     /// The list of options, stored locally
-    std::vector<Option_p> options_;
+    std::vector<Option_p> options_{};
 
     ///@}
     /// @name Help
     ///@{
 
     /// Footer to put after all options in the help output INHERITABLE
-    std::string footer_;
+    std::string footer_{};
 
     /// This is a function that generates a footer to put after all other options in help output
-    std::function<std::string()> footer_callback_;
+    std::function<std::string()> footer_callback_{};
 
     /// A pointer to the help flag if there is one INHERITABLE
     Option *help_ptr_{nullptr};
@@ -133,7 +133,7 @@ class App {
     std::shared_ptr<FormatterBase> formatter_{new Formatter()};
 
     /// The error message printing function INHERITABLE
-    std::function<std::string(const App *, const Error &e)> failure_message_ = FailureMessage::simple;
+    std::function<std::string(const App *, const Error &e)> failure_message_{FailureMessage::simple};
 
     ///@}
     /// @name Parsing
@@ -144,35 +144,35 @@ class App {
     /// Pair of classifier, string for missing options. (extra detail is removed on returning from parse)
     ///
     /// This is faster and cleaner than storing just a list of strings and reparsing. This may contain the -- separator.
-    missing_t missing_;
+    missing_t missing_{};
 
     /// This is a list of pointers to options with the original parse order
-    std::vector<Option *> parse_order_;
+    std::vector<Option *> parse_order_{};
 
     /// This is a list of the subcommands collected, in order
-    std::vector<App *> parsed_subcommands_;
+    std::vector<App *> parsed_subcommands_{};
 
     /// this is a list of subcommands that are exclusionary to this one
-    std::set<App *> exclude_subcommands_;
+    std::set<App *> exclude_subcommands_{};
 
     /// This is a list of options which are exclusionary to this App, if the options were used this subcommand should
     /// not be
-    std::set<Option *> exclude_options_;
+    std::set<Option *> exclude_options_{};
 
     /// this is a list of subcommands or option groups that are required by this one, the list is not mutual,  the
     /// listed subcommands do not require this one
-    std::set<App *> need_subcommands_;
+    std::set<App *> need_subcommands_{};
 
     /// This is a list of options which are required by this app, the list is not mutual, listed options do not need the
     /// subcommand not be
-    std::set<Option *> need_options_;
+    std::set<Option *> need_options_{};
 
     ///@}
     /// @name Subcommands
     ///@{
 
     /// Storage for subcommand list
-    std::vector<App_p> subcommands_;
+    std::vector<App_p> subcommands_{};
 
     /// If true, the program name is not case sensitive INHERITABLE
     bool ignore_case_{false};
@@ -204,32 +204,32 @@ class App {
     App *parent_{nullptr};
 
     /// Counts the number of times this command/subcommand was parsed
-    size_t parsed_ = 0;
+    size_t parsed_{0};
 
     /// Minimum required subcommands (not inheritable!)
-    size_t require_subcommand_min_ = 0;
+    size_t require_subcommand_min_{0};
 
     /// Max number of subcommands allowed (parsing stops after this number). 0 is unlimited INHERITABLE
-    size_t require_subcommand_max_ = 0;
+    size_t require_subcommand_max_{0};
 
     /// Minimum required options (not inheritable!)
-    size_t require_option_min_ = 0;
+    size_t require_option_min_{0};
 
     /// Max number of options allowed. 0 is unlimited (not inheritable)
-    size_t require_option_max_ = 0;
+    size_t require_option_max_{0};
 
     /// The group membership INHERITABLE
     std::string group_{"Subcommands"};
 
     /// Alias names for the subcommand
-    std::vector<std::string> aliases_;
+    std::vector<std::string> aliases_{};
 
     ///@}
     /// @name Config
     ///@{
 
     /// The name of the connected config file
-    std::string config_name_;
+    std::string config_name_{};
 
     /// True if ini is required (throws if not present), if false simply keep going.
     bool config_required_{false};
@@ -285,6 +285,9 @@ class App {
         set_help_flag("-h,--help", "Print this help message and exit");
     }
 
+    App(const App &) = delete;
+    App &operator=(const App &) = delete;
+
     /// virtual destructor
     virtual ~App() = default;
 
diff --git a/include/CLI/ConfigFwd.hpp b/include/CLI/ConfigFwd.hpp
index ba009ba6..d8f041b9 100644
--- a/include/CLI/ConfigFwd.hpp
+++ b/include/CLI/ConfigFwd.hpp
@@ -42,13 +42,13 @@ inline std::string ini_join(std::vector<std::string> args) {
 /// Holds values to load into Options
 struct ConfigItem {
     /// This is the list of parents
-    std::vector<std::string> parents;
+    std::vector<std::string> parents{};
 
     /// This is the name
-    std::string name;
+    std::string name{};
 
     /// Listing of inputs
-    std::vector<std::string> inputs;
+    std::vector<std::string> inputs{};
 
     /// The list of parents and name joined by "."
     std::string fullname() const {
@@ -61,7 +61,7 @@ struct ConfigItem {
 /// This class provides a converter for configuration files.
 class Config {
   protected:
-    std::vector<ConfigItem> items;
+    std::vector<ConfigItem> items{};
 
   public:
     /// Convert an app into a configuration
diff --git a/include/CLI/FormatterFwd.hpp b/include/CLI/FormatterFwd.hpp
index 4a398245..b3f04a45 100644
--- a/include/CLI/FormatterFwd.hpp
+++ b/include/CLI/FormatterFwd.hpp
@@ -39,7 +39,7 @@ class FormatterBase {
 
     /// @brief The required help printout labels (user changeable)
     /// Values are Needs, Excludes, etc.
-    std::map<std::string, std::string> labels_;
+    std::map<std::string, std::string> labels_{};
 
     ///@}
     /// @name Basic
diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
index 4a3871b8..52d5c167 100644
--- a/include/CLI/Option.hpp
+++ b/include/CLI/Option.hpp
@@ -233,33 +233,33 @@ class Option : public OptionBase<Option> {
     ///@{
 
     /// A list of the short names (`-a`) without the leading dashes
-    std::vector<std::string> snames_;
+    std::vector<std::string> snames_{};
 
     /// A list of the long names (`--long`) without the leading dashes
-    std::vector<std::string> lnames_;
+    std::vector<std::string> lnames_{};
 
     /// A list of the flag names with the appropriate default value, the first part of the pair should be duplicates of
     /// what is in snames or lnames but will trigger a particular response on a flag
-    std::vector<std::pair<std::string, std::string>> default_flag_values_;
+    std::vector<std::pair<std::string, std::string>> default_flag_values_{};
 
     /// a list of flag names with specified default values;
-    std::vector<std::string> fnames_;
+    std::vector<std::string> fnames_{};
 
     /// A positional name
-    std::string pname_;
+    std::string pname_{};
 
     /// If given, check the environment for this option
-    std::string envname_;
+    std::string envname_{};
 
     ///@}
     /// @name Help
     ///@{
 
     /// The description for help strings
-    std::string description_;
+    std::string description_{};
 
     /// A human readable default value, either manually set, captured, or captured by default
-    std::string default_str_;
+    std::string default_str_{};
 
     /// A human readable type value, set when App creates this
     ///
@@ -267,7 +267,7 @@ class Option : public OptionBase<Option> {
     std::function<std::string()> type_name_{[]() { return std::string(); }};
 
     /// Run this function to capture a default (ignore if empty)
-    std::function<std::string()> default_function_;
+    std::function<std::string()> default_function_{};
 
     ///@}
     /// @name Configuration
@@ -285,32 +285,32 @@ class Option : public OptionBase<Option> {
     int expected_max_{1};
 
     /// A list of Validators to run on each value parsed
-    std::vector<Validator> validators_;
+    std::vector<Validator> validators_{};
 
     /// A list of options that are required with this option
-    std::set<Option *> needs_;
+    std::set<Option *> needs_{};
 
     /// A list of options that are excluded with this option
-    std::set<Option *> excludes_;
+    std::set<Option *> excludes_{};
 
     ///@}
     /// @name Other
     ///@{
 
     /// Remember the parent app
-    App *parent_;
+    App *parent_{nullptr};
 
     /// Options store a callback to do all the work
-    callback_t callback_;
+    callback_t callback_{};
 
     ///@}
     /// @name Parsing results
     ///@{
 
     /// complete Results of parsing
-    results_t results_;
+    results_t results_{};
     /// results after reduction
-    results_t proc_results_;
+    results_t proc_results_{};
     /// enumeration for the option state machine
     enum class option_state {
         parsing = 0,      //!< The option is currently collecting parsed results
@@ -336,6 +336,9 @@ class Option : public OptionBase<Option> {
     /// @name Basic
     ///@{
 
+    Option(const Option &) = delete;
+    Option &operator=(const Option &) = delete;
+
     /// Count the total number of times an option was passed
     size_t count() const { return results_.size(); }
 
diff --git a/include/CLI/Validators.hpp b/include/CLI/Validators.hpp
index a786ff6c..51518057 100644
--- a/include/CLI/Validators.hpp
+++ b/include/CLI/Validators.hpp
@@ -58,7 +58,7 @@ class Validator {
     /// Returns a string error message if validation fails.
     std::function<std::string(std::string &)> func_{[](std::string &) { return std::string{}; }};
     /// The name for search purposes of the Validator
-    std::string name_;
+    std::string name_{};
     /// A Validator will only apply to an indexed value (-1 is all elements)
     int application_index_ = -1;
     /// Enable for Validator to allow it to be disabled if need be
diff --git a/tests/HelpTest.cpp b/tests/HelpTest.cpp
index 9be442ef..246362ba 100644
--- a/tests/HelpTest.cpp
+++ b/tests/HelpTest.cpp
@@ -570,8 +570,8 @@ TEST(Exit, ExitCodes) {
 
 struct CapturedHelp : public ::testing::Test {
     CLI::App app{"My Test Program"};
-    std::stringstream out;
-    std::stringstream err;
+    std::stringstream out{};
+    std::stringstream err{};
 
     int run(const CLI::Error &e) { return app.exit(e, out, err); }
 
diff --git a/tests/NewParseTest.cpp b/tests/NewParseTest.cpp
index 3aea3f0e..92c017b9 100644
--- a/tests/NewParseTest.cpp
+++ b/tests/NewParseTest.cpp
@@ -203,8 +203,8 @@ class spair {
   public:
     spair() = default;
     spair(const std::string &s1, const std::string &s2) : first(s1), second(s2) {}
-    std::string first;
-    std::string second;
+    std::string first{};
+    std::string second{};
 };
 // an example of custom converter that can be used to add new parsing options
 // On MSVC and possibly some other new compilers this can be a free standing function without the template
@@ -352,7 +352,7 @@ template <class X> class objWrapper {
     const X &value() const { return val_; }
 
   private:
-    X val_;
+    X val_{};
 };
 
 // I think there is a bug with the is_assignable in visual studio 2015 it is fixed in later versions
@@ -486,14 +486,17 @@ template <class X> class AobjWrapper {
     // delete all other constructors
     template <class TT> AobjWrapper(TT &&obj) = delete;
     // single assignment operator
-    void operator=(X val) { val_ = val; }
+    AobjWrapper &operator=(X val) {
+        val_ = val;
+        return *this;
+    }
     // delete all other assignment operators
     template <typename TT> void operator=(TT &&obj) = delete;
 
     const X &value() const { return val_; }
 
   private:
-    X val_;
+    X val_{};
 };
 
 static_assert(std::is_assignable<AobjWrapper<uint16_t> &, uint16_t>::value,
diff --git a/tests/OptionGroupTest.cpp b/tests/OptionGroupTest.cpp
index e109c3bf..bea3e452 100644
--- a/tests/OptionGroupTest.cpp
+++ b/tests/OptionGroupTest.cpp
@@ -370,16 +370,20 @@ TEST_F(TApp, InvalidOptions) {
 
 struct ManyGroups : public TApp {
 
-    CLI::Option_group *main;
-    CLI::Option_group *g1;
-    CLI::Option_group *g2;
-    CLI::Option_group *g3;
-    std::string name1;
-    std::string name2;
-    std::string name3;
-    std::string val1;
-    std::string val2;
-    std::string val3;
+    CLI::Option_group *main{nullptr};
+    CLI::Option_group *g1{nullptr};
+    CLI::Option_group *g2{nullptr};
+    CLI::Option_group *g3{nullptr};
+    std::string name1{};
+    std::string name2{};
+    std::string name3{};
+    std::string val1{};
+    std::string val2{};
+    std::string val3{};
+
+    ManyGroups(const ManyGroups &) = delete;
+    ManyGroups &operator=(const ManyGroups &) = delete;
+
     ManyGroups() {
         main = app.add_option_group("main", "the main outer group");
         g1 = main->add_option_group("g1", "group1 description");
diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp
index 336dd924..7923f26d 100644
--- a/tests/SubcommandTest.cpp
+++ b/tests/SubcommandTest.cpp
@@ -809,12 +809,15 @@ TEST_F(TApp, RequiredPosInSubcommand) {
 
 struct SubcommandProgram : public TApp {
 
-    CLI::App *start;
-    CLI::App *stop;
+    CLI::App *start{nullptr};
+    CLI::App *stop{nullptr};
 
-    int dummy;
-    std::string file;
-    int count;
+    int dummy{};
+    std::string file{};
+    int count{};
+
+    SubcommandProgram(const SubcommandProgram &) = delete;
+    SubcommandProgram &operator=(const SubcommandProgram &) = delete;
 
     SubcommandProgram() {
         app.set_help_all_flag("--help-all");
@@ -1092,10 +1095,10 @@ TEST_F(SubcommandProgram, CallbackOrderImmediate) {
 
 struct ManySubcommands : public TApp {
 
-    CLI::App *sub1;
-    CLI::App *sub2;
-    CLI::App *sub3;
-    CLI::App *sub4;
+    CLI::App *sub1{nullptr};
+    CLI::App *sub2{nullptr};
+    CLI::App *sub3{nullptr};
+    CLI::App *sub4{nullptr};
 
     ManySubcommands() {
         app.allow_extras();
@@ -1105,6 +1108,9 @@ struct ManySubcommands : public TApp {
         sub4 = app.add_subcommand("sub4");
         args = {"sub1", "sub2", "sub3"};
     }
+
+    ManySubcommands(const ManySubcommands &) = delete;
+    ManySubcommands &operator=(const ManySubcommands &) = delete;
 };
 
 TEST_F(ManySubcommands, Required1Exact) {
diff --git a/tests/app_helper.hpp b/tests/app_helper.hpp
index 03c162a3..eb8eb2b7 100644
--- a/tests/app_helper.hpp
+++ b/tests/app_helper.hpp
@@ -13,7 +13,7 @@ using input_t = std::vector<std::string>;
 
 struct TApp : public ::testing::Test {
     CLI::App app{"My Test Program"};
-    input_t args;
+    input_t args{};
 
     void run() {
         // It is okay to re-parse - clear is called automatically before a parse.
@@ -24,7 +24,7 @@ struct TApp : public ::testing::Test {
 };
 
 class TempFile {
-    std::string _name;
+    std::string _name{};
 
   public:
     explicit TempFile(std::string name) : _name(std::move(name)) {