Inline suppression for block of code or whole file (danmar#5402)

Added new suppress comments: - `cppcheck-suppress-begin` and `cppcheck-suppress-end` to remove blocks of suppression - `cppcheck-suppress-file` to remove suppression at file level The suppressions do not interfere with each others. For example, all the suppressions are matched in the following code: ```c // cppcheck-suppress-file uninitvar void f() { int a; // cppcheck-suppress-begin uninitvar // cppcheck-suppress uninitvar a++; // cppcheck-suppress-end uninitvar } ``` Tickets: https://trac.cppcheck.net/ticket/11902 https://trac.cppcheck.net/ticket/8528
cppchecksolutions · Oct 13, 2023 · 44ab976 · 44ab976
1 parent 8ef4da4
commit 44ab976
Show file tree

Hide file tree

Showing 6 changed files with 560 additions and 39 deletions.
diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp
@@ -76,8 +76,9 @@ Preprocessor::~Preprocessor()
 
 namespace {
     struct BadInlineSuppression {
-        BadInlineSuppression(const simplecpp::Location &l, std::string msg) : location(l), errmsg(std::move(msg)) {}
-        simplecpp::Location location;
+        BadInlineSuppression(std::string file, const int line, std::string msg) : file(std::move(file)), line(line), errmsg(std::move(msg)) {}
+        std::string file;
+        int line;
         std::string errmsg;
     };
 }
@@ -97,18 +98,50 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
     if (comment.substr(pos1, cppchecksuppress.size()) != cppchecksuppress)
         return false;
 
-    // skip spaces after "cppcheck-suppress"
-    const std::string::size_type pos2 = comment.find_first_not_of(' ', pos1+cppchecksuppress.size());
+    // check if it has a prefix
+    const std::string::size_type posEndComment = comment.find_first_of(" [", pos1+cppchecksuppress.size());
+
+    // skip spaces after "cppcheck-suppress" and its possible prefix
+    const std::string::size_type pos2 = comment.find_first_not_of(' ', posEndComment);
     if (pos2 == std::string::npos)
         return false;
 
+    Suppressions::Type errorType = Suppressions::Type::unique;
+
+    // determine prefix if specified
+    if (posEndComment >= (pos1 + cppchecksuppress.size() + 1)) {
+        if (comment.at(pos1 + cppchecksuppress.size()) != '-')
+            return false;
+
+        const unsigned int argumentLength =
+            posEndComment - (pos1 + cppchecksuppress.size() + 1);
+
+        const std::string suppressTypeString =
+            comment.substr(pos1 + cppchecksuppress.size() + 1, argumentLength);
+
+        if ("file" == suppressTypeString) {
+            errorType = Suppressions::Type::file;
+        } else if ("begin" == suppressTypeString) {
+            errorType = Suppressions::Type::blockBegin;
+        } else if ("end" == suppressTypeString) {
+            errorType = Suppressions::Type::blockEnd;
+        } else {
+            return false;
+        }
+    }
+
     if (comment[pos2] == '[') {
         // multi suppress format
         std::string errmsg;
         std::vector<Suppressions::Suppression> suppressions = Suppressions::parseMultiSuppressComment(comment, &errmsg);
 
+        for (Suppressions::Suppression &s : suppressions) {
+            s.type = errorType;
+            s.lineNumber = tok->location.line;
+        }
+
         if (!errmsg.empty())
-            bad.emplace_back(tok->location, std::move(errmsg));
+            bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
 
         std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const Suppressions::Suppression& s) {
             return !s.errorId.empty();
@@ -120,40 +153,58 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
         if (!s.parseComment(comment, &errmsg))
             return false;
 
+        s.type = errorType;
+        s.lineNumber = tok->location.line;
+
         if (!s.errorId.empty())
             inlineSuppressions.push_back(std::move(s));
 
         if (!errmsg.empty())
-            bad.emplace_back(tok->location, std::move(errmsg));
+            bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
     }
 
     return true;
 }
 
