Skip to content

Commit 3564714

Browse files
firewavefirewave
committed
refs #13810 - fixed missing column for invalidSuppression 
1 parent 31cb62d commit 3564714

File tree

7 files changed

+94
-72
lines changed

7 files changed

+94
-72
lines changed

‎cli/cppcheckexecutor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ static std::vector<ErrorMessage> getUnmatchedSuppressions(const std::list<Suppre
329329

330330
std::list<ErrorMessage::FileLocation> callStack;
331331
if (!s.fileName.empty()) {
332-
callStack.emplace_back(s.fileName, s.lineNumber == -1 ? 0 : s.lineNumber, 0); // TODO: get rid of s.lineNumber == -1 hack
332+
callStack.emplace_back(s.fileName, s.lineNumber == -1 ? 0 : s.lineNumber, 0); // TODO: set column - see #13810 / get rid of s.lineNumber == -1 hack
333333
}
334334
const std::string unmatchedSuppressionId = s.isPolyspace ? "unmatchedPolyspaceSuppression" : "unmatchedSuppression";
335335
errors.emplace_back(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, unmatchedSuppressionId, Certainty::normal);

‎lib/cppcheck.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1161,7 +1161,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
11611161

11621162
if (!hasValidConfig && currCfg == *configurations.rbegin()) {
11631163
// If there is no valid configuration then report error..
1164-
preprocessor.error(tokensP.file(o->location), o->location.line, o->location.col, o->msg, o->type);
1164+
preprocessor.error(o->location, o->msg, o->type);
11651165
}
11661166
skipCfg = true;
11671167
}

‎lib/preprocessor.cpp‎

Lines changed: 62 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,13 @@ Preprocessor::Preprocessor(simplecpp::TokenList& tokens, const Settings& setting
7272

7373
namespace {
7474
struct BadInlineSuppression {
75-
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)) {}
76-
std::string file;
77-
int line; // TODO: needs to be unsigned
78-
unsigned int col;
75+
BadInlineSuppression(const simplecpp::Location& loc, std::string msg) : location(loc), errmsg(std::move(msg)) {}
76+
simplecpp::Location location;
7977
std::string errmsg;
8078
};
8179
}
8280

83-
static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &tokens, const simplecpp::Token *tok, std::list<SuppressionList::Suppression> &inlineSuppressions, std::list<BadInlineSuppression> &bad)
81+
static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std::list<SuppressionList::Suppression> &inlineSuppressions, std::list<BadInlineSuppression> &bad)
8482
{
8583
const std::string cppchecksuppress("cppcheck-suppress");
8684

@@ -93,7 +91,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
9391
if (comment.substr(pos1, cppchecksuppress.size()) != cppchecksuppress)
9492
return false;
9593
if (pos1 + cppchecksuppress.size() >= comment.size()) {
96-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "suppression without error ID");
94+
bad.emplace_back(tok->location, "suppression without error ID");
9795
return false;
9896
}
9997

@@ -103,7 +101,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
103101
// skip spaces after "cppcheck-suppress" and its possible prefix
104102
const std::string::size_type pos2 = comment.find_first_not_of(' ', posEndComment);
105103
if (pos2 == std::string::npos) {
106-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "suppression without error ID");
104+
bad.emplace_back(tok->location, "suppression without error ID");
107105
return false;
108106
}
109107

@@ -113,7 +111,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
113111
if (posEndComment >= (pos1 + cppchecksuppress.size() + 1)) {
114112
const std::string suppressCmdString = comment.substr(pos1, pos2-pos1-1);
115113
if (comment.at(pos1 + cppchecksuppress.size()) != '-') {
116-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "unknown suppression type '" + suppressCmdString + "'"); // TODO: set column
114+
bad.emplace_back(tok->location, "unknown suppression type '" + suppressCmdString + "'");
117115
return false;
118116
}
119117

