Fixes wemake-services#3427

Author: MIXgleb

CHANGELOG.md

@@ -24,6 +24,7 @@ Semantic versioning in our case means:
 - Adds `WPS478`: forbids using non strict slice operations, #1011
 - Adds `WPS479`: forbids using multiline fstrings, #3405
 - Adds `WPS480`: forbids using comments inside formatted string, #3404
+- Adds `WPS481`: forbids using complex slice indexes, #3427
 
 ### Bugfixes
 

tests/fixtures/noqa/noqa.py

@@ -756,3 +756,6 @@ async def test_await_in_loop():
 
 some_sequence = some_sequence[::-1]  # noqa: WPS478
 
+array = array[my_dict["index"] :]  # noqa: WPS481
+array = array[: end_index()]  # noqa: WPS481
+array = array[:: (var_x * (var_y + 1))]  # noqa: WPS481

tests/test_checker/test_noqa.py

@@ -245,6 +245,7 @@
         'WPS478': 1,
         'WPS479': 0,
         'WPS480': 0,  # only triggers on 3.12+
+        'WPS481': 3,
         'WPS500': 1,
         'WPS501': 1,
         'WPS502': 0,  # disabled since 1.0.0

tests/test_visitors/test_ast/test_subscripts/test_slice_indexes.py

@@ -0,0 +1,78 @@
+import pytest
+
+from wemake_python_styleguide.violations.best_practices import (
+    ComplexSliceIndexViolation,
+)
+from wemake_python_styleguide.visitors.ast.subscripts import SliceIndexVisitor
+
+usage_template = 'constant[{0}]'
+
+
+@pytest.mark.parametrize(
+    'expression',
+    [
+        'my_dict["start_index"]:',
+        '::my_dict["step"]',
+        'my_list[0]:',
+        '::my_list[-1]',
+        'start_index():',
+        '::step()',
+        'Slice().start:',
+        'Slice(arg_1).start:',
+        'index.start():',
+        ':-(-x + 1)',
+        ':-(-x + -y)',
+        ':-(-x + -(y + 1))',
+        ':-(-(x + 1) + -(y + 1))',
+        '(1 - (x + 2)):',
+        '(1 - (x + 2) * 2):',
+        '(index.start + 1):',
+        '(x + y + z):',
+        ':x + y + z',
+    ],
+)
+def test_invalid_slice_indexes(
+    assert_errors,
+    parse_ast_tree,
+    expression,
+    default_options,
+):
+    """Testing that invalid indexes are forbidden."""
+    tree = parse_ast_tree(usage_template.format(expression))
+
+    visitor = SliceIndexVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [ComplexSliceIndexViolation])
+
+
+@pytest.mark.parametrize(
+    'expression',
+    [
+        '1:3',
+        '1:3:2',
+        'start:end',
+        '::step',
+        'index.start:',
+        'index.start::index.step',
+        'Slice.start:',
+        ':-start',
+        '(x + 1):',
+        '(-x + 1):',
+        ':-(x + 1)',
+        '::-index.step',
+    ],
+)
+def test_valid_slice_indexes(
+    assert_errors,
+    parse_ast_tree,
+    expression,
+    default_options,
+):
+    """Testing that valid indexes are allowed."""
+    tree = parse_ast_tree(usage_template.format(expression))
+
+    visitor = SliceIndexVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])

wemake_python_styleguide/presets/types/tree.py

@@ -75,6 +75,7 @@
     subscripts.ImplicitDictGetVisitor,
     subscripts.CorrectKeyVisitor,
     subscripts.StrictSliceOperations,
+    subscripts.SliceIndexVisitor,
     decorators.WrongDecoratorVisitor,
     redundancy.RedundantEnumerateVisitor,
     pm.MatchSubjectVisitor,

wemake_python_styleguide/violations/base.py

@@ -251,5 +251,6 @@ def _location(self) -> tuple[int, int]:
 
 def _prepend_skipping_whitespaces(prefix: str, text: str) -> str:
     lstripped_text = text.lstrip()
-    leading_whitespaces = text[: len(text) - len(lstripped_text)]
+    end_index = len(text) - len(lstripped_text)
+    leading_whitespaces = text[:end_index]
     return leading_whitespaces + prefix + lstripped_text

wemake_python_styleguide/violations/best_practices.py

@@ -97,6 +97,7 @@
    NonStrictSliceOperationsViolation
    MultilineFormattedStringViolation
    CommentInFormattedStringViolation
+   ComplexSliceIndexViolation
 
 Best practices
 --------------
@@ -182,6 +183,7 @@
 .. autoclass:: NonStrictSliceOperationsViolation
 .. autoclass:: MultilineFormattedStringViolation
 .. autoclass:: CommentInFormattedStringViolation
