Issue 244 (#260)

* Change adds conditional visitor that verifies that if statement isn't using True, False, None as it's case

* Change updates the error message for using NamedConstants

* Change adds ChangeLog feature line

* Change removes unnecessary line in test

* Change implements PR feedback.

This adds extra checks for Num, List, Set or Str types used in either an If or IfExp node in the AST.

* Change implements PR feedback and merges two functions and adds Dict type to not supported in conditional

CHANGELOG.md

@@ -28,6 +28,7 @@ to the project during `#hactoberfest`. List of awesome people:
 - Forbids too many arguments in `lambda` functions
 - Forbid `for` loops with unused `else`
 - Forbid `try` with `finally` without `except`
+- Forbit `if` statements with invalid conditionals
 - Forbid opening parenthesis from following keyword without space in between them
 
 ### Bugfixes

tests/test_visitors/test_ast/test_comparisons/test_conditionals.py

@@ -0,0 +1,79 @@
+# -*- coding: utf-8 -*-
+
+import pytest
+
+from wemake_python_styleguide.violations.consistency import (
+    WrongConditionalViolation,
+)
+from wemake_python_styleguide.visitors.ast.comparisons import (
+    WrongConditionalVisitor,
+)
+
+create_variable = """
+variable = 1
+{0}
+"""
+
+if_statement = 'if {0}: ...'
+ternary = 'ternary = 0 if {0} else 1'
+if_statement_in_comprehension = '[x for x in [1, 2, 3] if {0}]'
+
+
+@pytest.mark.parametrize('code', [
+    if_statement,
+    ternary,
+    if_statement_in_comprehension,
+])
+@pytest.mark.parametrize('comparators', [
+    'variable < 3',
+    'variable',
+    'variable is True',
+    'variable is False',
+    '[1,2,3].size > 3',
+    'variable is None',
+    'variable is int or not None',
+])
+def test_valid_conditional(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    comparators,
+    default_options,
+):
+    """Testing that conditionals work well."""
+    tree = parse_ast_tree(create_variable.format(code.format(comparators)))
+
+    visitor = WrongConditionalVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])
+
+
+@pytest.mark.parametrize('code', [
+    if_statement,
+    ternary,
+])
+@pytest.mark.parametrize('comparators', [
+    'True',
+    'False',
+    'None',
+    '{variable}',
+    '[variable]',
+    '4',
+    '"test"',
+    '{test : "1"}',
+])
+def test_redundant(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    comparators,
+    default_options,
+):
+    """Testing that violations are when using invalid conditional."""
+    tree = parse_ast_tree(create_variable.format(code.format(comparators)))
+
+    visitor = WrongConditionalVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [WrongConditionalViolation])

wemake_python_styleguide/types.py

@@ -28,7 +28,10 @@
 #: In cases we need to work with all function definitions (including lambdas).
 AnyFunctionDefAndLambda = Union[AnyFunctionDef, ast.Lambda]
 
-#: Flake8 API format to return error messages.
+#: In cases we need to work with both forms of if functions
+AnyIf = Union[ast.If, ast.IfExp]
+
+#: Flake8 API format to return error messages:
 CheckResult = Tuple[int, int, str, type]
 
 #: Tuple of AST node types for declarative syntax.

wemake_python_styleguide/violations/consistency.py

@@ -44,6 +44,7 @@
    ComparisonOrderViolation
    MultipleInComparisonViolation
    RedundantComparisonViolation
+   WrongConditionalViolation
    MissingSpaceBetweenKeywordAndParenViolation
 
 Consistency checks
@@ -62,8 +63,8 @@
 .. autoclass:: ComparisonOrderViolation
 .. autoclass:: MultipleInComparisonViolation
 .. autoclass:: RedundantComparisonViolation
+.. autoclass:: WrongConditionalViolation
 .. autoclass:: MissingSpaceBetweenKeywordAndParenViolation
-
 """
 
 from wemake_python_styleguide.types import final
@@ -505,7 +506,6 @@ class RedundantComparisonViolation(ASTViolation):
 
     Note:
         Returns Z312 as error code
-
     """
 
     should_use_text = False
@@ -542,10 +542,39 @@ def func():
 
     Note:
         Returns Z313 as error code
-
     """
 
     should_use_text = False
     #: Error message shown to the user.
     error_template = 'Found paren right after a keyword'
     code = 313
+
+
+class WrongConditionalViolation(ASTViolation):
+    """
+    Forbids using `if` statements that use invalid conditionals.
+
+    Reasoning:
+        When invalid conditional arguments are used
+        it is typically an indication of a mistake, since
+        the value of the conditional result will always be the same.
+
+    Solution:
+        Remove the conditional and any associated dead code.
+
+    Example::
+
+        # Correct:
+        if value is True: ...
+
+        # Wrong:
+        if True: ...
+
+    Note:
+        Returns Z314 as error code
+    """
+
+    should_use_text = False
+    #: Error message shown to the user.
+    error_template = 'Conditional always evaluates to same result'
+    code = 314

wemake_python_styleguide/visitors/ast/comparisons.py

@@ -5,14 +5,16 @@
 
 from wemake_python_styleguide.logics.nodes import is_literal
 from wemake_python_styleguide.logics.variables import is_same_variable
-from wemake_python_styleguide.types import AnyNodes, final
+from wemake_python_styleguide.types import AnyIf, AnyNodes, final
 from wemake_python_styleguide.violations.consistency import (
     ComparisonOrderViolation,
     ConstantComparisonViolation,
     MultipleInComparisonViolation,
     RedundantComparisonViolation,
+    WrongConditionalViolation,
 )
 from wemake_python_styleguide.visitors.base import BaseNodeVisitor
+from wemake_python_styleguide.visitors.decorators import alias
 
 
 @final
@@ -138,3 +140,35 @@ def visit_Compare(self, node: ast.Compare) -> None:
         """
         self._check_ordering(node)
         self.generic_visit(node)
+
+
+@alias('visit_any_if', (
+    'visit_If',
+    'visit_IfExp',
+))
+class WrongConditionalVisitor(BaseNodeVisitor):
+    """Finds wrong conditional arguments."""
+
+    _forbidden_nodes: ClassVar[AnyNodes] = (
+        ast.List,
+        ast.Set,
+        ast.Num,
+        ast.NameConstant,
+        ast.Str,
+        ast.Dict,
+    )
+
+    def visit_any_if(self, node: AnyIf) -> None:
+        """
+        Ensures that if statements are using valid conditionals.
+
+        Raises:
+            WrongConditionalViolation
+
+        """
+        self._check_if_statement_conditional(node)
+        self.generic_visit(node)
+
+    def _check_if_statement_conditional(self, node: AnyIf) -> None:
+        if isinstance(node.test, self._forbidden_nodes):
+            self.add_violation(WrongConditionalViolation(node))

wemake_python_styleguide/visitors/presets/general.py

@@ -37,6 +37,7 @@
     numbers.MagicNumberVisitor,
     WrongStringVisitor,
 
+    comparisons.WrongConditionalVisitor,
     comparisons.ComparisonSanityVisitor,
     comparisons.WrongComparisionOrderVisitor,