fix plist parsing for skip files

The introduced new plist xml parsing method with lxml
was not introduced for the use cases where a skip file
was used. In case of a skip file the report plist files
can be rewritten (the reports based on the skip file content)
are removed.

Previous tests did not fail because a second skip is done
at the server side during storage and the server skip tests
worked on the stored reports.

analyzer/tests/functional/skip/__init__.py

@@ -0,0 +1,44 @@
+# coding=utf-8
+# -----------------------------------------------------------------------------
+#                     The CodeChecker Infrastructure
+#   This file is distributed under the University of Illinois Open Source
+#   License. See LICENSE.TXT for details.
+# -----------------------------------------------------------------------------
+
+"""Setup for the skip test for the analyze command.
+"""
+from __future__ import print_function
+from __future__ import division
+from __future__ import absolute_import
+
+import os
+import shutil
+
+from libtest import env
+
+
+# Test workspace should be initialized in this module.
+TEST_WORKSPACE = None
+
+
+def setup_package():
+    """Setup the environment for the tests."""
+
+    global TEST_WORKSPACE
+    TEST_WORKSPACE = env.get_workspace('skip')
+
+    report_dir = os.path.join(TEST_WORKSPACE, 'reports')
+    os.makedirs(report_dir)
+
+    os.environ['TEST_WORKSPACE'] = TEST_WORKSPACE
+
+
+def teardown_package():
+    """Delete the workspace associated with this test"""
+
+    # TODO: If environment variable is set keep the workspace
+    # and print out the path.
+    global TEST_WORKSPACE
+
+    print("Removing: " + TEST_WORKSPACE)
+    shutil.rmtree(TEST_WORKSPACE)

analyzer/tests/functional/skip/test_files/Makefile

@@ -0,0 +1,14 @@
+OBJS = $(SRCS:.cpp=.o)
+
+CXXFLAGS = -Wno-all -Wno-extra -Wno-division-by-zero
+
+SRCS = skip_header.cpp \
+		file_to_be_skipped.cpp
+
+.cpp.o:
+	$(CXX) $(CXXFLAGS) -c $<  -o $@
+
+all: $(OBJS)
+
+clean:
+	rm -rf *.o

analyzer/tests/functional/skip/test_files/file_to_be_skipped.cpp

@@ -0,0 +1,12 @@
+// -----------------------------------------------------------------------------
+//                     The CodeChecker Infrastructure
+//   This file is distributed under the University of Illinois Open Source
+//   License. See LICENSE.TXT for details.
+// -----------------------------------------------------------------------------
+
+// This file is to be included in a skip list file.
+
+void skipped_test(int z) {
+  if (z == 0)
+    int x = 1 / z; // warn
+}

analyzer/tests/functional/skip/test_files/skip.h

@@ -0,0 +1,11 @@
+// -----------------------------------------------------------------------------
+//                     The CodeChecker Infrastructure
+//   This file is distributed under the University of Illinois Open Source
+//   License. See LICENSE.TXT for details.
+// -----------------------------------------------------------------------------
+
+int null_div(int z) {
+    int x = z / 0; // warn
+    return x;
+}
+

analyzer/tests/functional/skip/test_files/skip_header.cpp

@@ -0,0 +1,18 @@
+// -----------------------------------------------------------------------------
+//                     The CodeChecker Infrastructure
+//   This file is distributed under the University of Illinois Open Source
+//   License. See LICENSE.TXT for details.
+// -----------------------------------------------------------------------------
+
+#include "skip.h"
+
+int test() {
+    int x = null_div(42);
+    return x;
+}
+
+int test1(int i) {
+    int y = i / 0; // warn
+    return y;
+}
+

analyzer/tests/functional/skip/test_files/skipfile

@@ -0,0 +1,2 @@
+-*file_to_be_skipped.cpp
+-*skip.h

analyzer/tests/functional/skip/test_skip.py