@@ -132,7 +130,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
132130
else if ("macro" == suppressTypeString)
133131
errorType = SuppressionList::Type::macro;
134132
else {
135-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "unknown suppression type '" + suppressCmdString + "'"); // TODO: set column
133+
bad.emplace_back(tok->location, "unknown suppression type '" + suppressCmdString + "'");
136134
return false;
137135
}
138136
}
@@ -146,11 +144,12 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
146144
s.isInline = true;
147145
s.type = errorType;
148146
s.lineNumber = tok->location.line;
147+
s.column = tok->location.col;
149148
}
150149

151150
// TODO: return false?
152151
if (!errmsg.empty())
153-
bad.emplace_back(tokens.file(tok->location), tok->location.line, tok->location.col, std::move(errmsg));
152+
bad.emplace_back(tok->location, std::move(errmsg));
154153

155154
// TODO: report ones without ID - return false?
156155
std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) {
@@ -166,6 +165,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
166165
s.isInline = true;
167166
s.type = errorType;
168167
s.lineNumber = tok->location.line;
168+
s.column = tok->location.col;
169169

170170
// TODO: report when no ID - unreachable?
171171
if (!s.errorId.empty())
@@ -174,7 +174,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
174174
// TODO: unreachable?
175175
// TODO: return false?
176176
if (!errmsg.empty())
177-
bad.emplace_back(tokens.file(tok->location), tok->location.line, tok->location.col, std::move(errmsg));
177+
bad.emplace_back(tok->location, std::move(errmsg));
178178
}
179179

180180
return true;
@@ -213,7 +213,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
213213
if (polyspace::isPolyspaceComment(tok->str())) {
214214
inlineSuppressions = polyspaceParser.parse(tok->str(), tok->location.line, getRelativeFilename(tokens, tok, settings));
215215
} else {
216-
if (!parseInlineSuppressionCommentToken(tokens, tok, inlineSuppressions, bad))
216+
if (!parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad))
217217
continue;
218218

219219
if (!sameline(tok->previous, tok)) {
@@ -222,7 +222,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
222222
tok = tok->next;
223223

224224
while (tok->comment) {
225-
parseInlineSuppressionCommentToken(tokens, tok, inlineSuppressions, bad);
225+
parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad);
226226
if (tok->next) {
227227
tok = tok->next;
228228
} else {
@@ -255,6 +255,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
255255
// Add the suppressions.
256256
for (SuppressionList::Suppression &suppr : inlineSuppressions) {
257257
suppr.fileName = relativeFilename;
258+
suppr.fileIndex = tok->location.fileIndex;
258259

259260
if (SuppressionList::Type::block == suppr.type) {
260261
suppressions.addSuppression(std::move(suppr));
@@ -278,6 +279,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
278279
suppr.lineBegin = supprBegin->lineNumber;
279280
suppr.lineEnd = suppr.lineNumber;
280281
suppr.lineNumber = supprBegin->lineNumber;
282+
suppr.column = supprBegin->column;
281283
suppr.type = SuppressionList::Type::block;
282284
inlineSuppressionsBlockBegin.erase(supprBegin);
283285
suppressions.addSuppression(std::move(suppr)); // TODO: check result
@@ -289,8 +291,12 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
289291
}
290292

291293
if (throwError) {
294+
simplecpp::Location loc;
292295
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
293-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress End: No matching begin"); // TODO: set column
296+
loc.fileIndex = suppr.fileIndex;
297+
loc.line = suppr.lineNumber;
298+
loc.col = suppr.column;
299+
bad.emplace_back(loc, "Suppress End: No matching begin");
294300
}
295301
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
296302
// special handling when suppressing { warnings for backwards compatibility
@@ -304,20 +310,30 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
304310

305311
suppr.thisAndNextLine = thisAndNextLine;
306312
suppr.lineNumber = tok->location.line;
313+
suppr.column = tok->location.col;
307314
suppr.macroName = macroName;
308315
suppressions.addSuppression(std::move(suppr)); // TODO: check result
309316
} else if (SuppressionList::Type::file == suppr.type) {
310317
if (onlyComments)
311318
suppressions.addSuppression(std::move(suppr)); // TODO: check result
312-
else
313-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column
319+
else {
320+
simplecpp::Location loc;
321+
loc.fileIndex = suppr.fileIndex;
322+
loc.line = suppr.lineNumber;
323+
loc.col = suppr.column;
324+
bad.emplace_back(loc, "File suppression should be at the top of the file");
325+
}
314326
}
315327
}
316328
}
317329

