Merge master into issue-wemake-services#1172

CHANGELOG.md

@@ -40,6 +40,8 @@ Semantic versioning in our case means:
 - Adds baseline information for all complexity violation messages: `x > baseline`
 - Changes how cognitive complexity is calculated
 - Adds support for positional arguments in different checks
+- Forbids to use `continue` and `break` in `finally`. It is a terrible practice, because 
+  `finally` is implicitly called and can cause damage to logic with its implicitness.
 
 ### Bugfixes
 

CONTRIBUTING.md

@@ -47,11 +47,15 @@ you will have to do several things:
 2. Add docs about the error code to the `pages/usage/violations/index.rst`
 3. Add a test that the plugin is working to `tests/test_plugins.py`
 
-
 ## One magic command
 
 Run `make test` to run everything we have!
 
+#### Building on Windows
+
+- Building directly in Windows does not work.
+- Instead, use a Windows Subsystem for Linux (WSL) such as Ubuntu 18.04 LTS that you can get from the Microsoft Store. 
+- Clone the project to a part of the WSL where Windows does not overwrite permissions, for example _directly to the home of the WSL_ (do `cd` and then `git clone`). That problem looks like [this](https://github.com/wemake-services/wemake-python-styleguide/issues/1007#issuecomment-562719702) and you can read more about why changing the permissons does not work [here](https://github.com/Microsoft/WSL/issues/81).
 
 ## Tests
 

docs/pages/usage/setup.rst

@@ -16,7 +16,7 @@ Installation
 We also recommend to use `poetry <https://github.com/sdispater/poetry>`_
 instead of a default ``pip``.
 
-You might want to also intsall optional tools
+You might want to also install optional tools
 that pairs nicely with ``wemake-python-styleguide``:
 
 - :ref:`flakehell` for easy integration into a **legacy** codebase

tests/fixtures/noqa/noqa.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env pytho # noqa: WPS452 WPS323
+#!/usr/bin/env pytho # noqa: WPS453 WPS323
 # -*- coding: utf-8 -*-
 """
 This file contains all possible violations.
@@ -693,3 +693,14 @@ def consecutive_yields():
     'wrong',
 ]
 *numbers, = [4, 7]  # noqa: WPS356
+
+for element in range(10):
+    try:  # noqa: WPS452
+        my_print(1)
+    except AnyError:
+        my_print('nope')
+    finally:
+        # See:
+        # https://github.com/wemake-services/wemake-python-styleguide/issues/1082
+        break
+    my_print(4)

tests/fixtures/noqa/noqa38.py

@@ -49,3 +49,15 @@ def wrong_comprehension2():
 
 def positional_only(first, /, second):  # noqa: WPS451
     anti_wps428 = 1
+
+
+for element in range(10):
+    try:  # noqa: WPS452
+        my_print(1)
+    except AnyError:
+        my_print('nope')
+    finally:
+        # See:
+        # https://github.com/wemake-services/wemake-python-styleguide/issues/1082
+        continue
+    my_print(4)

tests/test_checker/test_noqa.py

@@ -47,6 +47,7 @@
 
     'WPS416': int(not PY38),  # only works for `< python3.8`
     'WPS451': int(PY38),  # only works for `>= python3.8`
+    'WPS452': int(PY38),  # only works for `>= python3.8`
 
     'WPS602': 2,
 })
@@ -217,7 +218,8 @@
     'WPS449': 1,
     'WPS450': 1,
     'WPS451': 0,  # defined in version specific table.
-    'WPS452': 2,
+    'WPS452': 1,  # also defined in version specific table.
+    'WPS453': 2,
 
     'WPS500': 1,
     'WPS501': 1,

tests/test_visitors/test_ast/test_exceptions/test_finally_in_loops.py

@@ -0,0 +1,174 @@
+# -*- coding: utf-8 -*-
+
+import pytest
+
+from wemake_python_styleguide.compat.constants import PY38
+from wemake_python_styleguide.violations.best_practices import (
+    LoopControlFinallyViolation,
+)
+from wemake_python_styleguide.visitors.ast.exceptions import (
+    WrongTryExceptVisitor,
+)
+
+right_try_example_with_for = """
+def function():
+    for element in range(10):
+        try:
+            ...
+        except:
+            ...
+        finally:
+            ...
+"""
+
+right_try_example_with_while = """
+def function():
+    while first_element < second_element:
+        try:
+            ...
+        except:
+            ...
+        finally:
+            ...
+"""
+
+right_example_with_for = """
+def function():
+    def some():
+        for elem in range(10):
+            if elem > 5:
+                {0}
+"""
+
+right_example_with_while = """
+def function():
+    def some():
+        while a < b:
+            if a == 5:
+                {0}
+"""
+
+wrong_try_example = """
+def function():
+    for element in range(10):
+        try:
+            ...
+        except:
+            ...
+        finally:
+            {0}
+"""
+
+wrong_try_example_with_while = """
+def function():
+    while first_element < second_element:
+        try:
+            ...
+        except:
+            ...
+        finally:
+            {0}
+"""
+
+
+@pytest.mark.parametrize('statement', [
+    'break',
+    'continue',
+])
+@pytest.mark.parametrize('code', [
+    right_example_with_for,
+    right_example_with_while,
+    right_try_example_with_for,
+    right_try_example_with_while,
+])
+def test_right_finally(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    statement,
+    default_options,
+    mode,
+):
+    """Testing that regular loops and loops with `try` are allowed."""
+    tree = parse_ast_tree(mode(code.format(statement)))
+
+    visitor = WrongTryExceptVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])
+
+
+@pytest.mark.parametrize('statement', [
+    'break',
+])
+@pytest.mark.parametrize('code', [
+    wrong_try_example,
+    wrong_try_example_with_while,
+])
+def test_finally_with_break(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    statement,
+    default_options,
+    mode,
+):
+    """Testing that `break` keyword is not allowed in `finally`."""
+    tree = parse_ast_tree(mode(code.format(statement)))
+
+    visitor = WrongTryExceptVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [LoopControlFinallyViolation])
+
+
+@pytest.mark.skipif(
+    not PY38,
+    reason='continue in finally works only since python3.8',
+)
+@pytest.mark.parametrize('statement', [
+    'continue',
+])
+@pytest.mark.parametrize('code', [
+    wrong_try_example,
+    wrong_try_example_with_while,
+])
+def test_finally_with_continue(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    statement,
+    default_options,
+    mode,
+):
+    """Testing that `continue` keyword is not allowed in `finally`."""
+    tree = parse_ast_tree(mode(code.format(statement)))
+
+    visitor = WrongTryExceptVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [LoopControlFinallyViolation])
+
+
+@pytest.mark.skipif(
+    PY38,
+    reason='continue in finally does not work till since python3.8',
+)
+@pytest.mark.parametrize('statement', [
+    'continue',
+])
+@pytest.mark.parametrize('code', [
+    wrong_try_example,
+    wrong_try_example_with_while,
+])
+def test_finally_with_continue_and_exception(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    statement,
+    default_options,
+    mode,
+):
+    """Testing using `continue` keyword in python < 3.8 raises an exception."""
+    with pytest.raises(SyntaxError):
+        parse_ast_tree(mode(code.format(statement)))

tests/test_visitors/test_ast/test_exceptions/test_finally_return_path.py

@@ -16,7 +16,7 @@
 
 right_outside1 = """
 def function():  # we need function to use ``return``
-    for _ in range(10):  # we need ``for`` to use ``break``
+    for _ in range(10):
         try:
             ...
         except:
@@ -363,7 +363,6 @@ def function():
     'return',
     'return None',
     'return 1',
-    'break',
     'raise ValueError',
     'raise ValueError()',
     'raise TypeError(1)',
@@ -408,7 +407,6 @@ def test_wrong_return_in_else_or_finally(
     'return',
     'return None',
     'return 1',
-    'break',
     'raise ValueError',
     'raise ValueError()',
     'raise TypeError(1)',
@@ -455,33 +453,33 @@ def test_correct_return_path_in_try_except(
     ('raise ValueError(1)', '...', '...', 'return 1', '...'),
     ('raise ValueError(1)', '...', '...', 'break', '...'),
 
-    ('...', '...', '...', 'raise ValueError', 'break'),
+    ('...', '...', '...', 'raise ValueError', 'return 0'),
     ('...', '...', '...', 'raise ValueError', 'return None'),
     ('...', '...', '...', 'break', 'return'),
     ('...', '...', '...', 'break', 'raise ValueError'),
-    ('...', '...', '...', 'return', 'break'),
+    ('...', '...', '...', 'return', 'return 1'),
     ('...', '...', '...', 'return', 'raise ValueError()'),
 
     ('break', '...', '...', '...', 'raise ValueError'),
     ('break', '...', '...', '...', 'return'),
-    ('return', '...', '...', '...', 'break'),
+    ('return', '...', '...', '...', 'raise ValueError(1)'),
     ('return 0', '...', '...', '...', 'raise ValueError'),
     ('raise ValueError(1)', '...', '...', '...', 'return 1'),
-    ('raise ValueError(1)', '...', '...', '...', 'break'),
+    ('raise ValueError(1)', '...', '...', '...', 'return'),
 
     ('...', 'break', '...', '...', 'raise ValueError'),
     ('...', 'break', '...', '...', 'return'),
-    ('...', 'return', '...', '...', 'break'),
+    ('...', 'return', '...', '...', 'raise ValueError(1)'),
     ('...', 'return 0', '...', '...', 'raise ValueError'),
     ('...', 'raise ValueError(1)', '...', '...', 'return 1'),
-    ('...', 'raise ValueError(1)', '...', '...', 'break'),
+    ('...', 'raise ValueError(1)', '...', '...', 'return 0'),
 
     ('...', '...', 'break', '...', 'raise ValueError'),
     ('...', '...', 'break', '...', 'return'),
-    ('...', '...', 'return', '...', 'break'),
+    ('...', '...', 'return', '...', 'raise ValueError'),
     ('...', '...', 'return 0', '...', 'raise ValueError'),
     ('...', '...', 'raise ValueError(1)', '...', 'return 1'),
-    ('...', '...', 'raise ValueError(1)', '...', 'break'),
+    ('...', '...', 'raise ValueError(1)', '...', 'return 0'),
 ])
 def test_different_nodes_trigger_violation(
     assert_errors,