From de76b6e11a007bf74d107cbd1f633d581f23e7b2 Mon Sep 17 00:00:00 2001 From: dev_agent <17+dev_agent@code.cubecraftcreations.com> Date: Sat, 26 Sep 2026 08:49:51 -0400 Subject: [PATCH] Feature review remediation: emoji whitespace-contract parity (U+FEFF) (#249) --- admin/src/icons/registry.test.ts | 26 +++++++++++++- custom_components/nestquest/core/icons.py | 12 +++++-- tests/test_core_icons.py | 41 +++++++++++++++++++++++ tools/icons/emoji-payload-cases.json | 26 ++++++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 tools/icons/emoji-payload-cases.json diff --git a/admin/src/icons/registry.test.ts b/admin/src/icons/registry.test.ts index 23aee40..2101500 100644 --- a/admin/src/icons/registry.test.ts +++ b/admin/src/icons/registry.test.ts @@ -3,12 +3,18 @@ import { join } from "node:path"; import { describe, expect, it } from "vitest"; // The offline generator is the only writer of both registry artefacts. import { OUTPUT_PATHS, REPO_ROOT, SOURCE_PATH, renderRegistry } from "../../../tools/icons/generate.mjs"; -import { FALLBACK_ICON, FA_ICONS, LUCIDE_ICONS, resolveIcon } from "./registry.generated"; +import { EMOJI_MAX_CODE_POINTS, FALLBACK_ICON, FA_ICONS, LUCIDE_ICONS, resolveIcon } from "./registry.generated"; type Curated = { lucide: { name: string; label: string }[]; fa: { name: string; label: string }[] }; const curated = JSON.parse(readFileSync(SOURCE_PATH, "utf8")) as Curated; +type EmojiPayloadCase = { payload: string; valid: boolean; why: string }; + +const emojiPayloadCases = JSON.parse( + readFileSync(join(REPO_ROOT, "tools", "icons", "emoji-payload-cases.json"), "utf8"), +) as { cases: EmojiPayloadCase[] }; + describe("shared icon registry โ€” one source of truth", () => { it("both committed artefacts are byte-identical to a fresh generation", async () => { const fresh = await renderRegistry(); @@ -86,6 +92,24 @@ describe("resolveIcon", () => { } expect(FALLBACK_ICON).toEqual({ kind: "fallback", key: null }); }); + + // tests/test_core_icons.py asserts the same table against the backend's + // validate_icon, so the two emoji rules cannot drift apart silently. + it.each(emojiPayloadCases.cases)("emoji verdict matches the shared table: $why", ({ payload, valid }) => { + const value = `emoji:${payload}`; + expect(resolveIcon(value)).toEqual(valid ? { kind: "emoji", key: value, text: payload } : FALLBACK_ICON); + }); + + it("the shared emoji table covers the contract edges", () => { + const verdicts = new Map(emojiPayloadCases.cases.map(({ payload, valid }) => [payload, valid])); + expect(verdicts.get("\ufeff")).toBe(false); + expect(verdicts.get("๐Ÿ‘จ\u200d๐Ÿ‘ฉ\u200d๐Ÿ‘ง\u200d๐Ÿ‘ฆ")).toBe(true); + expect(verdicts.get("๐Ÿ• ")).toBe(false); + expect(verdicts.get("๐Ÿ•\u0000")).toBe(false); + for (const markup of ["", "a>b", "a&b"]) expect(verdicts.get(markup)).toBe(false); + expect(verdicts.get("โญ".repeat(EMOJI_MAX_CODE_POINTS))).toBe(true); + expect(verdicts.get("โญ".repeat(EMOJI_MAX_CODE_POINTS + 1))).toBe(false); + }); }); function filesUnder(dir: string): string[] { diff --git a/custom_components/nestquest/core/icons.py b/custom_components/nestquest/core/icons.py index 7a2a04b..4d89c63 100644 --- a/custom_components/nestquest/core/icons.py +++ b/custom_components/nestquest/core/icons.py @@ -6,8 +6,9 @@ namespace is one of :data:`ICON_NAMESPACES`: - ``lucide:`` / ``fa:`` โ€” ```` is a lowercase kebab name (ASCII ``[a-z0-9-]``, not starting or ending with ``-``). - ``emoji:`` โ€” a short emoji string of at most - :data:`EMOJI_MAX_CODE_POINTS` code points, with no whitespace, no - control characters and none of ``<``, ``>``, ``&``. + :data:`EMOJI_MAX_CODE_POINTS` code points, with no whitespace (JS + ``\\s``, so including U+FEFF), no control characters and none of + ``<``, ``>``, ``&``. Rows written before namespacing hold a bare Lucide name (``dog``). :func:`validate_icon` (writes) normalizes such a legacy value to @@ -33,7 +34,12 @@ ICON_NAMESPACES = frozenset( EMOJI_MAX_CODE_POINTS = 8 _KEBAB_NAME = re.compile(r"[a-z0-9](?:[a-z0-9-]*[a-z0-9])?") -_EMOJI_FORBIDDEN = frozenset("<>&") +# U+FEFF (BOM / zero-width no-break space) is neither str.isspace() nor +# ``Cc`` in Python, but JS ``\s`` matches it, so it is listed here to keep +# this rule identical to the frontends' isEmojiPayload (/[\s<>&]|\p{Cc}/u). +# Other format characters, notably the ZWJ U+200D that emoji sequences +# need, stay valid in both runtimes. +_EMOJI_FORBIDDEN = frozenset("<>&\ufeff") def _is_kebab_name(value: str) -> bool: diff --git a/tests/test_core_icons.py b/tests/test_core_icons.py index de454d5..50de248 100644 --- a/tests/test_core_icons.py +++ b/tests/test_core_icons.py @@ -6,6 +6,9 @@ lets legacy bare Lucide names render without a data migration. """ from __future__ import annotations +import json +from pathlib import Path + import pytest from custom_components.nestquest.core.icons import ( @@ -70,6 +73,8 @@ def test_validate_icon_clears(value: str | None) -> None: "emoji:a&b", # entity start "emoji:๐Ÿ•\x00", # control character "emoji:๐Ÿ•\x07๐Ÿ•", # control character between emoji + "emoji:\ufeff", # U+FEFF BOM: JS \s matches it, so reject to agree + "emoji:๐Ÿ•\ufeff", # trailing U+FEFF # Edge whitespace/controls are part of the value, never trimmed away. "emoji:๐Ÿ• ", # trailing space "emoji:๐Ÿ•\n", # trailing newline @@ -109,3 +114,39 @@ def test_validate_icon_rejects_non_strings(value: object) -> None: ) def test_normalize_icon_for_read(value: str | None, expected: str | None) -> None: assert normalize_icon_for_read(value) == expected + + +# The emoji-payload rule is duplicated in the generated JS resolver +# (``isEmojiPayload`` in tools/icons/generate.mjs). Both suites assert +# this one table โ€” admin/src/icons/registry.test.ts runs it through +# resolveIcon โ€” so a divergence between the runtimes fails a test. +_EMOJI_PARITY_CASES = json.loads( + ( + Path(__file__).parent.parent / "tools" / "icons" / "emoji-payload-cases.json" + ).read_text(encoding="utf-8") +)["cases"] + + +@pytest.mark.parametrize( + "case", _EMOJI_PARITY_CASES, ids=[case["why"] for case in _EMOJI_PARITY_CASES] +) +def test_validate_icon_emoji_verdict_matches_shared_table(case: dict) -> None: + value = "emoji:" + case["payload"] + if case["valid"]: + assert validate_icon(value) == value + else: + with pytest.raises(ValueError, match=r"^icon\b"): + validate_icon(value) + + +def test_emoji_parity_table_covers_the_contract_edges() -> None: + payloads = {case["payload"]: case["valid"] for case in _EMOJI_PARITY_CASES} + assert payloads["\ufeff"] is False + assert payloads["๐Ÿ• "] is False + assert payloads["๐Ÿ•\x00"] is False + assert payloads[""] is False and payloads["a>b"] is False + assert payloads["a&b"] is False + assert payloads["โญ" * EMOJI_MAX_CODE_POINTS] is True + assert payloads["โญ" * (EMOJI_MAX_CODE_POINTS + 1)] is False + family = "๐Ÿ‘จ\u200d๐Ÿ‘ฉ\u200d๐Ÿ‘ง\u200d๐Ÿ‘ฆ" + assert payloads[family] is True diff --git a/tools/icons/emoji-payload-cases.json b/tools/icons/emoji-payload-cases.json new file mode 100644 index 0000000..b6513a5 --- /dev/null +++ b/tools/icons/emoji-payload-cases.json @@ -0,0 +1,26 @@ +{ + "_comment": "Shared emoji-payload verdicts. tests/test_core_icons.py (validate_icon) and admin/src/icons/registry.test.ts (resolveIcon) both assert every row, so the Python and JS rules cannot diverge silently.", + "cases": [ + { "payload": "๐Ÿ•", "valid": true, "why": "plain emoji" }, + { "payload": "๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ", "valid": true, "why": "ZWJ family (U+200D allowed)" }, + { "payload": "โญโญโญโญโญโญโญโญ", "valid": true, "why": "exactly 8 code points" }, + { "payload": "โญโญโญโญโญโญโญโญโญ", "valid": false, "why": "9 code points, over the cap" }, + { "payload": "", "valid": false, "why": "empty" }, + { "payload": "๏ปฟ", "valid": false, "why": "U+FEFF BOM alone" }, + { "payload": "๐Ÿ•๏ปฟ", "valid": false, "why": "trailing U+FEFF" }, + { "payload": "๐Ÿ• ", "valid": false, "why": "trailing space" }, + { "payload": "a b", "valid": false, "why": "inner space" }, + { "payload": "๐Ÿ•ย ", "valid": false, "why": "no-break space" }, + { "payload": "๐Ÿ•ใ€€", "valid": false, "why": "ideographic space" }, + { "payload": "๐Ÿ•โ€จ", "valid": false, "why": "line separator" }, + { "payload": "๐Ÿ•\n", "valid": false, "why": "newline" }, + { "payload": "๐Ÿ•\u0000", "valid": false, "why": "NUL control" }, + { "payload": "๐Ÿ•\u0007๐Ÿ•", "valid": false, "why": "BEL control" }, + { "payload": "๐Ÿ•\u001c", "valid": false, "why": "file-separator control" }, + { "payload": "๐Ÿ•\u0085", "valid": false, "why": "NEL C1 control" }, + { "payload": "๐Ÿ•\u007f", "valid": false, "why": "DEL control" }, + { "payload": "", "valid": false, "why": "markup <" }, + { "payload": "a>b", "valid": false, "why": "markup >" }, + { "payload": "a&b", "valid": false, "why": "entity start &" } + ] +}