fix(i18n): write each refusal where it is sent, and read only that form
A refusal's detail now stands where it is sent (HTTPException's detail, a JSONResponse's "detail") or is returned by a factory that only ever is, with any spread before its code and reason, so no spread can replace them. The scan follows no variable and no kwargs any more; what it cannot read in a reason (a lambda's or comprehension's name, a nonlocal or global, a local that reaches itself, a constant or map changed elsewhere, an app import it cannot resolve) is reported. The detail guard reads the same slots and fails on anything but a code dict or a factory's call.
This commit is contained in:
@@ -1,57 +1,16 @@
|
||||
"""No refusal answers in a hand-written English sentence.
|
||||
"""No refusal answers in a way the client cannot say.
|
||||
|
||||
The UI is bilingual and translates a structured `detail` (`{code, reason, …}`), but a string detail it
|
||||
can only print as it came — so ~150 of them reached the Hungarian UI verbatim, in the blocking error
|
||||
dialog and on every inline surface that shows `HttpError.detail`. `app/refusal.py` is the family they
|
||||
became; this test is what keeps a new one from being written the old way. Pure: it reads the sources.
|
||||
became; this test is what keeps a new one from being written the old way. It reads the same slots the
|
||||
refusal scan does (`refusal_sites.detail_slots`), so the two cannot disagree on where a detail is sent.
|
||||
Pure: it reads the sources.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import pathlib
|
||||
|
||||
APP = pathlib.Path(__file__).resolve().parents[1] / "app"
|
||||
|
||||
|
||||
def _is_text(node: ast.AST) -> bool:
|
||||
"""A detail that is a sentence: a literal, an f-string, `str(exc)`, or `"…" + x`."""
|
||||
if isinstance(node, ast.Constant) and isinstance(node.value, str):
|
||||
return True
|
||||
if isinstance(node, ast.JoinedStr):
|
||||
return True
|
||||
if isinstance(node, ast.Call) and getattr(node.func, "id", "") == "str":
|
||||
return True
|
||||
if isinstance(node, ast.BoolOp):
|
||||
return any(_is_text(v) for v in node.values)
|
||||
if isinstance(node, ast.BinOp):
|
||||
return _is_text(node.left) or _is_text(node.right)
|
||||
return False
|
||||
|
||||
|
||||
def test_no_http_exception_carries_a_sentence():
|
||||
offenders = []
|
||||
for path in sorted(APP.rglob("*.py")):
|
||||
for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))):
|
||||
if not (
|
||||
isinstance(node, ast.Call)
|
||||
and getattr(node.func, "id", getattr(node.func, "attr", "")) == "HTTPException"
|
||||
):
|
||||
continue
|
||||
kw = {k.arg: k.value for k in node.keywords}
|
||||
detail = kw.get("detail", node.args[1] if len(node.args) > 1 else None)
|
||||
if detail is not None and _is_text(detail):
|
||||
offenders.append(f"{path.relative_to(APP).as_posix()}:{node.lineno}")
|
||||
assert offenders == [], (
|
||||
"a refusal with an English sentence for a detail — raise `refused(status, reason)` "
|
||||
f"(app/refusal.py) and name the reason in both locales: {offenders}"
|
||||
)
|
||||
|
||||
|
||||
def test_the_scan_sees_a_sentence_when_there_is_one():
|
||||
"""A guard that matches nothing is green over nothing."""
|
||||
sample = ast.parse('HTTPException(status_code=400, detail=f"bad {x}")').body[0].value
|
||||
assert _is_text(sample.keywords[1].value)
|
||||
assert not _is_text(ast.parse('{"code": "refused", "reason": "x"}').body[0].value)
|
||||
import textwrap
|
||||
|
||||
from tests.refusal_sites import Scan, app_sources
|
||||
|
||||
# Answers that are no refusal of the app's, by the function they are sent from: the client shows a
|
||||
# detail only when it is an object (`lib/api/core.ts`), so these read as its generic line, and no screen
|
||||
@@ -59,62 +18,21 @@ def test_the_scan_sees_a_sentence_when_there_is_one():
|
||||
NO_REFUSAL = {
|
||||
("main.py", "setup_gate"): "the 503 before setup: the setup screen is what the user sees",
|
||||
("main.py", "spa_fallback"): "a bare 404 for an api path no route matches: no screen calls one",
|
||||
("main.py", "_validation_error"): "FastAPI's own 422 list: a body the client itself got wrong",
|
||||
("routes/diagnostics.py", "_parsed_events"): "the client-events beacon's 422: nothing shows it",
|
||||
}
|
||||
|
||||
|
||||
def _detail_offenders(source: str, path: str) -> list[str]:
|
||||
"""Each detail the client could not say: a dict without a `code`, a sentence, or none at all.
|
||||
|
||||
`HTTPException(status, detail)` and `JSONResponse({"detail": …})`; a variable or a call is the
|
||||
refusal scan's to read (`refusal_sites.py`, which follows each `code` dict)."""
|
||||
tree = ast.parse(source)
|
||||
parents = {child: node for node in ast.walk(tree) for child in ast.iter_child_nodes(node)}
|
||||
out = []
|
||||
for node in ast.walk(tree):
|
||||
name = isinstance(node, ast.Call) and getattr(
|
||||
node.func, "id", getattr(node.func, "attr", "")
|
||||
)
|
||||
if name == "HTTPException":
|
||||
kw = {k.arg: k.value for k in node.keywords}
|
||||
detail = kw.get("detail", node.args[1] if len(node.args) > 1 else None)
|
||||
elif name == "JSONResponse":
|
||||
content = {k.arg: k.value for k in node.keywords}.get(
|
||||
"content", node.args[0] if node.args else None
|
||||
)
|
||||
if not isinstance(content, ast.Dict) or not any(
|
||||
isinstance(k, ast.Constant) and k.value == "detail" for k in content.keys
|
||||
):
|
||||
continue # an answer, not a refusal
|
||||
detail = next(
|
||||
v
|
||||
for k, v in zip(content.keys, content.values, strict=True)
|
||||
if isinstance(k, ast.Constant) and k.value == "detail"
|
||||
)
|
||||
else:
|
||||
continue
|
||||
fn = parents.get(node)
|
||||
while fn is not None and not isinstance(fn, (ast.FunctionDef, ast.AsyncFunctionDef)):
|
||||
fn = parents.get(fn)
|
||||
if (path, getattr(fn, "name", None)) in NO_REFUSAL:
|
||||
continue
|
||||
keys = (
|
||||
[k.value for k in detail.keys if isinstance(k, ast.Constant)]
|
||||
if isinstance(detail, ast.Dict)
|
||||
else [k.arg for k in detail.keywords]
|
||||
if isinstance(detail, ast.Call) and getattr(detail.func, "id", "") == "dict"
|
||||
else None
|
||||
)
|
||||
if detail is None or _is_text(detail) or (keys is not None and "code" not in keys):
|
||||
out.append(f"{path}:{node.lineno}")
|
||||
return out
|
||||
def _offenders(sources: dict[str, str]) -> list[str]:
|
||||
return [
|
||||
f"{path}:{line}"
|
||||
for path, line, fn in Scan(sources).other_details()
|
||||
if (path, fn) not in NO_REFUSAL
|
||||
]
|
||||
|
||||
|
||||
def test_every_detail_is_one_the_client_can_say():
|
||||
offenders = [
|
||||
o
|
||||
for p in sorted(APP.rglob("*.py"))
|
||||
for o in _detail_offenders(p.read_text(encoding="utf-8"), p.relative_to(APP).as_posix())
|
||||
]
|
||||
offenders = _offenders(app_sources())
|
||||
assert offenders == [], (
|
||||
"a refusal the client can only answer with its generic line — send `{code, reason}` "
|
||||
f"(`refused(status, reason)`), or list it in NO_REFUSAL with why: {offenders}"
|
||||
@@ -122,23 +40,49 @@ def test_every_detail_is_one_the_client_can_say():
|
||||
|
||||
|
||||
def test_the_detail_scan_sees_each_shape_the_client_cannot_say():
|
||||
src = """
|
||||
def f(x):
|
||||
raise HTTPException(409, detail={"reason": "view_name_taken"})
|
||||
def g(x):
|
||||
raise HTTPException(409, dict(reason="taken"))
|
||||
def h(x):
|
||||
raise HTTPException(status_code=404)
|
||||
def i(x):
|
||||
return JSONResponse({"detail": "This instance isn't set up yet."}, status_code=503)
|
||||
def j(x):
|
||||
return JSONResponse(content={"detail": {"reason": "x"}}, status_code=409)
|
||||
def ok(x):
|
||||
raise HTTPException(409, {"code": "refused", "reason": "x"})
|
||||
raise HTTPException(409, detail(x))
|
||||
return JSONResponse({"detail": {"code": "too_large"}}, status_code=413)
|
||||
return JSONResponse({"ok": True})
|
||||
def setup_gate(x):
|
||||
return JSONResponse({"detail": "listed"}, status_code=503)
|
||||
"""
|
||||
assert _detail_offenders(src, "main.py") == [f"main.py:{n}" for n in (3, 5, 7, 9, 11)]
|
||||
"""A guard that matches nothing is green over nothing."""
|
||||
src = textwrap.dedent(
|
||||
"""
|
||||
def f(x):
|
||||
raise HTTPException(409, detail={"reason": "view_name_taken"})
|
||||
def g(x):
|
||||
raise HTTPException(409, dict(reason="taken"))
|
||||
def h(x):
|
||||
raise HTTPException(status_code=404)
|
||||
def i(x):
|
||||
return JSONResponse({"detail": "This instance isn't set up yet."}, status_code=503)
|
||||
def j(x):
|
||||
return JSONResponse(content={"detail": {"reason": "x"}}, status_code=409)
|
||||
def k(x):
|
||||
raise HTTPException(400, detail=f"bad {x}")
|
||||
def l(x):
|
||||
raise HTTPException(404, None)
|
||||
def n(x, msg):
|
||||
raise HTTPException(400, detail=msg)
|
||||
def o(x):
|
||||
raise HTTPException(400, detail=["a sentence"])
|
||||
def p(x):
|
||||
raise HTTPException(400, detail=describe(x))
|
||||
def q(x):
|
||||
raise HTTPException(400, detail={"code": "a"} if x else "text")
|
||||
def r(x):
|
||||
raise HTTPException(400, detail=nothing(x))
|
||||
def nothing(x):
|
||||
pass
|
||||
def ok(x):
|
||||
raise HTTPException(409, {"code": "refused", "reason": "x"})
|
||||
raise HTTPException(409, detail=quota(x))
|
||||
raise HTTPException(409, detail={"code": "a"} if x else {"code": "b"})
|
||||
return JSONResponse({"detail": {"code": "too_large"}}, status_code=413)
|
||||
return JSONResponse({"ok": True})
|
||||
def quota(x):
|
||||
return {"code": "quota", "reason": "files"}
|
||||
def setup_gate(x):
|
||||
return JSONResponse({"detail": "listed"}, status_code=503)
|
||||
"""
|
||||
)
|
||||
lines = (3, 5, 7, 9, 11, 13, 15, 17, 19, 21, 23, 25)
|
||||
assert _offenders({"main.py": src}) == [f"main.py:{n}" for n in lines]
|
||||
# A file whose code dicts are data still answers its refusals in a way the client can say.
|
||||
data = 'def f():\n raise HTTPException(400, "text")\n'
|
||||
assert _offenders({"plex/tmdb.py": data}) == ["plex/tmdb.py:2"]
|
||||
|
||||
Reference in New Issue
Block a user