318-
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
319-
// cppcheck-suppress useStlAlgorithm
320-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress Begin: No matching end"); // TODO: set column
330+
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) {
331+
simplecpp::Location loc;
332+
loc.fileIndex = suppr.fileIndex;
333+
loc.line = suppr.lineNumber;
334+
loc.col = suppr.column;
335+
bad.emplace_back(loc, "Suppress Begin: No matching end");
336+
}
321337
}
322338

323339
void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
@@ -330,7 +346,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
330346
::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err);
331347
}
332348
for (const BadInlineSuppression &bad : err) {
333-
invalidSuppression(bad.file, bad.line, bad.col, bad.errmsg); // TODO: column is always 0
349+
invalidSuppression(bad.location, bad.errmsg);
334350
}
335351
}
336352

@@ -937,7 +953,7 @@ const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList
937953
case simplecpp::Output::ERROR:
938954
out_ret = &out;
939955
if (!startsWith(out.msg,"#error") || showerror)
940-
error(mTokens.file(out.location), out.location.line, out.location.col, out.msg, out.type);
956+
error(out.location, out.msg, out.type);
941957
break;
942958
case simplecpp::Output::WARNING:
943959
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -947,20 +963,20 @@ const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList
947963
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
948964
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
949965
if (pos1 < pos2 && pos2 != std::string::npos)
950-
missingInclude(mTokens.file(out.location), out.location.line, out.location.col, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
966+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
951967
}
952968
break;
953969
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
954970
case simplecpp::Output::SYNTAX_ERROR:
955971
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
956972
out_ret = &out;
957-
error(mTokens.file(out.location), out.location.line, out.location.col, out.msg, out.type);
973+
error(out.location, out.msg, out.type);
958974
break;
959975
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
960976
case simplecpp::Output::FILE_NOT_FOUND:
961977
case simplecpp::Output::DUI_ERROR:
962978
out_ret = &out;
963-
error("", 0, 0, out.msg, out.type);
979+
error({}, out.msg, out.type);
964980
break;
965981
}
966982
}
@@ -995,20 +1011,20 @@ static std::string simplecppErrToId(simplecpp::Output::Type type)
9951011
cppcheck::unreachable();
9961012
}
9971013

998-
void Preprocessor::error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type)
1014+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type)
9991015
{
1000-
error(filename, linenr, col, msg, simplecppErrToId(type));
1016+
error(loc, msg, simplecppErrToId(type));
10011017
}
10021018