+.. autoclass:: ComplexSliceIndexViolation
 
 """
 
@@ -3150,3 +3152,42 @@ class CommentInFormattedStringViolation(TokenizeViolation):
 
     error_template = 'Found comment inside formatted string'
     code = 480
+
+
+@final
+class ComplexSliceIndexViolation(ASTViolation):
+    """
+    Forbid using complex grammar for slice index.
+
+    Reasoning:
+        It is probably not a good idea to use complex grammar in slice indexes.
+        Because indexes should be simple and easy to read.
+
+    Solution:
+        Use names, constants and attributes as indexes only.
+        Without complex expressions in parentheses.
+
+    Example::
+
+        # Correct:
+        array[3:7]
+        array[:-(x + 3)]
+        array[::-index.step]
+        array[start_index:end_index]
+        array[index.start:index.end]
+        array[Slice.start:]
+
+        # Wrong:
+        array[my_dict["start_index"]:my_list[-1]]
+        array[start_index():index.end()]
+        array[Slice().start:]
+        array[-(-x + 1):]
+        array[(x * (y + 2)):]
+        array[(x + y + z):]
+
+    .. versionadded:: 1.2.0
+
+    """
+
+    error_template = 'Found complex slice index'
+    code = 481

wemake_python_styleguide/visitors/ast/subscripts.py

@@ -1,15 +1,42 @@
 import ast
-from typing import ClassVar, final
+from typing import ClassVar, Final, final
 
 from wemake_python_styleguide.logic import source
-from wemake_python_styleguide.logic.tree import functions, operators, slices
+from wemake_python_styleguide.logic.tree import (
+    attributes,
+    functions,
+    operators,
+    slices,
+)
+from wemake_python_styleguide.types import AnyNodes
 from wemake_python_styleguide.violations import (
     best_practices,
     consistency,
     refactoring,
 )
 from wemake_python_styleguide.visitors import base
 
+_ALLOWED_BINOP_TYPES: Final = (
+    ast.Name,
+    ast.Constant,
+    ast.UnaryOp,
+)
+
+_ALLOWED_UNARYOP_TYPES: Final = (
+    ast.Name,
+    ast.Constant,
+    ast.BinOp,
+    ast.Attribute,
+)
+
+_ALLOWED_INDEX_TYPES: Final = (
+    ast.Name,
+    ast.Constant,
+    ast.BinOp,
+    ast.UnaryOp,
+    ast.Attribute,
+)
+
 
 @final
 class SubscriptVisitor(base.BaseNodeVisitor):
@@ -252,3 +279,67 @@ def _is_node_have_value(
             )
 
         return isinstance(node, ast.Constant) and node.value == value_to_check
+
+
+@final
+class SliceIndexVisitor(base.BaseNodeVisitor):
+    """Checks slices used in the code."""
+
+    def visit_Subscript(self, node: ast.Subscript) -> None:
+        """Visits slice."""
+        if not isinstance(node.slice, ast.Slice):
+            return
+
+        self._check_slice_complexity(node.slice)
+        self.generic_visit(node.slice)
+
+    def _check_slice_complexity(self, node: ast.Slice) -> None:
+        for index in (node.lower, node.upper, node.step):
+            if index is None:
+                continue
+
+            if not self._is_allowed_index(index, _ALLOWED_INDEX_TYPES):
+                self.add_violation(
+                    best_practices.ComplexSliceIndexViolation(index)
+                )
+
+    def _is_allowed_index(
+        self,
+        node: ast.expr,
+        allowed_types: AnyNodes,
+    ) -> bool:
+        for part in attributes.parts(node):
+            if isinstance(part, ast.UnaryOp) and not self._is_allowed_unaryop(
+                part, allowed_types
+            ):
+                return False
+
+            if isinstance(part, ast.BinOp) and not self._is_allowed_binop(
+                part, allowed_types
+            ):
+                return False
+
+            if not isinstance(part, allowed_types):
+                return False
+
+        return True
+
+    def _is_allowed_unaryop(
+        self,
+        node: ast.UnaryOp,
+        allowed_types: AnyNodes,
+    ) -> bool:
+        if allowed_types is _ALLOWED_INDEX_TYPES:
+            return self._is_allowed_index(node.operand, _ALLOWED_UNARYOP_TYPES)
+        return self._is_allowed_index(node.operand, allowed_types)
+
+    def _is_allowed_binop(
+        self, node: ast.BinOp, allowed_types: AnyNodes
+    ) -> bool:
+        if allowed_types is _ALLOWED_INDEX_TYPES:
+            return self._is_allowed_index(
+                node.left, _ALLOWED_BINOP_TYPES
+            ) and self._is_allowed_index(node.right, _ALLOWED_BINOP_TYPES)
+        return self._is_allowed_index(
+            node.left, allowed_types
+        ) and self._is_allowed_index(node.right, allowed_types)