New tests and documentation

src/python/review/inspectors/checkstyle/checkstyle.py

@@ -55,6 +55,11 @@ def inspect(self, path: Path, config: dict) -> List[BaseIssue]:
 
     @classmethod
     def choose_issue_type(cls, check_class: str) -> IssueType:
+        """
+        Defines IssueType by Checkstyle check class using config.
+        """
+
+        # Example: com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck -> LineLengthCheck
         check_class_name = check_class.split('.')[-1]
         issue_type = CHECK_CLASS_NAME_TO_ISSUE_TYPE.get(check_class_name)
         if not issue_type:

src/python/review/inspectors/parsers/checkstyle_parser.py

@@ -26,8 +26,11 @@
 logger = logging.getLogger(__name__)
 
 
-# Check if the result of the inspectors is correct: it exists and it is not empty
 def __is_result_file_correct(file_path: Path, inspector_type: InspectorType) -> bool:
+    """
+    Check if the result of the inspectors is correct: it exists and it is not empty.
+    """
+
     if not file_path.is_file():
         logger.error('%s: error - no output file' % inspector_type.value)
         return False
@@ -36,10 +39,16 @@ def __is_result_file_correct(file_path: Path, inspector_type: InspectorType) ->
     if not file_content:
         logger.error('%s: error - empty file' % inspector_type.value)
         return False
+
     return True
 
 
 def __parse_error_message(element: ElementTree) -> str:
+    """
+    Extracts the error message from the tree element and
+    removes the substring '(max allowed is \\d+)' from it, if possible.
+    """
+
     message = element.attrib['message']
     return re.sub(r'\(max allowed is \d+\). ', '', message)
 
@@ -48,6 +57,10 @@ def __parse_error_message(element: ElementTree) -> str:
 # e.g. BoolExprLenIssue, CyclomaticComplexityIssue and so on
 def __parse_measurable_issue(issue_data: Dict[str, Any], issue_type: IssueType,
                              measure_value: int) -> Optional[BaseIssue]:
+    """
+    Depending on the issue_type creates a corresponding measurable issue.
+    """
+
     if issue_type == IssueType.CYCLOMATIC_COMPLEXITY:
         issue_data[IssueData.CYCLOMATIC_COMPLEXITY.value] = measure_value
         issue_data[IssueData.DESCRIPTION.value] = get_cyclomatic_complexity_tip()
