moved global CheckUnusedFunctions instance to CppCheck / cleaned …

…up `CheckUnusedFunctions` interface (#6013)

lib/checkunusedfunctions.cpp

@@ -49,11 +49,6 @@ namespace CTU {
 
 //---------------------------------------------------------------------------
 
-// Register this check class
-namespace {
-    CheckUnusedFunctions instance;
-}
-
 static const CWE CWE561(561U);   // Dead Code
 
 static std::string stripTemplateParameters(const std::string& funcName) {
@@ -68,14 +63,10 @@ static std::string stripTemplateParameters(const std::string& funcName) {
 // FUNCTION USAGE - Check for unused functions etc
 //---------------------------------------------------------------------------
 
-void CheckUnusedFunctions::clear()
+void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Settings &settings)
 {
-    instance.mFunctions.clear();
-    instance.mFunctionCalls.clear();
-}
+    const char * const FileName = tokenizer.list.getFiles().front().c_str();
 
-void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings &settings)
-{
     const bool doMarkup = settings.library.markupFile(FileName);
 
     // Function declarations..
@@ -100,6 +91,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi
             if (!usage.lineNumber)
                 usage.lineNumber = func->token->linenr();
 
+            // TODO: why always overwrite this but not the filename and line?
             usage.fileIndex = func->token->fileIndex();
             const std::string& fileName = tokenizer.list.file(func->token);
 
@@ -319,8 +311,16 @@ static bool isOperatorFunction(const std::string & funcName)
     return std::find(additionalOperators.cbegin(), additionalOperators.cend(), funcName.substr(operatorPrefix.length())) != additionalOperators.cend();
 }
 
-bool CheckUnusedFunctions::check(ErrorLogger& errorLogger, const Settings& settings) const
+#define logChecker(id) \
+    do { \
+        const ErrorMessage errmsg({}, nullptr, Severity::internal, "logChecker", (id), CWE(0U), Certainty::normal); \
+        errorLogger.reportErr(errmsg); \
+    } while (false)
+
+bool CheckUnusedFunctions::check(const Settings& settings, ErrorLogger& errorLogger) const
 {
+    logChecker("CheckUnusedFunctions::check"); // unusedFunction
+
     using ErrorParams = std::tuple<std::string, unsigned int, unsigned int, std::string>;
     std::vector<ErrorParams> errors; // ensure well-defined order
 
@@ -367,23 +367,6 @@ void CheckUnusedFunctions::unusedFunctionError(ErrorLogger& errorLogger,
     errorLogger.reportErr(errmsg);
 }
 
-void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Settings &settings)
-{
-    instance.parseTokens(tokenizer, tokenizer.list.getFiles().front().c_str(), settings);
-}
-
-#define logChecker(id) \
-    do { \
-        const ErrorMessage errmsg({}, nullptr, Severity::internal, "logChecker", (id), CWE(0U), Certainty::normal); \
-        errorLogger.reportErr(errmsg); \
-    } while (false)
-
-bool CheckUnusedFunctions::check(const Settings& settings, ErrorLogger &errorLogger)
-{
-    logChecker("CheckUnusedFunctions::check"); // unusedFunction
-    return instance.check(errorLogger, settings);
-}
-
 CheckUnusedFunctions::FunctionDecl::FunctionDecl(const Function *f)
     : functionName(f->name()), lineNumber(f->token->linenr())
 {}
@@ -475,3 +458,23 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo
         }
     }
 }
+
+void CheckUnusedFunctions::updateFunctionData(const CheckUnusedFunctions& check)
+{
+    for (const auto& entry : check.mFunctions)
+    {
+        FunctionUsage &usage = mFunctions[entry.first];
+        if (!usage.lineNumber)
+            usage.lineNumber = entry.second.lineNumber;
+        // TODO: why always overwrite this but not the filename and line?
+        usage.fileIndex = entry.second.fileIndex;
+        if (usage.filename.empty())
+            usage.filename = entry.second.filename;
+        // cppcheck-suppress bitwiseOnBoolean - TODO: FP
+        usage.usedOtherFile |= entry.second.usedOtherFile;
+        // cppcheck-suppress bitwiseOnBoolean - TODO: FP
+        usage.usedSameFile |= entry.second.usedSameFile;
+    }
+    mFunctionDecl.insert(mFunctionDecl.cend(), check.mFunctionDecl.cbegin(), check.mFunctionDecl.cend());
+    mFunctionCalls.insert(check.mFunctionCalls.cbegin(), check.mFunctionCalls.cend());
+}

