Skip to content

Commit

Permalink
Closes #1207
Browse files Browse the repository at this point in the history
  • Loading branch information
sobolevn committed Mar 1, 2020
1 parent dc502f9 commit c5ecc89
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 24 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ Semantic versioning in our case means:
and other builtin functions without keyword arguments
- Fixes `WPS221` reporting differently on different `python` versions
- Fixes `WPS221` reporting nested variable annotations
- Fixes `WPS509` not reporting nested ternary in grandchildren of `if`
- Fixes `WPS509` not reporting nested ternary in ternary

### Misc

Expand Down
40 changes: 36 additions & 4 deletions tests/test_visitors/test_ast/test_compares/test_nested_ternary.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import pytest

from wemake_python_styleguide.compat.constants import PY38
from wemake_python_styleguide.violations.refactoring import (
NestedTernaryViolation,
)
Expand All @@ -15,37 +16,53 @@
wrong_compare4 = '(a if b else c) == x == y'
wrong_compare5 = 'x == (a if b else c) == y'
wrong_compare6 = 'x != (a if b else c)'
wrong_compare7 = 'x != call(a if b else c)'

wrong_boolop1 = 'x and (a if b else c)'
wrong_boolop2 = 'x or (a if b else c)'
wrong_boolop3 = '(a if b else c) or x'
wrong_boolop4 = 'x and (a if b else c) or y'
wrong_boolop5 = 'x and y or (a if b else c)'
wrong_boolop6 = '(a if b else c) and x or y'
wrong_boolop7 = 'call(a if b else c) and x or y'

wrong_binop1 = 'x + (a if b else c)'
wrong_binop2 = 'x - (a if b else c)'
wrong_binop3 = '(a if b else c) / y'
wrong_binop4 = 'x + (a if b else c) - y'
wrong_binop5 = 'x + y - (a if b else c)'
wrong_binop6 = '(a if b else c) * x / y'
wrong_binop7 = 'some(a if b else c) * x / y'

wrong_unary1 = '+(a if b else c)'
wrong_unary2 = '-(a if b else c)'
wrong_unary3 = '~(a if b else c)'
wrong_unary4 = 'not (a if b else c)'

wrong_if = 'if a if b else c: ...'
wrong_ternary1 = 'a if (b if some else c) else d'
wrong_ternary2 = 'a if call(b if some else c) else d'
wrong_ternary3 = 'a if b else (c if some else d)'
wrong_ternary4 = '(a if some else b) if c else d'

wrong_if1 = 'if a if b else c: ...'
wrong_if2 = 'if call(a if b else c): ...'
wrong_if3 = 'if attr.call(a if b else c): ...'
wrong_if4 = 'if x := 1 if True else 2: ...'

# Correct:

correct_if = """
correct_if1 = """
if x:
y = a if b else c
print(a if b else c, end=a if b else c)
d = {'key': a if b else c}
"""

correct_if2 = """
if x:
a if b else c
"""

correct_unary1 = '-a if b else c'
correct_unary2 = 'a if -b else c'
correct_unary3 = 'a if b else -c'
Expand All @@ -65,7 +82,8 @@


@pytest.mark.parametrize('code', [
correct_if,
correct_if1,
correct_if2,
correct_unary1,
correct_unary2,
Expand Down Expand Up @@ -106,27 +124,41 @@ def test_non_nested_ternary(
wrong_compare4,
wrong_compare5,
wrong_compare6,
wrong_binop7,
wrong_boolop1,
wrong_boolop2,
wrong_boolop3,
wrong_boolop4,
wrong_boolop5,
wrong_boolop6,
wrong_boolop7,
wrong_binop1,
wrong_binop2,
wrong_binop3,
wrong_binop4,
wrong_binop5,
wrong_binop6,
wrong_binop7,
wrong_unary1,
wrong_unary2,
wrong_unary3,
wrong_unary4,
wrong_if,
wrong_ternary1,
wrong_ternary2,
wrong_ternary3,
wrong_ternary4,
wrong_if1,
wrong_if2,
wrong_if3,
pytest.param(
wrong_if3,
marks=pytest.mark.skipif(not PY38, reason='walrus appeared in 3.8'),
),
])
def test_nested_ternary(
assert_errors,
Expand Down
11 changes: 0 additions & 11 deletions wemake_python_styleguide/logic/walk.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,6 @@ def get_closest_parent(
parent = get_parent(parent)


def is_child_of(node: ast.AST, parents: _IsInstanceContainer) -> bool:
"""
Checks whether node is inside a given set of parents or not.
Goes up by the tree of ``node`` to check all parents.
Works with general types.
"""
closest_parent = get_closest_parent(node, parents)
return closest_parent is not None


def is_contained_by(node: ast.AST, container: ast.AST) -> bool:
"""
Tells you if a node is contained by a given node.
Expand Down
21 changes: 14 additions & 7 deletions wemake_python_styleguide/visitors/ast/compares.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from wemake_python_styleguide.compat.aliases import AssignNodes
from wemake_python_styleguide.compat.functions import get_assign_targets
from wemake_python_styleguide.logic import nodes, source
from wemake_python_styleguide.logic import nodes, source, walk
from wemake_python_styleguide.logic.naming.name_nodes import is_same_variable
from wemake_python_styleguide.logic.tree import (
compares,
Expand Down Expand Up @@ -297,7 +297,7 @@ class WrongConditionalVisitor(BaseNodeVisitor):
)

