From c34a3746c9d9d2879d7914d9230a2215f9ffe40d Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 4 Nov 2019 23:01:24 +0300 Subject: [PATCH] Closes #866 --- CHANGELOG.md | 12 +-- tests/fixtures/noqa.py | 5 +- tests/test_violations/test_docs.py | 6 ++ .../test_keywords/test_keywords_spaces.py | 8 +- .../test_keywords/test_starts_with_dot.py | 86 +++++++++++++++++++ .../violations/consistency.py | 43 ++++++---- .../visitors/ast/operators.py | 20 ----- .../visitors/tokenize/keywords.py | 22 ++++- 8 files changed, 156 insertions(+), 46 deletions(-) create mode 100644 tests/test_visitors/test_tokenize/test_keywords/test_starts_with_dot.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 438b52e39..100beca39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,18 +9,15 @@ We used to have incremental versioning before `0.1.0`. - Adds cognitive complexity metric, introduced by `cognitive_complexity` - `WPS431` now allow customize whitelist via `nested-classes-whitelist` setting -- Forbids to have invalid strings like `**{'@': 1}` -- Forbids to use implicit primitive values in a form of `lambda` +- Forbids to have invalid strings in stared expressions like `**{'@': 1}` +- Forbids to use implicit primitive values in a form of `lambda: 0` - Forbids to use approximate math constants - Forbids to redefine string constants - Forbids using `Literal[None]` in function annotations - Forbids using nested `typing.Literal`, `typing.Union` and `typing.Annotated` - Forbids use of vague import names (e.g. `from json import loads`) - Makes `OveruseOfNoqaCommentViolation` configurable via `--max-noqa-comments` -- Improves tests: we now ensure that each violation with previous codes also - has corresponding versions changed in their documentation - Forbid incorrectly swapped variables -- Forbids to use `+=` with list arguments - Forbids to use redundant subscripts (e.g., `[0:7]` or `[3:None]`) - Allows `super()` as a valid overused expression - Forbids to use `super()` with other methods and properties @@ -28,12 +25,12 @@ We used to have incremental versioning before `0.1.0`. - Forbids to use `Optional[Union[...]]` in annotations - Forbids unnecessary literals - `WPS525` forbids comparisons where `in` is compared with single item container -- Fixes `BlockVariableVisitor` false positives on a properties - Forbids wrong annotations in assignment - Forbids using multiline `for` and `while` statements - `WPS113` now can be tweaked with `I_CONTROL_CODE` setting - Adds `WPS000` that indicates internal errors - Forbids to use implicit `yield from` +- Forbids to start lines with `.` ### Bugfixes @@ -46,6 +43,7 @@ We used to have incremental versioning before `0.1.0`. - Fixes `BadMagicModuleFunctionViolation` false positives on class-level methods - Fixes `InconsistentReturnViolation` false positives on nested functions - Fixes that `--i-dont-control-code` was not present in command line options +- Fixes `BlockVariableVisitor` false positives on a properties ### Misc @@ -54,6 +52,8 @@ We used to have incremental versioning before `0.1.0`. - Improves violation code testing - Improves testing of `.. versionchanged` and `previous_codes` properties - Reference `isort` settings requirement for compliance with `WSP318` in docstring +- Improves tests: we now ensure that each violation with previous codes also + has corresponding versions changed in their documentation ## 0.12.5 diff --git a/tests/fixtures/noqa.py b/tests/fixtures/noqa.py index 42e6569b6..55f19fe7f 100644 --- a/tests/fixtures/noqa.py +++ b/tests/fixtures/noqa.py @@ -564,7 +564,10 @@ async def async_gen(self): from json import loads # noqa: WPS347 -some_list += [1, 2, 3, 4] # noqa: WPS348 +some_model = ( + MyModel.objects.filter(...) + .exclude(...) # noqa: WPS348 +) swap_a = swap_b swap_b = swap_a # noqa: WPS523 diff --git a/tests/test_violations/test_docs.py b/tests/test_violations/test_docs.py index 837c225f7..e8d39c7e5 100644 --- a/tests/test_violations/test_docs.py +++ b/tests/test_violations/test_docs.py @@ -24,6 +24,12 @@ def test_violation_name(all_violations): assert class_name.endswith('Violation'), class_name +def test_violation_template_ending(all_violations): + """Ensures that all violation templates do not end with a dot.""" + for violation in all_violations: + assert not violation.error_template.endswith('.'), violation + + def test_previous_codes_versionchanged(all_violations): """Tests that we put both in case violation changes.""" for violation in all_violations: diff --git a/tests/test_visitors/test_tokenize/test_keywords/test_keywords_spaces.py b/tests/test_visitors/test_tokenize/test_keywords/test_keywords_spaces.py index b4190d806..e8967894b 100644 --- a/tests/test_visitors/test_tokenize/test_keywords/test_keywords_spaces.py +++ b/tests/test_visitors/test_tokenize/test_keywords/test_keywords_spaces.py @@ -66,7 +66,9 @@ def test_missing_space( """Ensures that parens right after keyword raise a warning.""" file_tokens = parse_tokens(code) - visitor = WrongKeywordTokenVisitor(default_options, file_tokens=file_tokens) + visitor = WrongKeywordTokenVisitor( + default_options, file_tokens=file_tokens, + ) visitor.run() assert_errors(visitor, [MissingSpaceBetweenKeywordAndParenViolation]) @@ -101,7 +103,9 @@ def test_space_between_keyword_and_parens( """Ensures that there's no violation if space in between.""" file_tokens = parse_tokens(code) - visitor = WrongKeywordTokenVisitor(default_options, file_tokens=file_tokens) + visitor = WrongKeywordTokenVisitor( + default_options, file_tokens=file_tokens, + ) visitor.run() assert_errors(visitor, []) diff --git a/tests/test_visitors/test_tokenize/test_keywords/test_starts_with_dot.py b/tests/test_visitors/test_tokenize/test_keywords/test_starts_with_dot.py new file mode 100644 index 000000000..5c6707faa --- /dev/null +++ b/tests/test_visitors/test_tokenize/test_keywords/test_starts_with_dot.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- + +import pytest + +from wemake_python_styleguide.violations.consistency import ( + LineStartsWithDotViolation, +) +from wemake_python_styleguide.visitors.tokenize.keywords import ( + WrongKeywordTokenVisitor, +) + +# Correct: + +correct_dot_attr = """ +some_line = some.attr( + some.other, +) +""" + +correct_elipsis = """ +first[ + 1, + ..., +] +""" + +correct_string_dot = '".start!"' + +# Wrong: + +wrong_dot_start1 = """ +some = ( + MyModel.objects.filter(some=1) + .exclude(other=2) +) +""" + +wrong_dot_start2 = """ +some = ( + MyModel.objects.filter(some=1) +.exclude(other=2) +) +""" + + +@pytest.mark.parametrize('code', [ + wrong_dot_start1, + wrong_dot_start2, +]) +def test_wrong_dot_start( + parse_tokens, + assert_errors, + default_options, + code, +): + """Ensures that lines cannot be started with ``.`` char.""" + file_tokens = parse_tokens(code) + + visitor = WrongKeywordTokenVisitor( + default_options, file_tokens=file_tokens, + ) + visitor.run() + + assert_errors(visitor, [LineStartsWithDotViolation]) + + +@pytest.mark.parametrize('code', [ + correct_dot_attr, + correct_elipsis, + correct_string_dot, +]) +def test_correct_dot_start( + parse_tokens, + assert_errors, + default_options, + code, +): + """Ensures that lines can be started with other chars.""" + file_tokens = parse_tokens(code) + + visitor = WrongKeywordTokenVisitor( + default_options, file_tokens=file_tokens, + ) + visitor.run() + + assert_errors(visitor, []) diff --git a/wemake_python_styleguide/violations/consistency.py b/wemake_python_styleguide/violations/consistency.py index f921735f9..586caf15c 100644 --- a/wemake_python_styleguide/violations/consistency.py +++ b/wemake_python_styleguide/violations/consistency.py @@ -73,7 +73,7 @@ MeaninglessNumberOperationViolation OperationSignNegationViolation VagueImportViolation - AdditionAssignmentOnListViolation + LineStartsWithDotViolation RedundantSubscriptViolation AugmentedAssignPatternViolation UnnecessaryLiteralsViolation @@ -130,7 +130,7 @@ .. autoclass:: MeaninglessNumberOperationViolation .. autoclass:: OperationSignNegationViolation .. autoclass:: VagueImportViolation -.. autoclass:: AdditionAssignmentOnListViolation +.. autoclass:: LineStartsWithDotViolation .. autoclass:: RedundantSubscriptViolation .. autoclass:: AugmentedAssignPatternViolation .. autoclass:: UnnecessaryLiteralsViolation @@ -1803,27 +1803,41 @@ class VagueImportViolation(ASTViolation): @final -class AdditionAssignmentOnListViolation(ASTViolation): +class LineStartsWithDotViolation(TokenizeViolation): """ - Forbids usage of += with list arguments. + Forbids to start lines with a dot. Reasoning: - ``+=`` works like ``extend()`` method. - Why not just use ``extend()`` instead of ``+=`` to be consistent. + We enforce strict consitency rules about how to break lines. + We also enforce strict rules about multi-line parameters. + Starting new lines with the dot means that this rule is broken. + + Solution: + Use ``()`` to break lines in a complex expression. Example:: # Correct: - some_list.extend([1, 2, 3]) + some = MyModel.objects.filter( + ..., + ).exclude( + ..., + ).annotate( + ..., + ) - # Wrong: - some_list += [1, 2, 3] + # Wrong + some = ( + MyModel.objects.filter(...) + .exclude(...) + .annotate(...) + ) .. versionadded:: 0.13.0 """ - error_template = 'Found addition assignment with list argument' + error_template = 'Found a line that starts with a dot' code = 348 @@ -1902,7 +1916,7 @@ class UnnecessaryLiteralsViolation(ASTViolation): """ - error_template = 'Found unnecessary literals.' + error_template = 'Found unnecessary literals' code = 351 @@ -1920,12 +1934,11 @@ class MultilineLoopViolation(ASTViolation): Example:: - # Correct - + # Correct: for num in some_function(arg1, arg2): ... - # Wrong + # Wrong: for num in range( arg1, arg2, @@ -1936,5 +1949,5 @@ class MultilineLoopViolation(ASTViolation): """ - error_template = 'Forbids multiline loops' + error_template = 'Found multiline loop' code = 352 diff --git a/wemake_python_styleguide/visitors/ast/operators.py b/wemake_python_styleguide/visitors/ast/operators.py index 0f54a73e9..1ffdbb082 100644 --- a/wemake_python_styleguide/visitors/ast/operators.py +++ b/wemake_python_styleguide/visitors/ast/operators.py @@ -181,11 +181,6 @@ def visit_AugAssign(self, node: ast.AugAssign) -> None: """ self._check_negation(node.op, node.value) self._check_string_concat(node.value, node.op) - self._check_addition_assignment_on_list( - node.target, - node.op, - node.value, - ) self.generic_visit(node) def _check_negation(self, op: ast.operator, right: ast.AST) -> None: @@ -228,18 +223,3 @@ def _check_string_concat( consistency.ExplicitStringConcatViolation(node), ) return - - def _check_addition_assignment_on_list( - self, - left: ast.AST, - op: ast.operator, - right: ast.AST, - ): - is_addition_assignment_on_list = ( - isinstance(op, ast.Add) and - isinstance(right, ast.List) - ) - if is_addition_assignment_on_list: - self.add_violation( - consistency.AdditionAssignmentOnListViolation(right), - ) diff --git a/wemake_python_styleguide/visitors/tokenize/keywords.py b/wemake_python_styleguide/visitors/tokenize/keywords.py index 7ee727935..baea21d16 100644 --- a/wemake_python_styleguide/visitors/tokenize/keywords.py +++ b/wemake_python_styleguide/visitors/tokenize/keywords.py @@ -6,6 +6,7 @@ from typing_extensions import final from wemake_python_styleguide.violations.consistency import ( + LineStartsWithDotViolation, MissingSpaceBetweenKeywordAndParenViolation, ) from wemake_python_styleguide.visitors.base import BaseTokenVisitor @@ -23,11 +24,28 @@ def visit_name(self, token: tokenize.TokenInfo) -> None: MissingSpaceBetweenKeywordAndParenViolation """ - if keyword.iskeyword(token.string): - self._check_space_before_open_paren(token) + self._check_space_before_open_paren(token) + + def visit_dot(self, token: tokenize.TokenInfo) -> None: + """ + Checks newline related rules. + + Raises: + LineStartsWithDotViolation + + """ + self._check_line_starts_with_dot(token) def _check_space_before_open_paren(self, token: tokenize.TokenInfo) -> None: + if not keyword.iskeyword(token.string): + return + if token.line[token.end[1]:].startswith('('): self.add_violation( MissingSpaceBetweenKeywordAndParenViolation(token), ) + + def _check_line_starts_with_dot(self, token: tokenize.TokenInfo) -> None: + line = token.line.lstrip() + if line.startswith('.') and not line.startswith('...'): + self.add_violation(LineStartsWithDotViolation(token))