Fix #15028 (release: check premium ids)

createrelease

@@ -23,14 +23,12 @@
 # cd ~/cppchecksolutions/addon/coverage
 # CPPCHECK_REPO=~/cppchecksolutions/cppcheck python3 coverage.py --code
 #
-# Check every isPremiumEnabled call: TODO write helper script
-# - every id should be in --errorlist
-#   git grep 'isPremiumEnabled[(]"' | sed 's/.*isPremiumEnabled[(]"//' | sed 's/".*//' | sort | uniq > ids1.txt
-#   ./cppcheck --errorlist | grep ' id="' | sed 's/.* id="//' | sed 's/".*//' | sort | uniq > ids2.txt
-#   diff -y ids1.txt ids2.txt
-# - premiumaddon: check coverage.py
-#   python3 coverage.py --id ; sort ids-*.txt | uniq > ~/cppcheck/ids3.txt
-#   diff -y ids2.txt ids3.txt
+# Check that every premium check id is a real, known Cppcheck error id: every
+# id passed to isPremiumEnabled(...) and every id referenced by the premium
+# addon's MISRA/CERT/AUTOSAR/CWE coverage mapping (coverage.py) must show up
+# in `cppcheck --errorlist`, otherwise the id was renamed/removed and the
+# premium check or compliance mapping is silently broken.
+# tools/release-check-premium-ids.py
 #
 # Windows installer:
 # - ensure latest build was successful

lib/checkclass.cpp

@@ -3841,6 +3841,18 @@ const Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLEleme
     return fileInfo;
 }
 
+static ErrorMessage oneDefinitionRuleViolationErrorMessage(std::list<ErrorMessage::FileLocation> locationList, const std::string &file0, const std::string &symbolName)
+{
+    return ErrorMessage(std::move(locationList),
+                        file0,
+                        Severity::error,
+                        "$symbol:" + symbolName +
+                        "\nThe one definition rule is violated, different classes/structs have the same name '$symbol'",
+                        "ctuOneDefinitionRuleViolation",
+                        CWE_ONE_DEFINITION_RULE,
+                        Certainty::normal);
+}
+
 bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::list<const Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger)
 {
     (void)ctu;
@@ -3879,15 +3891,7 @@ bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::list<c
             locationList.emplace_back(nameLoc.fileName, nameLoc.lineNumber, nameLoc.column);
             locationList.emplace_back(it->second.fileName, it->second.lineNumber, it->second.column);
 
-            const ErrorMessage errmsg(std::move(locationList),
-                                      fi->file0,
-                                      Severity::error,
-                                      "$symbol:" + nameLoc.className +
-                                      "\nThe one definition rule is violated, different classes/structs have the same name '$symbol'",
-                                      "ctuOneDefinitionRuleViolation",
-                                      CWE_ONE_DEFINITION_RULE,
-                                      Certainty::normal);
-            errorLogger.reportErr(errmsg);
+            errorLogger.reportErr(oneDefinitionRuleViolationErrorMessage(std::move(locationList), fi->file0, nameLoc.className));
 
             foundErrors = true;
         }
@@ -3968,5 +3972,5 @@ void CheckClass::getErrorMessages(ErrorLogger& errorLogger, const Settings &sett
     c.virtualFunctionCallInConstructorError(nullptr, std::list<const Token *>(), "f");
     c.thisUseAfterFree(nullptr, nullptr, nullptr);
     c.unsafeClassRefMemberError(nullptr, "UnsafeClass::var");
-    // TODO: ctuOneDefinitionRuleViolation
+    errorLogger.reportErr(oneDefinitionRuleViolationErrorMessage({}, "", "classname"));
 }

lib/checkcondition.cpp

@@ -1516,9 +1516,7 @@ void CheckConditionImpl::clarifyConditionError(const Token *tok, bool assign, bo
 
 void CheckConditionImpl::alwaysTrueFalse()
 {
-    const bool pedantic = mSettings.isPremiumEnabled("alwaysTrue") ||
-                          mSettings.isPremiumEnabled("alwaysFalse") ||
-                          mSettings.isPremiumEnabled("knownConditionTrueFalse");
+    const bool pedantic = mSettings.isPremiumEnabled("knownConditionTrueFalse");
 
     if (!pedantic && !mSettings.severity.isEnabled(Severity::style))
         return;

lib/checkother.cpp

@@ -4513,18 +4513,16 @@ void CheckOtherImpl::checkComparePointers()
             if (const Token* parent1 = getParentLifetime(v1.tokvalue, mSettings.library))
                 if (var2 == parent1->variable())
                     continue;
-            comparePointersError(tok, &v1, &v2);
+            comparePointersError(tok, &v1, &v2, Token::simpleMatch(tok, "-"));
         }
     }
 }
 