-static void addinlineSuppressions(const simplecpp::TokenList &tokens, const Settings &settings, Suppressions &suppressions, std::list<BadInlineSuppression> &bad)
+static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Settings &settings, Suppressions &suppressions, std::list<BadInlineSuppression> &bad)
 {
+    std::list<Suppressions::Suppression> inlineSuppressionsBlockBegin;
+
+    bool onlyComments = true;
+
     for (const simplecpp::Token *tok = tokens.cfront(); tok; tok = tok->next) {
-        if (!tok->comment)
+        if (!tok->comment) {
+            onlyComments = false;
             continue;
+        }
 
         std::list<Suppressions::Suppression> inlineSuppressions;
         if (!parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad))
             continue;
 
         if (!sameline(tok->previous, tok)) {
             // find code after comment..
-            tok = tok->next;
-            while (tok && tok->comment) {
-                parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad);
+            if (tok->next) {
                 tok = tok->next;
+
+                while (tok->comment) {
+                    parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad);
+                    if (tok->next) {
+                        tok = tok->next;
+                    } else {
+                        break;
+                    }
+                }
             }
-            if (!tok)
-                break;
         }
 
         if (inlineSuppressions.empty())
             continue;
 
+        // It should never happen
+        if (!tok)
+            continue;
+
         // Relative filename
         std::string relativeFilename(tok->location.file());
         if (settings.relativePaths) {
@@ -166,37 +217,80 @@ static void addinlineSuppressions(const simplecpp::TokenList &tokens, const Sett
         }
         relativeFilename = Path::simplifyPath(relativeFilename);
 
-        // special handling when suppressing { warnings for backwards compatibility
-        const bool thisAndNextLine = tok->previous &&
-                                     tok->previous->previous &&
-                                     tok->next &&
-                                     !sameline(tok->previous->previous, tok->previous) &&
-                                     tok->location.line + 1 == tok->next->location.line &&
-                                     tok->location.fileIndex == tok->next->location.fileIndex &&
-                                     tok->previous->str() == "{";
-
         // Add the suppressions.
         for (Suppressions::Suppression &suppr : inlineSuppressions) {
             suppr.fileName = relativeFilename;
-            suppr.lineNumber = tok->location.line;
-            suppr.thisAndNextLine = thisAndNextLine;
-            suppressions.addSuppression(std::move(suppr));
+
+            if (Suppressions::Type::blockBegin == suppr.type)
+            {
+                inlineSuppressionsBlockBegin.push_back(std::move(suppr));
+            } else if (Suppressions::Type::blockEnd == suppr.type) {
+                bool throwError = true;
+
+                if (!inlineSuppressionsBlockBegin.empty()) {
+                    const Suppressions::Suppression lastBeginSuppression = inlineSuppressionsBlockBegin.back();
+
+                    for (const Suppressions::Suppression &supprBegin : inlineSuppressionsBlockBegin)
+                    {
+                        if (lastBeginSuppression.lineNumber != supprBegin.lineNumber)
+                            continue;
+
+                        if (suppr.symbolName == supprBegin.symbolName && suppr.lineNumber > supprBegin.lineNumber) {
+                            suppr.lineBegin = supprBegin.lineNumber;
+                            suppr.lineEnd = suppr.lineNumber;
+                            suppr.lineNumber = supprBegin.lineNumber;
+                            suppr.type = Suppressions::Type::block;
+                            inlineSuppressionsBlockBegin.remove(supprBegin);
+                            suppressions.addSuppression(std::move(suppr));
+                            throwError = false;
+                            break;
+                        }
+                    }
+                }
+
+                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");
+                }
+            } else if (Suppressions::Type::unique == suppr.type) {
+                // special handling when suppressing { warnings for backwards compatibility
+                const bool thisAndNextLine = tok->previous &&
+                                             tok->previous->previous &&
+                                             tok->next &&
+                                             !sameline(tok->previous->previous, tok->previous) &&
+                                             tok->location.line + 1 == tok->next->location.line &&
+                                             tok->location.fileIndex == tok->next->location.fileIndex &&
+                                             tok->previous->str() == "{";
+
+                suppr.thisAndNextLine = thisAndNextLine;
+                suppr.lineNumber = tok->location.line;
+                suppressions.addSuppression(std::move(suppr));
+            } else if (Suppressions::Type::file == suppr.type) {
+                if (onlyComments)
+                    suppressions.addSuppression(std::move(suppr));
+                else
+                    bad.emplace_back(suppr.fileName, suppr.lineNumber, "File suppression should be at the top of the file");
+            }
         }
     }
