Repository: ericsson/codechecker
Issue: CodeChecker cmd diff does not find source files in 6.18

The reason is most probably that the analysis runs with `--trim-path-prefix` and the paths stored are relative, not absolute. It affects many `cmd diff` formats, including HTML and text output formats.

Example output:
```
[LOW] src/corelib/global/qforeach.h:58:7: class 'QForeachContainer' defines a copy constructor, a copy assignment operator, a move constructor and a move assignment operator but does not define a destructor [cppcoreguidelines-special-member-functions]
[ERROR 2022-01-03 09:44] - Failed to open file src/corelib/global/qforeach.h
```
(EDIT: worth mentioning is that when the output is to HTML, no error is printed to stdout or stderr, and the command looks successful, while it has generated bad report output)

If I `cd` to the source directory, then the command succeeds. It is a regression, it broke when we upgraded to 6.18.


Changed files:
tools/report-converter/codechecker_report_converter/report/__init__.py, web/client/codechecker_client/cmd_line_client.py, web/client/tests/unit/test_report_converter.py, web/tests/functional/diff_remote/__init__.py, web/tests/functional/diff_remote/test_diff_remote.py, web/client/codechecker_client/report_type_converter.py, tools/report-converter/codechecker_report_converter/report/output/plaintext.py

diff --git a/tools/report-converter/codechecker_report_converter/report/__init__.py b/tools/report-converter/codechecker_report_converter/report/__init__.py
index 4c8a91fc..9e58eadd 100644
--- a/tools/report-converter/codechecker_report_converter/report/__init__.py
+++ b/tools/report-converter/codechecker_report_converter/report/__init__.py
@@ -87,7 +87,14 @@ class File:
             self.__content = content
 
     def get_line(self, line: int) -> str:
-        return util.get_line(self.original_path, line)
+        """ Get content from the given line.
+
+        Load file content if it's not loaded yet.
+        """
+        if self.__content is None:
+            return util.get_line(self.original_path, line)
+
+        return self.__content.splitlines(keepends=True)[line - 1]
 
     def trim(self, path_prefixes: Optional[List[str]] = None) -> str:
         """ Removes the longest matching leading path from the file paths. """
@@ -116,7 +123,7 @@ class File:
         return builtins.hash(self.id)
 
     def __repr__(self):
