some small cleanups and refactorings (#4488)

* moved `plistFile` from `ErrorLogger` to `CppCheck`

* got rid of global CWE objects

* lib/CMakeLists.txt: suppress some `-Wfloat-equal` clang warning in matchcompiled builds as well

* lib/CMakeLists.txt: moved a loop into proper block

* test/CMakeLists.txt: simplified `add_fixture`

* test/CMakeLists.txt: moved `fixture_cost`

* fixed `naming-privateMemberVariable` selfcheck warning

cli/cmdlineparser.cpp

@@ -37,6 +37,7 @@
 #include <cstdio>
 #include <cstdlib> // EXIT_FAILURE
 #include <cstring>
+#include <fstream>
 #include <iostream>
 #include <list>
 #include <set>

lib/CMakeLists.txt

@@ -22,11 +22,11 @@ function(build_src output filename)
     set_source_files_properties(${outfile} PROPERTIES GENERATED TRUE)
 endfunction()
 
+if (NOT USE_MATCHCOMPILER_OPT STREQUAL "Off")
     foreach(file ${srcs})
         build_src(srcs_build ${file})
     endforeach()
 
-if (NOT USE_MATCHCOMPILER_OPT STREQUAL "Off")
     set(srcs_lib ${srcs_build})
 else()
     set(srcs_lib ${srcs})
@@ -53,7 +53,13 @@ endif()
 
 if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
     # -Wfloat-equal is generated by picojson.h
+    if (NOT USE_MATCHCOMPILER_OPT STREQUAL "Off")
+        set_source_files_properties(mc_cppcheck.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal)
+        set_source_files_properties(mc_importproject.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal)
+        set_source_files_properties(mc_settings.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal)
+    else()
         set_source_files_properties(cppcheck.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal)
         set_source_files_properties(importproject.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal)
         set_source_files_properties(settings.cpp PROPERTIES COMPILE_FLAGS -Wno-float-equal)
     endif()
+endif()

lib/checknullpointer.cpp

@@ -39,6 +39,10 @@
 
 //---------------------------------------------------------------------------
 
+// CWE ids used:
+static const struct CWE CWE_NULL_POINTER_DEREFERENCE(476U);
+static const struct CWE CWE_INCORRECT_CALCULATION(682U);
+
 // Register this check class (by creating a static instance of it)
 namespace {
     CheckNullPointer instance;

lib/checkuninitvar.cpp

@@ -49,6 +49,10 @@ namespace tinyxml2 {
 
 //---------------------------------------------------------------------------
 
+// CWE ids used:
+static const struct CWE CWE_USE_OF_UNINITIALIZED_VARIABLE(457U);
+static const struct CWE CWE_USE_OF_POTENTIALLY_DANGEROUS_FUNCTION(676U);
+
 // Register this check class (by creating a static instance of it)
 namespace {
     CheckUninitVar instance;

lib/checkunusedfunctions.cpp

@@ -34,6 +34,7 @@
 #include <cctype>
 #include <cstdlib>
 #include <cstring>
+#include <fstream> // IWYU pragma: keep
 #include <memory>
 #include <sstream> // IWYU pragma: keep
 #include <utility>

lib/cppcheck.cpp

@@ -45,6 +45,7 @@
 #include <cctype>
 #include <cstdlib>
 #include <exception>
+#include <fstream>
 #include <iostream> // <- TEMPORARY
 #include <memory>
 #include <new>
@@ -365,6 +366,11 @@ CppCheck::~CppCheck()
         mFileInfo.pop_back();
     }
     s_timerResults.showResults(mSettings.showtime);
+
+    if (mPlistFile.is_open()) {
+        mPlistFile << ErrorLogger::plistFooter();
+        mPlistFile.close();
+    }
 }
 
 const char * CppCheck::version()
@@ -606,9 +612,9 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
         }
     }
 
-    if (plistFile.is_open()) {
-        plistFile << ErrorLogger::plistFooter();
-        plistFile.close();
+    if (mPlistFile.is_open()) {
+        mPlistFile << ErrorLogger::plistFooter();
+        mPlistFile.close();
     }
 
     CheckUnusedFunctions checkUnusedFunctions(nullptr, nullptr, nullptr);
@@ -669,8 +675,8 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
                 filename2 = filename;
             std::size_t fileNameHash = std::hash<std::string> {}(filename);
             filename2 = mSettings.plistOutput + filename2.substr(0, filename2.find('.')) + "_" + std::to_string(fileNameHash) + ".plist";
-            plistFile.open(filename2);
-            plistFile << ErrorLogger::plistHeader(version(), files);
+            mPlistFile.open(filename2);
+            mPlistFile << ErrorLogger::plistHeader(version(), files);
         }
 
         std::ostringstream dumpProlog;