+
+    std::for_each(inlineSuppressionsBlockBegin.begin(), inlineSuppressionsBlockBegin.end(), [&](const Suppressions::Suppression & suppr) {
+        bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
+    });
 }
 
 void Preprocessor::inlineSuppressions(const simplecpp::TokenList &tokens, Suppressions &suppressions)
 {
     if (!mSettings.inlineSuppressions)
         return;
     std::list<BadInlineSuppression> err;
-    ::addinlineSuppressions(tokens, mSettings, suppressions, err);
+    ::addInlineSuppressions(tokens, mSettings, suppressions, err);
     for (std::map<std::string,simplecpp::TokenList*>::const_iterator it = mTokenLists.cbegin(); it != mTokenLists.cend(); ++it) {
         if (it->second)
-            ::addinlineSuppressions(*it->second, mSettings, suppressions, err);
+            ::addInlineSuppressions(*it->second, mSettings, suppressions, err);
     }
     for (const BadInlineSuppression &bad : err) {
-        error(bad.location.file(), bad.location.line, bad.errmsg);
+        error(bad.file, bad.line, bad.errmsg);
     }
 }
 

diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp
@@ -165,15 +165,17 @@ std::vector<Suppressions::Suppression> Suppressions::parseMultiSuppressComment(c
             return suppressions;
         }
 
