diff --git a/CHANGELOG.md b/CHANGELOG.md index f666a266d..bf5609541 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Semantic versioning in our case means: - Fixes false positive `WPS457` for ``while True`` loop with ``await`` expressions, #3753 - Fixes the false positive `WPS617` by assigning a function that receives a lambda expression as a parameter. +- Fixes false positive `WPS430` for whitelisted nested functions, #3589 ## 1.5.0 diff --git a/tests/fixtures/noqa/noqa.py b/tests/fixtures/noqa/noqa.py index d8fd06449..9c079d6bf 100644 --- a/tests/fixtures/noqa/noqa.py +++ b/tests/fixtures/noqa/noqa.py @@ -123,6 +123,13 @@ class Nested: # noqa: WPS431 def nested(): # noqa: WPS430 ... + if some_condition(): + def deep_nested(): # noqa: WPS430 + ... + else: + async def deep_nested(): # noqa: WPS430 + ... + del {'a': 1}['a'] # noqa: WPS420 delattr(object, 'some') # noqa: WPS421 diff --git a/tests/test_checker/test_noqa.py b/tests/test_checker/test_noqa.py index 838a859f7..162925148 100644 --- a/tests/test_checker/test_noqa.py +++ b/tests/test_checker/test_noqa.py @@ -197,7 +197,7 @@ 'WPS427': 1, 'WPS428': 0, # disabled since 1.0.0 'WPS429': 1, - 'WPS430': 1, + 'WPS430': 3, 'WPS431': 2, 'WPS432': 2, 'WPS433': 0, # disabled since 1.0.0 diff --git a/tests/test_visitors/test_ast/test_complexity/test_nested/test_nested_functions.py b/tests/test_visitors/test_ast/test_complexity/test_nested/test_nested_functions.py index 0254460e9..a9435e151 100644 --- a/tests/test_visitors/test_ast/test_complexity/test_nested/test_nested_functions.py +++ b/tests/test_visitors/test_ast/test_complexity/test_nested/test_nested_functions.py @@ -234,6 +234,34 @@ def test_whitelist_nested_functions( [ nested_function_in_if, nested_function_in_if_else, + ], +) +def test_deep_whitelist_nested_functions_allowed( + assert_errors, + assert_error_text, + parse_ast_tree, + whitelist_name, + code, + default_options, + mode, +): + """ + Test for allowed whitelisted functions inside single if(/else) block. + + See: https://github.com/wemake-services/wemake-python-styleguide/issues/3589 + """ + tree = parse_ast_tree(mode(code.format(whitelist_name))) + + visitor = NestedComplexityVisitor(default_options, tree=tree) + visitor.run() + + assert_errors(visitor, []) + + +@pytest.mark.parametrize('whitelist_name', NESTED_FUNCTIONS_WHITELIST) +@pytest.mark.parametrize( + 'code', + [ nested_function_while_loop, nested_function_in_for_loop, nested_function_in_try, @@ -251,7 +279,7 @@ def test_deep_whitelist_nested_functions( default_options, mode, ): - """Testing that it is possible to nest whitelisted functions.""" + """Testing that it is restricted to nest even whitelisted functions.""" tree = parse_ast_tree(mode(code.format(whitelist_name))) visitor = NestedComplexityVisitor(default_options, tree=tree) diff --git a/wemake_python_styleguide/visitors/ast/complexity/nested.py b/wemake_python_styleguide/visitors/ast/complexity/nested.py index 2fe3b3a7c..d1daf83d7 100644 --- a/wemake_python_styleguide/visitors/ast/complexity/nested.py +++ b/wemake_python_styleguide/visitors/ast/complexity/nested.py @@ -46,6 +46,9 @@ def visit_any_function(self, node: AnyFunctionDef) -> None: Used to find nested functions. Uses ``NESTED_FUNCTIONS_WHITELIST`` to respect some nested functions. + Only whitelisted functions are allowed, either directly inside + a function or inside a single function-level ``if`` / ``if-else``. + All other nesting is forbidden. """ self._check_nested_function(node) self.generic_visit(node) @@ -56,13 +59,26 @@ def visit_Lambda(self, node: ast.Lambda) -> None: self.generic_visit(node) def _check_nested_function(self, node: AnyFunctionDef) -> None: - is_inside_function = isinstance(get_context(node), FunctionNodes) + context = get_context(node) + if not isinstance(context, FunctionNodes): + return - is_direct = isinstance(get_parent(node), FunctionNodes) - is_bad = is_direct and node.name not in NESTED_FUNCTIONS_WHITELIST + parent = get_parent(node) - if is_bad or (is_inside_function and not is_direct): - self.add_violation(NestedFunctionViolation(node, text=node.name)) + is_direct = isinstance(parent, FunctionNodes) + + is_single_if = ( + isinstance(parent, ast.If) and get_parent(parent) is context + ) + + if node.name in NESTED_FUNCTIONS_WHITELIST and ( + is_direct or is_single_if + ): + return + + self.add_violation( + NestedFunctionViolation(node, text=node.name), + ) def _check_nested_classes(self, node: ast.ClassDef) -> None: parent_context = get_context(node)