lib/checkunusedfunctions.h

@@ -50,9 +50,7 @@ class CPPCHECKLIB CheckUnusedFunctions {
     // Parse current tokens and determine..
     // * Check what functions are used
     // * What functions are declared
-    void parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings &settings);
-
-    static void parseTokens(const Tokenizer &tokenizer, const Settings &settings);
+    void parseTokens(const Tokenizer &tokenizer, const Settings &settings);
 
     std::string analyzerInfo() const;
 
@@ -62,14 +60,12 @@ class CPPCHECKLIB CheckUnusedFunctions {
         unusedFunctionError(errorLogger, emptyString, 0, 0, "funcName");
     }
 
-    static bool check(const Settings& settings, ErrorLogger &errorLogger);
-
-private:
-    static void clear();
-
     // Return true if an error is reported.
-    bool check(ErrorLogger& errorLogger, const Settings& settings) const;
+    bool check(const Settings& settings, ErrorLogger& errorLogger) const;
+
+    void updateFunctionData(const CheckUnusedFunctions& check);
 
+private:
     static void unusedFunctionError(ErrorLogger& errorLogger,
                                     const std::string &filename, unsigned int fileIndex, unsigned int lineNumber,
                                     const std::string &funcname);

lib/cppcheck.cpp

@@ -429,6 +429,9 @@ static bool reportClangErrors(std::istream &is, const std::function<void(const E
 
 unsigned int CppCheck::checkClang(const std::string &path)
 {
+    if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
+        mUnusedFunctionsCheck.reset(new CheckUnusedFunctions());
+
     if (!mSettings.quiet)
         mErrorLogger.reportOut(std::string("Checking ") + path + " ...", Color::FgGreen);
 
@@ -560,6 +563,10 @@ unsigned int CppCheck::check(const std::string &path, const std::string &content
 
 unsigned int CppCheck::check(const FileSettings &fs)
 {
+    // TODO: move to constructor when CppCheck no longer owns the settings
+    if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
+        mUnusedFunctionsCheck.reset(new CheckUnusedFunctions());
+
     CppCheck temp(mErrorLogger, mUseGlobalSuppressions, mExecuteCommand);
     temp.mSettings = mSettings;
     if (!temp.mSettings.userDefines.empty())
@@ -578,10 +585,16 @@ unsigned int CppCheck::check(const FileSettings &fs)
         temp.mSettings.platform.set(fs.platformType);
     if (mSettings.clang) {
         temp.mSettings.includePaths.insert(temp.mSettings.includePaths.end(), fs.systemIncludePaths.cbegin(), fs.systemIncludePaths.cend());
-        return temp.check(Path::simplifyPath(fs.filename));
+        // TODO: propagate back suppressions
+        const unsigned int returnValue = temp.check(Path::simplifyPath(fs.filename));
+        if (mUnusedFunctionsCheck)
+            mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
+        return returnValue;
     }
     const unsigned int returnValue = temp.checkFile(Path::simplifyPath(fs.filename), fs.cfg);
     mSettings.supprs.nomsg.addSuppressions(temp.mSettings.supprs.nomsg.getSuppressions());
+    if (mUnusedFunctionsCheck)
+        mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
     return returnValue;
 }
 
@@ -595,6 +608,10 @@ static simplecpp::TokenList createTokenList(const std::string& filename, std::ve
 
 unsigned int CppCheck::checkFile(const std::string& filename, const std::string &cfgname, std::istream* fileStream)
 {
+    // TODO: move to constructor when CppCheck no longer owns the settings
+    if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
+        mUnusedFunctionsCheck.reset(new CheckUnusedFunctions());
+
     mExitCode = 0;
 
     FilesDeleter filesDeleter;
@@ -633,15 +650,15 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
 
     try {
         if (mSettings.library.markupFile(filename)) {
-            if (mSettings.isUnusedFunctionCheckEnabled() && mSettings.buildDir.empty()) {
+            if (mUnusedFunctionsCheck && mSettings.isUnusedFunctionCheckEnabled() && mSettings.buildDir.empty()) {
                 Tokenizer tokenizer(mSettings, this);
                 if (fileStream)
                     tokenizer.list.createTokens(*fileStream, filename);
                 else {
                     std::ifstream in(filename);
                     tokenizer.list.createTokens(in, filename);
                 }
-                CheckUnusedFunctions::parseTokens(tokenizer, mSettings);
+                mUnusedFunctionsCheck->parseTokens(tokenizer, mSettings);
             }
             return EXIT_SUCCESS;
         }
@@ -1103,10 +1120,10 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer)
     }
 
     if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mSettings.buildDir.empty()) {
-        unusedFunctionsChecker.parseTokens(tokenizer, tokenizer.list.getFiles().front().c_str(), mSettings);
+        unusedFunctionsChecker.parseTokens(tokenizer, mSettings);
     }
-    if (mSettings.isUnusedFunctionCheckEnabled() && mSettings.buildDir.empty()) {
-        CheckUnusedFunctions::parseTokens(tokenizer, mSettings);
+    if (mUnusedFunctionsCheck && mSettings.isUnusedFunctionCheckEnabled() && mSettings.buildDir.empty()) {
+        mUnusedFunctionsCheck->parseTokens(tokenizer, mSettings);
     }
 
     if (mSettings.clang) {
@@ -1789,8 +1806,8 @@ bool CppCheck::analyseWholeProgram()
     for (Check *check : Check::instances())
         errors |= check->analyseWholeProgram(&ctu, mFileInfo, mSettings, *this);  // TODO: ctu
 
-    if (mSettings.checks.isEnabled(Checks::unusedFunction))
-        errors |= CheckUnusedFunctions::check(mSettings, *this);
+    if (mUnusedFunctionsCheck)
+        errors |= mUnusedFunctionsCheck->check(mSettings, *this);
 
     return errors && (mExitCode > 0);
 }
@@ -1856,8 +1873,8 @@ void CppCheck::analyseWholeProgram(const std::string &buildDir, const std::list<
     for (Check *check : Check::instances())
         check->analyseWholeProgram(&ctuFileInfo, fileInfoList, mSettings, *this);
 
-    if (mSettings.checks.isEnabled(Checks::unusedFunction))
-        CheckUnusedFunctions::check(mSettings, *this);
+    if (mUnusedFunctionsCheck)
+        mUnusedFunctionsCheck->check(mSettings, *this);
 
     for (Check::FileInfo *fi : fileInfoList)
         delete fi;

lib/cppcheck.h

@@ -42,6 +42,7 @@
 class Tokenizer;
 enum class SHOWTIME_MODES;
 struct FileSettings;
+class CheckUnusedFunctions;
 
 /// @addtogroup Core
 /// @{
@@ -248,6 +249,8 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
     ExecuteCmdFn mExecuteCommand;
 
     std::ofstream mPlistFile;
+
+    std::unique_ptr<CheckUnusedFunctions> mUnusedFunctionsCheck;
 };
 
 /// @}

test/cli/unusedFunction/3.c

@@ -1,3 +1,3 @@
 void f3_1() {}
 void f3_2() {}
-void f3_3()  {}
+void f3_3() {}

test/cli/unusedFunction/4.c

@@ -0,0 +1,7 @@
+static void f4_0() {}
+
+// cppcheck-suppress unusedFunction
+void f4_1()
+{
+    f4_0();
+}

test/cli/unusedFunction/unusedFunction.cppcheck

@@ -4,5 +4,6 @@
         <dir name="1.c"/>
         <dir name="2.c"/>
         <dir name="3.c"/>
+        <dir name="4.c"/>
     </paths>
 </project>

test/cli/unused_function_test.py

@@ -14,23 +14,17 @@
 # TODO: make this a generic helper function
 def __create_compdb(tmpdir, projpath):
     compile_commands = os.path.join(tmpdir, 'compile_commands.json')
-    j = [
-        {
+    files = []
+    for f in os.listdir(projpath):
+        files.append(f)
+    files.sort()
+    j = []
+    for f in files:
+        j.append({
             'directory': projpath,
-            'file': os.path.join(projpath, '1.c'),
-            'command': 'gcc -c 1.c'
-        },
-        {
-            'directory': projpath,
-            'file': os.path.join(projpath, '2.c'),
-            'command': 'gcc -c 2.c'
-        },
-        {
-            'directory': projpath,
-            'file': os.path.join(projpath, '3.c'),
-            'command': 'gcc -c 3.c'
-        }
-        ]
+            'file': os.path.join(projpath, f),
+            'command': 'gcc -c {}'.format(f)
+        })
     with open(compile_commands, 'wt') as f:
         f.write(json.dumps(j, indent=4))
     return compile_commands
@@ -132,7 +126,8 @@ def test_unused_functions_builddir_j(tmpdir):
     assert stderr.splitlines() == [
         "{}1.c:4:0: style: The function 'f1' is never used. [unusedFunction]".format(__project_dir_sep),
         "{}2.c:4:0: style: The function 'f2' is never used. [unusedFunction]".format(__project_dir_sep),
-        "{}3.c:3:0: style: The function 'f3_3' is never used. [unusedFunction]".format(__project_dir_sep)
+        "{}3.c:3:0: style: The function 'f3_3' is never used. [unusedFunction]".format(__project_dir_sep),
+        "{}4.c:4:0: style: The function 'f4_1' is never used. [unusedFunction]".format(__project_dir_sep)
     ]
     assert ret == 0, stdout
 
@@ -168,7 +163,8 @@ def test_unused_functions_builddir_project_j(tmpdir):
     assert stderr.splitlines() == [
         "{}1.c:4:0: style: The function 'f1' is never used. [unusedFunction]".format(__project_dir_sep),
         "{}2.c:4:0: style: The function 'f2' is never used. [unusedFunction]".format(__project_dir_sep),
-        "{}3.c:3:0: style: The function 'f3_3' is never used. [unusedFunction]".format(__project_dir_sep)
+        "{}3.c:3:0: style: The function 'f3_3' is never used. [unusedFunction]".format(__project_dir_sep),
+        "{}4.c:4:0: style: The function 'f4_1' is never used. [unusedFunction]".format(__project_dir_sep)
     ]
     assert ret == 0, stdout
 
@@ -208,6 +204,7 @@ def test_unused_functions_builddir_compdb_j(tmpdir):
     assert stderr.splitlines() == [
         "{}1.c:4:0: style: The function 'f1' is never used. [unusedFunction]".format(__project_dir_sep),
         "{}2.c:4:0: style: The function 'f2' is never used. [unusedFunction]".format(__project_dir_sep),
-        "{}3.c:3:0: style: The function 'f3_3' is never used. [unusedFunction]".format(__project_dir_sep)
+        "{}3.c:3:0: style: The function 'f3_3' is never used. [unusedFunction]".format(__project_dir_sep),
+        "{}4.c:4:0: style: The function 'f4_1' is never used. [unusedFunction]".format(__project_dir_sep)
     ]
     assert ret == 0, stdout

test/testprocessexecutor.cpp

@@ -69,8 +69,6 @@ class TestProcessExecutorBase : public TestFixture {
     void check(unsigned int jobs, int files, int result, const std::string &data, const CheckOptions& opt = make_default_obj{}) {
         errout.str("");
 
-        CheckUnusedFunctions::clear();
-
         std::list<FileSettings> fileSettings;
 
         std::list<std::pair<std::string, std::size_t>> filelist;

test/testsingleexecutor.cpp

@@ -74,8 +74,6 @@ class TestSingleExecutorBase : public TestFixture {
     void check(int files, int result, const std::string &data, const CheckOptions& opt = make_default_obj{}) {
         errout.str("");
 
-        CheckUnusedFunctions::clear();
-
         std::list<FileSettings> fileSettings;
 
         std::list<std::pair<std::string, std::size_t>> filelist;

test/testsuppressions.cpp

@@ -230,8 +230,6 @@ class TestSuppressions : public TestFixture {
         // Clear the error log
         errout.str("");
 
-        CheckUnusedFunctions::clear();
-
         std::list<FileSettings> fileSettings;
 
         std::list<std::pair<std::string, std::size_t>> filelist;
@@ -284,8 +282,6 @@ class TestSuppressions : public TestFixture {
     unsigned int _checkSuppressionThreads(const char code[], bool useFS, const std::string &suppression = emptyString) {
         errout.str("");
 
-        CheckUnusedFunctions::clear();
-
         std::list<FileSettings> fileSettings;
 
         std::list<std::pair<std::string, std::size_t>> filelist;
@@ -334,8 +330,6 @@ class TestSuppressions : public TestFixture {
     unsigned int _checkSuppressionProcesses(const char code[], bool useFS, const std::string &suppression = emptyString) {
         errout.str("");
 
-        CheckUnusedFunctions::clear();
-
         std::list<FileSettings> fileSettings;
 
         std::list<std::pair<std::string, std::size_t>> filelist;

test/testthreadexecutor.cpp

@@ -69,8 +69,6 @@ class TestThreadExecutorBase : public TestFixture {
     void check(unsigned int jobs, int files, int result, const std::string &data, const CheckOptions& opt = make_default_obj{}) {
         errout.str("");
 
-        CheckUnusedFunctions::clear();
-
         std::list<FileSettings> fileSettings;
 
         std::list<std::pair<std::string, std::size_t>> filelist;