From f49fedb2adbc97fa1e197edc40ed2363566868f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Wed, 20 Sep 2023 14:45:44 +0200 Subject: [PATCH] fixed #11483 (FN unusedFunction for method with inline implementation) (#5457) Co-authored-by: chrchr-github <78114321+chrchr-github@users.noreply.github.com> --- .selfcheck_unused_suppressions | 5 ++--- Makefile | 2 +- cli/cmdlineparser.h | 7 ------- lib/checkunusedfunctions.cpp | 24 ++++++++++++++---------- lib/checkunusedfunctions.h | 5 +++-- lib/library.h | 9 +++++++++ lib/symboldatabase.h | 2 ++ lib/templatesimplifier.cpp | 7 ++----- lib/templatesimplifier.h | 5 +---- lib/token.h | 4 ++++ lib/tokenize.cpp | 3 +-- lib/tokenize.h | 12 ------------ lib/tokenlist.h | 6 +----- lib/utils.h | 1 + test/cli/test-other.py | 25 +++++++++++++++++++++++++ test/helpers.cpp | 1 + test/testcmdlineparser.cpp | 3 --- test/testunusedfunctions.cpp | 20 ++++++++++++++++++++ 18 files changed, 87 insertions(+), 54 deletions(-) diff --git a/.selfcheck_unused_suppressions b/.selfcheck_unused_suppressions index f66bcc905..8e470d2e4 100644 --- a/.selfcheck_unused_suppressions +++ b/.selfcheck_unused_suppressions @@ -1,12 +1,11 @@ # we are not using all methods of their interfaces -unusedFunction:externals/tinyxml2/tinyxml2.cpp -unusedFunction:externals/simplecpp/simplecpp.cpp +unusedFunction:externals/*/* # TODO: fix these # false positive - # 10660 unusedFunction:gui/mainwindow.cpp unusedFunction:gui/resultstree.cpp -unusedFunction:gui/codeeditor.cpp +unusedFunction:gui/codeeditor.* # usage is disabled unusedFunction:lib/symboldatabase.cpp # false positive - #10661 diff --git a/Makefile b/Makefile index edff57e1f..aa170d293 100644 --- a/Makefile +++ b/Makefile @@ -861,7 +861,7 @@ test/testtype.o: test/testtype.cpp lib/check.h lib/checktype.h lib/color.h lib/c test/testuninitvar.o: test/testuninitvar.cpp lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/ctu.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testuninitvar.cpp -test/testunusedfunctions.o: test/testunusedfunctions.cpp lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h +test/testunusedfunctions.o: test/testunusedfunctions.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkunusedfunctions.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testunusedfunctions.cpp test/testunusedprivfunc.o: test/testunusedprivfunc.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkclass.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h diff --git a/cli/cmdlineparser.h b/cli/cmdlineparser.h index 5ebf56a5b..3832b6851 100644 --- a/cli/cmdlineparser.h +++ b/cli/cmdlineparser.h @@ -78,13 +78,6 @@ public: return mPathNames; } - /** - * Return if help is shown to user. - */ - bool getShowHelp() const { - return mShowHelp; - } - /** * Return if we should exit after printing version, help etc. */ diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index fad92637e..f55cca8da 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -83,7 +83,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi const SymbolDatabase* symbolDatabase = tokenizer.getSymbolDatabase(); for (const Scope* scope : symbolDatabase->functionScopes) { const Function* func = scope->function; - if (!func || !func->token || scope->bodyStart->fileIndex() != 0) + if (!func || !func->token) continue; // Don't warn about functions that are marked by __attribute__((constructor)) or __attribute__((destructor)) @@ -100,12 +100,15 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi if (!usage.lineNumber) usage.lineNumber = func->token->linenr(); + usage.fileIndex = func->token->fileIndex(); + const std::string& fileName = tokenizer.list.file(func->token); + // No filename set yet.. if (usage.filename.empty()) { - usage.filename = tokenizer.list.getSourceFilePath(); + usage.filename = fileName; } // Multiple files => filename = "+" - else if (usage.filename != tokenizer.list.getSourceFilePath()) { + else if (usage.filename != fileName) { //func.filename = "+"; usage.usedOtherFile |= usage.usedSameFile; } @@ -142,7 +145,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi mFunctions[markupVarToken->str()].usedOtherFile = true; else if (markupVarToken->next()->str() == "(") { FunctionUsage &func = mFunctions[markupVarToken->str()]; - func.filename = tokenizer.list.getSourceFilePath(); + func.filename = tokenizer.list.getFiles()[markupVarToken->fileIndex()]; if (func.filename.empty() || func.filename == "+") func.usedOtherFile = true; else @@ -256,7 +259,7 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi if (funcname) { const auto baseName = stripTemplateParameters(funcname->str()); FunctionUsage &func = mFunctions[baseName]; - const std::string& called_from_file = tokenizer.list.getSourceFilePath(); + const std::string& called_from_file = tokenizer.list.getFiles()[funcname->fileIndex()]; if (func.filename.empty() || func.filename == "+" || func.filename != called_from_file) func.usedOtherFile = true; @@ -318,7 +321,7 @@ static bool isOperatorFunction(const std::string & funcName) bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) const { - using ErrorParams = std::tuple; + using ErrorParams = std::tuple; std::vector errors; // ensure well-defined order for (std::unordered_map::const_iterator it = mFunctions.cbegin(); it != mFunctions.cend(); ++it) { @@ -333,7 +336,7 @@ bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings std::string filename; if (func.filename != "+") filename = func.filename; - errors.emplace_back(filename, func.lineNumber, it->first); + errors.emplace_back(filename, func.fileIndex, func.lineNumber, it->first); } else if (!func.usedOtherFile) { /** @todo add error message "function is only used in it can be static" */ /* @@ -346,17 +349,18 @@ bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings } std::sort(errors.begin(), errors.end()); for (const auto& e : errors) - unusedFunctionError(errorLogger, std::get<0>(e), std::get<1>(e), std::get<2>(e)); + unusedFunctionError(errorLogger, std::get<0>(e), std::get<1>(e), std::get<2>(e), std::get<3>(e)); return !errors.empty(); } void CheckUnusedFunctions::unusedFunctionError(ErrorLogger * const errorLogger, - const std::string &filename, unsigned int lineNumber, + const std::string &filename, unsigned int fileIndex, unsigned int lineNumber, const std::string &funcname) { std::list locationList; if (!filename.empty()) { locationList.emplace_back(filename, lineNumber); + locationList.back().fileIndex = fileIndex; } const ErrorMessage errmsg(locationList, emptyString, Severity::style, "$symbol:" + funcname + "\nThe function '$symbol' is never used.", "unusedFunction", CWE561, Certainty::normal); @@ -470,7 +474,7 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo if (calls.find(functionName) == calls.end() && !isOperatorFunction(functionName)) { const Location &loc = decl->second; - unusedFunctionError(errorLogger, loc.fileName, loc.lineNumber, functionName); + unusedFunctionError(errorLogger, loc.fileName, /*fileIndex*/ 0, loc.lineNumber, functionName); } } } diff --git a/lib/checkunusedfunctions.h b/lib/checkunusedfunctions.h index dc834f96a..7568d87d6 100644 --- a/lib/checkunusedfunctions.h +++ b/lib/checkunusedfunctions.h @@ -75,7 +75,7 @@ public: private: void getErrorMessages(ErrorLogger *errorLogger, const Settings * /*settings*/) const override { - CheckUnusedFunctions::unusedFunctionError(errorLogger, emptyString, 0, "funcName"); + CheckUnusedFunctions::unusedFunctionError(errorLogger, emptyString, 0, 0, "funcName"); } void runChecks(const Tokenizer & /*tokenizer*/, ErrorLogger * /*errorLogger*/) override {} @@ -84,7 +84,7 @@ private: * Dummy implementation, just to provide error for --errorlist */ static void unusedFunctionError(ErrorLogger * const errorLogger, - const std::string &filename, unsigned int lineNumber, + const std::string &filename, unsigned int fileIndex, unsigned int lineNumber, const std::string &funcname); static std::string myName() { @@ -98,6 +98,7 @@ private: struct CPPCHECKLIB FunctionUsage { std::string filename; unsigned int lineNumber{}; + unsigned int fileIndex{}; bool usedSameFile{}; bool usedOtherFile{}; }; diff --git a/lib/library.h b/lib/library.h index 9c1b060f9..ef0e7b3b8 100644 --- a/lib/library.h +++ b/lib/library.h @@ -107,22 +107,27 @@ public: /** get reallocation id for function */ int getReallocId(const Token *tok, int arg) const; + // TODO: get rid of this /** get allocation info for function by name (deprecated, use other alloc) */ const AllocFunc* getAllocFuncInfo(const char name[]) const { return getAllocDealloc(mAlloc, name); } + // TODO: get rid of this /** get deallocation info for function by name (deprecated, use other alloc) */ const AllocFunc* getDeallocFuncInfo(const char name[]) const { return getAllocDealloc(mDealloc, name); } + // TODO: get rid of this /** get allocation id for function by name (deprecated, use other alloc) */ + // cppcheck-suppress unusedFunction int allocId(const char name[]) const { const AllocFunc* af = getAllocDealloc(mAlloc, name); return af ? af->groupId : 0; } + // TODO: get rid of this /** get deallocation id for function by name (deprecated, use other alloc) */ int deallocId(const char name[]) const { const AllocFunc* af = getAllocDealloc(mDealloc, name); @@ -130,16 +135,19 @@ public: } /** set allocation id for function */ + // cppcheck-suppress unusedFunction - test-only void setalloc(const std::string &functionname, int id, int arg) { mAlloc[functionname].groupId = id; mAlloc[functionname].arg = arg; } + // cppcheck-suppress unusedFunction - test-only void setdealloc(const std::string &functionname, int id, int arg) { mDealloc[functionname].groupId = id; mDealloc[functionname].arg = arg; } + // cppcheck-suppress unusedFunction - test-only void setrealloc(const std::string &functionname, int id, int arg, int reallocArg = 1) { mRealloc[functionname].groupId = id; mRealloc[functionname].arg = arg; @@ -147,6 +155,7 @@ public: } /** add noreturn function setting */ + // cppcheck-suppress unusedFunction - test-only void setnoreturn(const std::string& funcname, bool noreturn) { mNoReturn[funcname] = noreturn ? FalseTrueMaybe::True : FalseTrueMaybe::False; } diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index b620313c7..cb929a653 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -350,6 +350,7 @@ public: * Is variable in a namespace. * @return true if in a namespace, false if not */ + // cppcheck-suppress unusedFunction bool isNamespace() const { return mAccess == AccessControl::Namespace; } @@ -1356,6 +1357,7 @@ public: return const_cast(this->findScope(tok, const_cast(startScope))); } + // cppcheck-suppress unusedFunction bool isVarId(nonneg int varid) const { return varid < mVariableList.size(); } diff --git a/lib/templatesimplifier.cpp b/lib/templatesimplifier.cpp index d2a068d8f..555f2c0f9 100644 --- a/lib/templatesimplifier.cpp +++ b/lib/templatesimplifier.cpp @@ -3724,9 +3724,7 @@ void TemplateSimplifier::printOut(const std::string & text) const } } -void TemplateSimplifier::simplifyTemplates( - const std::time_t maxtime, - bool &codeWithTemplates) +void TemplateSimplifier::simplifyTemplates(const std::time_t maxtime) { // convert "sizeof ..." to "sizeof..." for (Token *tok = mTokenList.front(); tok; tok = tok->next()) { @@ -3796,10 +3794,9 @@ void TemplateSimplifier::simplifyTemplates( mTemplateNamePos.clear(); } - const bool hasTemplates = getTemplateDeclarations(); + getTemplateDeclarations(); if (passCount == 0) { - codeWithTemplates = hasTemplates; mDump.clear(); for (const TokenAndName& t: mTemplateDeclarations) mDump += t.dump(mTokenizer.list.getFiles()); diff --git a/lib/templatesimplifier.h b/lib/templatesimplifier.h index 29b78fb84..997c506e0 100644 --- a/lib/templatesimplifier.h +++ b/lib/templatesimplifier.h @@ -307,11 +307,8 @@ public: /** * Simplify templates * @param maxtime time when the simplification should be stopped - * @param codeWithTemplates output parameter that is set if code contains templates */ - void simplifyTemplates( - const std::time_t maxtime, - bool &codeWithTemplates); + void simplifyTemplates(const std::time_t maxtime); /** * Simplify constant calculations such as "1+2" => "3" diff --git a/lib/token.h b/lib/token.h index e50fbec9a..346364b40 100644 --- a/lib/token.h +++ b/lib/token.h @@ -441,6 +441,7 @@ public: void isSigned(const bool sign) { setFlag(fIsSigned, sign); } + // cppcheck-suppress unusedFunction bool isPointerCompare() const { return getFlag(fIsPointerCompare); } @@ -549,6 +550,7 @@ public: bool getCppcheckAttribute(TokenImpl::CppcheckAttributes::Type type, MathLib::bigint &value) const { return mImpl->getCppcheckAttribute(type, value); } + // cppcheck-suppress unusedFunction bool hasCppcheckAttributes() const { return nullptr != mImpl->mCppcheckAttributes; } @@ -691,6 +693,7 @@ public: setFlag(fIsInitComma, b); } + // cppcheck-suppress unusedFunction bool isBitfield() const { return mImpl->mBits > 0; } @@ -962,6 +965,7 @@ public: options.files = true; return options; } + // cppcheck-suppress unusedFunction static stringifyOptions forDebugVarId() { stringifyOptions options = forDebug(); options.varid = true; diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 141034fb7..018fbda1f 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -4014,8 +4014,7 @@ void Tokenizer::simplifyTemplates() const std::time_t maxTime = mSettings->templateMaxTime > 0 ? std::time(nullptr) + mSettings->templateMaxTime : 0; mTemplateSimplifier->simplifyTemplates( - maxTime, - mCodeWithTemplates); + maxTime); } //--------------------------------------------------------------------------- diff --git a/lib/tokenize.h b/lib/tokenize.h index cea8e5132..9ce946ff7 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -606,12 +606,6 @@ private: bool operatorEnd(const Token * tok) const; public: - - /** Was there templates in the code? */ - bool codeWithTemplates() const { - return mCodeWithTemplates; - } - const SymbolDatabase *getSymbolDatabase() const { return mSymbolDatabase; } @@ -724,12 +718,6 @@ private: /** unnamed count "Unnamed0", "Unnamed1", "Unnamed2", ... */ nonneg int mUnnamedCount{}; - /** - * was there any templates? templates that are "unused" are - * removed from the token list - */ - bool mCodeWithTemplates{}; - /** * TimerResults */ diff --git a/lib/tokenlist.h b/lib/tokenlist.h index a150d36f3..8056b3bd2 100644 --- a/lib/tokenlist.h +++ b/lib/tokenlist.h @@ -46,10 +46,6 @@ public: TokenList(const TokenList &) = delete; TokenList &operator=(const TokenList &) = delete; - void setSettings(const Settings *settings) { - mSettings = settings; - } - /** @return the source file path. e.g. "file.cpp" */ const std::string& getSourceFilePath() const; @@ -205,7 +201,7 @@ private: std::vector mOrigFiles; /** settings */ - const Settings* mSettings{}; + const Settings* const mSettings{}; /** File is known to be C/C++ code */ bool mIsC{}; diff --git a/lib/utils.h b/lib/utils.h index d49af4994..5218ab1c4 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -276,6 +276,7 @@ T strToInt(const std::string& str) * \return size of array * */ template +// cppcheck-suppress unusedFunction - only used in conditional code std::size_t getArrayLength(const T (& /*unused*/)[size]) { return size; diff --git a/test/cli/test-other.py b/test/cli/test-other.py index 52be3f45d..e4641d6e3 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -444,3 +444,28 @@ typedef int MISRA_5_6_VIOLATION; 'Checking {} ...'.format(test_file) ] assert stderr == 'test.cpp:1:1: style: msg [addon1-id]\n\n^\n' + + +# #11483 +def test_unused_function_include(tmpdir): + test_cpp_file = os.path.join(tmpdir, 'test.cpp') + with open(test_cpp_file, 'wt') as f: + f.write(""" + #include "test.h" + """) + + test_h_file = os.path.join(tmpdir, 'test.h') + with open(test_h_file, 'wt') as f: + f.write(""" + class A { + public: + void f() {} + // cppcheck-suppress unusedFunction + void f2() {} + }; + """) + + args = ['--enable=unusedFunction', '--inline-suppr', '--template={file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]', test_cpp_file] + + _, _, stderr = cppcheck(args) + assert stderr == "{}:4:0: style: The function 'f' is never used. [unusedFunction]\n".format(test_h_file) diff --git a/test/helpers.cpp b/test/helpers.cpp index dd832e3f2..2e1e42c0a 100644 --- a/test/helpers.cpp +++ b/test/helpers.cpp @@ -125,6 +125,7 @@ std::string PreprocessorHelper::getcode(Preprocessor &preprocessor, const std::s std::string ret; try { + // TODO: also preserve location information when #include exists - enabling that will fail since #line is treated like a regular token ret = preprocessor.getcode(tokens1, cfg, files, filedata.find("#file") != std::string::npos); } catch (const simplecpp::Output &) { ret.clear(); diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 109f4fa09..714a0ed2a 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -256,7 +256,6 @@ private: REDIRECT; const char * const argv[] = {"cppcheck"}; ASSERT(parser->parseFromArgs(1, argv)); - ASSERT_EQUALS(true, parser->getShowHelp()); ASSERT(startsWith(GET_REDIRECT_OUTPUT, "Cppcheck - A tool for static C/C++ code analysis")); } @@ -264,7 +263,6 @@ private: REDIRECT; const char * const argv[] = {"cppcheck", "-h"}; ASSERT(parser->parseFromArgs(2, argv)); - ASSERT_EQUALS(true, parser->getShowHelp()); ASSERT(startsWith(GET_REDIRECT_OUTPUT, "Cppcheck - A tool for static C/C++ code analysis")); } @@ -272,7 +270,6 @@ private: REDIRECT; const char * const argv[] = {"cppcheck", "--help"}; ASSERT(parser->parseFromArgs(2, argv)); - ASSERT_EQUALS(true, parser->getShowHelp()); ASSERT(startsWith(GET_REDIRECT_OUTPUT, "Cppcheck - A tool for static C/C++ code analysis")); } diff --git a/test/testunusedfunctions.cpp b/test/testunusedfunctions.cpp index 0cccc44cc..f9a07f8f7 100644 --- a/test/testunusedfunctions.cpp +++ b/test/testunusedfunctions.cpp @@ -18,7 +18,9 @@ #include "checkunusedfunctions.h" #include "errortypes.h" +#include "helpers.h" #include "platform.h" +#include "preprocessor.h" #include "settings.h" #include "fixture.h" #include "tokenize.h" @@ -74,6 +76,8 @@ private: TEST_CASE(entrypointsWin); TEST_CASE(entrypointsWinU); TEST_CASE(entrypointsUnix); + + TEST_CASE(includes); } #define check(...) check_(__FILE__, __LINE__, __VA_ARGS__) @@ -673,6 +677,22 @@ private: "int _fini() { }\n", cppcheck::Platform::Type::Native, &s); ASSERT_EQUALS("", errout.str()); } + + // TODO: fails because the location information is not be preserved by PreprocessorHelper::getcode() + void includes() + { + // #11483 + const char inc[] = "class A {\n" + "public:\n" + " void f() {}\n" + "};"; + const char code[] = R"(#include "test.h")"; + ScopedFile header("test.h", inc); + Preprocessor preprocessor(settings, this); + const std::string processed = PreprocessorHelper::getcode(preprocessor, code, "", "test.cpp"); + check(processed.c_str()); + TODO_ASSERT_EQUALS("[test.h:3]: (style) The function 'f' is never used.\n", "[test.cpp:3]: (style) The function 'f' is never used.\n", errout.str()); + } }; REGISTER_TEST(TestUnusedFunctions)