diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 50706799..4cc2a22b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,7 @@ jobs: # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,W503,WPS,C812,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,W503,WPS,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - name: Set up Eslint run: | npm install eslint --save-dev diff --git a/README.md b/README.md index 84a74445..43370839 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,10 @@ Python language: - [x] Pylint [GNU LGPL v2] * [Site and docs](https://www.pylint.org/) * [Repository](https://github.com/PyCQA/pylint) - + +- [x] Radon [MIT] + * [Site and docs](https://radon.readthedocs.io/en/latest/) + * [Repository](https://github.com/rubik/radon) Java language: @@ -92,7 +95,7 @@ Argument | Description --- | --- **‑h**, **‑‑help** | show the help message and exit. **‑v**, **‑‑verbosity** | choose logging level according [this](https://docs.python.org/3/library/logging.html#levels) list: `1` - **ERROR**; `2` - **INFO**; `3` - **DEBUG**; `0` - disable logging (**CRITICAL** value); default value is `0` (**CRITICAL**). -**‑d**, **‑‑disable** | disable inspectors. Available values: for **Python** language: `pylint` for [Pylint](https://github.com/PyCQA/pylint), `flake8` for [flake8](https://flake8.pycqa.org/en/latest/), `python_ast` to check different measures providing by AST; for **Java** language: `checkstyle` for the [Checkstyle](https://checkstyle.sourceforge.io/), `pmd` for [PMD](https://pmd.github.io/); for `Kotlin` language: detekt for [Detekt](https://detekt.github.io/detekt/); for **JavaScript** language: `eslint` for [ESlint](https://eslint.org/). Example: `-d pylint,flake8`. +**‑d**, **‑‑disable** | disable inspectors. Available values: for **Python** language: `pylint` for [Pylint](https://github.com/PyCQA/pylint), `flake8` for [flake8](https://flake8.pycqa.org/en/latest/), `radon` for [Radon](https://radon.readthedocs.io/en/latest/), `python_ast` to check different measures providing by AST; for **Java** language: `checkstyle` for the [Checkstyle](https://checkstyle.sourceforge.io/), `pmd` for [PMD](https://pmd.github.io/); for `Kotlin` language: detekt for [Detekt](https://detekt.github.io/detekt/); for **JavaScript** language: `eslint` for [ESlint](https://eslint.org/). Example: `-d pylint,flake8`. **‑‑allow-duplicates** | allow duplicate issues found by different linters. By default, duplicates are skipped. **‑‑language-version**, **‑‑language_version** | specify the language version for JAVA inspectors. Available values: `java7`, `java8`, `java9`, `java11`. **Note**: **‑‑language_version** is deprecated. Will be deleted in the future. **‑‑n-cpu**, **‑‑n_cpu** | specify number of _cpu_ that can be used to run inspectors. **Note**: **‑‑n_cpu** is deprecated. Will be deleted in the future. diff --git a/requirements.txt b/requirements.txt index e13a08f4..60409ee1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,6 +22,7 @@ flake8-broken-line==0.3.0 flake8-string-format==0.3.0 flake8-commas==2.0.0 cohesion==1.0.0 +radon==4.5.0 # extra libraries and frameworks django==3.0.8 diff --git a/setup.py b/setup.py index 12428183..ab75f040 100644 --- a/setup.py +++ b/setup.py @@ -45,22 +45,22 @@ def get_inspectors_additional_files() -> List[str]: 'Topic :: Software Development :: Build Tools', 'License :: OSI Approved :: MIT License', 'Programming Language :: Python :: 3', - 'Operating System :: OS Independent' + 'Operating System :: OS Independent', ], keywords='code review', python_requires='>=3.8, <4', install_requires=['upsourceapi'], packages=find_packages(exclude=[ '*.unit_tests', '*.unit_tests.*', 'unit_tests.*', 'unit_tests', - '*.functional_tests', '*.functional_tests.*', 'functional_tests.*', 'functional_tests' + '*.functional_tests', '*.functional_tests.*', 'functional_tests.*', 'functional_tests', ]), zip_safe=False, package_data={ - '': get_inspectors_additional_files() + '': get_inspectors_additional_files(), }, entry_points={ 'console_scripts': [ - 'review=review.run_tool:main' - ] - } + 'review=review.run_tool:main', + ], + }, ) diff --git a/src/python/review/common/file_system.py b/src/python/review/common/file_system.py index 6decb410..3faa962c 100644 --- a/src/python/review/common/file_system.py +++ b/src/python/review/common/file_system.py @@ -77,7 +77,7 @@ def create_directory(directory: str) -> None: def get_file_line(path: Path, line_number: int): return linecache.getline( str(path), - line_number + line_number, ).strip() diff --git a/src/python/review/common/java_compiler.py b/src/python/review/common/java_compiler.py index aee76032..e6b97861 100644 --- a/src/python/review/common/java_compiler.py +++ b/src/python/review/common/java_compiler.py @@ -18,7 +18,7 @@ def javac(javac_args: Union[str, Path]) -> bool: output_bytes: bytes = subprocess.check_output( f'javac {javac_args}', shell=True, - stderr=subprocess.STDOUT + stderr=subprocess.STDOUT, ) output_str = str(output_bytes, Encoding.UTF_ENCODING.value) diff --git a/src/python/review/common/parallel_runner.py b/src/python/review/common/parallel_runner.py index e9ab7d8d..c0347d23 100644 --- a/src/python/review/common/parallel_runner.py +++ b/src/python/review/common/parallel_runner.py @@ -37,7 +37,7 @@ def inspect_in_parallel(path: Path, with multiprocessing.Pool(config.n_cpu) as pool: issues = pool.map( functools.partial(run_inspector, path, config), - inspectors + inspectors, ) return list(itertools.chain(*issues)) diff --git a/src/python/review/common/subprocess_runner.py b/src/python/review/common/subprocess_runner.py index 3dd5cad3..a25cbdcd 100644 --- a/src/python/review/common/subprocess_runner.py +++ b/src/python/review/common/subprocess_runner.py @@ -9,7 +9,7 @@ def run_in_subprocess(command: List[str]) -> str: process = subprocess.run( command, stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() diff --git a/src/python/review/inspectors/checkstyle/checkstyle.py b/src/python/review/inspectors/checkstyle/checkstyle.py index 389d889f..c796fbf9 100644 --- a/src/python/review/inspectors/checkstyle/checkstyle.py +++ b/src/python/review/inspectors/checkstyle/checkstyle.py @@ -35,7 +35,7 @@ class CheckstyleInspector(BaseInspector): r'Boolean expression complexity is (\d+)', 'LineLengthCheck': - r'Line is longer than \d+ characters \(found (\d+)\)' + r'Line is longer than \d+ characters \(found (\d+)\)', } @classmethod @@ -43,7 +43,7 @@ def _create_command(cls, path: Path, output_path: Path) -> List[str]: return [ 'java', '-jar', PATH_TOOLS_CHECKSTYLE_JAR, '-c', PATH_TOOLS_CHECKSTYLE_CONFIG, - '-f', 'xml', '-o', output_path, str(path) + '-f', 'xml', '-o', output_path, str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/common.py b/src/python/review/inspectors/common.py new file mode 100644 index 00000000..dfedbb1e --- /dev/null +++ b/src/python/review/inspectors/common.py @@ -0,0 +1,12 @@ +from math import floor + + +def convert_percentage_of_value_to_lack_of_value(percentage_of_value: float) -> int: + """ + Converts percentage of value to lack of value. + Calculated by the formula: floor(100 - percentage_of_value). + + :param percentage_of_value: value in the range from 0 to 100. + :return: lack of value. + """ + return floor(100 - percentage_of_value) diff --git a/src/python/review/inspectors/detekt/detekt.py b/src/python/review/inspectors/detekt/detekt.py index 2c2b197e..8f5f0946 100644 --- a/src/python/review/inspectors/detekt/detekt.py +++ b/src/python/review/inspectors/detekt/detekt.py @@ -27,7 +27,7 @@ class DetektInspector(BaseInspector): 'ComplexCondition': r'This condition is too complex \((\d+)\)', 'ComplexMethod': - r'The function .* appears to be too complex \((\d+)\)' + r'The function .* appears to be too complex \((\d+)\)', } @classmethod @@ -38,7 +38,7 @@ def _create_command(cls, path: Path, output_path: Path): '--config', PATH_DETEKT_CONFIG, '--plugins', PATH_DETEKT_PLUGIN, '--report', f'xml:{output_path}', - '--input', str(path) + '--input', str(path), ] def inspect(self, path: Path, config) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/eslint/eslint.py b/src/python/review/inspectors/eslint/eslint.py index ff9f9a67..839a166c 100644 --- a/src/python/review/inspectors/eslint/eslint.py +++ b/src/python/review/inspectors/eslint/eslint.py @@ -17,7 +17,7 @@ class ESLintInspector(BaseInspector): origin_class_to_pattern = { 'complexity': - r'complexity of (\d+)' + r'complexity of (\d+)', } @classmethod diff --git a/src/python/review/inspectors/flake8/.flake8 b/src/python/review/inspectors/flake8/.flake8 index 6cb0d0cf..cc3e69cb 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -50,5 +50,4 @@ ignore=W291, # trailing whitespaces F524, # missing argument. TODO: Collision with "P201" and "P202" F525, # mixing automatic and manual numbering. TODO: Collision with "P205" # flake8-commas - C814, # missing trailing comma in Python - \ No newline at end of file + C814, # missing trailing comma in Python 2 diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index c7318d0b..b974015c 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -1,5 +1,4 @@ import logging -import math import re from pathlib import Path from typing import List @@ -17,6 +16,7 @@ CohesionIssue, ) from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip +from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value logger = logging.getLogger(__name__) @@ -69,7 +69,9 @@ def parse(cls, output: str) -> List[BaseIssue]: issues.append(CyclomaticComplexityIssue(**issue_data)) elif cohesion_match is not None: # flake8-cohesion issue_data[IssueData.DESCRIPTION.value] = description # TODO: Add tip - issue_data[IssueData.COHESION_LACK.value] = cls.__get_cohesion_lack(float(cohesion_match.group(1))) + issue_data[IssueData.COHESION_LACK.value] = convert_percentage_of_value_to_lack_of_value( + float(cohesion_match.group(1)), + ) issue_data[IssueData.ISSUE_TYPE.value] = IssueType.COHESION issues.append(CohesionIssue(**issue_data)) else: @@ -98,14 +100,3 @@ def choose_issue_type(code: str) -> IssueType: return IssueType.BEST_PRACTICES return issue_type - - @staticmethod - def __get_cohesion_lack(cohesion_percentage: float) -> int: - """ - Converts cohesion percentage to lack of cohesion. - Calculated by the formula: floor(100 - cohesion_percentage). - - :param cohesion_percentage: cohesion set as a percentage. - :return: lack of cohesion - """ - return math.floor(100 - cohesion_percentage) diff --git a/src/python/review/inspectors/inspector_type.py b/src/python/review/inspectors/inspector_type.py index 1b69ac2d..15482a8b 100644 --- a/src/python/review/inspectors/inspector_type.py +++ b/src/python/review/inspectors/inspector_type.py @@ -8,6 +8,7 @@ class InspectorType(Enum): PYLINT = 'PYLINT' PYTHON_AST = 'PYTHON_AST' FLAKE8 = 'FLAKE8' + RADON = 'RADON' # Java language PMD = 'PMD' @@ -29,6 +30,7 @@ def available_values(cls) -> List[str]: cls.PYLINT.value, cls.FLAKE8.value, cls.PYTHON_AST.value, + cls.RADON.value, # Java language cls.PMD.value, diff --git a/src/python/review/inspectors/intellij/intellij.py b/src/python/review/inspectors/intellij/intellij.py index 8b962947..b50dc0ca 100644 --- a/src/python/review/inspectors/intellij/intellij.py +++ b/src/python/review/inspectors/intellij/intellij.py @@ -48,7 +48,7 @@ def __init__(self): def create_command(output_dir_path) -> List[Union[str, Path]]: return [ INTELLIJ_INSPECTOR_EXECUTABLE, INTELLIJ_INSPECTOR_PROJECT, - INTELLIJ_INSPECTOR_SETTINGS, output_dir_path, '-v2' + INTELLIJ_INSPECTOR_SETTINGS, output_dir_path, '-v2', ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -134,8 +134,8 @@ def parse(cls, out_dir_path: Path, file_path = Path( text.replace( 'file://$PROJECT_DIR$', - str(INTELLIJ_INSPECTOR_PROJECT) - ) + str(INTELLIJ_INSPECTOR_PROJECT), + ), ) elif tag == 'line': line_no = int(text) @@ -160,7 +160,7 @@ def parse(cls, out_dir_path: Path, description=description, origin_class=issue_class, inspector_type=cls.inspector_type, - type=issue_type + type=issue_type, )) return issues diff --git a/src/python/review/inspectors/intellij/issue_types/kotlin.py b/src/python/review/inspectors/intellij/issue_types/kotlin.py index 97438dca..cb692510 100644 --- a/src/python/review/inspectors/intellij/issue_types/kotlin.py +++ b/src/python/review/inspectors/intellij/issue_types/kotlin.py @@ -378,5 +378,5 @@ '\'when\' that can be simplified by introducing an argument': IssueType.CODE_STYLE, - 'Annotator': IssueType.ERROR_PRONE + 'Annotator': IssueType.ERROR_PRONE, } diff --git a/src/python/review/inspectors/issue.py b/src/python/review/inspectors/issue.py index e4a21798..c910bf80 100644 --- a/src/python/review/inspectors/issue.py +++ b/src/python/review/inspectors/issue.py @@ -25,6 +25,7 @@ class IssueType(Enum): COHESION = 'COHESION' CLASS_RESPONSE = 'CLASS_RESPONSE' METHOD_NUMBER = 'METHOD_NUMBER' + MAINTAINABILITY = 'MAINTAINABILITY' # Keys in results dictionary @@ -46,6 +47,7 @@ class IssueData(Enum): BOOL_EXPR_LEN = 'bool_expr_len' CYCLOMATIC_COMPLEXITY = 'cc_value' COHESION_LACK = 'cohesion_lack' + MAINTAINABILITY_LACK = 'maintainability_lack' @classmethod def get_base_issue_data_dict(cls, @@ -59,7 +61,7 @@ def get_base_issue_data_dict(cls, cls.LINE_NUMBER.value: line_number, cls.COLUMN_NUMBER.value: column_number, cls.ORIGIN_ClASS.value: origin_class, - cls.INSPECTOR_TYPE.value: inspector_type + cls.INSPECTOR_TYPE.value: inspector_type, } @@ -184,3 +186,12 @@ class MethodNumberIssue(BaseIssue, Measurable): def measure(self) -> int: return self.method_number + + +@dataclass(frozen=True) +class MaintainabilityLackIssue(BaseIssue, Measurable): + maintainability_lack: int + type = IssueType.MAINTAINABILITY + + def measure(self) -> int: + return self.maintainability_lack diff --git a/src/python/review/inspectors/pmd/pmd.py b/src/python/review/inspectors/pmd/pmd.py index 227169fc..4b8c11f0 100644 --- a/src/python/review/inspectors/pmd/pmd.py +++ b/src/python/review/inspectors/pmd/pmd.py @@ -37,7 +37,7 @@ def _create_command(cls, path: Path, '-language', 'java', '-version', java_version.value, '-f', 'csv', '-r', str(output_path), - '-t', str(n_cpu) + '-t', str(n_cpu), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/pyast/python_ast.py b/src/python/review/inspectors/pyast/python_ast.py index 28b76e9f..6426d115 100644 --- a/src/python/review/inspectors/pyast/python_ast.py +++ b/src/python/review/inspectors/pyast/python_ast.py @@ -40,7 +40,7 @@ def visit(self, node: ast.AST): origin_class=BOOL_EXPR_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, bool_expr_len=length, - type=IssueType.BOOL_EXPR_LEN + type=IssueType.BOOL_EXPR_LEN, )) @@ -58,7 +58,7 @@ def __init__(self, content: str, file_path: Path, inspector_type: InspectorType) def visit(self, node): if isinstance(self._previous_node, (ast.FunctionDef, ast.AsyncFunctionDef)): func_length = self._find_func_len( - self._previous_node.lineno, node.lineno + self._previous_node.lineno, node.lineno, ) self._function_lens.append(FuncLenIssue( @@ -69,7 +69,7 @@ def visit(self, node): origin_class=FUNC_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, func_len=func_length, - type=IssueType.FUNC_LEN + type=IssueType.FUNC_LEN, )) self._previous_node = node @@ -80,7 +80,7 @@ def visit(self, node): def function_lens(self) -> List[FuncLenIssue]: if isinstance(self._previous_node, (ast.FunctionDef, ast.AsyncFunctionDef)): func_length = self._find_func_len( - self._previous_node.lineno, self._n_lines + 1 + self._previous_node.lineno, self._n_lines + 1, ) self._function_lens.append(FuncLenIssue( @@ -91,7 +91,7 @@ def function_lens(self) -> List[FuncLenIssue]: origin_class=FUNC_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, func_len=func_length, - type=IssueType.FUNC_LEN + type=IssueType.FUNC_LEN, )) self._previous_node = None @@ -125,13 +125,13 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: bool_gatherer = BoolExpressionLensGatherer(path_to_file, cls.inspector_type) bool_gatherer.visit(tree) metrics.extend( - bool_gatherer.bool_expression_lens + bool_gatherer.bool_expression_lens, ) func_gatherer = FunctionLensGatherer(file_content, path_to_file, cls.inspector_type) func_gatherer.visit(tree) metrics.extend( - func_gatherer.function_lens + func_gatherer.function_lens, ) return metrics diff --git a/src/python/review/inspectors/pylint/pylint.py b/src/python/review/inspectors/pylint/pylint.py index fb9db581..e5a256cb 100644 --- a/src/python/review/inspectors/pylint/pylint.py +++ b/src/python/review/inspectors/pylint/pylint.py @@ -21,7 +21,7 @@ class PylintInspector(BaseInspector): supported_issue_types = ( IssueType.CODE_STYLE, IssueType.BEST_PRACTICES, - IssueType.ERROR_PRONE + IssueType.ERROR_PRONE, ) @classmethod @@ -31,7 +31,7 @@ def inspect(cls, path: Path, config: dict) -> List[CodeIssue]: '--load-plugins', 'pylint_django', f'--rcfile={PATH_PYLINT_CONFIG}', f'--msg-template={MSG_TEMPLATE}', - str(path) + str(path), ] output = run_in_subprocess(command) @@ -70,7 +70,7 @@ def parse(cls, output: str) -> List[CodeIssue]: origin_class=origin_class, description=description, inspector_type=cls.inspector_type, - type=issue_type + type=issue_type, )) return issues diff --git a/src/python/review/inspectors/radon/__init__.py b/src/python/review/inspectors/radon/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py new file mode 100644 index 00000000..fb2bf299 --- /dev/null +++ b/src/python/review/inspectors/radon/radon.py @@ -0,0 +1,51 @@ +import re +from pathlib import Path +from typing import List + +from src.python.review.common.subprocess_runner import run_in_subprocess +from src.python.review.inspectors.base_inspector import BaseInspector +from src.python.review.inspectors.inspector_type import InspectorType +from src.python.review.inspectors.issue import BaseIssue, IssueData, IssueType, MaintainabilityLackIssue +from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value +from src.python.review.inspectors.tips import get_maintainability_index_tip + + +class RadonInspector(BaseInspector): + inspector_type = InspectorType.RADON + + @classmethod + def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: + mi_command = [ + "radon", "mi", # compute the Maintainability Index score + "--max", "F", # set the maximum MI rank to display + "--show", # actual MI value is shown in results, alongside the rank + path, + ] + + mi_output = run_in_subprocess(mi_command) + return cls.mi_parse(mi_output) + + @classmethod + def mi_parse(cls, mi_output: str) -> List[BaseIssue]: + """ + Parses the results of the "mi" command. + Description: https://radon.readthedocs.io/en/latest/commandline.html#the-mi-command + + :param mi_output: "mi" command output. + :return: list of issues. + """ + row_re = re.compile(r"^(.*) - \w \((.*)\)$", re.M) + + issues: List[BaseIssue] = [] + for groups in row_re.findall(mi_output): + file_path = Path(groups[0]) + maintainability_lack = convert_percentage_of_value_to_lack_of_value(float(groups[1])) + + issue_data = IssueData.get_base_issue_data_dict(file_path, cls.inspector_type) + issue_data[IssueData.DESCRIPTION.value] = get_maintainability_index_tip() + issue_data[IssueData.MAINTAINABILITY_LACK.value] = maintainability_lack + issue_data[IssueData.ISSUE_TYPE.value] = IssueType.MAINTAINABILITY + + issues.append(MaintainabilityLackIssue(**issue_data)) + + return issues diff --git a/src/python/review/inspectors/spotbugs/spotbugs.py b/src/python/review/inspectors/spotbugs/spotbugs.py index e9cd1cb2..a2ee8b38 100644 --- a/src/python/review/inspectors/spotbugs/spotbugs.py +++ b/src/python/review/inspectors/spotbugs/spotbugs.py @@ -30,7 +30,7 @@ def _create_command(cls, path: Path) -> List[str]: PATH_SPOTBUGS_EXCLUDE, '-textui', '-medium', - str(path) + str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -100,5 +100,5 @@ def _parse_single_line(cls, line: str, file_name_to_path: Dict[str, Path]) -> Ba type=IssueType.ERROR_PRONE, origin_class=issue_class, description=short_desc, - inspector_type=cls.inspector_type + inspector_type=cls.inspector_type, ) diff --git a/src/python/review/inspectors/springlint/springlint.py b/src/python/review/inspectors/springlint/springlint.py index 14ce8c89..cc90c487 100644 --- a/src/python/review/inspectors/springlint/springlint.py +++ b/src/python/review/inspectors/springlint/springlint.py @@ -51,7 +51,7 @@ class SpringlintInspector(BaseInspector): 'cbo': 'class_objects_coupling', 'lcom': 'cohesion_lack', 'rfc': 'class_response', - 'nom': 'method_number' + 'nom': 'method_number', } metric_name_to_description = { @@ -61,7 +61,7 @@ class SpringlintInspector(BaseInspector): 'cbo': get_class_coupling_tip(), 'lcom': get_cohesion_tip(), 'rfc': get_class_response_tip(), - 'nom': get_method_number_tip() + 'nom': get_method_number_tip(), } metric_name_to_issue_type = { @@ -71,7 +71,7 @@ class SpringlintInspector(BaseInspector): 'cbo': IssueType.COUPLING, 'lcom': IssueType.COHESION, 'rfc': IssueType.CLASS_RESPONSE, - 'nom': IssueType.METHOD_NUMBER + 'nom': IssueType.METHOD_NUMBER, } @classmethod @@ -81,7 +81,7 @@ def _create_command(cls, path: Path, output_path: Path) -> List[str]: PATH_SPRINGLINT_JAR, '--output', str(output_path), '-otype', 'html', - '--project', str(path) + '--project', str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -137,7 +137,7 @@ def _parse_smells(cls, file_content: AnyStr, origin_path: str = '') -> List[Base origin_class=smell['name'], inspector_type=cls.inspector_type, type=IssueType.ARCHITECTURE, - description=smell['description'] + description=smell['description'], ) for smell in file_smell['smells']]) return issues diff --git a/src/python/review/inspectors/tips.py b/src/python/review/inspectors/tips.py index 5211b569..eac449c1 100644 --- a/src/python/review/inspectors/tips.py +++ b/src/python/review/inspectors/tips.py @@ -84,3 +84,8 @@ def get_method_number_tip() -> str: 'The file has too many methods inside and is complicated to understand. ' 'Consider its decomposition to smaller classes.' ) + + +# TODO: Need to improve the tip. +def get_maintainability_index_tip() -> str: + return 'The maintainability index is too low.' diff --git a/src/python/review/logging_config.py b/src/python/review/logging_config.py index ce3c47a0..ee55bf46 100644 --- a/src/python/review/logging_config.py +++ b/src/python/review/logging_config.py @@ -5,22 +5,22 @@ 'formatters': { 'common': { 'class': 'logging.Formatter', - 'format': '%(asctime)s | %(levelname)s | %(message)s' - } + 'format': '%(asctime)s | %(levelname)s | %(message)s', + }, }, 'handlers': { 'console': { 'class': 'logging.StreamHandler', 'level': 'DEBUG', 'formatter': 'common', - 'stream': sys.stdout + 'stream': sys.stdout, }, }, 'loggers': { '': { 'handlers': ['console'], - 'level': 'INFO' - } + 'level': 'INFO', + }, }, - 'disable_existing_loggers': False + 'disable_existing_loggers': False, } diff --git a/src/python/review/quality/evaluate_quality.py b/src/python/review/quality/evaluate_quality.py index d7ee0211..3861d3b3 100644 --- a/src/python/review/quality/evaluate_quality.py +++ b/src/python/review/quality/evaluate_quality.py @@ -41,6 +41,10 @@ LANGUAGE_TO_COHESION_RULE_CONFIG, CohesionRule, ) +from src.python.review.quality.rules.maintainability_scoring import ( + LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG, + MaintainabilityRule, +) def __get_available_rules(language: Language) -> List[Rule]: @@ -56,6 +60,7 @@ def __get_available_rules(language: Language) -> List[Rule]: ResponseRule(LANGUAGE_TO_RESPONSE_RULE_CONFIG[language]), WeightedMethodsRule(LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language]), CohesionRule(LANGUAGE_TO_COHESION_RULE_CONFIG[language]), + MaintainabilityRule(LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[language]), ] diff --git a/src/python/review/quality/model.py b/src/python/review/quality/model.py index 741c761b..6e64ea81 100644 --- a/src/python/review/quality/model.py +++ b/src/python/review/quality/model.py @@ -20,7 +20,7 @@ def __le__(self, other: 'QualityType') -> bool: QualityType.BAD: 0, QualityType.MODERATE: 1, QualityType.GOOD: 2, - QualityType.EXCELLENT: 3 + QualityType.EXCELLENT: 3, } return order[self] < order[other] @@ -81,12 +81,11 @@ def __str__(self): message_deltas_part = '' if self.quality_type != QualityType.EXCELLENT: - message_next_level_part = textwrap.dedent( - f"""\ - Next level: {self.next_quality_type.value} - Next level requirements: - """ - ) + message_next_level_part = f"""\ + Next level: {self.next_quality_type.value} + Next level requirements: + """ + message_next_level_part = textwrap.dedent(message_next_level_part) for rule in self.next_level_requirements: message_deltas_part += f'{rule.rule_type.value}: {rule.next_level_delta}\n' diff --git a/src/python/review/quality/rules/best_practices_scoring.py b/src/python/review/quality/rules/best_practices_scoring.py index f014d6e8..633387f2 100644 --- a/src/python/review/quality/rules/best_practices_scoring.py +++ b/src/python/review/quality/rules/best_practices_scoring.py @@ -16,7 +16,7 @@ class BestPracticesRuleConfig: common_best_practices_rule_config = BestPracticesRuleConfig( n_best_practices_moderate=3, n_best_practices_good=1, - n_files=1 + n_files=1, ) LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG = { @@ -59,7 +59,7 @@ def merge(self, other: 'BestPracticesRule') -> 'BestPracticesRule': config = BestPracticesRuleConfig( min(self.config.n_best_practices_moderate, other.config.n_best_practices_moderate), min(self.config.n_best_practices_good, other.config.n_best_practices_good), - n_files=self.config.n_files + other.config.n_files + n_files=self.config.n_files + other.config.n_files, ) result_rule = BestPracticesRule(config) result_rule.apply(self.n_best_practices + other.n_best_practices) diff --git a/src/python/review/quality/rules/boolean_length_scoring.py b/src/python/review/quality/rules/boolean_length_scoring.py index b2d7c347..2a33c327 100644 --- a/src/python/review/quality/rules/boolean_length_scoring.py +++ b/src/python/review/quality/rules/boolean_length_scoring.py @@ -16,13 +16,13 @@ class BooleanExpressionRuleConfig: common_boolean_expression_rule_config = BooleanExpressionRuleConfig( bool_expr_len_bad=10, bool_expr_len_moderate=7, - bool_expr_len_good=5 + bool_expr_len_good=5, ) java_boolean_expression_rule_config = BooleanExpressionRuleConfig( bool_expr_len_bad=8, bool_expr_len_moderate=6, - bool_expr_len_good=4 + bool_expr_len_good=4, ) LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG = { @@ -66,7 +66,7 @@ def merge(self, other: 'BooleanExpressionRule') -> 'BooleanExpressionRule': config = BooleanExpressionRuleConfig( min(self.config.bool_expr_len_bad, other.config.bool_expr_len_bad), min(self.config.bool_expr_len_moderate, other.config.bool_expr_len_moderate), - min(self.config.bool_expr_len_good, other.config.bool_expr_len_good) + min(self.config.bool_expr_len_good, other.config.bool_expr_len_good), ) result_rule = BooleanExpressionRule(config) result_rule.apply(max(self.bool_expr_len, other.bool_expr_len)) diff --git a/src/python/review/quality/rules/class_response_scoring.py b/src/python/review/quality/rules/class_response_scoring.py index 4bfd4800..d4b82fa9 100644 --- a/src/python/review/quality/rules/class_response_scoring.py +++ b/src/python/review/quality/rules/class_response_scoring.py @@ -14,7 +14,7 @@ class ResponseRuleConfig: common_response_rule_config = ResponseRuleConfig( response_moderate=69, - response_good=59 + response_good=59, ) LANGUAGE_TO_RESPONSE_RULE_CONFIG = { @@ -52,7 +52,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'ResponseRule') -> 'ResponseRule': config = ResponseRuleConfig( min(self.config.response_moderate, other.config.response_moderate), - min(self.config.response_good, other.config.response_good) + min(self.config.response_good, other.config.response_good), ) result_rule = ResponseRule(config) result_rule.apply(max(self.response, other.response)) diff --git a/src/python/review/quality/rules/code_style_scoring.py b/src/python/review/quality/rules/code_style_scoring.py index 882b0c5c..a1edda09 100644 --- a/src/python/review/quality/rules/code_style_scoring.py +++ b/src/python/review/quality/rules/code_style_scoring.py @@ -19,7 +19,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.JAVA + language=Language.JAVA, ) python_code_style_rule_config = CodeStyleRuleConfig( @@ -27,7 +27,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=5, - language=Language.PYTHON + language=Language.PYTHON, ) kotlin_code_style_rule_config = CodeStyleRuleConfig( @@ -35,7 +35,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.07, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.KOTLIN + language=Language.KOTLIN, ) js_code_style_rule_config = CodeStyleRuleConfig( @@ -43,7 +43,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.JAVA + language=Language.JAVA, ) LANGUAGE_TO_CODE_STYLE_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/coupling_scoring.py b/src/python/review/quality/rules/coupling_scoring.py index 83ea987b..7fb0a782 100644 --- a/src/python/review/quality/rules/coupling_scoring.py +++ b/src/python/review/quality/rules/coupling_scoring.py @@ -14,7 +14,7 @@ class CouplingRuleConfig: common_coupling_rule_config = CouplingRuleConfig( coupling_bad=30, - coupling_moderate=20 + coupling_moderate=20, ) LANGUAGE_TO_COUPLING_RULE_CONFIG = { @@ -52,7 +52,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'CouplingRule') -> 'CouplingRule': config = CouplingRuleConfig( min(self.config.coupling_bad, other.config.coupling_bad), - min(self.config.coupling_moderate, other.config.coupling_moderate) + min(self.config.coupling_moderate, other.config.coupling_moderate), ) result_rule = CouplingRule(config) result_rule.apply(max(self.coupling, other.coupling)) diff --git a/src/python/review/quality/rules/cyclomatic_complexity_scoring.py b/src/python/review/quality/rules/cyclomatic_complexity_scoring.py index 79fb69dc..162284b4 100644 --- a/src/python/review/quality/rules/cyclomatic_complexity_scoring.py +++ b/src/python/review/quality/rules/cyclomatic_complexity_scoring.py @@ -15,20 +15,20 @@ class CyclomaticComplexityRuleConfig: LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG = { Language.JAVA: CyclomaticComplexityRuleConfig( cc_value_bad=14, - cc_value_moderate=13 + cc_value_moderate=13, ), Language.KOTLIN: CyclomaticComplexityRuleConfig( cc_value_bad=12, - cc_value_moderate=11 + cc_value_moderate=11, ), Language.PYTHON: CyclomaticComplexityRuleConfig( cc_value_bad=10, - cc_value_moderate=9 + cc_value_moderate=9, ), Language.JS: CyclomaticComplexityRuleConfig( cc_value_bad=14, - cc_value_moderate=13 - ) + cc_value_moderate=13, + ), } diff --git a/src/python/review/quality/rules/error_prone_scoring.py b/src/python/review/quality/rules/error_prone_scoring.py index 7a6dee0e..8df9c157 100644 --- a/src/python/review/quality/rules/error_prone_scoring.py +++ b/src/python/review/quality/rules/error_prone_scoring.py @@ -12,7 +12,7 @@ class ErrorProneRuleConfig: common_error_prone_rule_config = ErrorProneRuleConfig( - n_error_prone_bad=0 + n_error_prone_bad=0, ) LANGUAGE_TO_ERROR_PRONE_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/function_length_scoring.py b/src/python/review/quality/rules/function_length_scoring.py index f607f679..e728e9c2 100644 --- a/src/python/review/quality/rules/function_length_scoring.py +++ b/src/python/review/quality/rules/function_length_scoring.py @@ -13,17 +13,17 @@ class FunctionLengthRuleConfig: LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG = { Language.JAVA: FunctionLengthRuleConfig( - func_len_bad=69 + func_len_bad=69, ), Language.KOTLIN: FunctionLengthRuleConfig( - func_len_bad=69 + func_len_bad=69, ), Language.PYTHON: FunctionLengthRuleConfig( - func_len_bad=49 + func_len_bad=49, ), Language.JS: FunctionLengthRuleConfig( - func_len_bad=69 - ) + func_len_bad=69, + ), } @@ -48,7 +48,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'FunctionLengthRule') -> 'FunctionLengthRule': config = FunctionLengthRuleConfig( - min(self.config.func_len_bad, other.config.func_len_bad) + min(self.config.func_len_bad, other.config.func_len_bad), ) result_rule = FunctionLengthRule(config) result_rule.apply(max(self.func_len, other.func_len)) diff --git a/src/python/review/quality/rules/inheritance_depth_scoring.py b/src/python/review/quality/rules/inheritance_depth_scoring.py index cdabda6b..a3531afc 100644 --- a/src/python/review/quality/rules/inheritance_depth_scoring.py +++ b/src/python/review/quality/rules/inheritance_depth_scoring.py @@ -12,7 +12,7 @@ class InheritanceDepthRuleConfig: common_inheritance_depth_rule_config = InheritanceDepthRuleConfig( - depth_bad=3 + depth_bad=3, ) LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/line_len_scoring.py b/src/python/review/quality/rules/line_len_scoring.py index d52b0382..b188576f 100644 --- a/src/python/review/quality/rules/line_len_scoring.py +++ b/src/python/review/quality/rules/line_len_scoring.py @@ -13,7 +13,7 @@ class LineLengthRuleConfig: common_line_length_rule_config = LineLengthRuleConfig( n_line_len_bad=0.05, - n_line_len_good=0.035 + n_line_len_good=0.035, ) LANGUAGE_TO_LINE_LENGTH_RULE_CONFIG = { @@ -54,7 +54,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'LineLengthRule') -> 'LineLengthRule': config = LineLengthRuleConfig( min(self.config.n_line_len_bad, other.config.n_line_len_bad), - min(self.config.n_line_len_good, other.config.n_line_len_good) + min(self.config.n_line_len_good, other.config.n_line_len_good), ) result_rule = LineLengthRule(config) result_rule.apply(self.n_line_len + other.n_line_len, self.n_lines + other.n_lines) diff --git a/src/python/review/quality/rules/maintainability_scoring.py b/src/python/review/quality/rules/maintainability_scoring.py new file mode 100644 index 00000000..456e7b0a --- /dev/null +++ b/src/python/review/quality/rules/maintainability_scoring.py @@ -0,0 +1,74 @@ +from dataclasses import dataclass +from typing import Optional + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.quality.model import Rule, QualityType + + +@dataclass +class MaintainabilityRuleConfig: + maintainability_lack_good: int + maintainability_lack_moderate: int + maintainability_lack_bad: int + + +# TODO: Need testing +# In Radon, the maintainability index is ranked as follows: +# 20-100: Very high +# 10-19: Medium +# 0-9: Extremely low +# Therefore, maintainability_lack_bad = 90, and maintainability_lack_moderate = 80. +common_maintainability_rule_config = MaintainabilityRuleConfig( + maintainability_lack_good=50, + maintainability_lack_moderate=80, + maintainability_lack_bad=90, +) + +LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG = { + Language.JAVA: common_maintainability_rule_config, + Language.PYTHON: common_maintainability_rule_config, + Language.KOTLIN: common_maintainability_rule_config, + Language.JS: common_maintainability_rule_config, +} + + +class MaintainabilityRule(Rule): + def __init__(self, config: MaintainabilityRuleConfig): + self.config = config + self.rule_type = IssueType.MAINTAINABILITY + self.maintainability_lack: Optional[int] = None + + def apply(self, maintainability_lack): + self.maintainability_lack = maintainability_lack + if maintainability_lack > self.config.maintainability_lack_bad: + self.quality_type = QualityType.BAD + self.next_level_delta = maintainability_lack - self.config.maintainability_lack_bad + elif maintainability_lack > self.config.maintainability_lack_moderate: + self.quality_type = QualityType.MODERATE + self.next_level_delta = maintainability_lack - self.config.maintainability_lack_moderate + elif maintainability_lack > self.config.maintainability_lack_good: + self.quality_type = QualityType.GOOD + self.next_level_delta = maintainability_lack - self.config.maintainability_lack_good + else: + self.quality_type = QualityType.EXCELLENT + self.next_level_delta = 0 + self.next_level_type = self.__get_next_quality_type() + + def __get_next_quality_type(self) -> QualityType: + if self.quality_type == QualityType.BAD: + return QualityType.MODERATE + elif self.quality_type == QualityType.MODERATE: + return QualityType.GOOD + return QualityType.EXCELLENT + + def merge(self, other: 'MaintainabilityRule') -> 'MaintainabilityRule': + config = MaintainabilityRuleConfig( + min(self.config.maintainability_lack_bad, other.config.maintainability_lack_bad), + min(self.config.maintainability_lack_moderate, other.config.maintainability_lack_moderate), + min(self.config.maintainability_lack_good, other.config.maintainability_lack_good), + ) + result_rule = MaintainabilityRule(config) + result_rule.apply(max(self.maintainability_lack, other.maintainability_lack)) + + return result_rule diff --git a/src/python/review/quality/rules/method_number_scoring.py b/src/python/review/quality/rules/method_number_scoring.py index 112390c3..bc15a497 100644 --- a/src/python/review/quality/rules/method_number_scoring.py +++ b/src/python/review/quality/rules/method_number_scoring.py @@ -16,7 +16,7 @@ class MethodNumberRuleConfig: common_method_number_rule_config = MethodNumberRuleConfig( method_number_bad=32, method_number_moderate=24, - method_number_good=20 + method_number_good=20, ) LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG = { @@ -60,7 +60,7 @@ def merge(self, other: 'MethodNumberRule') -> 'MethodNumberRule': config = MethodNumberRuleConfig( min(self.config.method_number_bad, other.config.method_number_bad), min(self.config.method_number_moderate, other.config.method_number_moderate), - min(self.config.method_number_good, other.config.method_number_good) + min(self.config.method_number_good, other.config.method_number_good), ) result_rule = MethodNumberRule(config) result_rule.apply(max(self.method_number, other.method_number)) diff --git a/src/python/review/quality/rules/weighted_methods_scoring.py b/src/python/review/quality/rules/weighted_methods_scoring.py index 777d7b2d..633c4839 100644 --- a/src/python/review/quality/rules/weighted_methods_scoring.py +++ b/src/python/review/quality/rules/weighted_methods_scoring.py @@ -16,7 +16,7 @@ class WeightedMethodsRuleConfig: common_weighted_methods_rule_config = WeightedMethodsRuleConfig( weighted_methods_bad=105, weighted_methods_moderate=85, - weighted_methods_good=70 + weighted_methods_good=70, ) LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG = { @@ -58,7 +58,7 @@ def merge(self, other: 'WeightedMethodsRule') -> 'WeightedMethodsRule': config = WeightedMethodsRuleConfig( min(self.config.weighted_methods_bad, other.config.weighted_methods_bad), min(self.config.weighted_methods_moderate, other.config.weighted_methods_moderate), - min(self.config.weighted_methods_good, other.config.weighted_methods_good) + min(self.config.weighted_methods_good, other.config.weighted_methods_good), ) result_rule = WeightedMethodsRule(config) result_rule.apply(max(self.weighted_methods, other.weighted_methods)) diff --git a/src/python/review/reviewers/common.py b/src/python/review/reviewers/common.py index 0d6688c4..d7385fe5 100644 --- a/src/python/review/reviewers/common.py +++ b/src/python/review/reviewers/common.py @@ -8,6 +8,7 @@ from src.python.review.inspectors.detekt.detekt import DetektInspector from src.python.review.inspectors.eslint.eslint import ESLintInspector from src.python.review.inspectors.flake8.flake8 import Flake8Inspector +from src.python.review.inspectors.radon.radon import RadonInspector from src.python.review.inspectors.issue import BaseIssue from src.python.review.inspectors.pmd.pmd import PMDInspector from src.python.review.inspectors.pyast.python_ast import PythonAstInspector @@ -24,6 +25,7 @@ PylintInspector(), Flake8Inspector(), PythonAstInspector(), + RadonInspector(), ], Language.JAVA: [ CheckstyleInspector(), @@ -36,7 +38,7 @@ ], Language.JS: [ ESLintInspector(), - ] + ], } @@ -76,12 +78,12 @@ def perform_language_review(metadata: Metadata, file_review_results.append(FileReviewResult( file_metadata.path, issues, - quality + quality, )) return ReviewResult( file_review_results, - general_quality + general_quality, ) diff --git a/src/python/review/reviewers/perform_review.py b/src/python/review/reviewers/perform_review.py index 678e503a..61f9b1a7 100644 --- a/src/python/review/reviewers/perform_review.py +++ b/src/python/review/reviewers/perform_review.py @@ -13,7 +13,7 @@ from src.python.review.reviewers.utils.print_review import ( print_review_result_as_json, print_review_result_as_multi_file_json, - print_review_result_as_text + print_review_result_as_text, ) logger: Final = logging.getLogger(__name__) @@ -31,7 +31,7 @@ class PathNotExists(Exception): Language.PYTHON: perform_python_review, Language.JAVA: partial(perform_language_review, language=Language.JAVA), Language.KOTLIN: partial(perform_language_review, language=Language.KOTLIN), - Language.JS: partial(perform_language_review, language=Language.JS) + Language.JS: partial(perform_language_review, language=Language.JS), } diff --git a/src/python/review/reviewers/utils/code_statistics.py b/src/python/review/reviewers/utils/code_statistics.py index 41dc25a2..d8dd7b38 100644 --- a/src/python/review/reviewers/utils/code_statistics.py +++ b/src/python/review/reviewers/utils/code_statistics.py @@ -17,6 +17,7 @@ class CodeStatistics: max_cyclomatic_complexity: int max_cohesion_lack: int + max_maintainability_lack: int max_func_len: int max_bool_expr_len: int @@ -40,6 +41,7 @@ def issue_type_to_statistics_dict(self) -> Dict[IssueType, int]: IssueType.CYCLOMATIC_COMPLEXITY: self.max_cyclomatic_complexity, IssueType.COHESION: self.max_cohesion_lack, + IssueType.MAINTAINABILITY: self.max_maintainability_lack, IssueType.FUNC_LEN: self.max_func_len, IssueType.BOOL_EXPR_LEN: self.max_bool_expr_len, @@ -73,7 +75,7 @@ def get_code_style_lines(issues: List[BaseIssue]) -> int: def __get_max_measure_by_issue_type(issue_type: IssueType, issues: List[BaseIssue]) -> int: return max(map( lambda issue: issue.measure(), - filter(lambda issue: issue.type == issue_type, issues) + filter(lambda issue: issue.type == issue_type, issues), ), default=0) @@ -85,6 +87,7 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic func_lens = __get_max_measure_by_issue_type(IssueType.FUNC_LEN, issues) cyclomatic_complexities = __get_max_measure_by_issue_type(IssueType.CYCLOMATIC_COMPLEXITY, issues) cohesion_lacks = __get_max_measure_by_issue_type(IssueType.COHESION, issues) + maintainabilities = __get_max_measure_by_issue_type(IssueType.MAINTAINABILITY, issues) # Actually, we expect only one issue with each of the following metrics. inheritance_depths = __get_max_measure_by_issue_type(IssueType.INHERITANCE_DEPTH, issues) @@ -101,6 +104,7 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic max_func_len=func_lens, n_line_len=issue_type_counter[IssueType.LINE_LEN], max_cohesion_lack=cohesion_lacks, + max_maintainability_lack=maintainabilities, max_cyclomatic_complexity=cyclomatic_complexities, inheritance_depth=inheritance_depths, class_response=class_responses, diff --git a/src/python/review/reviewers/utils/issues_filter.py b/src/python/review/reviewers/utils/issues_filter.py index ee9be2a1..a952998c 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -12,6 +12,7 @@ from src.python.review.quality.rules.method_number_scoring import LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG from src.python.review.quality.rules.weighted_methods_scoring import LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG from src.python.review.quality.rules.cohesion_scoring import LANGUAGE_TO_COHESION_RULE_CONFIG +from src.python.review.quality.rules.maintainability_scoring import LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, int]: @@ -25,6 +26,9 @@ def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, IssueType.CLASS_RESPONSE: LANGUAGE_TO_RESPONSE_RULE_CONFIG[language].response_good, IssueType.WEIGHTED_METHOD: LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language].weighted_methods_good, IssueType.COHESION: LANGUAGE_TO_COHESION_RULE_CONFIG[language].cohesion_lack_good, + IssueType.MAINTAINABILITY: LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[ + language + ].maintainability_lack_good, } diff --git a/src/python/review/reviewers/utils/print_review.py b/src/python/review/reviewers/utils/print_review.py index 76c2de04..9da0e35b 100644 --- a/src/python/review/reviewers/utils/print_review.py +++ b/src/python/review/reviewers/utils/print_review.py @@ -24,7 +24,7 @@ def print_review_result_as_text(review_result: ReviewResult, for issue in sorted_issues: line_text = linecache.getline( str(issue.file_path), - issue.line_no + issue.line_no, ).strip() print(f'{issue.line_no} : ' @@ -80,7 +80,7 @@ def print_review_result_as_multi_file_json(review_result: ReviewResult) -> None: 'file_name': str(file_review_result.file_path), 'quality': { 'code': quality_value, - 'text': f'Code quality (beta): {quality_value}' + 'text': f'Code quality (beta): {quality_value}', }, 'issues': [], } @@ -106,7 +106,7 @@ def print_review_result_as_multi_file_json(review_result: ReviewResult) -> None: 'code': quality_value, 'text': f'Code quality (beta): {quality_value}', }, - 'file_review_results': file_review_result_jsons + 'file_review_results': file_review_result_jsons, } print(json.dumps(output_json)) diff --git a/src/python/review/run_tool.py b/src/python/review/run_tool.py index d0ba0891..1b879972 100644 --- a/src/python/review/run_tool.py +++ b/src/python/review/run_tool.py @@ -155,7 +155,7 @@ def main() -> int: inspectors_config = { 'language_version': LanguageVersion(args.language_version) if args.language_version is not None else None, - 'n_cpu': n_cpu + 'n_cpu': n_cpu, } config = ApplicationConfig( diff --git a/test/python/functional_tests/conftest.py b/test/python/functional_tests/conftest.py index cec7b0a0..eea5a9ab 100644 --- a/test/python/functional_tests/conftest.py +++ b/test/python/functional_tests/conftest.py @@ -43,7 +43,7 @@ def build(self) -> List[str]: command.extend([ '--n_cpu', str(self.n_cpu), '-f', self.format, - str(self.path) + str(self.path), ]) if self.start_line is not None: diff --git a/test/python/functional_tests/test_different_languages.py b/test/python/functional_tests/test_different_languages.py index 0cfbf80d..935df678 100644 --- a/test/python/functional_tests/test_different_languages.py +++ b/test/python/functional_tests/test_different_languages.py @@ -10,7 +10,7 @@ def test_python(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -26,7 +26,7 @@ def test_java(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -42,7 +42,7 @@ def test_kotlin(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -59,7 +59,7 @@ def test_all_java_inspectors(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_disable.py b/test/python/functional_tests/test_disable.py index 08655c1e..ffacbec1 100644 --- a/test/python/functional_tests/test_disable.py +++ b/test/python/functional_tests/test_disable.py @@ -10,7 +10,7 @@ def test_disable_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -20,7 +20,7 @@ def test_disable_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_duplicates.py b/test/python/functional_tests/test_duplicates.py index 13ff2a83..153a3ac7 100644 --- a/test/python/functional_tests/test_duplicates.py +++ b/test/python/functional_tests/test_duplicates.py @@ -13,7 +13,7 @@ def test_allow_duplicates(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout_allow_duplicates = process.stdout.decode() @@ -22,7 +22,7 @@ def test_allow_duplicates(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout_filter_duplicates = process.stdout.decode() diff --git a/test/python/functional_tests/test_exit_code.py b/test/python/functional_tests/test_exit_code.py index 70584387..bd24d250 100644 --- a/test/python/functional_tests/test_exit_code.py +++ b/test/python/functional_tests/test_exit_code.py @@ -11,7 +11,7 @@ def test_exit_code_zero(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 0 @@ -24,7 +24,7 @@ def test_exit_code_one(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 1 @@ -37,7 +37,7 @@ def test_exit_code_two(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 2 diff --git a/test/python/functional_tests/test_file_or_project.py b/test/python/functional_tests/test_file_or_project.py index 56bb1251..bd9d1d74 100644 --- a/test/python/functional_tests/test_file_or_project.py +++ b/test/python/functional_tests/test_file_or_project.py @@ -11,7 +11,7 @@ def test_inspect_file_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -27,7 +27,7 @@ def test_inspect_project_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_multi_file_project.py b/test/python/functional_tests/test_multi_file_project.py index db63472f..246f4f42 100644 --- a/test/python/functional_tests/test_multi_file_project.py +++ b/test/python/functional_tests/test_multi_file_project.py @@ -6,30 +6,30 @@ EXPECTED_JSON = { 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, 'file_review_results': [ { 'file_name': '__init__.py', 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, - 'issues': [] + 'issues': [], }, { 'file_name': 'one.py', 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, - 'issues': [] + 'issues': [], }, { 'file_name': 'other.py', 'quality': { 'code': 'GOOD', - 'text': 'Code quality (beta): GOOD' + 'text': 'Code quality (beta): GOOD', }, 'issues': [ { @@ -38,7 +38,7 @@ 'line': 'a = 1', 'line_number': 2, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -46,7 +46,7 @@ 'line': 'b = 2', 'line_number': 3, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -54,11 +54,11 @@ 'line': 'c = 3', 'line_number': 4, 'column_number': 5, - 'category': 'BEST_PRACTICES' - } - ] + 'category': 'BEST_PRACTICES', + }, + ], }, - ] + ], } @@ -73,7 +73,7 @@ def test_json_format(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() print(stdout) diff --git a/test/python/functional_tests/test_range_of_lines.py b/test/python/functional_tests/test_range_of_lines.py index bcadae0b..57b5530d 100644 --- a/test/python/functional_tests/test_range_of_lines.py +++ b/test/python/functional_tests/test_range_of_lines.py @@ -10,7 +10,7 @@ EXPECTED_JSON = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'category': 'CODE_STYLE', @@ -30,16 +30,16 @@ 'column_number': 2, 'line': 'c=a + b', 'line_number': 4, - 'text': 'Exactly one space required around assignment' - } - ] + 'text': 'Exactly one space required around assignment', + }, + ], } NO_ISSUES_JSON = { 'quality': { 'code': 'EXCELLENT', 'text': 'Code quality (beta): EXCELLENT'}, - 'issues': [] + 'issues': [], } @@ -85,8 +85,8 @@ def test_range_filter_when_start_line_is_not_first( 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json_with_one_issue @@ -145,7 +145,7 @@ def test_range_filter_when_end_line_is_first( expected_json_with_one_issue = { 'quality': { 'code': 'MODERATE', - 'text': 'Code quality (beta): MODERATE' + 'text': 'Code quality (beta): MODERATE', }, 'issues': [{ 'code': 'C0326', @@ -153,8 +153,8 @@ def test_range_filter_when_end_line_is_first( 'line': 'a=10', 'line_number': 1, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json_with_one_issue @@ -211,7 +211,7 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( expected_json = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'code': 'C0326', @@ -219,15 +219,15 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( 'line': 'b=20', 'line_number': 2, 'column_number': 2, - 'category': 'CODE_STYLE' + 'category': 'CODE_STYLE', }, { 'code': 'C0326', 'text': 'Exactly one space required around assignment', 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json diff --git a/test/python/functional_tests/test_single_file_json_format.py b/test/python/functional_tests/test_single_file_json_format.py index d71c115b..48d8f14d 100644 --- a/test/python/functional_tests/test_single_file_json_format.py +++ b/test/python/functional_tests/test_single_file_json_format.py @@ -12,8 +12,8 @@ 'type': 'object', 'properties': { 'code': {'type': 'string'}, - 'text': {'type': 'string'} - } + 'text': {'type': 'string'}, + }, }, 'issues': { 'type': 'array', @@ -25,11 +25,11 @@ 'column_number': {'type': 'number'}, 'line': {'type': 'string'}, 'line_number': {'type': 'number'}, - 'text': {'type': 'string'} - } - } - } - } + 'text': {'type': 'string'}, + }, + }, + }, + }, } @@ -43,7 +43,7 @@ def test_json_format(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() diff --git a/test/python/functional_tests/test_verbosity.py b/test/python/functional_tests/test_verbosity.py index 67df44eb..bed69f90 100644 --- a/test/python/functional_tests/test_verbosity.py +++ b/test/python/functional_tests/test_verbosity.py @@ -13,7 +13,7 @@ def test_disable_logs_text(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() output = output.lower() @@ -33,7 +33,7 @@ def test_disable_logs_json(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -49,7 +49,7 @@ def test_enable_all_logs(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() output = output.lower() diff --git a/test/python/inspectors/conftest.py b/test/python/inspectors/conftest.py index b2f8773c..d3df943c 100644 --- a/test/python/inspectors/conftest.py +++ b/test/python/inspectors/conftest.py @@ -31,16 +31,16 @@ def branch_info_response() -> Dict[str, Any]: 'reachability': 1, }, 'canCreateReview': { - 'isAllowed': True + 'isAllowed': True, }, 'stats': { 'parentBranch': 'bar', 'commitsAhead': 0, - 'commitsBehind': 0 + 'commitsBehind': 0, }, 'mergeInfo': {}, - 'isPullRequest': False - } + 'isPullRequest': False, + }, } @@ -52,15 +52,15 @@ def ownership_summary_response() -> Dict[str, Any]: { 'filePath': '/foo.py', 'state': 0, - 'userId': None + 'userId': None, }, { 'filePath': '/bar/baz.py', 'state': 0, - 'userId': None - } - ] - } + 'userId': None, + }, + ], + }, } @@ -74,6 +74,7 @@ class IssuesTestInfo: n_bool_expr_len: int = 0 n_other_complexity: int = 0 n_cohesion: int = 0 + n_maintainability: int = 0 def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: @@ -88,6 +89,7 @@ def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: n_bool_expr_len=counter[IssueType.BOOL_EXPR_LEN], n_other_complexity=counter[IssueType.COMPLEXITY], n_cohesion=counter[IssueType.COHESION], + n_maintainability=counter[IssueType.MAINTAINABILITY], ) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index c4750ace..be211040 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -115,7 +115,7 @@ def test_choose_issue_type(): expected_issue_types = [ IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, - IssueType.CODE_STYLE + IssueType.CODE_STYLE, ] issue_types = list(map(Flake8Inspector.choose_issue_type, error_codes)) diff --git a/test/python/inspectors/test_local_review.py b/test/python/inspectors/test_local_review.py index f32caab3..e1e028e6 100644 --- a/test/python/inspectors/test_local_review.py +++ b/test/python/inspectors/test_local_review.py @@ -14,7 +14,7 @@ 'allow_duplicates', 'disable', 'format', - 'handler' + 'handler', ]) @@ -24,7 +24,7 @@ def config() -> ApplicationConfig: disabled_inspectors={InspectorType.INTELLIJ}, allow_duplicates=False, n_cpu=1, - inspectors_config=dict(n_cpu=1) + inspectors_config=dict(n_cpu=1), ) diff --git a/test/python/inspectors/test_out_of_range_issues.py b/test/python/inspectors/test_out_of_range_issues.py index 016928b7..3f73d63d 100644 --- a/test/python/inspectors/test_out_of_range_issues.py +++ b/test/python/inspectors/test_out_of_range_issues.py @@ -44,7 +44,7 @@ def test_out_of_range_issues_when_the_same_borders() -> None: first_line_issues = [ create_code_issue_by_line(1), create_code_issue_by_line(1), - create_code_issue_by_line(1) + create_code_issue_by_line(1), ] assert filter_out_of_range_issues(first_line_issues, diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index e142e8aa..ab807765 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -72,7 +72,7 @@ def test_choose_issue_type(): IssueType.BEST_PRACTICES, IssueType.BEST_PRACTICES, IssueType.CODE_STYLE, - IssueType.ERROR_PRONE + IssueType.ERROR_PRONE, ] issue_types = list( diff --git a/test/python/inspectors/test_radon_inspector.py b/test/python/inspectors/test_radon_inspector.py new file mode 100644 index 00000000..72fd03e7 --- /dev/null +++ b/test/python/inspectors/test_radon_inspector.py @@ -0,0 +1,52 @@ +from textwrap import dedent + +import pytest + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.inspectors.radon.radon import RadonInspector +from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues +from test.python.inspectors import PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +from src.python.review.inspectors.tips import get_maintainability_index_tip + +FILE_NAMES_AND_N_ISSUES = [ + ("case13_complex_logic.py", 1), + ("case13_complex_logic_2.py", 1), + ("case8_good_class.py", 0), +] + + +@pytest.mark.parametrize(("file_name", "n_issues"), FILE_NAMES_AND_N_ISSUES) +def test_file_with_issues(file_name: str, n_issues: int): + inspector = RadonInspector() + + path_to_file = PYTHON_DATA_FOLDER / file_name + with use_file_metadata(path_to_file) as file_metadata: + issues = inspector.inspect(file_metadata.path, {}) + issues = filter_low_measure_issues(issues, Language.PYTHON) + + assert len(issues) == n_issues + + +def test_mi_parse(): + file_name = "test.py" + output = f"""\ + {file_name} - C (4.32) + {file_name} - B (13.7) + {file_name} - A (70.0) + """ + output = dedent(output) + + issues = RadonInspector.mi_parse(output) + + assert all(str(issue.file_path) == file_name for issue in issues) + assert [issue.line_no for issue in issues] == [1, 1, 1] + assert [issue.column_no for issue in issues] == [1, 1, 1] + assert [issue.description for issue in issues] == [get_maintainability_index_tip()] * 3 + assert [issue.type for issue in issues] == [ + IssueType.MAINTAINABILITY, + IssueType.MAINTAINABILITY, + IssueType.MAINTAINABILITY, + ] + assert [issue.maintainability_lack for issue in issues] == [95, 86, 30]