From 87996427adaeacedaac6a5c9ce2a1b6f1dfcd01f Mon Sep 17 00:00:00 2001 From: David Leong <116610336+leongdl@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:57:13 -0700 Subject: [PATCH 1/2] chore(deps): Bump openjd-* Rust crates to 0.7.0 openjd-expr 0.6.0 -> 0.7.0, openjd-model 0.6.1 -> 0.7.0, and openjd-sessions 0.5.6 -> 0.5.7. Three upstream changes land, reconciled against a source diff of the published crates rather than the changelog alone: * openjd-rs#374 escapes control characters, quotes and backslashes in repr_py and repr_python. Reachable here through ExprValue.__repr__, which previously emitted literals CPython cannot parse (a raw newline terminates the string, and a NUL cannot appear in source at all) or that parse as a different value -- 'a\b' is a backspace. * openjd-rs#373 changes FormatString::validate_expressions to take a target_type and return StaticResolution. This is the breaking change the minor bump carries, and the changelog did not mention the new parameter. The binding passes None, matching the resolve and resolve_string calls above, and discards the returned value: the Python method stays pass/fail and still returns None on success. * openjd-rs#369 bounds child output reading in the Windows session helper runner. Not reachable from this repo's tests; openjd-sessions forwards to it from subprocess.rs and its coverage lives upstream. TestExprValueReprEscaping pins the #374 behaviour over 28 inputs -- quotes, backslashes, the C0 controls, DEL, and the non-ASCII characters CPython escapes by category -- using CPython's own repr() as the oracle, which is the contract Expression Language 2.2.6 states. It adds an eval(repr(v)) == v round-trip and negative controls for numeric and keyword text that must pass through unaltered. Reverting the version pins fails 60 of these cases. test_repr_list_path_with_format and test_repr_list_list_path_with_format pinned the old bug: under WINDOWS the normalised '\a' was emitted unescaped, which Python parses as BEL. Both now assert the escaped spelling and round-trip, so nothing regressed -- the bump closed a silent-corruption path those tests had frozen. THIRD-PARTY-LICENSES.txt regenerated with scripts/check_third_party_licenses.sh --update; only the three crate versions changed, with no new transitive dependencies. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --- Cargo.lock | 12 +- THIRD-PARTY-LICENSES.txt | 6 +- rust-bindings/Cargo.toml | 6 +- rust-bindings/src/expr/format_string.rs | 14 ++- test/openjd/expr/test_expression_value.py | 128 +++++++++++++++++++--- 5 files changed, 137 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d86d888f..5eb3dc68 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -679,9 +679,9 @@ checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" [[package]] name = "openjd-expr" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a05a6e060a42d2b5f681ade212f38482c5fadd460fd73dbc12ebf964828bc290" +checksum = "b6e97fab933bcfb13e42a45d1f39311149508c454b21bf931274e288583d4e08" dependencies = [ "regex", "regex-syntax", @@ -696,9 +696,9 @@ dependencies = [ [[package]] name = "openjd-model" -version = "0.6.1" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dafcc75b2f46fbc178dd89d868667ac7e4c4a807e7f55387ae5b7e98c60031f3" +checksum = "fdfffa82b1dbb94865d10c935cd547b4319932280f37b87db475dca961b7ef91" dependencies = [ "indexmap", "openjd-expr", @@ -728,9 +728,9 @@ dependencies = [ [[package]] name = "openjd-sessions" -version = "0.5.6" +version = "0.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfa69de3cd3585885379735b7eaf90bcba315bd4a99beea77ada191e7947c15c" +checksum = "877446585bd38cff6a40c68293f62b2147e5fa4f0997a83a39c5d2af38585449" dependencies = [ "bitflags", "futures-util", diff --git a/THIRD-PARTY-LICENSES.txt b/THIRD-PARTY-LICENSES.txt index 3ed9fb53..3b23a560 100644 --- a/THIRD-PARTY-LICENSES.txt +++ b/THIRD-PARTY-LICENSES.txt @@ -2534,9 +2534,9 @@ limitations under the License. ** itoa; version 1.0.18 -- https://crates.io/crates/itoa ** libc; version 0.2.189 -- https://crates.io/crates/libc ** manyhow-macros; version 0.11.4 -- https://crates.io/crates/manyhow-macros -** openjd-expr; version 0.6.0 -- https://crates.io/crates/openjd-expr -** openjd-model; version 0.6.1 -- https://crates.io/crates/openjd-model -** openjd-sessions; version 0.5.6 -- https://crates.io/crates/openjd-sessions +** openjd-expr; version 0.7.0 -- https://crates.io/crates/openjd-expr +** openjd-model; version 0.7.0 -- https://crates.io/crates/openjd-model +** openjd-sessions; version 0.5.7 -- https://crates.io/crates/openjd-sessions ** pin-project-lite; version 0.2.17 -- https://crates.io/crates/pin-project-lite ** portable-atomic; version 1.15.0 -- https://crates.io/crates/portable-atomic ** proc-macro2; version 1.0.107 -- https://crates.io/crates/proc-macro2 diff --git a/rust-bindings/Cargo.toml b/rust-bindings/Cargo.toml index 5fb111ff..235bcc7c 100644 --- a/rust-bindings/Cargo.toml +++ b/rust-bindings/Cargo.toml @@ -12,9 +12,9 @@ name = "_openjd_rs" crate-type = ["cdylib", "rlib"] [dependencies] -openjd-expr = "0.6.0" -openjd-model = "0.6.1" -openjd-sessions = "0.5.6" +openjd-expr = "0.7.0" +openjd-model = "0.7.0" +openjd-sessions = "0.5.7" tokio = { version = "1", features = ["rt-multi-thread"] } uuid = { version = "1", features = ["v4"] } serde_json = "1" diff --git a/rust-bindings/src/expr/format_string.rs b/rust-bindings/src/expr/format_string.rs index 24ab0d80..7c6b1eba 100644 --- a/rust-bindings/src/expr/format_string.rs +++ b/rust-bindings/src/expr/format_string.rs @@ -106,8 +106,15 @@ impl PyFormatString { /// checking through the expression tree. /// /// Mirrors the Rust crate's - /// `FormatString::validate_expressions(symtab, lib)`. Returns - /// `None` on success. + /// `FormatString::validate_expressions(symtab, lib, target_type)`. + /// Returns `None` on success. + /// + /// `target_type` is passed as `None` because `resolve` and + /// `resolve_string` above resolve without one; validation has to + /// observe the same values resolution will produce. The crate + /// returns a `StaticResolution` (resolved-length bound and, when + /// fully concrete, the resolved value); this binding is pass/fail + /// only and discards it. #[pyo3(signature = (symtab, *, profile=None))] fn validate_expressions( &self, @@ -117,7 +124,8 @@ impl PyFormatString { let st = extract_symtab(symtab)?; let lib = profile_for_call(profile); self.inner - .validate_expressions(&st, &lib) + .validate_expressions(&st, &lib, None) + .map(|_| ()) .map_err(format_string_validation_err_to_py) } diff --git a/test/openjd/expr/test_expression_value.py b/test/openjd/expr/test_expression_value.py index 49ad870b..815495d4 100644 --- a/test/openjd/expr/test_expression_value.py +++ b/test/openjd/expr/test_expression_value.py @@ -337,27 +337,127 @@ def test_repr_empty_list_list_path(self) -> None: @pytest.mark.parametrize("pf", [PathFormat.POSIX, PathFormat.WINDOWS]) def test_repr_list_path_with_format(self, pf: PathFormat) -> None: - import re - v = ExprValue(["/a", "/b"], type="list[path]", path_format=pf) - r = repr(v) - assert re.match( - r"ExprValue\(\['(/|\\)a', '(/|\\)b'\], type='list\[path\]', " - rf"path_format=PathFormat\.{pf.name}\)", - r, + # Under WINDOWS the leading "/" normalises to "\", which the + # literal must escape -- so the expectation comes from CPython's + # repr of the normalised items rather than a regex that would + # accept either spelling. openjd-rs#374; before it, the WINDOWS + # case emitted '\a', which Python parses as BEL. + assert repr(v) == ( + f"ExprValue({v.item()!r}, type='list[path]', path_format=PathFormat.{pf.name})" ) + assert eval(repr(v)) == v @pytest.mark.parametrize("pf", [PathFormat.POSIX, PathFormat.WINDOWS]) def test_repr_list_list_path_with_format(self, pf: PathFormat) -> None: - import re - v = ExprValue([["/a"], ["/b"]], type="list[list[path]]", path_format=pf) - r = repr(v) - assert re.match( - r"ExprValue\(\[\['(/|\\)a'\], \['(/|\\)b'\]\], type='list\[list\[path\]\]', " - rf"path_format=PathFormat\.{pf.name}\)", - r, + assert repr(v) == ( + f"ExprValue({v.item()!r}, type='list[list[path]]', path_format=PathFormat.{pf.name})" ) + assert eval(repr(v)) == v + + +class TestExprValueReprEscaping: + """``__repr__`` escapes its embedded Python literal. + + ``repr_python`` previously escaped nothing, not even the quote or the + backslash, so a value carrying a quote, a backslash or a control + character produced a literal CPython cannot parse (a raw newline + terminates the string; a NUL cannot appear in source at all) or, worse, + one that parses as a different value (``'a\\b'`` is a backspace). + Fixed upstream in openjd-rs#374, reachable here via ``__repr__``. + + Expression Language 2.2.6 defines the escaping as following Python's + own ``repr``, so ``repr(str)`` is the oracle rather than a hand-written + expectation. + """ + + # Quotes and backslashes (delimiter selection and doubling), the C0 + # controls, DEL, and the non-ASCII characters CPython escapes by + # category -- U+0085, U+00A0, U+00AD, U+2028, U+2029, U+3000, U+200B, + # a private-use character and an astral non-printable -- alongside + # printable non-ASCII that must survive verbatim. + ESCAPING_CASES = [ + "it's", + 'say "hi"', + 'it\'s a "x"', + "'", + "a\\b", + "a\\", + "\\'", + "hello\nworld", + "a\rb", + "a\r\nb", + "a\tb", + "a\x00b", + "a\x1bb", + "a\x7fb", + "a\x0b\x0cb", + "café", + "a\U0001f600b", + "a\x85b", + "a\xa0b", + "a\xadb", + "a\u2028b", + "a\u2029b", + "a\u3000b", + "a\u200bb", + "a b", + "a\ue000b", + "a\U00100000b", + "a\U000e0100b", + ] + + @pytest.mark.parametrize("s", ESCAPING_CASES) + def test_repr_string_matches_cpython(self, s: str) -> None: + assert repr(ExprValue(s)) == f"ExprValue({s!r})" + + @pytest.mark.parametrize("s", ESCAPING_CASES) + def test_repr_string_round_trips(self, s: str) -> None: + # The point of escaping: the literal parses back to the value. + assert eval(repr(ExprValue(s))) == ExprValue(s) + + @pytest.mark.parametrize("s", ESCAPING_CASES) + def test_repr_list_element_matches_cpython(self, s: str) -> None: + # ``repr_python_list`` renders elements through the same writer; a + # list-only regression would go unnoticed by the scalar cases. + assert repr(ExprValue([s])) == f"ExprValue({[s]!r}, type='list[string]')" + + def test_repr_list_selects_delimiter_per_element(self) -> None: + # CPython picks a delimiter per element, so these two differ. + assert repr(ExprValue(["it's", "a\nb"])) == ( + "ExprValue([\"it's\", 'a\\nb'], type='list[string]')" + ) + + def test_repr_nested_list_element_escaped(self) -> None: + assert repr(ExprValue([["a\nb"]])) == r"ExprValue([['a\nb']], type='list[list[string]]')" + + def test_repr_path_escaped(self) -> None: + v = ExprValue("/tmp/a\nb.txt", type="path", path_format=PathFormat.POSIX) + assert repr(v) == r"ExprValue('/tmp/a\nb.txt', type='path', path_format=PathFormat.POSIX)" + + def test_repr_list_path_escaped(self) -> None: + # ``String`` and ``Path`` share one match arm; splitting them would + # leave ``list[path]`` unescaped. + v = ExprValue(["/a\nb"], type="list[path]", path_format=PathFormat.POSIX) + assert repr(v) == r"ExprValue(['/a\nb'], type='list[path]', path_format=PathFormat.POSIX)" + + @pytest.mark.parametrize( + "value,expected", + [ + (42, "ExprValue(42)"), + (True, "ExprValue(True)"), + (None, "ExprValue(None)"), + (Decimal("3.500"), "ExprValue('3.500', type='float')"), + ], + ) + def test_repr_non_string_text_unaltered(self, value: object, expected: str) -> None: + # Negative control: numeric and keyword text needs no escaping, so + # routing it through the shared writer must not change it. + assert repr(ExprValue(value)) == expected + + def test_repr_range_expr_unaltered(self) -> None: + assert repr(ExprValue("1-5", type="range_expr")) == "ExprValue('1-5', type='range_expr')" class TestMemorySize: From 8cd99f1b5e07b285120fd6c33d3bb0d77402c859 Mon Sep 17 00:00:00 2001 From: David Leong <116610336+leongdl@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:08:54 -0700 Subject: [PATCH 2/2] docs: Sync the validate_expressions docs and note the Zs control The doc comment on the binding moved to the 3-argument crate signature but its two derived copies did not, leaving three descriptions of one method disagreeing. _openjd_rs.pyi carried the old wording verbatim, including the paragraphs above it that this branch never touched, which confirms the stub is generated from the doc comment. Regenerated by hand: the local scripts/generate_stubs.sh cannot run on macOS -- its GNU `sed -i` patch step fails on BSD sed, and once applied manually the patch is inert because building stub_gen against the local Python 3.13 defines Py_3_10, so the `#[cfg(Py_3_10)]` guard admits a PyEncodingWarning that this pyo3 does not export. No workflow verifies stub freshness, so CI could not have caught the staleness either way. Also records why "a b" sits in ESCAPING_CASES. It is not a non-ASCII space flattened in editing; it is copied from the upstream py_escape tests, where it is the negative half of the Zs discrimination -- U+0020, U+00A0 and U+3000 are all Zs and CPython escapes only the latter two. The comment now says so, since the enumeration above it listed only the characters that do get escaped. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --- specs/python-expr-interface.md | 6 +++++- src/openjd/_openjd_rs.pyi | 11 +++++++++-- test/openjd/expr/test_expression_value.py | 6 ++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/specs/python-expr-interface.md b/specs/python-expr-interface.md index a88a412c..7ad6b38b 100644 --- a/specs/python-expr-interface.md +++ b/specs/python-expr-interface.md @@ -816,7 +816,11 @@ except FormatStringValidationError as e: The error message embeds the ``[start, end]`` byte offsets of the failing ``{{...}}`` pair so callers can produce structured diagnostics or syntax-highlight the failing segment. Mirrors the -Rust crate's ``FormatString::validate_expressions(symtab, lib)``. +Rust crate's +``FormatString::validate_expressions(symtab, lib, target_type)``, +which the binding calls with ``target_type=None`` to match ``resolve`` +and ``resolve_string``. The crate returns a ``StaticResolution``; the +binding is pass/fail only and discards it. **Equality and hashability.** `FormatString` implements `__eq__` and `__hash__` on the raw source string. Two format strings compare equal diff --git a/src/openjd/_openjd_rs.pyi b/src/openjd/_openjd_rs.pyi index c05192bb..9547295d 100644 --- a/src/openjd/_openjd_rs.pyi +++ b/src/openjd/_openjd_rs.pyi @@ -915,8 +915,15 @@ class FormatString: checking through the expression tree. Mirrors the Rust crate's - `FormatString::validate_expressions(symtab, lib)`. Returns - `None` on success. + `FormatString::validate_expressions(symtab, lib, target_type)`. + Returns `None` on success. + + `target_type` is passed as `None` because `resolve` and + `resolve_string` above resolve without one; validation has to + observe the same values resolution will produce. The crate + returns a `StaticResolution` (resolved-length bound and, when + fully concrete, the resolved value); this binding is pass/fail + only and discards it. """ def __str__(self) -> builtins.str: ... diff --git a/test/openjd/expr/test_expression_value.py b/test/openjd/expr/test_expression_value.py index 815495d4..4e2c1d50 100644 --- a/test/openjd/expr/test_expression_value.py +++ b/test/openjd/expr/test_expression_value.py @@ -377,6 +377,12 @@ class TestExprValueReprEscaping: # category -- U+0085, U+00A0, U+00AD, U+2028, U+2029, U+3000, U+200B, # a private-use character and an astral non-printable -- alongside # printable non-ASCII that must survive verbatim. + # + # "a b" is the negative half of the Zs discrimination: U+0020, + # U+00A0 and U+3000 are all Zs, and CPython escapes only the latter + # two. It is the case that isolates that distinction -- the quote + # cases above also carry a U+0020, but they vary the delimiter at + # the same time. ESCAPING_CASES = [ "it's", 'say "hi"',