Skip to content

Commit

Permalink
Fixes bug with InfiniteWhileLoopViolation #1857 (#1869)
Browse files Browse the repository at this point in the history 
* Fixes bug with InfiniteWhileLoopViolation #1857

* Closes #1872

* Closes #1871

* Fixes CI

* Fixes CI

* Fixes CI

Co-authored-by: sobolevn <mail@sobolevn.me>
  • Loading branch information
@Aleksey94Dan @sobolevn
Aleksey94Dan and sobolevn authored Feb 12, 2021
1 parent 1f94c39 commit b9e73c3
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 59 deletions.
30 changes: 24 additions & 6 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@ We follow [Semantic Versions](https://semver.org/) since the `0.1.0` release.
We used to have incremental versioning before `0.1.0`.

Semantic versioning in our case means:
- Bugfixes do not bring new features, code that passes on `x.y.0` should pass on `x.y.1`. With the only exception that bugfix can raise old violations in new places, if they were hidden by a buggy behaviour. But we do not add new checks.
- Minor releases do bring new features and configuration options. New violations can be added. Code that passes `x.0.y` might not pass on `x.1.y` release because of the new checks.
- Major releases inidicate significant milestones or serious breaking changes. There are no major releases right now: we are still at `0.x.y` version. But, in the future we might change the configuration names / logic, change the client facing API, change code conventions signigicantly, etc.
- Bugfixes do not bring new features, code that passes on `x.y.0`
should pass on `x.y.1`.
With the only exception that bugfix can raise old violations in new places,
if they were hidden by a buggy behaviour. But we do not add new checks.
- Minor releases do bring new features and configuration options.
New violations can be added. Code that passes on `x.0.y`
might not pass on `x.1.y` release because of the new checks.
- Major releases inidicate significant milestones or serious breaking changes.
There are no major releases right now: we are still at `0.x.y` version.
But, in the future we might change the configuration names / logic,
change the client facing API, change code conventions signigicantly, etc.


## 0.15.1
Expand All @@ -19,10 +27,18 @@ Semantic versioning in our case means:
- Fixes multiple `if` support for `InconsistentComprehensionViolation`
- Fixes that `NestedTernaryViolation` was not reported for a comprehension
- Fixes that `ConstantConditionViolation` was not reported for a comprehension
- Fixes that `ConstantConditionViolation` was triggering for `while x := True:`
- Fixes that `UselessElseViolation` was not reported
for `for`, `while`, and `try` keywords
- Fixes false positive `InfiniteWhileLoopViolation` for `try` #1857
- Fixes that `InfiniteWhileLoopViolation` was not triggered on `1`
or other truthy nodes

### Misc

- Refactors how `tokenize` tests are executed, now we have an option to compile fixture code to make sure it is syntatically valid
- Refactors how `tokenize` tests are executed,
now we have an option to compile fixture code
to make sure it is syntatically valid


## 0.15.0 aka python3.9
Expand All @@ -48,7 +64,8 @@ Semantic versioning in our case means:
- Forbids to use float zeros (`0.0`)
- Forbids `raise Exception` and `raise BaseException`
- Forbids to use `%` with zero as the divisor
- WPS531: Forbids testing conditions to just return booleans when it is possible to simply return the condition itself
- Forbids testing conditions to just return booleans
when it is possible to simply return the condition itself
- Forbids to use unsafe infinite loops
- Forbids to use raw strings `r''` when not necessary
- Forbids to use too complex `f`-strings
Expand Down Expand Up @@ -91,7 +108,8 @@ Semantic versioning in our case means:
- Updates lots of dependenices
- Fixed documentation for TooManyPublicAttributesViolation
- Updated isort config
- Introduce helper script to check for missing calls to `self.generic_visit(node)` in AST visitors
- Introduce helper script to check
for missing calls to `self.generic_visit(node)` in AST visitors
- Updates `poetry` version to `1.1`
- Updates `reviewdog` version to `0.11.0` and adds `action-depup`

Expand Down
52 changes: 45 additions & 7 deletions tests/test_visitors/test_ast/test_keywords/test_keyword_condition.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest

from wemake_python_styleguide.compat.constants import PY38
from wemake_python_styleguide.violations.best_practices import (
WrongKeywordConditionViolation,
)
Expand Down Expand Up @@ -32,6 +33,7 @@
'[]',
'()',
'{}', # noqa: P103
'+True',
'False',
'None',
'1',
Expand All @@ -45,6 +47,14 @@
'{1 for a in some()}',
'{"a": a for a in some()}',
'some if x else other',
pytest.param(
'(unique := +0)',
marks=pytest.mark.skipif(not PY38, reason='walrus appeared in 3.8'),
),
pytest.param(
'(unique := +True)',
marks=pytest.mark.skipif(not PY38, reason='walrus appeared in 3.8'),
),
])
def test_false_condition_keywords(
assert_errors,
Expand Down Expand Up @@ -85,12 +95,7 @@ def test_false_assert_condition_keywords(
assert_errors(visitor, [WrongKeywordConditionViolation])


@pytest.mark.parametrize('code', [
while_template,
assert_template,
assert_with_message_template,
])
@pytest.mark.parametrize('condition', [
fixtures = (
'call()',
'name',
'-name',
Expand All @@ -101,8 +106,41 @@ def test_false_assert_condition_keywords(
'value + 1',
'x > 1',
'x + 1 > 1 and y < 1',
)


@pytest.mark.parametrize('code', [
while_template,
])
@pytest.mark.parametrize('condition', [
*fixtures,
pytest.param(
'x := other()',
marks=pytest.mark.skipif(not PY38, reason='walrus appeared in 3.8'),
),
])
def test_true_condition_keywords_while(
assert_errors,
parse_ast_tree,
code,
condition,
default_options,
):
"""Testing that true coniditions in keywords are allowed."""
tree = parse_ast_tree(code.format(condition))

visitor = ConstantKeywordVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [])


@pytest.mark.parametrize('code', [
assert_template,
assert_with_message_template,
])
def test_true_condition_keywords(
@pytest.mark.parametrize('condition', fixtures)
def test_true_condition_keywords_assert(
assert_errors,
parse_ast_tree,
code,
Expand Down
132 changes: 110 additions & 22 deletions tests/test_visitors/test_ast/test_loops/test_loops/test_infinite_while_loops.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

template_nested_while2 = """
while other:
while True:
while +1:
{0}
{0}
"""
Expand All @@ -45,11 +45,81 @@ def wrapper():

template_double_while = """
while True:
while True:
while 'inf':
{0}
{1}
"""

# Correct

correct_while1 = """
while 1:
try:
...
except:
...
"""

correct_while2 = """
while other:
while [1, 2, 3]:
try:
...
except:
...
"""


correct_while3 = """
def wrapper():
while True:
try:
...
except:
...
"""

correct_while4 = """
while other:
...
"""

correct_while5 = """
while 1 + 1:
try:
...
except:
...
finally:
...
"""

correct_while6 = """
while 0:
...
"""

# Do raise:

wrong_while1 = """
while other:
try:
...
except:
...
while True:
...
"""

wrong_while2 = """
while True:
try:
...
finally:
...
"""


@pytest.mark.parametrize('template', [
template_simple,
Expand All @@ -65,7 +135,7 @@ def wrapper():
'raise Some()',
'raise',
])
def test_correct_while_loops(
def test_correct_while_loops_with_statements(
assert_errors,
parse_ast_tree,
keyword,
Expand All @@ -81,23 +151,22 @@ def test_correct_while_loops(
assert_errors(visitor, [])


@pytest.mark.parametrize('template', [
template_function,
@pytest.mark.parametrize('code', [
correct_while1,
correct_while2,
correct_while3,
correct_while4,
correct_while5,
correct_while6,
])
@pytest.mark.parametrize('keyword', [
'return',
'return some',
])
def test_correct_while_loops_function(
def test_correct_while_loops_with_try(
assert_errors,
parse_ast_tree,
keyword,
template,
code,
default_options,
mode,
):
"""Testing while loops with ``return`` statements."""
tree = parse_ast_tree(mode(template.format(keyword)))
"""Testing while loops with correct code."""
tree = parse_ast_tree(code)

visitor = WrongLoopVisitor(default_options, tree=tree)
visitor.run()
Expand All @@ -106,22 +175,22 @@ def test_correct_while_loops_function(


@pytest.mark.parametrize('template', [
template_other,
template_function,
])
@pytest.mark.parametrize('keyword', [
'print(some)',
'attr.method()',
'a = 1',
'return',
'return some',
])
def test_other_while_loops(
def test_correct_while_loops_function(
assert_errors,
parse_ast_tree,
keyword,
template,
default_options,
mode,
):
"""Testing other while loops with regular code."""
tree = parse_ast_tree(template.format(keyword))
"""Testing while loops with ``return`` statements."""
tree = parse_ast_tree(mode(template.format(keyword)))

visitor = WrongLoopVisitor(default_options, tree=tree)
visitor.run()
Expand Down Expand Up @@ -157,6 +226,25 @@ def test_wrong_while_loops(
assert_errors(visitor, [InfiniteWhileLoopViolation])


@pytest.mark.parametrize('code', [
wrong_while1,
wrong_while2,
])
def test_wrong_while_loops_with_try(
assert_errors,
parse_ast_tree,
code,
default_options,
):
"""Testing while loops with correct code."""
tree = parse_ast_tree(code)

visitor = WrongLoopVisitor(default_options, tree=tree)
visitor.run()

assert_errors(visitor, [InfiniteWhileLoopViolation])


@pytest.mark.parametrize('template', [
template_double_while,
])
Expand Down
28 changes: 8 additions & 20 deletions wemake_python_styleguide/visitors/ast/keywords.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
FunctionNodes,
TextNodes,
)
from wemake_python_styleguide.logic import walk
from wemake_python_styleguide.logic import walk, walrus
from wemake_python_styleguide.logic.naming import name_nodes
from wemake_python_styleguide.logic.nodes import get_parent
from wemake_python_styleguide.logic.tree import keywords, operators
Expand Down Expand Up @@ -405,32 +405,20 @@ class ConstantKeywordVisitor(BaseNodeVisitor):
)

def visit_While(self, node: ast.While) -> None:
"""
Visits ``while`` keyword and tests that loop will execute.
Raises:
WrongKeywordConditionViolation
"""
"""Visits ``while`` keyword and tests that loop will execute."""
self._check_condition(node, node.test)
self.generic_visit(node)

def visit_Assert(self, node: ast.Assert) -> None:
"""
Visits ``assert`` keyword and tests that condition is correct.
Raises:
WrongKeywordConditionViolation
"""
"""Visits ``assert`` keyword and tests that condition is correct."""
self._check_condition(node, node.test)
self.generic_visit(node)

def _check_condition(self, node: ast.AST, condition: ast.AST) -> None:
real_node = operators.unwrap_unary_node(condition)
if isinstance(real_node, ast.NameConstant) and real_node.value is True:
def _check_condition(self, node: ast.AST, cond: ast.AST) -> None:
if isinstance(cond, ast.NameConstant) and cond.value is True:
if isinstance(node, ast.While):
return # We should allow `while True:`
return # We should allow plain `while True:`

real_node = operators.unwrap_unary_node(walrus.get_assigned_expr(cond))
if isinstance(real_node, self._forbidden_nodes):
self.add_violation(WrongKeywordConditionViolation(condition))
self.add_violation(WrongKeywordConditionViolation(cond))
Loading

0 comments on commit b9e73c3

Please sign in to comment.