feat: forbid confusing syntax (WPS364); add visitor and tests (#3516)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: djeada

tests/fixtures/noqa/noqa.py

@@ -765,3 +765,6 @@ async def test_await_in_loop():
     my_print('zero division error')
     my_print(3 / 3)
     my_print('no zero division error')
+
+if not user in users:  # noqa: WPS364
+    my_print('legacy not-in style')

tests/test_checker/test_noqa.py

@@ -165,6 +165,7 @@
         'WPS361': 0,  # disabled since 1.0.0
         'WPS362': 2,
         'WPS363': 1,
+        'WPS364': 1,
         'WPS400': 0,  # defined in ignored violations.
         'WPS401': 0,  # logically unacceptable.
         'WPS402': 0,  # defined in ignored violations.

tests/test_visitors/test_ast/test_compares/test_not_in_unary.py

@@ -0,0 +1,61 @@
+import pytest
+
+from wemake_python_styleguide.violations.consistency import (
+    NotInWithUnaryOpViolation,
+)
+from wemake_python_styleguide.visitors.ast.compares import (
+    NotInUnaryVisitor,
+)
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        'if not x in items: ...',
+        'if not (x in items): ...',
+        'result = not value in data',
+        'flag = not (value in data)',
+        'while not a in b: ...',
+    ],
+)
+def test_not_in_unary_detects(
+    assert_errors,
+    parse_ast_tree,
+    default_options,
+    code,
+):
+    """Ensures that ``not a in b`` forms are reported."""
+    tree = parse_ast_tree(code)
+
+    visitor = NotInUnaryVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [NotInWithUnaryOpViolation])
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        'if x not in items: ...',
+        'result = x not in data',
+        'if (not x) in items: ...',
+        'if not_called() in seq: ...',
+        'if y in items: ...',
+        'if not a is b: ...',
+        'if not (a is b): ...',
+        'if not (x in y in z): ...',
+    ],
+)
+def test_not_in_unary_ok(
+    assert_errors,
+    parse_ast_tree,
+    default_options,
+    code,
+):
+    """Ensures that correct forms are not reported."""
+    tree = parse_ast_tree(code)
+
+    visitor = NotInUnaryVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])

wemake_python_styleguide/presets/types/tree.py

@@ -65,6 +65,7 @@
     compares.WrongConstantCompareVisitor,
     compares.InCompareSanityVisitor,
     compares.WrongFloatComplexCompareVisitor,
+    compares.NotInUnaryVisitor,
     conditions.IfStatementVisitor,
     conditions.BooleanConditionVisitor,
     conditions.MatchVisitor,

wemake_python_styleguide/violations/consistency.py

@@ -2381,3 +2381,37 @@ class RaiseSystemExitViolation(ASTViolation):
 
     error_template = 'Found `raise SystemExit`, instead of using `sys.exit`'
     code = 363
+
+
+@final
+class NotInWithUnaryOpViolation(ASTViolation):
+    """
+    Forbid using ``not a in b`` instead of ``a not in b``.
+
+    Reasoning:
+        The expression ``not a in b`` is parsed as ``not (a in b)`` and is
+        equivalent to ``a not in b``. However, it is easy to misread as
+        ``(not a) in b`` or require extra parsing effort. Using the explicit
+        ``not in`` operator is clearer and idiomatic.
+
+    Solution:
+        Replace ``not a in b`` (or ``not (a in b)``) with ``a not in b``.
+
+    Example::
+
+        # Correct:
+        if user not in users:
+            ...
+
+        # Wrong:
+        if not user in users:
+            ...
+        if not (user in users):
+            ...
+
+    .. versionadded:: 1.5.0
+
+    """
+
+    error_template = 'Found legacy `not ... in`, use `... not in` instead'
+    code = 364

wemake_python_styleguide/visitors/ast/compares.py

@@ -17,6 +17,7 @@
     ConstantCompareViolation,
     ConstantConditionViolation,
     MultipleInCompareViolation,
+    NotInWithUnaryOpViolation,
     ReversedComplexCompareViolation,
 )
 from wemake_python_styleguide.violations.refactoring import (
@@ -237,3 +238,33 @@ def _is_float_or_complex(self, node: ast.AST) -> bool:
             node.value,
             float | complex,
         )
+
+
+@final
+class NotInUnaryVisitor(BaseNodeVisitor):
+    """Forbids ``not a in b`` which should be written as ``a not in b``."""
+
+    def visit_UnaryOp(self, node: ast.UnaryOp) -> None:
+        """
+        Detects ``not (a in b)`` pattern syntactically.
+
+        Python parses both ``not a in b`` and ``not (a in b)`` as a
+        ``UnaryOp(Not, Compare(left, In, right))``; we catch this shape
+        and suggest ``a not in b`` instead.
+        """
+        self._check_legacy_not_in(node)
+        self.generic_visit(node)
+
+    def _check_legacy_not_in(self, node: ast.UnaryOp) -> None:
+        if not isinstance(node.op, ast.Not):
+            return
+        if not isinstance(node.operand, ast.Compare):
+            return
+
+        comp = node.operand
+        if len(comp.ops) != 1:
+            return
+        if not isinstance(comp.ops[0], ast.In):
+            return
+
+        self.add_violation(NotInWithUnaryOpViolation(node))