From 6c1ea01da2235ea5d62220e2512c479b574a5518 Mon Sep 17 00:00:00 2001 From: abebus Date: Sat, 31 Jan 2026 02:42:34 +0300 Subject: [PATCH 1/3] fix WPS430 false positive small edit added integration test case and updated CHANGELOG.md --- CHANGELOG.md | 1 + tests/fixtures/noqa/noqa.py | 14 ++++++++++++++ tests/test_checker/test_noqa.py | 2 +- .../test_nested/test_nested_functions.py | 3 +-- .../visitors/ast/complexity/nested.py | 7 +++++-- 5 files changed, 22 insertions(+), 5 deletions(-) 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..547e03787 100644 --- a/tests/fixtures/noqa/noqa.py +++ b/tests/fixtures/noqa/noqa.py @@ -122,6 +122,20 @@ class Nested: # noqa: WPS431 def nested(): # noqa: WPS430 ... + + def factory(): # ok + if some_condition(): + def deep_nested(): # noqa: WPS430 + ... + else: + def wrapper(): # ok + ... + if some_other_condition(): + def deep_nested(): # noqa: WPS430 + ... + else: + def decorator(): # ok + ... del {'a': 1}['a'] # noqa: WPS420 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..0da553783 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 @@ -257,8 +257,7 @@ def test_deep_whitelist_nested_functions( visitor = NestedComplexityVisitor(default_options, tree=tree) visitor.run() - assert_errors(visitor, [NestedFunctionViolation]) - assert_error_text(visitor, whitelist_name) + assert_errors(visitor, []) @pytest.mark.parametrize( diff --git a/wemake_python_styleguide/visitors/ast/complexity/nested.py b/wemake_python_styleguide/visitors/ast/complexity/nested.py index 2fe3b3a7c..a71a6e125 100644 --- a/wemake_python_styleguide/visitors/ast/complexity/nested.py +++ b/wemake_python_styleguide/visitors/ast/complexity/nested.py @@ -58,10 +58,13 @@ def visit_Lambda(self, node: ast.Lambda) -> None: def _check_nested_function(self, node: AnyFunctionDef) -> None: is_inside_function = isinstance(get_context(node), FunctionNodes) + is_not_whitelisted = node.name not in NESTED_FUNCTIONS_WHITELIST is_direct = isinstance(get_parent(node), FunctionNodes) - is_bad = is_direct and node.name not in NESTED_FUNCTIONS_WHITELIST + is_bad = is_direct and is_not_whitelisted - if is_bad or (is_inside_function and not is_direct): + if is_bad or ( + is_inside_function and not is_direct and is_not_whitelisted + ): self.add_violation(NestedFunctionViolation(node, text=node.name)) def _check_nested_classes(self, node: ast.ClassDef) -> None: From 8711e66c2a4ccddb1a770df0cdb96916bf6cc45f Mon Sep 17 00:00:00 2001 From: abebus Date: Fri, 13 Feb 2026 21:45:25 +0300 Subject: [PATCH 2/3] after review --- tests/fixtures/noqa/noqa.py | 21 ++++-------- .../test_nested/test_nested_functions.py | 33 +++++++++++++++++-- .../visitors/ast/complexity/nested.py | 21 ++++++++---- 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/tests/fixtures/noqa/noqa.py b/tests/fixtures/noqa/noqa.py index 547e03787..9c079d6bf 100644 --- a/tests/fixtures/noqa/noqa.py +++ b/tests/fixtures/noqa/noqa.py @@ -122,20 +122,13 @@ class Nested: # noqa: WPS431 def nested(): # noqa: WPS430 ... - - def factory(): # ok - if some_condition(): - def deep_nested(): # noqa: WPS430 - ... - else: - def wrapper(): # ok - ... - if some_other_condition(): - def deep_nested(): # noqa: WPS430 - ... - else: - def decorator(): # ok - ... + + if some_condition(): + def deep_nested(): # noqa: WPS430 + ... + else: + async def deep_nested(): # noqa: WPS430 + ... del {'a': 1}['a'] # noqa: WPS420 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 0da553783..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,13 +279,14 @@ 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) visitor.run() - assert_errors(visitor, []) + assert_errors(visitor, [NestedFunctionViolation]) + assert_error_text(visitor, whitelist_name) @pytest.mark.parametrize( diff --git a/wemake_python_styleguide/visitors/ast/complexity/nested.py b/wemake_python_styleguide/visitors/ast/complexity/nested.py index a71a6e125..af5883d7f 100644 --- a/wemake_python_styleguide/visitors/ast/complexity/nested.py +++ b/wemake_python_styleguide/visitors/ast/complexity/nested.py @@ -56,16 +56,23 @@ 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) + if not isinstance(get_context(node), FunctionNodes): + return - is_not_whitelisted = node.name not in NESTED_FUNCTIONS_WHITELIST - is_direct = isinstance(get_parent(node), FunctionNodes) - is_bad = is_direct and is_not_whitelisted + parent = get_parent(node) - if is_bad or ( - is_inside_function and not is_direct and is_not_whitelisted + is_direct = isinstance(parent, FunctionNodes) + + is_single_if = isinstance(parent, ast.If) and isinstance( + get_parent(parent), FunctionNodes + ) + + if node.name not in NESTED_FUNCTIONS_WHITELIST or not ( + is_direct or is_single_if ): - self.add_violation(NestedFunctionViolation(node, text=node.name)) + self.add_violation( + NestedFunctionViolation(node, text=node.name), + ) def _check_nested_classes(self, node: ast.ClassDef) -> None: parent_context = get_context(node) From 84f61a38ef9e01b365293740a2152f293998844b Mon Sep 17 00:00:00 2001 From: abebus Date: Fri, 13 Feb 2026 22:47:16 +0300 Subject: [PATCH 3/3] after review 2 --- .../visitors/ast/complexity/nested.py | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/wemake_python_styleguide/visitors/ast/complexity/nested.py b/wemake_python_styleguide/visitors/ast/complexity/nested.py index af5883d7f..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,23 +59,26 @@ def visit_Lambda(self, node: ast.Lambda) -> None: self.generic_visit(node) def _check_nested_function(self, node: AnyFunctionDef) -> None: - if not isinstance(get_context(node), FunctionNodes): + context = get_context(node) + if not isinstance(context, FunctionNodes): return parent = get_parent(node) is_direct = isinstance(parent, FunctionNodes) - is_single_if = isinstance(parent, ast.If) and isinstance( - get_parent(parent), FunctionNodes + is_single_if = ( + isinstance(parent, ast.If) and get_parent(parent) is context ) - if node.name not in NESTED_FUNCTIONS_WHITELIST or not ( + if node.name in NESTED_FUNCTIONS_WHITELIST and ( is_direct or is_single_if ): - self.add_violation( - NestedFunctionViolation(node, text=node.name), - ) + return + + self.add_violation( + NestedFunctionViolation(node, text=node.name), + ) def _check_nested_classes(self, node: ast.ClassDef) -> None: parent_context = get_context(node)