-        return self.to_json()
+        return json.dumps(self.to_json())
 
 
 def get_or_create_file(
diff --git a/tools/report-converter/codechecker_report_converter/report/output/plaintext.py b/tools/report-converter/codechecker_report_converter/report/output/plaintext.py
index 3cb4177a..86d1ef68 100644
--- a/tools/report-converter/codechecker_report_converter/report/output/plaintext.py
+++ b/tools/report-converter/codechecker_report_converter/report/output/plaintext.py
@@ -44,13 +44,13 @@ def __get_source_file_for_analyzer_result_file(
     return None
 
 
-def format_source_line(event: BugPathEvent) -> str:
+def format_main_report(report: Report) -> str:
     """ Format bug path event. """
-    line = event.file.get_line(event.line)
+    line = report.source_line
     if line == '':
         return ''
 
-    marker_line = line[0:(event.column - 1)]
+    marker_line = line[0:(report.column - 1)]
     marker_line = ' ' * (len(marker_line) + marker_line.count('\\t'))
 
     line = line.replace('\\t', '  ')
@@ -158,13 +158,11 @@ def convert(
         reports = sorted(source_file_report_map[file_path],
                          key=lambda report: report.line)
         for report in reports:
-            last_event = report.bug_path_events[-1]
-
             # If file content is changed, do not print the source code comments
             # (if available) and instead of the source line, print a warning
             # message.
             content_is_not_changed = \\
-                last_event.file.original_path not in report.changed_files
+                report.file.original_path not in report.changed_files
 
             if content_is_not_changed:
                 report.dump_source_code_comment_warnings()
@@ -177,7 +175,7 @@ def convert(
                     if source_code_comment.line:
                         output.write(f"{source_code_comment.line.rstrip()}\\n")
 
-                output.write(f"{format_source_line(last_event)}")
+                output.write(f"{format_main_report(report)}")
             else:
                 output.write(InvalidFileContentMsg)
 
diff --git a/web/client/codechecker_client/cmd_line_client.py b/web/client/codechecker_client/cmd_line_client.py
index 21e0bbdd..25950b6f 100644
--- a/web/client/codechecker_client/cmd_line_client.py
+++ b/web/client/codechecker_client/cmd_line_client.py
@@ -771,38 +771,46 @@ def handle_diff_results(args):
 
     context = webserver_context.get_context()
 
+    client = None
+
     file_cache: Dict[int, File] = {}
 
-    def cached_report_file_lookup(file_id):
+    def cached_report_file_lookup(file_id: int, file_path: str) -> File:
         """
-        Get source file data for the given file and caches it in a file cache
-        if file data is not found in the cache. Finally, it returns the source
-        file data from the cache.
+        It will create a file object with the given 'file_path' attribute if
+        file data is not found in the cache already based on the following
+        conditions:
+          - if HTML output is given it will try to get source file data for the
+            given file id from the server and set the content attribute of the
+            file object. File content is needed only by the HTML output
+            converter.
+          - otherwise it will create a file object where file content will not
+            be filled automatically but will be loaded from the host machine
+            when it is needed.
+
+        Finally, it returns the source file data from the cache.
         """
         nonlocal file_cache
 
         if file_id not in file_cache:
-            source = client.getSourceFileData(
-                file_id, True, ttypes.Encoding.BASE64)
-            content = convert.from_b64(source.fileContent)
+            file_cache[file_id] = File(file_path, file_id)
 
-            file_cache[file_id] = File(source.filePath, file_id, content)
+            if 'html' in args.output_format:
+                source = client.getSourceFileData(
+                    file_id, True, ttypes.Encoding.BASE64)
+                file_cache[file_id].content = convert.from_b64(
+                    source.fileContent)
 
         return file_cache[file_id]
 
-    def convert_report_data_to_report(
-        client,
-        reports_data: List[ttypes.ReportData]
-    ) -> List[Report]:
-        """ Convert the given report data list to local reports. """
-        reports = []
-
-        if not reports_data:
-            return reports
-
-        # Get source line contents from the server.
+    def get_source_line_contents(
+        reports: List[ttypes.ReportData]
+    ) -> Dict[int, Dict[int, str]]:
+        """
+        Get source line contents from the server for the given report data.
+        """
         source_lines = defaultdict(set)
-        for report_data in reports_data:
+        for report_data in reports:
             source_lines[report_data.fileId].add(report_data.line)
 
         lines_in_files_requested = []
@@ -811,22 +819,40 @@ def handle_diff_results(args):
                 ttypes.LinesInFilesRequested(fileId=file_id,
                                              lines=source_lines[file_id]))
 
-        source_line_contents = client.getLinesInSourceFileContents(
+        return client.getLinesInSourceFileContents(
             lines_in_files_requested, ttypes.Encoding.BASE64)
 
+    def convert_report_data_to_report(
+        reports_data: List[ttypes.ReportData]
+    ) -> List[Report]:
+        """
+        Convert the given report data list to local reports.
+
+        If one of the given output format is HTML it will get full source file
+        contents from the server otherwise for performance reason it will
+        get only necessarry line contents.
+        """
+        reports = []
+
+        if not reports_data:
+            return reports
+
+        source_line_contents = {}
+        if 'html' not in args.output_format:
+            source_line_contents = get_source_line_contents(reports_data)
+
         # Convert reports data to reports.
         for report_data in reports_data:
-            report = report_type_converter.to_report(report_data)
-
-            # For HTML output we need to override the file and get content
-            # from the server.
-            if 'html' in args.output_format:
-                report.file = cached_report_file_lookup(report_data.fileId)
+            report = report_type_converter.to_report(
+                report_data, cached_report_file_lookup)
 
             report.changed_files = []
             report.source_code_comments = []
-            report.source_line = \\
-                source_line_contents[report_data.fileId][report_data.line]
+
+            if source_line_contents:
+                source_line = convert.from_b64(
+                    source_line_contents[report_data.fileId][report_data.line])
+                report.source_line = f"{source_line}{os.linesep}"
 
             # TODO: get details
             reports.append(report)
@@ -862,8 +888,7 @@ def handle_diff_results(args):
             results = get_diff_base_results(
                 client, args, run_ids, remote_hashes, suppressed_in_code)
 
-            filtered_reports.extend(
-                convert_report_data_to_report(client, results))
+            filtered_reports.extend(convert_report_data_to_report(results))
         elif diff_type == ttypes.DiffType.UNRESOLVED:
             # Get remote hashes which can be found in the remote run and in the
             # local report directory.
@@ -889,8 +914,7 @@ def handle_diff_results(args):
                 for result in results:
                     filtered_report_hashes.discard(result.bugHash)
 
-                filtered_reports.extend(
-                    convert_report_data_to_report(client, results))
+                filtered_reports.extend(convert_report_data_to_report(results))
         elif diff_type == ttypes.DiffType.RESOLVED:
             # Get remote hashes which can be found in the remote run and in the
             # local report directory.
@@ -952,8 +976,7 @@ def handle_diff_results(args):
             results = get_diff_base_results(
                 client, args, run_ids, remote_hashes, suppressed_in_code)
 
-            filtered_reports.extend(
-                convert_report_data_to_report(client, results))
+            filtered_reports.extend(convert_report_data_to_report(results))
 
         return filtered_reports, filtered_report_hashes, run_names
 
@@ -995,7 +1018,7 @@ def handle_diff_results(args):
             client, base_ids, constants.MAX_QUERY_SIZE, 0, sort_mode,
             report_filter, cmp_data, False)
 
-        reports = convert_report_data_to_report(client, all_results)
+        reports = convert_report_data_to_report(all_results)
         return reports, base_run_names, new_run_names
 
     def get_diff_local_dirs(
@@ -1187,7 +1210,6 @@ def handle_diff_results(args):
         LOG.info("Matching local baseline files (--newname): %s",
                  ', '.join(newname_baseline_files))
 
-    client = None
     # We set up the client if we are not comparing two local report directories
     # or baseline files.
     if basename_run_names or newname_run_names:
diff --git a/web/client/codechecker_client/report_type_converter.py b/web/client/codechecker_client/report_type_converter.py
index 694c5ccb..fecd45e8 100644
--- a/web/client/codechecker_client/report_type_converter.py
+++ b/web/client/codechecker_client/report_type_converter.py
@@ -8,18 +8,22 @@
 
 """ Convert between Report type and thrift ReportData type. """
 
+from typing import Callable
 from codechecker_api.codeCheckerDBAccess_v6.ttypes import ReportData, Severity
 
 from codechecker_report_converter.report import File, Report
 
 
-def to_report(report: ReportData) -> Report:
+def to_report(
+    report: ReportData,
+    get_file: Callable[[int, str], File]
+) -> Report:
     """ Create a Report object from the given thrift report data. """
     severity = Severity._VALUES_TO_NAMES[report.severity] \\
         if report.severity else 'UNSPECIFIED'
 
     return Report(
-        File(report.checkedFile),
+        get_file(report.fileId, report.checkedFile),
         report.line,
         report.column,
         report.checkerMsg,
diff --git a/web/client/tests/unit/test_report_converter.py b/web/client/tests/unit/test_report_converter.py
index 8ca9bfc2..cab98cdf 100644
--- a/web/client/tests/unit/test_report_converter.py
+++ b/web/client/tests/unit/test_report_converter.py
@@ -59,7 +59,11 @@ class ReportTypeConverterTest(unittest.TestCase):
             bugPathLength=5,
         )
 
-        report = report_type_converter.to_report(rep_data)
+        def get_file(file_id: int, file_path: str) -> File:
+            """ Get file object for the given report. """
+            return File(file_path, file_id)
+
+        report = report_type_converter.to_report(rep_data, get_file)
         self.assertEqual(report.checker_name, rep_data.checkerId)
         self.assertEqual(report.report_hash, rep_data.bugHash)
         self.assertEqual(report.file.path, rep_data.checkedFile)
diff --git a/web/tests/functional/diff_remote/__init__.py b/web/tests/functional/diff_remote/__init__.py
index 77921067..f3b23282 100644
--- a/web/tests/functional/diff_remote/__init__.py
+++ b/web/tests/functional/diff_remote/__init__.py
@@ -75,7 +75,8 @@ def setup_package():
         'skip_list_file': skip_list_file,
         'check_env': test_env,
         'workspace': TEST_WORKSPACE,
-        'checkers': []
+        'checkers': [],
+        'trim_path_prefix': TEST_WORKSPACE
     }
 
     # Start or connect to the running CodeChecker server and get connection
diff --git a/web/tests/functional/diff_remote/test_diff_remote.py b/web/tests/functional/diff_remote/test_diff_remote.py
index d7b47fa7..0c712ab7 100644
--- a/web/tests/functional/diff_remote/test_diff_remote.py
+++ b/web/tests/functional/diff_remote/test_diff_remote.py
@@ -14,6 +14,7 @@ Test the compraison of two remote (in the database) runs.
 
 import os
 import re
+import shutil
 import unittest
 from datetime import datetime, timedelta
 
@@ -21,6 +22,8 @@ from codechecker_api.codeCheckerDBAccess_v6.ttypes import CompareData, \\
     DiffType, Order, ReportFilter, ReviewStatus, RunHistoryFilter, \\
     RunSortMode, RunSortType, Severity
 
+from codechecker_report_converter.report import InvalidFileContentMsg
+
 from libtest import env
 from libtest.codechecker import get_diff_results
 from libtest.debug_printer import print_run_results
@@ -48,23 +51,23 @@ class DiffRemote(unittest.TestCase):
     def setUp(self):
 
         # TEST_WORKSPACE is automatically set by test package __init__.py .
-        test_workspace = os.environ['TEST_WORKSPACE']
+        self.test_workspace = os.environ['TEST_WORKSPACE']
 
         test_class = self.__class__.__name__
-        print('Running ' + test_class + ' tests in ' + test_workspace)
+        print('Running ' + test_class + ' tests in ' + self.test_workspace)
 
         # Get the test configuration from the prepared int the test workspace.
-        self.test_cfg = env.import_test_cfg(test_workspace)
+        self.test_cfg = env.import_test_cfg(self.test_workspace)
 
         # Get the clang version which is tested.
         self._clang_to_test = env.clang_to_test()
 
         # Get the test project configuration from the prepared test workspace.
-        self._testproject_data = env.setup_test_proj_cfg(test_workspace)
+        self._testproject_data = env.setup_test_proj_cfg(self.test_workspace)
         self.assertIsNotNone(self._testproject_data)
 
         # Setup a viewer client to test viewer API calls.
-        self._cc_client = env.setup_viewer_client(test_workspace)
+        self._cc_client = env.setup_viewer_client(self.test_workspace)
         self.assertIsNotNone(self._cc_client)
 
         # Get the CodeChecker cmd if needed for the tests.
@@ -72,7 +75,7 @@ class DiffRemote(unittest.TestCase):
 
         # Get the run names which belong to this test.
         # Name order matters from __init__ !
-        run_names = env.get_run_names(test_workspace)
+        run_names = env.get_run_names(self.test_workspace)
 
         sort_mode = RunSortMode(RunSortType.DATE, Order.ASC)
         runs = self._cc_client.getRunData(None, None, 0, sort_mode)
@@ -706,3 +709,39 @@ class DiffRemote(unittest.TestCase):
                              '--unresolved', 'json', ["--url", self._url])
 
         self.assertNotEqual(len(unresolved_results[0]), 0)
+
+    def test_source_line_content(self):
+        """
+        Check that line / file contents are set properly for different
+        output types.
+        """
+        base_run_name = self._test_runs[0].name
+        new_run_name = self._test_runs[1].name
+
+        html_reports = os.path.join(self.test_workspace, "html_reports")
+
+        base_run_names = [base_run_name, new_run_name]
+        new_run_names = [new_run_name, base_run_name]
+        extra_args = [
+            "--url", self._url,
+            "--file", "*/divide_zero.cpp",
+            "--checker-name", "core.DivideZero",
+            "--output", "plaintext", "html",
+            '--export-dir', html_reports]
+
+        # Check plain text output.
+        out, _, _ = \\
+            get_diff_results(base_run_names, new_run_names,
+                             '--unresolved', None, extra_args)
+
+        lines = out.split(os.linesep)
+        for idx, line in enumerate(lines):
+            if '[core.DivideZero]' in line:
+                self.assertTrue(lines[idx + 1].strip(), "Invalid line content")
+
+        # Check HTML output
+        for file_path in os.listdir(html_reports):
+            with open(os.path.join(html_reports, file_path)) as f:
+                self.assertNotIn(InvalidFileContentMsg, f.read())
+
+        shutil.rmtree(html_reports, ignore_errors=True)