-void CheckOtherImpl::comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2)
+void CheckOtherImpl::comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2, bool subtract)
 {
     ErrorPath errorPath;
-    std::string verb = "Comparing";
-    if (Token::simpleMatch(tok, "-"))
-        verb = "Subtracting";
-    const char * const id = (verb[0] == 'C') ? "comparePointers" : "subtractPointers";
+    const std::string verb = subtract ? "Subtracting" : "Comparing";
+    const char * const id = subtract ? "subtractPointers" : "comparePointers";
     if (v1) {
         errorPath.emplace_back(v1->tokvalue->variable()->nameToken(), "Variable declared here.");
         errorPath.insert(errorPath.end(), v1->errorPath.cbegin(), v1->errorPath.cend());
@@ -4993,8 +4991,8 @@ void CheckOther::getErrorMessages(ErrorLogger& errorLogger, const Settings &sett
     c.shadowError(nullptr, "local variable", nullptr, "member");
     c.knownArgumentError(nullptr, nullptr, nullptr, "x", false);
     c.knownPointerToBoolError(nullptr, nullptr);
-    c.comparePointersError(nullptr, nullptr, nullptr);
-    // TODO: subtractPointers
+    c.comparePointersError(nullptr, nullptr, nullptr, false);
+    c.comparePointersError(nullptr, nullptr, nullptr, true);
     c.redundantAssignmentError(nullptr, nullptr, "var", false);
     c.redundantInitializationError(nullptr, nullptr, "var", false);
     c.redundantContinueError(nullptr);

lib/checkother.h

@@ -325,7 +325,7 @@ class CPPCHECKLIB CheckOtherImpl : public CheckImpl {
     void shadowError(const Token *shadows, const std::string &shadowsType, const Token *shadowed, const std::string &shadowedType);
     void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool isVariableExpressionHidden);
     void knownPointerToBoolError(const Token* tok, const ValueFlow::Value* value);
-    void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
+    void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2, bool subtract);
     void checkModuloOfOneError(const Token *tok);
     void unionZeroInitError(const Token *tok, const UnionMember& largestMember);
 

lib/checkuninitvar.cpp

@@ -1557,7 +1557,7 @@ void CheckUninitVarImpl::uninitdataError(const Token *tok, const std::string &va
 
 void CheckUninitVarImpl::uninitvarError(const Token *tok, const std::string &varname, ErrorPath errorPath)
 {
-    if (diag(tok))
+    if (tok && diag(tok))
         return;
     errorPath.emplace_back(tok, "");
     reportError(std::move(errorPath),
@@ -1572,7 +1572,7 @@ void CheckUninitVarImpl::uninitvarError(const Token* tok, const ValueFlow::Value
 {
     if (!mSettings.isEnabled(&v))
         return;
-    if (diag(tok))
+    if (tok && diag(tok))
         return;
     const Token* ltok = tok;
     if (tok && Token::simpleMatch(tok->astParent(), ".") && astIsRHS(tok))
@@ -1810,7 +1810,8 @@ void CheckUninitVar::getErrorMessages(ErrorLogger& errorLogger, const Settings&
 
     ValueFlow::Value v{};
 
-    c.uninitvarError(nullptr, v); // TODO: does not produce any output
+    c.uninitvarError(nullptr, v);
+    c.uninitvarError(nullptr, "varname", ErrorPath{});
     c.uninitdataError(nullptr, "varname");
     c.uninitStructMemberError(nullptr, "a.b");
 }

lib/preprocessor.cpp

@@ -1067,6 +1067,7 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se
     preprocessor.missingInclude(loc, "", SystemHeader);
     preprocessor.error(loc, "message", simplecpp::Output::ERROR);
     preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
+    preprocessor.error(loc, "message", simplecpp::Output::DIRECTIVE_AS_MACRO_PARAMETER);
     preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
     preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
     preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND);

tools/release-check-premium-ids.py

@@ -0,0 +1,114 @@
+#!/usr/bin/env python3
+#
+# Automates the release checklist item in `createrelease`:
+#
+#   # Check every isPremiumEnabled call: TODO write helper script
+#   # - every id should be in --errorlist
+#   # - premiumaddon: check coverage.py
+#
+# Two checks are performed:
+#
+#  1. Every id passed to isPremiumEnabled("...") in lib/ must be a real,
+#     known Cppcheck error id (i.e. it must show up in `cppcheck
+#     --errorlist`). Otherwise the premium check can never actually be
+#     turned on/off since its id does not exist.
+#
+#  2. Every Cppcheck id referenced by the premium addon's compliance
+#     mapping tables (MISRA/CERT/AUTOSAR/CWE -> Cppcheck id, generated by
+#     `coverage.py --id`) must also show up in `cppcheck --errorlist`.
+#     Otherwise the mapping refers to an id that has been renamed or
+#     removed.
+#
+# A warning is printed for every id that fails a check; exit status is 1
+# if any warning was printed, 0 otherwise.
+#
+# Usage:
+#   tools/release-check-premium-ids.py [path-to-addon/coverage]
+#
+# The addon/coverage directory defaults to ../addon/coverage (relative to
+# this repo, i.e. a checkout of the premium addon repo next to this one).
+# If it can't be found, check 2 is skipped.
+
+import argparse
+import glob
+import os
+import re
+import subprocess
+import sys
+
+REPO_DIR = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
+CPPCHECK_BIN = os.path.join(REPO_DIR, 'cppcheck')
+
+ID_RE = re.compile(r' id="([^"]*)"')
+IS_PREMIUM_ENABLED_RE = re.compile(r'isPremiumEnabled\s*\(\s*"([^"]*)"')
+
+warning_count = 0
+
+
+def warn(message):
+    global warning_count
+    warning_count += 1
+    print('warning: ' + message, file=sys.stderr)
+
+
+def get_errorlist_ids():
+    out = subprocess.check_output([CPPCHECK_BIN, '--errorlist'], universal_newlines=True)
+    return set(ID_RE.findall(out))
+
+
+def get_is_premium_enabled_ids():
+    ids = set()
+    for filename in glob.glob(os.path.join(REPO_DIR, 'lib', '*.cpp')) + glob.glob(os.path.join(REPO_DIR, 'lib', '*.h')):
+        with open(filename, 'rt', encoding='utf-8') as f:
+            ids.update(IS_PREMIUM_ENABLED_RE.findall(f.read()))
+    return ids
+
+
+def get_addon_coverage_ids(addon_coverage_dir):
+    coverage_py = os.path.join(addon_coverage_dir, 'coverage.py')
+    if not os.path.isfile(coverage_py):
+        print(f"(skipped premium addon coverage.py check: '{coverage_py}' not found)", file=sys.stderr)
+        return None
+
+    ids_files = glob.glob(os.path.join(addon_coverage_dir, 'ids-*.txt'))
+    for f in ids_files:
+        os.remove(f)
+
+    env = dict(os.environ, CPPCHECK_REPO=REPO_DIR)
+    subprocess.check_call(['python3', 'coverage.py', '--id'], cwd=addon_coverage_dir, env=env, stdout=subprocess.DEVNULL)
+
+    ids = set()
+    ids_files = glob.glob(os.path.join(addon_coverage_dir, 'ids-*.txt'))
+    for filename in ids_files:
+        with open(filename, 'rt', encoding='utf-8') as f:
+            ids.update(line.strip() for line in f if line.strip())
+        os.remove(filename)
+    return ids
+
+
+def main():
+    parser = argparse.ArgumentParser(description='Check that isPremiumEnabled() ids and premium addon coverage ids are known Cppcheck error ids.')
+    parser.add_argument('addon_coverage_dir', nargs='?', default=os.path.join(REPO_DIR, '..', 'addon', 'coverage'),
+                         help='path to the premium addon coverage/ directory (default: ../addon/coverage)')
+    args = parser.parse_args()
+
+    if not os.access(CPPCHECK_BIN, os.X_OK):
+        print(f"error: '{CPPCHECK_BIN}' not found or not executable, build it first (make -j$(nproc))", file=sys.stderr)
+        return 1
+
+    errorlist_ids = get_errorlist_ids()
+
+    for id_ in sorted(get_is_premium_enabled_ids() - errorlist_ids):
+        warn(f'isPremiumEnabled("{id_}") but there is no such id in --errorlist')
+
+    addon_coverage_dir = os.path.realpath(args.addon_coverage_dir)
+    addon_ids = get_addon_coverage_ids(addon_coverage_dir)
+    if addon_ids is not None:
+        for id_ in sorted(addon_ids - errorlist_ids):
+            warn(f'premium addon coverage.py references id "{id_}" but there is no such id in --errorlist')
+
+    return 1 if warning_count else 0
+
+
+if __name__ == '__main__':
+    sys.exit(main())