Skip to content

Commit 6db26cc

Browse files
gilesknapclaude
andcommitted
Fail fast on punctuation-only controller ids in EPICS GUI emission
The EPICS CA id validator accepts `[A-Za-z0-9_-]+`, so ids like `"___"` and `"-"` pass validation and reach `_coerce_pascal_name` in the GUI emission path. That helper delegates to `pvi.device.enforce_pascal_case`, which strips non-Pascal characters and unconditionally indexes `s[0]` on the result. When every character is stripped the index raises `IndexError`, blowing up GUI emission at `connect()` time with an opaque traceback. Pre-strip the id with the same regex pvi uses (`NON_PASCAL_CHARS_RE`) and raise `ValueError` with a message that names the offending id when the strip yields the empty string. Choosing fail-fast over an `"X"` fallback keeps the failure traceable: a silent fallback would generate nonsense GUI names that the user would have to reverse-engineer back to the bad id. Fixes #369 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4853d0e commit 6db26cc

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

src/fastcs/transports/epics/emission.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
from collections.abc import Callable
1212

1313
from pvi._format.dls import DLSFormatter # type: ignore
14-
from pvi.device import Device, DeviceRef, enforce_pascal_case # type: ignore
14+
from pvi.device import ( # type: ignore
15+
NON_PASCAL_CHARS_RE,
16+
Device,
17+
DeviceRef,
18+
enforce_pascal_case,
19+
)
1520

1621
from fastcs.controllers import ControllerAPI
1722
from fastcs.logging import logger
@@ -36,9 +41,24 @@ def _coerce_pascal_name(controller_id: str) -> str:
3641
union of every transport's charset and may legitimately start with a
3742
digit (e.g. UUID-flavoured test prefixes), so we prepend ``X`` when
3843
needed before delegating to ``pvi.device.enforce_pascal_case``.
44+
45+
Some otherwise-valid transport ids (the EPICS CA validator accepts
46+
``[A-Za-z0-9_-]+`` so ``"___"`` and ``"-"`` are legal) contain no
47+
Pascal-usable characters at all. ``enforce_pascal_case`` strips
48+
everything and then unconditionally indexes ``s[0]``, raising
49+
``IndexError`` on the empty string. We fail fast with a clearer
50+
``ValueError`` instead -- a silent fallback would produce nonsense
51+
GUI names that the user can't trace back to the offending id.
3952
"""
53+
stripped = NON_PASCAL_CHARS_RE.sub("", controller_id)
54+
if not stripped:
55+
raise ValueError(
56+
f"Controller id {controller_id!r} contains no characters usable "
57+
"in a Pascal-case name; pick an id with at least one ASCII "
58+
"letter or digit"
59+
)
4060
candidate = enforce_pascal_case(controller_id)
41-
if candidate and not candidate[0].isupper():
61+
if not candidate[0].isupper():
4262
candidate = "X" + candidate
4363
return candidate
4464

tests/transports/epics/test_emission.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from fastcs.transports.epics.emission import (
1111
DOCS_EXT,
1212
INDEX_STEM,
13+
_coerce_pascal_name,
1314
emit_docs_files,
1415
emit_gui_files,
1516
)
@@ -174,3 +175,32 @@ def test_docs_creates_missing_output_dir(two_apis, tmp_path: Path):
174175

175176
assert (nested / f"alpha{DOCS_EXT}").is_file()
176177
assert (nested / f"{INDEX_STEM}{DOCS_EXT}").is_file()
178+
179+
180+
# --- _coerce_pascal_name fail-fast (#369) ----------------------------------
181+
182+
183+
@pytest.mark.parametrize("bad_id", ["___", "-", "_-_", "----"])
184+
def test_coerce_pascal_name_rejects_all_punctuation_ids(bad_id: str):
185+
"""Ids that strip to empty would IndexError inside ``enforce_pascal_case``.
186+
187+
The EPICS CA validator accepts ``[A-Za-z0-9_-]+`` so ids like ``"___"``
188+
and ``"-"`` reach GUI emission. We raise ``ValueError`` rather than
189+
falling back to a generated name so the offending id surfaces in the
190+
error message instead of producing a nonsense PascalStr the user
191+
can't trace.
192+
"""
193+
with pytest.raises(ValueError, match=repr(bad_id)) as excinfo:
194+
_coerce_pascal_name(bad_id)
195+
196+
assert "Pascal-case" in str(excinfo.value)
197+
198+
199+
def test_coerce_pascal_name_accepts_digit_leading_id():
200+
"""Happy path: digit-leading ids are still coerced via ``X`` prefix."""
201+
assert _coerce_pascal_name("120e1b6") == "X120e1b6"
202+
203+
204+
def test_coerce_pascal_name_accepts_normal_pascal_id():
205+
"""Happy path: ordinary Pascal-friendly ids round-trip cleanly."""
206+
assert _coerce_pascal_name("alpha") == "Alpha"

0 commit comments

Comments
 (0)