Skip to content

Commit b080d6e

Browse files
ludviggunneludviggunne
authored
Fix 15030: Extra comment missing from inline multi-suppression (#8852)
1 parent ccbde01 commit b080d6e

2 files changed

Lines changed: 113 additions & 19 deletions

File tree

‎lib/suppressions.cpp‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,36 @@ std::string SuppressionList::parseXmlFile(const char *filename)
153153
return "";
154154
}
155155

156+
static std::string getExtraComment(const std::string &comment, std::string::size_type startPos, std::string::size_type *delimPos = nullptr)
157+
{
158+
const std::string::size_type semiPos = comment.find(';', startPos);
159+
const std::string::size_type slashPos = comment.find("//", startPos);
160+
std::string::size_type pos;
161+
162+
if (delimPos)
163+
*delimPos = std::min(semiPos, slashPos);
164+
165+
if (semiPos < slashPos) {
166+
pos = semiPos + 1;
167+
} else if (slashPos < semiPos) {
168+
pos = slashPos + 2;
169+
} else {
170+
return "";
171+
}
172+
173+
std::string extra = comment.substr(pos);
174+
175+
if (startsWith(comment, "/*") && endsWith(comment, "*/"))
176+
extra.erase(extra.size() - 2, 2);
177+
178+
extra = trim(extra);
179+
180+
for (auto it = extra.begin(); it != extra.end();)
181+
it = (*it & 0x80) ? extra.erase(it) : it + 1;
182+
183+
return extra;
184+
}
185+
156186
std::vector<SuppressionList::Suppression> SuppressionList::parseMultiSuppressComment(const std::string &comment, std::string *errorMessage)
157187
{
158188
std::vector<Suppression> suppressions;
@@ -207,6 +237,14 @@ std::vector<SuppressionList::Suppression> SuppressionList::parseMultiSuppressCom
207237
suppressions.push_back(std::move(s));
208238
}
209239

240+
const std::string extraComment = getExtraComment(comment, end_position);
241+
242+
if (extraComment.empty())
243+
return suppressions;
244+
245+
for (auto &suppression : suppressions)
246+
suppression.extraComment = extraComment;
247+
210248
return suppressions;
211249
}
212250

