Create Untrivial Logic in Finally Block Rule

CHANGELOG.md

@@ -19,27 +19,29 @@ Semantic versioning in our case means:
 
 ## 1.2.0 WIP
 
+Due to PEP-695, it's now allowed
+to use `[]` in decorators only for `python3.12+`.
+
+```python
+@MyClassDecorator[T, V]
+def some_function(): ...
+```
+
 ### Features
 
+- Adds `WPS243`: forbids complex `finally` bodies, #3458
 - Adds `WPS478`: forbids using non strict slice operations, #1011
 - Adds `WPS479`: forbids using multiline fstrings, #3405
 - Adds `WPS480`: forbids using comments inside formatted string, #3404
 
 ### Bugfixes
 
-- Removes unnecessary WPS604 and WPS614 rules from the noqa.py, #3420
+- Removes unnecessary `WPS604` and `WPS614` rules from the `noqa.py`, #3420
 - Fixes `WPS115` false-positive on `StrEnum`, `IntEnum`, `IntFlag` attributes, #3381
 - Fixes `WPS432`, now it ignores magic numbers in `Literal`, #3397
 - Fixes `WPS466` for generic type specifications `MyClassDecorator[T]`, #3417
-- Fixes `WPS212` to ignore nested classes and functions when counting `return` statements, #3413
-
-Due to PEP-695, it's now allowed to use `[]` in the decorator only for `python3.12+`.
-
-```python
-@MyClassDecorator[T, V]
-def some_function(): ...
-```
-
+- Fixes `WPS212` to ignore nested classes and functions
+  when counting `return` statements, #3413
 - Improves `WPS349` highlighting, #3437
 
 

tests/fixtures/noqa/noqa.py

@@ -756,3 +756,12 @@ async def test_await_in_loop():
 
 some_sequence = some_sequence[::-1]  # noqa: WPS478
 
+try: # noqa: WPS243
+    my_print("1")
+except AnyError:
+    my_print("oh no error")
+finally:
+    my_print("3 / 0")
+    my_print('zero division error')
+    my_print(3 / 3)
+    my_print('no zero division error')

tests/test_checker/test_noqa.py

@@ -100,6 +100,7 @@
         'WPS240': 0,  # only triggers on 3.12+
         'WPS241': 1,
         'WPS242': 1,
+        'WPS243': 1,
         'WPS300': 1,
         'WPS301': 1,
         'WPS302': 0,  # disabled since 1.0.0

tests/test_visitors/test_ast/test_exceptions/test_complex_finally.py

@@ -0,0 +1,207 @@
+import pytest
+
+from wemake_python_styleguide.violations.complexity import (
+    ComplexFinallyViolation,
+)
+from wemake_python_styleguide.visitors.ast.complexity.complex_finally import (
+    ComplexFinallyBlocksVisitor,
+)
+
+# Correct:
+
+no_finally = """
+try:
+    ...
+except:
+    ...
+"""
+
+correct_example1 = """
+try:
+    ...
+except:
+    ...
+finally:
+    ...
+"""
+
+correct_example2 = """
+try:
+    ...
+finally:
+    ...
+"""
+
+correct_example3 = """
+try:
+    ...
+except:
+    ...
+finally:
+    ...  # 2 lines
+    ...
+"""
+
+correct_example4 = """
+try:
+    ...
+finally:
+    if ...:  # 2 lines
+        ...
+"""
+
+# Wrong:
+
+wrong_example1 = """
+try:
+    ...
+finally:
+    if ...:  # 4 lines
+        ...
+    else:
+        ...
+"""
+
+wrong_example2 = """
+try:
+    ...
+except:
+    ...
+finally:
+    if ...:  # 3 lines
+        ...
+        ...
+    ...  # +1 line
+"""
+
+wrong_example3 = """
+try:
+    ...
+except:
+    ...
+else:
+    ...
+finally:
+    try:  # 4 lines
+        ...
+    except:
+        ...
+"""
+
+wrong_example4 = """
+try:
+    ...
+finally:
+    ...  # 4 lines
+    ...
+    ...
+    ...
+"""
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        wrong_example1,
+        wrong_example2,
+        wrong_example3,
+        wrong_example4,
+    ],
+)
+def test_untrivial_try_blocks(
+    assert_errors,
+    assert_error_text,
+    parse_ast_tree,
+    code,
+    default_options,
+):
+    """Violations are raised when finally blocks exceed default line limit."""
+    tree = parse_ast_tree(code)
+
+    visitor = ComplexFinallyBlocksVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [ComplexFinallyViolation])
+    assert_error_text(visitor, '4', baseline=2)
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        no_finally,
+        correct_example1,
+        correct_example2,
+        correct_example3,
+        correct_example4,
+    ],
+)
+def test_correct_try_blocks(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    default_options,
+):
+    """No violations for finally blocks within default line limit."""
+    tree = parse_ast_tree(code)
+
+    visitor = ComplexFinallyBlocksVisitor(default_options, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        no_finally,
+        correct_example1,
+        correct_example2,
+        correct_example3,
+        correct_example4,
+        wrong_example1,
+        wrong_example2,
+        wrong_example3,
+        wrong_example4,
+    ],
+)
+def test_custom_correct_try_blocks(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    options,
+):
+    """No violations for finally blocks within custom line limit."""
+    tree = parse_ast_tree(code)
+
+    option_values = options(max_lines_in_finally=5)
+    visitor = ComplexFinallyBlocksVisitor(option_values, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [])
+
+
+@pytest.mark.parametrize(
+    'code',
+    [
+        correct_example3,
+        correct_example4,
+        wrong_example1,
+        wrong_example2,
+        wrong_example3,
+        wrong_example4,
+    ],
+)
+def test_custom_low_max_lines_in_finally(
+    assert_errors,
+    parse_ast_tree,
+    code,
+    options,
+):
+    """Setting `max_lines_in_finally` to 1 will report many usages."""
+    tree = parse_ast_tree(code)
+
+    option_values = options(max_lines_in_finally=1)
+    visitor = ComplexFinallyBlocksVisitor(option_values, tree=tree)
+    visitor.run()
+
+    assert_errors(visitor, [ComplexFinallyViolation])

