Skip to content

normalise-handle gate: the ImportError special case is dead code, its three tests cannot fail, and the widening changelog was deleted #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jaylfc merged 1 commit into from
Aug 18, 2026
Merged

normalise-handle gate: the ImportError special case is dead code, its three tests cannot fail, and the widening changelog was deleted #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.d/tsk-xhvo4a-duplicate-definition-gate-scope.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
36 changes: 3 additions & 33 deletions scripts/normalise_handle_gate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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.
Expand All @@ -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}"
Expand All @@ -146,7 +125,6 @@ def _collect_definitions(
closure_scope,
class_path=(),
in_try=False,
in_import_error_except=False,
arm_tracker=None,
)
)
Expand All @@ -160,7 +138,6 @@ def _collect_definitions(
new_scope,
new_class_path,
in_try=False,
in_import_error_except=False,
arm_tracker=None,
)
)
Expand All @@ -182,7 +159,6 @@ def _collect_definitions(
scope,
class_path,
in_try,
in_import_error_except,
arm_tracker=body_tracker,
)
)
Expand All @@ -193,7 +169,6 @@ def _collect_definitions(
scope,
class_path,
in_try,
in_import_error_except,
arm_tracker=orelse_tracker,
)
)
Expand All @@ -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,
)
)
Expand All @@ -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,
)
)
Expand All @@ -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,
)
)
Expand Down
35 changes: 16 additions & 19 deletions tests/test_normalise_handle_gate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -428,38 +428,35 @@ 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) == []

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) == []

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) == []

Expand Down Expand Up @@ -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": (
Expand Down