Issue 1011: Non strict operation with slice (#3373)

Author: skv0zsneg

CHANGELOG.md

@@ -16,6 +16,12 @@ Semantic versioning in our case means:
   But, in the future we might change the configuration names/logic,
   change the client facing API, change code conventions significantly, etc.
 
+## 1.2.0 WIP
+
+### Features
+
+- Adds `WPS478`: forbids using non strict slice operations, #1011
+
 ## 1.1.1
 
 ### Bugfixes

tests/fixtures/noqa/noqa.py

@@ -752,3 +752,6 @@ def pos_only_problem(first_argpm=0, second_argpm=1, /):  # noqa: WPS475
 async def test_await_in_loop():
     for _ in range(10):
         await test_function()  # noqa: WPS476
+
+
+some_sequence = some_sequence[::-1]  # noqa: WPS478

tests/test_checker/test_noqa.py

@@ -242,6 +242,7 @@
         'WPS475': 1,
         'WPS476': 1,
         'WPS477': 0,  # enabled only in python 3.13+
+        'WPS478': 1,
         'WPS500': 1,
         'WPS501': 1,
         'WPS502': 0,  # disabled since 1.0.0

tests/test_visitors/test_ast/test_subscripts/test_stricter_slice_operations.py

@@ -0,0 +1,110 @@
+import pytest
+
+from wemake_python_styleguide.violations.best_practices import (
+    NonStrictSliceOperationsViolation,
+)
+from wemake_python_styleguide.visitors.ast.subscripts import (
+    StrictSliceOperations,
+)
+
+
+@pytest.mark.parametrize(
+    'expression',
+    [
+        # reverse
+        'items[::-1]',
+        'items[None::-1]',
+        'items[:None:-1]',
+        'items[None:None:-1]',
+        'items[-1::-1]',
+        'items[-1:None:-1]',
+        # copy
+        'items[:]',
+        'items[None:]',
+        'items[:None]',
+        'items[None:None]',
+        'items[::]',
+        'items[None::]',
+        'items[:None:]',
+        'items[::None]',
+        'items[None:None:]',
+        'items[None::None]',
+        'items[:None:None]',
+        'items[None:None:None]',
+        'items[::1]',
+        'items[None::1]',
+        'items[:None:1]',
+        'items[None:None:1]',
+        'items[0:]',
+        'items[0:None]',
+        'items[0::]',
+        'items[0:None:]',
+        'items[0::None]',
+        'items[0:None:None]',
+        'items[0::1]',
+        'items[0:None:1]',
+    ],
+)
+def test_non_strict_slice_operation_bad(
+    assert_errors,
+    parse_ast_tree,
+    expression,
+    default_options,
+):
+    """Testing for using non strict slice operations."""
+    tree = parse_ast_tree(expression)
+
+    visitor = StrictSliceOperations(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [NonStrictSliceOperationsViolation])
+
+
+@pytest.mark.parametrize(
+    'expression',
+    [
+        'items[0]',
+        'items[:x]',
+        'items[:2]',
+        'items[2:]',
+        'items[:-1]',
+        'items[2::]',
+        'items[2:2]',
+        'items[:-x]',
+        'items[::x]',
+        'items[::-x]',
+        'items[0:-1]',
+        'items[:-1:]',
+        'items[2::2]',
+        'items[1::-1]',
+        'items[None:x]',
+        'items[:None:x]',
+        'items[None:-1]',
+        'items[1:4:-1]',
+        'items[None:-x]',
+        'items[None::x]',
+        'items[0:-1:1]',
+        'items[:None:-x]',
+        'items[None:-1:]',
+        'items[:-1:None]',
+        'items[2:None:2]',
+        'items[0:-1:None]',
+        'items[None::-x]',
+        'items[1:None:-1]',
+        'items[None:None:x]',
+        'items[None:-1:None]',
+    ],
+)
+def test_non_strict_slice_operation_good(
+    assert_errors,
+    parse_ast_tree,
+    expression,
+    default_options,
+):
+    """Testing for using non strict slice operations."""
+    tree = parse_ast_tree(expression)
+
+    visitor = StrictSliceOperations(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])

