From a437ba62c67fe28b613bde58b9948dbf7d9539de Mon Sep 17 00:00:00 2001 From: Mark <399551+mwiebe@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:30:47 -0700 Subject: [PATCH 1/3] fix(expr)!: reject regex constructs outside the Python/Rust intersection The regex dialect is specified as the intersection of Python's re module and Rust's regex crate, but four Rust-only constructs were accepted: Unicode property classes (\p{...}), the (?...) capture group spelling, POSIX character classes ([[:alpha:]]), and character class set difference (--). The last two silently match differently in engines that parse them as ordinary bracket expressions. Pattern validation previously inspected only the HIR, which erases these syntax distinctions. It now walks the pattern AST first (via regex_syntax::ast::Visitor), rejecting the four reported constructs plus the rest of the Rust-only class-set family: intersection (&&), symmetric difference (~~), and nested character classes ([a[b]]). The AST is then translated to HIR for the existing belt-and-braces walk, avoiding a second parse. Fixes #310 BREAKING CHANGE: regex patterns using \p{...}/\P{...}, (?...), POSIX character classes ([[:alpha:]]), character class set operators (--, &&, ~~), or nested character classes ([a[b]]) are now rejected with an 'Unsupported regex feature' error instead of being accepted. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> --- crates/openjd-expr/src/functions/regex.rs | 135 ++++++++++++++++-- .../tests/integration/test_strings.rs | 108 ++++++++++++++ specs/expr/function-library.md | 21 ++- 3 files changed, 248 insertions(+), 16 deletions(-) diff --git a/crates/openjd-expr/src/functions/regex.rs b/crates/openjd-expr/src/functions/regex.rs index 70f7bb77..bf08915e 100644 --- a/crates/openjd-expr/src/functions/regex.rs +++ b/crates/openjd-expr/src/functions/regex.rs @@ -52,18 +52,135 @@ fn validate_regex_pattern(pattern: &str) -> Result<(), ExpressionError> { // anchor — without re-implementing the full regex grammar. reject_rust_only_features(pattern)?; - // Parse the pattern into its HIR. Using `regex_syntax` (instead of a - // pure substring scan) means we correctly ignore lookaround-shaped - // sequences inside character classes, escapes, or regex comments. - // `regex_syntax` rejects lookaround, backreferences, and `\Z` at - // parse time; the HIR walker below is a belt-and-braces guard. - let hir = match regex_syntax::Parser::new().parse(pattern) { - Ok(h) => h, - Err(e) => return Err(translate_parse_error(e)), - }; + // Parse the pattern into its AST first and walk it. Several Rust-only + // constructs are erased (or normalized away) by the AST→HIR translation + // — `(?...)` vs `(?P...)`, POSIX classes, `\p{...}`, and + // character class set operators all become plain captures/classes in + // HIR — so they can only be rejected at the AST level. Using + // `regex_syntax` (instead of a pure substring scan) means we correctly + // ignore lookaround-shaped sequences inside character classes, escapes, + // or regex comments. `regex_syntax` rejects lookaround, backreferences, + // and `\Z` at parse time. + let ast = regex_syntax::ast::parse::Parser::new() + .parse(pattern) + .map_err(|e| translate_parse_error(e.into()))?; + check_ast_portability(&ast)?; + + // Translate the already-parsed AST to HIR (equivalent to + // `regex_syntax::Parser::new().parse(pattern)` but without re-parsing) + // for the belt-and-braces HIR walk below. + let hir = regex_syntax::hir::translate::Translator::new() + .translate(pattern, &ast) + .map_err(|e| translate_parse_error(e.into()))?; check_hir_portability(&hir) } +/// Walk the pattern AST and reject constructs that Rust's `regex` crate +/// accepts but that fall outside the spec's Python/Rust intersection dialect +/// (§2.2.5). These are invisible after AST→HIR translation, so they must be +/// caught here: +/// +/// - `\p{...}` / `\P{...}` — Unicode property classes (Rust-only; Python +/// `re` rejects them as a bad escape) +/// - `(?...)` — Rust's capture group name spelling (Python requires +/// `(?P...)`) +/// - `[[:alpha:]]` — POSIX character classes (Python parses these as +/// ordinary bracket expressions with silently different semantics) +/// - `--` / `&&` / `~~` — character class set operators (Python parses +/// these as ordinary ranges/literals with silently different semantics) +/// - `[a[b]]` — nested character classes (the enabling construct behind the +/// set operators; Python treats the inner `[` as a literal) +fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionError> { + use regex_syntax::ast; + + struct PortabilityVisitor; + + impl ast::Visitor for PortabilityVisitor { + type Output = (); + type Err = ExpressionError; + + fn finish(self) -> Result<(), ExpressionError> { + Ok(()) + } + + fn visit_pre(&mut self, node: &ast::Ast) -> Result<(), ExpressionError> { + match node { + ast::Ast::ClassUnicode(_) => Err(unicode_property_error()), + ast::Ast::Group(g) => match &g.kind { + ast::GroupKind::CaptureName { + starts_with_p: false, + .. + } => Err(ExpressionError::new( + "Unsupported regex feature: (?...) capture group; use (?P...)", + )), + _ => Ok(()), + }, + _ => Ok(()), + } + } + + fn visit_class_set_item_pre( + &mut self, + item: &ast::ClassSetItem, + ) -> Result<(), ExpressionError> { + match item { + ast::ClassSetItem::Ascii(c) => Err(ExpressionError::new(format!( + "Unsupported regex feature: POSIX character class [[:{}{}:]]", + if c.negated { "^" } else { "" }, + posix_class_name(&c.kind), + ))), + ast::ClassSetItem::Unicode(_) => Err(unicode_property_error()), + ast::ClassSetItem::Bracketed(_) => Err(ExpressionError::new( + "Unsupported regex feature: nested character class", + )), + _ => Ok(()), + } + } + + fn visit_class_set_binary_op_pre( + &mut self, + op: &ast::ClassSetBinaryOp, + ) -> Result<(), ExpressionError> { + let feature = match op.kind { + ast::ClassSetBinaryOpKind::Difference => "character class difference --", + ast::ClassSetBinaryOpKind::Intersection => "character class intersection &&", + ast::ClassSetBinaryOpKind::SymmetricDifference => { + "character class symmetric difference ~~" + } + }; + Err(ExpressionError::new(format!( + "Unsupported regex feature: {feature}" + ))) + } + } + + fn unicode_property_error() -> ExpressionError { + ExpressionError::new("Unsupported regex feature: Unicode property class \\p{...}") + } + + fn posix_class_name(kind: ®ex_syntax::ast::ClassAsciiKind) -> &'static str { + use regex_syntax::ast::ClassAsciiKind::*; + match kind { + Alnum => "alnum", + Alpha => "alpha", + Ascii => "ascii", + Blank => "blank", + Cntrl => "cntrl", + Digit => "digit", + Graph => "graph", + Lower => "lower", + Print => "print", + Punct => "punct", + Space => "space", + Upper => "upper", + Word => "word", + Xdigit => "xdigit", + } + } + + ast::visit(ast, PortabilityVisitor) +} + /// Scan the pattern source for Rust-only escape sequences that /// `regex_syntax` accepts but the spec forbids. Respects backslash-escape /// parity so `\\z` (literal backslash + `z`) isn't mistaken for the `\z` diff --git a/crates/openjd-expr/tests/integration/test_strings.rs b/crates/openjd-expr/tests/integration/test_strings.rs index 34d95b6f..79aa28c7 100644 --- a/crates/openjd-expr/tests/integration/test_strings.rs +++ b/crates/openjd-expr/tests/integration/test_strings.rs @@ -1020,6 +1020,114 @@ fn end_of_string_z_rejected() { ); } +// === Constructs outside the Python/Rust intersection (issue #310) === +#[test] +fn unicode_property_class_rejected() { + assert_err( + "re_search('3', r'\\p{Nd}')", + &[ + "Unsupported regex feature: Unicode property class \\p{...}\n", + " re_search('3', r'\\p{Nd}')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn unicode_property_class_inside_class_rejected() { + assert_err( + "re_search('a', r'[\\p{L}]')", + &[ + "Unsupported regex feature: Unicode property class \\p{...}\n", + " re_search('a', r'[\\p{L}]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn rust_capture_group_name_syntax_rejected() { + assert_err( + "re_search('ab', r'(?a)b')", + &[ + "Unsupported regex feature: (?...) capture group; use (?P...)\n", + " re_search('ab', r'(?a)b')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn python_capture_group_name_syntax_accepted() { + assert_eq!( + eval("re_search('ab', r'(?Pa)b')").to_display_string(), + "[\"ab\", \"a\"]" + ); +} +#[test] +fn posix_character_class_rejected() { + assert_err( + "re_search('a', r'[[:alpha:]]')", + &[ + "Unsupported regex feature: POSIX character class [[:alpha:]]\n", + " re_search('a', r'[[:alpha:]]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn negated_posix_character_class_rejected() { + assert_err( + "re_search('5', r'[[:^digit:]]')", + &[ + "Unsupported regex feature: POSIX character class [[:^digit:]]\n", + " re_search('5', r'[[:^digit:]]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn class_set_difference_rejected() { + assert_err( + "re_search('b', r'[a-z--[aeiou]]')", + &[ + "Unsupported regex feature: character class difference --\n", + " re_search('b', r'[a-z--[aeiou]]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn class_set_intersection_rejected() { + assert_err( + "re_search('e', r'[a-z&&[aeiou]]')", + &[ + "Unsupported regex feature: character class intersection &&\n", + " re_search('e', r'[a-z&&[aeiou]]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn class_set_symmetric_difference_rejected() { + assert_err( + "re_search('e', r'[a-z~~[aeiou]]')", + &[ + "Unsupported regex feature: character class symmetric difference ~~\n", + " re_search('e', r'[a-z~~[aeiou]]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn nested_character_class_rejected() { + assert_err( + "re_search('b', r'[a[bc]]')", + &[ + "Unsupported regex feature: nested character class\n", + " re_search('b', r'[a[bc]]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} + // === TestRegexEscapedPatternsAccepted === #[test] fn escaped_lookahead_accepted() { diff --git a/specs/expr/function-library.md b/specs/expr/function-library.md index 1715c6dc..14bf3db2 100644 --- a/specs/expr/function-library.md +++ b/specs/expr/function-library.md @@ -468,13 +468,20 @@ sections 2.1 (Operators) and 2.2 (Built-in Functions). Key implementation choice toward negative infinity. This avoids flooring an already-rounded direct quotient. - **`round()`** uses banker's rounding / round-half-even (§2.2.2) - **Regex functions** reject lookahead, lookbehind, backreferences, and `\Z` (§2.2.5). - Validation uses `regex_syntax::Parser` to parse the pattern into its HIR and - inspect the result, rather than a substring scan. This correctly ignores - lookaround-shaped syntax that appears inside character classes, escaped - sequences, or regex comments (e.g., `[(?=]`, `\?=`, `(?#...)`). The parser - rejects forbidden constructs at parse time; the translated error names the - specific feature (e.g., "Unsupported regex feature: lookahead") so callers - can produce stable diagnostics. + Validation parses the pattern with `regex_syntax`, rather than a substring + scan. This correctly ignores lookaround-shaped syntax that appears inside + character classes, escaped sequences, or regex comments (e.g., `[(?=]`, + `\?=`, `(?#...)`). The parser rejects forbidden constructs at parse time; + the translated error names the specific feature (e.g., "Unsupported regex + feature: lookahead") so callers can produce stable diagnostics. + Validation walks the pattern's **AST** (not just its HIR) because several + Rust-only constructs outside the spec's Python/Rust intersection dialect + are erased by AST→HIR translation and must be rejected at the AST level: + Unicode property classes (`\p{...}`/`\P{...}`), the `(?...)` capture + group spelling (Python requires `(?P...)`), POSIX character classes + (`[[:alpha:]]`), character class set operators (`--`, `&&`, `~~`), and + nested character classes (`[a[b]]`). The AST is then translated to HIR for + a belt-and-braces walk over the remaining constructs. - **`repr_sh/cmd/pwsh`** produce shell-safe quoting per platform conventions (§2.2.6). `repr_pwsh` renders nested lists as nested array literals, using the unary comma for a one-element outer list (`@(,@(1, 2))`) since `@(@(1, 2))` From ae95735839135d2873c18f37446eb0d8746b1b64 Mon Sep 17 00:00:00 2001 From: Mark <399551+mwiebe@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:17:16 -0700 Subject: [PATCH 2/3] fix(expr): reject Rust-only inline flags and word-boundary spellings in regex Two more constructs outside the spec's Python/Rust intersection dialect (2.2.5) slipped past the AST portability walk because they are erased by AST->HIR translation: - Inline flags: (?U) swap greed and (?R) CRLF mode are Rust-only (Python rejects them as unknown flags), bare global negation (?-...) has no Python equivalent, and negated Unicode mode -u silently changes \w / \d / . semantics. The shared flags i, m, s, x (and positive u) remain allowed, both bare and scoped, including scoped negation (?-i:...). - Word boundaries: \b{start}, \b{end}, \b{start-half}, \b{end-half}, \< and \> are Rust-only spellings that Python reads as \b plus literal characters (or a bad escape), diverging silently. Both are now rejected in check_ast_portability, and the HIR walker rejects the WordStart*/WordEnd* Look variants as a belt-and-braces backstop instead of allow-listing them. Addresses review feedback on #337. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> --- crates/openjd-expr/src/functions/regex.rs | 95 +++++++++++- .../tests/integration/test_strings.rs | 138 ++++++++++++++++++ specs/expr/function-library.md | 12 +- 3 files changed, 239 insertions(+), 6 deletions(-) diff --git a/crates/openjd-expr/src/functions/regex.rs b/crates/openjd-expr/src/functions/regex.rs index bf08915e..4d3e1428 100644 --- a/crates/openjd-expr/src/functions/regex.rs +++ b/crates/openjd-expr/src/functions/regex.rs @@ -90,6 +90,13 @@ fn validate_regex_pattern(pattern: &str) -> Result<(), ExpressionError> { /// these as ordinary ranges/literals with silently different semantics) /// - `[a[b]]` — nested character classes (the enabling construct behind the /// set operators; Python treats the inner `[` as a literal) +/// - `(?U)` / `(?R)` — Rust-only inline flags (swap greed, CRLF mode); +/// Python rejects them as unknown flags. Negated Unicode mode (`-u`) and +/// bare global negation `(?-...)` are likewise Rust-only. Shared flags +/// `i`, `m`, `s`, `x` (and positive `u`) stay allowed. +/// - `\b{start}`, `\b{end}`, `\b{start-half}`, `\b{end-half}`, `\<`, `\>` — +/// Rust-only word boundary spellings (Python reads `\b{start}` as `\b` +/// followed by the literal `{start}`, silently diverging) fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionError> { use regex_syntax::ast; @@ -106,6 +113,8 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE fn visit_pre(&mut self, node: &ast::Ast) -> Result<(), ExpressionError> { match node { ast::Ast::ClassUnicode(_) => Err(unicode_property_error()), + ast::Ast::Flags(set) => check_flags(&set.flags, false), + ast::Ast::Assertion(a) => check_assertion(&a.kind), ast::Ast::Group(g) => match &g.kind { ast::GroupKind::CaptureName { starts_with_p: false, @@ -113,6 +122,7 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE } => Err(ExpressionError::new( "Unsupported regex feature: (?...) capture group; use (?P...)", )), + ast::GroupKind::NonCapturing(flags) => check_flags(flags, true), _ => Ok(()), }, _ => Ok(()), @@ -158,6 +168,79 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE ExpressionError::new("Unsupported regex feature: Unicode property class \\p{...}") } + /// Validate an inline flag sequence against the Python/Rust + /// intersection. `scoped` is true for `(?flags:...)` groups and false + /// for bare `(?flags)` settings. + /// + /// - `i`, `m`, `s`, `x` are shared by both engines and always allowed. + /// - Positive `u` is allowed: both engines accept it and both default + /// to Unicode mode. + /// - `U` (swap greed) and `R` (CRLF mode) are Rust-only; Python rejects + /// them as unknown flags. + /// - Negation is only allowed in the scoped form `(?-imsx:...)`; Python + /// has no bare `(?-...)` global negation. + /// - Negated `u` is Rust-only: Python never allows negating Unicode + /// mode, and in Rust it silently changes `\w`/`\d`/`.` semantics. + fn check_flags(flags: &ast::Flags, scoped: bool) -> Result<(), ExpressionError> { + let mut negated = false; + for item in &flags.items { + match &item.kind { + ast::FlagsItemKind::Negation => { + if !scoped { + return Err(ExpressionError::new( + "Unsupported regex feature: global flag negation (?-...); \ + use a scoped group (?-i:...)", + )); + } + negated = true; + } + ast::FlagsItemKind::Flag(f) => match f { + ast::Flag::CaseInsensitive + | ast::Flag::MultiLine + | ast::Flag::DotMatchesNewLine + | ast::Flag::IgnoreWhitespace => {} + ast::Flag::Unicode => { + if negated { + return Err(ExpressionError::new( + "Unsupported regex feature: negated Unicode flag -u", + )); + } + } + ast::Flag::SwapGreed => { + return Err(ExpressionError::new( + "Unsupported regex feature: inline flag U (swap greed)", + )); + } + ast::Flag::CRLF => { + return Err(ExpressionError::new( + "Unsupported regex feature: inline flag R (CRLF mode)", + )); + } + }, + } + } + Ok(()) + } + + /// Reject the Rust-only word boundary spellings. Python's `re` does not + /// error on these — it reads `\b{start}` as `\b` followed by the + /// literal characters `{start}` — so they diverge silently. + fn check_assertion(kind: &ast::AssertionKind) -> Result<(), ExpressionError> { + use ast::AssertionKind::*; + let spelling = match kind { + WordBoundaryStart => "\\b{start}", + WordBoundaryEnd => "\\b{end}", + WordBoundaryStartAngle => "\\<", + WordBoundaryEndAngle => "\\>", + WordBoundaryStartHalf => "\\b{start-half}", + WordBoundaryEndHalf => "\\b{end-half}", + _ => return Ok(()), + }; + Err(ExpressionError::new(format!( + "Unsupported regex feature: word boundary assertion {spelling}" + ))) + } + fn posix_class_name(kind: ®ex_syntax::ast::ClassAsciiKind) -> &'static str { use regex_syntax::ast::ClassAsciiKind::*; match kind { @@ -281,15 +364,21 @@ fn check_hir_portability(hir: ®ex_syntax::hir::Hir) -> Result<(), ExpressionE | Look::WordAscii | Look::WordAsciiNegate | Look::WordUnicode - | Look::WordUnicodeNegate - | Look::WordStartAscii + | Look::WordUnicodeNegate => Ok(()), + // Rust-only word boundary spellings (`\b{start}`, `\<`, ...) + // lower to these. They are rejected with precise messages by + // `check_ast_portability`; this arm is the belt-and-braces + // backstop. + Look::WordStartAscii | Look::WordEndAscii | Look::WordStartUnicode | Look::WordEndUnicode | Look::WordStartHalfAscii | Look::WordEndHalfAscii | Look::WordStartHalfUnicode - | Look::WordEndHalfUnicode => Ok(()), + | Look::WordEndHalfUnicode => Err(ExpressionError::new( + "Unsupported regex feature: Rust-only word boundary assertion", + )), }, HirKind::Capture(c) => check_hir_portability(&c.sub), HirKind::Repetition(r) => check_hir_portability(&r.sub), diff --git a/crates/openjd-expr/tests/integration/test_strings.rs b/crates/openjd-expr/tests/integration/test_strings.rs index 79aa28c7..e64cfea1 100644 --- a/crates/openjd-expr/tests/integration/test_strings.rs +++ b/crates/openjd-expr/tests/integration/test_strings.rs @@ -1127,6 +1127,144 @@ fn nested_character_class_rejected() { ], ); } +#[test] +fn swap_greed_inline_flag_rejected() { + assert_err( + "re_search('aaa', r'(?U)a+')", + &[ + "Unsupported regex feature: inline flag U (swap greed)\n", + " re_search('aaa', r'(?U)a+')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn crlf_inline_flag_rejected() { + assert_err( + "re_search('a', r'(?R)^a')", + &[ + "Unsupported regex feature: inline flag R (CRLF mode)\n", + " re_search('a', r'(?R)^a')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn scoped_swap_greed_flag_rejected() { + assert_err( + "re_search('aaa', r'(?U:a+)')", + &[ + "Unsupported regex feature: inline flag U (swap greed)\n", + " re_search('aaa', r'(?U:a+)')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn global_flag_negation_rejected() { + assert_err( + "re_search('A', r'(?-i)a')", + &[ + "Unsupported regex feature: global flag negation (?-...); use a scoped group (?-i:...)\n", + " re_search('A', r'(?-i)a')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn negated_unicode_flag_rejected() { + assert_err( + "re_search('a', r'(?-u:a)')", + &[ + "Unsupported regex feature: negated Unicode flag -u\n", + " re_search('a', r'(?-u:a)')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn shared_inline_flags_accepted() { + // `i`, `m`, `s`, `x` (and positive `u`) are in the Python/Rust + // intersection, bare and scoped, including scoped negation. + assert!(eval("re_search('HELLO', r'(?i)hello')").is_list()); + assert!(eval("re_search('a\\nb', r'(?m)^b')").is_list()); + assert!(eval("re_search('a\\nb', r'(?s)a.b')").is_list()); + assert!(eval("re_search('ab', r'(?x)a b')").is_list()); + assert!(eval("re_search('a', r'(?u)\\w')").is_list()); + assert!(eval("re_search('HELLO', r'(?i:hello)')").is_list()); + assert!(eval("re_search('hello', r'(?-i:hello)')").is_list()); +} +#[test] +fn word_boundary_start_rejected() { + assert_err( + "re_search('ab', r'\\b{start}a')", + &[ + "Unsupported regex feature: word boundary assertion \\b{start}\n", + " re_search('ab', r'\\b{start}a')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn word_boundary_end_rejected() { + assert_err( + "re_search('ab', r'a\\b{end}')", + &[ + "Unsupported regex feature: word boundary assertion \\b{end}\n", + " re_search('ab', r'a\\b{end}')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn word_boundary_start_angle_rejected() { + assert_err( + "re_search('ab', r'\\')", + &[ + "Unsupported regex feature: word boundary assertion \\>\n", + " re_search('ab', r'a\\>')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn word_boundary_start_half_rejected() { + assert_err( + "re_search('ab', r'\\b{start-half}a')", + &[ + "Unsupported regex feature: word boundary assertion \\b{start-half}\n", + " re_search('ab', r'\\b{start-half}a')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn word_boundary_end_half_rejected() { + assert_err( + "re_search('ab', r'a\\b{end-half}')", + &[ + "Unsupported regex feature: word boundary assertion \\b{end-half}\n", + " re_search('ab', r'a\\b{end-half}')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn portable_word_boundaries_accepted() { + assert!(eval("re_search('hello world', r'\\bworld\\b')").is_list()); + assert!(eval("re_search('hello', r'l\\Bl')").is_list()); +} // === TestRegexEscapedPatternsAccepted === #[test] diff --git a/specs/expr/function-library.md b/specs/expr/function-library.md index 14bf3db2..ec732dc8 100644 --- a/specs/expr/function-library.md +++ b/specs/expr/function-library.md @@ -479,9 +479,15 @@ sections 2.1 (Operators) and 2.2 (Built-in Functions). Key implementation choice are erased by AST→HIR translation and must be rejected at the AST level: Unicode property classes (`\p{...}`/`\P{...}`), the `(?...)` capture group spelling (Python requires `(?P...)`), POSIX character classes - (`[[:alpha:]]`), character class set operators (`--`, `&&`, `~~`), and - nested character classes (`[a[b]]`). The AST is then translated to HIR for - a belt-and-braces walk over the remaining constructs. + (`[[:alpha:]]`), character class set operators (`--`, `&&`, `~~`), + nested character classes (`[a[b]]`), Rust-only inline flags (`U` swap + greed, `R` CRLF mode, negated `u`, and bare global negation `(?-...)`; + the shared flags `i`, `m`, `s`, `x` and positive `u` remain allowed, + including scoped negation `(?-i:...)`), and Rust-only word boundary + spellings (`\b{start}`, `\b{end}`, `\b{start-half}`, `\b{end-half}`, + `\<`, `\>` — Python reads these as `\b` plus literal characters, silently + diverging). The AST is then translated to HIR for a belt-and-braces walk + over the remaining constructs. - **`repr_sh/cmd/pwsh`** produce shell-safe quoting per platform conventions (§2.2.6). `repr_pwsh` renders nested lists as nested array literals, using the unary comma for a one-element outer list (`@(,@(1, 2))`) since `@(@(1, 2))` From 0cd37009da939aca8fb383bb909a68252edbb06f Mon Sep 17 00:00:00 2001 From: Mark <399551+mwiebe@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:53:36 -0700 Subject: [PATCH 3/3] fix(expr): close three more silent regex dialect divergences Round 2 of review feedback on the Python/Rust intersection dialect guard (spec 2.2.5): - Verbose mode + character classes: Rust strips unescaped whitespace and # comments inside bracketed classes under (?x); Python's VERBOSE mode explicitly keeps them as literals, so (?x)[a b] matches differently with no error on either side. Rejected by scanning class source spans when any positive x flag is present (scope is not tracked, so this can over-reject a class outside an (?x:...) group, which is safe). - Bare inline flag position: Python 3.11+ rejects global flags that are not at the start of the pattern, and 3.6-3.10 applied them to the whole pattern where Rust applies them forward-only. Bare (?flags) groups must now form a contiguous run from offset 0; consecutive leading groups like (?i)(?s)ab stay allowed. - Capture group names: regex_syntax permits '.', '[', ']' in names (non-first position); Python raises 'bad character in group name'. Names must now be valid Python identifiers. Also documents the known accepted divergence for plain \$ (Python matches before a trailing newline, Rust is end-of-haystack only) in the check_hir_portability doc comment and the spec. Addresses review feedback on #337. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> --- crates/openjd-expr/src/functions/regex.rs | 150 ++++++++++++++++-- .../tests/integration/test_strings.rs | 79 +++++++++ specs/expr/function-library.md | 31 ++-- 3 files changed, 238 insertions(+), 22 deletions(-) diff --git a/crates/openjd-expr/src/functions/regex.rs b/crates/openjd-expr/src/functions/regex.rs index 4d3e1428..0e2e5d12 100644 --- a/crates/openjd-expr/src/functions/regex.rs +++ b/crates/openjd-expr/src/functions/regex.rs @@ -64,7 +64,7 @@ fn validate_regex_pattern(pattern: &str) -> Result<(), ExpressionError> { let ast = regex_syntax::ast::parse::Parser::new() .parse(pattern) .map_err(|e| translate_parse_error(e.into()))?; - check_ast_portability(&ast)?; + check_ast_portability(pattern, &ast)?; // Translate the already-parsed AST to HIR (equivalent to // `regex_syntax::Parser::new().parse(pattern)` but without re-parsing) @@ -94,26 +94,101 @@ fn validate_regex_pattern(pattern: &str) -> Result<(), ExpressionError> { /// Python rejects them as unknown flags. Negated Unicode mode (`-u`) and /// bare global negation `(?-...)` are likewise Rust-only. Shared flags /// `i`, `m`, `s`, `x` (and positive `u`) stay allowed. +/// - `a(?i)b` — bare inline flags anywhere but the start of the pattern. +/// Python 3.11+ rejects "global flags not at the start of the +/// expression" (3.6–3.10 applied them to the whole pattern, unlike +/// Rust's forward-only application). Consecutive leading flag groups +/// (`(?i)(?s)ab`) are accepted by both engines and stay allowed. +/// - `(?x)[a b]` — verbose mode combined with unescaped whitespace or `#` +/// inside a character class. Rust strips whitespace/comments inside +/// classes under `x`; Python's VERBOSE mode explicitly keeps them as +/// literals, so such patterns match differently with no error on either +/// side. +/// - `(?P...)` — capture group names that are not valid Python +/// identifiers. `regex_syntax` also permits `.`, `[`, and `]` in names; +/// Python raises "bad character in group name". /// - `\b{start}`, `\b{end}`, `\b{start-half}`, `\b{end-half}`, `\<`, `\>` — /// Rust-only word boundary spellings (Python reads `\b{start}` as `\b` /// followed by the literal `{start}`, silently diverging) -fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionError> { +fn check_ast_portability( + pattern: &str, + ast: ®ex_syntax::ast::Ast, +) -> Result<(), ExpressionError> { use regex_syntax::ast; - struct PortabilityVisitor; + struct PortabilityVisitor<'a> { + /// The pattern source, for inspecting character class bodies. + pattern: &'a str, + /// True if any positive `x` (verbose / ignore-whitespace) flag + /// appears anywhere in the pattern, bare or scoped. Scope is not + /// tracked: a class outside an `(?x:...)` group's scope may be + /// flagged too, which over-rejects but never diverges silently. + uses_verbose: bool, + /// Source spans of every bracketed character class. + class_spans: Vec, + /// Source spans of every bare `(?flags)` group. + bare_flag_spans: Vec, + } - impl ast::Visitor for PortabilityVisitor { + impl ast::Visitor for PortabilityVisitor<'_> { type Output = (); type Err = ExpressionError; fn finish(self) -> Result<(), ExpressionError> { + // Bare `(?flags)` groups must form a contiguous run from the + // start of the pattern. Python 3.11+ rejects global flags + // anywhere else, and pre-3.11 Pythons applied them to the whole + // pattern where Rust applies them only forward. + let mut spans = self.bare_flag_spans; + spans.sort_by_key(|s| s.start.offset); + let mut expected = 0; + for s in &spans { + if s.start.offset != expected { + return Err(ExpressionError::new( + "Unsupported regex feature: bare inline flags not at the start \ + of the pattern; use a scoped group like (?i:...)", + )); + } + expected = s.end.offset; + } + + // Under verbose mode, Rust strips unescaped whitespace and `#` + // comments inside character classes while Python keeps them as + // literals — a silent divergence. + if self.uses_verbose { + for span in &self.class_spans { + let body = &self.pattern[span.start.offset..span.end.offset]; + let mut escaped = false; + for c in body.chars() { + if escaped { + escaped = false; + } else if c == '\\' { + escaped = true; + } else if c.is_whitespace() || c == '#' { + return Err(ExpressionError::new( + "Unsupported regex feature: verbose mode (?x) with \ + whitespace or '#' in a character class; Python treats \ + them as literals", + )); + } + } + } + } Ok(()) } fn visit_pre(&mut self, node: &ast::Ast) -> Result<(), ExpressionError> { match node { ast::Ast::ClassUnicode(_) => Err(unicode_property_error()), - ast::Ast::Flags(set) => check_flags(&set.flags, false), + ast::Ast::ClassBracketed(c) => { + self.class_spans.push(c.span); + Ok(()) + } + ast::Ast::Flags(set) => { + self.bare_flag_spans.push(set.span); + self.uses_verbose |= check_flags(&set.flags, false)?; + Ok(()) + } ast::Ast::Assertion(a) => check_assertion(&a.kind), ast::Ast::Group(g) => match &g.kind { ast::GroupKind::CaptureName { @@ -122,7 +197,11 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE } => Err(ExpressionError::new( "Unsupported regex feature: (?...) capture group; use (?P...)", )), - ast::GroupKind::NonCapturing(flags) => check_flags(flags, true), + ast::GroupKind::CaptureName { name, .. } => check_capture_name(&name.name), + ast::GroupKind::NonCapturing(flags) => { + self.uses_verbose |= check_flags(flags, true)?; + Ok(()) + } _ => Ok(()), }, _ => Ok(()), @@ -170,7 +249,9 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE /// Validate an inline flag sequence against the Python/Rust /// intersection. `scoped` is true for `(?flags:...)` groups and false - /// for bare `(?flags)` settings. + /// for bare `(?flags)` settings. Returns true if a positive `x` + /// (verbose / ignore-whitespace) flag is present, so the caller can run + /// the whitespace-in-class check. /// /// - `i`, `m`, `s`, `x` are shared by both engines and always allowed. /// - Positive `u` is allowed: both engines accept it and both default @@ -181,8 +262,9 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE /// has no bare `(?-...)` global negation. /// - Negated `u` is Rust-only: Python never allows negating Unicode /// mode, and in Rust it silently changes `\w`/`\d`/`.` semantics. - fn check_flags(flags: &ast::Flags, scoped: bool) -> Result<(), ExpressionError> { + fn check_flags(flags: &ast::Flags, scoped: bool) -> Result { let mut negated = false; + let mut verbose = false; for item in &flags.items { match &item.kind { ast::FlagsItemKind::Negation => { @@ -197,8 +279,12 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE ast::FlagsItemKind::Flag(f) => match f { ast::Flag::CaseInsensitive | ast::Flag::MultiLine - | ast::Flag::DotMatchesNewLine - | ast::Flag::IgnoreWhitespace => {} + | ast::Flag::DotMatchesNewLine => {} + ast::Flag::IgnoreWhitespace => { + if !negated { + verbose = true; + } + } ast::Flag::Unicode => { if negated { return Err(ExpressionError::new( @@ -219,7 +305,31 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE }, } } - Ok(()) + Ok(verbose) + } + + /// Require capture group names to be valid Python identifiers. + /// `regex_syntax` additionally permits `.`, `[`, and `]` in names + /// (non-first position); Python's `re` raises "bad character in group + /// name" for those, so a pattern accepted here would fail outright + /// under the Python implementation. + fn check_capture_name(name: &str) -> Result<(), ExpressionError> { + let valid = name.chars().enumerate().all(|(i, c)| { + c == '_' + || if i == 0 { + c.is_alphabetic() + } else { + c.is_alphanumeric() + } + }); + if valid { + Ok(()) + } else { + Err(ExpressionError::new(format!( + "Unsupported regex feature: capture group name '{name}' is not a valid \ + Python identifier" + ))) + } } /// Reject the Rust-only word boundary spellings. Python's `re` does not @@ -261,7 +371,15 @@ fn check_ast_portability(ast: ®ex_syntax::ast::Ast) -> Result<(), ExpressionE } } - ast::visit(ast, PortabilityVisitor) + ast::visit( + ast, + PortabilityVisitor { + pattern, + uses_verbose: false, + class_spans: Vec::new(), + bare_flag_spans: Vec::new(), + }, + ) } /// Scan the pattern source for Rust-only escape sequences that @@ -350,6 +468,14 @@ fn translate_parse_error(err: regex_syntax::Error) -> ExpressionError { /// `regex_syntax` already rejects lookaround, backreferences, and `\Z`/`\z` /// at parse time, so in practice this walker is a belt-and-braces guard for /// any future grammar additions that expose these constructs via HIR. +/// +/// **Known divergence — `$` (`Look::End`):** without MULTILINE, Python's +/// `$` matches at end of string *or immediately before a trailing newline* +/// (`re.search(r"a$", "a\n")` matches), while Rust's `$` is end-of-haystack +/// only (Python's `$` is closer to Rust's `(?:\n?\z)`). The spec's +/// intersection dialect allows `$`, so it stays accepted here even though +/// results differ on newline-terminated input. This is the same divergence +/// family as `\Z` vs `\z`, both of which are rejected outright. fn check_hir_portability(hir: ®ex_syntax::hir::Hir) -> Result<(), ExpressionError> { use regex_syntax::hir::{HirKind, Look}; match hir.kind() { diff --git a/crates/openjd-expr/tests/integration/test_strings.rs b/crates/openjd-expr/tests/integration/test_strings.rs index e64cfea1..2925a82c 100644 --- a/crates/openjd-expr/tests/integration/test_strings.rs +++ b/crates/openjd-expr/tests/integration/test_strings.rs @@ -1265,6 +1265,85 @@ fn portable_word_boundaries_accepted() { assert!(eval("re_search('hello world', r'\\bworld\\b')").is_list()); assert!(eval("re_search('hello', r'l\\Bl')").is_list()); } +#[test] +fn verbose_mode_class_whitespace_rejected() { + assert_err( + "re_search('a', r'(?x)[a b]')", + &[ + "Unsupported regex feature: verbose mode (?x) with whitespace or '#' in a character class; Python treats them as literals\n", + " re_search('a', r'(?x)[a b]')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn scoped_verbose_mode_class_whitespace_rejected() { + assert_err( + "re_search('a', r'(?x:[a b])')", + &[ + "Unsupported regex feature: verbose mode (?x) with whitespace or '#' in a character class; Python treats them as literals\n", + " re_search('a', r'(?x:[a b])')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn verbose_mode_escaped_space_in_class_accepted() { + // An escaped space is a literal space in both engines, even under + // verbose mode. + assert!(eval("re_search(' ', r'(?x)[a\\ b]')").is_list()); +} +#[test] +fn verbose_mode_class_without_whitespace_accepted() { + assert!(eval("re_search('a', r'(?x) [ab] c?')").is_list()); +} +#[test] +fn bare_flags_mid_pattern_rejected() { + // Python 3.11+ raises "global flags not at the start of the + // expression"; pre-3.11 applied the flag to the whole pattern where + // Rust applies it only forward. + assert_err( + "re_search('ab', r'a(?i)b')", + &[ + "Unsupported regex feature: bare inline flags not at the start of the pattern; use a scoped group like (?i:...)\n", + " re_search('ab', r'a(?i)b')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn multiple_leading_bare_flags_accepted() { + // Consecutive flag groups at the very start are fine in both engines. + assert!(eval("re_search('A B', r'(?i)(?s)a.b')").is_list()); +} +#[test] +fn capture_name_with_dot_rejected() { + // regex_syntax permits `.` in group names; Python raises "bad + // character in group name". + assert_err( + "re_search('ab', r'(?Pa)b')", + &[ + "Unsupported regex feature: capture group name 'a.b' is not a valid Python identifier\n", + " re_search('ab', r'(?Pa)b')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn capture_name_with_bracket_rejected() { + assert_err( + "re_search('ab', r'(?Pa)b')", + &[ + "Unsupported regex feature: capture group name 'a[b' is not a valid Python identifier\n", + " re_search('ab', r'(?Pa)b')\n", + " ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~", + ], + ); +} +#[test] +fn capture_name_with_underscore_and_digits_accepted() { + assert!(eval("re_search('ab', r'(?Pa)b')").is_list()); +} // === TestRegexEscapedPatternsAccepted === #[test] diff --git a/specs/expr/function-library.md b/specs/expr/function-library.md index ec732dc8..17b01343 100644 --- a/specs/expr/function-library.md +++ b/specs/expr/function-library.md @@ -478,16 +478,27 @@ sections 2.1 (Operators) and 2.2 (Built-in Functions). Key implementation choice Rust-only constructs outside the spec's Python/Rust intersection dialect are erased by AST→HIR translation and must be rejected at the AST level: Unicode property classes (`\p{...}`/`\P{...}`), the `(?...)` capture - group spelling (Python requires `(?P...)`), POSIX character classes - (`[[:alpha:]]`), character class set operators (`--`, `&&`, `~~`), - nested character classes (`[a[b]]`), Rust-only inline flags (`U` swap - greed, `R` CRLF mode, negated `u`, and bare global negation `(?-...)`; - the shared flags `i`, `m`, `s`, `x` and positive `u` remain allowed, - including scoped negation `(?-i:...)`), and Rust-only word boundary - spellings (`\b{start}`, `\b{end}`, `\b{start-half}`, `\b{end-half}`, - `\<`, `\>` — Python reads these as `\b` plus literal characters, silently - diverging). The AST is then translated to HIR for a belt-and-braces walk - over the remaining constructs. + group spelling (Python requires `(?P...)`), capture group names + that are not valid Python identifiers (`regex_syntax` also permits `.`, + `[`, `]`; Python raises "bad character in group name"), POSIX character + classes (`[[:alpha:]]`), character class set operators (`--`, `&&`, + `~~`), nested character classes (`[a[b]]`), Rust-only inline flags (`U` + swap greed, `R` CRLF mode, negated `u`, and bare global negation + `(?-...)`; the shared flags `i`, `m`, `s`, `x` and positive `u` remain + allowed, including scoped negation `(?-i:...)`), bare inline flags + anywhere but the start of the pattern (`a(?i)b` — an error in Python + 3.11+, applied globally rather than forward-only in older Pythons; + consecutive leading flag groups stay allowed), verbose mode combined with + unescaped whitespace or `#` inside a character class (`(?x)[a b]` — Rust + strips them, Python VERBOSE keeps them as literals), and Rust-only word + boundary spellings (`\b{start}`, `\b{end}`, `\b{start-half}`, + `\b{end-half}`, `\<`, `\>` — Python reads these as `\b` plus literal + characters, silently diverging). The AST is then translated to HIR for a + belt-and-braces walk over the remaining constructs. + **Known accepted divergence:** plain `$` without MULTILINE matches before + a trailing newline in Python but is end-of-haystack only in Rust + (Python's `$` ≈ Rust's `(?:\n?\z)`); the intersection dialect allows `$`, + so results differ on newline-terminated input. - **`repr_sh/cmd/pwsh`** produce shell-safe quoting per platform conventions (§2.2.6). `repr_pwsh` renders nested lists as nested array literals, using the unary comma for a one-element outer list (`@(,@(1, 2))`) since `@(@(1, 2))`