From 41eddcdb2f340251865db81780a7cccfd58f257a Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 14 Mar 2021 19:33:10 +0300 Subject: [PATCH 01/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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 9c8de68e183af07027f1016a01732e6fd5eaf99b Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 3 Apr 2021 17:01:53 +0300 Subject: [PATCH 22/29] Moved WPS_RANGE_TO_ISSUE_TYPE into CODE_PREFIX_TO_ISSUE_TYPE. --- src/python/review/inspectors/flake8/flake8.py | 19 +++++-------------- .../review/inspectors/flake8/issue_types.py | 16 +++++++--------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 92a82a99..1cbc67e2 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -5,11 +5,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, - WPS_RANGE_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, CodeIssue, CyclomaticComplexityIssue, IssueType, IssueData from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip @@ -71,18 +67,13 @@ def choose_issue_type(code: str) -> IssueType: if code in CODE_TO_ISSUE_TYPE: return CODE_TO_ISSUE_TYPE[code] - regex_match = re.match(r'^([a-z]+)(\d+)$', code, re.IGNORECASE) + regex_match = re.match(r'^([A-Z]+)(\d)\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 + first_code_number = regex_match.group(2) # Handling other issues - issue_type = CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix) + issue_type = (CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix + first_code_number) or + CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix)) if not issue_type: logger.warning(f'flake8: {code} - unknown error code') return IssueType.BEST_PRACTICES diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index 9de7ec6e..749d708b 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -83,15 +83,6 @@ "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] = { 'B': IssueType.ERROR_PRONE, # flake8-bugbear 'A': IssueType.ERROR_PRONE, # flake8-builtins @@ -105,4 +96,11 @@ 'F': IssueType.BEST_PRACTICES, # standard flake8 'C': IssueType.BEST_PRACTICES, # flake8-comprehensions 'SC': IssueType.BEST_PRACTICES, # flake8-spellcheck + + 'WPS1': IssueType.CODE_STYLE, # WPS type: Naming + 'WPS2': IssueType.COMPLEXITY, # WPS type: Complexity + 'WPS3': IssueType.BEST_PRACTICES, # WPS type: Consistency + 'WPS4': IssueType.BEST_PRACTICES, # WPS type: Best practices + 'WPS5': IssueType.BEST_PRACTICES, # WPS type: Refactoring + 'WPS6': IssueType.BEST_PRACTICES, # WPS type: OOP } From 39e04436ca747917c4891e263781bb8c3a57476a Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 3 Apr 2021 17:21:32 +0300 Subject: [PATCH 23/29] Fixed line break after a binary operator --- src/python/review/inspectors/flake8/flake8.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 1cbc67e2..38db757c 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -72,8 +72,8 @@ def choose_issue_type(code: str) -> IssueType: first_code_number = regex_match.group(2) # Handling other issues - issue_type = (CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix + first_code_number) or - CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix)) + issue_type = (CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix + first_code_number) + or CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix)) if not issue_type: logger.warning(f'flake8: {code} - unknown error code') return IssueType.BEST_PRACTICES From 4988ebcbb03ab04b6746c72207642b8f84c691ea Mon Sep 17 00:00:00 2001 From: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Date: Sat, 3 Apr 2021 17:23:58 +0300 Subject: [PATCH 24/29] Add W503 to ignore https://www.flake8rules.com/rules/W503.html --- .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..c177f809 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,W503,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 From 9f05de77c14e7325b5fbf3cd4a713b517dba1f1c Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 3 Apr 2021 18:45:32 +0300 Subject: [PATCH 25/29] Add file_name into output multiline string --- test/python/inspectors/test_pylint_inspector.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index ca9f9cda..e142e8aa 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -48,9 +48,9 @@ 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 - test.py:2:12:C1444:test 2 + output = f"""\ + {file_name}:1:11:R0123:test 1 + {file_name}:2:12:C1444:test 2 """ output = textwrap.dedent(output) From 97c6f66e43a0bf3334cf498b769fdef42eb52bea Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sat, 3 Apr 2021 20:02:15 +0300 Subject: [PATCH 26/29] Add file_name into output multiline fstring --- test/python/inspectors/test_flake8_inspector.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index b8adb40d..e92c11e5 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -1,4 +1,5 @@ import pytest +from textwrap import dedent from src.python.review.common.language import Language from src.python.review.inspectors.flake8.flake8 import Flake8Inspector @@ -91,9 +92,12 @@ def test_file_with_issues_info(file_name: str, expected_issues_info: IssuesTestI def test_parse(): file_name = 'test.py' - output = ('test.py:1:11:W602:test 1\n' - 'test.py:2:12:E703:test 2\n' - 'test.py:3:13:SC200:test 3') + output = f"""\ + {file_name}:1:11:W602:test 1 + {file_name}:2:12:E703:test 2 + {file_name}:3:13:SC200:test 3 + """ + output = dedent(output) issues = Flake8Inspector.parse(output) From 346d3c96f0b3eedb457aaf2696d5afd5f103a017 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 4 Apr 2021 13:37:16 +0300 Subject: [PATCH 27/29] Fixed bug --- src/python/review/reviewers/utils/issues_filter.py | 2 +- test/python/inspectors/test_flake8_inspector.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/review/reviewers/utils/issues_filter.py b/src/python/review/reviewers/utils/issues_filter.py index 83960d92..ee9be2a1 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -24,7 +24,7 @@ def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, 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.COHESION: LANGUAGE_TO_COHESION_RULE_CONFIG[language].cohesion_lack_bad, + IssueType.COHESION: LANGUAGE_TO_COHESION_RULE_CONFIG[language].cohesion_lack_good, } diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index e92c11e5..c4750ace 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -17,7 +17,7 @@ ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), - ('case7_empty_lines.py', 1), + ('case7_empty_lines.py', 2), ('case10_unused_variable_in_loop.py', 1), ('case13_complex_logic.py', 12), ('case13_complex_logic_2.py', 3), From 3e4ac30bc487da80dced33d7b33f4fb2f3d65619 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Sun, 4 Apr 2021 13:37:41 +0300 Subject: [PATCH 28/29] Added good value to config --- src/python/review/quality/rules/cohesion_scoring.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/python/review/quality/rules/cohesion_scoring.py b/src/python/review/quality/rules/cohesion_scoring.py index 654a5ce6..897a5e35 100644 --- a/src/python/review/quality/rules/cohesion_scoring.py +++ b/src/python/review/quality/rules/cohesion_scoring.py @@ -10,11 +10,13 @@ class CohesionRuleConfig: cohesion_lack_bad: int cohesion_lack_moderate: int + cohesion_lack_good: int common_cohesion_rule_config = CohesionRuleConfig( cohesion_lack_bad=50, - cohesion_lack_moderate=30 + cohesion_lack_moderate=30, + cohesion_lack_good=10, ) LANGUAGE_TO_COHESION_RULE_CONFIG = { @@ -39,6 +41,9 @@ def apply(self, cohesion_lack: int): elif cohesion_lack > self.config.cohesion_lack_moderate: self.quality_type = QualityType.MODERATE self.next_level_delta = cohesion_lack - self.config.cohesion_lack_moderate + elif cohesion_lack > self.config.cohesion_lack_good: + self.quality_type = QualityType.GOOD + self.next_level_delta = cohesion_lack - self.config.cohesion_lack_good else: self.quality_type = QualityType.EXCELLENT self.next_level_delta = 0 @@ -47,12 +52,15 @@ def apply(self, cohesion_lack: int): def __get_next_quality_type(self) -> QualityType: if self.quality_type == QualityType.BAD: return QualityType.MODERATE + elif self.quality_type == QualityType.MODERATE: + return QualityType.GOOD return QualityType.EXCELLENT def merge(self, other: 'CohesionRule') -> 'CohesionRule': config = CohesionRuleConfig( min(self.config.cohesion_lack_bad, other.config.cohesion_lack_bad), min(self.config.cohesion_lack_moderate, other.config.cohesion_lack_moderate), + min(self.config.cohesion_lack_good, other.config.cohesion_lack_good), ) result_rule = CohesionRule(config) result_rule.apply(max(self.cohesion_lack, other.cohesion_lack)) From 6076e573b5bbd6f5122bfa209834dd664f4eb198 Mon Sep 17 00:00:00 2001 From: Ilya Vlasov Date: Mon, 5 Apr 2021 12:13:37 +0300 Subject: [PATCH 29/29] Added TODO --- src/python/review/quality/rules/cohesion_scoring.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/python/review/quality/rules/cohesion_scoring.py b/src/python/review/quality/rules/cohesion_scoring.py index 897a5e35..c73a426b 100644 --- a/src/python/review/quality/rules/cohesion_scoring.py +++ b/src/python/review/quality/rules/cohesion_scoring.py @@ -13,6 +13,9 @@ class CohesionRuleConfig: cohesion_lack_good: int +# TODO: Need testing +# Cohesion plugin by default only shows issues where cohesion is less than 50%. +# Therefore cohesion_lack_bad = 50. The other levels are set in steps of 20%. common_cohesion_rule_config = CohesionRuleConfig( cohesion_lack_bad=50, cohesion_lack_moderate=30,