PVS-Studio Static Code Analyzer support #4356

feeelin · 2024-09-30T11:23:31Z

Hello! We added support for JSON report of PVS-Studio static analyzer in report-converter so that you could view the analyzer's report in CodeChecker!

Usage:
report-converter -t pvs-studio -o <path to converted reports> <path to PVS-Studio's report.json>

Was added support for PVS-Studio JSON report format in CodeChecker report converter

vodorok

Hi @feeelin,

Thanks for the contribution!
Please add tests for your newly implemented report converter.

Look for already existing converter tests under: codechecker/tools/report-converter/tests/unit/analyzers.
You should create a separate pvs_studio_output_test_files in this folder, containing the sample outputs of your analyzer, the expected .plist (CodeChecker) output, and the source that's the basis of the analyzer output.
You can write your test implementation by using these sample files in test_pvs_studio_parser.py.

feeelin

I have added tests for the PVS-Studio report converter by analogy with other tools.

vodorok

Thank you, I see a few linter errors in the parser source and the test source..

https://github.com/Ericsson/codechecker/actions/runs/11106888906/job/30856471518?pr=4356

pycodestyle codechecker_report_converter tests --exclude tests/unit/analyzers/pyflakes_output_test_files/files
codechecker_report_converter/analyzers/pvs_studio/analyzer_result.py:12:80: E501 line too long (80 > 79 characters)
codechecker_report_converter/analyzers/pvs_studio/analyzer_result.py:39:80: E501 line too long (88 > 79 characters)
codechecker_report_converter/analyzers/pvs_studio/analyzer_result.py:53:80: E501 line too long (81 > 79 characters)
codechecker_report_converter/analyzers/pvs_studio/analyzer_result.py:57:80: E501 line too long (86 > 79 characters)
tests/unit/analyzers/test_pvs_studio_parser.py:10:1: E302 expected 2 blank lines, found 1
tests/unit/analyzers/test_pvs_studio_parser.py:28:80: E501 line too long (116 > 79 characters)
tests/unit/analyzers/test_pvs_studio_parser.py:37:80: E501 line too long (85 > 79 characters)
tests/unit/analyzers/test_pvs_studio_parser.py:38:1: W293 blank line contains whitespace
make: *** [tests/Makefile:29: pycodestyle] Error 1

feeelin · 2024-10-01T09:23:12Z

Okay, I'll fix that in the near future!

There is one question: will the severity field in the Report class be used when displaying the report in the future? If it is, I would add the transformation of our diagnostics' levels into CodeChecker's severity format so that in the future we could determine the level of PVS-Studio diagnostics more precisely.

feeelin

I fixed the crashing test. Our analyzer records the full path to the file in the report, so I made a method that will make the path to the nearby sample.cpp file up to date, don't be scared by this change :)

feeelin

Changed the specifics of the tests, now only the analyzer's warning will be checked instead of the whole report, because unittest caches the unupdated path to the file in .plist files.

Also added PVS-Studio to the list of supported analyzers and report-converter documentation.

feeelin · 2024-10-09T08:14:43Z

Hello @vodorok, @bruntib, @dkrupp!
Still waiting for your response to merge the changes

feeelin · 2024-10-14T09:14:30Z

Hello @vodorok, @bruntib, @dkrupp!
All tests is passed, still waiting for your review :)

dkrupp

Thanks a lot for your patch!!

I have some minor remarks for the test cases.
Could you please take a look at them?

dkrupp · 2024-10-21T15:58:39Z

tools/report-converter/tests/unit/analyzers/test_pvs_studio_parser.py

+
+        with open(plist_file, mode='rb') as pfile:
+            res = plistlib.load(pfile)
+            res['files'][0] = os.path.join('files', 'sample.cpp')


Why are you modifying transformation result plist after calling the transform()?
Shouldn't we have there an absolute path to sample.cpp? This should be compared to the location of sample.cpp in the sample.plist.

Probably forgot to remove that line when I added the method to set absolute paths in the json report. Fixed it.

dkrupp · 2024-10-21T15:59:22Z

tools/report-converter/tests/unit/analyzers/test_pvs_studio_parser.py

+        self.assertEqual(
+            res["diagnostics"][0]["location"]["line"],
+            exp["diagnostics"][0]["location"]["line"]
+        )


could you please add an assertion about the generated HASH? That would prove that the transform() generates the correct hash for the report.

All right, I added this.

dkrupp · 2024-10-21T15:59:59Z

tools/report-converter/tests/unit/analyzers/test_pvs_studio_parser.py

+        )
+
+    @staticmethod
+    def make_report_valid() -> None:


please add a docstring comment to this function explaining what it does.

All right, I added this.

dkrupp · 2024-10-21T16:02:30Z

tools/report-converter/tests/unit/analyzers/test_pvs_studio_parser.py

+        report_path = os.path.join(samples_path, "sample.json")
+        with open(report_path, 'r') as file:
+            data = json.loads(file.read())
+            data["warnings"][0]["positions"][0]["file"] = path_to_file


does PVS studio support relative path to the source files?
If it does, I think we could test the transform() function for .json files with relative paths.

No, PVS-Studio uses absolute paths to source files.

feeelin · 2024-10-30T07:19:04Z