_forbidden_expression_parents: ClassVar[AnyNodes] = (
ast.If,
ast.IfExp,
ast.BoolOp,
ast.BinOp,
ast.UnaryOp,
Expand All @@ -318,15 +318,15 @@ def visit_any_if(self, node: AnyIf) -> None:
self._check_simplifiable_if(node)
else:
self._check_simplifiable_ifexpr(node)
self._check_nested_ifexpr(node)

self._check_nested_ifexpr(node)
self._check_constant_condition(node)
self.generic_visit(node)

def _is_simplifiable_assign(
self,
node_body: List[ast.stmt],
) -> Optional[str]:
) -> Optional[str]: # TODO: move to logic at some point
wrong_length = len(node_body) != 1
if wrong_length or not isinstance(node_body[0], AssignNodes):
return None
Expand Down Expand Up @@ -363,9 +363,16 @@ def _check_simplifiable_ifexpr(self, node: ast.IfExp) -> None:
if conditions == {True, False}:
self.add_violation(SimplifiableIfViolation(node))

def _check_nested_ifexpr(self, node: ast.IfExp) -> None:
parent = nodes.get_parent(node)
if isinstance(parent, self._forbidden_expression_parents):
def _check_nested_ifexpr(self, node: AnyIf) -> None:
is_nested_in_if = bool(
isinstance(node, ast.If) and
list(walk.get_subnodes_by_type(node.test, ast.IfExp)),
)
is_nested_poorly = walk.get_closest_parent(
node, self._forbidden_expression_parents,
)

if is_nested_in_if or is_nested_poorly:
self.add_violation(NestedTernaryViolation(node))


Expand Down
4 changes: 2 additions & 2 deletions wemake_python_styleguide/visitors/ast/complexity/nested.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from wemake_python_styleguide.compat.aliases import FunctionNodes
from wemake_python_styleguide.constants import NESTED_FUNCTIONS_WHITELIST
from wemake_python_styleguide.logic.nodes import get_context, get_parent
from wemake_python_styleguide.logic.walk import is_child_of
from wemake_python_styleguide.logic.walk import get_closest_parent
from wemake_python_styleguide.types import AnyFunctionDef
from wemake_python_styleguide.violations.best_practices import (
NestedClassViolation,
Expand Down Expand Up @@ -93,7 +93,7 @@ def _check_nested_classes(self, node: ast.ClassDef) -> None:

def _check_nested_lambdas(self, node: ast.Lambda) -> None:
is_direct = isinstance(get_context(node), ast.Lambda)
is_deep = is_child_of(node, ast.Lambda)
is_deep = get_closest_parent(node, ast.Lambda)

if is_direct or is_deep:
self.add_violation(NestedFunctionViolation(node, text='lambda'))

0 comments on commit c5ecc89

Please sign in to comment.