test(drift): flag unreachable statements and raise the call-argument floor (t62 F2)
Two gate gaps let the t62 findings ship, and both are closed here. A pure-AST unreachable-statement pass (scan(): pass 6, entry point scan_unreachable()): in every statement block, a statement after return/raise/continue/break can never run. Per-block on purpose — an `else`, an except handler or a nested function is a different block and not dead, so the pass stays signal. Red proof: the pre-fix connection.py, saved from HEAD, is flagged as `connection.py:1113: unreachable-statement: return at line 1110 ends this block`; the live tree is clean. A new counter (statement_blocks, floor 500 of ~1102) keeps the "no findings" verdict from being a pass that stopped parsing. The call-argument floor was 20 while the walk counted 46, so deleting a flow-display owner from HA_CALL_OWNERS did not trip it. It is 44 now, and the suite proves the trip: removing async_show_menu yields 40 checked arguments and `only 40 call_args (floor: 44)`. Measured removals: show_form 25, abort 38, show_menu 40, create_entry 41, update_entry 41 all violate; the single-call-site async_update_reload_and_abort (45) is the one owner a count floor cannot watch, which is why the floor sits two below the count. Also (t62 nit a): the two comments that pinned the t40 entity-attribute finding to ``media_player.py:828`` now name the file and the attribute instead — the line had rotted to an unrelated property.
This commit is contained in:
1 parent
0ef72a1fbb
commit
93d0afcdcf
2 files changed
+203
-12
No files matched your search
+104
-12
@@ -35,7 +35,8 @@ dependency against an INSTALLED homeassistant:
|
||||
(``extra_state_attributes``, ``available``, …) declared as properties — HA
|
||||
merges them with ``attr |= self.extra_state_attributes``, so a plain method
|
||||
raises ``TypeError: 'method' object is not iterable`` on every state write and
|
||||
the entity can never render (t40 live install: ``media_player.py:828``). The
|
||||
the entity can never render (t40 live install: ``media_player.py``'s
|
||||
``extra_state_attributes`` — a property today, so the number rotted). The
|
||||
stub suites call the method happily; this pass is pure AST and, unlike 1-3,
|
||||
needs no homeassistant at all;
|
||||
5. the package's OWN relative imports (t41): ``from .helpers import X`` must
|
||||
@@ -44,10 +45,16 @@ dependency against an INSTALLED homeassistant:
|
||||
harness loaded that file the ImportError waited for a user's log — pass 1
|
||||
cannot see it (it only judges ``homeassistant.*``). Also pure AST, with its
|
||||
own entry point ``scan_package_imports()`` for the fast suite.
|
||||
6. unreachable statements (t62 F2): a statement after
|
||||
``return``/``raise``/``continue``/``break`` in the SAME block can never run.
|
||||
``connection.py`` carried a full duplicated copy of the label-drain body
|
||||
(docstring included) after its ``return cached`` — valid Python, dead, and
|
||||
invisible to every other pass here, so it shipped. Pure AST, with its own
|
||||
entry point ``scan_unreachable()``.
|
||||
|
||||
Both AST passes run before the homeassistant availability check, and the file
|
||||
All three AST passes run before the homeassistant availability check, and the file
|
||||
walk is recursive (``_iter_python_files``), so a platform or helper one
|
||||
directory down cannot hide from either of them.
|
||||
directory down cannot hide from any of them.
|
||||
|
||||
Known limits (deliberate, and why the sweep still catches the real classes):
|
||||
``**kwargs`` splats cannot be checked statically, calls on objects other than
|
||||
@@ -120,17 +127,28 @@ HA_CALL_OWNERS: dict[str, tuple[str, str]] = {
|
||||
}
|
||||
|
||||
#: Non-vacuity floors — the ONE source of truth for the standalone entry point
|
||||
#: and the pytest wrapper (t39 F1). They sit below the current walk (13 modules,
|
||||
#: 60 imports, 35 checked arguments, 7 awaited HA calls) so a broken or
|
||||
#: mis-targeted walk fails loudly instead of printing zeros and exiting 0.
|
||||
#: and the pytest wrapper (t39 F1). They sit below the current walk (24 modules,
|
||||
#: 58 imports, 179 relative imports, 46 checked arguments, 7 awaited HA calls,
|
||||
#: 1102 statement blocks) so a broken or mis-targeted walk fails loudly instead of
|
||||
#: printing zeros and exiting 0.
|
||||
FLOORS: dict[str, int] = {
|
||||
# t41: the sweep is recursive, so the floor sits above the top-level-only
|
||||
# count (13) — a regression to the shallow glob trips it.
|
||||
"modules": 20,
|
||||
"imports": 40,
|
||||
"relative_imports": 100,
|
||||
"call_args": 20,
|
||||
# t62 F2: raised from 20 to just under the current 46. A floor's job is to
|
||||
# notice an API OWNER going away, not only a broken walk: at 20, deleting
|
||||
# `async_show_menu` from HA_CALL_OWNERS left 40 checked arguments and still
|
||||
# looked green — only the pre-fix fixture tripped it. At 44 every owner that
|
||||
# carries a real settlement argument is load-bearing (show_form, show_menu,
|
||||
# abort, create_entry, update_entry). The floor sits two below the count so a
|
||||
# one-call-site refactor does not redden the gate.
|
||||
"call_args": 44,
|
||||
"awaited_calls": 5,
|
||||
# t62 F2: the unreachable-statement pass walks ~1100 statement blocks. A
|
||||
# floor here means a pass that silently stopped parsing cannot look green.
|
||||
"statement_blocks": 500,
|
||||
}
|
||||
|
||||
#: The interpreter floor Home Assistant itself declares: PyPI metadata for
|
||||
@@ -167,7 +185,7 @@ class Report:
|
||||
relative_imports: int = 0
|
||||
call_args: int = 0
|
||||
awaited_calls: int = 0
|
||||
|
||||
statement_blocks: int = 0
|
||||
def floor_violations(self, floors: dict[str, int] | None = None) -> list[str]:
|
||||
"""Counters below the non-vacuity floors — a sweep nobody should trust.
|
||||
|
||||
@@ -187,6 +205,7 @@ class Report:
|
||||
"no Home-Assistant API drift "
|
||||
f"({self.modules} modules, {self.imports} imports, "
|
||||
f"{self.relative_imports} relative imports, "
|
||||
f"{self.statement_blocks} statement blocks, "
|
||||
f"{self.call_args} call arguments, "
|
||||
f"{self.awaited_calls} awaited HA calls)"
|
||||
)
|
||||
@@ -690,7 +709,8 @@ def parse_requires_python(spec: str | None) -> tuple[int, ...] | None:
|
||||
#: HA: ``Entity.__async_calculate_state`` merges ``extra_state_attributes`` with
|
||||
#: ``attr |= self.extra_state_attributes``, which raises
|
||||
#: ``TypeError: 'method' object is not iterable`` on every state write. That was
|
||||
#: the t40 live-install blocker (``media_player.py:828``).
|
||||
#: the t40 live-install blocker — ``media_player.py``'s
|
||||
#: ``extra_state_attributes``, a property today (the old line number rotted).
|
||||
ENTITY_ATTRIBUTES = frozenset(
|
||||
{
|
||||
"activity",
|
||||
@@ -858,7 +878,77 @@ def scan_entity_contracts(paths=None) -> list[Finding]:
|
||||
_scan_entity_contracts(paths, report)
|
||||
return report.findings
|
||||
|
||||
# ------------------------------------------------------------ unreachable code
|
||||
|
||||
#: Statements that end the block they sit in: nothing AFTER one of these, in the
|
||||
#: SAME block, can ever run. ``return``/``raise`` end the block outright;
|
||||
#: ``continue``/``break`` end this pass through the enclosing loop, which is the
|
||||
#: same thing for the statements that follow them there.
|
||||
_TERMINAL_STATEMENTS = (ast.Return, ast.Raise, ast.Continue, ast.Break)
|
||||
|
||||
|
||||
def _statement_blocks(node: ast.AST):
|
||||
"""Every list-of-statements a node owns (bodies, ``orelse``, ``finalbody``)."""
|
||||
for _field, value in ast.iter_fields(node):
|
||||
if not isinstance(value, list) or not value:
|
||||
continue
|
||||
if all(isinstance(item, ast.stmt) for item in value):
|
||||
yield value
|
||||
|
||||
|
||||
def _scan_unreachable(tree: ast.AST, path: str, report: Report) -> None:
|
||||
"""t62 F2: flag statements that sit after a terminal one in the same block.
|
||||
|
||||
Per-block on purpose: a statement in a DIFFERENT block (an ``else``, an
|
||||
``except`` handler, a nested function) is not dead, and flagging those would
|
||||
make this pass noise. What it does see is the shape that shipped twice — a
|
||||
duplicated body left after ``return cached`` inside the same function
|
||||
(connection.py's label drain) — which is valid Python, invisible to every
|
||||
other gate here, and simply never executed.
|
||||
"""
|
||||
for node in ast.walk(tree):
|
||||
for block in _statement_blocks(node):
|
||||
report.statement_blocks += 1
|
||||
for index, statement in enumerate(block[:-1]):
|
||||
if not isinstance(statement, _TERMINAL_STATEMENTS):
|
||||
continue
|
||||
dead = block[index + 1]
|
||||
report.findings.append(
|
||||
Finding(
|
||||
path,
|
||||
dead.lineno,
|
||||
"unreachable-statement",
|
||||
f"{type(statement).__name__.lower()} at line "
|
||||
f"{statement.lineno} ends this block, so this statement "
|
||||
"can never run (duplicated or leftover code)",
|
||||
)
|
||||
)
|
||||
break
|
||||
|
||||
|
||||
def _scan_unreachable_tree(paths, report: Report) -> None:
|
||||
"""Walk the tree and run the unreachable-statement pass over every file."""
|
||||
for root in paths:
|
||||
root = Path(root)
|
||||
if not root.exists():
|
||||
continue # scan() reports bad-path for the same target
|
||||
for file in _iter_python_files(root):
|
||||
try:
|
||||
tree = ast.parse(file.read_text(encoding="utf-8"), filename=str(file))
|
||||
except (OSError, SyntaxError):
|
||||
continue
|
||||
_scan_unreachable(tree, file.name, report)
|
||||
|
||||
|
||||
def scan_unreachable(paths=None) -> list[Finding]:
|
||||
"""The unreachable-statement pass on its own (t62 F2) — the wrapper's proof."""
|
||||
if paths is None:
|
||||
paths = [SHIELD_DIR]
|
||||
elif isinstance(paths, (str, Path)):
|
||||
paths = [paths]
|
||||
report = Report()
|
||||
_scan_unreachable_tree(paths, report)
|
||||
return report.findings
|
||||
# ----------------------------------------------------------------------- scan
|
||||
|
||||
|
||||
@@ -875,11 +965,13 @@ def scan(paths=None) -> Report:
|
||||
paths = [paths]
|
||||
|
||||
report = Report()
|
||||
# t40/t41: pure-AST checks, so they run even without homeassistant — that is
|
||||
# the whole point of a contract stubs cannot enforce (and a relative import
|
||||
# needs no library to be wrong).
|
||||
# t40/t41/t62: pure-AST checks, so they run even without homeassistant —
|
||||
# that is the whole point of a contract stubs cannot enforce (a relative
|
||||
# import needs no library to be wrong, and dead code needs no API to be
|
||||
# dead).
|
||||
_scan_entity_contracts(paths, report)
|
||||
_scan_package_imports(paths, report)
|
||||
_scan_unreachable_tree(paths, report)
|
||||
if not homeassistant_available():
|
||||
# Neither a false green nor a wall of phantom findings: with no package
|
||||
# to resolve against, every import would "fail", so the sweep reports
|
||||
|
||||
@@ -381,3 +381,102 @@ def test_the_real_ha_matrix_covers_every_entity_class_in_the_tree():
|
||||
f"entity classes never exercised against real HA: {sorted(uncovered)} — "
|
||||
f"add them to _REAL_HA_MATRIX"
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------- t62: dead code and the floor
|
||||
#
|
||||
# Both defects pinned below shipped while every other gate was green:
|
||||
# connection.py carried a duplicated copy of the label-drain body AFTER its
|
||||
# `return cached` (valid Python, dead, invisible to this sweep), and the
|
||||
# call-argument floor sat at 20 while the counts had moved to ~46, so deleting a
|
||||
# flow-display API owner from HA_CALL_OWNERS did not trip it.
|
||||
|
||||
#: t62 F2: the shape that shipped — a bare docstring plus the whole drain left
|
||||
#: after `return cached`, inside the same method. Line 6 is the dead statement.
|
||||
UNREACHABLE_PRE_FIX = (
|
||||
"class Sweep:\n"
|
||||
" def labels(self, requests):\n"
|
||||
" cached = {}\n"
|
||||
" return cached\n"
|
||||
"\n"
|
||||
' """Start at most one label fetch per watchdog cycle."""\n'
|
||||
" if not requests:\n"
|
||||
" return\n"
|
||||
" requests.discard(min(requests))\n"
|
||||
)
|
||||
|
||||
|
||||
def test_the_sweep_flags_the_pre_fix_unreachable_body(tmp_path):
|
||||
"""t62 F2 red proof: the duplicated drain body after `return cached` is dead."""
|
||||
(tmp_path / "dead_sweep.py").write_text(UNREACHABLE_PRE_FIX, encoding="utf-8")
|
||||
|
||||
findings = ha_api_drift.scan_unreachable(tmp_path)
|
||||
|
||||
assert [(finding.path, finding.kind) for finding in findings] == [
|
||||
("dead_sweep.py", "unreachable-statement")
|
||||
], findings
|
||||
assert findings[0].lineno == 6 # the dead docstring, not the return
|
||||
|
||||
|
||||
def test_every_terminal_statement_kind_is_flagged(tmp_path):
|
||||
"""`return`, `raise`, `continue` and `break` all end a block (t62 F2)."""
|
||||
(tmp_path / "terminals.py").write_text(
|
||||
"def a():\n"
|
||||
" return 1\n"
|
||||
" x = 1\n"
|
||||
"\n"
|
||||
"\n"
|
||||
"def b():\n"
|
||||
" raise SystemExit(1)\n"
|
||||
" y = 2\n"
|
||||
"\n"
|
||||
"\n"
|
||||
"def c(items):\n"
|
||||
" for item in items:\n"
|
||||
" continue\n"
|
||||
" z = 3\n"
|
||||
" while True:\n"
|
||||
" break\n"
|
||||
" w = 4\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
findings = ha_api_drift.scan_unreachable(tmp_path)
|
||||
|
||||
assert [finding.kind for finding in findings] == ["unreachable-statement"] * 4, findings
|
||||
|
||||
|
||||
def test_the_production_tree_has_no_unreachable_statements():
|
||||
"""The gate itself: dead code is a finding, so it cannot ship silently."""
|
||||
assert ha_api_drift.scan_unreachable() == []
|
||||
|
||||
|
||||
def test_the_unreachable_pass_walks_real_statement_blocks():
|
||||
"""The floor proves "no findings" is not a pass that stopped parsing."""
|
||||
report = ha_api_drift.scan()
|
||||
|
||||
assert report.statement_blocks >= ha_api_drift.FLOORS["statement_blocks"]
|
||||
|
||||
|
||||
def test_removing_a_flow_display_owner_trips_the_call_argument_floor(monkeypatch):
|
||||
"""t62: the floor must notice an API OWNER going away, not just a bad walk."""
|
||||
_require_installed_homeassistant()
|
||||
|
||||
intact = ha_api_drift.scan()
|
||||
assert intact.floor_violations() == []
|
||||
|
||||
reduced = dict(ha_api_drift.HA_CALL_OWNERS)
|
||||
del reduced["async_show_menu"] # the t57 UNIT 0a owner: 6 checked arguments
|
||||
monkeypatch.setattr(ha_api_drift, "HA_CALL_OWNERS", reduced)
|
||||
|
||||
fewer = ha_api_drift.scan()
|
||||
|
||||
assert fewer.call_args < intact.call_args
|
||||
assert fewer.call_args < ha_api_drift.FLOORS["call_args"], (
|
||||
f"removing async_show_menu left {fewer.call_args} checked arguments, "
|
||||
f"still at or above the floor of {ha_api_drift.FLOORS['call_args']} — "
|
||||
"the flow-display owners are not load-bearing"
|
||||
)
|
||||
assert any("call_args" in line for line in fewer.floor_violations()), (
|
||||
fewer.floor_violations()
|
||||
)
|
||||
Reference in new issue
Block a user