+        const std::string SymbolNameString = "symbolName=";
+
         while (iss) {
             std::string word;
             iss >> word;
             if (!iss)
                 break;
             if (word.find_first_not_of("+-*/%#;") == std::string::npos)
                 break;
-            if (startsWith(word, "symbolName=")) {
-                s.symbolName = word.substr(11);
+            if (startsWith(word, SymbolNameString)) {
+                s.symbolName = word.substr(SymbolNameString.size());
             } else {
                 if (errorMessage && errorMessage->empty())
                     *errorMessage = "Bad multi suppression '" + comment + "'. legal format is cppcheck-suppress[errorId, errorId symbolName=arr, ...]";
@@ -302,22 +304,28 @@ bool Suppressions::Suppression::parseComment(std::string comment, std::string *e
     if (comment.compare(comment.size() - 2, 2, "*/") == 0)
         comment.erase(comment.size() - 2, 2);
 
+    const std::string cppchecksuppress = "cppcheck-suppress";
+
     std::istringstream iss(comment.substr(2));
     std::string word;
     iss >> word;
-    if (word != "cppcheck-suppress")
+    if (word.substr(0, cppchecksuppress.size()) != cppchecksuppress)
         return false;
+
     iss >> errorId;
     if (!iss)
         return false;
+
+    const std::string SymbolNameString = "symbolName=";
+
     while (iss) {
         iss >> word;
         if (!iss)
             break;
         if (word.find_first_not_of("+-*/%#;") == std::string::npos)
             break;
-        if (startsWith(word,"symbolName="))
-            symbolName = word.substr(11);
+        if (startsWith(word, SymbolNameString))
+            symbolName = word.substr(SymbolNameString.size());
         else if (errorMessage && errorMessage->empty())
             *errorMessage = "Bad suppression attribute '" + word + "'. You can write comments in the comment after a ; or //. Valid suppression attributes; symbolName=sym";
     }
@@ -332,10 +340,12 @@ bool Suppressions::Suppression::isSuppressed(const Suppressions::ErrorMessage &e
         return false;
     if (!fileName.empty() && !matchglob(fileName, errmsg.getFileName()))
         return false;
-    if (lineNumber != NO_LINE && lineNumber != errmsg.lineNumber) {
+    if ((Suppressions::Type::unique == type) && (lineNumber != NO_LINE) && (lineNumber != errmsg.lineNumber)) {
         if (!thisAndNextLine || lineNumber + 1 != errmsg.lineNumber)
             return false;
     }
+    if ((Suppressions::Type::block == type) && ((errmsg.lineNumber < lineBegin) || (errmsg.lineNumber > lineEnd)))
+        return false;
     if (!symbolName.empty()) {
         for (std::string::size_type pos = 0; pos < errmsg.symbolNames.size();) {
             const std::string::size_type pos2 = errmsg.symbolNames.find('\n',pos);
@@ -385,15 +395,16 @@ std::string Suppressions::Suppression::getText() const
 bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg, bool global)
 {
     const bool unmatchedSuppression(errmsg.errorId == "unmatchedSuppression");
+    bool return_value = false;
     for (Suppression &s : mSuppressions) {
         if (!global && !s.isLocal())
             continue;
         if (unmatchedSuppression && s.errorId != errmsg.errorId)
             continue;
         if (s.isMatch(errmsg))
-            return true;
+            return_value = true;
     }
-    return false;
+    return return_value;
 }
 
 bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg)
@@ -470,7 +481,15 @@ void Suppressions::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tok
             currLineNr = tok->linenr();
             currFileIdx = tok->fileIndex();
             for (auto &suppression : mSuppressions) {
-                if (!suppression.checked && (suppression.lineNumber == currLineNr) && (suppression.fileName == tokenizer.list.file(tok))) {
+                if (suppression.type == Suppressions::Type::unique) {
+                    if (!suppression.checked && (suppression.lineNumber == currLineNr) && (suppression.fileName == tokenizer.list.file(tok))) {
+                        suppression.checked = true;
+                    }
+                } else if (suppression.type == Suppressions::Type::block) {
+                    if ((!suppression.checked && (suppression.lineBegin <= currLineNr) && (suppression.lineEnd >= currLineNr) && (suppression.fileName == tokenizer.list.file(tok)))) {
+                        suppression.checked = true;
+                    }
+                } else if (!suppression.checked && suppression.fileName == tokenizer.list.file(tok)) {
                     suppression.checked = true;
                 }
             }

diff --git a/lib/suppressions.h b/lib/suppressions.h
@@ -41,6 +41,10 @@ enum class Certainty;
 class CPPCHECKLIB Suppressions {
 public:
 
+    enum class Type {
+        unique, file, block, blockBegin, blockEnd
+    };
+
     struct CPPCHECKLIB ErrorMessage {
         std::size_t hash;
         std::string errorId;
@@ -77,6 +81,26 @@ class CPPCHECKLIB Suppressions {
             return false;
         }
 
+        bool operator==(const Suppression &other) const {
+            if (errorId != other.errorId)
+                return false;
+            if (lineNumber < other.lineNumber)
+                return false;
+            if (fileName != other.fileName)
+                return false;
+            if (symbolName != other.symbolName)
+                return false;
+            if (hash != other.hash)
+                return false;
+            if (type != other.type)
+                return false;
+            if (lineBegin != other.lineBegin)
+                return false;
+            if (lineEnd != other.lineEnd)
+                return false;
+            return true;
+        }
+
         /**
          * Parse inline suppression in comment
          * @param comment the full comment text
@@ -107,6 +131,9 @@ class CPPCHECKLIB Suppressions {
         std::string errorId;
         std::string fileName;
         int lineNumber = NO_LINE;
+        int lineBegin = NO_LINE;
+        int lineEnd = NO_LINE;
+        Type type = Type::unique;
         std::string symbolName;
         std::size_t hash{};
         bool thisAndNextLine{}; // Special case for backwards compatibility: { // cppcheck-suppress something

diff --git a/lib/utils.h b/lib/utils.h
@@ -91,6 +91,11 @@ bool startsWith(const std::string& str, const char (&start)[N])
     return startsWith(str, start, N - 1);
 }
 
+inline bool startsWith(const std::string& str, const std::string& start)
+{
+    return startsWith(str, start.c_str(), start.length());
+}
+
 inline bool endsWith(const std::string &str, char c)
 {
     return !str.empty() && str.back() == c;