print_review.py refactoring

src/python/review/reviewers/common.py

@@ -16,7 +16,7 @@
 from src.python.review.quality.evaluate_quality import evaluate_quality
 from src.python.review.quality.model import Quality
 from src.python.review.quality.penalty import categorize, get_previous_issues_by_language, Punisher
-from src.python.review.reviewers.review_result import FileReviewResult, ReviewResult
+from src.python.review.reviewers.review_result import FileReviewResult, GeneralReviewResult
 from src.python.review.reviewers.utils.code_statistics import gather_code_statistics
 from src.python.review.reviewers.utils.issues_filter import filter_duplicate_issues, filter_low_measure_issues
 from src.python.review.reviewers.utils.metadata_exploration import FileMetadata, Metadata
@@ -43,9 +43,7 @@
 }
 
 
-def perform_language_review(metadata: Metadata,
-                            config: ApplicationConfig,
-                            language: Language) -> ReviewResult:
+def perform_language_review(metadata: Metadata, config: ApplicationConfig, language: Language) -> GeneralReviewResult:
     inspectors = LANGUAGE_TO_INSPECTORS[language]
 
     issues = inspect_in_parallel(metadata.path, config, inspectors)
@@ -73,27 +71,18 @@ def perform_language_review(metadata: Metadata,
 
     file_review_results = []
     for file_metadata in files_metadata:
-        issues = file_path_to_issues[file_metadata.path]
-        code_statistics = gather_code_statistics(issues, file_metadata.path)
+        file_issues = file_path_to_issues[file_metadata.path]
+        code_statistics = gather_code_statistics(file_issues, file_metadata.path)
         code_statistics.total_lines = min(code_statistics.total_lines,
                                           get_range_lines(config.start_line, config.end_line))
 
-        punisher = Punisher(issues, previous_issues)
+        punisher = Punisher(file_issues, previous_issues)
         quality = evaluate_quality(code_statistics, language)
         general_quality = general_quality.merge(quality)
 
-        file_review_results.append(FileReviewResult(
-            file_metadata.path,
-            issues,
-            quality,
-            punisher,
-        ))
-
-    return ReviewResult(
-        file_review_results,
-        general_quality,
-        general_punisher,
-    )
+        file_review_results.append(FileReviewResult(quality, punisher, file_issues, file_metadata.path))
+
+    return GeneralReviewResult(general_quality, general_punisher, issues, file_review_results)
 
 
 def filter_out_of_range_issues(issues: List[BaseIssue],

src/python/review/reviewers/perform_review.py

@@ -9,7 +9,7 @@
 from src.python.review.inspectors.issue import IssueType
 from src.python.review.reviewers.common import perform_language_review
 from src.python.review.reviewers.python import perform_python_review
-from src.python.review.reviewers.review_result import ReviewResult
+from src.python.review.reviewers.review_result import GeneralReviewResult
 from src.python.review.reviewers.utils.metadata_exploration import explore_file, explore_project
 from src.python.review.reviewers.utils.print_review import (
     print_review_result_as_json,
@@ -68,10 +68,10 @@ def perform_and_print_review(path: Path,
         print_review_result_as_text(review_result, path, config)
 
     # Don't count INFO issues too
-    return len(list(filter(lambda issue: issue.type != IssueType.INFO, review_result.all_issues)))
+    return len(list(filter(lambda issue: issue.type != IssueType.INFO, review_result.issues)))
 
 
-def perform_review(path: Path, config: ApplicationConfig) -> ReviewResult:
+def perform_review(path: Path, config: ApplicationConfig) -> GeneralReviewResult:
     if not path.exists():
         raise PathNotExists
 

src/python/review/reviewers/python.py

@@ -5,11 +5,11 @@
 from src.python.review.common.file_system import FileSystemItem, get_all_file_system_items
 from src.python.review.common.language import Language
 from src.python.review.reviewers.common import perform_language_review
-from src.python.review.reviewers.review_result import ReviewResult
+from src.python.review.reviewers.review_result import GeneralReviewResult
 from src.python.review.reviewers.utils.metadata_exploration import Metadata, ProjectMetadata
 
 
-def perform_python_review(metadata: Metadata, config: ApplicationConfig) -> ReviewResult:
+def perform_python_review(metadata: Metadata, config: ApplicationConfig) -> GeneralReviewResult:
     created_file_paths = []
     if isinstance(metadata, ProjectMetadata):
         created_file_paths.extend(create_init_scripts_in_subdirs(metadata.path))

src/python/review/reviewers/review_result.py

@@ -8,23 +8,26 @@
 
 
 @dataclass
-class FileReviewResult:
-    file_path: Path
-    issues: List[BaseIssue]
+class ReviewResult:
+    """
+    ReviewResult contains a list of issues, as well as quality and punisher obtained with these issues.
+    """
     quality: Quality
     punisher: Punisher
+    issues: List[BaseIssue]
 
 
 @dataclass
-class ReviewResult:
-    file_review_results: List[FileReviewResult]
-    general_quality: Quality
-    general_punisher: Punisher
+class FileReviewResult(ReviewResult):
+    """
+    FileReviewResult contains the information needed to output about a particular inspected file.
+    """
+    file_path: Path
 
-    @property
-    def all_issues(self) -> List[BaseIssue]:
-        issues = []
-        for file_review_result in self.file_review_results:
-            issues.extend(file_review_result.issues)
 
-        return issues
+@dataclass
+class GeneralReviewResult(ReviewResult):
+    """
+    GeneralReviewResult contains the information needed to output about the entire inspected project.
+    """
+    file_review_results: List[FileReviewResult]

src/python/review/reviewers/utils/print_review.py

@@ -9,16 +9,15 @@
 from src.python.review.common.file_system import get_file_line
 from src.python.review.inspectors.inspector_type import InspectorType
 from src.python.review.inspectors.issue import BaseIssue, IssueType
-from src.python.review.reviewers.review_result import ReviewResult
+from src.python.review.quality.model import QualityType
+from src.python.review.reviewers.review_result import FileReviewResult, GeneralReviewResult, ReviewResult
 
 
-def print_review_result_as_text(review_result: ReviewResult,
-                                path: Path,
-                                config: ApplicationConfig) -> None:
-    heading = f'\nReview of {str(path)} ({len(review_result.all_issues)} violations)'
+def print_review_result_as_text(review_result: GeneralReviewResult, path: Path, config: ApplicationConfig) -> None:
+    heading = f'\nReview of {str(path)} ({len(review_result.issues)} violations)'
     print(heading)
 
-    if len(review_result.all_issues) == 0:
+    if len(review_result.issues) == 0:
         print('There is no issues found')
     else:
         for file_review_result in review_result.file_review_results:
@@ -51,73 +50,89 @@ def print_review_result_as_text(review_result: ReviewResult,
 
     print('*' * len(heading))
     print('General quality:')
-    print(review_result.general_quality, end='')
+    print(review_result.quality, end='')
 
 
-def print_review_result_as_json(review_result: ReviewResult, config: ApplicationConfig) -> None:
-    issues = review_result.all_issues
+def _get_quality_without_penalty(review_result: ReviewResult) -> QualityType:
+    return review_result.quality.quality_type
 
+
+def _get_quality_with_penalty(review_result: ReviewResult) -> QualityType:
+    quality_without_penalty = _get_quality_without_penalty(review_result)
+    return review_result.punisher.get_quality_with_penalty(quality_without_penalty)
+
+
+def get_quality_json_dict(quality: QualityType) -> Dict:
+    return {
+        OutputJsonFields.CODE.value: quality.value,
+        OutputJsonFields.TEXT.value: f'Code quality (beta): {quality.value}',
+    }
+
+
+def get_influence_on_penalty_json_dict(origin_class: str, review_result: ReviewResult) -> int:
+    quality_without_penalty = _get_quality_without_penalty(review_result)
+    quality_with_penalty = _get_quality_with_penalty(review_result)
+
+    if quality_with_penalty != quality_without_penalty:
+        return review_result.punisher.get_issue_influence_on_penalty(origin_class)
+
+    return 0
+
+
+def convert_review_result_to_json_dict(review_result: ReviewResult, config: ApplicationConfig) -> Dict:
+    issues = review_result.issues
     issues.sort(key=lambda issue: issue.line_no)
 
-    quality_without_penalty = review_result.general_quality.quality_type
-    quality_with_penalty = review_result.general_punisher.get_quality_with_penalty(quality_without_penalty)
-    output_json = {'quality': {
-        'code': quality_with_penalty.value,
-        'text': f'Code quality (beta): {quality_with_penalty.value}',
-    }, 'issues': []}
+    quality_with_penalty = _get_quality_with_penalty(review_result)
 
-    for issue in issues:
-        influence_on_penalty = 0
-        if quality_with_penalty != quality_without_penalty:
-            influence_on_penalty = review_result.general_punisher.get_issue_influence_on_penalty(issue.origin_class)
+    output_json = {}
 
-        output_json['issues'].append(convert_issue_to_json(issue, config, influence_on_penalty))
+    if isinstance(review_result, FileReviewResult):
+        output_json[OutputJsonFields.FILE_NAME.value] = str(review_result.file_path)
 
-    print(json.dumps(output_json))
+    output_json[OutputJsonFields.QUALITY.value] = get_quality_json_dict(quality_with_penalty)
+    output_json[OutputJsonFields.ISSUES.value] = []
 
+    for issue in issues:
+        json_issue = convert_issue_to_json(issue, config)
 
-def print_review_result_as_multi_file_json(review_result: ReviewResult, config: ApplicationConfig) -> None:
-    file_review_result_jsons = []
+        json_issue[OutputJsonFields.INFLUENCE_ON_PENALTY.value] = get_influence_on_penalty_json_dict(
+            issue.origin_class, review_result,
+        )
 
-    review_result.file_review_results.sort(key=lambda result: result.file_path)
+        output_json[OutputJsonFields.ISSUES.value].append(json_issue)
 
-    for file_review_result in review_result.file_review_results:
-        quality_without_penalty = file_review_result.quality.quality_type
-        quality_with_penalty = file_review_result.punisher.get_quality_with_penalty(quality_without_penalty)
-        file_review_result_json = {
-            'file_name': str(file_review_result.file_path),
-            'quality': {
-                'code': quality_with_penalty.value,
-                'text': f'Code quality (beta): {quality_with_penalty.value}',
-            },
-            'issues': [],
-        }
+    return output_json
 
-        file_review_result_jsons.append(file_review_result_json)
 
-        for issue in file_review_result.issues:
-            influence_on_penalty = 0
-            if quality_with_penalty != quality_without_penalty:
-                influence_on_penalty = file_review_result.punisher.get_issue_influence_on_penalty(issue.origin_class)
+def print_review_result_as_json(review_result: GeneralReviewResult, config: ApplicationConfig) -> None:
+    print(json.dumps(convert_review_result_to_json_dict(review_result, config)))
 
-            file_review_result_json['issues'].append(convert_issue_to_json(issue, config, influence_on_penalty))
 
-    quality_without_penalty = review_result.general_quality.quality_type
-    quality_with_penalty = review_result.general_punisher.get_quality_with_penalty(quality_without_penalty)
+def print_review_result_as_multi_file_json(review_result: GeneralReviewResult, config: ApplicationConfig) -> None:
+    review_result.file_review_results.sort(key=lambda result: result.file_path)
+
+    file_review_result_jsons = []
+    for file_review_result in review_result.file_review_results:
+        file_review_result_jsons.append(convert_review_result_to_json_dict(file_review_result, config))
+
+    quality_with_penalty = _get_quality_with_penalty(review_result)
 
     output_json = {
-        'quality': {
-            'code': quality_with_penalty.value,
-            'text': f'Code quality (beta): {quality_with_penalty.value}',
-        },
-        'file_review_results': file_review_result_jsons,
+        OutputJsonFields.QUALITY.value: get_quality_json_dict(quality_with_penalty),
+        OutputJsonFields.FILE_REVIEW_RESULTS.value: file_review_result_jsons,
     }
 
     print(json.dumps(output_json))
 
 
 @unique
-class IssueJsonFields(Enum):
+class OutputJsonFields(Enum):
+    QUALITY = 'quality'
+    ISSUES = 'issues'
+    FILE_REVIEW_RESULTS = 'file_review_results'
+    FILE_NAME = 'file_name'
+
     CODE = 'code'
     TEXT = 'text'
     LINE = 'line'
@@ -127,21 +142,20 @@ class IssueJsonFields(Enum):
     INFLUENCE_ON_PENALTY = 'influence_on_penalty'
 
 
-def convert_issue_to_json(issue: BaseIssue, config: ApplicationConfig, influence_on_penalty: int = 0) -> Dict[str, Any]:
+def convert_issue_to_json(issue: BaseIssue, config: ApplicationConfig) -> Dict[str, Any]:
     line_text = get_file_line(issue.file_path, issue.line_no)
 
     issue_type = issue.type
     if not config.with_all_categories:
         issue_type = issue_type.to_main_type()
 
     return {
-        IssueJsonFields.CODE.value: issue.origin_class,
-        IssueJsonFields.TEXT.value: issue.description,
-        IssueJsonFields.LINE.value: line_text,
-        IssueJsonFields.LINE_NUMBER.value: issue.line_no,
-        IssueJsonFields.COLUMN_NUMBER.value: issue.column_no,
-        IssueJsonFields.CATEGORY.value: issue_type.value,
-        IssueJsonFields.INFLUENCE_ON_PENALTY.value: influence_on_penalty,
+        OutputJsonFields.CODE.value: issue.origin_class,
+        OutputJsonFields.TEXT.value: issue.description,
+        OutputJsonFields.LINE.value: line_text,
+        OutputJsonFields.LINE_NUMBER.value: issue.line_no,
+        OutputJsonFields.COLUMN_NUMBER.value: issue.column_no,
+        OutputJsonFields.CATEGORY.value: issue_type.value,
     }
 
 
@@ -151,15 +165,15 @@ def convert_json_to_issues(issues_json: List[dict]) -> List[PenaltyIssue]:
     for issue in issues_json:
         issues.append(
             PenaltyIssue(
-                origin_class=issue[IssueJsonFields.CODE.value],
-                description=issue[IssueJsonFields.TEXT.value],
-                line_no=int(issue[IssueJsonFields.LINE_NUMBER.value]),
-                column_no=int(issue[IssueJsonFields.COLUMN_NUMBER.value]),
-                type=IssueType(issue[IssueJsonFields.CATEGORY.value]),
+                origin_class=issue[OutputJsonFields.CODE.value],
+                description=issue[OutputJsonFields.TEXT.value],
+                line_no=int(issue[OutputJsonFields.LINE_NUMBER.value]),
+                column_no=int(issue[OutputJsonFields.COLUMN_NUMBER.value]),
+                type=IssueType(issue[OutputJsonFields.CATEGORY.value]),
 
                 file_path=Path(),
                 inspector_type=InspectorType.UNDEFINED,
-                influence_on_penalty=issue.get(IssueJsonFields.INFLUENCE_ON_PENALTY.value, 0),
+                influence_on_penalty=issue.get(OutputJsonFields.INFLUENCE_ON_PENALTY.value, 0),
             ),
         )
     return issues