[chore] add a workflow for cppcheck and fix all cppcheck issues

Suppress a few errors


[chore] Pass std::string by const-ref: deal with the case where it's the first argument only

```
exts = [".hpp", "_Impl.hpp", ".cpp"]
found_diffs = {}
for file in files:
    found_diffs[file] = {}
    for ext in exts:
        try:
            with open(f"../{file}{ext}", 'r') as f:
                content = f.read()
        except FileNotFoundError:
            continue
        lines = content.splitlines()
        new_lines = []
        has_diff = False
        
        for line in lines:
            if '(std::string ' in line:
                if not ext in found_diffs[file]:
                    found_diffs[file][ext] = 1
                else:
                    found_diffs[file][ext] += 1
                print(f"{file}{ext}, {line}")
                has_diff = True
                new_lines.append(line.replace('std::string', 'const std::string&'))
            else:
                new_lines.append(line)
        if has_diff:
            with open(f"../{file}{ext}", 'w') as f:
                f.write("\n".join(new_lines) + "\n")
```

Manually fix the rest of `passedByValue`cppcheck


Correct a funcArgNamesDifferent and a typo in arg in Curves

in french, independant... not independent like in enligsh, so pardon my french!
Fix the funcArgNamesDifferent


ScheduleTypeLimits: cannot pass const std::string& since string is mutated.


Fix a lot more warnings, mostly missingOverride


Fix a invalidPrintfArgType_sint warning by using fmt instead of sprintf (faster anyways)


Fix a few more


add a few more global suppressions


False positives for Intersection: `[src/utilities/geometry/Intersection.cpp:158]:(warning),[constStatement],Found suspicious operator '>'`


Fix a few more things


Fix 60 cppcheck warnings in RoofGeometry_Details.hpp


Fix isomodel noConstructor


Fix last warnings in isomodel


delete the copy/assignment operators for OutFiles instread of runtime error 


suppress something that was never implemented


TODO: review please : Ignore a warning about RubyInterpreter::evalString that could be static


Fix a few more warnings in ThreeJS


cppcheck seems to be chocking on the gtest macros for these core tests, so ignore


Not sure if it's really a problem, please double check

`Using iterator to local container 'ws' that may be invalid.`
I'm specifically excluding the nano folder from cppcheck (`-i src/nano`) yet cppcheck still reports a warning, so excluding it


Ignore some false warnings about const


Fix more


More, almost there!


Minor changes


A bit more, almooooost


Almost final tweaks, I have functional changes to Date::Date(std::string) to make


clang-format touched files



Update gitignore


Suppress last one


Tweak cppcheck: print progress to screen, and output problems to the file.

3>&1 1>&2 2>&3 is how you swap stderr and stdout, because tee can only accept stdout.

.github/workflows/cppcheck.yml

@@ -37,12 +37,15 @@ jobs:
             --inline-suppr \
             --inconclusive \
             --template='[{file}:{line}]:({severity}),[{id}],{message}' \
-            --force -q -j $(nproc) \
+            -j $(nproc) \
+            --force \
             -i src/cli/test \
             -i src/airflow/contam \
             -i src/polypartition \
             -i src/nano \
-            ./src 2>&1 | tee cppcheck.txt
+            ./src \
+            3>&1 1>&2 2>&3 | tee cppcheck.txt
+
 
     - name: Parse and colorize cppcheck
       shell: bash

.gitignore

@@ -15,3 +15,5 @@ developer/msvc/Visualizers/all_concat.natvis
 *.sublime-projet
 *.cache
 .clangd/
+cppcheck.txt*
+clang_format.patch

ci/colorize_cppcheck_results.py

