|
| 1 | +"""Detect blind exception chaining that hides original errors. |
| 2 | +
|
| 3 | +Usage: |
| 4 | + python scripts/check_blind_chaining.py [paths ...] |
| 5 | +""" |
| 6 | + |
| 7 | +from __future__ import annotations |
| 8 | + |
| 9 | +import ast |
| 10 | +import sys |
| 11 | +from collections.abc import Iterable |
| 12 | +from pathlib import Path |
| 13 | + |
| 14 | +SKIP_COMMENT = "# noqa: BC" |
| 15 | +ALLOWLIST_DIRS = ( |
| 16 | + Path("tidy3d/web/cli/develop"), |
| 17 | + Path("tidy3d/packaging"), |
| 18 | +) |
| 19 | +ALLOWLIST_PATHS = ( |
| 20 | + Path("tidy3d/packaging.py"), |
| 21 | + Path("tidy3d/updater.py"), |
| 22 | +) |
| 23 | + |
| 24 | + |
| 25 | +def contains_name(node: ast.AST | None, target: str) -> bool: |
| 26 | + """Return True if any ``ast.Name`` inside ``node`` matches ``target``.""" |
| 27 | + |
| 28 | + if node is None: |
| 29 | + return False |
| 30 | + return any(isinstance(child, ast.Name) and child.id == target for child in ast.walk(node)) |
| 31 | + |
| 32 | + |
| 33 | +def iter_python_files(paths: Iterable[Path]) -> Iterable[Path]: |
| 34 | + """Yield Python files under the provided paths, respecting skips.""" |
| 35 | + |
| 36 | + for root in paths: |
| 37 | + if root.is_file() and root.suffix == ".py": |
| 38 | + yield root |
| 39 | + continue |
| 40 | + if not root.is_dir(): |
| 41 | + continue |
| 42 | + yield from root.rglob("*.py") |
| 43 | + |
| 44 | + |
| 45 | +def _primary_message_expr(exc: ast.AST) -> ast.AST | None: |
| 46 | + """ |
| 47 | + Heuristic: identify the expression that most serializers/GUI layers display. |
| 48 | +
|
| 49 | + - For `SomeError("msg", ...)`: first positional arg. |
| 50 | + - For `SomeError(message="msg")` / `detail=...`: preferred keyword. |
| 51 | + - Otherwise: None (unknown). |
| 52 | + """ |
| 53 | + if not isinstance(exc, ast.Call): |
| 54 | + return None |
| 55 | + |
| 56 | + if exc.args: |
| 57 | + return exc.args[0] |
| 58 | + return None |
| 59 | + |
| 60 | + |
| 61 | +def find_blind_chaining(path: Path) -> list[tuple[Path, int, int, str]]: |
| 62 | + """ |
| 63 | + Find `raise <new_exc> from <cause>` where `<cause>` is *not* referenced in the |
| 64 | + user-visible message expression (first positional arg or message-like kwarg). |
| 65 | +
|
| 66 | + Returns: (path, lineno, col_offset, cause_name) |
| 67 | + """ |
| 68 | + errors: list[tuple[Path, int, int, str]] = [] |
| 69 | + try: |
| 70 | + src = path.read_text(encoding="utf-8") |
| 71 | + tree = ast.parse(src, filename=str(path)) |
| 72 | + except SyntaxError: |
| 73 | + return errors |
| 74 | + |
| 75 | + lines = src.splitlines() |
| 76 | + |
| 77 | + for node in ast.walk(tree): |
| 78 | + if not isinstance(node, ast.Raise): |
| 79 | + continue |
| 80 | + |
| 81 | + # Handles `raise from e` (node.exc is None) safely. |
| 82 | + if node.exc is None: |
| 83 | + continue |
| 84 | + |
| 85 | + # Ignore `raise X from None` (intentional suppression). |
| 86 | + if isinstance(node.cause, ast.Constant) and node.cause.value is None: |
| 87 | + continue |
| 88 | + |
| 89 | + # Only enforce for simple `from <name>` patterns (e.g., `except ... as e:`). |
| 90 | + if not isinstance(node.cause, ast.Name): |
| 91 | + continue |
| 92 | + cause_name = node.cause.id |
| 93 | + |
| 94 | + # Avoid noisy false positives for `raise existing_exc from e`. |
| 95 | + if isinstance(node.exc, ast.Name): |
| 96 | + continue |
| 97 | + |
| 98 | + # Prefer checking the “primary message” expression if we can identify it. |
| 99 | + msg_expr = _primary_message_expr(node.exc) |
| 100 | + |
| 101 | + if msg_expr is not None: |
| 102 | + ok = contains_name(msg_expr, cause_name) |
| 103 | + else: |
| 104 | + # Fallback: at least require the cause to be used somewhere in the exc expression. |
| 105 | + ok = contains_name(node.exc, cause_name) |
| 106 | + |
| 107 | + if not ok: |
| 108 | + lineno = getattr(node, "lineno", 1) |
| 109 | + col = getattr(node, "col_offset", 0) |
| 110 | + if lineno - 1 < len(lines): |
| 111 | + if SKIP_COMMENT in lines[lineno - 1]: |
| 112 | + continue |
| 113 | + errors.append((path, lineno, col, cause_name)) |
| 114 | + |
| 115 | + return errors |
| 116 | + |
| 117 | + |
| 118 | +def is_allowlisted(path: Path) -> bool: |
| 119 | + """Return True if ``path`` resides in an allowlisted directory.""" |
| 120 | + |
| 121 | + resolved_path = path.resolve() |
| 122 | + if any(resolved_path == allow_path.resolve() for allow_path in ALLOWLIST_PATHS): |
| 123 | + return True |
| 124 | + for allow_dir in ALLOWLIST_DIRS: |
| 125 | + if resolved_path.is_relative_to(allow_dir.resolve()): |
| 126 | + return True |
| 127 | + return False |
| 128 | + |
| 129 | + |
| 130 | +def main(argv: list[str]) -> int: |
| 131 | + paths = [Path(arg) for arg in argv] if argv else [Path("tidy3d")] |
| 132 | + existing_paths = [path for path in paths if path.exists()] |
| 133 | + if not existing_paths: |
| 134 | + existing_paths = [Path(".")] |
| 135 | + |
| 136 | + failures: list[tuple[Path, int, int, str]] = [] |
| 137 | + for file_path in iter_python_files(existing_paths): |
| 138 | + failures.extend(find_blind_chaining(file_path)) |
| 139 | + |
| 140 | + filtered_failures = [ |
| 141 | + (path, lineno, cause_name) |
| 142 | + for path, lineno, _, cause_name in failures |
| 143 | + if not is_allowlisted(path) |
| 144 | + ] |
| 145 | + |
| 146 | + if filtered_failures: |
| 147 | + print("Blind exception chaining detected (missing original cause in raised message):") |
| 148 | + for path, lineno, cause_name in sorted(filtered_failures): |
| 149 | + print( |
| 150 | + f" {path}:{lineno} cause variable '{cause_name}' not referenced in raised exception" |
| 151 | + ) |
| 152 | + print(f"Add '{SKIP_COMMENT}' to the raise line to suppress intentionally.") |
| 153 | + return 1 |
| 154 | + |
| 155 | + print("No blind exception chaining instances found.") |
| 156 | + return 0 |
| 157 | + |
| 158 | + |
| 159 | +if __name__ == "__main__": |
| 160 | + sys.exit(main(sys.argv[1:])) |
0 commit comments