diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index c87a752..01668c1 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://anthropic.com/claude-code/plugin.schema.json", "name": "plc-programming", - "version": "0.2.0", + "version": "0.2.1", "description": "PLC programming across IEC 61131-3 (ST, LD, FBD, SFC, IL), PLCopen XML / IEC 61131-10, Siemens TIA (SCL, SimaticML, SIMATIC SD), Schneider (Machine Expert, Control Expert), CODESYS, TwinCAT, ABB Freelance, Rockwell L5X/L5K, IEC 61499, AutomationML, DEXPI, PackML/ISA-88 and OPC UA: design, write, convert, review and debug programs, with a stdlib-only structural validator.", "author": { "name": "SmailG", diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 61ab843..6a7f7ee 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -11,5 +11,5 @@ - [ ] `python3 -m unittest discover -s tests` passes - [ ] A new validator rule has a must-catch input and a valid control that must pass - [ ] New facts in a reference file cite their source, or are marked unverified -- [ ] Version bumped in `.claude-plugin/plugin.json` +- [ ] Version bumped in both `.claude-plugin/plugin.json` and the root `plugin.json` - [ ] README updated if a skill, command or validator rule changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57cc341..517b174 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,8 +28,9 @@ privately, as described in [SECURITY.md](SECURITY.md). Pull requests use the tem ask the user rather than assume. - **The hook must never break a session:** non-PLC files and unreadable events exit 0, and a validator bug exits 1, which Claude Code shows but does not block on. Only real errors exit 2. -- **Bump the version** in `.claude-plugin/plugin.json` on every change; `claude plugin update` - does nothing while it is unchanged. +- **Bump the version** in both `.claude-plugin/plugin.json` and the root `plugin.json` on every + change, to the same value (a test checks it); `claude plugin update` does nothing while it is + unchanged. - Do not bundle vendor schemas or manuals whose licence does not clearly permit redistribution. - **Test fixtures for the `.project` reader are synthetic.** Never commit a real `.project`, or names, text or guids taken from one: it is someone's program. @@ -44,3 +45,10 @@ claude plugin eval . --trust-plugin --allow-tools Write Edit "Bash(python3 *)" - Read an eval's `Δ` (with plugin minus without), not its score: a case the baseline already passes shows nothing about the plugin. See the README for the macOS eval workaround. + +### Mutation tests + +Run mutation tests (by hand or with an AI agent) one at a time, each as +`timeout -k 10 300 `: an external wrapper, because a timeout inside the runner dies with +it. That is GNU `timeout` (macOS: `brew install coreutils`, then `gtimeout`). Check for +leftover processes at the end: `ps -eo pid,etime,pcpu,command | grep `. diff --git a/plugin.json b/plugin.json index bfedbe6..b97a096 100644 --- a/plugin.json +++ b/plugin.json @@ -1,7 +1,7 @@ { "name": "plc-programming", "displayName": "PLC Programming", - "version": "0.2.0", + "version": "0.2.1", "description": "PLC programming across IEC 61131-3 (ST, LD, FBD, SFC, IL), PLCopen XML / IEC 61131-10, Siemens TIA, Schneider, CODESYS, TwinCAT, ABB Freelance, Rockwell L5X/L5K, IEC 61499, AutomationML, DEXPI, PackML/ISA-88 and OPC UA with a stdlib-only structural validator.", "author": { "name": "SmailG", diff --git a/scripts/plccheck/aml.py b/scripts/plccheck/aml.py index 82edad6..c3f6ecc 100644 --- a/scripts/plccheck/aml.py +++ b/scripts/plccheck/aml.py @@ -72,7 +72,8 @@ def check_aml(root: Node, path: Path | None = None) -> list[Finding]: continue if not target.exists(): out.append(warning("AM011", f"refURI target {target.name} not found next to this file", value.line)) - elif fragment and cls.endswith("PLCopenXMLInterface"): + elif fragment and cls.endswith("PLCopenXMLInterface") and target.is_file(): + # An absolute refURI can name a directory, FIFO or device; only a regular file is read. try: doc = parse_file(target) except XmlError: diff --git a/scripts/plccheck/xmlutil.py b/scripts/plccheck/xmlutil.py index 4c6b20f..0c55b02 100644 --- a/scripts/plccheck/xmlutil.py +++ b/scripts/plccheck/xmlutil.py @@ -1,7 +1,10 @@ """Minimal namespace-aware XML tree with line numbers (expat, stdlib only). ElementTree drops line numbers, and a finding without a line is half as useful. -External entities and DTDs are never fetched. +External entities and DTDs are never fetched, and entity declarations are refused. Because +entity declarations precede the root element, the format sniffer stops at the first one too: +a file that declares an entity is not detected as any format, so the edit-time hook skips it, +and validating it with an explicit format reports XML001. """ from __future__ import annotations @@ -55,6 +58,10 @@ def all_text(self) -> str: return self.text + "".join(c.all_text() for c in self.children) +class _EntityDeclared(Exception): + """Raised inside sniff_root's parser at the first entity declaration.""" + + class XmlError(Exception): def __init__(self, message: str, line: int | None): super().__init__(message) @@ -66,6 +73,7 @@ def parse_bytes(data: bytes) -> Node: parser.SetParamEntityParsing(xml.parsers.expat.XML_PARAM_ENTITY_PARSING_NEVER) root: list[Node] = [] stack: list[Node] = [] + chunks: list[list[str]] = [] # character data of each open element, joined once at its end def split(name: str) -> tuple[str, str]: if "}" in name: @@ -81,20 +89,26 @@ def start(name, attrs): else: root.append(node) stack.append(node) + chunks.append([]) def end(_name): - stack.pop() + node = stack.pop() + node.text = "".join(chunks.pop()) def chars(data): if stack: - node = stack[-1] - if not node.text: - node.text_line = parser.CurrentLineNumber - node.text += data + if not chunks[-1]: + stack[-1].text_line = parser.CurrentLineNumber + chunks[-1].append(data) + + def entity_decl(*_args): + # Expanding entities makes the work grow with the nesting, not the file size. + raise XmlError("XML entity declarations are not supported", parser.CurrentLineNumber) parser.StartElementHandler = start parser.EndElementHandler = end parser.CharacterDataHandler = chars + parser.EntityDeclHandler = entity_decl try: parser.Parse(data, True) except xml.parsers.expat.ExpatError as exc: @@ -109,7 +123,13 @@ def parse_file(path: Path) -> Node: def sniff_root(path: Path) -> tuple[str, str] | None: - """(namespace, local name) of the root element, or None if it is not XML.""" + """(namespace, local name) of the root element, or None if it is not XML. + + A document that declares an entity gives ("", ""), which matches no format: the declaration + comes before the root, so the parser stops before any entity is expanded (a root attribute + or an ATTLIST default could otherwise expand to megabytes). None would read as broken XML, + and the hook would then block edits to ordinary non-PLC .xml files that declare entities. + """ try: head = path.read_bytes()[:4096] except OSError: @@ -124,11 +144,17 @@ def start(name, _attrs): found.append(name) raise StopIteration + def entity_decl(*_args): + raise _EntityDeclared + parser.StartElementHandler = start + parser.EntityDeclHandler = entity_decl try: parser.Parse(head, False) except StopIteration: pass + except _EntityDeclared: + return "", "" except xml.parsers.expat.ExpatError: return None if not found: diff --git a/tests/test_formats.py b/tests/test_formats.py index 4f0b871..7032694 100644 --- a/tests/test_formats.py +++ b/tests/test_formats.py @@ -10,6 +10,7 @@ import subprocess import sys import tempfile +import time import unittest from pathlib import Path @@ -17,7 +18,9 @@ sys.path.insert(0, str(HERE.parent / "scripts")) import plc_validate # noqa: E402 +from plccheck.xmlutil import XmlError, parse_bytes, sniff_root # noqa: E402 +TC6_NS = "http://www.plcopen.org/xml/tc6_0201" PLCOPEN = """ @@ -441,6 +444,24 @@ def test_aml_rules(self): bad = AML.replace('ID="i1"', 'ID="e1"').replace("file:///plc.xml#Main", "file:///missing.xml") self.assertEqual(codes({"plant.aml": bad})[1], ["AM003", "AM004", "AM011"]) + def test_aml_link_to_a_directory_is_skipped_and_to_a_file_is_checked(self): + # A target that exists but is not a regular file is never read. + with tempfile.TemporaryDirectory() as d: + base = Path(d) + (base / "docs").mkdir() + (base / "plc.xml").write_text(PLCOPEN, encoding="utf-8") + targets = { + "relative directory": ("docs#Main", []), + "absolute directory": (f"{base / 'docs'}#Main", []), + "regular file": ("plc.xml#Main", ["AM012"]), + } + for name, (ref, expected) in targets.items(): + with self.subTest(target=name): + aml = base / "plant.aml" + aml.write_text(AML.replace("file:///plc.xml#Main", ref), encoding="utf-8") + fmt, findings = plc_validate.check_file(aml, None) + self.assertEqual((fmt, sorted(f.code for f in findings)), ("aml", expected)) + def test_dexpi_rules(self): bad = DEXPI.replace('ID="pif1"', 'ID="pump1"').replace('ItemID="pump1"', 'ItemID="nothing"') self.assertEqual(codes({"dexpi.xml": bad})[1], ["DX003", "DX004"]) @@ -454,6 +475,137 @@ def test_broken_xml(self): self.assertEqual(codes({"x.TcPOU": ""})[1], ["XML001"]) +class XmlEntityGuard(unittest.TestCase): + """An entity declaration is refused; a bare DOCTYPE and plain text are not.""" + + PLCOPEN_ENTITY = PLCOPEN.replace("]>\n XmlError: + with self.assertRaises(XmlError) as ctx: + parse_bytes(data) + return ctx.exception + + def test_entity_declaration_is_refused_with_its_line(self): + exc = self.refused(self.ENTITY_DOC.encode()) + self.assertEqual(str(exc), "XML entity declarations are not supported") + self.assertEqual(exc.line, 3) + + def test_utf16_entity_declaration_is_refused(self): + exc = self.refused(self.ENTITY_DOC.replace('"1.0"?>', '"1.0" encoding="utf-16"?>').encode("utf-16")) + self.assertEqual((str(exc), exc.line), ("XML entity declarations are not supported", 3)) + + def test_every_declaration_kind_is_refused(self): + kinds = { + "internal": '', + "parameter": '', + "external": '', + "unparsed": '', + } + for name, decl in kinds.items(): + with self.subTest(kind=name): + exc = self.refused(f"".encode()) + self.assertEqual(str(exc), "XML entity declarations are not supported") + + def test_entity_declaration_is_an_xml001_finding_for_an_explicit_format(self): + fmt, findings = plc_validate.check_file(self.write("a.xml", self.PLCOPEN_ENTITY), "plcopen") + self.assertEqual((fmt, [(f.code, f.line) for f in findings]), ("plcopen", [("XML001", 2)])) + + def test_detect_skips_a_document_that_declares_an_entity(self): + # The sniffer must not expand entities, so it gives up on the file; the edit-time hook + # skips it like any non-PLC .xml. + self.assertIsNone(plc_validate.detect(self.write("a.xml", self.PLCOPEN_ENTITY))) + other = ']>\n\n' + path = self.write("note.xml", other) + self.assertEqual(plc_validate.check_file(path, None), (None, [])) + + def test_sniff_root_does_not_expand_an_entity_used_in_a_root_attribute(self): + doc = f']>\n' + self.assertEqual(sniff_root(self.write("a.xml", doc)), ("", "")) + + def test_sniff_root_does_not_expand_an_entity_used_in_an_attlist_default(self): + doc = f']>\n' + self.assertEqual(sniff_root(self.write("a.xml", doc)), ("", "")) + + def test_utf16_plcopen_with_an_entity_is_xml_broken_then_xml001(self): + # sniff_root only reads ASCII-compatible heads, so a UTF-16 file never reaches the + # entity guard there; parse_bytes then reports the declaration. + doc = self.PLCOPEN_ENTITY.replace('encoding="utf-8"', 'encoding="utf-16"') + path = self.write("a.xml", "") + path.write_bytes(doc.encode("utf-16")) + fmt, findings = plc_validate.check_file(path, None) + self.assertEqual((fmt, [(f.code, f.line) for f in findings]), ("xml-broken", [("XML001", 2)])) + + def test_entity_text_outside_a_declaration_is_not_refused(self): + entity = '' + docs = { + "comment": f'', + "comment in subset": f']>', + "processing instruction": f'', + "cdata": f'', + "element, attlist, notation": ( + '' + f']>'), + } + for name, doc in docs.items(): + with self.subTest(case=name): + self.assertEqual(parse_bytes(doc.encode()).tag, "project") + self.assertEqual(sniff_root(self.write("a.xml", doc)), (TC6_NS, "project")) + + def write(self, name: str, content: str) -> Path: + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + path = Path(tmp.name) / name + path.write_text(content, encoding="utf-8") + return path + + def test_doctype_with_external_dtd_still_validates(self): + dtd = '\n' + fbdk = FBT.replace("" + "x<" * 200 + "intail&\nend").encode() + root = parse_bytes(data) + self.assertEqual(root.text, "x<" * 200 + "tail&cd\nend") + self.assertEqual(root.text_line, 1) + self.assertEqual([c.text for c in root.children], ["in", ""]) + + def test_text_line_is_the_first_chunk_line(self): + root = parse_bytes(b"\n\nlate") + self.assertEqual((root.text, root.text_line), ("\n\nlate", 1)) + root = parse_bytes(b"\n\nx") + self.assertEqual((root.text, root.text_line), ("\n\n", 1)) + + +class XmlTextJoinScaling(unittest.TestCase): + """Text split into many chunks (one per entity reference) must parse in linear time.""" + + RUNS = 3 + SMALL, BIG = 40_000, 160_000 # BIG is 4x SMALL: linear work is about 4x, quadratic about 16x + MAX_RATIO = 8 + TIMER_FLOOR = 0.01 # seconds; keeps the ratio meaningful if the small run is below timer noise + + def best_of_runs(self, n: int) -> float: + data = (f'' + "x<" * n + "").encode() + best = float("inf") + for _ in range(self.RUNS): + start = time.perf_counter() + parse_bytes(data) + best = min(best, time.perf_counter() - start) + return best + + def test_text_join_time_grows_linearly_with_the_chunk_count(self): + # A ratio, not an absolute bound, so a slow CI runner does not flake: both sizes slow + # down together. Best of three drops a run that a scheduling hiccup inflated, which would + # otherwise push the ratio up. Appending each chunk to node.text copies the text so far + # each time (quadratic) and lands near 16x; one join per element lands near 4x. + t_small, t_big = self.best_of_runs(self.SMALL), self.best_of_runs(self.BIG) + self.assertLess(t_big, self.MAX_RATIO * max(t_small, self.TIMER_FLOOR), + f"{self.SMALL} chunks took {t_small:.3f}s, {self.BIG} chunks took {t_big:.3f}s") + + class ShippedTemplates(unittest.TestCase): """Every template a skill hands to the model must itself pass the validator.""" diff --git a/tests/test_skills.py b/tests/test_skills.py index 30b0201..c4f2f0c 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -6,6 +6,7 @@ from __future__ import annotations +import json import re import sys import unittest @@ -97,6 +98,14 @@ def test_every_iecst_block(self): self.assertGreater(blocks, 10, "no iecst blocks found; the fence pattern is wrong") +class PluginManifests(unittest.TestCase): + def test_both_manifests_carry_the_same_version(self): + versions = {str(rel): json.loads((ROOT / rel).read_text(encoding="utf-8"))["version"] + for rel in (Path("plugin.json"), Path(".claude-plugin") / "plugin.json")} + self.assertEqual(len(versions), 2) + self.assertEqual(len(set(versions.values())), 1, versions) + + class CrossReferences(unittest.TestCase): def test_relative_links_resolve(self): link = re.compile(r"\[[^\]]*\]\(([^)\s#]+)(?:#[^)]*)?\)")