@@ -0,0 +1,108 @@
+#
+# -----------------------------------------------------------------------------
+#                     The CodeChecker Infrastructure
+#   This file is distributed under the University of Illinois Open Source
+#   License. See LICENSE.TXT for details.
+# -----------------------------------------------------------------------------
+
+"""
+Test skipping the analysis of a file and the removal
+of skipped reports from the report files.
+"""
+from __future__ import print_function
+from __future__ import division
+from __future__ import absolute_import
+
+import os
+import plistlib
+import subprocess
+import unittest
+import shutil
+
+from libtest import env
+
+
+class TestSkip(unittest.TestCase):
+    _ccClient = None
+
+    def setUp(self):
+
+        # TEST_WORKSPACE is automatically set by test package __init__.py .
+        self.test_workspace = os.environ['TEST_WORKSPACE']
+
+        test_class = self.__class__.__name__
+        print('Running ' + test_class + ' tests in ' + self.test_workspace)
+
+        # Get the CodeChecker cmd if needed for the tests.
+        self._codechecker_cmd = env.codechecker_cmd()
+        self.report_dir = os.path.join(self.test_workspace, "reports")
+        self.test_dir = os.path.join(os.path.dirname(__file__), 'test_files')
+        # Change working dir to testfile dir so CodeChecker can be run easily.
+        self.__old_pwd = os.getcwd()
+        os.chdir(self.test_dir)
+
+    def tearDown(self):
+        """Restore environment after tests have ran."""
+        os.chdir(self.__old_pwd)
+        if os.path.isdir(self.report_dir):
+            shutil.rmtree(self.report_dir)
+
+    def test_skip(self):
+        """
+        """
+        build_json = os.path.join(self.test_workspace, "build.json")
+
+        clean_cmd = ["make", "clean"]
+        out = subprocess.check_output(clean_cmd)
+        print(out)
+
+        # Create and run log command.
+        log_cmd = [self._codechecker_cmd, "log", "-b", "make",
+                   "-o", build_json]
+        out = subprocess.check_output(log_cmd)
+        print(out)
+        # Create and run analyze command.
+        analyze_cmd = [self._codechecker_cmd, "analyze", build_json,
+                       "--analyzers", "clangsa", "--verbose", "debug",
+                       "--ignore", "skipfile", "-o", self.report_dir]
+
+        process = subprocess.Popen(
+            analyze_cmd, stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE, cwd=self.test_dir)
+        out, err = process.communicate()
+
+        print(out)
+        print(err)
+        errcode = process.returncode
+        self.assertEquals(errcode, 0)
+
+        # Check if file is skipped.
+        report_dir_files = os.listdir(self.report_dir)
+        for f in report_dir_files:
+            self.assertFalse("file_to_be_skipped.cpp" in f)
+
+        # Check if report from the report file is removed.
+        report_dir_files = os.listdir(self.report_dir)
+        report_file_to_check = None
+        for f in report_dir_files:
+            if "skip_header.cpp" in f:
+                report_file_to_check = os.path.join(self.report_dir, f)
+                break
+
+        self.assertIsNotNone(report_file_to_check,
+                             "Report file should be generated.")
+        report_data = plistlib.readPlist(report_file_to_check)
+        files = report_data['files']
+
+        skiped_file_index = None
+        for i, f in enumerate(files):
+            if "skip.h" in f:
+                skiped_file_index = i
+                break
+
+        for diag in report_data['diagnostics']:
+            self.assertNotEqual(diag['location']['file'],
+                                skiped_file_index,
+                                "Found a location which points to "
+                                "skiped file, this report should "
+                                "have been removed.")

codechecker_common/plist_parser.py

@@ -40,7 +40,7 @@
 import os
 import sys
 import traceback
-from plistlib import PlistParser, writePlist, readPlist
+from plistlib import PlistParser, writePlist, writePlistToString, readPlist
 from xml.parsers.expat import ExpatError
 
 from codechecker_common import util
@@ -97,7 +97,7 @@ def parse(self, fileobj):
         return self.root
 
 
-def read_plist(path):
+def parse_plist(plist_file_obj):
     """
     Read a .plist file. Return the unpacked root object (which usually is a
     dictionary).
@@ -107,14 +107,12 @@ def read_plist(path):
     """
     try:
         imp.find_module('lxml')
-
-        with io.open(path, 'r') as data:
-            parser = LXMLPlistParser()
-            return parser.parse(data)
+        parser = LXMLPlistParser()
+        return parser.parse(plist_file_obj)
     except ImportError:
         LOG.debug("lxml library is not available. Use plistlib to parse plist "
                   "files.")
