From b286e1213ae355b400ae41a69f2f9bb951d79e8b Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 28 Oct 2025 14:53:14 +0100 Subject: [PATCH] fixed #14265 - fixed error location for several preprocessor errors [skip ci] --- cli/cppcheckexecutor.cpp | 2 +- lib/preprocessor.cpp | 79 +++++++++++++++++++++++++-------------- lib/preprocessor.h | 4 +- lib/suppressions.h | 3 ++ test/cli/other_test.py | 13 +++---- test/testsuppressions.cpp | 8 ++-- 6 files changed, 65 insertions(+), 44 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 58659739d79..c5fe5a16110 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -327,7 +327,7 @@ static bool reportUnmatchedSuppressions(const std::list callStack; if (!s.fileName.empty()) { - callStack.emplace_back(s.fileName, s.lineNumber, 0); + callStack.emplace_back(s.fileName, s.lineNumber, 0); // TODO: set column - see #13810 } errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal)); err = true; diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index b315766f8e6..48e1ed8f9ef 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -70,9 +70,8 @@ Preprocessor::Preprocessor(simplecpp::TokenList& tokens, const Settings& setting namespace { struct BadInlineSuppression { - BadInlineSuppression(std::string file, const int line, std::string msg) : file(std::move(file)), line(line), errmsg(std::move(msg)) {} - std::string file; - int line; + BadInlineSuppression(const simplecpp::Location& loc, std::string msg) : location(loc), errmsg(std::move(msg)) {} + simplecpp::Location location; std::string errmsg; }; } @@ -134,10 +133,11 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std: s.isInline = true; s.type = errorType; s.lineNumber = tok->location.line; + s.column = tok->location.col; } if (!errmsg.empty()) - bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg)); + bad.emplace_back(tok->location, std::move(errmsg)); std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) { return !s.errorId.empty(); @@ -152,12 +152,13 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std: s.isInline = true; s.type = errorType; s.lineNumber = tok->location.line; + s.column = tok->location.col; if (!s.errorId.empty()) inlineSuppressions.push_back(std::move(s)); if (!errmsg.empty()) - bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg)); + bad.emplace_back(tok->location, std::move(errmsg)); } return true; @@ -232,6 +233,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett // Add the suppressions. for (SuppressionList::Suppression &suppr : inlineSuppressions) { suppr.fileName = relativeFilename; + suppr.fileIndex = tok->location.fileIndex; if (SuppressionList::Type::blockBegin == suppr.type) { @@ -254,6 +256,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett suppr.lineBegin = supprBegin->lineNumber; suppr.lineEnd = suppr.lineNumber; suppr.lineNumber = supprBegin->lineNumber; + suppr.column = supprBegin->column; suppr.type = SuppressionList::Type::block; inlineSuppressionsBlockBegin.erase(supprBegin); suppressions.addSuppression(std::move(suppr)); // TODO: check result @@ -265,8 +268,12 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett } if (throwError) { + simplecpp::Location loc(tokens.getFiles()); // NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false - bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin"); + loc.fileIndex = suppr.fileIndex; + loc.line = suppr.lineNumber; + loc.col = suppr.column; + bad.emplace_back(loc, "Suppress End: No matching begin"); } } else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) { // special handling when suppressing { warnings for backwards compatibility @@ -280,20 +287,31 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett suppr.thisAndNextLine = thisAndNextLine; suppr.lineNumber = tok->location.line; + suppr.column = tok->location.col; suppr.macroName = macroName; suppressions.addSuppression(std::move(suppr)); // TODO: check result } else if (SuppressionList::Type::file == suppr.type) { if (onlyComments) suppressions.addSuppression(std::move(suppr)); // TODO: check result - else - bad.emplace_back(suppr.fileName, suppr.lineNumber, "File suppression should be at the top of the file"); + else { + simplecpp::Location loc(tokens.getFiles()); + loc.fileIndex = suppr.fileIndex; + loc.line = suppr.lineNumber; + loc.col = suppr.column; + bad.emplace_back(loc, "File suppression should be at the top of the file"); + } } } } - for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) + for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) { + simplecpp::Location loc(tokens.getFiles()); + loc.fileIndex = suppr.fileIndex; + loc.line = suppr.lineNumber; + loc.col = suppr.column; // cppcheck-suppress useStlAlgorithm - bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end"); + bad.emplace_back(loc, "Suppress Begin: No matching end"); + } } void Preprocessor::inlineSuppressions(SuppressionList &suppressions) @@ -306,7 +324,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions) ::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err); } for (const BadInlineSuppression &bad : err) { - error(bad.file, bad.line, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID + error(bad.location, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID } } @@ -854,7 +872,7 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh case simplecpp::Output::ERROR: hasError = true; if (!startsWith(out.msg,"#error") || showerror) - error(out.location.file(), out.location.line, out.msg, out.type); + error(out.location, out.msg, out.type); break; case simplecpp::Output::WARNING: case simplecpp::Output::PORTABILITY_BACKSLASH: @@ -864,20 +882,22 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh const std::string::size_type pos1 = out.msg.find_first_of("<\""); const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U); if (pos1 < pos2 && pos2 != std::string::npos) - missingInclude(out.location.file(), out.location.line, out.location.col, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader); + missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader); } break; case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY: case simplecpp::Output::SYNTAX_ERROR: case simplecpp::Output::UNHANDLED_CHAR_ERROR: hasError = true; - error(out.location.file(), out.location.line, out.msg, out.type); + error(out.location, out.msg, out.type); break; case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND: case simplecpp::Output::FILE_NOT_FOUND: case simplecpp::Output::DUI_ERROR: hasError = true; - error("", 0, out.msg, out.type); + std::vector f; + simplecpp::Location loc(f); + error(loc, out.msg, out.type); break; } } @@ -912,15 +932,15 @@ static std::string simplecppErrToId(simplecpp::Output::Type type) cppcheck::unreachable(); } -void Preprocessor::error(const std::string &filename, unsigned int linenr, const std::string &msg, simplecpp::Output::Type type) +void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type) { std::list locationList; - if (!filename.empty()) { - std::string file = Path::fromNativeSeparators(filename); + if (!loc.file().empty()) { + std::string file = Path::fromNativeSeparators(loc.file()); if (mSettings.relativePaths) file = Path::getRelativePath(file, mSettings.basePaths); - locationList.emplace_back(file, linenr, 0); // TODO: set column + locationList.emplace_back(file, loc.line, loc.col); // TODO: set column } mErrorLogger.reportErr(ErrorMessage(std::move(locationList), mFile0, @@ -931,14 +951,14 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, const } // Report that include is missing -void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType) +void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType) { if (!mSettings.checks.isEnabled(Checks::missingInclude)) return; std::list locationList; - if (!filename.empty()) { - locationList.emplace_back(filename, linenr, col); + if (!loc.file().empty()) { + locationList.emplace_back(loc.file(), loc.line, loc.col); } ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information, (headerType==SystemHeader) ? @@ -954,13 +974,14 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se std::vector files; simplecpp::TokenList tokens(files); Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP); - preprocessor.missingInclude("", 1, 2, "", UserHeader); - preprocessor.missingInclude("", 1, 2, "", SystemHeader); - preprocessor.error("", 1, "message", simplecpp::Output::ERROR); - preprocessor.error("", 1, "message", simplecpp::Output::SYNTAX_ERROR); - preprocessor.error("", 1, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR); - preprocessor.error("", 1, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY); - preprocessor.error("", 1, "message", simplecpp::Output::FILE_NOT_FOUND); + simplecpp::Location loc(files); + preprocessor.missingInclude(loc, "", UserHeader); + preprocessor.missingInclude(loc, "", SystemHeader); + preprocessor.error(loc, "message", simplecpp::Output::ERROR); + preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR); + preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR); + preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY); + preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND); } void Preprocessor::dump(std::ostream &out) const diff --git a/lib/preprocessor.h b/lib/preprocessor.h index 562d91fe877..c3b84a4fd46 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -141,7 +141,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor { bool reportOutput(const simplecpp::OutputList &outputList, bool showerror); - void error(const std::string &filename, unsigned int linenr, const std::string &msg, simplecpp::Output::Type type); + void error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type); private: static bool hasErrors(const simplecpp::Output &output); @@ -158,7 +158,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor { SystemHeader }; - void missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType); + void missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType); void addRemarkComments(const simplecpp::TokenList &tokens, std::vector &remarkComments) const; diff --git a/lib/suppressions.h b/lib/suppressions.h index 0ff4dbaa36f..2dafcaf9b2d 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -149,9 +149,12 @@ class CPPCHECKLIB SuppressionList { std::string errorId; std::string fileName; std::string extraComment; + // TODO: use simplecpp::Location? + int fileIndex{}; int lineNumber = NO_LINE; int lineBegin = NO_LINE; int lineEnd = NO_LINE; + int column{}; Type type = Type::unique; std::string symbolName; std::string macroName; diff --git a/test/cli/other_test.py b/test/cli/other_test.py index 26a6fa35e56..affed3d83cd 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -3889,8 +3889,7 @@ def test_simplecpp_unhandled_char(tmp_path): assert exitcode == 0, stdout assert stdout.splitlines() == [] assert stderr.splitlines() == [ - # TODO: lacks column information - '{}:2:0: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [unhandledChar]'.format(test_file) + '{}:2:5: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [unhandledChar]'.format(test_file) ] @@ -3921,9 +3920,8 @@ def test_simplecpp_include_nested_too_deeply(tmp_path): test_h = tmp_path / 'test_398.h' assert stderr.splitlines() == [ # TODO: should only report the error once - # TODO: lacks column information - '{}:1:0: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h), - '{}:1:0: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h) + '{}:1:2: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h), + '{}:1:2: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h) ] @@ -3944,9 +3942,8 @@ def test_simplecpp_syntax_error(tmp_path): assert stdout.splitlines() == [] assert stderr.splitlines() == [ # TODO: should only report the error once - # TODO: lacks column information - '{}:1:0: error: No header in #include [syntaxError]'.format(test_file), - '{}:1:0: error: No header in #include [syntaxError]'.format(test_file) + '{}:1:2: error: No header in #include [syntaxError]'.format(test_file), + '{}:1:2: error: No header in #include [syntaxError]'.format(test_file) ] diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 90bd5bac31f..de1c93db5ff 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -426,7 +426,7 @@ class TestSuppressions : public TestFixture { " a++;\n" "}\n", "")); - ASSERT_EQUALS("[test.cpp:2:0]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n" + ASSERT_EQUALS("[test.cpp:2:5]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n" "[test.cpp:4:5]: (error) Uninitialized variable: a [uninitvar]\n", errout_str()); ASSERT_EQUALS(1, (this->*check)("void f() {\n" @@ -435,7 +435,7 @@ class TestSuppressions : public TestFixture { "}\n" "// cppcheck-suppress-file uninitvar\n", "")); - ASSERT_EQUALS("[test.cpp:5:0]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n" + ASSERT_EQUALS("[test.cpp:5:1]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n" "[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n", errout_str()); ASSERT_EQUALS(0, (this->*check)("// cppcheck-suppress-file uninitvar\n" @@ -687,7 +687,7 @@ class TestSuppressions : public TestFixture { " b++;\n" "}\n", "")); - ASSERT_EQUALS("[test.cpp:2:0]: (error) Suppress Begin: No matching end [preprocessorErrorDirective]\n" + ASSERT_EQUALS("[test.cpp:2:5]: (error) Suppress Begin: No matching end [preprocessorErrorDirective]\n" "[test.cpp:4:5]: (error) Uninitialized variable: a [uninitvar]\n" "[test.cpp:6:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str()); @@ -699,7 +699,7 @@ class TestSuppressions : public TestFixture { " // cppcheck-suppress-end uninitvar\n" "}\n", "")); - ASSERT_EQUALS("[test.cpp:6:0]: (error) Suppress End: No matching begin [preprocessorErrorDirective]\n" + ASSERT_EQUALS("[test.cpp:6:5]: (error) Suppress End: No matching begin [preprocessorErrorDirective]\n" "[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n" "[test.cpp:5:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str());