@@ -0,0 +1,148 @@
+import re
+from collections import Counter
+
+
+def colorize(lines):
+    def bold(s):
+        return '\x1b[1m{}\x1b[0m'.format(s)
+
+    def red(s):
+        return '\x1b[31m{}\x1b[0m'.format(s)
+
+    def green(s):
+        return '\x1b[32m{}\x1b[0m'.format(s)
+
+    def yellow(s):
+        return '\x1b[33m{}\x1b[0m'.format(s)
+
+    def blue(s):
+        return '\x1b[34m{}\x1b[0m'.format(s)
+
+    def magenta(s):  # purple
+        return '\x1b[35m{}\x1b[0m'.format(s)
+
+    def cyan(s):
+        return '\x1b[36m{}\x1b[0m'.format(s)
+
+    def format_severity(txt, severity):
+        """
+        http://cppcheck.sourceforge.net/devinfo/doxyoutput/classSeverity.html
+        enum:
+            none, error, warning, style, performance,
+            portability, information, debug
+        """
+        if severity == "none":
+            return txt
+        if severity == "error":
+            return red(txt)
+        if severity == "warning":
+            return yellow(txt)
+        if severity == 'style':
+            return blue(txt)
+        if severity == "performance":
+            return cyan(txt)
+        if severity == "portability":
+            return magenta(txt)
+        if severity == "information":
+            return green(txt)
+        if severity == "debug":
+            return txt
+
+        return txt
+
+    re_message = re.compile(r'\[(?P<file>.*):(?P<line>.*?)\]:'
+                            r'\((?P<severity>.*?)\),\[(?P<id>.*?)\],'
+                            r'(?P<message>.*)')
+
+    colored_lines = []
+    matched_messages = []
+
+    colored_lines = []
+    matched_messages = []
+
+    for line in lines:
+        m = re_message.match(line)
+        if m:
+            d = m.groupdict()
+            matched_messages.append(d)
+        else:
+            colored_lines.append(red(line))
+
+    severity_order = ['error', 'warning', 'performance', 'portability',
+                      'style', 'information', 'debug', 'none']
+
+    counter = Counter(d['severity'] for d in matched_messages)
+    summary_line = "\n\n==========================================\n"
+    summary_line += "             {}:\n".format(bold(red("CPPCHECK Summary")))
+    summary_line += "------------------------------------------"
+
+    for severity in severity_order:
+        n_severity = counter[severity]
+        summary_line += "\n * "
+        if n_severity:
+            summary_line += format_severity(n_severity, severity)
+        else:
+            # summary_line += green("No {}(s)".format(severity))
+            summary_line += green("No")
+        summary_line += " {}(s)".format(format_severity(severity, severity))
+
+    summary_line += "\n==========================================\n\n"
+
+    n_errors = counter['error']
+    # if n_errors:
+    #     summary_line += red("{} Errors".format(n_errors))
+    # else:
+    #     summary_line = green("No Errors")
+
+    n_warnings = counter['warning']
+    # if n_warnings:
+    #     summary_line += yellow("{} Warnings".format(n_warnings))
+    # else:
+    #     summary_line = green("No Warnings")
+
+    n_styles = counter['style']
+    n_performances = counter['performance']
+    n_portabilities = counter['portability']
+    # n_informations = counter['information']
+
+    # n_debugs = counter['debug']
+
+    # Start by sorting by filename
+    matched_messages.sort(key=lambda d: d['file'])
+    matched_messages.sort(key=lambda d: severity_order.index(d['severity']))
+
+    # Now sort by the severity we cared about
+    for d in matched_messages:
+
+        f = d['file']
+        line = d['line']
+        severity = d['severity']
+        iid = d['id']
+        message = d['message']
+
+        colored_lines.append(
+            "[{f}:{line}]:({severity}),[{i}],{message}"
+            .format(f=magenta(f),  # format_severity(f, severity),
+                    line=green(line),
+                    severity=format_severity(severity, severity),
+                    i=bold(iid),
+                    message=message))
+
+    return (colored_lines, summary_line, n_errors, n_warnings,
+            n_performances, n_portabilities, n_styles)
+
+
+if __name__ == '__main__':
+    with open('cppcheck.txt', 'r') as f:
+        content = f.read()
+
+    lines = content.splitlines()
+    (colored_lines, summary_line, n_errors, n_warnings,
+     n_performances,  n_portabilities, n_styles) = colorize(lines)
+    print(summary_line)
+    # sys.stdout.writelines(colored_lines)
+    print("\n".join(colored_lines))
+    n_tot = (n_errors + n_warnings + n_performances
+             + n_portabilities + n_styles)
+    if n_tot > 0:
+        exit(1)

src/airflow/contam/PrjReader.hpp