@@ -360,20 +398,11 @@ bool SuppressionList::Suppression::parseComment(std::string comment, std::string
360398
if (comment.compare(comment.size() - 2, 2, "*/") == 0)
361399
comment.erase(comment.size() - 2, 2);
362400

363-
std::string::size_type extraPos = comment.find(';');
364-
std::string::size_type extraDelimiterSize = 1;
365-
366-
if (extraPos == std::string::npos) {
367-
extraPos = comment.find("//", 2);
368-
extraDelimiterSize = 2;
369-
}
401+
std::string::size_type extraPos;
402+
extraComment = getExtraComment(comment, 2, &extraPos);
370403

371-
if (extraPos != std::string::npos) {
372-
extraComment = trim(comment.substr(extraPos + extraDelimiterSize));
373-
for (auto it = extraComment.begin(); it != extraComment.end();)
374-
it = *it & 0x80 ? extraComment.erase(it) : it + 1;
404+
if (!extraComment.empty())
375405
comment.erase(extraPos);
376-
}
377406

378407
const std::set<std::string> cppchecksuppress{
379408
"cppcheck-suppress",

‎test/testsuppressions.cpp‎

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,14 +1240,37 @@ class TestSuppressions : public TestFixture {
12401240
}
12411241

12421242
void inlinesuppress_comment() const {
1243-
SuppressionList::Suppression s;
12441243
std::string errMsg;
1245-
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc ; some comment", &errMsg));
1246-
ASSERT_EQUALS("", errMsg);
1247-
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc // some comment", &errMsg));
1248-
ASSERT_EQUALS("", errMsg);
1249-
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc -- some comment", &errMsg));
1250-
ASSERT_EQUALS("", errMsg);
1244+
{
1245+
SuppressionList::Suppression s;
1246+
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc ; some comment // extra stuff", &errMsg));
1247+
ASSERT_EQUALS("", errMsg);
1248+
ASSERT_EQUALS("some comment // extra stuff", s.extraComment);
1249+
}
1250+
{
1251+
SuppressionList::Suppression s;
1252+
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc; some comment // extra stuff", &errMsg));
1253+
ASSERT_EQUALS("", errMsg);
1254+
ASSERT_EQUALS("some comment // extra stuff", s.extraComment);
1255+
}
1256+
{
1257+
SuppressionList::Suppression s;
1258+
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc // some comment ; extra stuff", &errMsg));
1259+
ASSERT_EQUALS("", errMsg);
1260+
ASSERT_EQUALS("some comment ; extra stuff", s.extraComment);
1261+
}
1262+
{
1263+
SuppressionList::Suppression s;
1264+
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc// some comment ; extra stuff", &errMsg));
1265+
ASSERT_EQUALS("", errMsg);
1266+
ASSERT_EQUALS("some comment ; extra stuff", s.extraComment);
1267+
}
1268+
{
1269+
SuppressionList::Suppression s;
1270+
ASSERT_EQUALS(true, s.parseComment("// cppcheck-suppress abc -- some comment", &errMsg));
1271+
ASSERT_EQUALS("", errMsg);
1272+
ASSERT_EQUALS("", s.extraComment);
1273+
}
12511274
}
12521275

12531276
// TODO: tests internal function - should it be private?
@@ -1388,6 +1411,48 @@ class TestSuppressions : public TestFixture {
13881411
suppressions=SuppressionList::parseMultiSuppressComment("/*cppcheck-suppress[errorId1, errorId2 symbolName=arr]*/", &errMsg);
13891412
ASSERT_EQUALS(2, suppressions.size());
13901413
ASSERT_EQUALS(true, errMsg.empty());
1414+
1415+
errMsg = "";
1416+
suppressions=SuppressionList::parseMultiSuppressComment("//cppcheck-suppress[errorId1, errorId2 symbolName=arr] ; extra comment", &errMsg);
1417+
ASSERT_EQUALS(2, suppressions.size());
1418+
ASSERT_EQUALS(true, errMsg.empty());
1419+
ASSERT_EQUALS("extra comment", suppressions[0].extraComment);
1420+
ASSERT_EQUALS("extra comment", suppressions[1].extraComment);
1421+
1422+
errMsg = "";
1423+
suppressions=SuppressionList::parseMultiSuppressComment("//cppcheck-suppress[errorId1, errorId2 symbolName=arr] // extra comment", &errMsg);
1424+
ASSERT_EQUALS(2, suppressions.size());
1425+
ASSERT_EQUALS(true, errMsg.empty());
1426+
ASSERT_EQUALS("extra comment", suppressions[0].extraComment);
1427+
ASSERT_EQUALS("extra comment", suppressions[1].extraComment);
1428+
1429+
errMsg = "";
1430+
suppressions=SuppressionList::parseMultiSuppressComment("/*cppcheck-suppress[errorId1, errorId2 symbolName=arr] ; extra comment */", &errMsg);
1431+
ASSERT_EQUALS(2, suppressions.size());
1432+
ASSERT_EQUALS(true, errMsg.empty());
1433+
ASSERT_EQUALS("extra comment", suppressions[0].extraComment);
1434+
ASSERT_EQUALS("extra comment", suppressions[1].extraComment);
1435+
1436+
errMsg = "";
1437+
suppressions=SuppressionList::parseMultiSuppressComment("/*cppcheck-suppress[errorId1, errorId2 symbolName=arr] // extra comment */", &errMsg);
1438+
ASSERT_EQUALS(2, suppressions.size());
1439+
ASSERT_EQUALS(true, errMsg.empty());
1440+
ASSERT_EQUALS("extra comment", suppressions[0].extraComment);
1441+
ASSERT_EQUALS("extra comment", suppressions[1].extraComment);
1442+
1443+
errMsg = "";
1444+
suppressions=SuppressionList::parseMultiSuppressComment("/*cppcheck-suppress[errorId1, errorId2 symbolName=arr] ; extra comment // more */", &errMsg);
1445+
ASSERT_EQUALS(2, suppressions.size());
1446+
ASSERT_EQUALS(true, errMsg.empty());
1447+
ASSERT_EQUALS("extra comment // more", suppressions[0].extraComment);
1448+
ASSERT_EQUALS("extra comment // more", suppressions[1].extraComment);
1449+
1450+
errMsg = "";
1451+
suppressions=SuppressionList::parseMultiSuppressComment("/*cppcheck-suppress[errorId1, errorId2 symbolName=arr] // extra comment ; more */", &errMsg);
1452+
ASSERT_EQUALS(2, suppressions.size());
1453+
ASSERT_EQUALS(true, errMsg.empty());
1454+
ASSERT_EQUALS("extra comment ; more", suppressions[0].extraComment);
1455+
ASSERT_EQUALS("extra comment ; more", suppressions[1].extraComment);
13911456
}
13921457

13931458
void globalSuppressions() { // Testing that Cppcheck::useGlobalSuppressions works (#8515)

0 commit comments

Comments
 (0)