From 154410967d3bee23a00103248e25f92058c98b9b Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:26:59 +0000 Subject: [PATCH] Remove code no control flow can reach, and the check that proves it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit playwright_scraper.py carried fifteen lines whose `def` line had been lost. What remained -- a docstring and a `try: return page.content()` -- was indented at function level, so Python attached it to the end of _mask_credentials, after that function's own return. It parses, it imports, --help works, compileall passes and this suite was green, because unreachable code is still valid code. Nothing called it, and _content_when_settled directly below does the same job. The same fifteen lines are in six repos of this family, byte for byte, and have been since each one's first commit -- so this is inheritance rather than authorship, the shape CLAUDE.md §16 describes. The check added with it: a statement sitting after a return/raise/break/continue in the SAME block. Deliberately narrow, claiming nothing about reachability in general. The undefined-name walk beside it cannot catch this class, and correctly so -- it pools every binding in the file rather than tracking scopes, so `page` used inside the dead block resolves against the `page` parameter of a real function elsewhere in the module. That coarseness is the right trade for what that check is for, which is why this is a separate check rather than a tightening of it. Measured across the eighteen repos of this family: six problems reported, zero false positives. Verified by control -- appending a function whose body is `return 1` then `x = 2` turns the suite red. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 23 +++++++++++++++++++++++ playwright_scraper.py | 15 --------------- pyproject.toml | 2 +- smoke_test.py | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c8ae4c..faffc62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,29 @@ as a CLI toolkit can. A **patch** release means fixes — it does not mean every flag and every default is frozen. Where a patch changes behaviour an existing user would notice, the release notes say so first. +## [0.1.1] — 2026-09-16 + +### Fixed + +- **Fifteen lines of unreachable code removed from `playwright_scraper.py`.** + A function's `def` line had been lost at some point before this repo's + first commit, leaving its docstring and its `try: return page.content()` + body indented into the end of `_mask_credentials`, where the control flow + can never arrive. Nothing called it — `_content_when_settled` below it + does the job — so no behaviour changes. The same fifteen lines, byte for + byte, were in six repos of this family. + +### Added + +- **A check for a statement the control flow can never reach.** The + undefined-name walk beside it cannot see this class by design: it pools + every binding in a file rather than tracking scopes, so a name used inside + dead code passes as long as anything else in the module binds it. The new + one is narrow — a statement after a `return`/`raise`/`break`/`continue` in + the SAME block — and measured across the eighteen repos of this family it + found six real problems and zero false positives. Verified by control: + appending `return 1` followed by a statement turns the suite red. + ## [0.1.0] — 2026-09-14 The first release of this repository as a member of the diff --git a/playwright_scraper.py b/playwright_scraper.py index 9982490..3bbaa7b 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -603,21 +603,6 @@ def _mask_credentials(text: str) -> str: return _CREDENTIALS_IN_URL_RE.sub(r"\1***:***@", text or "") - """The page's HTML, or None when it cannot be read right now. - - Playwright RAISES rather than returning empty while a navigation is in - flight ("Unable to retrieve content because the page is navigating"), and - a DataDome interstitial resolves by navigating — so the one moment this - is called is the one moment it can fail. Returning None keeps - `page_flow.settle_datadome` waiting instead of crashing the run, which is - what the first live run of this engine did. - """ - try: - return page.content() - except (PWError, PWTimeout): - return None - - def _content_when_settled(page, attempts: int = 4, pause_ms: int = 700): """page.content() that tolerates a page mid-navigation. diff --git a/pyproject.toml b/pyproject.toml index f88bef2..710d8da 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "dubizzle-scraper" -version = "0.1.0" +version = "0.1.1" description = "dubizzle UAE classifieds scraper: cars, property, electronics, jobs and community listings to JSON or CSV." readme = "README.md" license = "MIT" diff --git a/smoke_test.py b/smoke_test.py index 4fda80d..dfda18c 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -1801,6 +1801,14 @@ def test_no_undefined_names(): for k, v in sorted(missing.items())) ok &= check("%s references no undefined name%s" % (name, ": " + detail if missing else ""), not missing) + + # And a statement that can never RUN — see `_unreachable_statements`. + for name in sorted(f for f in os.listdir(REPO_ROOT) if f.endswith(".py")): + dead = _unreachable_statements(os.path.join(REPO_ROOT, name)) + ok &= check("%s has no statement the control flow can never reach%s" + % (name, "" if not dead else ": line %d" % dead[0]), + not dead) + return ok @@ -2731,6 +2739,39 @@ def _undefined_names(path): return missing + +def _unreachable_statements(path): + """Line numbers of statements that can never run. + + A statement sitting after a `return`/`raise`/`break`/`continue` in the + SAME block. Deliberately narrow: it makes no claim about conditions or + reachability in general, only about a block whose control flow has + already left. Measured across the eighteen repos in this family on + 2026-09-16 it reported six problems and zero false positives. + + `_undefined_names` above cannot see this class at all, by design — it + pools every binding in the file rather than tracking scopes, so a name + used inside dead code passes as long as anything else in the module + binds it. What was hiding there: a function whose `def` line had been + lost, leaving its docstring and body absorbed into the end of the + function above it. Identical in six repos, present since each one's + first commit, invisible to import, `--help`, `compileall` and every + green run of this suite. + """ + tree = ast.parse(open(path, encoding="utf-8").read()) + dead = [] + for node in ast.walk(tree): + for field in ("body", "orelse", "finalbody"): + block = getattr(node, field, None) + if not isinstance(block, list): + continue + for i, stmt in enumerate(block[:-1]): + if isinstance(stmt, (ast.Return, ast.Raise, + ast.Continue, ast.Break)): + dead.append(block[i + 1].lineno) + break + return sorted(dead) + def test_public_names_have_consumers(): group("no public name is dead code or unenforced policy") ok = True