diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index fc4986c..ad6a9d6 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -165,7 +165,18 @@ jobs: # ruff is pre-installed in the ci-python image — no install # step needed, lint runs in ~2s. - name: Lint - run: ruff check src/ + run: ruff check src/ scripts/ + + # Design tokens: does the frontend's CSS agree with the stylesheet the + # design system generates? Fails only on an unresolvable var() reference — + # that count is at zero, so this is a ratchet rather than a backlog. The + # literal findings are printed, not gated; hundreds exist and a + # permanently-red job is one nobody reads. + # + # Stdlib only, no install, no network: the source of truth is theme.css, + # which is generated from the design system and committed. + - name: Design token check + run: python3 scripts/check_design_tokens.py --report-literals test: name: Python tests diff --git a/frontend/src/assets/theme.css b/frontend/src/assets/theme.css index 1b26bfb..0484367 100644 --- a/frontend/src/assets/theme.css +++ b/frontend/src/assets/theme.css @@ -178,6 +178,14 @@ --fs-text-tertiary: #9A9890; } +/* SUPERSEDES — write the token, not the literal. + * #fff -> --fs-text-primary + * #ffffff -> --fs-text-primary + * white -> --fs-text-primary + * bold -> --fs-weight-medium + * bolder -> --fs-weight-medium + */ + /* ========================================================================== COMPATIBILITY ALIASES — the app's historical names, pointing at the system. diff --git a/scripts/check_design_tokens.py b/scripts/check_design_tokens.py new file mode 100644 index 0000000..558f2b1 --- /dev/null +++ b/scripts/check_design_tokens.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Check the frontend's CSS against the tokens its stylesheet declares. + +The gap this closes: `services/design_stylesheet.check_code_against_tokens` has +always been able to answer "does this code use the sheet correctly?", but the +only thing ever fed to it was recorded SNIPPETS. The app's own components — where +sixteen unresolvable references were found living quietly (#2319) — were checked +by nothing at all. + +That was structural rather than an oversight. The drift panel runs in the browser +and cannot read source files, and the server has no repo access. CI is the only +place holding both the component sources and the ability to run the check, and it +only became cheap once `theme.css` became a generated artifact — so the source of +truth is a local file, with no network and no credentials. + +INSTANCE-AGNOSTIC ON PURPOSE (rule #115). Nothing here knows what a token should +be called or which literals are discouraged. Both come from the stylesheet: the +declarations, and the `SUPERSEDES` block the generator emits. Point it at a +different install's sheet and it checks that install's rules. + +Two severities, and the split is deliberate: + + FAIL an unresolvable `var()` reference. Currently zero, so this is a ratchet + that holds a line already reached rather than a backlog that keeps CI + red. It also cannot false-positive: either the name is declared or it + is not. + REPORT superseded literals and raw colour literals. Hundreds today, so gating + on them would mean a permanently failing job that everyone learns to + ignore — which is worse than no check. +""" +from __future__ import annotations + +import argparse +import pathlib +import re +import sys + +# A declaration is `--name:`; a reference is `var(--name)` or `var(--name, …)`. +DECLARATION = re.compile(r"(?\s*(--[A-Za-z0-9_-]+)\s*$") +HEX_LITERAL = re.compile(r"#[0-9a-fA-F]{3,8}\b") +STYLE_BLOCK = re.compile(r"]*>(.*?)", re.S) +CSS_COMMENT = re.compile(r"/\*.*?\*/", re.S) + + +def declared_tokens(sheet: str) -> set[str]: + """Every custom property the stylesheet declares. + + Anchored on the colon alone. Anchoring on `{` or `;` instead silently drops + every declaration that follows a comment — a mistake made once already, which + lost `--color-bg` and 2 others without erroring. + """ + return set(DECLARATION.findall(sheet)) + + +def superseded_literals(sheet: str) -> dict[str, str]: + """`{literal: token}` from the generator's SUPERSEDES block, lowercased.""" + out: dict[str, str] = {} + for line in sheet.splitlines(): + match = SUPERSEDES_LINE.match(line) + if match: + out[match.group(1).lower()] = match.group(2) + return out + + +def _literal_pattern(literal: str) -> re.Pattern: + """Match a literal without matching a longer one containing it. + + `#fff` must not fire inside `#ffffff`: different colours, and a finding on + the wrong one sends someone to change correct code. + """ + return re.compile( + r"(? str: + """The CSS in a file — `\n' + ) + css = check.style_source(sfc) + assert "--fs-text-primary" in css + assert "#fff" not in css + + +def test_a_comment_explaining_a_rule_is_not_a_violation(tmp_path): + """LOAD-BEARING, and it fired on the first real run. A comment documenting + why a literal is avoided necessarily contains that literal — this codebase's + own stylesheet says so about `#fff`. A checker that flags the documentation + of a rule teaches people to stop documenting rules.""" + sfc = tmp_path / "Y.vue" + sfc.write_text( + "\n" + ) + css = check.style_source(sfc) + assert check._literal_pattern("#fff").search(css) is None + + +def test_a_plain_css_file_is_read_whole(tmp_path): + css_file = tmp_path / "shared.css" + css_file.write_text(".a { color: #fff; }") + assert "#fff" in check.style_source(css_file) + + +# --- the gate --------------------------------------------------------------- + +def _run(tmp_path, sheet: str, component: str, monkeypatch, capsys): + (tmp_path / "assets").mkdir(parents=True, exist_ok=True) + sheet_path = tmp_path / "assets" / "theme.css" + sheet_path.write_text(sheet) + (tmp_path / "C.vue").write_text(f"") + monkeypatch.setattr( + "sys.argv", + ["check", "--sheet", str(sheet_path), "--root", str(tmp_path)], + ) + code = check.main() + return code, capsys.readouterr().out + + +def test_an_unresolvable_reference_fails_the_build(tmp_path, monkeypatch, capsys): + """The one hard gate. It is safe to gate on because the count is zero today — + a ratchet holding a line already reached, not a backlog that keeps CI red.""" + code, out = _run(tmp_path, SHEET, ".a { color: var(--nope); }", monkeypatch, capsys) + assert code == 1 + assert "--nope" in out + + +def test_a_resolvable_reference_passes(tmp_path, monkeypatch, capsys): + code, out = _run( + tmp_path, SHEET, ".a { color: var(--fs-text-primary); }", monkeypatch, capsys + ) + assert code == 0 + assert "every var() reference resolves" in out + + +def test_a_locally_declared_property_is_not_unresolved(tmp_path, monkeypatch, capsys): + """A component may legitimately define its own custom property for local use — + a keyframe variable, a per-instance override. Only a reference to a name that + exists NOWHERE is broken.""" + code, _ = _run( + tmp_path, SHEET, ".a { --local: 4px; padding: var(--local); }", monkeypatch, capsys + ) + assert code == 0 + + +def test_a_superseded_literal_reports_but_does_not_fail(tmp_path, monkeypatch, capsys): + """Hundreds exist. Gating would make a permanently-red job, which is a check + nobody reads — worse than no check at all.""" + code, out = _run(tmp_path, SHEET, ".a { color: #fff; }", monkeypatch, capsys) + assert code == 0 + assert "#fff -> --fs-text-primary" in out + + +def test_a_missing_stylesheet_is_an_error_not_a_pass(tmp_path, monkeypatch, capsys): + """If the sheet moves, the check must fail loudly rather than silently + passing with zero tokens to compare against — which would look identical to + a clean run.""" + monkeypatch.setattr( + "sys.argv", ["check", "--sheet", str(tmp_path / "gone.css"), "--root", str(tmp_path)] + ) + assert check.main() == 2