-        return readPlist(path)
+        return readPlist(plist_file_obj)
 
 
 def get_checker_name(diagnostic, path=""):
@@ -145,7 +143,7 @@ def get_report_hash(diagnostic, source_file):
     return report_hash
 
 
-def parse_plist(path, source_root=None, allow_plist_update=True):
+def parse_plist_file(path, source_root=None, allow_plist_update=True):
     """
     Parse the reports from a plist file.
     One plist file can contain multiple reports.
@@ -155,7 +153,8 @@ def parse_plist(path, source_root=None, allow_plist_update=True):
     reports = []
     files = []
     try:
-        plist = read_plist(path)
+        with io.open(path, 'r') as plist_file_obj:
+            plist = parse_plist(plist_file_obj)
 
         files = plist['files']
 
@@ -287,7 +286,7 @@ def fids_in_path(report_data, file_ids_to_remove):
     return all_fids, kept_diagnostics
 
 
-def remove_report_from_plist(plist_content, skip_handler):
+def remove_report_from_plist(plist_file_obj, skip_handler):
     """
     Parse the original plist content provided by the analyzer
     and return a new plist content where reports were removed
@@ -298,8 +297,9 @@ def remove_report_from_plist(plist_content, skip_handler):
     diagnostic section (control, event ...) nodes should be
     re indexed to use the proper file array indexes!!!
     """
+    report_data = None
     try:
-        report_data = plistlib.readPlistFromString(plist_content)
+        report_data = parse_plist(plist_file_obj)
     except (ExpatError, TypeError, AttributeError) as ex:
         LOG.error("Failed to parse plist content, "
                   "keeping the original version")
@@ -317,7 +317,7 @@ def remove_report_from_plist(plist_content, skip_handler):
         _, kept_diagnostics = fids_in_path(report_data, file_ids_to_remove)
         report_data['diagnostics'] = kept_diagnostics
 
-        res = plistlib.writePlistToString(report_data)
+        res = writePlistToString(report_data)
         return res
 
     except KeyError:
@@ -331,9 +331,10 @@ def skip_report_from_plist(plist_file, skip_handler):
     Rewrites the provided plist file where reports
     were removed if they should be skipped.
     """
-    with open(plist_file, 'r+') as plist:
-        new_plist_content = remove_report_from_plist(plist.read(),
+    with io.open(plist_file, 'r+') as plist:
+        new_plist_content = remove_report_from_plist(plist,
                                                      skip_handler)
+        new_plist_content = new_plist_content.decode("utf8")
         plist.seek(0)
         plist.write(new_plist_content)
         plist.truncate()
@@ -467,7 +468,7 @@ def parse(plist_file):
         """
         files, reports = [], []
         try:
-            files, reports = parse_plist(plist_file)
+            files, reports = parse_plist_file(plist_file)
         except Exception as ex:
             traceback.print_stack()
             LOG.error('The generated plist is not valid!')

web/client/codechecker_client/cmd/store.py

@@ -230,7 +230,7 @@ def collect_file_hashes_from_plist(plist_file):
         source_file_mod_times = {}
         missing_files = []
         try:
-            files, reports = plist_parser.parse_plist(plist_file)
+            files, reports = plist_parser.parse_plist_file(plist_file)
 
             for f in files:
                 if not os.path.isfile(f):

web/client/codechecker_client/cmd_line_client.py

@@ -459,7 +459,7 @@ def get_report_dir_results(reportdir):
                 file_path = os.path.join(reportdir, filename)
                 LOG.debug("Parsing: %s", file_path)
                 try:
-                    files, reports = plist_parser.parse_plist(file_path)
+                    files, reports = plist_parser.parse_plist_file(file_path)
                     for report in reports:
                         path_hash = get_report_path_hash(report, files)
                         if path_hash in processed_path_hashes:

web/server/codechecker_server/api/report_server.py

@@ -2288,7 +2288,7 @@ def checker_is_unavailable(checker_name):
             LOG.debug("Parsing input file '%s'", f)
 
             try:
-                files, reports = plist_parser.parse_plist(
+                files, reports = plist_parser.parse_plist_file(
                     os.path.join(report_dir, f), source_root)
             except Exception as ex:
                 LOG.error('Parsing the plist failed: %s', str(ex))