wemake_python_styleguide/options/config.py

@@ -155,6 +155,9 @@ class of violations that are forbidden to ignore inline, defaults to
 - ``max-match-cases`` - maximum number of cases in a match block of code
     defaults to
     :str:`wemake_python_styleguide.options.defaults.MAX_MATCH_CASES`
+- ``max-lines-in-finally`` - maximum lines in finally block of code
+    defaults to
+    :str:`wemake_python_styleguide.options.defaults.MAX_LINES_IN_FINALLY`
 
 .. rubric:: Formatter options
 
@@ -431,6 +434,11 @@ class Configuration:
             defaults.MAX_MATCH_CASES,
             'Maximum number of match cases in a single match.',
         ),
+        _Option(
+            '--max-lines-in-finally',
+            defaults.MAX_LINES_IN_FINALLY,
+            'Maximum lines of expressions in a finally block.',
+        ),
         # Formatter:
         _Option(
             '--show-violation-links',

wemake_python_styleguide/options/defaults.py

@@ -56,6 +56,9 @@
 # Complexity:
 # ===========
 
+#: Maximum lines of expressions in a `finally` block.
+MAX_LINES_IN_FINALLY: Final = 2  # guessed
+
 #: Maximum number of `return` statements allowed in a single function.
 MAX_RETURNS: Final = 5  # 7-2
 

wemake_python_styleguide/options/validation.py

@@ -101,6 +101,7 @@ class ValidatedOptions:
     max_type_params: int = attr.ib(validator=[_min_max(min=1)])
     max_match_subjects: int = attr.ib(validator=[_min_max(min=1)])
     max_match_cases: int = attr.ib(validator=[_min_max(min=1)])
+    max_lines_in_finally: int = attr.ib(validator=[_min_max(min=1)])
     show_violation_links: bool
     exps_for_one_empty_line: int
 

wemake_python_styleguide/presets/topics/complexity.py

@@ -5,6 +5,7 @@
     annotations,
     calls,
     classes,
+    complex_finally,
     counts,
     function,
     imports,
@@ -39,4 +40,5 @@
     annotations.AnnotationComplexityVisitor,
     pm.MatchSubjectsVisitor,
     pm.MatchCasesVisitor,
+    complex_finally.ComplexFinallyBlocksVisitor,
 )

wemake_python_styleguide/violations/complexity.py

@@ -63,6 +63,7 @@
    TooManyTypeParamsViolation
    TooManyMatchSubjectsViolation
    TooManyMatchCaseViolation
+   ComplexFinallyViolation
 
 Module complexity
 -----------------
@@ -109,6 +110,7 @@
 .. autoclass:: TooManyTypeParamsViolation
 .. autoclass:: TooManyMatchSubjectsViolation
 .. autoclass:: TooManyMatchCaseViolation
+.. autoclass:: ComplexFinallyViolation
 
 """
 
@@ -1397,3 +1399,32 @@ class TooManyMatchCaseViolation(ASTViolation):
 
     error_template = 'Found too many cases in `match` block: {0}'
     code = 242
+
+
+@final
+class ComplexFinallyViolation(ASTViolation):
+    """
+    Forbids complex ``finally`` block.
+
+    Reasoning:
+        ``finally`` is very special. It executes code in all
+        cases and therefore can't fail. When there are many lines in ``finally``
+        it indicates a larger problem: brittle and complex cleanups.
+
+    Solution:
+        Simplify the ``finally`` block. Use context managers, use ``ExitStack``.
+
+    Configuration:
+        This rule is configurable with ``--max-lines-in-finally``.
+        Default:
+        :str:`wemake_python_styleguide.options.defaults.MAX_LINES_IN_FINALLY`
+
+    See also:
+        https://peps.python.org/pep-0765
+
+    .. versionadded:: 1.2.0
+
+    """
+
+    error_template = 'Found too many lines in `finally` block: {0}'
+    code = 243