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/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 49ad870b..4e2c1d50 100644 --- a/test/openjd/expr/test_expression_value.py +++ b/test/openjd/expr/test_expression_value.py @@ -337,27 +337,133 @@ 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. + # + # "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"', + '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: