From 21fc2136122b2365d7b86be81a3bc5c094c3a803 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:18:23 +0000 Subject: [PATCH 1/2] repo_shape.py: a quote opens a quoted scalar only where a scalar begins, and the scalar must end at its closing quote, so `k: [it's, a]` reads the two items its block twin reads, `k: it's fine # note` drops its comment, and an unclosed `"abc`, `- "a\"` or `"a" b "c"` is refused instead of misread (#249, #250) #249: `_strip_comment`, `_split_key` and `_split_flow` each toggled a quote at ANY `"` or `'`, so a quote in the middle of a plain scalar swallowed the `,`, `#` or `: ` after it. They now read structure through one generator, `_unquoted`: a quote opens a quoted scalar only at the start of the text or after `- `, `: `, `[`, `{`, or a `,` inside a flow; on a block line a `,` or `[` in the middle of a plain scalar is not structure (`k: a, 'b # c'` is the plain `a, 'b`). `_quote_end` is the one definition of where the scalar ends: a `"` no backslash escapes, a `'` that is not doubled. #250: `_scalar` took any text starting and ending with a quote for one quoted scalar. `_quoted_scalar` now requires the whole text to be one, and raises `quoted scalar without a closing quote` or `quoted scalar with text after its closing quote`; `_unquote_key` sends every key that begins with a quote through it. Every #249 row reads as PyYAML 6.0.3 reads it, and every #250 row is refused, as PyYAML refuses it. shape_materialize.py: `render` pasted free text raw between a YAML template's double quotes, so `scaffold-project.py --elected-by 'Robert "Bob" Smith'` wrote `"Robert "Bob" Smith"`: read by accident before, and refused by the stricter reader after three repositories exist. A value between a `.yaml`/`.yml` template's double quotes is now spelled by `double_quoted`; for any value with no quote, backslash or non-printable character that is the value itself, so no template changes by a byte. On 150c41d `--reference 'docs\notes # 1'` already read back as `docs`, a line feed and `otes # 1`. Tests: tests/test_yaml_quote_boundaries.py (reader) and tests/test_template_quoted_values.py (renderer, plus one real scaffold into local bare remotes); 65 of their 138 cases fail on 150c41d. In tests/test_yaml_flow_quotes.py the four N5 rows that pinned a dangling backslash's plain-text read are now refusals together with their block twins, the change that table said to expect. Each changed function reverted to 150c41d, and each rule of the new helpers mutated, fails at least one new test. Reach: the 14 tracked YAML files read identically before and after. The flow-against-block fuzz (seed 2130213, 100,000 docs) and the #199 code-point fuzz (60,000) show 0 differences. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/repo_shape.py | 184 +++++++++++++++++--------- scripts/shape_materialize.py | 18 ++- tests/test_template_quoted_values.py | 148 +++++++++++++++++++++ tests/test_yaml_flow_quotes.py | 55 ++++---- tests/test_yaml_quote_boundaries.py | 191 +++++++++++++++++++++++++++ 5 files changed, 500 insertions(+), 96 deletions(-) create mode 100644 tests/test_template_quoted_values.py create mode 100644 tests/test_yaml_quote_boundaries.py diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index dd6c5199..df12b02a 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -169,6 +169,12 @@ class YamlError(Exception): # than one document. Each of those has a meaning this reader would have to # invent, and an invented meaning in a validator is a wrong answer with a # confident tone. +# +# A QUOTE OPENS A QUOTED SCALAR ONLY WHERE A SCALAR BEGINS, as in YAML; in +# the middle of a plain scalar (`it's`) it is a character of it (#249). A +# quoted scalar must close on its own line, with nothing after its closing +# quote but a comment or the structure that ends it, or it is REFUSED (#250): +# a quoted scalar spanning lines is outside this subset. See `_unquoted`. _DASH_RE = re.compile(r"^-(\s+|$)") @@ -193,56 +199,95 @@ def __init__(self, indent: int, text: str, dash: bool, line: int): self.line = line -def _strip_comment(text: str) -> str: - """Drop a trailing `#` comment that is not inside a quoted scalar.""" - out = [] - quote = None - i = 0 +def _quote_end(text: str, start: int) -> int | None: + """The index just past the quoted scalar that opens at `text[start]`, + or None when nothing on the line closes it. + + A `"` closes it where no backslash escapes it: the character after each + backslash is taken with the backslash, so the `"` of `\\\\"` closes and + the `"` of `\\"` does not. A `'` closes it where it is not doubled, `''` + being a single-quoted scalar's one escape. + """ + quote = text[start] + i = start + 1 while i < len(text): - ch = text[i] - if quote: - out.append(ch) - if ch == "\\" and quote == '"' and i + 1 < len(text): - out.append(text[i + 1]) - i += 2 - continue - if ch == quote: - quote = None - elif ch in "\"'": - quote = ch - out.append(ch) - elif ch == "#" and (not out or out[-1] in " \t"): - break + if quote == '"' and text[i] == "\\": + i += 2 + elif text[i] != quote: + i += 1 + elif quote == "'" and text[i + 1:i + 2] == "'": + i += 2 else: - out.append(ch) - i += 1 - return "".join(out).rstrip() + return i + 1 + return None -def _split_key(text: str) -> tuple[str, str] | None: - """Split `key: rest` on the first top-level `:` followed by space or EOL.""" - quote = None - i = 0 +def _unquoted(text: str, flow: bool = False): + """Each `(index, character)` of `text` that no quoted scalar holds: the + characters `_strip_comment`, `_split_key` and `_split_flow` may read as + structure. + + THE ONE DEFINITION OF WHERE A QUOTED SCALAR STARTS AND ENDS, which those + three used to spell as three toggles that opened a quote at ANY `"` or + `'` (#249). A quote opens one only where a scalar BEGINS: at the start + of the text, or after `- `, `: `, `[` or `{`, or after a `,` inside a + flow, with any spaces after each. Anywhere else, as in `it's` or `a"b`, + it is a character of a plain scalar, so the `,`, `#` or `: ` after it is + still structure and `k: [it's, a]` is two items, as its block twin is. + The quoted scalar ends at `_quote_end`. When nothing closes it, the rest + of the text is inside it and nothing more is yielded; `_scalar` then + refuses it by name (#250). + + `flow` says `text` is a flow collection's body, where every `,`, `[` and + `{` is structure. On a block line they are structure only inside a flow + the line opens where a scalar begins, so `k: a, 'b # c'` is the plain + scalar `a, 'b` and a comment, as YAML reads it. + """ + depth, begins, i = 0, True, 0 while i < len(text): ch = text[i] - if quote: - if ch == "\\" and quote == '"': - i += 2 - continue - if ch == quote: - quote = None - elif ch in "\"'": - quote = ch - elif ch in "[{": + if begins and ch in "\"'": + end = _quote_end(text, i) + if end is None: + return + i, begins = end, False + continue + yield i, ch + i += 1 + if ch in "[{" and (begins or flow or depth): + depth += 1 + elif ch in "]}" and depth: + depth -= 1 + spaced = text[i:i + 1] in ("", " ", "\t") + if ch not in " \t" and not (ch == "-" and begins and spaced): + begins = (ch in "[{," and (flow or depth > 0) + or ch == ":" and spaced) + + +def _strip_comment(text: str) -> str: + """Drop a trailing `#` comment: a `#` no quoted scalar holds + (`_unquoted`), at the start of the text or after a space or tab.""" + for i, ch in _unquoted(text): + if ch == "#" and (i == 0 or text[i - 1] in " \t"): + return text[:i].rstrip() + return text.rstrip() + + +def _split_key(text: str) -> tuple[str, str] | None: + """Split `key: rest` on the first `:` followed by space or EOL that no + quoted scalar holds (`_unquoted`).""" + for i, ch in _unquoted(text): + if ch in "[{": return None # a flow collection cannot be a key in this subset - elif ch == ":" and (i + 1 == len(text) or text[i + 1] in " \t"): + if ch == ":" and (i + 1 == len(text) or text[i + 1] in " \t"): return _unquote_key(text[:i].strip()), text[i + 1:].strip() - i += 1 return None def _unquote_key(key: str) -> str: - if len(key) >= 2 and key[0] == key[-1] and key[0] in "\"'": + """A key that begins with a quote is one quoted scalar, or `_scalar` + refuses it: `"a" b: 1` is not the key `"a" b` (#250).""" + if key[:1] in ('"', "'"): return str(_scalar(key)) return key @@ -353,14 +398,33 @@ def _unescape(body: str) -> str: return "".join(out) +def _quoted_scalar(text: str) -> str: + """The value of `text`, which begins with a quote and must be ONE whole + quoted scalar (#250). + + Not "starts and ends with a quote": that took `"a" b "c"` for one scalar, + `"a\\"` for `a\\` and an unclosed `"abc` for plain text, where YAML + refuses each. Nothing may follow the closing quote, because a comment + after it is gone before this runs and the structure after it is not part + of `text`. + """ + end = _quote_end(text, 0) + if end is None: + raise YamlError(f"quoted scalar without a closing quote: {text!r}") + if end < len(text): + raise YamlError( + f"quoted scalar with text after its closing quote: {text!r}") + if text[0] == "'": + return text[1:-1].replace("''", "'") + return _unescape(text[1:-1]) + + def _scalar(raw: str) -> Any: text = raw.strip() if text == "" or text in ("null", "~", "Null", "NULL"): return None - if text[0] == "'" and text[-1] == "'" and len(text) >= 2: - return text[1:-1].replace("''", "'") - if text[0] == '"' and text[-1] == '"' and len(text) >= 2: - return _unescape(text[1:-1]) + if text[0] in "\"'": + return _quoted_scalar(text) if text[0] in "&*!": raise YamlError(f"anchors, aliases and tags are not supported: {text!r}") if text in ("true", "True", "TRUE"): @@ -412,35 +476,23 @@ def _split_key_flow(text: str) -> tuple[str, str] | None: def _split_flow(body: str) -> list[str]: """A flow collection's body, split at its top-level commas. - Inside a DOUBLE-quoted item the character after a backslash is taken - with it, as `_strip_comment` and `_split_key` take it, so `\\"` neither - ends the item nor lets a `,` or `]` after it read as structure (#213). A - single-quoted item has no backslash escape: its `''` is two quotes, and - the toggle reads it. + The commas, brackets and braces it reads are the ones `_unquoted` yields, + so one inside a quoted item is not structure, and neither is a quote in + the middle of a plain item (#249). Inside a DOUBLE-quoted item the + character after a backslash is taken with it, so `\\"` neither ends the + item nor lets a `,` or `]` after it read as structure (#213). A + single-quoted item has no backslash escape: its `''` is a quote. """ - parts, depth, quote, cur = [], 0, None, [] - chars = iter(body) - for ch in chars: - if quote: - cur.append(ch) - if ch == "\\" and quote == '"': - cur.append(next(chars, "")) - elif ch == quote: - quote = None - continue - if ch in "\"'": - quote = ch - elif ch in "[{": + parts, depth, start = [], 0, 0 + for i, ch in _unquoted(body, flow=True): + if ch in "[{": depth += 1 elif ch in "]}": depth -= 1 elif ch == "," and depth == 0: - parts.append("".join(cur).strip()) - cur = [] - continue - cur.append(ch) - if cur: - parts.append("".join(cur).strip()) + parts.append(body[start:i].strip()) + start = i + 1 + parts.append(body[start:].strip()) return [p for p in parts if p != ""] diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 81780a70..607dc357 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -40,7 +40,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 CHAIN_RECORD_FIELD, SAFE_ARG_RE, NamingPolicy, Refusal, checked_value, - file_sha256, + double_quoted, file_sha256, ) SHAPE_REPOSITORY = "opensoft/openRepoShape" @@ -926,8 +926,24 @@ def lane_trailer_argument(quote=None, env: dict | None = None) -> str: def render(text: str, values: dict[str, str], source: str) -> str: + """`text` with every `{{KEY}}` replaced by its value, or a refusal. + + A VALUE BETWEEN A YAML TEMPLATE'S DOUBLE QUOTES, `"{{ELECTED_BY}}"`, is + spelled by `double_quoted`, the writer `repo_shape`'s reader takes back + unchanged. Pasted raw, `--elected-by 'Robert "Bob" Smith'` wrote + `"Robert "Bob" Smith"`, which is not YAML: the reader read it by + accident until it learned where a quoted scalar ends (#250), and then + refused the manifest of three repositories already created. A name ending + in a backslash closed no quote, and `C:\\new` or `x" # y` were misread. + The spelling is the value itself for any text without a quote, a + backslash or a character `str.isprintable` refuses -- a commit, a digest, + an ordinary name -- so no other template changes by a byte. + """ out = text + yaml_file = source.endswith((".yaml", ".yml")) for key, value in values.items(): + if yaml_file: + out = out.replace('"{{' + key + '}}"', double_quoted(str(value))) out = out.replace("{{" + key + "}}", str(value)) left = PLACEHOLDER_RE.findall(out) if left: diff --git a/tests/test_template_quoted_values.py b/tests/test_template_quoted_values.py new file mode 100644 index 00000000..7168df5e --- /dev/null +++ b/tests/test_template_quoted_values.py @@ -0,0 +1,148 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A free-text value between a YAML template's double quotes reads back. + +`project.yaml`, `family.yaml`, the pins and `single-repository.yaml` quote +their free-text fields: `elected_by: "{{ELECTED_BY}}"`. `render` pasted the +value raw, so `--elected-by 'Robert "Bob" Smith'` wrote +`elected_by: "Robert "Bob" Smith"`, which is not YAML. The reader took it +by accident while it read any text that starts and ends with a quote as one +quoted scalar; once it learned where a quoted scalar ends (#250) it refused +the manifest, after the scaffold had created and pushed three repositories. +A name ending in a backslash closed no quote at all, and `C:\\new` or +`x" # y` were misread even then: an escape, and a comment. + +`render` now spells such a value with `double_quoted`, the writer the reader +takes back unchanged. For any value with no quote, no backslash and no +character `str.isprintable` refuses -- every commit, digest, date and +ordinary name -- the spelling is the value, so no template changes by a byte. + +NO NETWORK AND NO GITHUB: templates rendered in memory, and one scaffold +into local bare remotes (`--local-remote-dir`), which never calls `gh`. +""" + +from __future__ import annotations + +import re +import sys + +import pytest + +from conftest import ORG, REPO, SCAFFOLD, run_script + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import load_yaml, parse_yaml # noqa: E402 +from shape_materialize import render # noqa: E402 + +PLACEHOLDER = re.compile(r"\{\{([A-Z_]+)\}\}") +QUOTED_PLACEHOLDER = re.compile(r'"\{\{([A-Z_]+)\}\}"') + +#: Every YAML template that quotes a placeholder. Found, not listed, so a +#: template that gains a quoted field is covered without an edit here. +QUOTING_TEMPLATES = sorted( + path.relative_to(REPO).as_posix() + for pattern in ("*.yaml", "*.yml") + for path in (REPO / "templates").rglob(pattern) + if QUOTED_PLACEHOLDER.search(path.read_text(encoding="utf-8"))) + +#: Free text a person can pass as `--elected-by`, `--name`, `--reference`, +#: `--created-by` or put in an adoption plan. The first is the one main read +#: by accident and a stricter reader refuses; the rest it misread or refused. +FREE_TEXT = [ + pytest.param('Robert "Bob" Smith', id="double-quotes"), + pytest.param("Jane\\", id="trailing-backslash"), + pytest.param("C:\\new", id="backslash-letter"), + pytest.param('x" # y', id="quote-then-comment"), + pytest.param("\\x41", id="code-point-escape-spelled"), + pytest.param("tab\there", id="tab"), + pytest.param("O'Brien # it's", id="apostrophes-and-hash"), +] + + +def rendered(template: str, free_text: str) -> tuple[str, int]: + """The template with `free_text` in every quoted placeholder and a + harmless stand-in everywhere else, and how many quoted fields it has.""" + text = (REPO / template).read_text(encoding="utf-8") + quoted = set(QUOTED_PLACEHOLDER.findall(text)) + values = {} + for key in set(PLACEHOLDER.findall(text)): + if key in quoted: + values[key] = free_text + elif re.search(r"^\{\{" + key + r"\}\}", text, re.M): + values[key] = f" stand_in_{key.lower()}: 1" # a block placeholder + else: + values[key] = f"v-{key.lower()}" + return render(text, values, template), len(QUOTED_PLACEHOLDER.findall(text)) + + +def scalars(node) -> list: + if isinstance(node, dict): + return [s for value in node.values() for s in scalars(value)] + if isinstance(node, list): + return [s for value in node for s in scalars(value)] + return [node] + + +def test_the_templates_that_quote_a_placeholder_are_found(): + """The free-text fields of the manifest, the family and the + single-repository record, and the quoted hex of every pin.""" + assert "templates/assembly-root/project.yaml" in QUOTING_TEMPLATES + assert "templates/family-root/family.yaml" in QUOTING_TEMPLATES + assert ("templates/single-repository/single-repository.yaml" + in QUOTING_TEMPLATES) + + +@pytest.mark.parametrize("free_text", FREE_TEXT) +@pytest.mark.parametrize("template", QUOTING_TEMPLATES) +def test_a_quoted_free_text_field_reads_back_exactly(template, free_text): + text, fields = rendered(template, free_text) + assert scalars(parse_yaml(text)).count(free_text) == fields, text + + +@pytest.mark.parametrize("template", QUOTING_TEMPLATES) +def test_an_ordinary_value_renders_byte_for_byte_as_before(template): + """A commit, a digest, a date and a name without a quote or a backslash + are written exactly as a plain substitution writes them.""" + text = (REPO / template).read_text(encoding="utf-8") + values = {key: ("0123456789abcdef" * 4 if "SHA256" in key else + "c0ffee" * 6 + "abcd" if "COMMIT" in key else + "2026-09-02" if key.endswith("_ON") else "Brett Heap — é") + for key in set(PLACEHOLDER.findall(text))} + plain = text + for key, value in values.items(): + plain = plain.replace("{{" + key + "}}", value) + assert render(text, values, template) == plain + + +def test_a_template_that_is_not_yaml_keeps_the_value_raw(): + """A README or AGENTS.md is read by a person, not the YAML reader: a + quote there is a quote, and escaping it would print a backslash.""" + text = 'Elected by "{{ELECTED_BY}}".\n' + values = {"ELECTED_BY": 'Robert "Bob" Smith'} + assert render(text, values, "README.md") == ( + 'Elected by "Robert "Bob" Smith".\n') + assert render(text, values, "templates/assembly-root/README.md") == ( + 'Elected by "Robert "Bob" Smith".\n') + + +def test_a_scaffold_with_quotes_and_backslashes_writes_a_manifest_it_reads( + tmp_path): + """The scaffold itself, end to end into local bare remotes: the + manifest it writes passes its own validator and holds what was typed.""" + remotes, work = tmp_path / "remotes", tmp_path / "work" + elected_by = 'Robert "Bob" Smith' + reference = "docs\\notes # 1" + name = 'Atlas "the" Project' + result = run_script(SCAFFOLD, "--org", ORG, "--project", "Atlas", + "--name", name, "--elected-by", elected_by, + "--reference", reference, + "--elected-on", "2026-09-02", + "--local-remote-dir", str(remotes), + "--work-dir", str(work)) + assert result.returncode == 0, result.stderr + result.stdout + root = work / "Atlas" + check = run_script(root / "scripts" / "validate-manifest.py", cwd=root) + assert check.returncode == 0, check.stderr + check.stdout + manifest = load_yaml(root / "project.yaml") + assert manifest["elected_by"] == elected_by + assert manifest["reference"] == reference + assert manifest["name"] == name diff --git a/tests/test_yaml_flow_quotes.py b/tests/test_yaml_flow_quotes.py index 73924ee3..75b3ae9f 100644 --- a/tests/test_yaml_flow_quotes.py +++ b/tests/test_yaml_flow_quotes.py @@ -18,13 +18,13 @@ rather than guessing. A single-quoted scalar has no backslash escape. Its one escape is `''`, which -both scanners read by toggling twice, and that is unchanged. +both scanners read as a quote rather than the end of the item, and that is +unchanged. A backslash that is the last character of a double-quoted flow item's text has -no character after it to take. That is not valid YAML, and the reader must say -so as a `YamlError` or read it as its block twin does, never die with a -`StopIteration` out of the scanner (the `""` default of `next(chars, "")` in -`_split_flow`). +no character after it to take. That is not valid YAML, and the reader says so +as a `YamlError`, for the flow item and its block twin alike (#250), never as a +bare exception out of the scanner (#213 guarded a `StopIteration` there). NO NETWORK AND NO GITHUB: strings, a few temporary files and an import. """ @@ -97,37 +97,34 @@ def test_a_single_quoted_flow_item_keeps_its_toggle(text, expected): #: A double-quoted flow item whose last character is a backslash. The closing #: bracket right after it belongs to the collection, so the scanner reaches -#: the end of the body with an escape still open. Each is the read at the head -#: of this change, and each equals what the block twin reads: the backslash is -#: kept, and the quote is not closed, so the item is plain text. The text is -#: raw: the one backslash in each is one backslash in the YAML. +#: the end of the body with an escape still open. The text is raw: the one +#: backslash in each is one backslash in the YAML. #: -#: These are not valid YAML (PyYAML refuses every one), so what they pin is -#: that the reader does not crash. If a later change refuses an unterminated -#: double-quoted scalar, in a block as well as in a flow, these become -#: refusals together with their block twins, and that is the change to make -#: here, not a regression. +#: These are not valid YAML (PyYAML refuses every one): the backslash escapes +#: the only quote that could close the item. Until the reader learned where a +#: quoted scalar ends (#250) each read as plain text, quote and backslash +#: kept, the same as its block twin. Now each is refused by name, together +#: with its block twin, which is the change this table was written to expect. DANGLING_BACKSLASH_BEFORE_THE_CLOSER = [ - pytest.param(r'k: ["a\]', {"k": ['"a\\']}, 'k:\n - "a\\', - id="sequence"), - pytest.param(r'k: ["\]', {"k": ['"\\']}, 'k:\n - "\\', + pytest.param(r'k: ["a\]', 'k:\n - "a\\', id="sequence"), + pytest.param(r'k: ["\]', 'k:\n - "\\', id="sequence-the-quote-and-the-backslash-only"), - pytest.param(r'k: {a: "x\}', {"k": {"a": '"x\\'}}, 'k:\n a: "x\\', - id="mapping-value"), - pytest.param(r'k: [["\]]', {"k": [['"\\']]}, 'k:\n - - "\\', - id="nested-sequence"), + pytest.param(r'k: {a: "x\}', 'k:\n a: "x\\', id="mapping-value"), + pytest.param(r'k: [["\]]', 'k:\n - - "\\', id="nested-sequence"), ] -@pytest.mark.parametrize("text, expected, block_twin", +@pytest.mark.parametrize("text, block_twin", DANGLING_BACKSLASH_BEFORE_THE_CLOSER) -def test_a_dangling_backslash_before_the_closer_reads_as_plain_text( - text, expected, block_twin): - """With `next(chars)` for `next(chars, "")` these die with a bare - `StopIteration` out of `parse_yaml`, and `load_yaml` would let it through - as a traceback rather than a `REFUSED` line.""" - assert parse_yaml(text) == expected - assert parse_yaml(block_twin) == expected, "its block twin reads the same" +def test_a_dangling_backslash_before_the_closer_is_refused_with_its_twin( + text, block_twin): + """A `YamlError` naming the unclosed scalar, never a bare exception out + of a scanner, which `load_yaml` would let through as a traceback rather + than a `REFUSED` line.""" + for document in (text, block_twin): + with pytest.raises(YamlError, + match="quoted scalar without a closing quote"): + parse_yaml(document) #: The same backslash where nothing follows it. At the very end of the text diff --git a/tests/test_yaml_quote_boundaries.py b/tests/test_yaml_quote_boundaries.py new file mode 100644 index 00000000..e52e2a58 --- /dev/null +++ b/tests/test_yaml_quote_boundaries.py @@ -0,0 +1,191 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Where a quoted scalar starts and where it ends (#249, #250). + +A QUOTE OPENS A QUOTED SCALAR ONLY WHERE A SCALAR BEGINS: at the start of a +value, an item, a key or a flow entry, after optional spaces. In the middle +of a plain scalar (`it's`, `a"b`) it is an ordinary character. The reader's +three scanners -- `_strip_comment`, `_split_key` and `_split_flow` -- each +toggled a quote at ANY `"` or `'`, so `k: [it's, a]` read as the one item +`it's, a` where its block twin reads two, and `k: it's fine # note` kept the +comment in the value (#249). They now ask one definition, `_unquoted`. + +A QUOTED SCALAR ENDS AT ITS CLOSING QUOTE, and nothing but a comment or the +structure that ends the node may follow it. `_scalar` took any text that +starts and ends with a quote for a quoted scalar and anything else for plain +text, so an unclosed `"abc` read as `"abc`, `- "a\\"` as `a\\`, and +`"a" b "c"` as `a" b "c` (#250). Each is now a `YamlError` by name, which +`load_yaml` turns into `REFUSED yaml-unparsable`. + +Every expected read below is PyYAML 6.0.3's, and every refused document is +one PyYAML refuses, except `k: "a"#c`: PyYAML reads `a`, but YAML requires a +space before a comment, and the subset refuses rather than guess. PyYAML is +not imported here; nothing in this suite depends on it. + +NO NETWORK AND NO GITHUB: strings, one temporary file and an import. +""" + +from __future__ import annotations + +import sys + +import pytest + +from conftest import REPO + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import (Refusal, YamlError, double_quoted, # noqa: E402 + load_yaml, parse_yaml) + +#: #249's table, and the neighbours that share its cause. The text is raw: +#: every backslash here is a backslash in the YAML. +QUOTE_IN_A_PLAIN_SCALAR = [ + pytest.param("k: [it's, a]", {"k": ["it's", "a"]}, + id="flow-sequence-two-items-read-as-one"), + pytest.param("k: {a: it's, b: 1}", {"k": {"a": "it's", "b": 1}}, + id="flow-mapping-two-entries-read-as-one"), + pytest.param("k: it's fine # note", {"k": "it's fine"}, + id="comment-kept-in-the-value"), + pytest.param("k: [it's, a] # note", {"k": ["it's", "a"]}, + id="flow-then-comment-refused"), + pytest.param("it's: 1", {"it's": 1}, id="key-refused"), + pytest.param('k: [a"b, c]', {"k": ['a"b', "c"]}, + id="double-quote-mid-item"), + pytest.param(r'k: [a"b\", c]', {"k": ['a"b\\"', "c"]}, + id="double-quote-and-backslash-mid-item"), + pytest.param("- it's # c", ["it's"], id="sequence-item-then-comment"), + pytest.param("k: {it's: a, b: c}", {"k": {"it's": "a", "b": "c"}}, + id="flow-mapping-key"), + pytest.param("k: x'y'", {"k": "x'y'"}, id="plain-scalar-ending-in-quotes"), + pytest.param("k: [-'a', b]", {"k": ["-'a'", "b"]}, + id="a-dash-with-no-space-is-plain"), + # On a block line a `,` or `[` in the middle of a plain scalar is a + # character of it, not structure, so the quote after it opens nothing. + pytest.param("k: a, 'b # c'", {"k": "a, 'b"}, + id="block-comma-is-not-a-flow-separator"), + pytest.param("k: a[b, 'c # d']", {"k": "a[b, 'c"}, + id="block-bracket-mid-scalar-opens-no-flow"), +] + + +@pytest.mark.parametrize("text, expected", QUOTE_IN_A_PLAIN_SCALAR) +def test_a_quote_in_the_middle_of_a_plain_scalar_is_a_character(text, + expected): + assert parse_yaml(text + "\n") == expected + + +def test_a_flow_sequence_reads_as_its_block_twin(): + """#249's own comparison: the same two items, flow and block.""" + assert parse_yaml("k:\n - it's\n - a\n") == {"k": ["it's", "a"]} + assert parse_yaml("k: [it's, a]\n") == parse_yaml("k:\n - it's\n - a\n") + + +#: #250's table, then the same defect in a key, a flow mapping and a +#: comment written with no space before it. The text is raw. +NOT_ONE_WHOLE_QUOTED_SCALAR = [ + pytest.param('created_by: "Brett Heap', "without a closing quote", + id="closing-quote-missing"), + pytest.param('k: "abc', "without a closing quote", id="double-unclosed"), + pytest.param('k:\n - "a\\"', "without a closing quote", + id="only-closing-quote-escaped"), + pytest.param(r'k: "a\", b', "without a closing quote", + id="escaped-quote-then-more"), + pytest.param('k: "a" b "c"', "with text after its closing quote", + id="text-after-the-closing-quote"), + pytest.param('k: "a"b', "with text after its closing quote", + id="text-right-after-the-closing-quote"), + pytest.param("k: 'abc", "without a closing quote", id="single-unclosed"), + pytest.param("k: 'a' b 'c'", "with text after its closing quote", + id="single-text-after-the-closing-quote"), + pytest.param(r'k: ["a\", b]', "without a closing quote", + id="flow-escaped-quote"), + pytest.param('k: ["a, b]', "without a closing quote", + id="flow-unclosed"), + pytest.param("k: '''", "without a closing quote", + id="single-doubled-quote-then-nothing"), + pytest.param('"a" b: 1', "with text after its closing quote", + id="key-text-after-the-closing-quote"), + pytest.param(r'- "a\": b', "without a closing quote", + id="sequence-item-escaped-quote"), + pytest.param('k: {a: "x" y, b: 1}', "with text after its closing quote", + id="flow-mapping-value"), + pytest.param('k: "a"#c', "with text after its closing quote", + id="comment-with-no-space-before-it"), +] + + +@pytest.mark.parametrize("text, message", NOT_ONE_WHOLE_QUOTED_SCALAR) +def test_a_scalar_that_begins_with_a_quote_is_one_whole_quoted_scalar( + text, message): + with pytest.raises(YamlError, match=f"quoted scalar {message}"): + parse_yaml(text + "\n") + + +def test_everything_after_an_unclosed_quote_is_inside_it(): + """A `#` or `,` after a quote that never closes is text of the scalar, + not a comment or a separator, so the refusal quotes all of it.""" + with pytest.raises(YamlError, match=r"""closing quote: '"a # b, c'$"""): + parse_yaml('k: "a # b, c\n') + with pytest.raises(YamlError, match=r"""closing quote: '"a, b # c'$"""): + parse_yaml('k: ["a, b # c]\n') + + +#: What must NOT change: whole quoted scalars, the `''` escape, a quote of +#: one kind inside the other, comments after a closing quote, flow items +#: with spaces around them. +STILL_READ = [ + pytest.param('k: "a" # c', {"k": "a"}, id="double-then-comment"), + pytest.param("k: 'a, b # c' # d", {"k": "a, b # c"}, + id="hash-and-comma-inside-a-quoted-value"), + pytest.param("k: 'it''s'", {"k": "it's"}, id="single-doubled-quote"), + pytest.param("k: ''''", {"k": "'"}, id="single-only-a-quote"), + pytest.param(r'k: "\""', {"k": '"'}, id="double-only-a-quote"), + pytest.param(r'k: "a\\"', {"k": "a\\"}, id="double-escaped-backslash"), + pytest.param("k: \"it's\"", {"k": "it's"}, id="single-inside-double"), + pytest.param("k: 'say \"hi\"'", {"k": 'say "hi"'}, + id="double-inside-single"), + pytest.param("k: ['it''s, a', b]", {"k": ["it's, a", "b"]}, + id="flow-single-doubled-quote-and-comma"), + pytest.param("k: [ 'a' , \"b\" ]", {"k": ["a", "b"]}, + id="flow-items-with-spaces"), + pytest.param("k: [\"a\", b] # c", {"k": ["a", "b"]}, + id="flow-then-comment"), + pytest.param("k: {\"a\": 'b', 'c': \"d\"}", {"k": {"a": "b", "c": "d"}}, + id="flow-mapping-quoted-keys-and-values"), + pytest.param("- \"a\" # c", ["a"], id="sequence-item-then-comment"), + pytest.param("- - 'a # b'", [["a # b"]], id="nested-sequence-item"), + pytest.param('"a b": c', {"a b": "c"}, id="double-quoted-key"), + pytest.param("'a': 1", {"a": 1}, id="single-quoted-key"), +] + + +@pytest.mark.parametrize("text, expected", STILL_READ) +def test_a_whole_quoted_scalar_still_reads(text, expected): + assert parse_yaml(text + "\n") == expected + + +@pytest.mark.parametrize("value", [ + 'a"b', "x\\", "it's, a", '" # ', "\\\"", "]", "a: b", +]) +def test_the_writer_still_round_trips_beside_a_plain_quote(value): + """`double_quoted` is the subset's writer. Its output must read back + wherever a scalar begins, including after a plain item that holds a + quote of its own, in a flow and in a block.""" + spelled = double_quoted(value) + assert parse_yaml(f"k: [it's, {spelled}]\n") == {"k": ["it's", value]} + assert parse_yaml(f"k: {{it's: {spelled}}}\n") == {"k": {"it's": value}} + assert parse_yaml(f"k:\n - it's\n - {spelled} # c\n") == { + "k": ["it's", value]} + + +def test_an_unclosed_quote_in_a_file_is_a_refusal_naming_it(tmp_path): + """#250's first row, read from a file the way every caller reads one: + a `created_by` whose closing quote was lost in a hand edit. Bytes, so + the file is LF on every platform.""" + path = tmp_path / "family.yaml" + path.write_bytes(b'created_by: "Brett Heap\n') + with pytest.raises(Refusal) as caught: + load_yaml(path) + assert caught.value.code == "yaml-unparsable" + assert str(path) in caught.value.detail + assert "quoted scalar without a closing quote" in caught.value.detail + assert "Brett Heap" in caught.value.detail From fc02c0d19c7dffe42aa4d11658df332cd482abc4 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:12:59 +0000 Subject: [PATCH 2/2] fix round 1 (#249, #250): a `[` or `{` later in a key, before its `: `, is refused by name in `_split_key`, so `- note "[1]": x` and `- a[b]: c` are no longer read as the strings they are not, and a `:` with no space after it is pinned as opening no quoted scalar Review of 21fc213, SHOULD-1. Reading quotes only where a scalar begins let `_split_key`'s old rule, "a `[` or `{` means this is not a key", see a bracket that a quote toggle had hidden. As a mapping entry that is a refusal, but as a sequence item `_parse_sequence` takes `None` for "a scalar item", so `- note "[1]": x` read as the string `note "[1]": x` where PyYAML reads the mapping `{'note "[1]"': 'x'}`. On the reviewer's seed 7249250, 300 valid documents regressed and 20 of them were silent misreads; every one has a `[` or `{` in a key. `_split_key` now returns None for a `[` or `{` that OPENS the text, so `- [a, b]` and `- {a: b}` stay flow items, and raises YamlError ("a key holding `[` or `{` is not supported: ") for one LATER in the key when a `: ` follows. The change stays inside `_split_key`, so C's `_split_key_flow`, which calls it, is untouched. On that seed the regressions are all refusals (300, 0 misreads), the same 7,039 documents are fixed and the same 3,173 invalid ones are read as on 21fc213, and 1,550 more of 150c41d's own silent misreads, `- a[b]: c` among them, become refusals. Reading `a[b]: c` as YAML does needs block keys and flow-entry keys told apart; that is a follow-up. Tests (tests/test_yaml_quote_boundaries.py, 54 cases to 73): - `- note "[1]": x`, `note "[1]": x`, `- a[b]: c`, `a[b]: c`, a nested `- a{b}: c`, `- - a[b]: c`, `- x {y}: z` and the flow entry `{a[b]: c}` are each refused by name, the message quotes the key, and a file read through `load_yaml` is `REFUSED yaml-unparsable` naming it. All 10 fail on 21fc213. - `- [a, b]`, `- {a: b}`, flow items under a key, `- a: [b]`, `- a: {b: [c]}`, `k: a[b]` and `- see [1]` still read (7 guards, which pass on both). - Nit 1: `k: [a:'b, c']` reads as the two items `a:'b` and `c'`, and `k: a:'b # c'` as `a:'b`, both PyYAML's reads. Mutant X1 (any `:` opens a quoted scalar) survived the 148 old tests and now fails exactly these two. - Mutants of the new `_split_key` (no raise, start test inverted, a bracket always None, the flag never set) each fail at least 10 of the new rows. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/repo_shape.py | 20 ++++++-- tests/test_yaml_quote_boundaries.py | 80 +++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 4e4c2690..482662f1 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -279,11 +279,25 @@ def _strip_comment(text: str) -> str: def _split_key(text: str) -> tuple[str, str] | None: """Split `key: rest` on the first `:` followed by space or EOL that no - quoted scalar holds (`_unquoted`).""" + quoted scalar holds (`_unquoted`). + + A `[` OR `{` THAT OPENS THE TEXT makes it a flow collection, which is not + a key in this subset, so `- [a, b]` and `- {a: b}` stay flow items. One + LATER IN A KEY, before its `: `, is REFUSED by name: YAML reads + `a[b]: c` as the key `a[b]`, but a block key and a flow-entry key would + have to be told apart to read it, and returning `None` made `- a[b]: c` + the string `'a[b]: c'` where a mapping was meant (#249). + """ + bracket = False for i, ch in _unquoted(text): if ch in "[{": - return None # a flow collection cannot be a key in this subset - if ch == ":" and (i + 1 == len(text) or text[i + 1] in " \t"): + if not text[:i].strip(): + return None # a flow collection cannot be a key in this subset + bracket = True + elif ch == ":" and (i + 1 == len(text) or text[i + 1] in " \t"): + if bracket: + raise YamlError("a key holding `[` or `{` is not supported: " + f"{text[:i].strip()!r}") return _unquote_key(text[:i].strip()), text[i + 1:].strip() return None diff --git a/tests/test_yaml_quote_boundaries.py b/tests/test_yaml_quote_boundaries.py index e52e2a58..74ca0d1f 100644 --- a/tests/test_yaml_quote_boundaries.py +++ b/tests/test_yaml_quote_boundaries.py @@ -16,6 +16,17 @@ `"a" b "c"` as `a" b "c` (#250). Each is now a `YamlError` by name, which `load_yaml` turns into `REFUSED yaml-unparsable`. +A `[` OR `{` LATER IN A KEY IS REFUSED BY NAME. `_split_key` has always +taken a `[` or `{` for "this is a flow collection, not a key". On 150c41d a +quote toggle hid it from that rule when the key held a quote pair, so +`note "[1]": x` read as a mapping entry; reading quotes only where a scalar +begins lets the rule see the bracket, and the entry would have stopped being +one. YAML reads the key `note "[1]"`, and `- a[b]: c` the key `a[b]`, but +`_split_key` would have to tell a block key from a flow-entry key to read +them, so it refuses them rather than read a mapping as the string +`'a[b]: c'`. A `[` or `{` that OPENS the text still makes a flow item +(`- [a, b]`, `- {a: b}`). + Every expected read below is PyYAML 6.0.3's, and every refused document is one PyYAML refuses, except `k: "a"#c`: PyYAML reads `a`, but YAML requires a space before a comment, and the subset refuses rather than guess. PyYAML is @@ -64,6 +75,12 @@ id="block-comma-is-not-a-flow-separator"), pytest.param("k: a[b, 'c # d']", {"k": "a[b, 'c"}, id="block-bracket-mid-scalar-opens-no-flow"), + # A `:` opens a quoted scalar only with a space or the end after it, so + # in `a:'b` the `'` opens nothing and `a:'b` is one plain item. + pytest.param("k: [a:'b, c']", {"k": ["a:'b", "c'"]}, + id="flow-colon-with-no-space-opens-no-quote"), + pytest.param("k: a:'b # c'", {"k": "a:'b"}, + id="block-colon-with-no-space-opens-no-quote"), ] @@ -163,6 +180,69 @@ def test_a_whole_quoted_scalar_still_reads(text, expected): assert parse_yaml(text + "\n") == expected +#: A `[` or `{` LATER in a key, before its `: `. PyYAML reads every block one +#: of these as a key; `_split_key` refuses it by name instead of returning +#: `None`, which made `- a[b]: c` the string `'a[b]: c'`. +BRACKET_IN_A_KEY = [ + pytest.param('- note "[1]": x', id="item-quote-pair-around-a-bracket"), + pytest.param('note "[1]": x', id="entry-quote-pair-around-a-bracket"), + pytest.param("- a[b]: c", id="item-bracket"), + pytest.param("a[b]: c", id="entry-bracket"), + pytest.param("k:\n - a{b}: c", id="nested-item-brace"), + pytest.param("- - a[b]: c", id="item-in-an-item"), + pytest.param("- x {y}: z", id="item-brace-after-a-space"), + pytest.param("k: {a[b]: c}", id="flow-mapping-entry"), +] + + +@pytest.mark.parametrize("text", BRACKET_IN_A_KEY) +def test_a_bracket_or_brace_later_in_a_key_is_refused_by_name(text): + with pytest.raises(YamlError, + match=r"a key holding `\[` or `\{` is not supported"): + parse_yaml(text + "\n") + + +def test_the_refusal_quotes_the_key_it_names(): + with pytest.raises(YamlError, match=r"""supported: 'note "\[1\]"'$"""): + parse_yaml('- note "[1]": x\n') + with pytest.raises(YamlError, match=r"supported: 'a\[b\]'$"): + parse_yaml("a[b]: c\n") + + +#: What the refusal must NOT take: a `[` or `{` that OPENS the text is a flow +#: collection, one after the `: ` belongs to the value, and one in a scalar +#: with no `: ` after it makes no key. +BRACKETS_THAT_ARE_NOT_IN_A_KEY = [ + pytest.param("- [a, b]", [["a", "b"]], id="flow-sequence-item"), + pytest.param("- {a: b}", [{"a": "b"}], id="flow-mapping-item"), + pytest.param("k:\n - [a, b]\n - {c: d}", {"k": [["a", "b"], {"c": "d"}]}, + id="flow-items-under-a-key"), + pytest.param("- a: [b]", [{"a": ["b"]}], id="flow-value-of-an-item"), + pytest.param("- a: {b: [c]}", [{"a": {"b": ["c"]}}], + id="nested-flow-value-of-an-item"), + pytest.param("k: a[b]", {"k": "a[b]"}, id="bracket-in-a-value"), + pytest.param("- see [1]", ["see [1]"], id="item-bracket-and-no-colon"), +] + + +@pytest.mark.parametrize("text, expected", BRACKETS_THAT_ARE_NOT_IN_A_KEY) +def test_a_bracket_that_is_not_in_a_key_still_reads(text, expected): + assert parse_yaml(text + "\n") == expected + + +def test_a_refused_bracket_key_in_a_file_names_the_key(tmp_path): + """Read from a file, the way every caller reads one. Bytes, so the file + is LF on every platform.""" + path = tmp_path / "family.yaml" + path.write_bytes(b'members:\n - note "[1]": x\n') + with pytest.raises(Refusal) as caught: + load_yaml(path) + assert caught.value.code == "yaml-unparsable" + assert str(path) in caught.value.detail + assert """a key holding `[` or `{` is not supported: 'note "[1]"'""" \ + in caught.value.detail + + @pytest.mark.parametrize("value", [ 'a"b', "x\\", "it's, a", '" # ', "\\\"", "]", "a: b", ])