Skip to content

Commit c30d76b

Browse files
firewavefirewave
committed
refs #13810 - fixed missing column for invalidSuppression 
1 parent 61a4ae9 commit c30d76b

File tree

6 files changed

+93
-71
lines changed

6 files changed

+93
-71
lines changed

‎cli/cppcheckexecutor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppres
327327

328328
std::list<::ErrorMessage::FileLocation> callStack;
329329
if (!s.fileName.empty()) {
330-
callStack.emplace_back(s.fileName, s.lineNumber, 0);
330+
callStack.emplace_back(s.fileName, s.lineNumber, 0); // TODO: set column - see #13810
331331
}
332332
errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
333333
err = true;

‎lib/cppcheck.cpp‎

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

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

‎lib/preprocessor.cpp‎

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

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

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

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

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

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

@@ -131,7 +129,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
131129
else if ("macro" == suppressTypeString)
132130
errorType = SuppressionList::Type::macro;
133131
else {
134-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "unknown suppression type '" + suppressCmdString + "'"); // TODO: set column
132+
bad.emplace_back(tok->location, "unknown suppression type '" + suppressCmdString + "'");
135133
return false;
136134
}
137135
}
@@ -145,11 +143,12 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
145143
s.isInline = true;
146144
s.type = errorType;
147145
s.lineNumber = tok->location.line;
146+
s.column = tok->location.col;
148147
}
149148

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

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

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

179179
return true;
@@ -207,7 +207,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
207207
}
208208

209209
std::list<SuppressionList::Suppression> inlineSuppressions;
210-
if (!parseInlineSuppressionCommentToken(tokens, tok, inlineSuppressions, bad))
210+
if (!parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad))
211211
continue;
212212

213213
if (!sameline(tok->previous, tok)) {
@@ -216,7 +216,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
216216
tok = tok->next;
217217

218218
while (tok->comment) {
219-
parseInlineSuppressionCommentToken(tokens, tok, inlineSuppressions, bad);
219+
parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad);
220220
if (tok->next) {
221221
tok = tok->next;
222222
} else {
@@ -248,6 +248,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
248248
// Add the suppressions.
249249
for (SuppressionList::Suppression &suppr : inlineSuppressions) {
250250
suppr.fileName = relativeFilename;
251+
suppr.fileIndex = tok->location.fileIndex;
251252

252253
if (SuppressionList::Type::blockBegin == suppr.type)
253254
{
@@ -270,6 +271,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
270271
suppr.lineBegin = supprBegin->lineNumber;
271272
suppr.lineEnd = suppr.lineNumber;
272273
suppr.lineNumber = supprBegin->lineNumber;
274+
suppr.column = supprBegin->column;
273275
suppr.type = SuppressionList::Type::block;
274276
inlineSuppressionsBlockBegin.erase(supprBegin);
275277
suppressions.addSuppression(std::move(suppr)); // TODO: check result
@@ -281,8 +283,11 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
281283
}
282284

283285
if (throwError) {
284-
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
285-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress End: No matching begin"); // TODO: set column
286+
simplecpp::Location loc;
287+
loc.fileIndex = suppr.fileIndex;
288+
loc.line = suppr.lineNumber;
289+
loc.col = suppr.column;
290+
bad.emplace_back(loc, "Suppress End: No matching begin");
286291
}
287292
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
288293
// special handling when suppressing { warnings for backwards compatibility
@@ -296,20 +301,31 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
296301

297302
suppr.thisAndNextLine = thisAndNextLine;
298303
suppr.lineNumber = tok->location.line;
304+
suppr.column = tok->location.col;
299305
suppr.macroName = macroName;
300306
suppressions.addSuppression(std::move(suppr)); // TODO: check result
301307
} else if (SuppressionList::Type::file == suppr.type) {
302308
if (onlyComments)
303309
suppressions.addSuppression(std::move(suppr)); // TODO: check result
304-
else
305-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column
310+
else {
311+
simplecpp::Location loc;
312+
loc.fileIndex = suppr.fileIndex;
313+
loc.line = suppr.lineNumber;
314+
loc.col = suppr.column;
315+
bad.emplace_back(loc, "File suppression should be at the top of the file");
316+
}
306317
}
307318
}
308319
}
309320

310-
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
321+
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) {
322+
simplecpp::Location loc;
323+
loc.fileIndex = suppr.fileIndex;
324+
loc.line = suppr.lineNumber;
325+
loc.col = suppr.column;
311326
// cppcheck-suppress useStlAlgorithm
312-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress Begin: No matching end"); // TODO: set column
327+
bad.emplace_back(loc, "Suppress Begin: No matching end");
328+
}
313329
}
314330

315331
void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
@@ -322,7 +338,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
322338
::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err);
323339
}
324340
for (const BadInlineSuppression &bad : err) {
325-
invalidSuppression(bad.file, bad.line, bad.col, bad.errmsg); // TODO: column is always 0
341+
invalidSuppression(bad.location, bad.errmsg);
326342
}
327343
}
328344