wemake_python_styleguide/presets/types/tree.py

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

wemake_python_styleguide/violations/best_practices.py

@@ -94,6 +94,7 @@
    ProblematicFunctionParamsViolation
    AwaitInLoopViolation
    SneakyTypeVarWithDefaultViolation
+   NonStrictSliceOperationsViolation
 
 Best practices
 --------------
@@ -176,6 +177,7 @@
 .. autoclass:: ProblematicFunctionParamsViolation
 .. autoclass:: AwaitInLoopViolation
 .. autoclass:: SneakyTypeVarWithDefaultViolation
+.. autoclass:: NonStrictSliceOperationsViolation
 
 """
 
@@ -3042,3 +3044,33 @@ class Class[T=int]:
 
     error_template = 'Found a TypeVarTuple following a TypeVar with default'
     code = 477
+
+
+@final
+class NonStrictSliceOperationsViolation(ASTViolation):
+    """
+    Forbid using non strict slice operations.
+
+    Reasoning:
+        We have two ways to do something.
+
+    Solution:
+        Prefer a more descriptive way.
+
+    Example::
+
+        # Correct:
+        items.reverse()
+        ''.join(reversed('abc'))
+        items.copy()
+
+        # Wrong:
+        items[::-1]  # `.reverse()` or `reversed()`
+        items[:]  # `.copy()` or `copy.copy()`
+
+    .. versionadded:: 1.2.0
+
+    """
+
+    error_template = 'Found non strict slice operation'
+    code = 478

wemake_python_styleguide/visitors/ast/subscripts.py

@@ -182,3 +182,69 @@ def _is_float_key(self, node: ast.expr) -> bool:
         return isinstance(real_node, ast.Constant) and isinstance(
             real_node.value, float
         )
+
+
+@final
+class StrictSliceOperations(base.BaseNodeVisitor):
+    """Check for stricter operation with slices."""
+
+    def visit_Slice(self, node: ast.Slice) -> None:
+        """Visit slice."""
+        self._check_reverse(node)
+        self._check_copy(node)
+        self.generic_visit(node)
+
+    def _check_reverse(self, node: ast.Slice) -> None:
+        if not (
+            self._is_node_or_none(node.lower)
+            or self._is_node_have_value(node.lower, value_to_check=-1)
+        ):
+            return
+
+        if not (
+            self._is_node_or_none(node.upper)
+            and self._is_node_have_value(node.step, value_to_check=-1)
+        ):
+            return
+
+        self.add_violation(
+            best_practices.NonStrictSliceOperationsViolation(node)
+        )
+
+    def _check_copy(self, node: ast.Slice) -> None:
+        if not (
+            self._is_node_or_none(node.lower)
+            or self._is_node_have_value(node.lower, value_to_check=0)
+        ):
+            return
+
+        if not self._is_node_or_none(node.upper):
+            return
+
+        if not (
+            self._is_node_or_none(node.step)
+            or self._is_node_have_value(node.step, value_to_check=1)
+        ):
+            return
+
+        self.add_violation(
+            best_practices.NonStrictSliceOperationsViolation(node)
+        )
+
+    def _is_node_or_none(self, node: ast.AST | None) -> bool:
+        return node is None or (
+            isinstance(node, ast.Constant) and node.value is None
+        )
+
+    def _is_node_have_value(
+        self, node: ast.AST | None, value_to_check: int
+    ) -> bool:
+        if value_to_check < 0:
+            return (
+                isinstance(node, ast.UnaryOp)
+                and isinstance(node.op, ast.USub)
+                and isinstance(node.operand, ast.Constant)
+                and node.operand.value == abs(value_to_check)
+            )
+
+        return isinstance(node, ast.Constant) and node.value == value_to_check