Hello @vodorok, @bruntib, @dkrupp!
The tests worked properly (except for the GUI, but I didn't make any changes to the interface and I suspect that the problem is not in my patch). Do any other changes need to be made, or we can end this branch?

dkrupp

LGTM

bruntib

Please, add the license header to __init__.py

Sorry for the slow review process. After after fixing these minor things, we can merge it. Thanks for your work!

docs/tools/report-converter.md

tools/report-converter/codechecker_report_converter/analyzers/pvs_studio/analyzer_result.py

feeelin · 2024-10-30T13:37:54Z

Committed the latest edits. Thanks for your help!

vodorok

LGTM! Thanks a lot!

feeelin · 2024-11-11T09:31:22Z

Hi, @bruntib, @dkrupp !
When will the changes from this branch make it into the release (pypi and github)?

vodorok · 2024-11-11T14:16:08Z

Hi, @feeelin, we plan our next feature release for early December, CodeChecker 6.25.0. This change will be included in that release.

feeelin · 2024-12-04T07:23:17Z

Hi, @bruntib, @dkrupp!
Have the plans for the 6.25.0 release changed? And will it be released before the end of this week?

bruntib · 2024-12-04T16:14:55Z

Hi,
We're planning to deploy a release candidate this year. The main release will come in January.

feeelin added 2 commits September 30, 2024 14:01

PVS-Studio support

7c0238c

Was added support for PVS-Studio JSON report format in CodeChecker report converter

Delete RulesMap.xml

9f8ad8b

feeelin requested review from bruntib and vodorok as code owners September 30, 2024 11:23

vodorok requested changes Sep 30, 2024

View reviewed changes

PVS-Studio report's tests

fcb4f41

feeelin commented Sep 30, 2024

View reviewed changes

vodorok requested changes Oct 1, 2024

View reviewed changes

feeelin added 3 commits October 1, 2024 14:16

Fix pylint errors

7c4c165

E1120 fix and PVS-Studio's diagnostics' dynamic severity

56202e8

Tests fix

2af178c

feeelin commented Oct 3, 2024

View reviewed changes

More specific tests and documentation

308b874

feeelin requested a review from dkrupp as a code owner October 3, 2024 11:53

feeelin commented Oct 3, 2024

View reviewed changes

Small typo fix

52a5d71

feeelin requested a review from vodorok October 9, 2024 11:07

dkrupp requested changes Oct 21, 2024

View reviewed changes

Updates after @dkrupp review

88e657f

feeelin requested a review from dkrupp October 25, 2024 06:30

dkrupp approved these changes Oct 30, 2024

View reviewed changes

bruntib reviewed Oct 30, 2024

View reviewed changes

docs/tools/report-converter.md Outdated Show resolved Hide resolved

tools/report-converter/codechecker_report_converter/analyzers/pvs_studio/analyzer_result.py Outdated Show resolved Hide resolved

Small fixes

67394b5

Linter E501 fix

9061b09

vodorok approved these changes Oct 30, 2024

View reviewed changes

bruntib merged commit 3320e9a into Ericsson:master Oct 30, 2024
7 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PVS-Studio Static Code Analyzer support #4356

PVS-Studio Static Code Analyzer support #4356

feeelin commented Sep 30, 2024

vodorok left a comment

feeelin left a comment

vodorok left a comment

feeelin commented Oct 1, 2024

feeelin left a comment

feeelin left a comment

feeelin commented Oct 9, 2024

feeelin commented Oct 14, 2024

dkrupp left a comment

dkrupp Oct 21, 2024

feeelin Oct 25, 2024

dkrupp Oct 21, 2024

feeelin Oct 25, 2024

dkrupp Oct 21, 2024

feeelin Oct 25, 2024

dkrupp Oct 21, 2024

feeelin Oct 25, 2024

feeelin commented Oct 30, 2024 •

edited

Loading

dkrupp left a comment

bruntib left a comment

feeelin commented Oct 30, 2024

vodorok left a comment

feeelin commented Nov 11, 2024

vodorok commented Nov 11, 2024

feeelin commented Dec 4, 2024

bruntib commented Dec 4, 2024

PVS-Studio Static Code Analyzer support #4356

PVS-Studio Static Code Analyzer support #4356

Conversation

feeelin commented Sep 30, 2024

vodorok left a comment

Choose a reason for hiding this comment

feeelin left a comment

Choose a reason for hiding this comment

vodorok left a comment

Choose a reason for hiding this comment

feeelin commented Oct 1, 2024

feeelin left a comment

Choose a reason for hiding this comment

feeelin left a comment

Choose a reason for hiding this comment

feeelin commented Oct 9, 2024

feeelin commented Oct 14, 2024

dkrupp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feeelin commented Oct 30, 2024 • edited Loading

dkrupp left a comment

Choose a reason for hiding this comment

bruntib left a comment

Choose a reason for hiding this comment

feeelin commented Oct 30, 2024

vodorok left a comment

Choose a reason for hiding this comment

feeelin commented Nov 11, 2024

vodorok commented Nov 11, 2024

feeelin commented Dec 4, 2024

bruntib commented Dec 4, 2024

feeelin commented Oct 30, 2024 •

edited

Loading