@@ -865,7 +881,7 @@ const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList
865881
case simplecpp::Output::ERROR:
866882
out_ret = &out;
867883
if (!startsWith(out.msg,"#error") || showerror)
868-
error(mTokens.file(out.location), out.location.line, out.location.col, out.msg, out.type);
884+
error(out.location, out.msg, out.type);
869885
break;
870886
case simplecpp::Output::WARNING:
871887
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -875,20 +891,20 @@ const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList
875891
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
876892
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
877893
if (pos1 < pos2 && pos2 != std::string::npos)
878-
missingInclude(mTokens.file(out.location), out.location.line, out.location.col, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
894+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
879895
}
880896
break;
881897
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
882898
case simplecpp::Output::SYNTAX_ERROR:
883899
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
884900
out_ret = &out;
885-
error(mTokens.file(out.location), out.location.line, out.location.col, out.msg, out.type);
901+
error(out.location, out.msg, out.type);
886902
break;
887903
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
888904
case simplecpp::Output::FILE_NOT_FOUND:
889905
case simplecpp::Output::DUI_ERROR:
890906
out_ret = &out;
891-
error("", 0, 0, out.msg, out.type);
907+
error({}, out.msg, out.type);
892908
break;
893909
}
894910
}
@@ -923,20 +939,20 @@ static std::string simplecppErrToId(simplecpp::Output::Type type)
923939
cppcheck::unreachable();
924940
}
925941

926-
void Preprocessor::error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type)
942+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type)
927943
{
928-
error(filename, linenr, col, msg, simplecppErrToId(type));
944+
error(loc, msg, simplecppErrToId(type));
929945
}
930946

931-
void Preprocessor::error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, const std::string& id)
947+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, const std::string& id)
932948
{
933949
std::list<ErrorMessage::FileLocation> locationList;
934-
if (!filename.empty()) {
935-
std::string file = Path::fromNativeSeparators(filename);
950+
if (!mTokens.file(loc).empty()) {
951+
std::string file = Path::fromNativeSeparators(mTokens.file(loc));
936952
if (mSettings.relativePaths)
937953
file = Path::getRelativePath(file, mSettings.basePaths);
938954

939-
locationList.emplace_back(file, linenr, col);
955+
locationList.emplace_back(file, loc.line, loc.col);
940956
}
941957
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
942958
mFile0,
@@ -947,15 +963,15 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, unsig
947963
}
948964

949965
// Report that include is missing
950-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType)
966+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
951967
{
952968
if (!mSettings.checks.isEnabled(Checks::missingInclude))
953969
return;
954970

955971
std::list<ErrorMessage::FileLocation> locationList;
956-
if (!filename.empty()) {
972+
if (!mTokens.file(loc).empty()) {
957973
// TODO: add relative path handling?
958-
locationList.emplace_back(filename, linenr, col);
974+
locationList.emplace_back(mTokens.file(loc), loc.line, loc.col);
959975
}
960976
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
961977
(headerType==SystemHeader) ?
@@ -966,24 +982,27 @@ void Preprocessor::missingInclude(const std::string &filename, unsigned int line
966982
mErrorLogger.reportErr(errmsg);
967983
}
968984

969-
void Preprocessor::invalidSuppression(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg)
985+
void Preprocessor::invalidSuppression(const simplecpp::Location& loc, const std::string &msg)
970986
{
971-
error(filename, linenr, col, msg, "invalidSuppression");
987+
error(loc, msg, "invalidSuppression");
972988
}
973989

974990
void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &settings)
975991
{
976992
std::vector<std::string> files;
977993
simplecpp::TokenList tokens(files);
978994
Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP);
979-
preprocessor.missingInclude("", 1, 2, "", UserHeader);
980-
preprocessor.missingInclude("", 1, 2, "", SystemHeader);
981-
preprocessor.error("", 1, 2, "message", simplecpp::Output::ERROR);
982-
preprocessor.error("", 1, 2, "message", simplecpp::Output::SYNTAX_ERROR);
983-
preprocessor.error("", 1, 2, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
984-
preprocessor.error("", 1, 2, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
985-
preprocessor.error("", 1, 2, "message", simplecpp::Output::FILE_NOT_FOUND);
986-
preprocessor.invalidSuppression("", 1, 2, "message");
995+
simplecpp::Location loc;
996+
loc.line = 1;
997+
loc.col = 2;
998+
preprocessor.missingInclude(loc, "", UserHeader);
999+
preprocessor.missingInclude(loc, "", SystemHeader);
1000+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
1001+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
1002+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
1003+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
1004+
preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND);
1005+
preprocessor.invalidSuppression(loc, "message");
9871006
}
9881007

9891008
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
@@ -149,9 +149,12 @@ class CPPCHECKLIB SuppressionList {
149149
std::string errorId;
150150
std::string fileName;
151151
std::string extraComment;
152+
// TODO: use simplecpp::Location?
153+
int fileIndex{};
152154
int lineNumber = NO_LINE; // TODO: needs to be unsigned
153155
int lineBegin = NO_LINE;
154156
int lineEnd = NO_LINE;
157+
int column{};
155158
Type type = Type::unique;
156159
std::string symbolName;
157160
std::string macroName;

0 commit comments

Comments
 (0)