1003-
void Preprocessor::error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, const std::string& id)
1019+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, const std::string& id)
10041020
{
10051021
std::list<ErrorMessage::FileLocation> locationList;
1006-
if (!filename.empty()) {
1007-
std::string file = Path::fromNativeSeparators(filename);
1022+
if (!mTokens.file(loc).empty()) {
1023+
std::string file = Path::fromNativeSeparators(mTokens.file(loc));
10081024
if (mSettings.relativePaths)
10091025
file = Path::getRelativePath(file, mSettings.basePaths);
10101026

1011-
locationList.emplace_back(file, linenr, col);
1027+
locationList.emplace_back(file, loc.line, loc.col);
10121028
}
10131029
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
10141030
mFile0,
@@ -1019,15 +1035,15 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, unsig
10191035
}
10201036

10211037
// Report that include is missing
1022-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType)
1038+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
10231039
{
10241040
if (!mSettings.checks.isEnabled(Checks::missingInclude))
10251041
return;
10261042

10271043
std::list<ErrorMessage::FileLocation> locationList;
1028-
if (!filename.empty()) {
1044+
if (!mTokens.file(loc).empty()) {
10291045
// TODO: add relative path handling?
1030-
locationList.emplace_back(filename, linenr, col);
1046+
locationList.emplace_back(mTokens.file(loc), loc.line, loc.col);
10311047
}
10321048
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
10331049
(headerType==SystemHeader) ?
@@ -1038,24 +1054,27 @@ void Preprocessor::missingInclude(const std::string &filename, unsigned int line
10381054
mErrorLogger.reportErr(errmsg);
10391055
}
10401056

1041-
void Preprocessor::invalidSuppression(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg)
1057+
void Preprocessor::invalidSuppression(const simplecpp::Location& loc, const std::string &msg)
10421058
{
1043-
error(filename, linenr, col, msg, "invalidSuppression");
1059+
error(loc, msg, "invalidSuppression");
10441060
}
10451061

10461062
void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &settings)
10471063
{
10481064
std::vector<std::string> files;
10491065
simplecpp::TokenList tokens(files);
10501066
Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP);
1051-
preprocessor.missingInclude("", 1, 2, "", UserHeader);
1052-
preprocessor.missingInclude("", 1, 2, "", SystemHeader);
1053-
preprocessor.error("", 1, 2, "message", simplecpp::Output::ERROR);
1054-
preprocessor.error("", 1, 2, "message", simplecpp::Output::SYNTAX_ERROR);
1055-
preprocessor.error("", 1, 2, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
1056-
preprocessor.error("", 1, 2, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
1057-
preprocessor.error("", 1, 2, "message", simplecpp::Output::FILE_NOT_FOUND);
1058-
preprocessor.invalidSuppression("", 1, 2, "message");
1067+
simplecpp::Location loc;
1068+
loc.line = 1;
1069+
loc.col = 2;
1070+
preprocessor.missingInclude(loc, "", UserHeader);
1071+
preprocessor.missingInclude(loc, "", SystemHeader);
1072+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
1073+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
1074+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
1075+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
1076+
preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND);
1077+
preprocessor.invalidSuppression(loc, "message");
10591078
}
10601079

10611080
void Preprocessor::dump(std::ostream &out) const

‎lib/preprocessor.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
141141

142142
const simplecpp::Output* reportOutput(const simplecpp::OutputList &outputList, bool showerror);
143143

144-
void error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type);
144+
void error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type);
145145

146146
const simplecpp::Output* handleErrors(const simplecpp::OutputList &outputList);
147147

@@ -156,9 +156,9 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
156156
SystemHeader
157157
};
158158

159-
void missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType);
160-
void invalidSuppression(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg);
161-
void error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, const std::string& id);
159+
void missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType);
160+
void invalidSuppression(const simplecpp::Location& loc, const std::string &msg);
161+
void error(const simplecpp::Location& loc, const std::string &msg, const std::string& id);
162162

163163
void addRemarkComments(const simplecpp::TokenList &tokens, std::vector<RemarkComment> &remarkComments) const;
164164

‎lib/suppressions.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,12 @@ class CPPCHECKLIB SuppressionList {
151151
std::string errorId;
152152
std::string fileName;
153153
std::string extraComment;
154+
// TODO: use simplecpp::Location?
155+
int fileIndex{};
154156
int lineNumber = NO_LINE; // TODO: needs to be unsigned
155157
int lineBegin = NO_LINE;
156158
int lineEnd = NO_LINE;
159+
int column{};
157160
Type type = Type::unique;
158161
std::string symbolName;
159162
std::string macroName;

‎test/cli/project_test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def test_json_entry_file_not_found(tmpdir):
6464
"--project=" + str(project_file)
6565
])
6666
assert 0 == ret
67-
assert stderr == f"nofile:0:0: error: File is missing: {missing_file_posix} [missingFile]\n"
67+
assert stderr == f"{missing_file}:0:0: error: File is missing: {missing_file_posix} [missingFile]\n"
6868

6969

7070
def test_json_no_arguments(tmpdir):

0 commit comments

Comments
 (0)