@@ -58,7 +58,7 @@ namespace contam {
 
     void skipSection();
     std::string readSection();
-    int lineNumber() {
+    int lineNumber() const {
       return m_lineNumber;
     }
 

src/cli/RubyInterpreter.hpp

@@ -756,6 +756,8 @@ class RubyInterpreter
 
   // evaluate a ruby statement with no return value. If a ruby exception is raised
   // the description is translated into a C++ exception, which is thrown as an openstudio::RubyException.
+  // TODO: should this be static?
+  // cppcheck-suppress functionStatic
   void evalString(const std::string& t_str) {
 
     VALUE val = rb_str_new2(t_str.c_str());

src/generateiddfactory/GenerateIddFactoryOutFiles.cpp

@@ -55,6 +55,7 @@ std::string IddFactoryOutFile::finalize(const std::string& oldChecksum) {
   // ETH@20111122 Always copy for now. CMake/build process can't yet handle "sometimes generated"
   // files.
   bool copyFile = true;  // (newChecksum != oldChecksum);
+  // cppcheck-suppress knownConditionTrueFalse
   if (copyFile) {
     if (openstudio::filesystem::exists(finalPath)) {
       openstudio::filesystem::remove(finalPath);

src/generateiddfactory/GenerateIddFactoryOutFiles.hpp

@@ -49,13 +49,9 @@ struct IddFactoryOutFile
 
   IddFactoryOutFile(const std::string& filename, const openstudio::path& outPath, const std::string& outFileHeader);
 
-  IddFactoryOutFile(const IddFactoryOutFile& other) {
-    throw std::runtime_error("Cannot copy IddFactoryOutFiles.");
-  }
-
-  IddFactoryOutFile operator=(const IddFactoryOutFile& other) {
-    throw std::runtime_error("Cannot copy IddFactoryOutFiles.");
-  }
+  // Prevent copy and assignment
+  IddFactoryOutFile(const IddFactoryOutFile&) = delete;
+  IddFactoryOutFile operator=(const IddFactoryOutFile&) = delete;
 
   std::string finalize(const std::string& oldChecksum);
 };

src/generateiddfactory/IddFileFactoryData.cpp

@@ -345,6 +345,8 @@ void IddFileFactoryData::parseFile(const path& outPath, const std::string& outFi
                     << std::endl;
         }
         assert(!fieldNames.empty());
+        // cppcheck 1.x chokes on this, 2.3 doesn't, but ignore it in case someone has an older version
+        // cppcheck-suppress containerOutOfBounds
         fieldName = fieldNames.back();
         // strip out numbers and transfer from fieldNames to extensibleFieldNames
         fieldName = boost::regex_replace(fieldName, boost::regex("\\s?[0-9]+"), "");
@@ -402,7 +404,7 @@ IddFileFactoryData::FileNameRemovedObjectsPair IddFileFactoryData::includedFile(
   return m_includedFiles[index];
 }
 
-std::string IddFileFactoryData::m_convertName(const std::string& originalName) const {
+std::string IddFileFactoryData::m_convertName(const std::string& originalName) {
   std::string result(originalName);
   boost::trim(result);
   result = boost::regex_replace(result, boost::regex("^100 ?%"), "All");
@@ -416,7 +418,7 @@ std::string IddFileFactoryData::m_convertName(const std::string& originalName) c
   return result;
 }
 
-std::string IddFileFactoryData::m_readyLineForOutput(const std::string& line) const {
+std::string IddFileFactoryData::m_readyLineForOutput(const std::string& line) {
   std::string result(line);
   result = boost::regex_replace(result, boost::regex("\\\\"), "\\\\\\\\");
   result = boost::regex_replace(result, boost::regex("\""), "\\\\\"");

src/generateiddfactory/IddFileFactoryData.hpp

@@ -73,8 +73,8 @@ class IddFileFactoryData
   std::vector<StringPair> m_objectNames;  // first is cleaned version
   std::vector<FileNameRemovedObjectsPair> m_includedFiles;
 
-  std::string m_convertName(const std::string& originalName) const;
-  std::string m_readyLineForOutput(const std::string& line) const;
+  static std::string m_convertName(const std::string& originalName);
+  static std::string m_readyLineForOutput(const std::string& line);
 };
 
 typedef std::vector<IddFileFactoryData> IddFileFactoryDataVector;

src/isomodel/Building.hpp

@@ -80,13 +80,13 @@ namespace isomodel {
     }
 
    private:
-    double _lightingOccupancySensor;
-    double _constantIllumination;
-    double _electricApplianceHeatGainOccupied;
-    double _electricApplianceHeatGainUnoccupied;
-    double _gasApplianceHeatGainOccupied;
-    double _gasApplianceHeatGainUnoccupied;
-    double _buildingEnergyManagement;
+    double _lightingOccupancySensor = 0;
+    double _constantIllumination = 0;
+    double _electricApplianceHeatGainOccupied = 0;
+    double _electricApplianceHeatGainUnoccupied = 0;
+    double _gasApplianceHeatGainOccupied = 0;
+    double _gasApplianceHeatGainUnoccupied = 0;
+    double _buildingEnergyManagement = 0;
   };
 
 }  // namespace isomodel

src/isomodel/Cooling.hpp

@@ -74,12 +74,12 @@ namespace isomodel {
     }
 
    private:
-    double _temperatureSetPointOccupied;
-    double _temperatureSetPointUnoccupied;
-    double _cOP;
-    double _partialLoadValue;
-    double _hvacLossFactor;
-    double _pumpControlReduction;
+    double _temperatureSetPointOccupied = 0;
+    double _temperatureSetPointUnoccupied = 0;
+    double _cOP = 0;
+    double _partialLoadValue = 0;
+    double _hvacLossFactor = 0;
+    double _pumpControlReduction = 0;
   };
 
 }  // namespace isomodel

src/isomodel/EpwData.cpp

@@ -170,9 +170,9 @@ namespace isomodel {
     return sstream.str();
   }
 
-  void EpwData::loadData(const openstudio::path& fn) {
+  void EpwData::loadData(const openstudio::path& t_path) {
     // Array was fully initialized in constructor
-    std::ifstream myfile(openstudio::toSystemFilename(fn));
+    std::ifstream myfile(openstudio::toSystemFilename(t_path));
     if (myfile.is_open()) {
       size_t i = 0;
       size_t row = 0;
@@ -188,7 +188,7 @@ namespace isomodel {
       }
       myfile.close();
     } else {
-      throw std::runtime_error("Unable to open weather file: " + openstudio::toString(fn));
+      throw std::runtime_error("Unable to open weather file: " + openstudio::toString(t_path));
     }
   }
 }  // namespace isomodel

src/isomodel/Heating.hpp

@@ -104,17 +104,17 @@ namespace isomodel {
     }
 
    private:
-    double _temperatureSetPointOccupied;
-    double _temperatureSetPointUnoccupied;
-    double _hvacLossFactor;
-    double _efficiency;
-    double _energyType;
-    double _pumpControlReduction;
-    double _hotWaterDemand;
-    double _hotWaterDistributionEfficiency;
-    double _hotWaterSystemEfficiency;
-    double _hotWaterEnergyType;
-    double _hotcoldWasteFactor;
+    double _temperatureSetPointOccupied = 0;
+    double _temperatureSetPointUnoccupied = 0;
+    double _hvacLossFactor = 0;
+    double _efficiency = 0;
+    double _energyType = 0;
+    double _pumpControlReduction = 0;
+    double _hotWaterDemand = 0;
+    double _hotWaterDistributionEfficiency = 0;
+    double _hotWaterSystemEfficiency = 0;
+    double _hotWaterEnergyType = 0;
+    double _hotcoldWasteFactor = 0;
   };
 
 }  // namespace isomodel

src/isomodel/Lighting.hpp

@@ -62,10 +62,10 @@ namespace isomodel {
     }
 
    private:
-    double _powerDensityOccupied;
-    double _powerDensityUnoccupied;
-    double _dimmingFraction;
-    double _exteriorEnergy;
+    double _powerDensityOccupied = 0;
+    double _powerDensityUnoccupied = 0;
+    double _dimmingFraction = 0;
+    double _exteriorEnergy = 0;
   };
 
 }  // namespace isomodel

src/isomodel/Location.hpp

@@ -52,7 +52,7 @@ namespace isomodel {
     }
 
    private:
-    double _terrain;
+    double _terrain = 0;
     std::shared_ptr<WeatherData> _weather;
   };
 

src/isomodel/Population.hpp

@@ -80,13 +80,13 @@ namespace isomodel {
     }
 
    private:
-    double _hoursEnd;
-    double _hoursStart;
-    double _daysEnd;
-    double _daysStart;
-    double _densityOccupied;
-    double _densityUnoccupied;
-    double _heatGainPerPerson;
+    double _hoursEnd = 0;
+    double _hoursStart = 0;
+    double _daysEnd = 0;
+    double _daysStart = 0;
+    double _densityOccupied = 0;
+    double _densityUnoccupied = 0;
+    double _heatGainPerPerson = 0;
   };
 
 }  // namespace isomodel