diff --git a/changelog.d/tsk-xhvo4a-duplicate-definition-gate-scope.md b/changelog.d/tsk-xhvo4a-duplicate-definition-gate-scope.md new file mode 100644 index 00000000..8e56582a --- /dev/null +++ b/changelog.d/tsk-xhvo4a-duplicate-definition-gate-scope.md @@ -0,0 +1,2 @@ +### Fixed +- The duplicate-definition gate now catches definitions inside module-level ``if``/``try``/``for``/``while``/``with`` blocks and same-name closures within one parent function, matching Python's actual binding rules. Same-name closures in different parents remain legal, and the ``try:`` / ``except ImportError:`` fallback pattern stays silent. diff --git a/scripts/normalise_handle_gate.py b/scripts/normalise_handle_gate.py index 9ffd023c..db6b5852 100644 --- a/scripts/normalise_handle_gate.py +++ b/scripts/normalise_handle_gate.py @@ -19,7 +19,8 @@ ``if`` / ``elif`` / ``else`` chain and the ``body`` / ``handlers`` / ``orelse`` arms of one ``try`` statement are mutually exclusive and do not collide. The ``try:`` / ``except ImportError:`` and ``except -ModuleNotFoundError:`` fallback patterns are recognised and left silent. +ModuleNotFoundError:`` fallback patterns are therefore left silent by the +same sibling-arm rule, since at most one arm ever binds. Nested classes are scanned at any depth. """ from __future__ import annotations @@ -48,7 +49,6 @@ class _Def: name: str lineno: int in_try: bool = False - in_import_error_except: bool = False arm_tracker: tuple[int, str] | None = None @@ -89,32 +89,11 @@ def _is_exempt(node: ast.FunctionDef | ast.AsyncFunctionDef) -> bool: return _has_overload_decorator(node) or _has_property_decorator(node) -def _is_import_error_handler(handler: ast.ExceptHandler) -> bool: - if handler.type is None: - return False - - def _is_import_error_node(node): - if isinstance(node, ast.Name): - return node.id in ("ImportError", "ModuleNotFoundError") - if isinstance(node, ast.Attribute): - return ( - isinstance(node.value, ast.Name) - and node.value.id == "builtins" - and node.attr == "ImportError" - ) - return False - - if isinstance(handler.type, ast.Tuple): - return any(_is_import_error_node(elt) for elt in handler.type.elts) - return _is_import_error_node(handler.type) - - def _collect_definitions( body: list[ast.stmt], scope: str = "module", class_path: tuple[str, ...] = (), in_try: bool = False, - in_import_error_except: bool = False, arm_tracker: tuple[int, str] | None = None, ) -> list[_Def]: """Yield ``_Def`` for every non-exempt function/method. @@ -133,7 +112,7 @@ def _collect_definitions( if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): if not _is_exempt(node): found.append( - _Def(scope, node.name, node.lineno, in_try, in_import_error_except, arm_tracker) + _Def(scope, node.name, node.lineno, in_try, arm_tracker) ) closure_scope = ( f"{scope} > {node.name}" @@ -146,7 +125,6 @@ def _collect_definitions( closure_scope, class_path=(), in_try=False, - in_import_error_except=False, arm_tracker=None, ) ) @@ -160,7 +138,6 @@ def _collect_definitions( new_scope, new_class_path, in_try=False, - in_import_error_except=False, arm_tracker=None, ) ) @@ -182,7 +159,6 @@ def _collect_definitions( scope, class_path, in_try, - in_import_error_except, arm_tracker=body_tracker, ) ) @@ -193,7 +169,6 @@ def _collect_definitions( scope, class_path, in_try, - in_import_error_except, arm_tracker=orelse_tracker, ) ) @@ -216,19 +191,16 @@ def _collect_definitions( scope, class_path, in_try=True, - in_import_error_except=in_import_error_except, arm_tracker=body_tracker, ) ) for handler in node.handlers: - is_ie = _is_import_error_handler(handler) found.extend( _collect_definitions( handler.body, scope, class_path, in_try=True, - in_import_error_except=is_ie, arm_tracker=handler_tracker, ) ) @@ -239,7 +211,6 @@ def _collect_definitions( scope, class_path, in_try=True, - in_import_error_except=in_import_error_except, arm_tracker=orelse_tracker, ) ) @@ -250,7 +221,6 @@ def _collect_definitions( scope, class_path, in_try=True, - in_import_error_except=in_import_error_except, arm_tracker=finalbody_tracker, ) ) diff --git a/tests/test_normalise_handle_gate.py b/tests/test_normalise_handle_gate.py index 273da404..d25b9626 100644 --- a/tests/test_normalise_handle_gate.py +++ b/tests/test_normalise_handle_gate.py @@ -18,8 +18,8 @@ ``body`` / ``handlers`` / ``orelse`` arms of one ``try`` statement are mutually exclusive and do not collide. - The ``try:`` / ``except ImportError:``, ``except ModuleNotFoundError:``, - and ``except builtins.ImportError:`` fallback patterns are recognised and - left silent. + and ``except builtins.ImportError:`` fallback patterns are left silent by + the same sibling-arm rule, since at most one arm ever binds. - Nested classes are scanned at any depth. - Files that cannot be decoded as UTF-8 are skipped with a warning, and files that fail to parse are left silent. @@ -428,12 +428,11 @@ def test_module_not_found_error_bare_is_silent(self, tmp_path): f = tmp_path / "mod.py" f.write_text( "try:\n" - " import something\n" + " def foo():\n" + " pass\n" "except ModuleNotFoundError:\n" - " pass\n" - "\n" - "def foo():\n" - " pass\n" + " def foo():\n" + " pass\n" ) assert _duplicate_definitions(f) == [] @@ -441,12 +440,11 @@ def test_module_not_found_error_in_tuple_is_silent(self, tmp_path): f = tmp_path / "mod.py" f.write_text( "try:\n" - " import something\n" + " def foo():\n" + " pass\n" "except (ModuleNotFoundError, OSError):\n" - " pass\n" - "\n" - "def foo():\n" - " pass\n" + " def foo():\n" + " pass\n" ) assert _duplicate_definitions(f) == [] @@ -454,12 +452,11 @@ def test_builtins_import_error_is_silent(self, tmp_path): f = tmp_path / "mod.py" f.write_text( "try:\n" - " import something\n" + " def foo():\n" + " pass\n" "except builtins.ImportError:\n" - " pass\n" - "\n" - "def foo():\n" - " pass\n" + " def foo():\n" + " pass\n" ) assert _duplicate_definitions(f) == [] @@ -532,11 +529,11 @@ def test_scope_parity_corpus(self, tmp_path): [], ), "import_error_fallback_silent": ( - "try:\n import something\nexcept ImportError:\n pass\n\ndef foo(): pass\n", + "try:\n def foo(): pass\nexcept ImportError:\n def foo(): pass\n", [], ), "module_not_found_error_fallback_silent": ( - "try:\n import something\nexcept ModuleNotFoundError:\n pass\n\ndef foo(): pass\n", + "try:\n def foo(): pass\nexcept ModuleNotFoundError:\n def foo(): pass\n", [], ), "platform_if_else_silent": (