MISRA: Fix rules suppression, add tests (#1996)

* MISRA: Fix suppressed rules line numbers

Line numbers represented as strings in lxml ETree, but we use it in
integer comparison later.

* MISRA: Use standard library function instead file_prefix.

* MISRA: Use pytest's capsys for capturing, add suppressions tests.

* travis.yml: Update pytest version

.travis.yml

@@ -18,7 +18,8 @@ env:
 before_install:
 # install needed deps
  - travis_retry sudo apt-get update -qq
- - travis_retry sudo apt-get install -qq python-pygments python-pytest qt5-default qt5-qmake qtbase5-dev qtcreator libxml2-utils libpcre3 gdb unzip wx-common xmlstarlet
+ - travis_retry sudo apt-get install -qq python-pygments qt5-default qt5-qmake qtbase5-dev qtcreator libxml2-utils libpcre3 gdb unzip wx-common xmlstarlet
+ - travis_retry sudo python2 -m pip install --upgrade pytest==4.6.4
 
 matrix:
 # do notify immediately about it when a job of a build fails.

addons/misra.py

@@ -543,30 +543,6 @@ def generateTable():
             print(num[:8] + s)
 
 
-def remove_file_prefix(file_path, prefix):
-    """
-    Remove a file path prefix from a give path.  leftover
-    directory separators at the beginning of a file
-    after the removal are also stripped.
-
-    Example:
-        '/remove/this/path/file.c'
-    with a prefix of:
-        '/remove/this/path'
-    becomes:
-        file.c
-    """
-    result = None
-    if file_path.startswith(prefix):
-        result = file_path[len(prefix):]
-        # Remove any leftover directory separators at the
-        # beginning
-        result = result.lstrip('\\/')
-    else:
-        result = file_path
-    return result
-
-
 class Rule(object):
     """Class to keep rule text and metadata"""
 
@@ -660,9 +636,6 @@ class MisraChecker:
         # List of suppression extracted from the dumpfile
         self.dumpfileSuppressions = None
 
-        # Prefix to ignore when matching suppression files.
-        self.filePrefix = None
-
         # Statistics of all violations suppressed per rule
         self.suppressionStats   = dict()
 
@@ -2037,10 +2010,7 @@ class MisraChecker:
         # Remove any prefix listed in command arguments from the filename.
         filename = None
         if location.file is not None:
-            if self.filePrefix is not None:
+            filename = os.path.basename(location.file)
-                filename = remove_file_prefix(location.file, self.filePrefix)
-            else:
-                filename = location.file
 
         if ruleNum in self.suppressedRules:
             fileDict = self.suppressedRules[ruleNum]
@@ -2090,8 +2060,10 @@ class MisraChecker:
             if res:
                 num1 = int(res.group(2)) * 100
                 ruleNum = num1 + int(res.group(3))
-                self.addSuppressedRule(ruleNum, each.fileName,
+                linenr = None
-                                       each.lineNumber, each.symbolName)
+                if each.lineNumber:
+                    linenr = int(each.lineNumber)
+                self.addSuppressedRule(ruleNum, each.fileName, linenr, each.symbolName)
 
 
     def showSuppressedRules(self):
@@ -2119,14 +2091,6 @@ class MisraChecker:
                 print("  %s" % line)
 
 
-    def setFilePrefix(self, prefix):
-        """
-        Set the file prefix to ignnore from files when matching
-        suppression files
-        """
-        self.filePrefix = prefix
-
-
     def setSuppressionList(self, suppressionlist):
         num1 = 0
         num2 = 0
@@ -2440,7 +2404,6 @@ def get_args():
     parser.add_argument("-generate-table", help=argparse.SUPPRESS, action="store_true")
     parser.add_argument("dumpfile", nargs='*', help="Path of dump file from cppcheck")
     parser.add_argument("--show-suppressed-rules", help="Print rule suppression list", action="store_true")
-    parser.add_argument("-P", "--file-prefix", type=str, help="Prefix to strip when matching suppression file rules")
     parser.add_argument("--cli", help="Addon is executed from Cppcheck", action="store_true")
     return parser.parse_args()
 
@@ -2472,9 +2435,6 @@ def main():
     if args.suppress_rules:
         checker.setSuppressionList(args.suppress_rules)
 
-    if args.file_prefix:
-        checker.setFilePrefix(args.file_prefix)
-
     if args.dumpfile:
         exitCode = 0
         for item in args.dumpfile:

addons/test/test-misra.py

@@ -1,6 +1,7 @@
 # python -m pytest addons/test/test-misra.py
 import json
 import pytest
+import re
 import sys
 try:
     from cStringIO import StringIO
@@ -9,52 +10,33 @@ except ImportError:
 import subprocess
 
 
-class CapturingStdout(object):
-
-    def __enter__(self):
-        self._stdout = sys.stdout
-        sys.stdout = self._stringio = StringIO()
-        self.captured = []
-        return self
-
-    def __exit__(self, *args):
-        self.captured.extend(self._stringio.getvalue().splitlines())
-        del self._stringio  # free up some memory
-        sys.stdout = self._stdout
-
-
-class CapturingStderr(object):
-
-    def __enter__(self):
-        self._stderr = sys.stderr
-        sys.stderr = self._stringio = StringIO()
-        self.captured = []
-        return self
-
-    def __exit__(self, *args):
-        self.captured.extend(self._stringio.getvalue().splitlines())
-        del self._stringio  # free up some memory
-        sys.stderr = self._stderr
-
-
 TEST_SOURCE_FILES = ['./addons/test/misra-test.c']
 
 
+def dump_create(fpath, *argv):
+    cmd = ["./cppcheck", "--dump", "--quiet", fpath] + list(argv)
+    p = subprocess.Popen(cmd)
+    p.communicate()
+    if p.returncode != 0:
+        raise OSError("cppcheck returns error code: %d" % p.returncode)
+    subprocess.Popen(["sync"])
+
+
+def dump_remove(fpath):
+    subprocess.Popen(["rm", "-f", fpath + ".dump"])
+
+
 def setup_module(module):
     for f in TEST_SOURCE_FILES:
-        p = subprocess.Popen(["./cppcheck", "--dump", "--quiet", f])
+        dump_create(f)
-        p.communicate()
-        if p.returncode != 0:
-            raise OSError("cppcheck returns error code: %d" % p.returncode)
-    subprocess.Popen(["sync"])
 
 
 def teardown_module(module):
     for f in TEST_SOURCE_FILES:
-        subprocess.Popen(["rm", "-f", f + ".dump"])
+        dump_remove(f)
 
 
-@pytest.fixture
+@pytest.fixture(scope="function")
 def checker():
     from addons.misra import MisraChecker, MisraSettings, get_args
     args = get_args()
@@ -85,11 +67,10 @@ def test_loadRuleTexts_mutiple_lines(checker):
     assert(checker.ruleTexts[106].text == "Should")
 
 
-def test_verifyRuleTexts(checker):
+def test_verifyRuleTexts(checker, capsys):
     checker.loadRuleTexts("./addons/test/assets/misra_rules_dummy.txt")
-    with CapturingStdout() as output:
+    checker.verifyRuleTexts()
-        checker.verifyRuleTexts()
+    captured = capsys.readouterr().out
-    captured = ''.join(output.captured)
     assert("21.3" not in captured)
     assert("1.3" in captured)
 
@@ -102,23 +83,15 @@ def test_rules_misra_severity(checker):
     assert(checker.ruleTexts[2104].misra_severity == '')
 
 
-def test_extra_output_from_misra_py(checker):
+def test_json_out(checker, capsys):
-    # Extra data generated by misra.py addon are available only through --cli option.
-    checker.loadRuleTexts("./addons/test/assets/misra_rules_dummy.txt")
-    with CapturingStderr() as output:
-        checker.parseDump("./addons/test/misra-test.c.dump")
-    captured = ''.join(output.captured)
-    assert("Mandatory" in captured)
-    assert("Required" in captured)
-    assert("Advisory" in captured)
-
     sys.argv.append("--cli")
     checker.loadRuleTexts("./addons/test/assets/misra_rules_dummy.txt")
-    with CapturingStdout() as output:
+    checker.parseDump("./addons/test/misra-test.c.dump")
-        checker.parseDump("./addons/test/misra-test.c.dump")
+    captured = capsys.readouterr()
+    captured = captured.out.splitlines()
     sys.argv.remove("--cli")
     json_output = {}
-    for line in output.captured:
+    for line in captured:
         try:
             json_line = json.loads(line)
             json_output[json_line['errorId']] = json_line
@@ -129,11 +102,25 @@ def test_extra_output_from_misra_py(checker):
     assert("Advisory" in json_output['c2012-20.1']['extra'])
 
 
-def test_rules_cppcheck_severity(checker):
+def test_rules_cppcheck_severity(checker, capsys):
     checker.loadRuleTexts("./addons/test/assets/misra_rules_dummy.txt")
-    with CapturingStderr() as output:
+    checker.parseDump("./addons/test/misra-test.c.dump")
-        checker.parseDump("./addons/test/misra-test.c.dump")
+    captured = capsys.readouterr().err
-    captured = ''.join(output.captured)
     assert("(error)" not in captured)
     assert("(warning)" not in captured)
     assert("(style)" in captured)
+
+
+def test_rules_suppression(checker, capsys):
+    test_sources = ["addons/test/misra-suppressions1-test.c",
+                    "addons/test/misra-suppressions2-test.c"]
+
+    for src in test_sources:
+        re_suppressed= r"\[%s\:[0-9]+\]" % src
+        dump_remove(src)
+        dump_create(src, "--suppressions-list=addons/test/suppressions.txt")
+        checker.parseDump(src + ".dump")
+        captured = capsys.readouterr().err
+        found = re.search(re_suppressed, captured)
+        assert(found is None)
+        dump_remove(src)