Skip to content

fixed #14265 - added column information to some simplecpp errors #7968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fixed #14265 - added column information to some simplecpp errors #7968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/cppcheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str

if (!hasValidConfig && currCfg == *configurations.rbegin()) {
// If there is no valid configuration then report error..
preprocessor.error(o.location.file(), o.location.line, o.msg, o.type);
preprocessor.error(o.location.file(), o.location.line, o.location.col, o.msg, o.type);
}
continue;

Expand Down
37 changes: 19 additions & 18 deletions lib/preprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ 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)) {}
BadInlineSuppression(std::string file, const int line, unsigned int col, std::string msg) : file(std::move(file)), line(line), col(col), errmsg(std::move(msg)) {}
std::string file;
int line;
int line; // TODO: needs to be unsigned
unsigned int col;
std::string errmsg;
};
}
Expand Down Expand Up @@ -143,7 +144,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
}

if (!errmsg.empty())
bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
bad.emplace_back(tok->location.file(), tok->location.line, tok->location.col, std::move(errmsg));

std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) {
return !s.errorId.empty();
Expand All @@ -163,7 +164,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
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.file(), tok->location.line, tok->location.col, std::move(errmsg));
}

return true;
Expand Down Expand Up @@ -272,7 +273,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett

if (throwError) {
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin");
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress End: No matching begin"); // TODO: set column
}
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
// special handling when suppressing { warnings for backwards compatibility
Expand All @@ -292,14 +293,14 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
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");
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column
}
}
}

for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
// cppcheck-suppress useStlAlgorithm
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress Begin: No matching end"); // TODO: set column
}

void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
Expand All @@ -312,7 +313,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.file, bad.line, bad.col, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID
}
}

Expand Down Expand Up @@ -860,7 +861,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.file(), out.location.line, out.location.col, out.msg, out.type);
break;
case simplecpp::Output::WARNING:
case simplecpp::Output::PORTABILITY_BACKSLASH:
Expand All @@ -877,13 +878,13 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
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.file(), out.location.line, out.location.col, 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);
error("", 0, 0, out.msg, out.type);
break;
}
}
Expand Down Expand Up @@ -918,15 +919,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 std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type)
{
std::list<ErrorMessage::FileLocation> locationList;
if (!filename.empty()) {
std::string file = Path::fromNativeSeparators(filename);
if (mSettings.relativePaths)
file = Path::getRelativePath(file, mSettings.basePaths);

locationList.emplace_back(file, linenr, 0); // TODO: set column
locationList.emplace_back(file, linenr, col);
}
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
mFile0,
Expand Down Expand Up @@ -962,11 +963,11 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se
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);
preprocessor.error("", 1, 2, "message", simplecpp::Output::ERROR);
preprocessor.error("", 1, 2, "message", simplecpp::Output::SYNTAX_ERROR);
preprocessor.error("", 1, 2, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
preprocessor.error("", 1, 2, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
preprocessor.error("", 1, 2, "message", simplecpp::Output::FILE_NOT_FOUND);
}

void Preprocessor::dump(std::ostream &out) const
Expand Down
2 changes: 1 addition & 1 deletion lib/preprocessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,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 std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type);

private:
static bool hasErrors(const simplecpp::Output &output);
Expand Down
4 changes: 2 additions & 2 deletions lib/suppressions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class CPPCHECKLIB SuppressionList {
const std::string &getFileName() const {
return mFileName;
}
int lineNumber;
int lineNumber; // TODO: need to be unsigned
Certainty certainty;
std::string symbolNames;
std::set<std::string> macroNames;
Expand Down Expand Up @@ -149,7 +149,7 @@ class CPPCHECKLIB SuppressionList {
std::string errorId;
std::string fileName;
std::string extraComment;
int lineNumber = NO_LINE;
int lineNumber = NO_LINE; // TODO: needs to be unsigned
int lineBegin = NO_LINE;
int lineEnd = NO_LINE;
Type type = Type::unique;
Expand Down
16 changes: 6 additions & 10 deletions test/cli/other_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3429,8 +3429,7 @@ def test_preprocess_enforced_cpp(tmp_path): # #10989
assert exitcode == 0, stdout if stdout else stderr
assert stdout.splitlines() == []
assert stderr.splitlines() == [
# TODO: lacks column information
'{}:2:0: error: #error "err" [preprocessorErrorDirective]'.format(test_file)
'{}:2:2: error: #error "err" [preprocessorErrorDirective]'.format(test_file)
]


Expand Down Expand Up @@ -3889,8 +3888,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)
]


