From 4f9a0b8420e94e17665be3a56915aa4eab7a4dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 23 Feb 2020 19:49:53 +0100 Subject: [PATCH] Refactoring suppressions --- lib/preprocessor.cpp | 88 +++++++++++++++++++-------------------- lib/suppressions.cpp | 62 ++++++++------------------- lib/suppressions.h | 2 +- test/testsuppressions.cpp | 19 +-------- 4 files changed, 62 insertions(+), 109 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 6006a0cc8..74acc576c 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -32,6 +32,11 @@ #include // back_inserter #include +static bool sameline(const simplecpp::Token *tok1, const simplecpp::Token *tok2) +{ + return tok1 && tok2 && tok1->location.sameline(tok2->location); +} + /** * Remove heading and trailing whitespaces from the input parameter. * If string is all spaces/tabs, return empty string. @@ -76,42 +81,42 @@ namespace { }; } -static void parseCommentToken(const simplecpp::Token *tok, std::list &inlineSuppressions, std::list *bad) +static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std::list &inlineSuppressions, std::list *bad) { - if (tok->str().size() < 19) - return; - const std::string::size_type pos1 = tok->str().find_first_not_of("/* \t"); + const std::string &comment = tok->str(); + if (comment.size() < 19) + return false; + const std::string::size_type pos1 = comment.find_first_not_of("/* \t"); if (pos1 == std::string::npos) - return; - if (pos1 + 17 >= tok->str().size()) - return; - if (tok->str().compare(pos1, 17, "cppcheck-suppress") != 0) - return; + return false; + if (pos1 + 17 >= comment.size()) + return false; + if (comment.compare(pos1, 17, "cppcheck-suppress") != 0) + return false; // skip spaces after "cppcheck-suppress" - const std::string::size_type pos2 = tok->str().find_first_not_of(" ", pos1+17); + const std::string::size_type pos2 = comment.find_first_not_of(" ", pos1+17); if (pos2 == std::string::npos) - return; + return false; - if (tok->str()[pos2] == '[') { + if (comment[pos2] == '[') { // multi suppress format std::string errmsg; - std::vector ss; - ss = Suppressions::parseMultiSuppressComment(tok->str(), &errmsg); + std::vector suppressions = Suppressions::parseMultiSuppressComment(comment, &errmsg); if (!errmsg.empty()) bad->push_back(BadInlineSuppression(tok->location, errmsg)); - for (std::vector::iterator iter = ss.begin(); iter!=ss.end(); ++iter) { - if (!(*iter).errorId.empty()) - inlineSuppressions.push_back(*iter); + for (const Suppressions::Suppression &s : suppressions) { + if (!s.errorId.empty()) + inlineSuppressions.push_back(s); } } else { //single suppress format std::string errmsg; Suppressions::Suppression s; - if (!s.parseComment(tok->str(), &errmsg)) - return; + if (!s.parseComment(comment, &errmsg)) + return false; if (!s.errorId.empty()) inlineSuppressions.push_back(s); @@ -119,37 +124,36 @@ static void parseCommentToken(const simplecpp::Token *tok, std::listpush_back(BadInlineSuppression(tok->location, errmsg)); } + + return true; } static void inlineSuppressions(const simplecpp::TokenList &tokens, Settings &mSettings, std::list *bad) { - const simplecpp::Token *current_non_comment_tok; - std::list inlineSuppressions; + for (const simplecpp::Token *tok = tokens.cfront(); tok; tok = tok ? tok->next : nullptr) { + if (!tok->comment) + continue; - for (const simplecpp::Token *tok = tokens.cfront(); tok; /*none*/) { - //parse all comment before current non-comment tok - if (tok->comment) { - parseCommentToken(tok, inlineSuppressions, bad); + std::list inlineSuppressions; + if (!parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad)) + continue; + + if (!sameline(tok->previous, tok)) { + // find code after comment.. tok = tok->next; - continue; - } - - current_non_comment_tok = tok; - - //parse all comment tok after current non-comment tok in the same line - for (tok = tok->next; tok; tok = tok->next) { - if ((tok->location.line != current_non_comment_tok->location.line) || !(tok->comment)) + while (tok && tok->comment) { + parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad); + tok = tok->next; + } + if (!tok) break; - parseCommentToken(tok, inlineSuppressions, bad); } - //if there is no suppress, jump it! - if (inlineSuppressions.empty()) { + if (inlineSuppressions.empty()) continue; - } // Relative filename - std::string relativeFilename(current_non_comment_tok->location.file()); + std::string relativeFilename(tok->location.file()); if (mSettings.relativePaths) { for (const std::string & basePath : mSettings.basePaths) { const std::string bp = basePath + "/"; @@ -163,10 +167,9 @@ static void inlineSuppressions(const simplecpp::TokenList &tokens, Settings &mSe // Add the suppressions. for (Suppressions::Suppression &suppr : inlineSuppressions) { suppr.fileName = relativeFilename; - suppr.lineNumber = current_non_comment_tok->location.line; + suppr.lineNumber = tok->location.line; mSettings.nomsg.addSuppression(suppr); } - inlineSuppressions.clear(); } } @@ -219,11 +222,6 @@ void Preprocessor::setDirectives(const simplecpp::TokenList &tokens) } } -static bool sameline(const simplecpp::Token *tok1, const simplecpp::Token *tok2) -{ - return tok1 && tok2 && tok1->location.sameline(tok2->location); -} - static std::string readcondition(const simplecpp::Token *iftok, const std::set &defined, const std::set &undefined) { const simplecpp::Token *cond = iftok->next; diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index 3ef2a43cf..3ed9ee777 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -112,57 +112,31 @@ std::string Suppressions::parseXmlFile(const char *filename) return ""; } -std::vector Suppressions::parseMultiSuppressComment(std::string comment, std::string *errorMessage) +std::vector Suppressions::parseMultiSuppressComment(const std::string &comment, std::string *errorMessage) { std::vector suppressions; - if (comment.size() < 2) - return suppressions; - const std::size_t first_non_blank_position = comment.find_first_not_of(" \t\n", 2); - const std::size_t suppress_position = comment.find("cppcheck-suppress", 2); - if (suppress_position == std::string::npos) - return suppressions; - if (suppress_position != first_non_blank_position) { - if (errorMessage && errorMessage->empty()) - *errorMessage = "Bad multi suppression '" + comment + "'. there shouldn't have non-blank character before cppcheck-suppress"; - return suppressions; - } - - const std::size_t start_position = comment.find("[", suppress_position); - const std::size_t end_position = comment.find("]", suppress_position); - if (start_position == std::string::npos - || end_position == std::string::npos - || start_position != suppress_position+17 //there must be no space before "[" - || start_position >= end_position) { + // If this function is called we assume that comment starts with "cppcheck-suppress[". + const std::string::size_type start_position = comment.find("["); + const std::string::size_type end_position = comment.find("]", start_position); + if (end_position == std::string::npos) { if (errorMessage && errorMessage->empty()) *errorMessage = "Bad multi suppression '" + comment + "'. legal format is cppcheck-suppress[errorId, errorId symbolName=arr, ...]"; return suppressions; } - //extract supressions - std::size_t current_comma_position = 0; - //multi_suppressions_word maybe "[errorId1, errorId2 symbolName=arr", who just has left bracket - std::string multi_suppressions_word = comment.substr(start_position, end_position-start_position); - do { - std::string suppression_word; + // parse all suppressions + for (std::string::size_type pos = start_position; pos < end_position;) { + const std::string::size_type pos1 = pos + 1; + pos = comment.find(",", pos1); + const std::string::size_type pos2 = (pos < end_position) ? pos : end_position; + if (pos1 == pos2) + continue; - //single suppression word - const std::size_t previous_comma_position=current_comma_position; - current_comma_position=multi_suppressions_word.find(",", previous_comma_position+1); //find "," after previous comma - if (current_comma_position == std::string::npos) { - suppression_word = multi_suppressions_word.substr(previous_comma_position+1); - } else { - suppression_word = multi_suppressions_word.substr(previous_comma_position+1, current_comma_position-previous_comma_position-1); - } - - //parse single suppression word Suppression s; - std::string word; - std::string errorId; - std::string symbolName; - std::istringstream iss(suppression_word); + std::istringstream iss(comment.substr(pos1, pos2-pos1)); - iss >> errorId; + iss >> s.errorId; if (!iss) { if (errorMessage && errorMessage->empty()) *errorMessage = "Bad multi suppression '" + comment + "'. legal format is cppcheck-suppress[errorId, errorId symbolName=arr, ...]"; @@ -171,13 +145,14 @@ std::vector Suppressions::parseMultiSuppressComment(s } while (iss) { + std::string word; iss >> word; if (!iss) break; if (word.find_first_not_of("+-*/%#;") == std::string::npos) break; if (word.compare(0, 11, "symbolName=") == 0) { - symbolName = word.substr(11); + s.symbolName = word.substr(11); } else { if (errorMessage && errorMessage->empty()) *errorMessage = "Bad multi suppression '" + comment + "'. legal format is cppcheck-suppress[errorId, errorId symbolName=arr, ...]"; @@ -186,11 +161,8 @@ std::vector Suppressions::parseMultiSuppressComment(s } } - s.errorId = errorId; - s.symbolName = symbolName; suppressions.push_back(s); - - } while (current_comma_position!=std::string::npos); + } return suppressions; } diff --git a/lib/suppressions.h b/lib/suppressions.h index 8ebfa209f..ca4c4855f 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -122,7 +122,7 @@ public: * @param errorMessage output parameter for error message (wrong suppression attribute) * @return empty vector if something wrong. */ - static std::vector parseMultiSuppressComment(std::string comment, std::string *errorMessage); + static std::vector parseMultiSuppressComment(const std::string &comment, std::string *errorMessage); /** * @brief Don't show the given error. diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 5fff8ceeb..63a9266aa 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -576,15 +576,10 @@ private: ASSERT_EQUALS("arr", suppressions[1].symbolName); ASSERT_EQUALS("", errMsg); - errMsg = ""; - suppressions=ss.parseMultiSuppressComment("// cppcheck-suppress [errorId]", &errMsg); - ASSERT_EQUALS(0, suppressions.size()); - ASSERT_EQUALS(false, errMsg.empty()); - errMsg = ""; suppressions=ss.parseMultiSuppressComment("// cppcheck-suppress[]", &errMsg); ASSERT_EQUALS(0, suppressions.size()); - ASSERT_EQUALS(false, errMsg.empty()); + ASSERT_EQUALS(true, errMsg.empty()); errMsg = ""; suppressions=ss.parseMultiSuppressComment("// cppcheck-suppress[errorId", &errMsg); @@ -622,23 +617,11 @@ private: ASSERT_EQUALS(2, suppressions.size()); ASSERT_EQUALS(true, errMsg.empty()); - //error, shouldn't have "some text" at beginning - errMsg = ""; - suppressions=ss.parseMultiSuppressComment("//some text cppcheck-suppress[errorId1, errorId2 symbolName=arr]", &errMsg); - ASSERT_EQUALS(0, suppressions.size()); - ASSERT_EQUALS(false, errMsg.empty()); - errMsg = ""; suppressions=ss.parseMultiSuppressComment("//cppcheck-suppress[errorId1, errorId2 symbolName=arr] some text", &errMsg); ASSERT_EQUALS(2, suppressions.size()); ASSERT_EQUALS(true, errMsg.empty()); - //error, shouldn't have "some text" at beginning - errMsg = ""; - suppressions=ss.parseMultiSuppressComment("//some text cppcheck-suppress[errorId1, errorId2 symbolName=arr] some text", &errMsg); - ASSERT_EQUALS(0, suppressions.size()); - ASSERT_EQUALS(false, errMsg.empty()); - errMsg = ""; suppressions=ss.parseMultiSuppressComment("/*cppcheck-suppress[errorId1, errorId2 symbolName=arr]*/", &errMsg); ASSERT_EQUALS(2, suppressions.size());