diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 54dd6ade..87adf3fa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,34 +10,60 @@ jobs: container: stepik/hyperstyle-base:py3.8.11-java11.0.11-node14.17.3 steps: + - name: Install git + run: | + apt-get update + apt-get -y install git + - name: Checkout - uses: actions/checkout@v1 + uses: actions/checkout@v2 - name: Install requirements run: | pip install --no-cache-dir -r requirements-test.txt -r requirements.txt + - name: Lint with flake8 run: | # 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 + # TODO: remove R504, A003, E800, E402, WPS1, WPS2, WPS3, WPS4, WPS5, WPS6, H601 flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=R504,A003,E800,E402,W503,WPS,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + + - name: Sort whitelists + run: | + for file in "whitelist.txt" "src/python/review/inspectors/flake8/whitelist.txt" + do + LC_ALL=C sort $file -o $file + done + + - name: Commit sorted whitelists + uses: EndBug/add-and-commit@v7.2.1 + with: + add: "['whitelist.txt', 'src/python/review/inspectors/flake8/whitelist.txt']" + message: 'Sort whitelists (Github Actions)' + - name: Set up Eslint run: | # Consistent with eslint version in Dockerfile npm install eslint@7.5.0 -g && eslint --init + - name: Test with pytest run: | pytest + - name: Install review module run: | pip install . + - name: Check installed module can run python linters run: | review setup.py + - name: Check installed module can run java linters run: | review test/resources/inspectors/java/test_algorithm_with_scanner.java + - name: Check installed module can run js linters run: | - review test/resources/inspectors/js/case0_no_issues.js \ No newline at end of file + review test/resources/inspectors/js/case0_no_issues.js diff --git a/requirements-test.txt b/requirements-test.txt index dc0f70dc..824641d9 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -2,10 +2,3 @@ pytest==6.2.3 pytest-runner==5.2 pytest-subtests==0.4.0 jsonschema==3.2.0 -pandas==1.2.3 -django==3.2 -pylint==2.7.4 -requests==2.25.1 -setuptools==56.0.0 -openpyxl==3.0.7 -pandarallel==1.5.2 \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index 51a5d0cb..4cfad8e5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,4 @@ radon==4.5.0 django==3.2 requests==2.25.1 argparse==1.4.0 -pyyaml \ No newline at end of file +pyyaml==5.4.1 \ No newline at end of file diff --git a/src/python/review/inspectors/checkstyle/files/config.xml b/src/python/review/inspectors/checkstyle/files/config.xml index 20689d93..f49b5267 100644 --- a/src/python/review/inspectors/checkstyle/files/config.xml +++ b/src/python/review/inspectors/checkstyle/files/config.xml @@ -36,9 +36,9 @@ - + - + @@ -81,16 +81,42 @@ + + + + + + + + + + + + + + + + - - - - - - - - + + + + + + + + + diff --git a/src/python/review/inspectors/flake8/.flake8 b/src/python/review/inspectors/flake8/.flake8 index 815863c0..42b81a88 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -27,11 +27,15 @@ ignore=W291, # trailing whitespaces WPS303, # Forbid underscores in numbers. WPS305, # Forbid f strings. WPS306, # Forbid writing classes without base classes. + WPS317, # Forbid incorrect indentation for parameters. (Because of the unnecessary strictness) WPS318, # Forbid extra indentation. TODO: Collision with standard flake8 indentation check + WPS319, # Forbid brackets in the wrong position. (Because of the unnecessary strictness) WPS323, # Forbid % formatting on strings. WPS324, # Enforce consistent return statements. TODO: Collision with flake8-return WPS335, # Forbid wrong for loop iter targets. + WPS347, # Forbid imports that may cause confusion outside of the module. (controversial) WPS358, # Forbid using float zeros: 0.0. + WPS359, # Forbids to unpack iterable objects to lists. (Because of its similarity to "WPS414") WPS362, # Forbid assignment to a subscript slice. # WPS: Best practices WPS404, # Forbid complex defaults. TODO: Collision with "B006" @@ -50,6 +54,7 @@ ignore=W291, # trailing whitespaces P101, # format string does contain unindexed parameters P102, # docstring does contain unindexed parameters P103, # other string does contain unindexed parameters + F405, # Name may be undefined, or defined from star imports (Collision with the stricter "F403") F522, # unused named arguments. TODO: Collision with "P302" F523, # unused positional arguments. TODO: Collision with "P301" F524, # missing argument. TODO: Collision with "P201" and "P202" diff --git a/src/python/review/inspectors/flake8/whitelist.txt b/src/python/review/inspectors/flake8/whitelist.txt index ab05c2fa..38715032 100644 --- a/src/python/review/inspectors/flake8/whitelist.txt +++ b/src/python/review/inspectors/flake8/whitelist.txt @@ -1,9 +1,12 @@ aggfunc appendleft +arange argmax asctime astype +barmode betavariate +bgcolor birthdate blackbox bs4 @@ -13,6 +16,8 @@ capwords casefold caseless concat +config +configs consts coord copysign @@ -40,6 +45,7 @@ expm1 falsy fillna floordiv +fromkeys fromstring fullmatch gensim @@ -47,6 +53,7 @@ gmtime groupby halfs hashable +hline href hyp hyperskill @@ -72,6 +79,8 @@ lemmatizer lifes lim linalg +linecolor +linewidth linspace lowercased lvl @@ -79,6 +88,7 @@ lxml matmul minsize multiline +nbins ndarray ndigits ndim @@ -100,6 +110,7 @@ rindex rmdir schur scipy +showline sigmoid sqrt src @@ -117,6 +128,7 @@ tokenize tokenized tokenizer tolist +toplevel tracklist truediv truthy @@ -125,6 +137,11 @@ unpickled upd util utils +vline webpage whitespaces writeback +xanchor +xaxes +yanchor +yaxis diff --git a/src/python/review/inspectors/pmd/files/bin/basic.xml b/src/python/review/inspectors/pmd/files/bin/basic.xml index 724ee58b..7dffeeed 100644 --- a/src/python/review/inspectors/pmd/files/bin/basic.xml +++ b/src/python/review/inspectors/pmd/files/bin/basic.xml @@ -53,25 +53,31 @@ - + - + + - + - + - - + + @@ -93,7 +99,8 @@ - + diff --git a/src/python/review/inspectors/pylint/issue_types.py b/src/python/review/inspectors/pylint/issue_types.py index edfe8818..5df676b1 100644 --- a/src/python/review/inspectors/pylint/issue_types.py +++ b/src/python/review/inspectors/pylint/issue_types.py @@ -8,6 +8,8 @@ # E errors, for probable bugs in the code CODE_TO_ISSUE_TYPE: Dict[str, IssueType] = { + 'C0200': IssueType.BEST_PRACTICES, # consider using enumerate + 'C0415': IssueType.BEST_PRACTICES, # import-outside-toplevel 'W0101': IssueType.ERROR_PRONE, # unreachable code 'W0102': IssueType.ERROR_PRONE, # dangerous default value 'W0104': IssueType.ERROR_PRONE, # statement doesn't have any effect diff --git a/src/python/review/inspectors/pylint/pylintrc b/src/python/review/inspectors/pylint/pylintrc index c435b63b..357ba058 100644 --- a/src/python/review/inspectors/pylint/pylintrc +++ b/src/python/review/inspectors/pylint/pylintrc @@ -164,6 +164,8 @@ disable=invalid-name, no-else-raise, no-else-break, no-else-continue, + W0614, + W0622, # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/test/python/functional_tests/test_difficulty_levels.py b/test/python/functional_tests/test_difficulty_levels.py index 4c6e37b6..5f758ca1 100644 --- a/test/python/functional_tests/test_difficulty_levels.py +++ b/test/python/functional_tests/test_difficulty_levels.py @@ -171,16 +171,6 @@ def _get_output_json(local_command: LocalCommandBuilder, file_path: str, history 'line_number': 10, 'text': 'continuation line under-indented for hanging indent', }, - { - 'category': 'CODE_STYLE', - 'code': 'WPS319', - 'column_number': 21, - 'difficulty': 'EASY', - 'influence_on_penalty': {'EASY': 0, 'HARD': 0, 'MEDIUM': 0}, - 'line': '"Hello, World")', - 'line_number': 10, - 'text': 'Found bracket in wrong position', - }, ], } @@ -394,16 +384,6 @@ def test_difficulty_levels(local_command: LocalCommandBuilder, file: str, expect 'line_number': 10, 'text': 'continuation line under-indented for hanging indent', }, - { - 'category': 'CODE_STYLE', - 'code': 'WPS319', - 'column_number': 21, - 'difficulty': 'EASY', - 'influence_on_penalty': {'EASY': 0, 'MEDIUM': 0, 'HARD': 0}, - 'line': '"Hello, World")', - 'line_number': 10, - 'text': 'Found bracket in wrong position', - }, ], } diff --git a/test/python/inspectors/test_checkstyle_inspector.py b/test/python/inspectors/test_checkstyle_inspector.py index 55bd84da..4d2c7889 100644 --- a/test/python/inspectors/test_checkstyle_inspector.py +++ b/test/python/inspectors/test_checkstyle_inspector.py @@ -200,6 +200,7 @@ def test_output_parsing(file_name: str, expected_issues: List[CodeIssue]): ('test_indentation_with_spaces.java', 5), ('test_indentation_with_tabs.java', 5), ('test_indentation_google_style.java', 6), + ('test_multiple_literals.java', 1), ] diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 961e4c22..4d84a3c7 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -13,7 +13,7 @@ ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 8), ('case3_redefining_builtin.py', 2), - ('case4_naming.py', 11), + ('case4_naming.py', 8), ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), @@ -25,14 +25,17 @@ ('case14_returns_errors.py', 4), ('case16_comments.py', 0), ('case17_dangerous_default_value.py', 1), - # ('case18_comprehensions.py', 9), + ('case18_comprehensions.py', 9), ('case19_bad_indentation.py', 3), ('case21_imports.py', 2), ('case25_django.py', 0), - ('case31_line_break.py', 11), + ('case31_spellcheck.py', 0), ('case32_string_format.py', 34), ('case33_commas.py', 14), - # ('case34_cohesion.py', 1), + ('case34_cohesion.py', 1), + ('case35_line_break.py', 11), + ('case36_unpacking.py', 3), + ('case37_wildcard_import.py', 1), ] @@ -43,7 +46,7 @@ def test_file_with_issues(file_name: str, n_issues: int): path_to_file = PYTHON_DATA_FOLDER / file_name with use_file_metadata(path_to_file) as file_metadata: issues = inspector.inspect(file_metadata.path, {}) - issues = filter_low_measure_issues(issues, Language.PYTHON) + issues = list(filter(lambda i: i.type != IssueType.INFO, filter_low_measure_issues(issues, Language.PYTHON))) assert len(issues) == n_issues @@ -69,12 +72,14 @@ 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, + ('case35_line_break.py', IssuesTestInfo(n_best_practices=1, n_code_style=10, n_cc=1)), ('case32_string_format.py', IssuesTestInfo(n_error_prone=28, n_other_complexity=6)), ('case33_commas.py', IssuesTestInfo(n_code_style=14, n_cc=4)), ('case34_cohesion.py', IssuesTestInfo(n_cc=6, n_cohesion=2)), + ('case36_unpacking.py', IssuesTestInfo(n_error_prone=2, n_cc=1, n_other_complexity=1)), + ('case37_wildcard_import.py', IssuesTestInfo(n_best_practices=1, n_cc=1)), ] diff --git a/test/python/inspectors/test_pmd_inspector.py b/test/python/inspectors/test_pmd_inspector.py index ad7db75f..e4688c66 100644 --- a/test/python/inspectors/test_pmd_inspector.py +++ b/test/python/inspectors/test_pmd_inspector.py @@ -99,7 +99,7 @@ def test_output_parsing(file_name: str, expected_issues: List[CodeIssue]): ('test_comparing_strings.java', 4), ('test_constants.java', 4), ('test_covariant_equals.java', 1), - ('test_curly_braces.java', 2), + ('test_curly_braces.java', 0), ('test_double_checked_locking.java', 2), ('test_for_loop.java', 2), ('test_implementation_types.java', 0), @@ -116,6 +116,7 @@ def test_output_parsing(file_name: str, expected_issues: List[CodeIssue]): ('test_valid_curly_braces.java', 0), ('test_when_only_equals_overridden.java', 1), ('test_valid_spaces.java', 0), + ('test_multiple_literals.java', 1), ] diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index ba4be12f..fc381234 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -11,7 +11,7 @@ ('case0_spaces.py', 0), ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 3), - ('case3_redefining_builtin.py', 2), + ('case3_redefining_builtin.py', 0), ('case4_naming.py', 3), ('case5_returns.py', 1), ('case6_unused_variables.py', 4), @@ -32,6 +32,8 @@ ('case25_django.py', 0), ('case27_using_requests.py', 0), ('case30_allow_else_return.py', 0), + ('case36_unpacking.py', 0), + ('case37_wildcard_import.py', 1), ] diff --git a/test/resources/functional_tests/different_languages/python/__init__.py b/test/resources/functional_tests/different_languages/python/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/test/resources/inspectors/java/test_multiple_literals.java b/test/resources/inspectors/java/test_multiple_literals.java new file mode 100644 index 00000000..58a9e2ee --- /dev/null +++ b/test/resources/inspectors/java/test_multiple_literals.java @@ -0,0 +1,32 @@ +class Main { + public static void main(String[] args) { + // ok + String shortRareLiteral1 = "12"; + String shortRareLiteral2 = "12"; + String shortRareLiteral3 = "12"; + + // ok + String longRareLiteral1 = "123"; + String longRareLiteral2 = "123"; + String longRareLiteral3 = "123"; + + // ok + String shortFrequentLiteral1 = "34"; + String shortFrequentLiteral2 = "34"; + String shortFrequentLiteral3 = "34"; + String shortFrequentLiteral4 = "34"; + + // warning + String longFrequentLiteral1 = "456"; + String longFrequentLiteral2 = "456"; + String longFrequentLiteral3 = "456"; + String longFrequentLiteral4 = "456"; + + System.out.println( + shortRareLiteral1 + shortRareLiteral2 + shortRareLiteral3 + + longRareLiteral1 + longRareLiteral2 + longRareLiteral3 + + shortFrequentLiteral1 + shortFrequentLiteral2 + shortFrequentLiteral3 + shortFrequentLiteral4 + + longFrequentLiteral1 + longFrequentLiteral2 + longFrequentLiteral3 + longFrequentLiteral4 + ); + } +} diff --git a/test/resources/inspectors/python/__init__.py b/test/resources/inspectors/python/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/test/resources/inspectors/python/case31_spellcheck.py b/test/resources/inspectors/python/case31_spellcheck.py new file mode 100644 index 00000000..b777eb79 --- /dev/null +++ b/test/resources/inspectors/python/case31_spellcheck.py @@ -0,0 +1,3 @@ +import math + +number = math.sqrt(float(input())) diff --git a/test/resources/inspectors/python/case35_line_break.py b/test/resources/inspectors/python/case35_line_break.py new file mode 100644 index 00000000..ba718ea8 --- /dev/null +++ b/test/resources/inspectors/python/case35_line_break.py @@ -0,0 +1,54 @@ +# Wrong +from math import exp, \ + log + +# Correct +from math import ( + sin, + cos, +) + + +# Wrong +def do_something_wrong(x: float, y: float): + if sin(x) == cos(y) \ + and exp(x) == log(y): + print("Do not do that!") + + +print(do_something_wrong(1, 2)) + +# Wrong +wrong_string = "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. " \ + + "Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, " \ + + "nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. " \ + + "Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, " \ + + "vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. " \ + + "Nullam dictum felis eu pede mollis pretium. Integer tincidunt. Cras dapibus. " \ + + "Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, " \ + + "porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, " \ + + "viverra quis, feugiat a," +print(wrong_string) + +# Correct +correct_string = ("Lorem ipsum dolor sit amet, consectetuer adipiscing elit. " + + "Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis " + + "parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, " + + "pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, " + + "aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, " + + "venenatis vitae, justo. Nullam dictum felis eu pede mollis pretium. Integer tincidunt. " + + "Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. " + + "Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. " + + "Aliquam lorem ante, dapibus in, viverra quis, feugiat a," + ) +print(correct_string) + +other_correct_string = """ + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec euismod vitae libero ut consequat. + Fusce quis ultrices sem, vitae viverra mi. Praesent fermentum quam ac volutpat condimentum. + Proin mauris orci, molestie vel fermentum vel, consectetur vel metus. Quisque vitae mollis magna. + In hac habitasse platea dictumst. Pellentesque sed diam eget dolor ultricies faucibus id sed quam. + Nam risus erat, varius ut risus a, tincidunt vulputate magna. Etiam lacinia a eros non faucibus. + In facilisis tempor nisl, sit amet feugiat lacus viverra quis. +""" +print(other_correct_string) diff --git a/test/resources/inspectors/python/case36_unpacking.py b/test/resources/inspectors/python/case36_unpacking.py new file mode 100644 index 00000000..74b4ec9c --- /dev/null +++ b/test/resources/inspectors/python/case36_unpacking.py @@ -0,0 +1,10 @@ +[a, b, c], [x, y, z] = (sorted(map(int, input().split())) for _ in 'lm') +if [a, b, c] == [x, y, z]: + a = "Boxes are equal" +elif a <= x and b <= y and c <= z: + a = "The first box is smaller than the second one" +elif a >= x and b >= y and c >= z: + a = "The first box is larger than the second one" +else: + a = "Boxes are incomparable" +print(a) diff --git a/test/resources/inspectors/python/case37_wildcard_import.py b/test/resources/inspectors/python/case37_wildcard_import.py new file mode 100644 index 00000000..235cb852 --- /dev/null +++ b/test/resources/inspectors/python/case37_wildcard_import.py @@ -0,0 +1,8 @@ +from math import * + + +def quick_reversed_sqrt(number): + """ + It's quick. Trust me. + """ + return 1 / sqrt(number) diff --git a/test/resources/inspectors/python_ast/__init__.py b/test/resources/inspectors/python_ast/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/whitelist.txt b/whitelist.txt index 0660d745..e6557afa 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -1,14 +1,32 @@ +Checkstyle +Cyclomatic D3 DARK2 DARK24 +DOTALL +Detekt +ECMA +EXPR G10 +IGNORECASE +INTELLIJ +Intelli +JS +KTS LIGHT24 +Multithreading +Namespace +OOP PASTEL2 Pastel1 SET1 SET2 SET3 +Spotbugs +SpringLint T10 +WPS +XLSX abstractmethod arange astype @@ -61,7 +79,6 @@ eq eslint etree eval -eval exc expr exprs @@ -190,6 +207,7 @@ textposition textwrap tmp tokenizer +toplevel uncommented uniformtext uniq