diff --git a/bot/reviewbot/tools/pmd.py b/bot/reviewbot/tools/pmd.py
index 53ffe1d5c703601c5c1f1aae25117d05384cac08..b9f616250eac70e72f709b7c0f0bdee8ede7f094 100644
--- a/bot/reviewbot/tools/pmd.py
+++ b/bot/reviewbot/tools/pmd.py
@@ -4,6 +4,8 @@ from __future__ import annotations
import json
import os
+import re
+from typing import Optional, cast
from reviewbot.config import config
from reviewbot.tools.base import (BaseTool,
@@ -44,7 +46,7 @@ class PMDTool(JavaToolMixin, FilePatternsFromSettingMixin, BaseTool):
'cols': 80,
'rows': 10,
},
- }
+ },
},
{
'name': 'file_ext',
@@ -61,6 +63,54 @@ class PMDTool(JavaToolMixin, FilePatternsFromSettingMixin, BaseTool):
},
]
+ ######################
+ # Instance variables #
+ ######################
+
+ #: The saved base command. This is used for unit testing.
+ _base_command: list[str]
+
+ #: The PMD major version (6 or 7)
+ _pmd_version: Optional[int] = None
+
+ @property
+ def pmd_version(self) -> int:
+ """The version of PMD installed.
+
+ Type:
+ int
+
+ Raises:
+ Exception:
+ The version could not be found.
+ """
+ if self._pmd_version is not None:
+ return self._pmd_version
+
+ pmd_path = cast(str, config['exe_paths']['pmd'])
+
+ # First try the PMD 7 format.
+ output = cast(str, execute(
+ [pmd_path, '--version'],
+ ignore_errors=True,
+ return_errors=False))
+
+ if re.search(r'^PMD 7\.', output, re.MULTILINE):
+ self._pmd_version = 7
+ return self._pmd_version
+
+ # Now try PMD 6.
+ output = cast(str, execute(
+ [pmd_path, 'pmd', '--version'],
+ ignore_errors=True,
+ return_errors=False))
+
+ if re.search(r'^PMD 6\.', output, re.MULTILINE):
+ self._pmd_version = 6
+ return self._pmd_version
+
+ raise Exception('Unable to determine PMD version.')
+
def build_base_command(self, **kwargs):
"""Build the base command line used to review files.
@@ -81,13 +131,29 @@ class PMDTool(JavaToolMixin, FilePatternsFromSettingMixin, BaseTool):
if rulesets.startswith(')'
- ))
- def test_execute_with_ruleset_xml_bad(self):
- """Testing PMDTool.execute with ruleset configuration XML with
- validation problems
- """
- ruleset_xml = (
- '\n'
- '\n'
- ' xsi:schemaLocation="'
- 'http://pmd.sourceforge.net/ruleset/2.0.0 '
- 'http://pmd.sourceforge.net/ruleset_2_0_0.xsd">\n'
- ' My ruleset\n'
- ''
- )
-
- review, review_file = self.run_tool_execute(
- filename='test.java',
- file_contents=(
- b'public class Cls {\n'
- b' public int a(int b) {\n'
- b' if (b == true)\n'
- b' return 1;\n'
- b' return 0;\n'
- b' }\n'
- b'}\n'
- ),
- tool_settings={
- 'file_ext': '',
- 'rulesets': ruleset_xml,
- })
-
- self.assertEqual(review.comments, [
- {
- 'filediff_id': review_file.id,
- 'first_line': 1,
- 'num_lines': 1,
- 'text': (
- 'PMD was unable to process this file:\n'
- '\n'
- '```\n'
- 'SEVERE: No rules found. Maybe you misspelled a rule '
- 'name? ()\n'
- '```'
- ),
- 'issue_opened': True,
- 'rich_text': False,
- },
- ])
- self.assertEqual(review.general_comments, [])
-
- self.assertSpyCalledWith(
- execute,
- [
- self.tool_exe_path,
- 'pmd',
- '-no-cache',
- '-f', 'json',
- '-R', tmpfiles[-2],
- '-d', os.path.join(tmpdirs[-1], 'test.java'),
- '-r', tmpfiles[-1],
- ],
- ignore_errors=True)
+ ])
@integration_test()
@simulation_test(output_payload={
@@ -385,12 +320,7 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
self.assertSpyCalledWith(
execute,
- [
- self.tool_exe_path,
- 'pmd',
- '-no-cache',
- '-f', 'json',
- '-R', 'category/java/codestyle.xml/ShortClassName',
+ self.tool._base_command + [
'-d', os.path.join(tmpdirs[-1], 'test.java'),
'-r', tmpfiles[-1],
],
@@ -405,7 +335,14 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
'processingErrors': [
{
'filename': '/path/to/test.java',
- 'message': 'PMDException: Error while parsing test.java'
+ 'message': (
+ 'ParseException: Parse exception in file \'test.java\' '
+ 'at line 1, column 8: Encountered .\n'
+ 'Was expecting one of:\n'
+ ' "class" ...\n'
+ ' "interface" ...\n'
+ ' "@" ...\n'
+ ),
},
],
})
@@ -421,20 +358,45 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
'rulesets': 'category/java/codestyle.xml/ShortClassName',
})
+ if self.tool_exe_path == '/path/to/pmd':
+ # Running in simulation mode.
+ filename = 'test.java'
+ else:
+ # Running in integration mode.
+ filename = f'{tmpdirs[-1]}/test.java'
+
+ if self.tool.pmd_version == 7:
+ error_text = (
+ f'PMD was unable to process this file:\n'
+ f'\n'
+ f'```\n'
+ f'ParseException: Parse exception in file \'{filename}\' '
+ f'at line 1, column 8: Encountered .\n'
+ f'Was expecting one of:\n'
+ f' "class" ...\n'
+ f' "interface" ...\n'
+ f' "@" ...\n'
+ f'```\n'
+ f'\n'
+ f'Check the file locally for more information.'
+ )
+ else:
+ error_text = (
+ f'PMD was unable to process this file:\n'
+ f'\n'
+ f'```\n'
+ f'PMDException: Error while parsing {filename}\n'
+ f'```\n'
+ f'\n'
+ f'Check the file locally for more information.'
+ )
+
self.assertEqual(review.comments, [
{
'filediff_id': review_file.id,
'first_line': 1,
'num_lines': 1,
- 'text': (
- 'PMD was unable to process this file:\n'
- '\n'
- '```\n'
- 'PMDException: Error while parsing test.java\n'
- '```\n'
- '\n'
- 'Check the file locally for more information.'
- ),
+ 'text': error_text,
'issue_opened': True,
'rich_text': False,
},
@@ -443,12 +405,7 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
self.assertSpyCalledWith(
execute,
- [
- self.tool_exe_path,
- 'pmd',
- '-no-cache',
- '-f', 'json',
- '-R', 'category/java/codestyle.xml/ShortClassName',
+ self.tool._base_command + [
'-d', os.path.join(tmpdirs[-1], 'test.java'),
'-r', tmpfiles[-1],
],
@@ -486,12 +443,7 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
self.assertSpyCalledWith(
execute,
- [
- self.tool_exe_path,
- 'pmd',
- '-no-cache',
- '-f', 'json',
- '-R', 'category/java/codestyle.xml/ShortClassName',
+ self.tool._base_command + [
'-d', os.path.join(tmpdirs[-1], 'test.java'),
'-r', tmpfiles[-1],
],
@@ -531,12 +483,7 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
self.assertSpyCalledWith(
execute,
- [
- self.tool_exe_path,
- 'pmd',
- '-no-cache',
- '-f', 'json',
- '-R', 'category/java/codestyle.xml/ShortClassName',
+ self.tool._base_command + [
'-d', os.path.join(tmpdirs[-1], 'test.java'),
'-r', tmpfiles[-1],
],
@@ -562,9 +509,26 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
self.assertEqual(review.comments, [])
self.assertEqual(review.general_comments, [])
- self.assertSpyNotCalled(execute)
+ n_calls = len(execute.calls)
- def setup_simulation_test(self, output_payload=None, stderr=''):
+ # Make sure that the only calls we got were to check the PMD version.
+ self.assertIn(n_calls, (1, 2))
+
+ if n_calls > 1:
+ self.assertSpyCalledWith(
+ execute,
+ [self.tool_exe_path, '--version'])
+
+ if n_calls > 2:
+ self.assertSpyCalledWith(
+ execute,
+ [self.tool_exe_path, 'pmd', '--version'])
+
+ def setup_simulation_test(
+ self,
+ output_payload: Optional[dict[str, Any]] = None,
+ stderr: str = '',
+ ) -> None:
"""Set up the simulation test for PMD.
This will spy on :py:func:`~reviewbot.utils.process.execute`, making
@@ -580,10 +544,21 @@ class PMDToolTests(BaseToolTestCase, metaclass=ToolTestCaseMetaclass):
"""
assert isinstance(stderr, str)
- @self.spy_for(execute)
def _execute(cmdline, *args, **kwargs):
if output_payload is not None:
with open(tmpfiles[-1], 'w') as fp:
json.dump(output_payload, fp)
return ('stdout junk', stderr)
+
+ self.spy_on(execute, op=kgb.SpyOpMatchInOrder([
+ {
+ 'args': (
+ [self.tool_exe_path, '--version'],
+ ),
+ 'op': kgb.SpyOpReturn('PMD 7.0.0'),
+ },
+ {
+ 'call_fake': _execute,
+ },
+ ]))