Expand Down Expand Up @@ -3921,9 +3919,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)
]


Expand All @@ -3944,7 +3941,6 @@ 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)
]
34 changes: 17 additions & 17 deletions test/testpreprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ class TestPreprocessor : public TestFixture {
const auto settings = dinit(Settings, $.userDefines = "__cplusplus");
const char code[] = "#error hello world!\n";
(void)getcodeforcfg(settings, *this, code, "X", "test.c");
ASSERT_EQUALS("[test.c:1:0]: (error) #error hello world! [preprocessorErrorDirective]\n", errout_str());
ASSERT_EQUALS("[test.c:1:2]: (error) #error hello world! [preprocessorErrorDirective]\n", errout_str());
}

// Ticket #2919 - wrong filename reported for #error
Expand All @@ -447,15 +447,15 @@ class TestPreprocessor : public TestFixture {
const auto settings = dinit(Settings, $.userDefines = "TEST");
const char code[] = "#file \"ab.h\"\n#error hello world!\n#endfile";
(void)getcodeforcfg(settings, *this, code, "TEST", "test.c");
ASSERT_EQUALS("[ab.h:1:0]: (error) #error hello world! [preprocessorErrorDirective]\n", errout_str());
ASSERT_EQUALS("[ab.h:1:2]: (error) #error hello world! [preprocessorErrorDirective]\n", errout_str());
}

// After including a file
{
const auto settings = dinit(Settings, $.userDefines = "TEST");
const char code[] = "#file \"ab.h\"\n\n#endfile\n#error aaa";
(void)getcodeforcfg(settings, *this, code, "TEST", "test.c");
ASSERT_EQUALS("[test.c:2:0]: (error) #error aaa [preprocessorErrorDirective]\n", errout_str());
ASSERT_EQUALS("[test.c:2:2]: (error) #error aaa [preprocessorErrorDirective]\n", errout_str());
}
}

Expand Down Expand Up @@ -1529,7 +1529,7 @@ class TestPreprocessor : public TestFixture {
const std::map<std::string, std::string> actual = getcode(settings0, *this, filedata);

ASSERT_EQUALS(0, actual.size());
ASSERT_EQUALS("[file.c:2:0]: (error) No pair for character ('). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.c:2:14]: (error) No pair for character ('). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
}
}

