diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 54fe8f08..912f8a92 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,7 +22,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 --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=R504,A003,E800,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: | @@ -32,3 +32,19 @@ jobs: - name: Test with pytest run: | pytest + + - name: Install review module + run: | + pip install . + + - name: Check installed module can run python linters + run: | + review setup.py + + - name: Check installed module can run java linters + run: | + review test/resources/inspectors/java/test_algorithm_with_scanner.java + + - name: Check installed module can run js linters + run: | + review test/resources/inspectors/js/case0_no_issues.js diff --git a/README.md b/README.md index 4f9856dc..24db74c0 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. @@ -193,7 +196,7 @@ __Note__: If you have `ModuleNotFoundError` while you try to run tests, please c __Note__: We use [eslint](https://eslint.org/) and [open-jdk 11](https://openjdk.java.net/projects/jdk/11/) in the tests. Please, set up the environment before running the tests. -You can see en example of the environment configuration in +You can see en example of the environment configuration in the [Dockerfile](Dockerfile) file. Use `pytest` from the root directory to run __ALL__ tests. diff --git a/VERSION.md b/VERSION.md index 524cb552..26aaba0e 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1 +1 @@ -1.1.1 +1.2.0 diff --git a/requirements-test.txt b/requirements-test.txt index c2bf26e0..cc876366 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -2,4 +2,8 @@ pytest==6.2.3 pytest-runner==5.2 pytest-subtests==0.4.0 jsonschema==3.2.0 -pandas==1.2.3 \ No newline at end of file +pandas==1.2.3 +django==3.2 +pylint==2.7.4 +requests==2.25.1 +setuptools==56.0.0 \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index a859e972..484495a5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,12 @@ flake8-return==1.1.2 flake8-spellcheck==0.24.0 mccabe==0.6.1 pep8-naming==0.11.1 +wps-light==0.15.2 +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.2 diff --git a/setup.py b/setup.py index 2e1e7717..8b23d17a 100644 --- a/setup.py +++ b/setup.py @@ -19,13 +19,11 @@ def get_version() -> str: def get_inspectors_additional_files() -> List[str]: inspectors_path = current_dir / 'src' / 'python' / 'review' / 'inspectors' - result = [] for root, _, files in os.walk(inspectors_path): for file in files: - file_path = Path(root) / file - if not file_path.name.endswith('.py'): - result.append(str(file_path)) + if not file.endswith('.py'): + result.append(str(Path(root) / file)) return result @@ -45,22 +43,28 @@ 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' + '*.unit_tests', + '*.unit_tests.*', + 'unit_tests.*', + 'unit_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=src.python.review.run_tool:main' - ] - } + 'review=src.python.review.run_tool:main', + ], + }, ) diff --git a/test/resources/functional_tests/different_languages/python/__init__.py b/src/python/common/__init__.py similarity index 100% rename from test/resources/functional_tests/different_languages/python/__init__.py rename to src/python/common/__init__.py diff --git a/src/python/common/tool_arguments.py b/src/python/common/tool_arguments.py new file mode 100644 index 00000000..db0c77da --- /dev/null +++ b/src/python/common/tool_arguments.py @@ -0,0 +1,74 @@ +from dataclasses import dataclass +from enum import Enum, unique +from typing import List, Optional + +from src.python.review.application_config import LanguageVersion +from src.python.review.inspectors.inspector_type import InspectorType + + +@unique +class VerbosityLevel(Enum): + """ + Same meaning as the logging level. Should be used in command-line args. + """ + DEBUG = 3 + INFO = 2 + ERROR = 1 + DISABLE = 0 + + @classmethod + def values(cls) -> List[int]: + return [member.value for member in VerbosityLevel] + + +@dataclass(frozen=True) +class ArgumentsInfo: + short_name: Optional[str] + long_name: str + description: str + + +@unique +class RunToolArgument(Enum): + VERBOSITY = ArgumentsInfo('-v', '--verbosity', + 'Choose logging level: ' + f'{VerbosityLevel.ERROR.value} - ERROR; ' + f'{VerbosityLevel.INFO.value} - INFO; ' + f'{VerbosityLevel.DEBUG.value} - DEBUG; ' + f'{VerbosityLevel.DISABLE.value} - disable logging; ' + 'default is 0') + + inspectors = [inspector.lower() for inspector in InspectorType.available_values()] + disabled_inspectors_example = f'-d {inspectors[0].lower()},{inspectors[1].lower()}' + + DISABLE = ArgumentsInfo('-d', '--disable', + 'Disable inspectors. ' + f'Available values: {", ".join(inspectors)}. ' + f'Example: {disabled_inspectors_example}') + + DUPLICATES = ArgumentsInfo(None, '--allow-duplicates', + 'Allow duplicate issues found by different linters. ' + 'By default, duplicates are skipped.') + + LANG_VERSION = ArgumentsInfo(None, '--language-version', + 'Specify the language version for JAVA inspectors.' + 'Available values are: ' + f'{LanguageVersion.PYTHON_3.value}, {LanguageVersion.JAVA_8.value}, ' + f'{LanguageVersion.JAVA_11.value}, {LanguageVersion.KOTLIN.value}.') + + CPU = ArgumentsInfo(None, '--n-cpu', + 'Specify number of cpu that can be used to run inspectors') + + PATH = ArgumentsInfo(None, 'path', 'Path to file or directory to inspect.') + + FORMAT = ArgumentsInfo('-f', '--format', + 'The output format. Default is JSON.') + + START_LINE = ArgumentsInfo('-s', '--start-line', + 'The first line to be analyzed. It starts from 1.') + + END_LINE = ArgumentsInfo('-e', '--end-line', 'The end line to be analyzed or None.') + + NEW_FORMAT = ArgumentsInfo(None, '--new-format', + 'The argument determines whether the tool ' + 'should use the new format') diff --git a/src/python/review/application_config.py b/src/python/review/application_config.py index c678605f..6032aef3 100644 --- a/src/python/review/application_config.py +++ b/src/python/review/application_config.py @@ -1,7 +1,8 @@ from dataclasses import dataclass from enum import Enum, unique -from typing import Optional, Set, List +from typing import List, Optional, Set +from src.python.review.common.file_system import Extension from src.python.review.inspectors.inspector_type import InspectorType @@ -22,7 +23,30 @@ class LanguageVersion(Enum): JAVA_8 = 'java8' JAVA_9 = 'java9' JAVA_11 = 'java11' + PYTHON_3 = 'python3' + KOTLIN = 'kotlin' @classmethod def values(cls) -> List[str]: return [member.value for member in cls.__members__.values()] + + @classmethod + def language_to_extension_dict(cls) -> dict: + return {cls.PYTHON_3.value: Extension.PY.value, + cls.JAVA_7.value: Extension.JAVA.value, + cls.JAVA_8.value: Extension.JAVA.value, + cls.JAVA_9.value: Extension.JAVA.value, + cls.JAVA_11.value: Extension.JAVA.value, + cls.KOTLIN.value: Extension.KT.value} + + @classmethod + def language_by_extension(cls, lang: str) -> str: + return cls.language_to_extension_dict()[lang] + + def is_java(self) -> bool: + return ( + self == LanguageVersion.JAVA_7 + or self == LanguageVersion.JAVA_8 + or self == LanguageVersion.JAVA_9 + or self == LanguageVersion.JAVA_11 + ) diff --git a/src/python/review/common/file_system.py b/src/python/review/common/file_system.py index 6decb410..5c53d9d4 100644 --- a/src/python/review/common/file_system.py +++ b/src/python/review/common/file_system.py @@ -4,7 +4,7 @@ from contextlib import contextmanager from enum import Enum, unique from pathlib import Path -from typing import List, Union, Callable +from typing import Callable, List, Union @unique @@ -65,19 +65,16 @@ def new_temp_dir() -> Path: def create_file(file_path: Union[str, Path], content: str): file_path = Path(file_path) - create_directory(os.path.dirname(file_path)) - with open(file_path, 'w') as f: - f.write(content) - - -def create_directory(directory: str) -> None: - os.makedirs(directory, exist_ok=True) + os.makedirs(os.path.dirname(file_path), exist_ok=True) + with open(file_path, 'w+') as f: + f.writelines(content) + yield Path(file_path) 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/checkstyle/files/config.xml b/src/python/review/inspectors/checkstyle/files/config.xml index 00222e43..81fbc370 100644 --- a/src/python/review/inspectors/checkstyle/files/config.xml +++ b/src/python/review/inspectors/checkstyle/files/config.xml @@ -44,8 +44,9 @@ - - + + + @@ -87,16 +88,42 @@ + + + + + + + + + + + + + + + + - - - - - - - - + + + + + + + + + diff --git a/src/python/review/inspectors/common.py b/src/python/review/inspectors/common.py new file mode 100644 index 00000000..a307bd96 --- /dev/null +++ b/src/python/review/inspectors/common.py @@ -0,0 +1,23 @@ +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) + + +# TODO: When upgrading to python 3.9+, replace it with removeprefix. +# See: https://docs.python.org/3.9/library/stdtypes.html#str.removeprefix +def remove_prefix(text: str, prefix: str) -> str: + """ + Removes the prefix if it is present, otherwise returns the original string. + """ + if text.startswith(prefix): + return text[len(prefix):] + return text diff --git a/src/python/review/inspectors/detekt/detekt.py b/src/python/review/inspectors/detekt/detekt.py index 741a334d..041c6b2d 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: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/detekt/issue_types.py b/src/python/review/inspectors/detekt/issue_types.py index e05d1a30..2379bba8 100644 --- a/src/python/review/inspectors/detekt/issue_types.py +++ b/src/python/review/inspectors/detekt/issue_types.py @@ -15,7 +15,7 @@ # complexity: 'ComplexCondition': IssueType.BOOL_EXPR_LEN, 'ComplexInterface': IssueType.COMPLEXITY, - 'ComplexMethod': IssueType.COMPLEXITY, + 'ComplexMethod': IssueType.CYCLOMATIC_COMPLEXITY, 'LabeledExpression': IssueType.COMPLEXITY, 'LargeClass': IssueType.COMPLEXITY, 'LongMethod': IssueType.FUNC_LEN, diff --git a/src/python/review/inspectors/eslint/eslint.py b/src/python/review/inspectors/eslint/eslint.py index 19d801cf..c7d0b1b4 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 6270ce48..f0452e5a 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -1,5 +1,8 @@ [flake8] disable_noqa=True + +dictionaries=en_US,python,technical,django + ignore=W291, # trailing whitespaces W292, # no newline at end of file W293, # blank line contains whitespaces @@ -13,3 +16,47 @@ ignore=W291, # trailing whitespaces E301, E302, E303, E304, E305, # problem with stepik templates E402, # module level import not at top of file I100, # Import statements are in the wrong order + # WPS: Naming + WPS110, # Forbid blacklisted variable names. + WPS111, # Forbid short variable or module names. + WPS112, # Forbid private name pattern. + WPS114, # Forbid names with underscored numbers pattern. + WPS125, # Forbid variable or module names which shadow builtin names. TODO: Collision with flake8-builtins + # WPS: Consistency + WPS303, # Forbid underscores in numbers. + WPS305, # Forbid f strings. + WPS306, # Forbid writing classes without base classes. + WPS317, # Forbid incorrect indentation for parameters. (Because of the unnecessary strictness) + WPS318, # Forbid extra indentation. TODO: Collision with standard flake8 indentation check + WPS319, # Forbid brackets in the wrong position. (Because of the unnecessary strictness) + WPS323, # Forbid % formatting on strings. + WPS324, # Enforce consistent return statements. TODO: Collision with flake8-return + WPS335, # Forbid wrong for loop iter targets. + WPS347, # Forbid imports that may cause confusion outside of the module. (controversial) + WPS358, # Forbid using float zeros: 0.0. + WPS359, # Forbids to unpack iterable objects to lists. (Because of its similarity to "WPS414") + WPS362, # Forbid assignment to a subscript slice. + # WPS: Best practices + WPS404, # Forbid complex defaults. TODO: Collision with "B006" + WPS420, # Forbid some python keywords. + WPS421, # Forbid calling some built-in functions.(e.g., print) + WPS429, # Forbid multiple assignments on the same line. + WPS430, # Forbid nested functions. + WPS431, # Forbid nested classes. + WPS435, # Forbid multiplying lists. + # WPS: Refactoring + WPS518, # Forbid implicit enumerate() calls. TODO: Collision with "C0200" + WPS527, # Require tuples as arguments for frozenset. + # WPS: OOP + WPS602, # Forbid @staticmethod decorator. + # flake8-string-format + P101, # format string does contain unindexed parameters + P102, # docstring does contain unindexed parameters + P103, # other string does contain unindexed parameters + F405, # Name may be undefined, or defined from star imports (Collision with the stricter "F403") + F522, # unused named arguments. TODO: Collision with "P302" + F523, # unused positional arguments. TODO: Collision with "P301" + 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 2 diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 6ea27f09..c5dfd274 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -5,9 +5,17 @@ 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.common import convert_percentage_of_value_to_lack_of_value from src.python.review.inspectors.flake8.issue_types import CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.issue import BaseIssue, CodeIssue, CyclomaticComplexityIssue, IssueType, IssueData +from src.python.review.inspectors.issue import ( + BaseIssue, + CodeIssue, + CohesionIssue, + CyclomaticComplexityIssue, + IssueData, + IssueType, +) from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip logger = logging.getLogger(__name__) @@ -32,7 +40,8 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: f'--config={PATH_FLAKE8_CONFIG}', f'--whitelist={PATH_FLAKE8_SPELLCHECK_WHITELIST}', '--max-complexity', '0', - path + '--cohesion-below', '100', + path, ] output = run_in_subprocess(command) return cls.parse(output) @@ -41,12 +50,14 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: def parse(cls, output: str) -> List[BaseIssue]: row_re = re.compile(r'^(.*):(\d+):(\d+):([A-Z]+\d{3}):(.*)$', re.M) cc_description_re = re.compile(r"'(.+)' is too complex \((\d+)\)") + cohesion_description_re = re.compile(r"class has low \((\d*\.?\d*)%\) cohesion") issues: List[BaseIssue] = [] for groups in row_re.findall(output): description = groups[4] origin_class = groups[3] cc_match = cc_description_re.match(description) + cohesion_match = cohesion_description_re.match(description) file_path = Path(groups[0]) line_no = int(groups[1]) @@ -56,26 +67,39 @@ def parse(cls, output: str) -> List[BaseIssue]: line_number=line_no, column_number=column_number, origin_class=origin_class) - if cc_match is not None: - issue_data['description'] = get_cyclomatic_complexity_tip() - issue_data['cc_value'] = int(cc_match.groups()[1]) - issue_data['type'] = IssueType.CYCLOMATIC_COMPLEXITY + if cc_match is not None: # mccabe: cyclomatic complexity + issue_data[IssueData.DESCRIPTION.value] = get_cyclomatic_complexity_tip() + issue_data[IssueData.CYCLOMATIC_COMPLEXITY.value] = int(cc_match.groups()[1]) + issue_data[IssueData.ISSUE_TYPE.value] = IssueType.CYCLOMATIC_COMPLEXITY 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] = 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: issue_type = cls.choose_issue_type(origin_class) - issue_data['type'] = issue_type - issue_data['description'] = description + issue_data[IssueData.ISSUE_TYPE.value] = issue_type + issue_data[IssueData.DESCRIPTION.value] = description issues.append(CodeIssue(**issue_data)) return issues @staticmethod def choose_issue_type(code: str) -> IssueType: + # Handling individual codes if code in CODE_TO_ISSUE_TYPE: return CODE_TO_ISSUE_TYPE[code] - code_prefix = re.match(r'^([a-z]+)\d+$', code, re.IGNORECASE).group(1) - issue_type = CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix) + regex_match = re.match(r'^([A-Z]+)(\d)\d*$', code, re.IGNORECASE) + code_prefix = regex_match.group(1) + first_code_number = regex_match.group(2) + + # Handling other issues + issue_type = (CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix + first_code_number) + or CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix)) if not issue_type: logger.warning(f'flake8: {code} - unknown error code') return IssueType.BEST_PRACTICES diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index 7997d32c..b1074b52 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -15,12 +15,92 @@ # builtin naming 'A003': IssueType.BEST_PRACTICES, + + # flake8-broken-line + 'N400': IssueType.CODE_STYLE, + + # flake8-commas + 'C812': IssueType.CODE_STYLE, + 'C813': IssueType.CODE_STYLE, + 'C815': IssueType.CODE_STYLE, + 'C816': IssueType.CODE_STYLE, + 'C818': IssueType.CODE_STYLE, + 'C819': IssueType.CODE_STYLE, + + # The categorization for WPS was created using the following document: https://bit.ly/3yms06n + + # WPS: Naming + 'WPS117': IssueType.CODE_STYLE, # Forbid naming variables self, cls, or mcs. + 'WPS125': IssueType.ERROR_PRONE, # Forbid variable or module names which shadow builtin names. + + # WPS: Consistency + 'WPS300': IssueType.CODE_STYLE, # Forbid imports relative to the current folder. + 'WPS301': IssueType.CODE_STYLE, # Forbid imports like import os.path. + 'WPS304': IssueType.CODE_STYLE, # Forbid partial floats like .05 or 23.. + 'WPS310': IssueType.BEST_PRACTICES, # Forbid uppercase X, O, B, and E in numbers. + 'WPS313': IssueType.CODE_STYLE, # Enforce separation of parenthesis from keywords with spaces. + 'WPS317': IssueType.CODE_STYLE, # Forbid incorrect indentation for parameters. + 'WPS318': IssueType.CODE_STYLE, # Forbid extra indentation. + 'WPS319': IssueType.CODE_STYLE, # Forbid brackets in the wrong position. + 'WPS320': IssueType.CODE_STYLE, # Forbid multi-line function type annotations. + 'WPS321': IssueType.CODE_STYLE, # Forbid uppercase string modifiers. + 'WPS324': IssueType.ERROR_PRONE, # If any return has a value, all return nodes should have a value. + 'WPS325': IssueType.ERROR_PRONE, # If any yield has a value, all yield nodes should have a value. + 'WPS326': IssueType.ERROR_PRONE, # Forbid implicit string concatenation. + 'WPS329': IssueType.ERROR_PRONE, # Forbid meaningless except cases. + 'WPS330': IssueType.ERROR_PRONE, # Forbid unnecessary operators in your code. + 'WPS338': IssueType.BEST_PRACTICES, # Forbid incorrect order of methods inside a class. + 'WPS339': IssueType.CODE_STYLE, # Forbid meaningless zeros. + 'WPS340': IssueType.CODE_STYLE, # Forbid extra + signs in the exponent. + 'WPS341': IssueType.CODE_STYLE, # Forbid lowercase letters as hex numbers. + 'WPS343': IssueType.CODE_STYLE, # Forbid uppercase complex number suffix. + 'WPS344': IssueType.ERROR_PRONE, # Forbid explicit division (or modulo) by zero. + 'WPS347': IssueType.ERROR_PRONE, # Forbid imports that may cause confusion outside of the module. + 'WPS348': IssueType.CODE_STYLE, # Forbid starting lines with a dot. + 'WPS350': IssueType.CODE_STYLE, # Enforce using augmented assign pattern. + 'WPS355': IssueType.CODE_STYLE, # Forbid useless blank lines before and after brackets. + 'WPS361': IssueType.CODE_STYLE, # Forbids inconsistent newlines in comprehensions. + + # WPS: Best practices + 'WPS405': IssueType.ERROR_PRONE, # Forbid anything other than ast.Name to define loop variables. + 'WPS406': IssueType.ERROR_PRONE, # Forbid anything other than ast.Name to define contexts. + 'WPS408': IssueType.ERROR_PRONE, # Forbid using the same logical conditions in one expression. + 'WPS414': IssueType.ERROR_PRONE, # Forbid tuple unpacking with side-effects. + 'WPS415': IssueType.ERROR_PRONE, # Forbid the same exception class in multiple except blocks. + 'WPS416': IssueType.ERROR_PRONE, # Forbid yield keyword inside comprehensions. + 'WPS417': IssueType.ERROR_PRONE, # Forbid duplicate items in hashes. + 'WPS418': IssueType.ERROR_PRONE, # Forbid exceptions inherited from BaseException. + 'WPS419': IssueType.ERROR_PRONE, # Forbid multiple returning paths with try / except case. + 'WPS424': IssueType.ERROR_PRONE, # Forbid BaseException exception. + 'WPS426': IssueType.ERROR_PRONE, # Forbid lambda inside loops. + 'WPS432': IssueType.INFO, # Forbid magic numbers. + 'WPS433': IssueType.CODE_STYLE, # Forbid imports nested in functions. + 'WPS439': IssueType.ERROR_PRONE, # Forbid Unicode escape sequences in binary strings. + 'WPS440': IssueType.ERROR_PRONE, # Forbid overlapping local and block variables. + 'WPS441': IssueType.ERROR_PRONE, # Forbid control variables after the block body. + 'WPS442': IssueType.ERROR_PRONE, # Forbid shadowing variables from outer scopes. + 'WPS443': IssueType.ERROR_PRONE, # Forbid explicit unhashable types of asset items and dict keys. + 'WPS445': IssueType.ERROR_PRONE, # Forbid incorrectly named keywords in starred dicts. + 'WPS448': IssueType.ERROR_PRONE, # Forbid incorrect order of except. + 'WPS449': IssueType.ERROR_PRONE, # Forbid float keys. + 'WPS456': IssueType.ERROR_PRONE, # Forbids using float("NaN") construct to generate NaN. + 'WPS457': IssueType.ERROR_PRONE, # Forbids use of infinite while True: loops. + 'WPS458': IssueType.ERROR_PRONE, # Forbids to import from already imported modules. + + # WPS: Refactoring + 'WPS524': IssueType.ERROR_PRONE, # Forbid misrefactored self assignment. + + # WPS: OOP + 'WPS601': IssueType.ERROR_PRONE, # Forbid shadowing class level attributes with instance level attributes. + 'WPS613': IssueType.ERROR_PRONE, # Forbid super() with incorrect method or property access. + 'WPS614': IssueType.ERROR_PRONE, # Forbids descriptors in regular functions. } CODE_PREFIX_TO_ISSUE_TYPE: Dict[str, IssueType] = { 'B': IssueType.ERROR_PRONE, # flake8-bugbear 'A': IssueType.ERROR_PRONE, # flake8-builtins 'R': IssueType.ERROR_PRONE, # flake8-return + 'P': IssueType.ERROR_PRONE, # flake8-format-string 'E': IssueType.CODE_STYLE, # standard flake8 'W': IssueType.CODE_STYLE, # standard flake8 @@ -30,4 +110,11 @@ 'F': IssueType.BEST_PRACTICES, # standard flake8 'C': IssueType.BEST_PRACTICES, # flake8-comprehensions 'SC': IssueType.INFO, # flake8-spellcheck + + 'WPS1': IssueType.CODE_STYLE, # WPS type: Naming + 'WPS2': IssueType.COMPLEXITY, # WPS type: Complexity + 'WPS3': IssueType.BEST_PRACTICES, # WPS type: Consistency + 'WPS4': IssueType.BEST_PRACTICES, # WPS type: Best practices + 'WPS5': IssueType.BEST_PRACTICES, # WPS type: Refactoring + 'WPS6': IssueType.BEST_PRACTICES, # WPS type: OOP } diff --git a/src/python/review/inspectors/flake8/whitelist.txt b/src/python/review/inspectors/flake8/whitelist.txt index 8ab08f11..38715032 100644 --- a/src/python/review/inspectors/flake8/whitelist.txt +++ b/src/python/review/inspectors/flake8/whitelist.txt @@ -1,9 +1,12 @@ aggfunc appendleft +arange argmax asctime astype +barmode betavariate +bgcolor birthdate blackbox bs4 @@ -13,6 +16,8 @@ capwords casefold caseless concat +config +configs consts coord copysign @@ -40,6 +45,7 @@ expm1 falsy fillna floordiv +fromkeys fromstring fullmatch gensim @@ -47,6 +53,7 @@ gmtime groupby halfs hashable +hline href hyp hyperskill @@ -72,12 +79,16 @@ lemmatizer lifes lim linalg +linecolor +linewidth linspace lowercased lvl lxml matmul +minsize multiline +nbins ndarray ndigits ndim @@ -99,6 +110,7 @@ rindex rmdir schur scipy +showline sigmoid sqrt src @@ -109,19 +121,27 @@ subdir subdirs substr substring +textposition textwrap todos tokenize tokenized tokenizer tolist +toplevel tracklist truediv truthy +uniformtext unpickled upd util utils +vline webpage whitespaces -writeback \ No newline at end of file +writeback +xanchor +xaxes +yanchor +yaxis 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/__init__.py b/src/python/review/inspectors/intellij/issue_types/__init__.py index c6f21127..97b8d31e 100644 --- a/src/python/review/inspectors/intellij/issue_types/__init__.py +++ b/src/python/review/inspectors/intellij/issue_types/__init__.py @@ -1,15 +1,18 @@ from typing import Dict -from src.python.review import IssueType -from .java import ISSUE_CLASS_TO_ISSUE_TYPE as \ - JAVA_ISSUE_CLASS_TO_ISSUE_TYPE -from .kotlin import ISSUE_CLASS_TO_ISSUE_TYPE as \ - KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE -from .python import ISSUE_CLASS_TO_ISSUE_TYPE as \ - PYTHON_ISSUE_CLASS_TO_ISSUE_TYPE +from src.python.review.inspectors.intellij.issue_types.java import ( + ISSUE_CLASS_TO_ISSUE_TYPE as JAVA_ISSUE_CLASS_TO_ISSUE_TYPE, +) +from src.python.review.inspectors.intellij.issue_types.kotlin import ( + ISSUE_CLASS_TO_ISSUE_TYPE as KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE, +) +from src.python.review.inspectors.intellij.issue_types.python import ( + ISSUE_CLASS_TO_ISSUE_TYPE as PYTHON_ISSUE_CLASS_TO_ISSUE_TYPE, +) +from src.python.review.inspectors.issue import IssueType ISSUE_CLASS_TO_ISSUE_TYPE: Dict[str, IssueType] = { **JAVA_ISSUE_CLASS_TO_ISSUE_TYPE, **PYTHON_ISSUE_CLASS_TO_ISSUE_TYPE, - **KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE + **KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE, } 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 3174d7c6..b1b3cf52 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' INFO = 'INFO' @@ -46,6 +47,8 @@ class IssueData(Enum): FUNCTION_LEN = 'func_len' 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 +62,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 +187,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/parsers/checkstyle_parser.py b/src/python/review/inspectors/parsers/checkstyle_parser.py index 9cf75acb..1db9874c 100644 --- a/src/python/review/inspectors/parsers/checkstyle_parser.py +++ b/src/python/review/inspectors/parsers/checkstyle_parser.py @@ -1,15 +1,27 @@ import logging import re from pathlib import Path -from typing import Callable, Dict, List, Any, Optional +from typing import Any, Callable, Dict, List, Optional from xml.etree import ElementTree from src.python.review.common.file_system import get_content_from_file from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.issue import BaseIssue, BoolExprLenIssue, CodeIssue, CyclomaticComplexityIssue, \ - FuncLenIssue, IssueType, LineLenIssue, IssueData -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.inspectors.issue import ( + BaseIssue, + BoolExprLenIssue, + CodeIssue, + CyclomaticComplexityIssue, + FuncLenIssue, + IssueData, + IssueType, + LineLenIssue, +) +from src.python.review.inspectors.tips import ( + get_bool_expr_len_tip, + get_cyclomatic_complexity_tip, + get_func_len_tip, + get_line_len_tip, +) logger = logging.getLogger(__name__) diff --git a/src/python/review/inspectors/pmd/files/bin/basic.xml b/src/python/review/inspectors/pmd/files/bin/basic.xml index 6cacd764..612bbf0e 100644 --- a/src/python/review/inspectors/pmd/files/bin/basic.xml +++ b/src/python/review/inspectors/pmd/files/bin/basic.xml @@ -46,29 +46,35 @@ - + - + + - + - + - - + + @@ -90,7 +96,8 @@ - + diff --git a/src/python/review/inspectors/pmd/issue_types.py b/src/python/review/inspectors/pmd/issue_types.py index e4609146..2188702b 100644 --- a/src/python/review/inspectors/pmd/issue_types.py +++ b/src/python/review/inspectors/pmd/issue_types.py @@ -2,7 +2,7 @@ from src.python.review.inspectors.issue import IssueType -RULE_TO_ISSUE_TYPE: Dict[str, IssueType] = { +PMD_RULE_TO_ISSUE_TYPE: Dict[str, IssueType] = { # Best Practices 'AbstractClassWithoutAbstractMethod': IssueType.BEST_PRACTICES, 'AccessorClassGeneration': IssueType.BEST_PRACTICES, diff --git a/src/python/review/inspectors/pmd/pmd.py b/src/python/review/inspectors/pmd/pmd.py index 227169fc..eb4878dd 100644 --- a/src/python/review/inspectors/pmd/pmd.py +++ b/src/python/review/inspectors/pmd/pmd.py @@ -8,15 +8,17 @@ from src.python.review.common.file_system import new_temp_dir 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.common import remove_prefix from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.inspectors.issue import BaseIssue, CodeIssue, IssueType -from src.python.review.inspectors.pmd.issue_types import RULE_TO_ISSUE_TYPE +from src.python.review.inspectors.pmd.issue_types import PMD_RULE_TO_ISSUE_TYPE logger = logging.getLogger(__name__) PATH_TOOLS_PMD_FILES = Path(__file__).parent / 'files' PATH_TOOLS_PMD_SHELL_SCRIPT = PATH_TOOLS_PMD_FILES / 'bin' / 'run.sh' PATH_TOOLS_PMD_RULES_SET = PATH_TOOLS_PMD_FILES / 'bin' / 'basic.xml' +DEFAULT_JAVA_VERSION = LanguageVersion.JAVA_11 class PMDInspector(BaseInspector): @@ -28,16 +30,16 @@ def __init__(self): @classmethod def _create_command(cls, path: Path, output_path: Path, - java_version: LanguageVersion, + language_version: LanguageVersion, n_cpu: int) -> List[str]: return [ PATH_TOOLS_PMD_SHELL_SCRIPT, 'pmd', '-d', str(path), '-no-cache', '-R', PATH_TOOLS_PMD_RULES_SET, '-language', 'java', - '-version', java_version.value, + '-version', cls._get_java_version(language_version), '-f', 'csv', '-r', str(output_path), - '-t', str(n_cpu) + '-t', str(n_cpu), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -46,13 +48,21 @@ def inspect(self, path: Path, config: dict) -> List[BaseIssue]: language_version = config.get('language_version') if language_version is None: - language_version = LanguageVersion.JAVA_11 + logger.info( + f"The version of Java is not passed. The version to be used is: {DEFAULT_JAVA_VERSION.value}.", + ) + language_version = DEFAULT_JAVA_VERSION command = self._create_command(path, output_path, language_version, config['n_cpu']) run_in_subprocess(command) 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 [] @@ -65,17 +75,37 @@ def parse_output(self, output_path: Path) -> List[BaseIssue]: line_no=int(row['Line']), column_no=1, type=self.choose_issue_type(row['Rule']), - origin_class=row['Rule set'], + origin_class=row['Rule'], description=row['Description'], inspector_type=self.inspector_type, ) for row in reader] @classmethod def choose_issue_type(cls, rule: str) -> IssueType: - issue_type = RULE_TO_ISSUE_TYPE.get(rule) + """ + 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' % (cls.inspector_type.value, rule)) return IssueType.BEST_PRACTICES return issue_type + + @staticmethod + def _get_java_version(language_version: LanguageVersion) -> str: + """ + Converts language_version to the version of Java that PMD can work with. + + For example, java11 will be converted to 11. + """ + java_version = language_version.value + + if not language_version.is_java(): + logger.warning( + f"The version passed is not the Java version. The version to be used is: {DEFAULT_JAVA_VERSION.value}.", + ) + java_version = DEFAULT_JAVA_VERSION.value + + return remove_prefix(java_version, "java") 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/issue_types.py b/src/python/review/inspectors/pylint/issue_types.py index edfe8818..5df676b1 100644 --- a/src/python/review/inspectors/pylint/issue_types.py +++ b/src/python/review/inspectors/pylint/issue_types.py @@ -8,6 +8,8 @@ # E errors, for probable bugs in the code CODE_TO_ISSUE_TYPE: Dict[str, IssueType] = { + 'C0200': IssueType.BEST_PRACTICES, # consider using enumerate + 'C0415': IssueType.BEST_PRACTICES, # import-outside-toplevel 'W0101': IssueType.ERROR_PRONE, # unreachable code 'W0102': IssueType.ERROR_PRONE, # dangerous default value 'W0104': IssueType.ERROR_PRONE, # statement doesn't have any effect 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/pylint/pylintrc b/src/python/review/inspectors/pylint/pylintrc index c435b63b..357ba058 100644 --- a/src/python/review/inspectors/pylint/pylintrc +++ b/src/python/review/inspectors/pylint/pylintrc @@ -164,6 +164,8 @@ disable=invalid-name, no-else-raise, no-else-break, no-else-continue, + W0614, + W0622, # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/test/resources/inspectors/python/__init__.py b/src/python/review/inspectors/radon/__init__.py similarity index 100% rename from test/resources/inspectors/python/__init__.py rename to src/python/review/inspectors/radon/__init__.py diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py new file mode 100644 index 00000000..79223164 --- /dev/null +++ b/src/python/review/inspectors/radon/radon.py @@ -0,0 +1,56 @@ +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.common import convert_percentage_of_value_to_lack_of_value +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.tips import get_maintainability_index_tip + + +MAINTAINABILITY_ORIGIN_CLASS = "RAD100" + + +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, origin_class=MAINTAINABILITY_ORIGIN_CLASS, + ) + 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 18f9d6b0..b5eca5a4 100644 --- a/src/python/review/inspectors/springlint/springlint.py +++ b/src/python/review/inspectors/springlint/springlint.py @@ -3,17 +3,34 @@ import re from pathlib import Path from shutil import copy -from typing import AnyStr, List, Optional, Dict, Any +from typing import Any, AnyStr, Dict, List, Optional from src.python.review.common.file_system import new_temp_dir 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, ChildrenNumberIssue, ClassResponseIssue, CodeIssue, \ - CohesionIssue, \ - CouplingIssue, InheritanceIssue, IssueType, MethodNumberIssue, WeightedMethodIssue, IssueData -from src.python.review.inspectors.tips import get_child_number_tip, get_class_coupling_tip, get_class_response_tip, \ - get_cohesion_tip, get_inheritance_depth_tip, get_method_number_tip, get_weighted_method_tip +from src.python.review.inspectors.issue import ( + BaseIssue, + ChildrenNumberIssue, + ClassResponseIssue, + CodeIssue, + CohesionIssue, + CouplingIssue, + InheritanceIssue, + IssueData, + IssueType, + MethodNumberIssue, + WeightedMethodIssue, +) +from src.python.review.inspectors.tips import ( + get_child_number_tip, + get_class_coupling_tip, + get_class_response_tip, + get_cohesion_tip, + get_inheritance_depth_tip, + get_method_number_tip, + get_weighted_method_tip, +) PATH_TOOLS_SPRINGLINT_FILES = Path(__file__).parent / 'files' PATH_SPRINGLINT_JAR = PATH_TOOLS_SPRINGLINT_FILES / 'springlint-0.6.jar' @@ -32,7 +49,7 @@ class SpringlintInspector(BaseInspector): 'cbo': 'class_objects_coupling', 'lcom': 'cohesion_lack', 'rfc': 'class_response', - 'nom': 'method_number' + 'nom': 'method_number', } metric_name_to_description = { @@ -42,7 +59,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 = { @@ -52,7 +69,7 @@ class SpringlintInspector(BaseInspector): 'cbo': IssueType.COUPLING, 'lcom': IssueType.COHESION, 'rfc': IssueType.CLASS_RESPONSE, - 'nom': IssueType.METHOD_NUMBER + 'nom': IssueType.METHOD_NUMBER, } @classmethod @@ -62,7 +79,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]: @@ -118,7 +135,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 c1d3fba1..eac449c1 100644 --- a/src/python/review/inspectors/tips.py +++ b/src/python/review/inspectors/tips.py @@ -1,24 +1,32 @@ def get_bool_expr_len_tip() -> str: - return 'Too long boolean expression. ' \ - 'Try to split it into smaller expressions.' + return ( + 'Too long boolean expression. ' + 'Try to split it into smaller expressions.' + ) def get_func_len_tip() -> str: - return 'Too long function. ' \ - 'Try to split it into smaller functions / methods.' \ - 'It will make your code easy to understand and less error prone.' + return ( + 'Too long function. ' + 'Try to split it into smaller functions / methods. ' + 'It will make your code easy to understand and less error prone.' + ) def get_line_len_tip() -> str: - return 'Too long line. ' \ - 'Try to split it into smaller lines.' \ - 'It will make your code easy to understand.' + return ( + 'Too long line. ' + 'Try to split it into smaller lines. ' + 'It will make your code easy to understand.' + ) def get_cyclomatic_complexity_tip() -> str: - return 'Too complex function. You can figure out how to simplify this code ' \ - 'or split it into a set of small functions / methods. ' \ - 'It will make your code easy to understand and less error prone.' + return ( + 'Too complex function. You can figure out how to simplify this code ' + 'or split it into a set of small functions / methods. ' + 'It will make your code easy to understand and less error prone.' + ) def add_complexity_tip(description: str) -> str: @@ -31,8 +39,10 @@ def add_complexity_tip(description: str) -> str: def get_inheritance_depth_tip() -> str: - return 'Too deep inheritance tree is complicated to understand. ' \ - 'Try to reduce it (maybe you should use a composition instead).' + return ( + 'Too deep inheritance tree is complicated to understand. ' + 'Try to reduce it (maybe you should use a composition instead).' + ) # This issue will not be reported at this version @@ -41,14 +51,18 @@ def get_child_number_tip() -> str: def get_weighted_method_tip() -> str: - return 'The number of methods and their complexity may be too hight. ' \ - 'It may require too much time and effort to develop and maintain the class.' + return ( + 'The number of methods and their complexity may be too hight. ' + 'It may require too much time and effort to develop and maintain the class.' + ) def get_class_coupling_tip() -> str: - return 'The class seems to depend on too many other classes. ' \ - 'Increased coupling increases interclass dependencies, ' \ - 'making the code less modular and less suitable for reuse and testing.' + return ( + 'The class seems to depend on too many other classes. ' + 'Increased coupling increases interclass dependencies, ' + 'making the code less modular and less suitable for reuse and testing.' + ) # This issue will not be reported at this version @@ -57,12 +71,21 @@ def get_cohesion_tip() -> str: def get_class_response_tip() -> str: - return 'The class have too many methods that can potentially ' \ - 'be executed in response to a single message received by an object of that class. ' \ - 'The larger the number of methods that can be invoked from a class, ' \ - 'the greater the complexity of the class' + return ( + 'The class have too many methods that can potentially ' + 'be executed in response to a single message received by an object of that class. ' + 'The larger the number of methods that can be invoked from a class, ' + 'the greater the complexity of the class' + ) def get_method_number_tip() -> str: - return 'The file has too many methods inside and is complicated to understand. ' \ - 'Consider its decomposition to smaller classes.' + return ( + '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 3d78e392..b6329653 100644 --- a/src/python/review/quality/evaluate_quality.py +++ b/src/python/review/quality/evaluate_quality.py @@ -4,25 +4,46 @@ from src.python.review.inspectors.issue import IssueType from src.python.review.quality.model import Quality, Rule from src.python.review.quality.rules.best_practices_scoring import ( - BestPracticesRule, LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG + BestPracticesRule, + LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG, +) +from src.python.review.quality.rules.boolean_length_scoring import ( + BooleanExpressionRule, + LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG, ) -from src.python.review.quality.rules.boolean_length_scoring import BooleanExpressionRule, \ - LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG from src.python.review.quality.rules.class_response_scoring import LANGUAGE_TO_RESPONSE_RULE_CONFIG, ResponseRule from src.python.review.quality.rules.code_style_scoring import CodeStyleRule, LANGUAGE_TO_CODE_STYLE_RULE_CONFIG +from src.python.review.quality.rules.cohesion_scoring import ( + CohesionRule, + LANGUAGE_TO_COHESION_RULE_CONFIG, +) from src.python.review.quality.rules.coupling_scoring import CouplingRule, LANGUAGE_TO_COUPLING_RULE_CONFIG -from src.python.review.quality.rules.cyclomatic_complexity_scoring import CyclomaticComplexityRule, \ - LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG +from src.python.review.quality.rules.cyclomatic_complexity_scoring import ( + CyclomaticComplexityRule, + LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG, +) from src.python.review.quality.rules.error_prone_scoring import ErrorProneRule, LANGUAGE_TO_ERROR_PRONE_RULE_CONFIG -from src.python.review.quality.rules.function_length_scoring import FunctionLengthRule, \ - LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG -from src.python.review.quality.rules.inheritance_depth_scoring import InheritanceDepthRule, \ - LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG +from src.python.review.quality.rules.function_length_scoring import ( + FunctionLengthRule, + LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG, +) +from src.python.review.quality.rules.inheritance_depth_scoring import ( + InheritanceDepthRule, + LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG, +) from src.python.review.quality.rules.line_len_scoring import LANGUAGE_TO_LINE_LENGTH_RULE_CONFIG, LineLengthRule -from src.python.review.quality.rules.method_number_scoring import LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG, \ - MethodNumberRule -from src.python.review.quality.rules.weighted_methods_scoring import LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG, \ - WeightedMethodsRule +from src.python.review.quality.rules.maintainability_scoring import ( + LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG, + MaintainabilityRule, +) +from src.python.review.quality.rules.method_number_scoring import ( + LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG, + MethodNumberRule, +) +from src.python.review.quality.rules.weighted_methods_scoring import ( + LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG, + WeightedMethodsRule, +) from src.python.review.reviewers.utils.code_statistics import CodeStatistics @@ -37,7 +58,9 @@ def __get_available_rules(language: Language) -> List[Rule]: MethodNumberRule(LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG[language]), CouplingRule(LANGUAGE_TO_COUPLING_RULE_CONFIG[language]), ResponseRule(LANGUAGE_TO_RESPONSE_RULE_CONFIG[language]), - WeightedMethodsRule(LANGUAGE_TO_WEIGHTED_METHODS_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 ae71f211..7a57dcf6 100644 --- a/src/python/review/quality/model.py +++ b/src/python/review/quality/model.py @@ -1,4 +1,5 @@ import abc +import textwrap from enum import Enum, unique from functools import total_ordering from typing import List @@ -19,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] @@ -49,7 +50,7 @@ def quality_type(self) -> QualityType: def next_quality_type(self) -> QualityType: return min(map(lambda rule: rule.next_level_type, self.rules), default=QualityType.EXCELLENT) - # TODO@nbirillo: why rule.quality_type == quality_type for next level???? + # TODO: why rule.quality_type == quality_type for next level???? @property def next_level_requirements(self) -> List[Rule]: quality_type = self.quality_type @@ -80,8 +81,11 @@ def __str__(self): message_deltas_part = '' if self.quality_type != QualityType.EXCELLENT: - message_next_level_part = f'Next level: {self.next_quality_type.value}\n' \ - f'Next level requirements:\n' + 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/cohesion_scoring.py b/src/python/review/quality/rules/cohesion_scoring.py new file mode 100644 index 00000000..623fd2db --- /dev/null +++ b/src/python/review/quality/rules/cohesion_scoring.py @@ -0,0 +1,71 @@ +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 QualityType, Rule + + +@dataclass +class CohesionRuleConfig: + cohesion_lack_bad: int + cohesion_lack_moderate: int + cohesion_lack_good: int + + +# TODO: Need testing +# Cohesion plugin by default only shows issues where cohesion is less than 50%. +# Therefore cohesion_lack_bad = 50. The other levels are set in steps of 20%. +common_cohesion_rule_config = CohesionRuleConfig( + cohesion_lack_bad=50, + cohesion_lack_moderate=30, + cohesion_lack_good=10, +) + +LANGUAGE_TO_COHESION_RULE_CONFIG = { + Language.JAVA: common_cohesion_rule_config, + Language.PYTHON: common_cohesion_rule_config, + Language.KOTLIN: common_cohesion_rule_config, + Language.JS: common_cohesion_rule_config, +} + + +class CohesionRule(Rule): + def __init__(self, config: CohesionRuleConfig): + self.config = config + self.rule_type = IssueType.COHESION + self.cohesion_lack: Optional[int] = None + + def apply(self, cohesion_lack: int): + self.cohesion_lack = cohesion_lack + if cohesion_lack > self.config.cohesion_lack_bad: + self.quality_type = QualityType.BAD + self.next_level_delta = cohesion_lack - self.config.cohesion_lack_bad + elif cohesion_lack > self.config.cohesion_lack_moderate: + self.quality_type = QualityType.MODERATE + self.next_level_delta = cohesion_lack - self.config.cohesion_lack_moderate + elif cohesion_lack > self.config.cohesion_lack_good: + self.quality_type = QualityType.GOOD + self.next_level_delta = cohesion_lack - self.config.cohesion_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: 'CohesionRule') -> 'CohesionRule': + config = CohesionRuleConfig( + min(self.config.cohesion_lack_bad, other.config.cohesion_lack_bad), + min(self.config.cohesion_lack_moderate, other.config.cohesion_lack_moderate), + min(self.config.cohesion_lack_good, other.config.cohesion_lack_good), + ) + result_rule = CohesionRule(config) + result_rule.apply(max(self.cohesion_lack, other.cohesion_lack)) + + return result_rule 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..ade6fd4b --- /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 QualityType, Rule + + +@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..9340d824 100644 --- a/src/python/review/reviewers/common.py +++ b/src/python/review/reviewers/common.py @@ -12,6 +12,7 @@ from src.python.review.inspectors.pmd.pmd import PMDInspector from src.python.review.inspectors.pyast.python_ast import PythonAstInspector from src.python.review.inspectors.pylint.pylint import PylintInspector +from src.python.review.inspectors.radon.radon import RadonInspector from src.python.review.quality.evaluate_quality import evaluate_quality from src.python.review.quality.model import Quality from src.python.review.reviewers.review_result import FileReviewResult, ReviewResult @@ -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 277b5187..9f495751 100644 --- a/src/python/review/reviewers/perform_review.py +++ b/src/python/review/reviewers/perform_review.py @@ -14,7 +14,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__) @@ -32,7 +32,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/python.py b/src/python/review/reviewers/python.py index a239913a..d51b526a 100644 --- a/src/python/review/reviewers/python.py +++ b/src/python/review/reviewers/python.py @@ -2,7 +2,7 @@ from typing import List from src.python.review.application_config import ApplicationConfig -from src.python.review.common.file_system import get_all_file_system_items, FileSystemItem +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 diff --git a/src/python/review/reviewers/utils/code_statistics.py b/src/python/review/reviewers/utils/code_statistics.py index b0523355..19218a7b 100644 --- a/src/python/review/reviewers/utils/code_statistics.py +++ b/src/python/review/reviewers/utils/code_statistics.py @@ -1,7 +1,7 @@ from collections import Counter from dataclasses import dataclass from pathlib import Path -from typing import List, Dict +from typing import Dict, List from src.python.review.common.file_system import get_content_from_file from src.python.review.inspectors.issue import BaseIssue, IssueType @@ -16,6 +16,8 @@ class CodeStatistics: method_number: int max_cyclomatic_complexity: int + max_cohesion_lack: int + max_maintainability_lack: int max_func_len: int max_bool_expr_len: int @@ -38,6 +40,8 @@ def issue_type_to_statistics_dict(self) -> Dict[IssueType, int]: IssueType.METHOD_NUMBER: self.method_number, 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, @@ -45,7 +49,7 @@ def issue_type_to_statistics_dict(self) -> Dict[IssueType, int]: IssueType.INHERITANCE_DEPTH: self.inheritance_depth, IssueType.COUPLING: self.coupling, IssueType.CLASS_RESPONSE: self.class_response, - IssueType.WEIGHTED_METHOD: self.weighted_method_complexities + IssueType.WEIGHTED_METHOD: self.weighted_method_complexities, } @@ -71,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) @@ -82,6 +86,8 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic bool_expr_lens = __get_max_measure_by_issue_type(IssueType.BOOL_EXPR_LEN, issues) 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) @@ -97,6 +103,8 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic max_bool_expr_len=bool_expr_lens, 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, @@ -104,5 +112,5 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic weighted_method_complexities=weighted_method_complexities, method_number=method_numbers, total_lines=__get_total_lines(path), - code_style_lines=get_code_style_lines(issues) + code_style_lines=get_code_style_lines(issues), ) diff --git a/src/python/review/reviewers/utils/issues_filter.py b/src/python/review/reviewers/utils/issues_filter.py index 9ac9b0b6..b6e5529e 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -5,10 +5,12 @@ from src.python.review.inspectors.issue import BaseIssue, IssueType, Measurable from src.python.review.quality.rules.boolean_length_scoring import LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG from src.python.review.quality.rules.class_response_scoring import LANGUAGE_TO_RESPONSE_RULE_CONFIG +from src.python.review.quality.rules.cohesion_scoring import LANGUAGE_TO_COHESION_RULE_CONFIG from src.python.review.quality.rules.coupling_scoring import LANGUAGE_TO_COUPLING_RULE_CONFIG from src.python.review.quality.rules.cyclomatic_complexity_scoring import LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG from src.python.review.quality.rules.function_length_scoring import LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG from src.python.review.quality.rules.inheritance_depth_scoring import LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG +from src.python.review.quality.rules.maintainability_scoring import LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG 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 @@ -22,7 +24,11 @@ def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, IssueType.METHOD_NUMBER: LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG[language].method_number_good, IssueType.COUPLING: LANGUAGE_TO_COUPLING_RULE_CONFIG[language].coupling_moderate, 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.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, } @@ -38,7 +44,7 @@ def filter_low_measure_issues(issues: List[BaseIssue], issue_type_to_low_measure_dict = __get_issue_type_to_low_measure_dict(language) # Disable this types of issue, requires further investigation. - ignored_issues = [IssueType.COHESION, IssueType.CHILDREN_NUMBER] + ignored_issues = [IssueType.CHILDREN_NUMBER] return list(filter( lambda issue: issue.type not in ignored_issues and __more_than_low_measure(issue, 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 fdbb4b67..9b3b89f8 100644 --- a/src/python/review/run_tool.py +++ b/src/python/review/run_tool.py @@ -1,39 +1,30 @@ import argparse +import enum import logging.config import os import sys import traceback -from enum import Enum, unique from pathlib import Path -from typing import Set, List +from typing import Set + sys.path.append('') sys.path.append('../../..') +from src.python.common.tool_arguments import RunToolArgument, VerbosityLevel from src.python.review.application_config import ApplicationConfig, LanguageVersion from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.logging_config import logging_config -from src.python.review.reviewers.perform_review import OutputFormat, PathNotExists, perform_and_print_review, \ - UnsupportedLanguage +from src.python.review.reviewers.perform_review import ( + OutputFormat, + PathNotExists, + perform_and_print_review, + UnsupportedLanguage, +) logger = logging.getLogger(__name__) -@unique -class VerbosityLevel(Enum): - """ - Same meaning as the logging level. Should be used in command-line args. - """ - DEBUG = '3' - INFO = '2' - ERROR = '1' - DISABLE = '0' - - @classmethod - def values(cls) -> List[str]: - return [member.value for _, member in VerbosityLevel.__members__.items()] - - def parse_disabled_inspectors(value: str) -> Set[InspectorType]: passed_names = value.upper().split(',') allowed_names = {inspector.value for inspector in InspectorType} @@ -51,70 +42,66 @@ def positive_int(value: str) -> int: return value_int -def configure_arguments(parser: argparse.ArgumentParser) -> None: - parser.add_argument('-v', '--verbosity', - help='Choose logging level: ' - f'{VerbosityLevel.ERROR.value} - ERROR; ' - f'{VerbosityLevel.INFO.value} - INFO; ' - f'{VerbosityLevel.DEBUG.value} - DEBUG; ' - f'{VerbosityLevel.DISABLE.value} - disable logging; ' - 'default is 0', +def configure_arguments(parser: argparse.ArgumentParser, tool_arguments: enum.EnumMeta) -> None: + parser.add_argument(tool_arguments.VERBOSITY.value.short_name, + tool_arguments.VERBOSITY.value.long_name, + help=tool_arguments.VERBOSITY.value.description, default=VerbosityLevel.DISABLE.value, choices=VerbosityLevel.values(), - type=str) + type=int) # Usage example: -d Flake8,Intelli - inspectors = [inspector.lower() for inspector in InspectorType.available_values()] - example = f'-d {inspectors[0].lower()},{inspectors[1].lower()}' - - parser.add_argument('-d', '--disable', - help='Disable inspectors. ' - f'Allowed values: {", ".join(inspectors)}. ' - f'Example: {example}', + parser.add_argument(tool_arguments.DISABLE.value.short_name, + tool_arguments.DISABLE.value.long_name, + help=tool_arguments.DISABLE.value.description, type=parse_disabled_inspectors, default=set()) - parser.add_argument('--allow-duplicates', action='store_true', - help='Allow duplicate issues found by different linters. ' - 'By default, duplicates are skipped.') + parser.add_argument(tool_arguments.DUPLICATES.value.long_name, + action='store_true', + help=tool_arguments.DUPLICATES.value.description) # TODO: deprecated argument: language_version. Delete after several releases. - parser.add_argument('--language_version', '--language-version', - help='Specify the language version for JAVA inspectors.', + parser.add_argument('--language_version', + tool_arguments.LANG_VERSION.value.long_name, + help=tool_arguments.LANG_VERSION.value.description, default=None, choices=LanguageVersion.values(), type=str) # TODO: deprecated argument: --n_cpu. Delete after several releases. - parser.add_argument('--n_cpu', '--n-cpu', - help='Specify number of cpu that can be used to run inspectors', + parser.add_argument('--n_cpu', + tool_arguments.CPU.value.long_name, + help=tool_arguments.CPU.value.description, default=1, type=positive_int) - parser.add_argument('path', + parser.add_argument(tool_arguments.PATH.value.long_name, type=lambda value: Path(value).absolute(), - help='Path to file or directory to inspect.') + help=tool_arguments.PATH.value.description) - parser.add_argument('-f', '--format', + parser.add_argument(tool_arguments.FORMAT.value.short_name, + tool_arguments.FORMAT.value.long_name, default=OutputFormat.JSON.value, choices=OutputFormat.values(), type=str, - help='The output format. Default is JSON.') + help=tool_arguments.FORMAT.value.description) - parser.add_argument('-s', '--start-line', + parser.add_argument(tool_arguments.START_LINE.value.short_name, + tool_arguments.START_LINE.value.long_name, default=1, type=positive_int, - help='The first line to be analyzed. It starts from 1.') + help=tool_arguments.START_LINE.value.description) - parser.add_argument('-e', '--end-line', + parser.add_argument(tool_arguments.END_LINE.value.short_name, + tool_arguments.END_LINE.value.long_name, default=None, type=positive_int, - help='The end line to be analyzed or None.') + help=tool_arguments.END_LINE.value.description) - parser.add_argument('--new-format', + parser.add_argument(tool_arguments.NEW_FORMAT.value.long_name, action='store_true', - help='The argument determines whether the tool ' - 'should use the new format') + help=tool_arguments.NEW_FORMAT.value.description) def configure_logging(verbosity: VerbosityLevel) -> None: @@ -132,7 +119,7 @@ def configure_logging(verbosity: VerbosityLevel) -> None: def main() -> int: parser = argparse.ArgumentParser() - configure_arguments(parser) + configure_arguments(parser, RunToolArgument) try: args = parser.parse_args() @@ -150,7 +137,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..da01adde 100644 --- a/test/python/functional_tests/conftest.py +++ b/test/python/functional_tests/conftest.py @@ -1,11 +1,10 @@ from dataclasses import dataclass, field from pathlib import Path +from test.python import TEST_DATA_FOLDER from typing import List, Optional import pytest - from src.python import MAIN_FOLDER -from test.python import TEST_DATA_FOLDER DATA_PATH = TEST_DATA_FOLDER / 'functional_tests' @@ -43,7 +42,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..c55e56f9 100644 --- a/test/python/functional_tests/test_different_languages.py +++ b/test/python/functional_tests/test_different_languages.py @@ -1,5 +1,4 @@ import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -10,7 +9,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 +25,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 +41,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 +58,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..7967e1bc 100644 --- a/test/python/functional_tests/test_disable.py +++ b/test/python/functional_tests/test_disable.py @@ -1,5 +1,4 @@ import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -10,7 +9,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 +19,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..9158e030 100644 --- a/test/python/functional_tests/test_duplicates.py +++ b/test/python/functional_tests/test_duplicates.py @@ -1,6 +1,5 @@ import re import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -13,7 +12,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 +21,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..4d5dad1a 100644 --- a/test/python/functional_tests/test_exit_code.py +++ b/test/python/functional_tests/test_exit_code.py @@ -1,6 +1,5 @@ import subprocess from pathlib import Path - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -11,7 +10,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 +23,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 +36,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..074c77aa 100644 --- a/test/python/functional_tests/test_file_or_project.py +++ b/test/python/functional_tests/test_file_or_project.py @@ -1,5 +1,4 @@ import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -11,7 +10,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 +26,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..86a029d1 100644 --- a/test/python/functional_tests/test_multi_file_project.py +++ b/test/python/functional_tests/test_multi_file_project.py @@ -1,35 +1,34 @@ import json import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder 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 +37,7 @@ 'line': 'a = 1', 'line_number': 2, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -46,7 +45,7 @@ 'line': 'b = 2', 'line_number': 3, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -54,11 +53,11 @@ 'line': 'c = 3', 'line_number': 4, 'column_number': 5, - 'category': 'BEST_PRACTICES' - } - ] + 'category': 'BEST_PRACTICES', + }, + ], }, - ] + ], } @@ -73,7 +72,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 21710909..1ef4a8f5 100644 --- a/test/python/functional_tests/test_range_of_lines.py +++ b/test/python/functional_tests/test_range_of_lines.py @@ -1,16 +1,15 @@ import json +from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder import pytest - from src.python.review.common.subprocess_runner import run_in_subprocess -from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder PATH_TO_FILE = DATA_PATH / 'lines_range' / 'code_with_multiple_issues.py' EXPECTED_JSON = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'category': 'CODE_STYLE', @@ -30,16 +29,16 @@ 'column_number': 2, 'line': 'c=a + b', 'line_number': 4, - 'text': 'missing whitespace around operator' - } - ] + 'text': 'missing whitespace around operator', + }, + ], } NO_ISSUES_JSON = { 'quality': { 'code': 'EXCELLENT', 'text': 'Code quality (beta): EXCELLENT'}, - 'issues': [] + 'issues': [], } @@ -85,8 +84,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 +144,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': 'E225', @@ -153,8 +152,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 +210,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': 'E225', @@ -219,15 +218,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': 'E225', 'text': 'missing whitespace around operator', '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..85616145 100644 --- a/test/python/functional_tests/test_single_file_json_format.py +++ b/test/python/functional_tests/test_single_file_json_format.py @@ -1,10 +1,9 @@ import json import subprocess +from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder from jsonschema import validate -from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder - schema = { 'type': 'object', 'properties': { @@ -12,8 +11,8 @@ 'type': 'object', 'properties': { 'code': {'type': 'string'}, - 'text': {'type': 'string'} - } + 'text': {'type': 'string'}, + }, }, 'issues': { 'type': 'array', @@ -25,11 +24,11 @@ 'column_number': {'type': 'number'}, 'line': {'type': 'string'}, 'line_number': {'type': 'number'}, - 'text': {'type': 'string'} - } - } - } - } + 'text': {'type': 'string'}, + }, + }, + }, + }, } @@ -43,7 +42,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..e835258b 100644 --- a/test/python/functional_tests/test_verbosity.py +++ b/test/python/functional_tests/test_verbosity.py @@ -1,6 +1,5 @@ import json import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -13,7 +12,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 +32,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 +48,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 615f95dc..f9072db7 100644 --- a/test/python/inspectors/conftest.py +++ b/test/python/inspectors/conftest.py @@ -5,7 +5,6 @@ from typing import Any, Dict, List import pytest - from src.python.review.common.file_system import new_temp_dir from src.python.review.inspectors.issue import BaseIssue, IssueType from src.python.review.reviewers.utils.metadata_exploration import explore_file, FileMetadata @@ -31,16 +30,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 +51,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, + }, + ], + }, } @@ -73,6 +72,8 @@ class IssuesTestInfo: n_cc: int = 0 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: @@ -85,7 +86,9 @@ def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: n_func_len=counter[IssueType.FUNC_LEN], n_cc=counter[IssueType.CYCLOMATIC_COMPLEXITY], n_bool_expr_len=counter[IssueType.BOOL_EXPR_LEN], - n_other_complexity=counter[IssueType.COMPLEXITY] + n_other_complexity=counter[IssueType.COMPLEXITY], + n_cohesion=counter[IssueType.COHESION], + n_maintainability=counter[IssueType.MAINTAINABILITY], ) diff --git a/test/python/inspectors/test_checkstyle_inspector.py b/test/python/inspectors/test_checkstyle_inspector.py index 892ec97f..ac691ff6 100644 --- a/test/python/inspectors/test_checkstyle_inspector.py +++ b/test/python/inspectors/test_checkstyle_inspector.py @@ -1,10 +1,10 @@ -import pytest +from test.python.inspectors import JAVA_DATA_FOLDER +from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.checkstyle.checkstyle import CheckstyleInspector from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import JAVA_DATA_FOLDER -from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('test_simple_valid_program.java', 0), @@ -43,6 +43,7 @@ ('test_indentation_with_spaces.java', 0), ('test_indentation_with_tabs.java', 0), ('test_indentation_google_style.java', 4), + ('test_multiple_literals.java', 1), ] diff --git a/test/python/inspectors/test_detekt_inspector.py b/test/python/inspectors/test_detekt_inspector.py index 4dbafd2d..99f84f27 100644 --- a/test/python/inspectors/test_detekt_inspector.py +++ b/test/python/inspectors/test_detekt_inspector.py @@ -1,10 +1,10 @@ -import pytest +from test.python.inspectors import KOTLIN_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.detekt.detekt import DetektInspector from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import KOTLIN_DATA_FOLDER -from test.python.inspectors.conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_good_program.kt', 0), @@ -23,6 +23,7 @@ ('case16_redundant_unit.kt', 1), ('case18_redundant_braces.kt', 3), ('case20_cyclomatic_complexity.kt', 0), + ('case21_cyclomatic_complexity_bad.kt', 2), ('case22_too_many_arguments.kt', 1), ('case23_bad_range_performance.kt', 3), ('case24_duplicate_when_bug.kt', 1), diff --git a/test/python/inspectors/test_eslint_inspector.py b/test/python/inspectors/test_eslint_inspector.py index f6681c90..d7aceeac 100644 --- a/test/python/inspectors/test_eslint_inspector.py +++ b/test/python/inspectors/test_eslint_inspector.py @@ -1,10 +1,10 @@ -import pytest +from test.python.inspectors import JS_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.eslint.eslint import ESLintInspector from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import JS_DATA_FOLDER -from test.python.inspectors.conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_no_issues.js', 0), diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 3c655331..2a2ebf26 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -1,25 +1,26 @@ -import pytest +from test.python.inspectors import PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata +from textwrap import dedent +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.flake8.flake8 import Flake8Inspector from src.python.review.inspectors.issue import IssueType 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 gather_issues_test_info, IssuesTestInfo, use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_spaces.py', 5), ('case1_simple_valid_program.py', 0), - ('case2_boolean_expressions.py', 1), - ('case3_redefining_builtin.py', 1), - ('case4_naming.py', 10), + ('case2_boolean_expressions.py', 8), + ('case3_redefining_builtin.py', 2), + ('case4_naming.py', 8), ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), - ('case7_empty_lines.py', 0), + ('case7_empty_lines.py', 2), ('case10_unused_variable_in_loop.py', 1), - ('case13_complex_logic.py', 3), - ('case13_complex_logic_2.py', 1), + ('case13_complex_logic.py', 12), + ('case13_complex_logic_2.py', 3), ('case11_redundant_parentheses.py', 0), ('case14_returns_errors.py', 4), ('case16_comments.py', 0), @@ -29,6 +30,12 @@ ('case21_imports.py', 2), ('case25_django.py', 0), ('case31_spellcheck.py', 0), + ('case32_string_format.py', 34), + ('case33_commas.py', 14), + ('case34_cohesion.py', 1), + ('case35_line_break.py', 11), + ('case36_unpacking.py', 3), + ('case37_wildcard_import.py', 7), ] @@ -39,7 +46,7 @@ def test_file_with_issues(file_name: str, n_issues: int): 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) + issues = list(filter(lambda i: i.type != IssueType.INFO, filter_low_measure_issues(issues, Language.PYTHON))) assert len(issues) == n_issues @@ -48,20 +55,31 @@ def test_file_with_issues(file_name: str, n_issues: int): ('case0_spaces.py', IssuesTestInfo(n_code_style=5)), ('case1_simple_valid_program.py', IssuesTestInfo()), ('case2_boolean_expressions.py', IssuesTestInfo(n_code_style=1, - n_cc=8)), - ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=1)), - ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=0, n_cc=5)), + n_best_practices=4, + n_error_prone=1, + n_cc=8, + n_other_complexity=2)), + ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=2)), + ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_cc=5, n_cohesion=1)), ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, n_cc=1)), - ('case8_good_class.py', IssuesTestInfo(n_cc=1)), - ('case7_empty_lines.py', IssuesTestInfo(n_cc=4)), + ('case8_good_class.py', IssuesTestInfo(n_cc=1, n_cohesion=1)), + ('case7_empty_lines.py', IssuesTestInfo(n_cc=5, n_cohesion=2)), ('case10_unused_variable_in_loop.py', IssuesTestInfo(n_best_practices=1, n_cc=1)), - ('case13_complex_logic.py', IssuesTestInfo(n_cc=6)), - ('case13_complex_logic_2.py', IssuesTestInfo(n_cc=2)), + ('case13_complex_logic.py', IssuesTestInfo(n_cc=5, n_other_complexity=10)), + ('case13_complex_logic_2.py', IssuesTestInfo(n_cc=2, n_other_complexity=2)), ('case14_returns_errors.py', IssuesTestInfo(n_best_practices=1, n_error_prone=3, n_cc=4)), + ('case35_line_break.py', IssuesTestInfo(n_best_practices=1, + n_code_style=10, + n_cc=1)), + ('case32_string_format.py', IssuesTestInfo(n_error_prone=28, n_other_complexity=6)), + ('case33_commas.py', IssuesTestInfo(n_code_style=14, n_cc=4)), + ('case34_cohesion.py', IssuesTestInfo(n_cc=6, n_cohesion=2)), + ('case36_unpacking.py', IssuesTestInfo(n_error_prone=2, n_cc=1, n_other_complexity=1)), + ('case37_wildcard_import.py', IssuesTestInfo(n_best_practices=1, n_error_prone=3, n_cc=2, n_other_complexity=2)), ] @@ -79,9 +97,12 @@ def test_file_with_issues_info(file_name: str, expected_issues_info: IssuesTestI def test_parse(): file_name = 'test.py' - output = ('test.py:1:11:W602:test 1\n' - 'test.py:2:12:E703:test 2\n' - 'test.py:3:13:SC200:test 3') + output = f"""\ + {file_name}:1:11:W602:test 1 + {file_name}:2:12:E703:test 2 + {file_name}:3:13:SC200:test 3 + """ + output = dedent(output) issues = Flake8Inspector.parse(output) @@ -99,7 +120,7 @@ def test_choose_issue_type(): expected_issue_types = [ IssueType.ERROR_PRONE, IssueType.INFO, 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 8f9491a0..5b182540 100644 --- a/test/python/inspectors/test_local_review.py +++ b/test/python/inspectors/test_local_review.py @@ -1,20 +1,19 @@ import json from collections import namedtuple +from test.python.inspectors import PYTHON_DATA_FOLDER import pytest - from src.python.review.application_config import ApplicationConfig from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.quality.model import QualityType from src.python.review.reviewers.perform_review import OutputFormat, PathNotExists, perform_and_print_review -from test.python.inspectors import PYTHON_DATA_FOLDER Args = namedtuple('Args', [ 'path', 'allow_duplicates', 'disable', 'format', - 'handler' + 'handler', ]) @@ -24,7 +23,7 @@ def config() -> ApplicationConfig: disabled_inspectors={InspectorType.INTELLIJ}, allow_duplicates=False, n_cpu=1, - inspectors_config={'n_cpu': 1} + inspectors_config={"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_pmd_inspector.py b/test/python/inspectors/test_pmd_inspector.py index 18b13af4..3702a79b 100644 --- a/test/python/inspectors/test_pmd_inspector.py +++ b/test/python/inspectors/test_pmd_inspector.py @@ -1,7 +1,8 @@ -import pytest +from test.python.inspectors import JAVA_DATA_FOLDER +import pytest from src.python.review.inspectors.pmd.pmd import PMDInspector -from test.python.inspectors import JAVA_DATA_FOLDER + from .conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ @@ -14,7 +15,7 @@ ('test_comparing_strings.java', 3), ('test_constants.java', 4), ('test_covariant_equals.java', 1), - ('test_curly_braces.java', 2), + ('test_curly_braces.java', 0), ('test_double_checked_locking.java', 2), ('test_for_loop.java', 2), ('test_implementation_types.java', 0), @@ -31,6 +32,7 @@ ('test_valid_curly_braces.java', 0), ('test_when_only_equals_overridden.java', 1), ('test_valid_spaces.java', 0), + ('test_multiple_literals.java', 1), ] diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index e1da830a..fc381234 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -1,15 +1,17 @@ -import pytest +import textwrap +from test.python.inspectors import PYTHON_DATA_FOLDER +import pytest from src.python.review.inspectors.issue import IssueType from src.python.review.inspectors.pylint.pylint import PylintInspector -from test.python.inspectors import PYTHON_DATA_FOLDER + from .conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_spaces.py', 0), ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 3), - ('case3_redefining_builtin.py', 1), + ('case3_redefining_builtin.py', 0), ('case4_naming.py', 3), ('case5_returns.py', 1), ('case6_unused_variables.py', 4), @@ -30,6 +32,8 @@ ('case25_django.py', 0), ('case27_using_requests.py', 0), ('case30_allow_else_return.py', 0), + ('case36_unpacking.py', 0), + ('case37_wildcard_import.py', 1), ] @@ -46,8 +50,11 @@ def test_file_with_issues(file_name: str, n_issues: int): def test_parse(): file_name = 'test.py' - output = 'test.py:1:11:R0123:test 1\n' \ - 'test.py:2:12:C1444:test 2' + output = f"""\ + {file_name}:1:11:R0123:test 1 + {file_name}:2:12:C1444:test 2 + """ + output = textwrap.dedent(output) issues = PylintInspector.parse(output) @@ -67,7 +74,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_python_ast.py b/test/python/inspectors/test_python_ast.py index 09192cd3..880e4775 100644 --- a/test/python/inspectors/test_python_ast.py +++ b/test/python/inspectors/test_python_ast.py @@ -1,12 +1,14 @@ import ast +from test.python.inspectors import PYTHON_AST_DATA_FOLDER, PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata import pytest - from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.pyast.python_ast import BoolExpressionLensGatherer, FunctionLensGatherer, \ - PythonAstInspector -from test.python.inspectors import PYTHON_DATA_FOLDER, PYTHON_AST_DATA_FOLDER -from test.python.inspectors.conftest import use_file_metadata +from src.python.review.inspectors.pyast.python_ast import ( + BoolExpressionLensGatherer, + FunctionLensGatherer, + PythonAstInspector, +) FILE_NAMES_AND_N_ISSUES = [ ('case0_spaces.py', 0), diff --git a/test/python/inspectors/test_radon_inspector.py b/test/python/inspectors/test_radon_inspector.py new file mode 100644 index 00000000..4ed41e1e --- /dev/null +++ b/test/python/inspectors/test_radon_inspector.py @@ -0,0 +1,52 @@ +from test.python.inspectors import PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +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.inspectors.tips import get_maintainability_index_tip +from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues + + +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] diff --git a/test/python/inspectors/test_springlint_inspector.py b/test/python/inspectors/test_springlint_inspector.py index 18ee8dfb..673aa30c 100644 --- a/test/python/inspectors/test_springlint_inspector.py +++ b/test/python/inspectors/test_springlint_inspector.py @@ -1,6 +1,7 @@ +from test.python.inspectors import SPRING_DATA_FOLDER + from src.python.review.inspectors.issue import IssueType from src.python.review.inspectors.springlint.springlint import SpringlintInspector -from test.python.inspectors import SPRING_DATA_FOLDER def test_controller_with_smells(): diff --git a/test/resources/inspectors/java/test_multiple_literals.java b/test/resources/inspectors/java/test_multiple_literals.java new file mode 100644 index 00000000..58a9e2ee --- /dev/null +++ b/test/resources/inspectors/java/test_multiple_literals.java @@ -0,0 +1,32 @@ +class Main { + public static void main(String[] args) { + // ok + String shortRareLiteral1 = "12"; + String shortRareLiteral2 = "12"; + String shortRareLiteral3 = "12"; + + // ok + String longRareLiteral1 = "123"; + String longRareLiteral2 = "123"; + String longRareLiteral3 = "123"; + + // ok + String shortFrequentLiteral1 = "34"; + String shortFrequentLiteral2 = "34"; + String shortFrequentLiteral3 = "34"; + String shortFrequentLiteral4 = "34"; + + // warning + String longFrequentLiteral1 = "456"; + String longFrequentLiteral2 = "456"; + String longFrequentLiteral3 = "456"; + String longFrequentLiteral4 = "456"; + + System.out.println( + shortRareLiteral1 + shortRareLiteral2 + shortRareLiteral3 + + longRareLiteral1 + longRareLiteral2 + longRareLiteral3 + + shortFrequentLiteral1 + shortFrequentLiteral2 + shortFrequentLiteral3 + shortFrequentLiteral4 + + longFrequentLiteral1 + longFrequentLiteral2 + longFrequentLiteral3 + longFrequentLiteral4 + ); + } +} diff --git a/test/resources/inspectors/python/case13_complex_logic.py b/test/resources/inspectors/python/case13_complex_logic.py index 60718a89..3208a9bf 100644 --- a/test/resources/inspectors/python/case13_complex_logic.py +++ b/test/resources/inspectors/python/case13_complex_logic.py @@ -8,16 +8,22 @@ def max_of_three(a, b, c): return a +FEW_UNITS_NUMBER = 9 +PACK_UNITS_NUMBER = 49 +HORDE_UNITS_NUMBER = 499 +SWARM_UNITS_NUMBER = 999 + + def army_of_units(count): if count < 1: print("no army") - elif count <= 9: + elif count <= FEW_UNITS_NUMBER: print('few') - elif count <= 49: + elif count <= PACK_UNITS_NUMBER: print('pack') - elif count <= 499: + elif count <= HORDE_UNITS_NUMBER: print("horde") - elif count <= 999: + elif count <= SWARM_UNITS_NUMBER: print('swarm') else: print('legion') @@ -65,60 +71,38 @@ def determine_strange_quark(spin, charge): print("Higgs boson Boson") +SHEEP_PRICE = 6769 +COW_PRICE = 3848 +PIG_PRICE = 1296 +GOAT_PRICE = 678 +DOG_PRICE = 137 +CHICKEN_PRICE = 23 + + def buy_animal(money): - if money >= 6769: - print(str(money // 6769) + " sheep") - elif money >= 3848: + if money >= SHEEP_PRICE: + number_of_sheep = money // SHEEP_PRICE + print(f"{number_of_sheep} sheep") + elif money >= COW_PRICE: print("1 cow") - elif money >= 1296: - if money // 1296 == 1: + elif money >= PIG_PRICE: + if money // PIG_PRICE == 1: print("1 pig") else: print("2 pigs") - elif money >= 678: + elif money >= GOAT_PRICE: print("1 goat") - elif money >= 137: - if money // 137 == 1: + elif money >= DOG_PRICE: + if money // DOG_PRICE == 1: print("1 dog") else: - print(str(money // 137) + " dogs") - elif money >= 23: - if money // 23 == 1: + number_of_dogs = money // DOG_PRICE + print(f"{number_of_dogs} dogs") + elif money >= CHICKEN_PRICE: + if money // CHICKEN_PRICE == 1: print("1 chicken") else: - print(str(money // 23) + " chickens") + number_of_chickens = money // CHICKEN_PRICE + print(f"{number_of_chickens} chickens") else: print("None") - - -def fun_with_complex_logic(a, b, c): - d = 0 - if a > 10: - d = 30 - elif a < 100: - d = 50 - elif a == 300 and b == 40: - for i in range(9): - a += i - elif a == 200: - if b > 300 and c < 50: - d = 400 - else: - d = 800 - elif a == 2400: - if b > 500 and c < 50: - d = 400 - else: - d = 800 - elif a == 1000: - if b == 900: - if c == 1000: - d = 10000 - else: - d = 900 - elif c == 300: - d = 1000 - elif a + b == 400: - d = 400 - print(d) - return d diff --git a/test/resources/inspectors/python/case13_complex_logic_2.py b/test/resources/inspectors/python/case13_complex_logic_2.py index 1eaf13dc..8f670bc1 100644 --- a/test/resources/inspectors/python/case13_complex_logic_2.py +++ b/test/resources/inspectors/python/case13_complex_logic_2.py @@ -1,10 +1,14 @@ elements = list(input('Enter cells: ')) y = 0 o = 0 + +CROSS_SYMBOL = 'X' +NOUGHT_SYMBOL = 'O' + for x in elements: - if x == 'X': + if x == CROSS_SYMBOL: y = y + 1 - elif x == 'O': + elif x == NOUGHT_SYMBOL: o = o + 1 odds = abs(y - o) @@ -22,8 +26,8 @@ full_field = [up_row, up_col, mid_row, mid_col, down_row, down_col, diagonal_1, diagonal_2] -x_win = ['X', 'X', 'X'] -o_win = ['O', 'O', 'O'] +x_win = [CROSS_SYMBOL, CROSS_SYMBOL, CROSS_SYMBOL] +o_win = [NOUGHT_SYMBOL, NOUGHT_SYMBOL, NOUGHT_SYMBOL] field = f""" --------- @@ -35,10 +39,10 @@ if odds < 2: if x_win in full_field and o_win not in full_field: print(field) - print('X wins') + print(f'{CROSS_SYMBOL} wins') elif o_win in full_field and x_win not in full_field: print(field) - print('O wins') + print(f'{NOUGHT_SYMBOL} wins') elif o_win in full_field and x_win in full_field: print(field) print('Impossible') diff --git a/test/resources/inspectors/python/case18_comprehensions.py b/test/resources/inspectors/python/case18_comprehensions.py index 3ed9be6f..4365018e 100644 --- a/test/resources/inspectors/python/case18_comprehensions.py +++ b/test/resources/inspectors/python/case18_comprehensions.py @@ -11,7 +11,7 @@ dict(((1, 2),)) # {1: 2} -sum([x ** 2 for x in range(10)]) # sum(x ** 2 for x in range(10)) +sum([x ** 2 for x in range(10)]) # we allow this since flake8-comprehension 3.4.0 (see its changelog) test_dict = dict() # we allow this test_list = list() # we allow this diff --git a/test/resources/inspectors/python/case32_string_format.py b/test/resources/inspectors/python/case32_string_format.py new file mode 100644 index 00000000..bee24195 --- /dev/null +++ b/test/resources/inspectors/python/case32_string_format.py @@ -0,0 +1,143 @@ +hello_world = "Hello, World!" +hello = "Hello" +world = "World" +some_list = [hello, world] +some_dict = {"hello": hello, "world": world} + +# ----------------------------------------------------------------------------------- + +# Correct +print("{0}!".format(hello_world)) + +# Correct +print("{}!".format(hello_world)) + +# Correct +print("{0}, {1}!".format(hello, world)) + +# Correct +print("{0}, {1}!".format(*some_list)) + +# Correct +print("{1}, {0}!".format(world, hello)) + +# Correct +print("{1}, {0}!".format(*some_list)) + +# Correct +print("{}, {}!".format(hello, world)) + +# Correct +print("{}, {}!".format(*some_list)) + +# Correct +print("{hello}, {world}!".format(hello=hello, world=world)) + +# Correct +print("{hello}, {world}!".format(**some_dict)) + +# ----------------------------------------------------------------------------------- + +# Correct +print(str.format("{0}!", hello_world)) + +# Correct +print(str.format("{}!", hello_world)) + +# Correct +print(str.format("{0}, {1}!", hello, world)) + +# Correct +print(str.format("{0}, {1}!", *some_list)) + +# Correct +print(str.format("{1}, {0}!", world, hello)) + +# Correct +print(str.format("{1}, {0}!", *some_list)) + +# Correct +print(str.format("{}, {}!", hello, world)) + +# Correct +print(str.format("{}, {}!", *some_list)) + +# Correct +print(str.format("{hello}, {world}!", hello=hello, world=world)) + +# Correct +print(str.format("{hello}, {world}!", **some_dict)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-201 +print("{0}, {1}!".format(hello)) + +# Wrong: P-201 +print("{}, {}!".format(hello)) + +# Wrong: P-201 +print(str.format("{0}, {1}!", hello)) + +# Wrong: P-201 +print(str.format("{}, {}!", hello)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-202 +print("{hello}, {world}!".format(hello=hello)) + +# Wrong: P-202 +print(str.format("{hello}, {world}!", hello=hello)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-203 +print("{0}!".format(**some_dict)) + +# Wrong: P-203 +print("{}!".format(**some_dict)) + +# Wrong: P-203 +print(str.format("{0}!", **some_dict)) + +# Wrong: P-203 +print(str.format("{}!", **some_dict)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-204 +print("{hello_world}!".format(*some_list)) + +# Wrong: P-204 +print(str.format("{hello_world}!", *some_list)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-205 +print("{}, {0}!".format(hello, world)) + +# Wrong: P-205 +print(str.format("{}, {0}!", hello, world)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-301 +print("{0}".format(hello_world, hello, world)) + +# Wrong: P-301 +print("{}".format(hello_world, hello, world)) + +# Wrong: P-301 +print(str.format("{0}", hello_world, hello, world)) + +# Wrong: P-301 +print(str.format("{}", hello_world, hello, world)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-302 +print("{hello_world}".format(hello_world=hello_world, hello=hello, world=world)) + +# Wrong: P-302 +print(str.format("{hello_world}", hello_world=hello_world, hello=hello, world=world)) diff --git a/test/resources/inspectors/python/case33_commas.py b/test/resources/inspectors/python/case33_commas.py new file mode 100644 index 00000000..6af55963 --- /dev/null +++ b/test/resources/inspectors/python/case33_commas.py @@ -0,0 +1,153 @@ +# Wrong C-812 +from math import ( + log +) + +# Correct +from math import ( + sin, +) + +# Wrong C-812 +bad_multiline_dict = { + "first": 1, + "second": 2 +} + +# Correct +good_multiline_dict = { + "first": 1, + "second": 2, +} + +# Wrong C-812 +bad_multiline_list = [ + 1, + 2, + 3 +] + +# Correct +good_multiline_list = [ + 1, + 2, + 3, +] + +# Wrong C-812 +bad_multiline_tuple = ( + 3, + 4 +) + +good_multiline_tuple = ( + 3, + 4, +) + + +# Wrong C-812 +def bad_function( + a, + b +): + return log(a, b) + + +bad_function( + 1, + 2 +) + +bad_function( + a=1, + b=2 +) + + +# Correct +def good_function( + a, + b, +): + return a + sin(b) + + +good_function( + 1, + 2, +) + +good_function( + a=1, + b=2, +) + +# Wrong: C-813 +print( + "Hello", + "World" +) + +# Correct +print( + "Hello", + "World", +) + + +# Wrong: C-816 +def bad_function_with_unpacking( + a, + b, + **kwargs +): + pass + + +# Correct +def good_function_with_unpacking( + a, + b, + **kwargs, +): + pass + + +# Wrong: C-815 +good_function_with_unpacking( + 1, + 2, + **good_multiline_dict +) + +# Correct +good_function_with_unpacking( + 1, + 2, + **good_multiline_dict, +) + +# Wrong: C-818 +bad_comma = 1, + +# Correct +good_comma = (1,) + +# Wrong: C-819 +bad_list = [1, 2, 3, ] + +# Correct: +good_list = [1, 2, 3] + +# Wrong: C-819 +bad_dict = {"1": 1, "2": 2, "3": 3, } + +# Correct: +good_dict = {"1": 1, "2": 2, "3": 3} + +# Wrong: C-819 +bad_tuple = (1, 2, 3,) + +# Correct +good_tuple = (1, 2, 3) diff --git a/test/resources/inspectors/python/case34_cohesion.py b/test/resources/inspectors/python/case34_cohesion.py new file mode 100644 index 00000000..8102675a --- /dev/null +++ b/test/resources/inspectors/python/case34_cohesion.py @@ -0,0 +1,28 @@ +from math import sqrt + + +class BadClass: + def __init__(self, x: int, y: int): + self.x = x + self.y = y + + @staticmethod + def length(x: int, y: int) -> float: + return sqrt(x ** 2 + y ** 2) + + @staticmethod + def dot(self_x: int, self_y: int, other_x: int, other_y: int) -> int: + return self_x * other_x + self_y * other_y + + +class GoodClass(object): + def __init__(self, x: int, y: int): + self.x = x + self.y = y + + @property + def length(self) -> float: + return sqrt(self.dot(self.x, self.y)) + + def dot(self, other_x: int, other_y: int) -> int: + return self.x * other_x + self.y * other_y diff --git a/test/resources/inspectors/python/case35_line_break.py b/test/resources/inspectors/python/case35_line_break.py new file mode 100644 index 00000000..ba718ea8 --- /dev/null +++ b/test/resources/inspectors/python/case35_line_break.py @@ -0,0 +1,54 @@ +# Wrong +from math import exp, \ + log + +# Correct +from math import ( + sin, + cos, +) + + +# Wrong +def do_something_wrong(x: float, y: float): + if sin(x) == cos(y) \ + and exp(x) == log(y): + print("Do not do that!") + + +print(do_something_wrong(1, 2)) + +# Wrong +wrong_string = "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. " \ + + "Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, " \ + + "nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. " \ + + "Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, " \ + + "vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. " \ + + "Nullam dictum felis eu pede mollis pretium. Integer tincidunt. Cras dapibus. " \ + + "Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, " \ + + "porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, " \ + + "viverra quis, feugiat a," +print(wrong_string) + +# Correct +correct_string = ("Lorem ipsum dolor sit amet, consectetuer adipiscing elit. " + + "Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis " + + "parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, " + + "pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, " + + "aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, " + + "venenatis vitae, justo. Nullam dictum felis eu pede mollis pretium. Integer tincidunt. " + + "Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. " + + "Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. " + + "Aliquam lorem ante, dapibus in, viverra quis, feugiat a," + ) +print(correct_string) + +other_correct_string = """ + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec euismod vitae libero ut consequat. + Fusce quis ultrices sem, vitae viverra mi. Praesent fermentum quam ac volutpat condimentum. + Proin mauris orci, molestie vel fermentum vel, consectetur vel metus. Quisque vitae mollis magna. + In hac habitasse platea dictumst. Pellentesque sed diam eget dolor ultricies faucibus id sed quam. + Nam risus erat, varius ut risus a, tincidunt vulputate magna. Etiam lacinia a eros non faucibus. + In facilisis tempor nisl, sit amet feugiat lacus viverra quis. +""" +print(other_correct_string) diff --git a/test/resources/inspectors/python/case36_unpacking.py b/test/resources/inspectors/python/case36_unpacking.py new file mode 100644 index 00000000..74b4ec9c --- /dev/null +++ b/test/resources/inspectors/python/case36_unpacking.py @@ -0,0 +1,10 @@ +[a, b, c], [x, y, z] = (sorted(map(int, input().split())) for _ in 'lm') +if [a, b, c] == [x, y, z]: + a = "Boxes are equal" +elif a <= x and b <= y and c <= z: + a = "The first box is smaller than the second one" +elif a >= x and b >= y and c >= z: + a = "The first box is larger than the second one" +else: + a = "Boxes are incomparable" +print(a) diff --git a/test/resources/inspectors/python/case37_wildcard_import.py b/test/resources/inspectors/python/case37_wildcard_import.py new file mode 100644 index 00000000..ed12a4b1 --- /dev/null +++ b/test/resources/inspectors/python/case37_wildcard_import.py @@ -0,0 +1,33 @@ +from numpy import * + +n, m = [int(_) for _ in input().split()] +mat = zeros((n, m)) +s = 1 +for j in range(n): + for i in range(j, m - j): + if s > n * m: + break + mat[j][i] = s + s += 1 + + for i in range(j - n + 1, -j): + if s > n * m: + break + mat[i][-j - 1] = s + s += 1 + for i in range(-j - 2, -m + j - 1, -1): + if s > n * m: + break + mat[-j - 1][i] = s + s += 1 + + for i in range(-2 - j, -n + j, -1): + if s > n * m: + break + mat[i][j] = s + s += 1 + +for r in range(n): + for c in range(m): + print(str(int(mat[r][c])).ljust(2), end=' ') + print() diff --git a/test/resources/inspectors/python/case3_redefining_builtin.py b/test/resources/inspectors/python/case3_redefining_builtin.py index 81d7021b..ddcca3c4 100644 --- a/test/resources/inspectors/python/case3_redefining_builtin.py +++ b/test/resources/inspectors/python/case3_redefining_builtin.py @@ -1,4 +1,7 @@ a = int(input()) b = int(input()) -list = list(filter(lambda x: x % 3 == 0, range(a, b + 1))) + +range = range(a, b + 1) + +list = list(filter(lambda x: x % 3 == 0, range)) print(sum(list) / len(list)) diff --git a/test/resources/inspectors/python/case4_naming.py b/test/resources/inspectors/python/case4_naming.py index 7d4664e8..522ae0eb 100644 --- a/test/resources/inspectors/python/case4_naming.py +++ b/test/resources/inspectors/python/case4_naming.py @@ -13,7 +13,7 @@ def myFun(self): print('hello 1') def my_fun(self, QQ): - print('hello 2' + QQ) + print('hello 2 {}'.format(QQ)) @classmethod def test_fun(first): diff --git a/test/resources/inspectors/python/case7_empty_lines.py b/test/resources/inspectors/python/case7_empty_lines.py index 81b73b98..25760099 100644 --- a/test/resources/inspectors/python/case7_empty_lines.py +++ b/test/resources/inspectors/python/case7_empty_lines.py @@ -17,6 +17,7 @@ def minus_age(self, value): self.age -= value class AnotherClass: - pass + def do_something(self): + print(1) -print(10 + 20) +print(10) diff --git a/test/resources/inspectors/python_ast/__init__.py b/test/resources/inspectors/python_ast/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/whitelist.txt b/whitelist.txt index 8f29cf65..b1834c32 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -54,6 +54,35 @@ unlink utils param params +changelog +multiline +sqrt +WPS +OOP +mccabe +mcs +dicts +misrefactored +src +textwrap +dedent +maintainabilities +parsers +fs +KTS +nl +splitext +dirname +hyperstyle +XLSX +Eval +eval +openpyxl +dataframe +writelines +rmdir +df +unique # Springlint issues cbo dit @@ -61,3 +90,9 @@ lcom noc nom wmc +util +Namespace +case18 +case34 +removeprefix +toplevel