From 41eddcdb2f340251865db81780a7cccfd58f257a Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 14 Mar 2021 19:33:10 +0300 Subject: [PATCH 01/37] Added param and params to whitelist --- whitelist.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/whitelist.txt b/whitelist.txt index b80187db..8f29cf65 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -52,6 +52,8 @@ subdirs sym unlink utils +param +params # Springlint issues cbo dit From d176f7709dd9580f1170f4899da8d2e5e7d4a90f Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 14 Mar 2021 19:33:53 +0300 Subject: [PATCH 02/37] Fixed flake8-spellcheck issue --- src/python/review/inspectors/flake8/flake8.py | 2 +- test/python/inspectors/test_flake8_inspector.py | 4 ++-- .../functional_tests/file_or_project/project/one.py | 4 ++-- .../functional_tests/file_or_project/project/other.py | 2 +- .../resources/inspectors/python/case13_complex_logic_2.py | 6 +++--- test/resources/inspectors/python/case14_returns_errors.py | 8 ++++---- test/resources/inspectors/python/case18_comprehensions.py | 6 +++--- test/resources/inspectors/python/case21_imports.py | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 8fe5a567..74104af6 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -34,7 +34,7 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: @classmethod def parse(cls, output: str) -> List[BaseIssue]: - row_re = re.compile(r'^(.*):(\d+):(\d+):([A-Z]\d{3}):(.*)$', re.M) + row_re = re.compile(r'^(.*):(\d+):(\d+):([A-Z]+\d{3}):(.*)$', re.M) cc_description_re = re.compile(r"'(.+)' is too complex \((\d+)\)") issues: List[BaseIssue] = [] diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index c723a2db..07864b72 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -12,7 +12,7 @@ ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 1), ('case3_redefining_builtin.py', 1), - ('case4_naming.py', 7), + ('case4_naming.py', 10), ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), @@ -49,7 +49,7 @@ def test_file_with_issues(file_name: str, n_issues: int): ('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_cc=5)), + ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=3, n_cc=5)), ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, n_cc=1)), ('case8_good_class.py', IssuesTestInfo(n_cc=1)), diff --git a/test/resources/functional_tests/file_or_project/project/one.py b/test/resources/functional_tests/file_or_project/project/one.py index 965b9075..1ed52ea9 100644 --- a/test/resources/functional_tests/file_or_project/project/one.py +++ b/test/resources/functional_tests/file_or_project/project/one.py @@ -1,4 +1,4 @@ -from other import do_smth_useless +from other import do_something_useless if __name__ == '__main__': - do_smth_useless() \ No newline at end of file + do_something_useless() \ No newline at end of file diff --git a/test/resources/functional_tests/file_or_project/project/other.py b/test/resources/functional_tests/file_or_project/project/other.py index 5ef7e762..8553df01 100644 --- a/test/resources/functional_tests/file_or_project/project/other.py +++ b/test/resources/functional_tests/file_or_project/project/other.py @@ -1,4 +1,4 @@ -def do_smth_useless(): +def do_something_useless(): a = 1 b = 2 c = 3 \ No newline at end of file diff --git a/test/resources/inspectors/python/case13_complex_logic_2.py b/test/resources/inspectors/python/case13_complex_logic_2.py index 23e0ee63..1eaf13dc 100644 --- a/test/resources/inspectors/python/case13_complex_logic_2.py +++ b/test/resources/inspectors/python/case13_complex_logic_2.py @@ -17,10 +17,10 @@ mid_col = [elements[1], elements[4], elements[7]] down_col = [elements[2], elements[5], elements[8]] -diagonal1 = [elements[0], elements[4], elements[8]] -diagonal2 = [elements[2], elements[4], elements[6]] +diagonal_1 = [elements[0], elements[4], elements[8]] +diagonal_2 = [elements[2], elements[4], elements[6]] -full_field = [up_row, up_col, mid_row, mid_col, down_row, down_col, diagonal1, diagonal2] +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'] diff --git a/test/resources/inspectors/python/case14_returns_errors.py b/test/resources/inspectors/python/case14_returns_errors.py index 5b91e4d1..ff66f62b 100644 --- a/test/resources/inspectors/python/case14_returns_errors.py +++ b/test/resources/inspectors/python/case14_returns_errors.py @@ -1,22 +1,22 @@ -def f1(y): +def f_1(y): if not y: return return None # error! -def f2(y): +def f_2(y): if not y: return # error! return 1 -def f3(y): +def f_3(y): if not y: return # error! return 1 -def f4(): +def f_4(): a = 1 # some code that not using `a` print('test') diff --git a/test/resources/inspectors/python/case18_comprehensions.py b/test/resources/inspectors/python/case18_comprehensions.py index 0c01854a..3ed9be6f 100644 --- a/test/resources/inspectors/python/case18_comprehensions.py +++ b/test/resources/inspectors/python/case18_comprehensions.py @@ -13,9 +13,9 @@ sum([x ** 2 for x in range(10)]) # sum(x ** 2 for x in range(10)) -dct = dict() # we allow this -lst = list() # we allow this -tpl = tuple() # we allow this +test_dict = dict() # we allow this +test_list = list() # we allow this +test_tuple = tuple() # we allow this list([0 for _ in range(10)]) diff --git a/test/resources/inspectors/python/case21_imports.py b/test/resources/inspectors/python/case21_imports.py index 3c1ed10e..bdac468b 100644 --- a/test/resources/inspectors/python/case21_imports.py +++ b/test/resources/inspectors/python/case21_imports.py @@ -1,5 +1,5 @@ import itertools import functools -from functools import partialmethod +from math import ceil -print(partialmethod) \ No newline at end of file +print(ceil(0.1)) \ No newline at end of file From 1f77c30a14b51ac39b57f61b2f76c1017812b3a3 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 16 Mar 2021 13:52:22 +0300 Subject: [PATCH 03/37] Added new test --- test/python/inspectors/test_flake8_inspector.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 07864b72..7de27bc3 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -79,16 +79,18 @@ 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') + 'test.py:2:12:E703:test 2\n' + 'test.py:3:13:SC200:test 3') issues = Flake8Inspector.parse(output) assert all(str(issue.file_path) == file_name for issue in issues) - assert [issue.line_no for issue in issues] == [1, 2] - assert [issue.column_no for issue in issues] == [11, 12] - assert [issue.description for issue in issues] == ['test 1', 'test 2'] + assert [issue.line_no for issue in issues] == [1, 2, 3] + assert [issue.column_no for issue in issues] == [11, 12, 13] + assert [issue.description for issue in issues] == ['test 1', 'test 2', 'test 3'] assert [issue.type for issue in issues] == [IssueType.CODE_STYLE, - IssueType.CODE_STYLE] + IssueType.CODE_STYLE, + IssueType.BEST_PRACTICES] def test_choose_issue_type(): From 79f229bb6e896f8491437564637bd86f41b11093 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 21 Mar 2021 12:27:01 +0300 Subject: [PATCH 04/37] Added wps-light support --- requirements.txt | 1 + src/python/review/inspectors/flake8/.flake8 | 28 +++++++ src/python/review/inspectors/flake8/flake8.py | 16 +++- .../review/inspectors/flake8/issue_types.py | 75 +++++++++++++++++++ 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 6fe19370..caa6e2e2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,7 @@ flake8-return==1.1.1 flake8-spellcheck==0.14.0 mccabe==0.6.1 pep8-naming==0.11.1 +wps-light==0.15.2 # extra libraries and frameworks django==3.0.8 diff --git a/src/python/review/inspectors/flake8/.flake8 b/src/python/review/inspectors/flake8/.flake8 index 6270ce48..0ad30a62 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -13,3 +13,31 @@ 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. + WPS318, # Forbid extra indentation. TODO: Collision with standard flake8 indentation check + WPS323, # Forbid % formatting on strings. + WPS324, # Enforce consistent return statements. TODO: Collision with flake8-return + WPS335, # Forbid wrong for loop iter targets. + WPS358, # Forbid using float zeros: 0.0. + 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 + WPS527, # Require tuples as arguments for frozenset. + # WPS: OOP + WPS602, # Forbid @staticmethod decorator. diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 74104af6..facead83 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -5,7 +5,8 @@ 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.flake8.issue_types import CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE +from src.python.review.inspectors.flake8.issue_types import CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE, \ + WPS_RANGE_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.tips import get_cyclomatic_complexity_tip @@ -63,10 +64,21 @@ def parse(cls, output: str) -> List[BaseIssue]: @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) + regex_match = re.match(r'^([a-z]+)(\d+)$', code, re.IGNORECASE) + code_prefix = regex_match.group(1) + + # Handling WPS issues + if code_prefix == "WPS": + code_number = int(regex_match.group(2)) + for wps_range, issue_type in WPS_RANGE_TO_ISSUE_TYPE.items(): + if code_number in wps_range: + return issue_type + + # Handling other issues issue_type = CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix) if not issue_type: logger.warning(f'flake8: {code} - unknown error code') diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index e98a4836..9de7ec6e 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -15,6 +15,81 @@ # builtin naming 'A003': IssueType.BEST_PRACTICES, + + # 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.CODE_STYLE, # 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. +} + +WPS_RANGE_TO_ISSUE_TYPE: Dict[range, IssueType] = { + range(100, 200): IssueType.CODE_STYLE, # WPS type: Naming + range(200, 300): IssueType.COMPLEXITY, # WPS type: Complexity + range(300, 400): IssueType.BEST_PRACTICES, # WPS type: Consistency + range(400, 500): IssueType.BEST_PRACTICES, # WPS type: Best practices + range(500, 600): IssueType.BEST_PRACTICES, # WPS type: Refactoring + range(600, 700): IssueType.BEST_PRACTICES, # WPS type: OOP } CODE_PREFIX_TO_ISSUE_TYPE: Dict[str, IssueType] = { From 3e00358401f9d5e7236541b6ee89e3e718943388 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 21 Mar 2021 12:28:11 +0300 Subject: [PATCH 05/37] Fixed tests --- .../inspectors/test_flake8_inspector.py | 21 +++-- .../inspectors/test_pylint_inspector.py | 2 +- .../inspectors/python/case13_complex_logic.py | 82 ++++++++----------- .../python/case13_complex_logic_2.py | 16 ++-- .../python/case3_redefining_builtin.py | 5 +- .../inspectors/python/case4_naming.py | 2 +- .../inspectors/python/case7_empty_lines.py | 5 +- 7 files changed, 64 insertions(+), 69 deletions(-) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 7de27bc3..f5892902 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -10,16 +10,16 @@ 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), + ('case2_boolean_expressions.py', 8), + ('case3_redefining_builtin.py', 2), ('case4_naming.py', 10), ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), ('case7_empty_lines.py', 0), ('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), @@ -47,17 +47,20 @@ 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)), + n_best_practices=4, + n_error_prone=1, + n_cc=7, + n_other_complexity=2)), + ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=2)), ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=3, n_cc=5)), ('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)), + ('case7_empty_lines.py', IssuesTestInfo(n_cc=5)), ('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)), diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index 1ee00735..55ce551a 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -9,7 +9,7 @@ ('case0_spaces.py', 3), ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 3), - ('case3_redefining_builtin.py', 1), + ('case3_redefining_builtin.py', 2), ('case4_naming.py', 3), ('case5_returns.py', 1), ('case6_unused_variables.py', 4), 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/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) From c295334a12be74282e9d2210a36448298de8dbe5 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:06:45 +0300 Subject: [PATCH 06/37] Update build.yml --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6c56547d..f1bbf186 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,7 @@ jobs: # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,WPS --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - name: Set up Eslint run: | npm install eslint --save-dev @@ -40,4 +40,4 @@ jobs: run: java -version - name: Test with pytest run: | - pytest \ No newline at end of file + pytest From 5c9c32955636ee4c0e69fabe30b122c130f6fa37 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Sun, 21 Mar 2021 13:17:21 +0300 Subject: [PATCH 07/37] Small test fix --- test/python/inspectors/test_flake8_inspector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index f5892902..3b21c051 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -49,7 +49,7 @@ def test_file_with_issues(file_name: str, n_issues: int): ('case2_boolean_expressions.py', IssuesTestInfo(n_code_style=1, n_best_practices=4, n_error_prone=1, - n_cc=7, + 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_best_practices=3, n_cc=5)), From 542068e0ef762cd7300ee5ef71dea8d5041d9ba8 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Mon, 22 Mar 2021 09:32:59 +0300 Subject: [PATCH 08/37] Small code refactoring: 1) Fixed line break; 2) Added trailing comma. --- src/python/review/inspectors/flake8/flake8.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index facead83..92a82a99 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -5,8 +5,11 @@ 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.flake8.issue_types import CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE, \ - WPS_RANGE_TO_ISSUE_TYPE +from src.python.review.inspectors.flake8.issue_types import ( + CODE_PREFIX_TO_ISSUE_TYPE, + CODE_TO_ISSUE_TYPE, + WPS_RANGE_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.tips import get_cyclomatic_complexity_tip @@ -28,7 +31,7 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: f'--format={FORMAT}', f'--config={PATH_FLAKE8_CONFIG}', '--max-complexity', '0', - path + path, ] output = run_in_subprocess(command) return cls.parse(output) From aa3a6e7c46c1e26f2567a42206bb67d6effbad75 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 23 Mar 2021 16:04:18 +0300 Subject: [PATCH 09/37] Added support for flake8-broken-line --- requirements.txt | 1 + .../review/inspectors/flake8/issue_types.py | 3 ++ .../inspectors/python/case31_line_break.py | 54 +++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 test/resources/inspectors/python/case31_line_break.py diff --git a/requirements.txt b/requirements.txt index caa6e2e2..d4f4f31d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,6 +18,7 @@ flake8-spellcheck==0.14.0 mccabe==0.6.1 pep8-naming==0.11.1 wps-light==0.15.2 +flake8-broken-line==0.3.0 # extra libraries and frameworks django==3.0.8 diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index 9de7ec6e..5199b7f8 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -16,6 +16,9 @@ # builtin naming 'A003': IssueType.BEST_PRACTICES, + # flake8-broken-line + 'N400': IssueType.CODE_STYLE, + # 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. diff --git a/test/resources/inspectors/python/case31_line_break.py b/test/resources/inspectors/python/case31_line_break.py new file mode 100644 index 00000000..ba718ea8 --- /dev/null +++ b/test/resources/inspectors/python/case31_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) From f8c2a873e81d3a3ae25237c40413ad3e63ddd965 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 23 Mar 2021 16:06:16 +0300 Subject: [PATCH 10/37] Code refactoring --- .../intellij/issue_types/__init__.py | 22 ++++--- .../inspectors/parsers/checkstyle_parser.py | 22 +++++-- .../inspectors/springlint/springlint.py | 29 ++++++-- src/python/review/inspectors/tips.py | 66 ++++++++++++------- src/python/review/quality/evaluate_quality.py | 39 +++++++---- src/python/review/quality/model.py | 9 ++- src/python/review/run_tool.py | 9 ++- .../inspectors/test_flake8_inspector.py | 4 ++ .../inspectors/test_pylint_inspector.py | 9 ++- test/python/inspectors/test_python_ast.py | 9 ++- 10 files changed, 156 insertions(+), 62 deletions(-) diff --git a/src/python/review/inspectors/intellij/issue_types/__init__.py b/src/python/review/inspectors/intellij/issue_types/__init__.py index c6f21127..3482d8b4 100644 --- a/src/python/review/inspectors/intellij/issue_types/__init__.py +++ b/src/python/review/inspectors/intellij/issue_types/__init__.py @@ -1,15 +1,21 @@ 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.issue import IssueType + +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, +) 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/parsers/checkstyle_parser.py b/src/python/review/inspectors/parsers/checkstyle_parser.py index 9cf75acb..cae3593f 100644 --- a/src/python/review/inspectors/parsers/checkstyle_parser.py +++ b/src/python/review/inspectors/parsers/checkstyle_parser.py @@ -6,10 +6,24 @@ 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, + 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, +) logger = logging.getLogger(__name__) diff --git a/src/python/review/inspectors/springlint/springlint.py b/src/python/review/inspectors/springlint/springlint.py index 18f9d6b0..14ce8c89 100644 --- a/src/python/review/inspectors/springlint/springlint.py +++ b/src/python/review/inspectors/springlint/springlint.py @@ -9,11 +9,30 @@ 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, + 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, +) PATH_TOOLS_SPRINGLINT_FILES = Path(__file__).parent / 'files' PATH_SPRINGLINT_JAR = PATH_TOOLS_SPRINGLINT_FILES / 'springlint-0.6.jar' diff --git a/src/python/review/inspectors/tips.py b/src/python/review/inspectors/tips.py index c1d3fba1..5211b569 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,16 @@ 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.' + ) diff --git a/src/python/review/quality/evaluate_quality.py b/src/python/review/quality/evaluate_quality.py index 3d78e392..0f301db0 100644 --- a/src/python/review/quality/evaluate_quality.py +++ b/src/python/review/quality/evaluate_quality.py @@ -4,25 +4,38 @@ 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.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.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 diff --git a/src/python/review/quality/model.py b/src/python/review/quality/model.py index ae71f211..741c761b 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 @@ -80,8 +81,12 @@ 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 = textwrap.dedent( + f"""\ + Next level: {self.next_quality_type.value} + Next level requirements: + """ + ) 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/run_tool.py b/src/python/review/run_tool.py index fdbb4b67..d0ba0891 100644 --- a/src/python/review/run_tool.py +++ b/src/python/review/run_tool.py @@ -13,8 +13,13 @@ 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__) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 3b21c051..f844011d 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -28,6 +28,7 @@ ('case19_bad_indentation.py', 3), ('case21_imports.py', 2), ('case25_django.py', 0), + ('case31_line_break.py', 11), ] @@ -64,6 +65,9 @@ def test_file_with_issues(file_name: str, n_issues: int): ('case14_returns_errors.py', IssuesTestInfo(n_best_practices=1, n_error_prone=3, n_cc=4)), + ('case31_line_break.py', IssuesTestInfo(n_best_practices=1, + n_code_style=10, + n_cc=1)), ] diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index 55ce551a..ca9f9cda 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -1,3 +1,5 @@ +import textwrap + import pytest from src.python.review.inspectors.issue import IssueType @@ -46,8 +48,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 = """\ + test.py:1:11:R0123:test 1 + test.py:2:12:C1444:test 2 + """ + output = textwrap.dedent(output) issues = PylintInspector.parse(output) diff --git a/test/python/inspectors/test_python_ast.py b/test/python/inspectors/test_python_ast.py index 09192cd3..fed3c542 100644 --- a/test/python/inspectors/test_python_ast.py +++ b/test/python/inspectors/test_python_ast.py @@ -3,8 +3,13 @@ import pytest from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.pyast.python_ast import BoolExpressionLensGatherer, FunctionLensGatherer, \ - PythonAstInspector + +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 48ef2af736900e2cde6257ae0ac73a1a1140cb38 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Fri, 26 Mar 2021 12:23:05 +0300 Subject: [PATCH 11/37] Added support for flake8-string-format --- requirements.txt | 1 + src/python/review/inspectors/flake8/.flake8 | 8 ++++++++ src/python/review/inspectors/flake8/issue_types.py | 1 + test/python/inspectors/test_flake8_inspector.py | 2 ++ 4 files changed, 12 insertions(+) diff --git a/requirements.txt b/requirements.txt index d4f4f31d..8d913520 100644 --- a/requirements.txt +++ b/requirements.txt @@ -19,6 +19,7 @@ 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 # extra libraries and frameworks django==3.0.8 diff --git a/src/python/review/inspectors/flake8/.flake8 b/src/python/review/inspectors/flake8/.flake8 index 0ad30a62..2cd204ff 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -41,3 +41,11 @@ ignore=W291, # trailing whitespaces 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 + 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" diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index 5199b7f8..02bf45c8 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -99,6 +99,7 @@ '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 diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index f844011d..53c14cc1 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -29,6 +29,7 @@ ('case21_imports.py', 2), ('case25_django.py', 0), ('case31_line_break.py', 11), + ('case32_string_format.py', 34), ] @@ -68,6 +69,7 @@ def test_file_with_issues(file_name: str, n_issues: int): ('case31_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)), ] From d142938d8454a5a6e5f94bc647c9880460ccf637 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Fri, 26 Mar 2021 12:23:29 +0300 Subject: [PATCH 12/37] Added tests for flake8-string-format --- .../inspectors/python/case32_string_format.py | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 test/resources/inspectors/python/case32_string_format.py 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)) From cff07760d6c6ade90ce794ffdc56d0d91a5ba973 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 27 Mar 2021 15:12:13 +0300 Subject: [PATCH 13/37] Added support for flake8-commas --- requirements.txt | 1 + src/python/review/inspectors/flake8/.flake8 | 2 ++ src/python/review/inspectors/flake8/issue_types.py | 8 ++++++++ 3 files changed, 11 insertions(+) diff --git a/requirements.txt b/requirements.txt index 8d913520..79d33da9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,6 +20,7 @@ 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 # extra libraries and frameworks django==3.0.8 diff --git a/src/python/review/inspectors/flake8/.flake8 b/src/python/review/inspectors/flake8/.flake8 index 2cd204ff..cc3e69cb 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -49,3 +49,5 @@ ignore=W291, # trailing whitespaces 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/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index 02bf45c8..e55f2d00 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -19,6 +19,14 @@ # 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, + # 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. From a5a6c82b950b419f93f064d5621c99a1f1a356dc Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 27 Mar 2021 15:12:24 +0300 Subject: [PATCH 14/37] Added tests for flake8-commas --- .../inspectors/test_flake8_inspector.py | 2 + .../inspectors/python/case33_commas.py | 153 ++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 test/resources/inspectors/python/case33_commas.py diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 53c14cc1..147995c1 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -30,6 +30,7 @@ ('case25_django.py', 0), ('case31_line_break.py', 11), ('case32_string_format.py', 34), + ('case33_commas.py', 14), ] @@ -70,6 +71,7 @@ def test_file_with_issues(file_name: str, n_issues: int): 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)), ] 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) From 0c88f537018346897739cbcd180d330f29ce61c0 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 27 Mar 2021 15:12:36 +0300 Subject: [PATCH 15/37] Added multiline --- whitelist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/whitelist.txt b/whitelist.txt index 8f29cf65..aa40d349 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -61,3 +61,4 @@ lcom noc nom wmc +multiline From 323f748055d4238c5232607fedf7572792357366 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Sat, 27 Mar 2021 15:14:58 +0300 Subject: [PATCH 16/37] Added C812 to ignore --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f1bbf186..83804e87 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,7 @@ jobs: # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,WPS --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,WPS,C812 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - name: Set up Eslint run: | npm install eslint --save-dev From 4287cf089d17e350c93eae790a8382fd429e77fe Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Mon, 29 Mar 2021 15:55:54 +0300 Subject: [PATCH 17/37] Added support for cohesion --- requirements.txt | 1 + src/python/review/inspectors/flake8/flake8.py | 41 ++++++++++--- src/python/review/inspectors/issue.py | 1 + src/python/review/quality/evaluate_quality.py | 7 ++- .../review/quality/rules/cohesion_scoring.py | 60 +++++++++++++++++++ .../review/reviewers/utils/code_statistics.py | 8 ++- .../review/reviewers/utils/issues_filter.py | 6 +- 7 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 src/python/review/quality/rules/cohesion_scoring.py diff --git a/requirements.txt b/requirements.txt index 79d33da9..e13a08f4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,6 +21,7 @@ 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 # extra libraries and frameworks django==3.0.8 diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 92a82a99..98e66f22 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -1,4 +1,5 @@ import logging +import math import re from pathlib import Path from typing import List @@ -11,7 +12,14 @@ WPS_RANGE_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, + CyclomaticComplexityIssue, + IssueType, + IssueData, + CohesionIssue, +) from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip logger = logging.getLogger(__name__) @@ -31,6 +39,7 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: f'--format={FORMAT}', f'--config={PATH_FLAKE8_CONFIG}', '--max-complexity', '0', + '--cohesion-below', '100', path, ] output = run_in_subprocess(command) @@ -40,27 +49,34 @@ 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]) issue_data = IssueData.get_base_issue_data_dict(file_path, cls.inspector_type, line_number=line_no, column_number=int(groups[2]) if int(groups[2]) > 0 else 1, 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] = cls.__get_cohesion_lack(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 @@ -88,3 +104,14 @@ def choose_issue_type(code: str) -> IssueType: return IssueType.BEST_PRACTICES return issue_type + + @staticmethod + def __get_cohesion_lack(cohesion_percentage: float) -> int: + """ + Converts cohesion percentage to lack of cohesion. + Calculated by the formula: floor(100 - cohesion_percentage). + + :param cohesion_percentage: cohesion set as a percentage. + :return: lack of cohesion + """ + return math.floor(100 - cohesion_percentage) diff --git a/src/python/review/inspectors/issue.py b/src/python/review/inspectors/issue.py index d9d41659..22e4043a 100644 --- a/src/python/review/inspectors/issue.py +++ b/src/python/review/inspectors/issue.py @@ -43,6 +43,7 @@ class IssueData(Enum): FUNCTION_LEN = 'func_len' BOOL_EXPR_LEN = 'bool_expr_len' CYCLOMATIC_COMPLEXITY = 'cc_value' + COHESION_LACK = 'cohesion_lack' @classmethod def get_base_issue_data_dict(cls, file_path: Union[str, Path], inspector_type: InspectorType, line_number: int = 1, diff --git a/src/python/review/quality/evaluate_quality.py b/src/python/review/quality/evaluate_quality.py index 0f301db0..d7ee0211 100644 --- a/src/python/review/quality/evaluate_quality.py +++ b/src/python/review/quality/evaluate_quality.py @@ -37,6 +37,10 @@ WeightedMethodsRule, ) from src.python.review.reviewers.utils.code_statistics import CodeStatistics +from src.python.review.quality.rules.cohesion_scoring import ( + LANGUAGE_TO_COHESION_RULE_CONFIG, + CohesionRule, +) def __get_available_rules(language: Language) -> List[Rule]: @@ -50,7 +54,8 @@ 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]), ] 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..654a5ce6 --- /dev/null +++ b/src/python/review/quality/rules/cohesion_scoring.py @@ -0,0 +1,60 @@ +from dataclasses import dataclass +from typing import Optional + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.quality.model import Rule, QualityType + + +@dataclass +class CohesionRuleConfig: + cohesion_lack_bad: int + cohesion_lack_moderate: int + + +common_cohesion_rule_config = CohesionRuleConfig( + cohesion_lack_bad=50, + cohesion_lack_moderate=30 +) + +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 + 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 + 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), + ) + result_rule = CohesionRule(config) + result_rule.apply(max(self.cohesion_lack, other.cohesion_lack)) + + return result_rule diff --git a/src/python/review/reviewers/utils/code_statistics.py b/src/python/review/reviewers/utils/code_statistics.py index b0523355..41dc25a2 100644 --- a/src/python/review/reviewers/utils/code_statistics.py +++ b/src/python/review/reviewers/utils/code_statistics.py @@ -16,6 +16,7 @@ class CodeStatistics: method_number: int max_cyclomatic_complexity: int + max_cohesion_lack: int max_func_len: int max_bool_expr_len: int @@ -38,6 +39,7 @@ 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.FUNC_LEN: self.max_func_len, IssueType.BOOL_EXPR_LEN: self.max_bool_expr_len, @@ -45,7 +47,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, } @@ -82,6 +84,7 @@ 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) # 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 +100,7 @@ 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_cyclomatic_complexity=cyclomatic_complexities, inheritance_depth=inheritance_depths, class_response=class_responses, @@ -104,5 +108,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..83960d92 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -11,6 +11,7 @@ from src.python.review.quality.rules.inheritance_depth_scoring import LANGUAGE_TO_INHERITANCE_DEPTH_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 +from src.python.review.quality.rules.cohesion_scoring import LANGUAGE_TO_COHESION_RULE_CONFIG def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, int]: @@ -22,7 +23,8 @@ 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_bad, } @@ -38,7 +40,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, From afaf0dd3dc1db8b3d74a8ae2b9e33b2d803cafda Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Mon, 29 Mar 2021 15:56:01 +0300 Subject: [PATCH 18/37] Fixed tests --- test/python/inspectors/conftest.py | 4 +++- test/python/inspectors/test_flake8_inspector.py | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/test/python/inspectors/conftest.py b/test/python/inspectors/conftest.py index 615f95dc..b2f8773c 100644 --- a/test/python/inspectors/conftest.py +++ b/test/python/inspectors/conftest.py @@ -73,6 +73,7 @@ class IssuesTestInfo: n_cc: int = 0 n_bool_expr_len: int = 0 n_other_complexity: int = 0 + n_cohesion: int = 0 def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: @@ -85,7 +86,8 @@ 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], ) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 147995c1..433eaef1 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -12,11 +12,11 @@ ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 8), ('case3_redefining_builtin.py', 2), - ('case4_naming.py', 10), + ('case4_naming.py', 11), ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), - ('case7_empty_lines.py', 0), + ('case7_empty_lines.py', 1), ('case10_unused_variable_in_loop.py', 1), ('case13_complex_logic.py', 12), ('case13_complex_logic_2.py', 3), @@ -55,11 +55,11 @@ def test_file_with_issues(file_name: str, n_issues: int): 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_best_practices=3, n_cc=5)), + ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=3, 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=5)), + ('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=5, n_other_complexity=10)), From 7b79c6b0919e72de9eb4e5d3c4b4dc6edcf9dc00 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Mon, 29 Mar 2021 16:07:08 +0300 Subject: [PATCH 19/37] Added H601 (cohesion) to ignore --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 83804e87..276420bf 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,7 @@ jobs: # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,WPS,C812 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,WPS,C812,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - name: Set up Eslint run: | npm install eslint --save-dev From 0d2a9cbca4703119a01b8b080a75e1c24f3c9b6a Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 30 Mar 2021 11:34:21 +0300 Subject: [PATCH 20/37] Added test for cohesion --- .../inspectors/test_flake8_inspector.py | 2 ++ .../inspectors/python/case34_cohesion.py | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 test/resources/inspectors/python/case34_cohesion.py diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 433eaef1..b8adb40d 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -31,6 +31,7 @@ ('case31_line_break.py', 11), ('case32_string_format.py', 34), ('case33_commas.py', 14), + ('case34_cohesion.py', 1), ] @@ -72,6 +73,7 @@ def test_file_with_issues(file_name: str, n_issues: int): 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)), ] 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 From 923c18af68bef287b0781bed9711e2a346b9ada2 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 30 Mar 2021 11:34:34 +0300 Subject: [PATCH 21/37] Added sqrt --- whitelist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/whitelist.txt b/whitelist.txt index aa40d349..c2473e99 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -62,3 +62,4 @@ noc nom wmc multiline +sqrt From 43d519315b94a04d13c8fbfde7e39b9d0d8cf28e Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 3 Apr 2021 15:50:46 +0300 Subject: [PATCH 22/37] Added Radon inspector and maintainability index metric --- README.md | 7 +- requirements.txt | 1 + .../review/inspectors/inspector_type.py | 2 + src/python/review/inspectors/issue.py | 11 +++ src/python/review/inspectors/radon/radon.py | 75 +++++++++++++++++++ src/python/review/quality/evaluate_quality.py | 5 ++ .../quality/rules/maintainability_scoring.py | 68 +++++++++++++++++ src/python/review/reviewers/common.py | 2 + .../review/reviewers/utils/code_statistics.py | 4 + .../review/reviewers/utils/issues_filter.py | 4 + test/python/inspectors/conftest.py | 2 + 11 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 src/python/review/inspectors/radon/radon.py create mode 100644 src/python/review/quality/rules/maintainability_scoring.py diff --git a/README.md b/README.md index 84a74445..43370839 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,10 @@ Python language: - [x] Pylint [GNU LGPL v2] * [Site and docs](https://www.pylint.org/) * [Repository](https://github.com/PyCQA/pylint) - + +- [x] Radon [MIT] + * [Site and docs](https://radon.readthedocs.io/en/latest/) + * [Repository](https://github.com/rubik/radon) Java language: @@ -92,7 +95,7 @@ Argument | Description --- | --- **‑h**, **‑‑help** | show the help message and exit. **‑v**, **‑‑verbosity** | choose logging level according [this](https://docs.python.org/3/library/logging.html#levels) list: `1` - **ERROR**; `2` - **INFO**; `3` - **DEBUG**; `0` - disable logging (**CRITICAL** value); default value is `0` (**CRITICAL**). -**‑d**, **‑‑disable** | disable inspectors. Available values: for **Python** language: `pylint` for [Pylint](https://github.com/PyCQA/pylint), `flake8` for [flake8](https://flake8.pycqa.org/en/latest/), `python_ast` to check different measures providing by AST; for **Java** language: `checkstyle` for the [Checkstyle](https://checkstyle.sourceforge.io/), `pmd` for [PMD](https://pmd.github.io/); for `Kotlin` language: detekt for [Detekt](https://detekt.github.io/detekt/); for **JavaScript** language: `eslint` for [ESlint](https://eslint.org/). Example: `-d pylint,flake8`. +**‑d**, **‑‑disable** | disable inspectors. Available values: for **Python** language: `pylint` for [Pylint](https://github.com/PyCQA/pylint), `flake8` for [flake8](https://flake8.pycqa.org/en/latest/), `radon` for [Radon](https://radon.readthedocs.io/en/latest/), `python_ast` to check different measures providing by AST; for **Java** language: `checkstyle` for the [Checkstyle](https://checkstyle.sourceforge.io/), `pmd` for [PMD](https://pmd.github.io/); for `Kotlin` language: detekt for [Detekt](https://detekt.github.io/detekt/); for **JavaScript** language: `eslint` for [ESlint](https://eslint.org/). Example: `-d pylint,flake8`. **‑‑allow-duplicates** | allow duplicate issues found by different linters. By default, duplicates are skipped. **‑‑language-version**, **‑‑language_version** | specify the language version for JAVA inspectors. Available values: `java7`, `java8`, `java9`, `java11`. **Note**: **‑‑language_version** is deprecated. Will be deleted in the future. **‑‑n-cpu**, **‑‑n_cpu** | specify number of _cpu_ that can be used to run inspectors. **Note**: **‑‑n_cpu** is deprecated. Will be deleted in the future. diff --git a/requirements.txt b/requirements.txt index e13a08f4..60409ee1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,6 +22,7 @@ flake8-broken-line==0.3.0 flake8-string-format==0.3.0 flake8-commas==2.0.0 cohesion==1.0.0 +radon==4.5.0 # extra libraries and frameworks django==3.0.8 diff --git a/src/python/review/inspectors/inspector_type.py b/src/python/review/inspectors/inspector_type.py index bac74e01..c2d7b0f1 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]: InspectorType.PYLINT.value, InspectorType.FLAKE8.value, InspectorType.PYTHON_AST.value, + InspectorType.RADON.value, # Java language InspectorType.PMD.value, diff --git a/src/python/review/inspectors/issue.py b/src/python/review/inspectors/issue.py index 22e4043a..ab8dca9e 100644 --- a/src/python/review/inspectors/issue.py +++ b/src/python/review/inspectors/issue.py @@ -24,6 +24,7 @@ class IssueType(Enum): COHESION = 'COHESION' CLASS_RESPONSE = 'CLASS_RESPONSE' METHOD_NUMBER = 'METHOD_NUMBER' + MAINTAINABILITY = 'MAINTAINABILITY' # Keys in results dictionary @@ -44,6 +45,7 @@ class IssueData(Enum): BOOL_EXPR_LEN = 'bool_expr_len' CYCLOMATIC_COMPLEXITY = 'cc_value' COHESION_LACK = 'cohesion_lack' + MAINTAINABILITY_LACK = 'maintainability_lack' @classmethod def get_base_issue_data_dict(cls, file_path: Union[str, Path], inspector_type: InspectorType, line_number: int = 1, @@ -178,3 +180,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/radon/radon.py b/src/python/review/inspectors/radon/radon.py new file mode 100644 index 00000000..bdaa3ff2 --- /dev/null +++ b/src/python/review/inspectors/radon/radon.py @@ -0,0 +1,75 @@ +import re +from math import floor +from pathlib import Path +from typing import List + +from src.python.review.common.subprocess_runner import run_in_subprocess +from src.python.review.inspectors.base_inspector import BaseInspector +from src.python.review.inspectors.inspector_type import InspectorType +from src.python.review.inspectors.issue import BaseIssue, IssueData, IssueType, MaintainabilityLackIssue + + +class RadonInspector(BaseInspector): + inspector_type = InspectorType.RADON + + @classmethod + def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: + mi_command = [ + "radon", "mi", + "--max", "F", + "--show", + path + ] + + hal_command = [ + "radon", "hal", + path + ] + + mi_output = run_in_subprocess(mi_command) + mi_issues = cls.mi_parse(mi_output) + + hal_output = run_in_subprocess(hal_command) + hal_issues = cls.hal_parse(hal_output) + + return mi_issues + hal_issues + + @classmethod + def mi_parse(cls, mi_output: str) -> List[BaseIssue]: + """ + Parses the results of 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 = cls.__get_maintainability_lack(float(groups[1])) + + issue_data = IssueData.get_base_issue_data_dict(file_path, cls.inspector_type) + issue_data[IssueData.DESCRIPTION.value] = "" # TODO: add tip + issue_data[IssueData.MAINTAINABILITY_LACK.value] = maintainability_lack + issue_data[IssueData.ISSUE_TYPE.value] = IssueType.MAINTAINABILITY + + issues.append(MaintainabilityLackIssue(**issue_data)) + + return issues + + @staticmethod + def __get_maintainability_lack(maintainability_index: float) -> int: + """ + Converts maintainability index to lack of maintainability. + Calculated by the formula: floor(100 - maintainability_index). + + :param maintainability_index: value in the range from 0 to 100. + :return: lack of maintainability. + """ + return floor(100 - maintainability_index) + + # TODO: add Halstead complexity + @classmethod + def hal_parse(cls, hal_output: str) -> List[BaseIssue]: + return [] diff --git a/src/python/review/quality/evaluate_quality.py b/src/python/review/quality/evaluate_quality.py index d7ee0211..3861d3b3 100644 --- a/src/python/review/quality/evaluate_quality.py +++ b/src/python/review/quality/evaluate_quality.py @@ -41,6 +41,10 @@ LANGUAGE_TO_COHESION_RULE_CONFIG, CohesionRule, ) +from src.python.review.quality.rules.maintainability_scoring import ( + LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG, + MaintainabilityRule, +) def __get_available_rules(language: Language) -> List[Rule]: @@ -56,6 +60,7 @@ def __get_available_rules(language: Language) -> List[Rule]: ResponseRule(LANGUAGE_TO_RESPONSE_RULE_CONFIG[language]), WeightedMethodsRule(LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language]), CohesionRule(LANGUAGE_TO_COHESION_RULE_CONFIG[language]), + MaintainabilityRule(LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[language]), ] diff --git a/src/python/review/quality/rules/maintainability_scoring.py b/src/python/review/quality/rules/maintainability_scoring.py new file mode 100644 index 00000000..72bccb05 --- /dev/null +++ b/src/python/review/quality/rules/maintainability_scoring.py @@ -0,0 +1,68 @@ +from dataclasses import dataclass +from typing import Optional + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.quality.model import Rule, QualityType + + +@dataclass +class MaintainabilityRuleConfig: + maintainability_lack_good: int + maintainability_lack_moderate: int + maintainability_lack_bad: int + + +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/reviewers/common.py b/src/python/review/reviewers/common.py index 0d6688c4..17d54afb 100644 --- a/src/python/review/reviewers/common.py +++ b/src/python/review/reviewers/common.py @@ -8,6 +8,7 @@ from src.python.review.inspectors.detekt.detekt import DetektInspector from src.python.review.inspectors.eslint.eslint import ESLintInspector from src.python.review.inspectors.flake8.flake8 import Flake8Inspector +from src.python.review.inspectors.radon.radon import RadonInspector from src.python.review.inspectors.issue import BaseIssue from src.python.review.inspectors.pmd.pmd import PMDInspector from src.python.review.inspectors.pyast.python_ast import PythonAstInspector @@ -24,6 +25,7 @@ PylintInspector(), Flake8Inspector(), PythonAstInspector(), + RadonInspector(), ], Language.JAVA: [ CheckstyleInspector(), diff --git a/src/python/review/reviewers/utils/code_statistics.py b/src/python/review/reviewers/utils/code_statistics.py index 41dc25a2..3c218f74 100644 --- a/src/python/review/reviewers/utils/code_statistics.py +++ b/src/python/review/reviewers/utils/code_statistics.py @@ -17,6 +17,7 @@ class CodeStatistics: max_cyclomatic_complexity: int max_cohesion_lack: int + max_maintainability_lack: int max_func_len: int max_bool_expr_len: int @@ -40,6 +41,7 @@ def issue_type_to_statistics_dict(self) -> Dict[IssueType, int]: IssueType.CYCLOMATIC_COMPLEXITY: self.max_cyclomatic_complexity, IssueType.COHESION: self.max_cohesion_lack, + IssueType.MAINTAINABILITY: self.max_maintainability_lack, IssueType.FUNC_LEN: self.max_func_len, IssueType.BOOL_EXPR_LEN: self.max_bool_expr_len, @@ -85,6 +87,7 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic func_lens = __get_max_measure_by_issue_type(IssueType.FUNC_LEN, issues) cyclomatic_complexities = __get_max_measure_by_issue_type(IssueType.CYCLOMATIC_COMPLEXITY, issues) cohesion_lacks = __get_max_measure_by_issue_type(IssueType.COHESION, issues) + maintainabilities = __get_max_measure_by_issue_type(IssueType.MAINTAINABILITY, issues) # Actually, we expect only one issue with each of the following metrics. inheritance_depths = __get_max_measure_by_issue_type(IssueType.INHERITANCE_DEPTH, issues) @@ -101,6 +104,7 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic max_func_len=func_lens, n_line_len=issue_type_counter[IssueType.LINE_LEN], max_cohesion_lack=cohesion_lacks, + max_maintainability_lack=maintainabilities, max_cyclomatic_complexity=cyclomatic_complexities, inheritance_depth=inheritance_depths, class_response=class_responses, diff --git a/src/python/review/reviewers/utils/issues_filter.py b/src/python/review/reviewers/utils/issues_filter.py index 83960d92..5c96d97d 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -12,6 +12,7 @@ from src.python.review.quality.rules.method_number_scoring import LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG from src.python.review.quality.rules.weighted_methods_scoring import LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG from src.python.review.quality.rules.cohesion_scoring import LANGUAGE_TO_COHESION_RULE_CONFIG +from src.python.review.quality.rules.maintainability_scoring import LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, int]: @@ -25,6 +26,9 @@ def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, IssueType.CLASS_RESPONSE: LANGUAGE_TO_RESPONSE_RULE_CONFIG[language].response_good, IssueType.WEIGHTED_METHOD: LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language].weighted_methods_good, IssueType.COHESION: LANGUAGE_TO_COHESION_RULE_CONFIG[language].cohesion_lack_bad, + IssueType.MAINTAINABILITY: LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[ + language + ].maintainability_lack_good } diff --git a/test/python/inspectors/conftest.py b/test/python/inspectors/conftest.py index b2f8773c..502c946b 100644 --- a/test/python/inspectors/conftest.py +++ b/test/python/inspectors/conftest.py @@ -74,6 +74,7 @@ class IssuesTestInfo: n_bool_expr_len: int = 0 n_other_complexity: int = 0 n_cohesion: int = 0 + n_maintainability: int = 0 def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: @@ -88,6 +89,7 @@ def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: n_bool_expr_len=counter[IssueType.BOOL_EXPR_LEN], n_other_complexity=counter[IssueType.COMPLEXITY], n_cohesion=counter[IssueType.COHESION], + n_maintainability=counter[IssueType.MAINTAINABILITY] ) From 47cb02ecba890e34f60da77321361b4b413a51d2 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 3 Apr 2021 15:51:37 +0300 Subject: [PATCH 23/37] Added tests for Radon inspector and maintainability metric --- .../review/inspectors/radon/__init__.py | 0 .../python/inspectors/test_radon_inspector.py | 51 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 src/python/review/inspectors/radon/__init__.py create mode 100644 test/python/inspectors/test_radon_inspector.py diff --git a/src/python/review/inspectors/radon/__init__.py b/src/python/review/inspectors/radon/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test/python/inspectors/test_radon_inspector.py b/test/python/inspectors/test_radon_inspector.py new file mode 100644 index 00000000..2d7f18f7 --- /dev/null +++ b/test/python/inspectors/test_radon_inspector.py @@ -0,0 +1,51 @@ +from textwrap import dedent + +import pytest + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.inspectors.radon.radon import RadonInspector +from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues +from test.python.inspectors import PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata + +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] == ["", "", ""] + assert [issue.type for issue in issues] == [ + IssueType.MAINTAINABILITY, + IssueType.MAINTAINABILITY, + IssueType.MAINTAINABILITY, + ] + assert [issue.maintainability_lack for issue in issues] == [95, 86, 30] From f2d337732373ecdf3a92b1b0f37cabb4e2ed779e Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 6 Apr 2021 14:39:44 +0300 Subject: [PATCH 24/37] Remove Halstead complexity placeholders --- src/python/review/inspectors/radon/radon.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py index bdaa3ff2..2ea32c47 100644 --- a/src/python/review/inspectors/radon/radon.py +++ b/src/python/review/inspectors/radon/radon.py @@ -21,18 +21,8 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: path ] - hal_command = [ - "radon", "hal", - path - ] - mi_output = run_in_subprocess(mi_command) - mi_issues = cls.mi_parse(mi_output) - - hal_output = run_in_subprocess(hal_command) - hal_issues = cls.hal_parse(hal_output) - - return mi_issues + hal_issues + return cls.mi_parse(mi_output) @classmethod def mi_parse(cls, mi_output: str) -> List[BaseIssue]: @@ -68,8 +58,3 @@ def __get_maintainability_lack(maintainability_index: float) -> int: :return: lack of maintainability. """ return floor(100 - maintainability_index) - - # TODO: add Halstead complexity - @classmethod - def hal_parse(cls, hal_output: str) -> List[BaseIssue]: - return [] From 5a7378fdda08c088ed3f3cab83b3f69a46729a6b Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Tue, 6 Apr 2021 14:40:53 +0300 Subject: [PATCH 25/37] Add comments --- src/python/review/quality/rules/maintainability_scoring.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/python/review/quality/rules/maintainability_scoring.py b/src/python/review/quality/rules/maintainability_scoring.py index 72bccb05..dfc0fd0d 100644 --- a/src/python/review/quality/rules/maintainability_scoring.py +++ b/src/python/review/quality/rules/maintainability_scoring.py @@ -13,6 +13,12 @@ class MaintainabilityRuleConfig: 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, From ffd212e3d97e4fc3a6c29c5b9295b1594f249538 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Tue, 6 Apr 2021 15:19:08 +0300 Subject: [PATCH 26/37] Fixed F401 --- src/python/review/inspectors/flake8/flake8.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index e02d3648..c24fd7eb 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -9,7 +9,6 @@ from src.python.review.inspectors.flake8.issue_types import ( CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE, - WPS_RANGE_TO_ISSUE_TYPE, ) from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.inspectors.issue import ( From 528599aeabf83d5ca15b923eb53635725713ef73 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Tue, 6 Apr 2021 15:19:56 +0300 Subject: [PATCH 27/37] Small import refactoring --- src/python/review/inspectors/flake8/flake8.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index c24fd7eb..c7318d0b 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -6,10 +6,7 @@ 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.flake8.issue_types import ( - CODE_PREFIX_TO_ISSUE_TYPE, - CODE_TO_ISSUE_TYPE, -) +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, From 95565464c9b0eca742ed9dba6e325b8617785c46 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Tue, 6 Apr 2021 15:21:43 +0300 Subject: [PATCH 28/37] Fixed W293 --- src/python/review/inspectors/inspector_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/review/inspectors/inspector_type.py b/src/python/review/inspectors/inspector_type.py index 51694e1f..15482a8b 100644 --- a/src/python/review/inspectors/inspector_type.py +++ b/src/python/review/inspectors/inspector_type.py @@ -31,7 +31,7 @@ def available_values(cls) -> List[str]: cls.FLAKE8.value, cls.PYTHON_AST.value, cls.RADON.value, - + # Java language cls.PMD.value, cls.CHECKSTYLE.value, From ffbcf4c43787e7299f815e96b0cbae101a7adc58 Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Thu, 8 Apr 2021 16:16:45 +0500 Subject: [PATCH 29/37] Trailing commas fix (#20) Fix trailing commas --- .github/workflows/build.yml | 2 +- setup.py | 12 ++++---- src/python/review/common/file_system.py | 2 +- src/python/review/common/java_compiler.py | 2 +- src/python/review/common/parallel_runner.py | 2 +- src/python/review/common/subprocess_runner.py | 2 +- .../inspectors/checkstyle/checkstyle.py | 4 +-- src/python/review/inspectors/detekt/detekt.py | 4 +-- src/python/review/inspectors/eslint/eslint.py | 2 +- .../review/inspectors/intellij/intellij.py | 8 +++--- .../inspectors/intellij/issue_types/kotlin.py | 2 +- src/python/review/inspectors/issue.py | 2 +- src/python/review/inspectors/pmd/pmd.py | 2 +- .../review/inspectors/pyast/python_ast.py | 14 +++++----- src/python/review/inspectors/pylint/pylint.py | 6 ++-- src/python/review/inspectors/radon/radon.py | 2 +- .../review/inspectors/spotbugs/spotbugs.py | 4 +-- .../inspectors/springlint/springlint.py | 10 +++---- src/python/review/logging_config.py | 12 ++++---- src/python/review/quality/model.py | 13 ++++----- .../quality/rules/best_practices_scoring.py | 4 +-- .../quality/rules/boolean_length_scoring.py | 6 ++-- .../quality/rules/class_response_scoring.py | 4 +-- .../quality/rules/code_style_scoring.py | 8 +++--- .../review/quality/rules/coupling_scoring.py | 4 +-- .../rules/cyclomatic_complexity_scoring.py | 10 +++---- .../quality/rules/error_prone_scoring.py | 2 +- .../quality/rules/function_length_scoring.py | 12 ++++---- .../rules/inheritance_depth_scoring.py | 2 +- .../review/quality/rules/line_len_scoring.py | 4 +-- .../quality/rules/maintainability_scoring.py | 2 +- .../quality/rules/method_number_scoring.py | 4 +-- .../quality/rules/weighted_methods_scoring.py | 4 +-- src/python/review/reviewers/common.py | 6 ++-- src/python/review/reviewers/perform_review.py | 4 +-- .../review/reviewers/utils/code_statistics.py | 2 +- .../review/reviewers/utils/issues_filter.py | 2 +- .../review/reviewers/utils/print_review.py | 6 ++-- src/python/review/run_tool.py | 2 +- test/python/functional_tests/conftest.py | 2 +- .../test_different_languages.py | 8 +++--- test/python/functional_tests/test_disable.py | 4 +-- .../functional_tests/test_duplicates.py | 4 +-- .../python/functional_tests/test_exit_code.py | 6 ++-- .../functional_tests/test_file_or_project.py | 4 +-- .../test_multi_file_project.py | 26 ++++++++--------- .../functional_tests/test_range_of_lines.py | 28 +++++++++---------- .../test_single_file_json_format.py | 16 +++++------ .../python/functional_tests/test_verbosity.py | 6 ++-- test/python/inspectors/conftest.py | 20 ++++++------- .../inspectors/test_flake8_inspector.py | 2 +- test/python/inspectors/test_local_review.py | 4 +-- .../inspectors/test_out_of_range_issues.py | 2 +- .../inspectors/test_pylint_inspector.py | 2 +- 54 files changed, 164 insertions(+), 165 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 50706799..4cc2a22b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,7 @@ jobs: # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,W503,WPS,C812,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402,W503,WPS,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - name: Set up Eslint run: | npm install eslint --save-dev diff --git a/setup.py b/setup.py index 12428183..ab75f040 100644 --- a/setup.py +++ b/setup.py @@ -45,22 +45,22 @@ def get_inspectors_additional_files() -> List[str]: 'Topic :: Software Development :: Build Tools', 'License :: OSI Approved :: MIT License', 'Programming Language :: Python :: 3', - 'Operating System :: OS Independent' + 'Operating System :: OS Independent', ], keywords='code review', python_requires='>=3.8, <4', install_requires=['upsourceapi'], packages=find_packages(exclude=[ '*.unit_tests', '*.unit_tests.*', 'unit_tests.*', 'unit_tests', - '*.functional_tests', '*.functional_tests.*', 'functional_tests.*', 'functional_tests' + '*.functional_tests', '*.functional_tests.*', 'functional_tests.*', 'functional_tests', ]), zip_safe=False, package_data={ - '': get_inspectors_additional_files() + '': get_inspectors_additional_files(), }, entry_points={ 'console_scripts': [ - 'review=review.run_tool:main' - ] - } + 'review=review.run_tool:main', + ], + }, ) diff --git a/src/python/review/common/file_system.py b/src/python/review/common/file_system.py index 6decb410..3faa962c 100644 --- a/src/python/review/common/file_system.py +++ b/src/python/review/common/file_system.py @@ -77,7 +77,7 @@ def create_directory(directory: str) -> None: def get_file_line(path: Path, line_number: int): return linecache.getline( str(path), - line_number + line_number, ).strip() diff --git a/src/python/review/common/java_compiler.py b/src/python/review/common/java_compiler.py index aee76032..e6b97861 100644 --- a/src/python/review/common/java_compiler.py +++ b/src/python/review/common/java_compiler.py @@ -18,7 +18,7 @@ def javac(javac_args: Union[str, Path]) -> bool: output_bytes: bytes = subprocess.check_output( f'javac {javac_args}', shell=True, - stderr=subprocess.STDOUT + stderr=subprocess.STDOUT, ) output_str = str(output_bytes, Encoding.UTF_ENCODING.value) diff --git a/src/python/review/common/parallel_runner.py b/src/python/review/common/parallel_runner.py index e9ab7d8d..c0347d23 100644 --- a/src/python/review/common/parallel_runner.py +++ b/src/python/review/common/parallel_runner.py @@ -37,7 +37,7 @@ def inspect_in_parallel(path: Path, with multiprocessing.Pool(config.n_cpu) as pool: issues = pool.map( functools.partial(run_inspector, path, config), - inspectors + inspectors, ) return list(itertools.chain(*issues)) diff --git a/src/python/review/common/subprocess_runner.py b/src/python/review/common/subprocess_runner.py index 3dd5cad3..a25cbdcd 100644 --- a/src/python/review/common/subprocess_runner.py +++ b/src/python/review/common/subprocess_runner.py @@ -9,7 +9,7 @@ def run_in_subprocess(command: List[str]) -> str: process = subprocess.run( command, stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() diff --git a/src/python/review/inspectors/checkstyle/checkstyle.py b/src/python/review/inspectors/checkstyle/checkstyle.py index 389d889f..c796fbf9 100644 --- a/src/python/review/inspectors/checkstyle/checkstyle.py +++ b/src/python/review/inspectors/checkstyle/checkstyle.py @@ -35,7 +35,7 @@ class CheckstyleInspector(BaseInspector): r'Boolean expression complexity is (\d+)', 'LineLengthCheck': - r'Line is longer than \d+ characters \(found (\d+)\)' + r'Line is longer than \d+ characters \(found (\d+)\)', } @classmethod @@ -43,7 +43,7 @@ def _create_command(cls, path: Path, output_path: Path) -> List[str]: return [ 'java', '-jar', PATH_TOOLS_CHECKSTYLE_JAR, '-c', PATH_TOOLS_CHECKSTYLE_CONFIG, - '-f', 'xml', '-o', output_path, str(path) + '-f', 'xml', '-o', output_path, str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/detekt/detekt.py b/src/python/review/inspectors/detekt/detekt.py index 2c2b197e..8f5f0946 100644 --- a/src/python/review/inspectors/detekt/detekt.py +++ b/src/python/review/inspectors/detekt/detekt.py @@ -27,7 +27,7 @@ class DetektInspector(BaseInspector): 'ComplexCondition': r'This condition is too complex \((\d+)\)', 'ComplexMethod': - r'The function .* appears to be too complex \((\d+)\)' + r'The function .* appears to be too complex \((\d+)\)', } @classmethod @@ -38,7 +38,7 @@ def _create_command(cls, path: Path, output_path: Path): '--config', PATH_DETEKT_CONFIG, '--plugins', PATH_DETEKT_PLUGIN, '--report', f'xml:{output_path}', - '--input', str(path) + '--input', str(path), ] def inspect(self, path: Path, config) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/eslint/eslint.py b/src/python/review/inspectors/eslint/eslint.py index ff9f9a67..839a166c 100644 --- a/src/python/review/inspectors/eslint/eslint.py +++ b/src/python/review/inspectors/eslint/eslint.py @@ -17,7 +17,7 @@ class ESLintInspector(BaseInspector): origin_class_to_pattern = { 'complexity': - r'complexity of (\d+)' + r'complexity of (\d+)', } @classmethod diff --git a/src/python/review/inspectors/intellij/intellij.py b/src/python/review/inspectors/intellij/intellij.py index 8b962947..b50dc0ca 100644 --- a/src/python/review/inspectors/intellij/intellij.py +++ b/src/python/review/inspectors/intellij/intellij.py @@ -48,7 +48,7 @@ def __init__(self): def create_command(output_dir_path) -> List[Union[str, Path]]: return [ INTELLIJ_INSPECTOR_EXECUTABLE, INTELLIJ_INSPECTOR_PROJECT, - INTELLIJ_INSPECTOR_SETTINGS, output_dir_path, '-v2' + INTELLIJ_INSPECTOR_SETTINGS, output_dir_path, '-v2', ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -134,8 +134,8 @@ def parse(cls, out_dir_path: Path, file_path = Path( text.replace( 'file://$PROJECT_DIR$', - str(INTELLIJ_INSPECTOR_PROJECT) - ) + str(INTELLIJ_INSPECTOR_PROJECT), + ), ) elif tag == 'line': line_no = int(text) @@ -160,7 +160,7 @@ def parse(cls, out_dir_path: Path, description=description, origin_class=issue_class, inspector_type=cls.inspector_type, - type=issue_type + type=issue_type, )) return issues diff --git a/src/python/review/inspectors/intellij/issue_types/kotlin.py b/src/python/review/inspectors/intellij/issue_types/kotlin.py index 97438dca..cb692510 100644 --- a/src/python/review/inspectors/intellij/issue_types/kotlin.py +++ b/src/python/review/inspectors/intellij/issue_types/kotlin.py @@ -378,5 +378,5 @@ '\'when\' that can be simplified by introducing an argument': IssueType.CODE_STYLE, - 'Annotator': IssueType.ERROR_PRONE + 'Annotator': IssueType.ERROR_PRONE, } diff --git a/src/python/review/inspectors/issue.py b/src/python/review/inspectors/issue.py index b873cb65..c910bf80 100644 --- a/src/python/review/inspectors/issue.py +++ b/src/python/review/inspectors/issue.py @@ -61,7 +61,7 @@ def get_base_issue_data_dict(cls, cls.LINE_NUMBER.value: line_number, cls.COLUMN_NUMBER.value: column_number, cls.ORIGIN_ClASS.value: origin_class, - cls.INSPECTOR_TYPE.value: inspector_type + cls.INSPECTOR_TYPE.value: inspector_type, } diff --git a/src/python/review/inspectors/pmd/pmd.py b/src/python/review/inspectors/pmd/pmd.py index 227169fc..4b8c11f0 100644 --- a/src/python/review/inspectors/pmd/pmd.py +++ b/src/python/review/inspectors/pmd/pmd.py @@ -37,7 +37,7 @@ def _create_command(cls, path: Path, '-language', 'java', '-version', java_version.value, '-f', 'csv', '-r', str(output_path), - '-t', str(n_cpu) + '-t', str(n_cpu), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/pyast/python_ast.py b/src/python/review/inspectors/pyast/python_ast.py index 28b76e9f..6426d115 100644 --- a/src/python/review/inspectors/pyast/python_ast.py +++ b/src/python/review/inspectors/pyast/python_ast.py @@ -40,7 +40,7 @@ def visit(self, node: ast.AST): origin_class=BOOL_EXPR_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, bool_expr_len=length, - type=IssueType.BOOL_EXPR_LEN + type=IssueType.BOOL_EXPR_LEN, )) @@ -58,7 +58,7 @@ def __init__(self, content: str, file_path: Path, inspector_type: InspectorType) def visit(self, node): if isinstance(self._previous_node, (ast.FunctionDef, ast.AsyncFunctionDef)): func_length = self._find_func_len( - self._previous_node.lineno, node.lineno + self._previous_node.lineno, node.lineno, ) self._function_lens.append(FuncLenIssue( @@ -69,7 +69,7 @@ def visit(self, node): origin_class=FUNC_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, func_len=func_length, - type=IssueType.FUNC_LEN + type=IssueType.FUNC_LEN, )) self._previous_node = node @@ -80,7 +80,7 @@ def visit(self, node): def function_lens(self) -> List[FuncLenIssue]: if isinstance(self._previous_node, (ast.FunctionDef, ast.AsyncFunctionDef)): func_length = self._find_func_len( - self._previous_node.lineno, self._n_lines + 1 + self._previous_node.lineno, self._n_lines + 1, ) self._function_lens.append(FuncLenIssue( @@ -91,7 +91,7 @@ def function_lens(self) -> List[FuncLenIssue]: origin_class=FUNC_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, func_len=func_length, - type=IssueType.FUNC_LEN + type=IssueType.FUNC_LEN, )) self._previous_node = None @@ -125,13 +125,13 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: bool_gatherer = BoolExpressionLensGatherer(path_to_file, cls.inspector_type) bool_gatherer.visit(tree) metrics.extend( - bool_gatherer.bool_expression_lens + bool_gatherer.bool_expression_lens, ) func_gatherer = FunctionLensGatherer(file_content, path_to_file, cls.inspector_type) func_gatherer.visit(tree) metrics.extend( - func_gatherer.function_lens + func_gatherer.function_lens, ) return metrics diff --git a/src/python/review/inspectors/pylint/pylint.py b/src/python/review/inspectors/pylint/pylint.py index fb9db581..e5a256cb 100644 --- a/src/python/review/inspectors/pylint/pylint.py +++ b/src/python/review/inspectors/pylint/pylint.py @@ -21,7 +21,7 @@ class PylintInspector(BaseInspector): supported_issue_types = ( IssueType.CODE_STYLE, IssueType.BEST_PRACTICES, - IssueType.ERROR_PRONE + IssueType.ERROR_PRONE, ) @classmethod @@ -31,7 +31,7 @@ def inspect(cls, path: Path, config: dict) -> List[CodeIssue]: '--load-plugins', 'pylint_django', f'--rcfile={PATH_PYLINT_CONFIG}', f'--msg-template={MSG_TEMPLATE}', - str(path) + str(path), ] output = run_in_subprocess(command) @@ -70,7 +70,7 @@ def parse(cls, output: str) -> List[CodeIssue]: origin_class=origin_class, description=description, inspector_type=cls.inspector_type, - type=issue_type + type=issue_type, )) return issues diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py index 2ea32c47..3c2a52e7 100644 --- a/src/python/review/inspectors/radon/radon.py +++ b/src/python/review/inspectors/radon/radon.py @@ -18,7 +18,7 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: "radon", "mi", "--max", "F", "--show", - path + path, ] mi_output = run_in_subprocess(mi_command) diff --git a/src/python/review/inspectors/spotbugs/spotbugs.py b/src/python/review/inspectors/spotbugs/spotbugs.py index e9cd1cb2..a2ee8b38 100644 --- a/src/python/review/inspectors/spotbugs/spotbugs.py +++ b/src/python/review/inspectors/spotbugs/spotbugs.py @@ -30,7 +30,7 @@ def _create_command(cls, path: Path) -> List[str]: PATH_SPOTBUGS_EXCLUDE, '-textui', '-medium', - str(path) + str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -100,5 +100,5 @@ def _parse_single_line(cls, line: str, file_name_to_path: Dict[str, Path]) -> Ba type=IssueType.ERROR_PRONE, origin_class=issue_class, description=short_desc, - inspector_type=cls.inspector_type + inspector_type=cls.inspector_type, ) diff --git a/src/python/review/inspectors/springlint/springlint.py b/src/python/review/inspectors/springlint/springlint.py index 14ce8c89..cc90c487 100644 --- a/src/python/review/inspectors/springlint/springlint.py +++ b/src/python/review/inspectors/springlint/springlint.py @@ -51,7 +51,7 @@ class SpringlintInspector(BaseInspector): 'cbo': 'class_objects_coupling', 'lcom': 'cohesion_lack', 'rfc': 'class_response', - 'nom': 'method_number' + 'nom': 'method_number', } metric_name_to_description = { @@ -61,7 +61,7 @@ class SpringlintInspector(BaseInspector): 'cbo': get_class_coupling_tip(), 'lcom': get_cohesion_tip(), 'rfc': get_class_response_tip(), - 'nom': get_method_number_tip() + 'nom': get_method_number_tip(), } metric_name_to_issue_type = { @@ -71,7 +71,7 @@ class SpringlintInspector(BaseInspector): 'cbo': IssueType.COUPLING, 'lcom': IssueType.COHESION, 'rfc': IssueType.CLASS_RESPONSE, - 'nom': IssueType.METHOD_NUMBER + 'nom': IssueType.METHOD_NUMBER, } @classmethod @@ -81,7 +81,7 @@ def _create_command(cls, path: Path, output_path: Path) -> List[str]: PATH_SPRINGLINT_JAR, '--output', str(output_path), '-otype', 'html', - '--project', str(path) + '--project', str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -137,7 +137,7 @@ def _parse_smells(cls, file_content: AnyStr, origin_path: str = '') -> List[Base origin_class=smell['name'], inspector_type=cls.inspector_type, type=IssueType.ARCHITECTURE, - description=smell['description'] + description=smell['description'], ) for smell in file_smell['smells']]) return issues diff --git a/src/python/review/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/model.py b/src/python/review/quality/model.py index 741c761b..6e64ea81 100644 --- a/src/python/review/quality/model.py +++ b/src/python/review/quality/model.py @@ -20,7 +20,7 @@ def __le__(self, other: 'QualityType') -> bool: QualityType.BAD: 0, QualityType.MODERATE: 1, QualityType.GOOD: 2, - QualityType.EXCELLENT: 3 + QualityType.EXCELLENT: 3, } return order[self] < order[other] @@ -81,12 +81,11 @@ def __str__(self): message_deltas_part = '' if self.quality_type != QualityType.EXCELLENT: - message_next_level_part = textwrap.dedent( - f"""\ - Next level: {self.next_quality_type.value} - Next level requirements: - """ - ) + message_next_level_part = f"""\ + Next level: {self.next_quality_type.value} + Next level requirements: + """ + message_next_level_part = textwrap.dedent(message_next_level_part) for rule in self.next_level_requirements: message_deltas_part += f'{rule.rule_type.value}: {rule.next_level_delta}\n' diff --git a/src/python/review/quality/rules/best_practices_scoring.py b/src/python/review/quality/rules/best_practices_scoring.py index f014d6e8..633387f2 100644 --- a/src/python/review/quality/rules/best_practices_scoring.py +++ b/src/python/review/quality/rules/best_practices_scoring.py @@ -16,7 +16,7 @@ class BestPracticesRuleConfig: common_best_practices_rule_config = BestPracticesRuleConfig( n_best_practices_moderate=3, n_best_practices_good=1, - n_files=1 + n_files=1, ) LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG = { @@ -59,7 +59,7 @@ def merge(self, other: 'BestPracticesRule') -> 'BestPracticesRule': config = BestPracticesRuleConfig( min(self.config.n_best_practices_moderate, other.config.n_best_practices_moderate), min(self.config.n_best_practices_good, other.config.n_best_practices_good), - n_files=self.config.n_files + other.config.n_files + n_files=self.config.n_files + other.config.n_files, ) result_rule = BestPracticesRule(config) result_rule.apply(self.n_best_practices + other.n_best_practices) diff --git a/src/python/review/quality/rules/boolean_length_scoring.py b/src/python/review/quality/rules/boolean_length_scoring.py index b2d7c347..2a33c327 100644 --- a/src/python/review/quality/rules/boolean_length_scoring.py +++ b/src/python/review/quality/rules/boolean_length_scoring.py @@ -16,13 +16,13 @@ class BooleanExpressionRuleConfig: common_boolean_expression_rule_config = BooleanExpressionRuleConfig( bool_expr_len_bad=10, bool_expr_len_moderate=7, - bool_expr_len_good=5 + bool_expr_len_good=5, ) java_boolean_expression_rule_config = BooleanExpressionRuleConfig( bool_expr_len_bad=8, bool_expr_len_moderate=6, - bool_expr_len_good=4 + bool_expr_len_good=4, ) LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG = { @@ -66,7 +66,7 @@ def merge(self, other: 'BooleanExpressionRule') -> 'BooleanExpressionRule': config = BooleanExpressionRuleConfig( min(self.config.bool_expr_len_bad, other.config.bool_expr_len_bad), min(self.config.bool_expr_len_moderate, other.config.bool_expr_len_moderate), - min(self.config.bool_expr_len_good, other.config.bool_expr_len_good) + min(self.config.bool_expr_len_good, other.config.bool_expr_len_good), ) result_rule = BooleanExpressionRule(config) result_rule.apply(max(self.bool_expr_len, other.bool_expr_len)) diff --git a/src/python/review/quality/rules/class_response_scoring.py b/src/python/review/quality/rules/class_response_scoring.py index 4bfd4800..d4b82fa9 100644 --- a/src/python/review/quality/rules/class_response_scoring.py +++ b/src/python/review/quality/rules/class_response_scoring.py @@ -14,7 +14,7 @@ class ResponseRuleConfig: common_response_rule_config = ResponseRuleConfig( response_moderate=69, - response_good=59 + response_good=59, ) LANGUAGE_TO_RESPONSE_RULE_CONFIG = { @@ -52,7 +52,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'ResponseRule') -> 'ResponseRule': config = ResponseRuleConfig( min(self.config.response_moderate, other.config.response_moderate), - min(self.config.response_good, other.config.response_good) + min(self.config.response_good, other.config.response_good), ) result_rule = ResponseRule(config) result_rule.apply(max(self.response, other.response)) diff --git a/src/python/review/quality/rules/code_style_scoring.py b/src/python/review/quality/rules/code_style_scoring.py index 882b0c5c..a1edda09 100644 --- a/src/python/review/quality/rules/code_style_scoring.py +++ b/src/python/review/quality/rules/code_style_scoring.py @@ -19,7 +19,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.JAVA + language=Language.JAVA, ) python_code_style_rule_config = CodeStyleRuleConfig( @@ -27,7 +27,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=5, - language=Language.PYTHON + language=Language.PYTHON, ) kotlin_code_style_rule_config = CodeStyleRuleConfig( @@ -35,7 +35,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.07, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.KOTLIN + language=Language.KOTLIN, ) js_code_style_rule_config = CodeStyleRuleConfig( @@ -43,7 +43,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.JAVA + language=Language.JAVA, ) LANGUAGE_TO_CODE_STYLE_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/coupling_scoring.py b/src/python/review/quality/rules/coupling_scoring.py index 83ea987b..7fb0a782 100644 --- a/src/python/review/quality/rules/coupling_scoring.py +++ b/src/python/review/quality/rules/coupling_scoring.py @@ -14,7 +14,7 @@ class CouplingRuleConfig: common_coupling_rule_config = CouplingRuleConfig( coupling_bad=30, - coupling_moderate=20 + coupling_moderate=20, ) LANGUAGE_TO_COUPLING_RULE_CONFIG = { @@ -52,7 +52,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'CouplingRule') -> 'CouplingRule': config = CouplingRuleConfig( min(self.config.coupling_bad, other.config.coupling_bad), - min(self.config.coupling_moderate, other.config.coupling_moderate) + min(self.config.coupling_moderate, other.config.coupling_moderate), ) result_rule = CouplingRule(config) result_rule.apply(max(self.coupling, other.coupling)) diff --git a/src/python/review/quality/rules/cyclomatic_complexity_scoring.py b/src/python/review/quality/rules/cyclomatic_complexity_scoring.py index 79fb69dc..162284b4 100644 --- a/src/python/review/quality/rules/cyclomatic_complexity_scoring.py +++ b/src/python/review/quality/rules/cyclomatic_complexity_scoring.py @@ -15,20 +15,20 @@ class CyclomaticComplexityRuleConfig: LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG = { Language.JAVA: CyclomaticComplexityRuleConfig( cc_value_bad=14, - cc_value_moderate=13 + cc_value_moderate=13, ), Language.KOTLIN: CyclomaticComplexityRuleConfig( cc_value_bad=12, - cc_value_moderate=11 + cc_value_moderate=11, ), Language.PYTHON: CyclomaticComplexityRuleConfig( cc_value_bad=10, - cc_value_moderate=9 + cc_value_moderate=9, ), Language.JS: CyclomaticComplexityRuleConfig( cc_value_bad=14, - cc_value_moderate=13 - ) + cc_value_moderate=13, + ), } diff --git a/src/python/review/quality/rules/error_prone_scoring.py b/src/python/review/quality/rules/error_prone_scoring.py index 7a6dee0e..8df9c157 100644 --- a/src/python/review/quality/rules/error_prone_scoring.py +++ b/src/python/review/quality/rules/error_prone_scoring.py @@ -12,7 +12,7 @@ class ErrorProneRuleConfig: common_error_prone_rule_config = ErrorProneRuleConfig( - n_error_prone_bad=0 + n_error_prone_bad=0, ) LANGUAGE_TO_ERROR_PRONE_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/function_length_scoring.py b/src/python/review/quality/rules/function_length_scoring.py index f607f679..e728e9c2 100644 --- a/src/python/review/quality/rules/function_length_scoring.py +++ b/src/python/review/quality/rules/function_length_scoring.py @@ -13,17 +13,17 @@ class FunctionLengthRuleConfig: LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG = { Language.JAVA: FunctionLengthRuleConfig( - func_len_bad=69 + func_len_bad=69, ), Language.KOTLIN: FunctionLengthRuleConfig( - func_len_bad=69 + func_len_bad=69, ), Language.PYTHON: FunctionLengthRuleConfig( - func_len_bad=49 + func_len_bad=49, ), Language.JS: FunctionLengthRuleConfig( - func_len_bad=69 - ) + func_len_bad=69, + ), } @@ -48,7 +48,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'FunctionLengthRule') -> 'FunctionLengthRule': config = FunctionLengthRuleConfig( - min(self.config.func_len_bad, other.config.func_len_bad) + min(self.config.func_len_bad, other.config.func_len_bad), ) result_rule = FunctionLengthRule(config) result_rule.apply(max(self.func_len, other.func_len)) diff --git a/src/python/review/quality/rules/inheritance_depth_scoring.py b/src/python/review/quality/rules/inheritance_depth_scoring.py index cdabda6b..a3531afc 100644 --- a/src/python/review/quality/rules/inheritance_depth_scoring.py +++ b/src/python/review/quality/rules/inheritance_depth_scoring.py @@ -12,7 +12,7 @@ class InheritanceDepthRuleConfig: common_inheritance_depth_rule_config = InheritanceDepthRuleConfig( - depth_bad=3 + depth_bad=3, ) LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/line_len_scoring.py b/src/python/review/quality/rules/line_len_scoring.py index d52b0382..b188576f 100644 --- a/src/python/review/quality/rules/line_len_scoring.py +++ b/src/python/review/quality/rules/line_len_scoring.py @@ -13,7 +13,7 @@ class LineLengthRuleConfig: common_line_length_rule_config = LineLengthRuleConfig( n_line_len_bad=0.05, - n_line_len_good=0.035 + n_line_len_good=0.035, ) LANGUAGE_TO_LINE_LENGTH_RULE_CONFIG = { @@ -54,7 +54,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'LineLengthRule') -> 'LineLengthRule': config = LineLengthRuleConfig( min(self.config.n_line_len_bad, other.config.n_line_len_bad), - min(self.config.n_line_len_good, other.config.n_line_len_good) + min(self.config.n_line_len_good, other.config.n_line_len_good), ) result_rule = LineLengthRule(config) result_rule.apply(self.n_line_len + other.n_line_len, self.n_lines + other.n_lines) diff --git a/src/python/review/quality/rules/maintainability_scoring.py b/src/python/review/quality/rules/maintainability_scoring.py index dfc0fd0d..456e7b0a 100644 --- a/src/python/review/quality/rules/maintainability_scoring.py +++ b/src/python/review/quality/rules/maintainability_scoring.py @@ -66,7 +66,7 @@ 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) + 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)) 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 17d54afb..d7385fe5 100644 --- a/src/python/review/reviewers/common.py +++ b/src/python/review/reviewers/common.py @@ -38,7 +38,7 @@ ], Language.JS: [ ESLintInspector(), - ] + ], } @@ -78,12 +78,12 @@ def perform_language_review(metadata: Metadata, file_review_results.append(FileReviewResult( file_metadata.path, issues, - quality + quality, )) return ReviewResult( file_review_results, - general_quality + general_quality, ) diff --git a/src/python/review/reviewers/perform_review.py b/src/python/review/reviewers/perform_review.py index 678e503a..61f9b1a7 100644 --- a/src/python/review/reviewers/perform_review.py +++ b/src/python/review/reviewers/perform_review.py @@ -13,7 +13,7 @@ from src.python.review.reviewers.utils.print_review import ( print_review_result_as_json, print_review_result_as_multi_file_json, - print_review_result_as_text + print_review_result_as_text, ) logger: Final = logging.getLogger(__name__) @@ -31,7 +31,7 @@ class PathNotExists(Exception): Language.PYTHON: perform_python_review, Language.JAVA: partial(perform_language_review, language=Language.JAVA), Language.KOTLIN: partial(perform_language_review, language=Language.KOTLIN), - Language.JS: partial(perform_language_review, language=Language.JS) + Language.JS: partial(perform_language_review, language=Language.JS), } diff --git a/src/python/review/reviewers/utils/code_statistics.py b/src/python/review/reviewers/utils/code_statistics.py index 3c218f74..d8dd7b38 100644 --- a/src/python/review/reviewers/utils/code_statistics.py +++ b/src/python/review/reviewers/utils/code_statistics.py @@ -75,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) diff --git a/src/python/review/reviewers/utils/issues_filter.py b/src/python/review/reviewers/utils/issues_filter.py index af34b427..a952998c 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -28,7 +28,7 @@ def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, IssueType.COHESION: LANGUAGE_TO_COHESION_RULE_CONFIG[language].cohesion_lack_good, IssueType.MAINTAINABILITY: LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[ language - ].maintainability_lack_good + ].maintainability_lack_good, } diff --git a/src/python/review/reviewers/utils/print_review.py b/src/python/review/reviewers/utils/print_review.py index 76c2de04..9da0e35b 100644 --- a/src/python/review/reviewers/utils/print_review.py +++ b/src/python/review/reviewers/utils/print_review.py @@ -24,7 +24,7 @@ def print_review_result_as_text(review_result: ReviewResult, for issue in sorted_issues: line_text = linecache.getline( str(issue.file_path), - issue.line_no + issue.line_no, ).strip() print(f'{issue.line_no} : ' @@ -80,7 +80,7 @@ def print_review_result_as_multi_file_json(review_result: ReviewResult) -> None: 'file_name': str(file_review_result.file_path), 'quality': { 'code': quality_value, - 'text': f'Code quality (beta): {quality_value}' + 'text': f'Code quality (beta): {quality_value}', }, 'issues': [], } @@ -106,7 +106,7 @@ def print_review_result_as_multi_file_json(review_result: ReviewResult) -> None: 'code': quality_value, 'text': f'Code quality (beta): {quality_value}', }, - 'file_review_results': file_review_result_jsons + 'file_review_results': file_review_result_jsons, } print(json.dumps(output_json)) diff --git a/src/python/review/run_tool.py b/src/python/review/run_tool.py index d0ba0891..1b879972 100644 --- a/src/python/review/run_tool.py +++ b/src/python/review/run_tool.py @@ -155,7 +155,7 @@ def main() -> int: inspectors_config = { 'language_version': LanguageVersion(args.language_version) if args.language_version is not None else None, - 'n_cpu': n_cpu + 'n_cpu': n_cpu, } config = ApplicationConfig( diff --git a/test/python/functional_tests/conftest.py b/test/python/functional_tests/conftest.py index cec7b0a0..eea5a9ab 100644 --- a/test/python/functional_tests/conftest.py +++ b/test/python/functional_tests/conftest.py @@ -43,7 +43,7 @@ def build(self) -> List[str]: command.extend([ '--n_cpu', str(self.n_cpu), '-f', self.format, - str(self.path) + str(self.path), ]) if self.start_line is not None: diff --git a/test/python/functional_tests/test_different_languages.py b/test/python/functional_tests/test_different_languages.py index 0cfbf80d..935df678 100644 --- a/test/python/functional_tests/test_different_languages.py +++ b/test/python/functional_tests/test_different_languages.py @@ -10,7 +10,7 @@ def test_python(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -26,7 +26,7 @@ def test_java(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -42,7 +42,7 @@ def test_kotlin(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -59,7 +59,7 @@ def test_all_java_inspectors(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_disable.py b/test/python/functional_tests/test_disable.py index 08655c1e..ffacbec1 100644 --- a/test/python/functional_tests/test_disable.py +++ b/test/python/functional_tests/test_disable.py @@ -10,7 +10,7 @@ def test_disable_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -20,7 +20,7 @@ def test_disable_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_duplicates.py b/test/python/functional_tests/test_duplicates.py index 13ff2a83..153a3ac7 100644 --- a/test/python/functional_tests/test_duplicates.py +++ b/test/python/functional_tests/test_duplicates.py @@ -13,7 +13,7 @@ def test_allow_duplicates(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout_allow_duplicates = process.stdout.decode() @@ -22,7 +22,7 @@ def test_allow_duplicates(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout_filter_duplicates = process.stdout.decode() diff --git a/test/python/functional_tests/test_exit_code.py b/test/python/functional_tests/test_exit_code.py index 70584387..bd24d250 100644 --- a/test/python/functional_tests/test_exit_code.py +++ b/test/python/functional_tests/test_exit_code.py @@ -11,7 +11,7 @@ def test_exit_code_zero(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 0 @@ -24,7 +24,7 @@ def test_exit_code_one(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 1 @@ -37,7 +37,7 @@ def test_exit_code_two(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 2 diff --git a/test/python/functional_tests/test_file_or_project.py b/test/python/functional_tests/test_file_or_project.py index 56bb1251..bd9d1d74 100644 --- a/test/python/functional_tests/test_file_or_project.py +++ b/test/python/functional_tests/test_file_or_project.py @@ -11,7 +11,7 @@ def test_inspect_file_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -27,7 +27,7 @@ def test_inspect_project_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_multi_file_project.py b/test/python/functional_tests/test_multi_file_project.py index db63472f..246f4f42 100644 --- a/test/python/functional_tests/test_multi_file_project.py +++ b/test/python/functional_tests/test_multi_file_project.py @@ -6,30 +6,30 @@ EXPECTED_JSON = { 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, 'file_review_results': [ { 'file_name': '__init__.py', 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, - 'issues': [] + 'issues': [], }, { 'file_name': 'one.py', 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, - 'issues': [] + 'issues': [], }, { 'file_name': 'other.py', 'quality': { 'code': 'GOOD', - 'text': 'Code quality (beta): GOOD' + 'text': 'Code quality (beta): GOOD', }, 'issues': [ { @@ -38,7 +38,7 @@ 'line': 'a = 1', 'line_number': 2, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -46,7 +46,7 @@ 'line': 'b = 2', 'line_number': 3, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -54,11 +54,11 @@ 'line': 'c = 3', 'line_number': 4, 'column_number': 5, - 'category': 'BEST_PRACTICES' - } - ] + 'category': 'BEST_PRACTICES', + }, + ], }, - ] + ], } @@ -73,7 +73,7 @@ def test_json_format(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() print(stdout) diff --git a/test/python/functional_tests/test_range_of_lines.py b/test/python/functional_tests/test_range_of_lines.py index bcadae0b..57b5530d 100644 --- a/test/python/functional_tests/test_range_of_lines.py +++ b/test/python/functional_tests/test_range_of_lines.py @@ -10,7 +10,7 @@ EXPECTED_JSON = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'category': 'CODE_STYLE', @@ -30,16 +30,16 @@ 'column_number': 2, 'line': 'c=a + b', 'line_number': 4, - 'text': 'Exactly one space required around assignment' - } - ] + 'text': 'Exactly one space required around assignment', + }, + ], } NO_ISSUES_JSON = { 'quality': { 'code': 'EXCELLENT', 'text': 'Code quality (beta): EXCELLENT'}, - 'issues': [] + 'issues': [], } @@ -85,8 +85,8 @@ def test_range_filter_when_start_line_is_not_first( 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json_with_one_issue @@ -145,7 +145,7 @@ def test_range_filter_when_end_line_is_first( expected_json_with_one_issue = { 'quality': { 'code': 'MODERATE', - 'text': 'Code quality (beta): MODERATE' + 'text': 'Code quality (beta): MODERATE', }, 'issues': [{ 'code': 'C0326', @@ -153,8 +153,8 @@ def test_range_filter_when_end_line_is_first( 'line': 'a=10', 'line_number': 1, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json_with_one_issue @@ -211,7 +211,7 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( expected_json = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'code': 'C0326', @@ -219,15 +219,15 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( 'line': 'b=20', 'line_number': 2, 'column_number': 2, - 'category': 'CODE_STYLE' + 'category': 'CODE_STYLE', }, { 'code': 'C0326', 'text': 'Exactly one space required around assignment', 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json diff --git a/test/python/functional_tests/test_single_file_json_format.py b/test/python/functional_tests/test_single_file_json_format.py index d71c115b..48d8f14d 100644 --- a/test/python/functional_tests/test_single_file_json_format.py +++ b/test/python/functional_tests/test_single_file_json_format.py @@ -12,8 +12,8 @@ 'type': 'object', 'properties': { 'code': {'type': 'string'}, - 'text': {'type': 'string'} - } + 'text': {'type': 'string'}, + }, }, 'issues': { 'type': 'array', @@ -25,11 +25,11 @@ 'column_number': {'type': 'number'}, 'line': {'type': 'string'}, 'line_number': {'type': 'number'}, - 'text': {'type': 'string'} - } - } - } - } + 'text': {'type': 'string'}, + }, + }, + }, + }, } @@ -43,7 +43,7 @@ def test_json_format(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() diff --git a/test/python/functional_tests/test_verbosity.py b/test/python/functional_tests/test_verbosity.py index 67df44eb..bed69f90 100644 --- a/test/python/functional_tests/test_verbosity.py +++ b/test/python/functional_tests/test_verbosity.py @@ -13,7 +13,7 @@ def test_disable_logs_text(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() output = output.lower() @@ -33,7 +33,7 @@ def test_disable_logs_json(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -49,7 +49,7 @@ def test_enable_all_logs(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() output = output.lower() diff --git a/test/python/inspectors/conftest.py b/test/python/inspectors/conftest.py index 502c946b..d3df943c 100644 --- a/test/python/inspectors/conftest.py +++ b/test/python/inspectors/conftest.py @@ -31,16 +31,16 @@ def branch_info_response() -> Dict[str, Any]: 'reachability': 1, }, 'canCreateReview': { - 'isAllowed': True + 'isAllowed': True, }, 'stats': { 'parentBranch': 'bar', 'commitsAhead': 0, - 'commitsBehind': 0 + 'commitsBehind': 0, }, 'mergeInfo': {}, - 'isPullRequest': False - } + 'isPullRequest': False, + }, } @@ -52,15 +52,15 @@ def ownership_summary_response() -> Dict[str, Any]: { 'filePath': '/foo.py', 'state': 0, - 'userId': None + 'userId': None, }, { 'filePath': '/bar/baz.py', 'state': 0, - 'userId': None - } - ] - } + 'userId': None, + }, + ], + }, } @@ -89,7 +89,7 @@ def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: n_bool_expr_len=counter[IssueType.BOOL_EXPR_LEN], n_other_complexity=counter[IssueType.COMPLEXITY], n_cohesion=counter[IssueType.COHESION], - n_maintainability=counter[IssueType.MAINTAINABILITY] + n_maintainability=counter[IssueType.MAINTAINABILITY], ) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index c4750ace..be211040 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -115,7 +115,7 @@ def test_choose_issue_type(): expected_issue_types = [ IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, - IssueType.CODE_STYLE + IssueType.CODE_STYLE, ] issue_types = list(map(Flake8Inspector.choose_issue_type, error_codes)) diff --git a/test/python/inspectors/test_local_review.py b/test/python/inspectors/test_local_review.py index f32caab3..e1e028e6 100644 --- a/test/python/inspectors/test_local_review.py +++ b/test/python/inspectors/test_local_review.py @@ -14,7 +14,7 @@ 'allow_duplicates', 'disable', 'format', - 'handler' + 'handler', ]) @@ -24,7 +24,7 @@ def config() -> ApplicationConfig: disabled_inspectors={InspectorType.INTELLIJ}, allow_duplicates=False, n_cpu=1, - inspectors_config=dict(n_cpu=1) + inspectors_config=dict(n_cpu=1), ) diff --git a/test/python/inspectors/test_out_of_range_issues.py b/test/python/inspectors/test_out_of_range_issues.py index 016928b7..3f73d63d 100644 --- a/test/python/inspectors/test_out_of_range_issues.py +++ b/test/python/inspectors/test_out_of_range_issues.py @@ -44,7 +44,7 @@ def test_out_of_range_issues_when_the_same_borders() -> None: first_line_issues = [ create_code_issue_by_line(1), create_code_issue_by_line(1), - create_code_issue_by_line(1) + create_code_issue_by_line(1), ] assert filter_out_of_range_issues(first_line_issues, diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index e142e8aa..ab807765 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -72,7 +72,7 @@ def test_choose_issue_type(): IssueType.BEST_PRACTICES, IssueType.BEST_PRACTICES, IssueType.CODE_STYLE, - IssueType.ERROR_PRONE + IssueType.ERROR_PRONE, ] issue_types = list( From 81356dc39854e89477b3eb2d2f839b14e26c365b Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 13:50:08 +0300 Subject: [PATCH 30/37] Added convert_percentage_of_value_to_lack_of_value --- src/python/review/inspectors/common.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/python/review/inspectors/common.py diff --git a/src/python/review/inspectors/common.py b/src/python/review/inspectors/common.py new file mode 100644 index 00000000..e435efec --- /dev/null +++ b/src/python/review/inspectors/common.py @@ -0,0 +1,12 @@ +from math import floor + + +def convert_percentage_of_value_to_lack_of_value(percentage_of_value: float) -> int: + """ + Converts percentage of value to lack of value. + Calculated by the formula: floor(100 - percentage_of_value). + + :param percentage_of_value: value in the range from 0 to 100. + :return: lack of maintainability. + """ + return floor(100 - percentage_of_value) From 06553f37207339430cb9e85ce32902a3e756bac9 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 13:51:11 +0300 Subject: [PATCH 31/37] Replaced __get_cohesion_lack with convert_percentage_of_value_to_lack_of_value --- src/python/review/inspectors/flake8/flake8.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index c7318d0b..89592e0a 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -17,6 +17,7 @@ CohesionIssue, ) from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip +from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value logger = logging.getLogger(__name__) @@ -69,7 +70,9 @@ def parse(cls, output: str) -> List[BaseIssue]: issues.append(CyclomaticComplexityIssue(**issue_data)) elif cohesion_match is not None: # flake8-cohesion issue_data[IssueData.DESCRIPTION.value] = description # TODO: Add tip - issue_data[IssueData.COHESION_LACK.value] = cls.__get_cohesion_lack(float(cohesion_match.group(1))) + issue_data[IssueData.COHESION_LACK.value] = convert_percentage_of_value_to_lack_of_value( + float(cohesion_match.group(1)) + ) issue_data[IssueData.ISSUE_TYPE.value] = IssueType.COHESION issues.append(CohesionIssue(**issue_data)) else: @@ -98,14 +101,3 @@ def choose_issue_type(code: str) -> IssueType: return IssueType.BEST_PRACTICES return issue_type - - @staticmethod - def __get_cohesion_lack(cohesion_percentage: float) -> int: - """ - Converts cohesion percentage to lack of cohesion. - Calculated by the formula: floor(100 - cohesion_percentage). - - :param cohesion_percentage: cohesion set as a percentage. - :return: lack of cohesion - """ - return math.floor(100 - cohesion_percentage) From ac4349b7c363c204df4dbd6151e50c8a10049848 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 13:51:38 +0300 Subject: [PATCH 32/37] typo fix --- src/python/review/inspectors/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/review/inspectors/common.py b/src/python/review/inspectors/common.py index e435efec..dfedbb1e 100644 --- a/src/python/review/inspectors/common.py +++ b/src/python/review/inspectors/common.py @@ -7,6 +7,6 @@ def convert_percentage_of_value_to_lack_of_value(percentage_of_value: float) -> Calculated by the formula: floor(100 - percentage_of_value). :param percentage_of_value: value in the range from 0 to 100. - :return: lack of maintainability. + :return: lack of value. """ return floor(100 - percentage_of_value) From a442f84ab95ab971a9020d38f43eebea8c1a8076 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 13:53:07 +0300 Subject: [PATCH 33/37] Small code refactoring: 1) Replaced __get_maintainability_lack with convert_percentage_of_value_to_lack_of_value; 2) Added some comments. --- src/python/review/inspectors/radon/radon.py | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py index 3c2a52e7..5b054b20 100644 --- a/src/python/review/inspectors/radon/radon.py +++ b/src/python/review/inspectors/radon/radon.py @@ -7,6 +7,7 @@ from src.python.review.inspectors.base_inspector import BaseInspector from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.inspectors.issue import BaseIssue, IssueData, IssueType, MaintainabilityLackIssue +from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value class RadonInspector(BaseInspector): @@ -15,9 +16,9 @@ class RadonInspector(BaseInspector): @classmethod def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: mi_command = [ - "radon", "mi", - "--max", "F", - "--show", + "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, ] @@ -28,6 +29,7 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: 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. @@ -37,7 +39,7 @@ def mi_parse(cls, mi_output: str) -> List[BaseIssue]: issues: List[BaseIssue] = [] for groups in row_re.findall(mi_output): file_path = Path(groups[0]) - maintainability_lack = cls.__get_maintainability_lack(float(groups[1])) + maintainability_lack = convert_percentage_of_value_to_lack_of_value(float(groups[1])) issue_data = IssueData.get_base_issue_data_dict(file_path, cls.inspector_type) issue_data[IssueData.DESCRIPTION.value] = "" # TODO: add tip @@ -47,14 +49,3 @@ def mi_parse(cls, mi_output: str) -> List[BaseIssue]: issues.append(MaintainabilityLackIssue(**issue_data)) return issues - - @staticmethod - def __get_maintainability_lack(maintainability_index: float) -> int: - """ - Converts maintainability index to lack of maintainability. - Calculated by the formula: floor(100 - maintainability_index). - - :param maintainability_index: value in the range from 0 to 100. - :return: lack of maintainability. - """ - return floor(100 - maintainability_index) From 636749342024afff2a60641bede5fb6efebc6c7d Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 13:56:06 +0300 Subject: [PATCH 34/37] Removed unused import --- src/python/review/inspectors/radon/radon.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py index 5b054b20..0b4441f6 100644 --- a/src/python/review/inspectors/radon/radon.py +++ b/src/python/review/inspectors/radon/radon.py @@ -1,5 +1,4 @@ import re -from math import floor from pathlib import Path from typing import List From 805a49235fe0f8072f4a264515f5e50da58d11e2 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 13:57:28 +0300 Subject: [PATCH 35/37] Small code refactoring: 1) Removed unused import; 2) Fixed trailing comma. --- src/python/review/inspectors/flake8/flake8.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 89592e0a..b974015c 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -1,5 +1,4 @@ import logging -import math import re from pathlib import Path from typing import List @@ -71,7 +70,7 @@ def parse(cls, output: str) -> List[BaseIssue]: 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)) + float(cohesion_match.group(1)), ) issue_data[IssueData.ISSUE_TYPE.value] = IssueType.COHESION issues.append(CohesionIssue(**issue_data)) From 62600e33e163c196fa91b1682768a2d15e7e4c01 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 17:04:15 +0300 Subject: [PATCH 36/37] Added maintainability index tip --- src/python/review/inspectors/radon/radon.py | 3 ++- src/python/review/inspectors/tips.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py index 0b4441f6..fb2bf299 100644 --- a/src/python/review/inspectors/radon/radon.py +++ b/src/python/review/inspectors/radon/radon.py @@ -7,6 +7,7 @@ from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.inspectors.issue import BaseIssue, IssueData, IssueType, MaintainabilityLackIssue from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value +from src.python.review.inspectors.tips import get_maintainability_index_tip class RadonInspector(BaseInspector): @@ -41,7 +42,7 @@ def mi_parse(cls, mi_output: str) -> List[BaseIssue]: maintainability_lack = convert_percentage_of_value_to_lack_of_value(float(groups[1])) issue_data = IssueData.get_base_issue_data_dict(file_path, cls.inspector_type) - issue_data[IssueData.DESCRIPTION.value] = "" # TODO: add tip + 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 diff --git a/src/python/review/inspectors/tips.py b/src/python/review/inspectors/tips.py index 5211b569..eac449c1 100644 --- a/src/python/review/inspectors/tips.py +++ b/src/python/review/inspectors/tips.py @@ -84,3 +84,8 @@ def get_method_number_tip() -> str: 'The file has too many methods inside and is complicated to understand. ' 'Consider its decomposition to smaller classes.' ) + + +# TODO: Need to improve the tip. +def get_maintainability_index_tip() -> str: + return 'The maintainability index is too low.' From bacb25f96bb49f0f244ee39083fe1fd8a8eb04d5 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 10 Apr 2021 17:13:29 +0300 Subject: [PATCH 37/37] Fixed test --- test/python/inspectors/test_radon_inspector.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/python/inspectors/test_radon_inspector.py b/test/python/inspectors/test_radon_inspector.py index 2d7f18f7..72fd03e7 100644 --- a/test/python/inspectors/test_radon_inspector.py +++ b/test/python/inspectors/test_radon_inspector.py @@ -8,6 +8,7 @@ from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues from test.python.inspectors import PYTHON_DATA_FOLDER from test.python.inspectors.conftest import use_file_metadata +from src.python.review.inspectors.tips import get_maintainability_index_tip FILE_NAMES_AND_N_ISSUES = [ ("case13_complex_logic.py", 1), @@ -42,7 +43,7 @@ def test_mi_parse(): 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] == ["", "", ""] + assert [issue.description for issue in issues] == [get_maintainability_index_tip()] * 3 assert [issue.type for issue in issues] == [ IssueType.MAINTAINABILITY, IssueType.MAINTAINABILITY,