Expand All @@ -1544,7 +1544,7 @@ class TestPreprocessor : public TestFixture {
const std::string actual(expandMacros(filedata, *this));

ASSERT_EQUALS("", actual);
ASSERT_EQUALS("[file.cpp:3:0]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.cpp:3:1]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
}

{
Expand All @@ -1557,7 +1557,7 @@ class TestPreprocessor : public TestFixture {
const std::string actual(expandMacros(filedata, *this));

ASSERT_EQUALS("", actual);
ASSERT_EQUALS("[abc.h:2:0]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[abc.h:2:1]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
}

{
Expand All @@ -1570,7 +1570,7 @@ class TestPreprocessor : public TestFixture {
const std::string actual(expandMacros(filedata, *this));

ASSERT_EQUALS("", actual);
ASSERT_EQUALS("[file.cpp:2:0]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.cpp:2:1]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
}

{
Expand All @@ -1582,7 +1582,7 @@ class TestPreprocessor : public TestFixture {
const std::string actual(expandMacros(filedata, *this));

ASSERT_EQUALS("", actual);
ASSERT_EQUALS("[file.cpp:2:0]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.cpp:2:11]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
}

{
Expand All @@ -1598,7 +1598,7 @@ class TestPreprocessor : public TestFixture {
// expand macros..
(void)expandMacros(filedata, *this);

ASSERT_EQUALS("[file.cpp:7:0]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.cpp:7:12]: (error) No pair for character (\"). Can't process file. File is either invalid or unicode, which is currently not supported. [syntaxError]\n", errout_str());
}
}

Expand Down Expand Up @@ -1651,7 +1651,7 @@ class TestPreprocessor : public TestFixture {
// Compare results..
ASSERT_EQUALS(1, actual.size());
ASSERT_EQUALS("", actual.at(""));
ASSERT_EQUALS("[file.c:6:0]: (error) failed to expand 'BC', Wrong number of parameters for macro 'BC'. [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.c:6:3]: (error) failed to expand 'BC', Wrong number of parameters for macro 'BC'. [syntaxError]\n", errout_str());
}

void newline_in_macro() {
Expand Down Expand Up @@ -1968,12 +1968,12 @@ class TestPreprocessor : public TestFixture {

void invalid_define_1() {
(void)getcode(settings0, *this, "#define =\n");
ASSERT_EQUALS("[file.c:1:0]: (error) Failed to parse #define [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.c:1:2]: (error) Failed to parse #define [syntaxError]\n", errout_str());
}

void invalid_define_2() { // #4036
(void)getcode(settings0, *this, "#define () {(int f(x) }\n");
ASSERT_EQUALS("[file.c:1:0]: (error) Failed to parse #define [syntaxError]\n", errout_str());
ASSERT_EQUALS("[file.c:1:2]: (error) Failed to parse #define [syntaxError]\n", errout_str());
}

void inline_suppressions() {
Expand Down Expand Up @@ -2119,7 +2119,7 @@ class TestPreprocessor : public TestFixture {
const char code[] = "#elif (){\n";
const std::string actual = getcodeforcfg(settings0, *this, code, "TEST", "test.c");
ASSERT_EQUALS("", actual);
ASSERT_EQUALS("[test.c:1:0]: (error) #elif without #if [syntaxError]\n", errout_str());
ASSERT_EQUALS("[test.c:1:2]: (error) #elif without #if [syntaxError]\n", errout_str());
}

void getConfigs1() {
Expand Down Expand Up @@ -2368,8 +2368,8 @@ class TestPreprocessor : public TestFixture {
// Preprocess => don't crash..
(void)getcode(settings0, *this, filedata);
ASSERT_EQUALS(
"[file.c:1:0]: (error) Syntax error in #ifdef [syntaxError]\n"
"[file.c:1:0]: (error) Syntax error in #ifdef [syntaxError]\n", errout_str());
"[file.c:1:2]: (error) Syntax error in #ifdef [syntaxError]\n"
"[file.c:1:2]: (error) Syntax error in #ifdef [syntaxError]\n", errout_str());
}

void garbage() {
Expand All @@ -2385,7 +2385,7 @@ class TestPreprocessor : public TestFixture {
const auto settings = dinit(Settings, $.userDefines = "foo");
const char code[] = "#error hello world!\n";
(void)getcodeforcfg(settings, *this, code, "X", "./././test.c");
ASSERT_EQUALS("[test.c:1:0]: (error) #error hello world! [preprocessorErrorDirective]\n", errout_str());
ASSERT_EQUALS("[test.c:1:2]: (error) #error hello world! [preprocessorErrorDirective]\n", errout_str());
}

// test for existing local include
Expand Down Expand Up @@ -2631,7 +2631,7 @@ class TestPreprocessor : public TestFixture {

settings.standards.setStd("c++11");
ASSERT_EQUALS("", getcodeforcfg(settings, *this, code, "", "test.cpp"));
ASSERT_EQUALS("[test.cpp:1:0]: (error) failed to evaluate #if condition, undefined function-like macro invocation: __has_include( ... ) [syntaxError]\n", errout_str()); // TODO: use individual ID
ASSERT_EQUALS("[test.cpp:1:2]: (error) failed to evaluate #if condition, undefined function-like macro invocation: __has_include( ... ) [syntaxError]\n", errout_str()); // TODO: use individual ID

settings.standards.setStd("c++17");
ASSERT_EQUALS("", getcodeforcfg(settings, *this, code, "", "test.cpp"));
Expand Down
Loading