@@ -68,19 +81,35 @@ def __parse_measurable_issue(issue_data: Dict[str, Any], issue_type: IssueType,
 
 
 def __should_handle_element(element: ElementTree) -> bool:
+    """
+    Checks if a tree element is a file.
+    """
+
     return element.tag == 'file'
 
 
 def __is_error(element: ElementTree) -> bool:
+    """
+    Checks if a tree element is an error.
+    """
+
     return element.tag == 'error'
 
 
-# TODO Needs testing
+# TODO: Needs refactoring
 def parse_checkstyle_file_result(
         file_path: Path,
         inspector_type: InspectorType,
         issue_type_selector: Callable[[str], IssueType],
         origin_class_to_description: Dict[str, str]) -> List[BaseIssue]:
+    """
+    Parses the output, which is a xml file, and returns a list of the issues found there.
+    issue_type_selector determines the IssueType of an inspection.
+    origin_class_to_description allows to parse measurable errors.
+
+    If the passed path is not a correct file, an empty list is returned.
+    """
+
     if not __is_result_file_correct(file_path, inspector_type):
         return []
 
@@ -98,6 +127,7 @@ def parse_checkstyle_file_result(
                 continue
 
             message = __parse_error_message(inner_element)
+            # Example: com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck -> LineLengthCheck
             origin_class = inner_element.attrib['source'].split('.')[-1]
             issue_data = IssueData.get_base_issue_data_dict(code_file_path, inspector_type,
                                                             line_number=int(inner_element.attrib['line']),

src/python/review/inspectors/pmd/pmd.py

@@ -58,6 +58,11 @@ def inspect(self, path: Path, config: dict) -> List[BaseIssue]:
             return self.parse_output(output_path)
 
     def parse_output(self, output_path: Path) -> List[BaseIssue]:
+        """
+        Parses the PMD output, which is a csv file, and returns a list of the issues found there.
+
+        If the passed path is not a file, an empty list is returned.
+        """
         if not output_path.is_file():
             logger.error('%s: error - no output file' % self.inspector_type.value)
             return []
@@ -77,6 +82,9 @@ def parse_output(self, output_path: Path) -> List[BaseIssue]:
 
     @classmethod
     def choose_issue_type(cls, rule: str) -> IssueType:
+        """
+        Defines IssueType by PMD rule name using config.
+        """
         issue_type = PMD_RULE_TO_ISSUE_TYPE.get(rule)
         if not issue_type:
             logger.warning('%s: %s - unknown rule' %

test/python/inspectors/__init__.py

@@ -13,3 +13,7 @@
 JS_DATA_FOLDER = CURRENT_TEST_DATA_FOLDER / 'js'
 
 PYTHON_AST_DATA_FOLDER = CURRENT_TEST_DATA_FOLDER / 'python_ast'
+
+CHECKSTYLE_DATA_FOLDER = CURRENT_TEST_DATA_FOLDER / 'checkstyle'
+
+PMD_DATA_FOLDER = CURRENT_TEST_DATA_FOLDER / 'pmd'

test/python/inspectors/test_checkstyle_inspector.py

@@ -1,11 +1,154 @@
-from test.python.inspectors import JAVA_DATA_FOLDER
+from pathlib import Path
+from test.python.inspectors import CHECKSTYLE_DATA_FOLDER, JAVA_DATA_FOLDER
 from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata
+from typing import List
 
 import pytest
 from src.python.review.common.language import Language
 from src.python.review.inspectors.checkstyle.checkstyle import CheckstyleInspector
+from src.python.review.inspectors.inspector_type import InspectorType
+from src.python.review.inspectors.issue import (
+    BoolExprLenIssue,
+    CodeIssue,
+    CyclomaticComplexityIssue,
+    FuncLenIssue,
+    IssueType,
+    LineLenIssue,
+)
+from src.python.review.inspectors.parsers.checkstyle_parser import parse_checkstyle_file_result
+from src.python.review.inspectors.tips import (
+    get_bool_expr_len_tip,
+    get_cyclomatic_complexity_tip,
+    get_func_len_tip,
+    get_line_len_tip,
+)
 from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues
 
+FILE_NAME_AND_ISSUES = [
+    ('empty_file.xml', []),
+    ('single_file_project_without_issues.xml', []),
+    ('multi_file_project_without_issues.xml', []),
+    (
+        'single_file_project_without_metric_issues.xml',
+        [
+            CodeIssue(
+                origin_class='MultipleStringLiteralsCheck', type=IssueType.BEST_PRACTICES,
+                description='The String "Howdy" appears 4 times in the file.',
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=6, column_no=13, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+            CodeIssue(
+                origin_class='WhitespaceAroundCheck', type=IssueType.CODE_STYLE,
+                description="'{' is not followed by whitespace.",
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=12, column_no=32, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+            CodeIssue(
+                origin_class='StringLiteralEqualityCheck', type=IssueType.ERROR_PRONE,
+                description="Literal Strings should be compared using equals(), not '=='.",
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=37, column_no=33, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+        ],
+    ),
+    (
+        'single_file_project_with_metric_issues.xml',
+        [
+            FuncLenIssue(
+                origin_class='JavaNCSSCheck', type=IssueType.FUNC_LEN,
+                description=get_func_len_tip(),
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=5, column_no=5, inspector_type=InspectorType.CHECKSTYLE,
+                func_len=42,
+            ),
+            CyclomaticComplexityIssue(
+                origin_class='CyclomaticComplexityCheck', type=IssueType.CYCLOMATIC_COMPLEXITY,
+                description=get_cyclomatic_complexity_tip(),
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=5, column_no=5, inspector_type=InspectorType.CHECKSTYLE,
+                cc_value=69,
+            ),
+            CodeIssue(
+                origin_class='WhitespaceAroundCheck', type=IssueType.CODE_STYLE,
+                description="'switch' is not followed by whitespace.",
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=31, column_no=25, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+        ],
+    ),
+    (
+        'multi_file_project_without_metric_issues.xml',
+        [
+            CodeIssue(
+                origin_class='EmptyBlockCheck', type=IssueType.BEST_PRACTICES,
+                description='Must have at least one statement.',
+                file_path=Path('/home/user/Desktop/some_project/main1.java'),
+                line_no=62, column_no=38, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+            CodeIssue(
+                origin_class='CovariantEqualsCheck', type=IssueType.ERROR_PRONE,
+                description='covariant equals without overriding equals(java.lang.Object).',
+                file_path=Path('/home/user/Desktop/some_project/main1.java'),
+                line_no=68, column_no=20, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+            CodeIssue(
+                origin_class='IndentationCheck', type=IssueType.CODE_STYLE,
+                description="'if' child has incorrect indentation level 2, expected level should be 12.",
+                file_path=Path('/home/user/Desktop/some_project/main2.java'),
+                line_no=116, column_no=3, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+            CodeIssue(
+                origin_class='UpperEllCheck', type=IssueType.BEST_PRACTICES,
+                description="Should use uppercase 'L'.",
+                file_path=Path('/home/user/Desktop/some_project/main2.java'),
+                line_no=123, column_no=15, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+        ],
+    ),
+    (
+        'multi_file_project_with_metric_issues.xml',
+        [
+            CodeIssue(
+                origin_class='OneStatementPerLineCheck', type=IssueType.CODE_STYLE,
+                description='Only one statement per line allowed.',
+                file_path=Path('/home/user/Desktop/some_project/main1.java'),
+                line_no=69, column_no=31, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+            BoolExprLenIssue(
+                origin_class='BooleanExpressionComplexityCheck', type=IssueType.BOOL_EXPR_LEN,
+                description=get_bool_expr_len_tip(),
+                file_path=Path('/home/user/Desktop/some_project/main1.java'),
+                line_no=112, column_no=9, inspector_type=InspectorType.CHECKSTYLE, bool_expr_len=77,
+            ),
+            LineLenIssue(
+                origin_class='LineLengthCheck', type=IssueType.LINE_LEN,
+                description=get_line_len_tip(),
+                file_path=Path('/home/user/Desktop/some_project/main2.java'),
+                line_no=62, column_no=1, inspector_type=InspectorType.CHECKSTYLE, line_len=228,
+            ),
+            CodeIssue(
+                origin_class='WhitespaceAfterCheck', type=IssueType.CODE_STYLE,
+                description="',' is not followed by whitespace.",
+                file_path=Path('/home/user/Desktop/some_project/main2.java'),
+                line_no=136, column_no=19, inspector_type=InspectorType.CHECKSTYLE,
+            ),
+        ],
+    ),
+]
+
+
+@pytest.mark.parametrize(('file_name', 'expected_issues'), FILE_NAME_AND_ISSUES)
+def test_output_parsing(file_name: str, expected_issues: List[CodeIssue]):
+    path_to_file = CHECKSTYLE_DATA_FOLDER / file_name
+    issues = parse_checkstyle_file_result(
+        path_to_file,
+        InspectorType.CHECKSTYLE,
+        CheckstyleInspector.choose_issue_type,
+        CheckstyleInspector.origin_class_to_pattern,
+    )
+    assert issues == expected_issues
+
+
 FILE_NAMES_AND_N_ISSUES = [
     ('test_simple_valid_program.java', 0),
     ('test_spaces.java', 14),

test/python/inspectors/test_pmd_inspector.py

@@ -1,10 +1,86 @@
-from test.python.inspectors import JAVA_DATA_FOLDER
+from pathlib import Path
+from test.python.inspectors import JAVA_DATA_FOLDER, PMD_DATA_FOLDER
+from typing import List
 
 import pytest
+from src.python.review.inspectors.inspector_type import InspectorType
+from src.python.review.inspectors.issue import CodeIssue, IssueType
 from src.python.review.inspectors.pmd.pmd import PMDInspector
 
 from .conftest import use_file_metadata
 
+FILE_NAME_AND_ISSUES = [
+    ('empty_file.csv', []),
+    ('project_without_issues.csv', []),
+    (
+        'single_file_project.csv',
+        [
+            CodeIssue(
+                origin_class='AvoidDuplicateLiterals', type=IssueType.BEST_PRACTICES,
+                description="The String literal 'Howdy' appears 4 times in this file; "
+                            "the first occurrence is on line 6",
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=6, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+            CodeIssue(
+                origin_class='UncommentedEmptyMethodBody', type=IssueType.BEST_PRACTICES,
+                description='Document empty method body',
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=12, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+            CodeIssue(
+                origin_class='UnusedLocalVariable', type=IssueType.BEST_PRACTICES,
+                description="Avoid unused local variables such as 'result'.",
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=31, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+            CodeIssue(
+                origin_class='UnusedPrivateMethod', type=IssueType.BEST_PRACTICES,
+                description="Avoid unused private methods such as 'emptyLoop()'.",
+                file_path=Path('/home/user/Desktop/some_project/main.java'),
+                line_no=61, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+        ],
+    ),
+    (
+        'multi_file_project.csv',
+        [
+            CodeIssue(
+                origin_class='CompareObjectsWithEquals', type=IssueType.ERROR_PRONE,
+                description='Use equals() to compare object references.',
+                file_path=Path('/home/user/Desktop/some_project/main1.java'),
+                line_no=37, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+            CodeIssue(
+                origin_class='SuspiciousEqualsMethodName', type=IssueType.ERROR_PRONE,
+                description='The method name and parameter number are suspiciously close to equals(Object)',
+                file_path=Path('/home/user/Desktop/some_project/main1.java'),
+                line_no=68, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+            CodeIssue(
+                origin_class='UselessParentheses', type=IssueType.CODE_STYLE,
+                description='Useless parentheses.',
+                file_path=Path('/home/user/Desktop/some_project/main2.java'),
+                line_no=113, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+            CodeIssue(
+                origin_class='EmptyIfStmt', type=IssueType.BEST_PRACTICES,
+                description='Avoid empty if statements',
+                file_path=Path('/home/user/Desktop/some_project/main2.java'),
+                line_no=131, column_no=1, inspector_type=InspectorType.PMD,
+            ),
+        ],
+    ),
+]
+
+
+@pytest.mark.parametrize(('file_name', 'expected_issues'), FILE_NAME_AND_ISSUES)
+def test_output_parsing(file_name: str, expected_issues: List[CodeIssue]):
+    path_to_file = PMD_DATA_FOLDER / file_name
+    issues = PMDInspector().parse_output(path_to_file)
+    assert issues == expected_issues
+
+
 FILE_NAMES_AND_N_ISSUES = [
     ('test_algorithm_with_scanner.java', 0),
     ('test_simple_valid_program.java', 0),

test/resources/inspectors/checkstyle/multi_file_project_with_metric_issues.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<checkstyle version="8.44">
+    <file name="/home/user/Desktop/some_project/main1.java">
+        <error line="69" column="31" severity="error" message="Only one statement per line allowed."
+               source="com.puppycrawl.tools.checkstyle.checks.coding.OneStatementPerLineCheck"/>
+        <error line="112" column="9" severity="error" message="Boolean expression complexity is 77 (max allowed is 0)."
+               source="com.puppycrawl.tools.checkstyle.checks.metrics.BooleanExpressionComplexityCheck"/>
+    </file>
+    <file name="/home/user/Desktop/some_project/main2.java">
+        <error line="62" severity="error" message="Line is longer than 120 characters (found 228)."
+               source="com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck"/>
+        <error line="136" column="19" severity="error" message="',' is not followed by whitespace."
+               source="com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAfterCheck"/>
+    </file>
+</checkstyle>

test/resources/inspectors/checkstyle/multi_file_project_without_issues.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<checkstyle version="8.44">
+    <file name="/home/user/Desktop/some_project/main1.java"></file>
+    <file name="/home/user/Desktop/some_project/main2.java"></file>
+    <file name="/home/user/Desktop/some_project/main3.java"></file>
+    <file name="/home/user/Desktop/some_project/main4.java"></file>
+    <file name="/home/user/Desktop/some_project/main5.java"></file>
+    <file name="/home/user/Desktop/some_project/main6.java"></file>
+</checkstyle>