@@ -1551,9 +1557,9 @@ void CppCheck::reportErr(const ErrorMessage &msg)
 
     mErrorLogger.reportErr(msg);
     // check if plistOutput should be populated and the current output file is open and the error is not suppressed
-    if (!mSettings.plistOutput.empty() && plistFile.is_open() && !mSettings.nomsg.isSuppressed(errorMessage)) {
+    if (!mSettings.plistOutput.empty() && mPlistFile.is_open() && !mSettings.nomsg.isSuppressed(errorMessage)) {
         // add error to plist output file
-        plistFile << ErrorLogger::plistData(msg);
+        mPlistFile << ErrorLogger::plistData(msg);
     }
 }
 

lib/cppcheck.h

@@ -30,6 +30,7 @@
 #include "settings.h"
 
 #include <cstddef>
+#include <fstream> // IWYU pragma: keep
 #include <functional>
 #include <istream>
 #include <list>
@@ -238,6 +239,8 @@ private:
 
     /** Callback for executing a shell command (exe, args, output) */
     std::function<bool(std::string,std::vector<std::string>,std::string,std::string*)> mExecuteCommand;
+
+    std::ofstream mPlistFile;
 };
 
 /// @}

lib/errorlogger.h

@@ -27,24 +27,11 @@
 #include "color.h"
 
 #include <cstddef>
-#include <fstream>
 #include <list>
 #include <string>
 #include <utility>
 #include <vector>
 
-/**
- * CWE id (Common Weakness Enumeration)
- * See https://cwe.mitre.org/ for further reference.
- * */
-// CWE list: https://cwe.mitre.org/data/published/cwe_v3.4.1.pdf
-static const struct CWE CWE_USE_OF_UNINITIALIZED_VARIABLE(457U);
-static const struct CWE CWE_NULL_POINTER_DEREFERENCE(476U);
-static const struct CWE CWE_USE_OF_POTENTIALLY_DANGEROUS_FUNCTION(676U);
-static const struct CWE CWE_INCORRECT_CALCULATION(682U);
-static const struct CWE CWE_EXPIRED_POINTER_DEREFERENCE(825U);
-
-
 class Token;
 class TokenList;
 
@@ -231,16 +218,9 @@ private:
  * should implement.
  */
 class CPPCHECKLIB ErrorLogger {
-protected:
-    std::ofstream plistFile;
 public:
     ErrorLogger() {}
-    virtual ~ErrorLogger() {
-        if (plistFile.is_open()) {
-            plistFile << ErrorLogger::plistFooter();
-            plistFile.close();
-        }
-    }
+    virtual ~ErrorLogger() {}
 
     /**
      * Information about progress is directed here.

test/CMakeLists.txt

@@ -63,26 +63,10 @@ if (BUILD_TESTS)
         set(SKIP_TESTS "" CACHE STRING "A list of tests to skip")
 
         function(add_fixture NAME)
-            set(options)
-            set(oneValueArgs WORKING_DIRECTORY)
-            set(multiValueArgs)
-
-            cmake_parse_arguments(PARSE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
-
             if (${NAME} IN_LIST SKIP_TESTS)
             elseif(TEST ${NAME})
             else()
-                set(WORKING_DIRECTORY ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})
-                if (PARSE_WORKING_DIRECTORY)
-                    set(WORKING_DIRECTORY ${PARSE_WORKING_DIRECTORY})
-                endif()
-                add_test(NAME ${NAME} COMMAND testrunner ${NAME} WORKING_DIRECTORY ${WORKING_DIRECTORY})
-            endif()
-        endfunction()
-
-        function(fixture_cost NAME COST)
-            if(TEST ${NAME})
-                set_tests_properties(${NAME} PROPERTIES COST ${COST})
+                add_test(NAME ${NAME} COMMAND testrunner ${NAME} WORKING_DIRECTORY ${CMAKE_RUNTIME_OUTPUT_DIRECTORY})
             endif()
         endfunction()
 
@@ -162,6 +146,12 @@ if (BUILD_TESTS)
         add_cfg(windows.cpp INCONCLUSIVE NAME windows64 PLATFORM win64)
         add_cfg(wxwidgets.cpp INCONCLUSIVE)
 
+        function(fixture_cost NAME COST)
+            if(TEST ${NAME})
+                set_tests_properties(${NAME} PROPERTIES COST ${COST})
+            endif()
+        endfunction()
+
         # Set cost of the more expensive tests to help improve parallel scheduling
         # of tests
         fixture_cost(TestIO 20)

test/testutils.h

@@ -27,6 +27,7 @@
 #include "tokenlist.h"
 
 #include <cstdio>
+#include <fstream>
 #include <list>
 #include <sstream> // IWYU pragma: keep
 #include <string>