diff --git a/crates/openjd-expr/src/functions/arithmetic.rs b/crates/openjd-expr/src/functions/arithmetic.rs index 6576f113..9609bcb4 100644 --- a/crates/openjd-expr/src/functions/arithmetic.rs +++ b/crates/openjd-expr/src/functions/arithmetic.rs @@ -180,12 +180,32 @@ pub fn truediv_float(_: Ctx, a: &[ExprValue]) -> R { Ok(ExprValue::Float(Float64::new(l / r)?)) } +// Matches CPython's finite-float divmod algorithm. +fn float_divmod(l: f64, r: f64) -> (f64, f64) { + let mut rem = l % r; + let mut div = (l - rem) / r; + if rem != 0.0 { + if rem.is_sign_negative() != r.is_sign_negative() { + rem += r; + div -= 1.0; + } + } else { + rem = 0.0_f64.copysign(r); + } + + let mut quotient = div.floor(); + if div - quotient > 0.5 { + quotient += 1.0; + } + (quotient, rem) +} + pub fn floordiv_float(_: Ctx, a: &[ExprValue]) -> R { let (l, r) = get_two_floats(a)?; if r == 0.0 { return Err(ExpressionError::division_by_zero("Division")); } - let v = (l / r).floor(); + let (v, _) = float_divmod(l, r); if !float_fits_i64(v) { return Err(ExpressionError::integer_overflow()); } @@ -197,8 +217,8 @@ pub fn mod_float(_: Ctx, a: &[ExprValue]) -> R { if r == 0.0 { return Err(ExpressionError::division_by_zero("Modulo")); } - // Python uses floored modulo: l - r * floor(l / r) - Ok(ExprValue::Float(Float64::new(l - r * (l / r).floor())?)) + let (_, rem) = float_divmod(l, r); + Ok(ExprValue::Float(Float64::new(rem)?)) } pub fn pow_float(_: Ctx, a: &[ExprValue]) -> R { diff --git a/crates/openjd-expr/src/functions/path.rs b/crates/openjd-expr/src/functions/path.rs index 0e7750f0..f8d35c9a 100644 --- a/crates/openjd-expr/src/functions/path.rs +++ b/crates/openjd-expr/src/functions/path.rs @@ -421,14 +421,37 @@ pub fn is_absolute(path_str: &str, fmt: PathFormat) -> bool { /// Join two path strings using the separator and absoluteness rules for `fmt`. /// -/// If `right` is absolute (according to `fmt`), it replaces `left` entirely. -/// On Windows, if `right` starts with a single `/` or `\` (root-relative), -/// the drive letter from `left` is preserved (matching `ntpath.join` behavior). +/// An absolute `right` replaces `left` entirely. For URI left operands, a +/// non-absolute leading path separator resets the path below the existing +/// authority; this is the Windows root-relative case. A Windows drive-relative +/// `right` replaces a URI left operand entirely. +/// On Windows filesystem paths, if `right` starts with a single `/` or `\` +/// (root-relative), the drive or UNC root from `left` is preserved; if `left` +/// has no root, `right` replaces it entirely (matching `ntpath.join` behavior). /// Otherwise, `right` is appended to `left` with the appropriate separator. pub fn join(left: &str, right: &str, fmt: PathFormat) -> String { if is_absolute(right, fmt) { return right.to_string(); } + if let Some(uri) = crate::uri_path::parse(left) { + let right_bytes = right.as_bytes(); + if fmt == PathFormat::Windows + && right_bytes.len() >= 2 + && right_bytes[0].is_ascii_alphabetic() + && right_bytes[1] == b':' + { + return right.to_string(); + } + let right = if fmt == PathFormat::Windows { + std::borrow::Cow::Owned(right.replace('\\', "/")) + } else { + std::borrow::Cow::Borrowed(right) + }; + if right.starts_with('/') { + return format!("{}{right}", uri.authority); + } + return format!("{}/{}", left.trim_end_matches('/'), right); + } // Windows root-relative: /foo or \foo (but not \\server) replaces the path // but keeps the root from left. Matches ntpath.join behavior. // For drive paths (C:\...), the root is "C:". @@ -445,27 +468,16 @@ pub fn join(left: &str, right: &str, fmt: PathFormat) -> String { if let Some(unc_root) = extract_unc_root(left) { return format!("{unc_root}{right}"); } + return right.to_string(); } } - let left_is_uri = crate::uri_path::is_uri(left); - let (sep, trim_chars): (&str, &[char]) = if left_is_uri { - ("/", &['/']) - } else { - match fmt { - // On Windows, both / and \ are separators - PathFormat::Windows => ("\\", &['/', '\\']), - // On POSIX, only / is a separator (\ is a valid filename char) - PathFormat::Posix | PathFormat::Uri => ("/", &['/']), - } + let (sep, trim_chars): (&str, &[char]) = match fmt { + // On Windows, both / and \ are separators + PathFormat::Windows => ("\\", &['/', '\\']), + // On POSIX, only / is a separator (\ is a valid filename char) + PathFormat::Posix | PathFormat::Uri => ("/", &['/']), }; let left = left.trim_end_matches(trim_chars); - // When appending to a URI from a Windows context, normalize backslashes to forward slashes. - // In POSIX context, backslashes are valid filename characters and must not be converted. - let right = if left_is_uri && fmt == PathFormat::Windows { - std::borrow::Cow::Owned(right.replace('\\', "/")) - } else { - std::borrow::Cow::Borrowed(right) - }; format!("{left}{sep}{right}") } @@ -476,7 +488,7 @@ pub fn join(left: &str, right: &str, fmt: PathFormat) -> String { /// `is_absolute` without URI recognition). This prevents `scheme://...` strings /// from being treated as absolute when URI support is disabled. pub fn non_uri_join(left: &str, right: &str, fmt: PathFormat) -> String { - // Windows root-relative: /foo or \foo keeps the root from left + // Windows root-relative: keep a drive/UNC root, or replace a rootless left path. if fmt == PathFormat::Windows { let rb = right.as_bytes(); if rb.first() == Some(&b'/') || rb.first() == Some(&b'\\') { @@ -487,6 +499,7 @@ pub fn non_uri_join(left: &str, right: &str, fmt: PathFormat) -> String { if let Some(unc_root) = extract_unc_root(left) { return format!("{unc_root}{right}"); } + return right.to_string(); } } let (sep, trim_chars): (&str, &[char]) = match fmt { diff --git a/crates/openjd-expr/src/functions/string.rs b/crates/openjd-expr/src/functions/string.rs index 779c313b..58ca3ff3 100644 --- a/crates/openjd-expr/src/functions/string.rs +++ b/crates/openjd-expr/src/functions/string.rs @@ -374,7 +374,7 @@ pub fn center_fn(ctx: Ctx, a: &[ExprValue]) -> R { return Ok(budget.finish(s.to_string())); } let pad = width - char_len; - let left = pad / 2; + let left = pad / 2 + (pad & width & 1); let right = pad - left; let mut result = String::with_capacity(output_bytes); for _ in 0..left { diff --git a/crates/openjd-expr/src/value.rs b/crates/openjd-expr/src/value.rs index 01af370e..996174b1 100644 --- a/crates/openjd-expr/src/value.rs +++ b/crates/openjd-expr/src/value.rs @@ -1479,8 +1479,8 @@ impl ExprValue { (Self::Bool(a), Self::Bool(b)) => Ok(a.cmp(b)), (Self::String(a), Self::String(b)) => Ok(a.cmp(b)), (Self::Path { value: a, .. }, Self::Path { value: b, .. }) => Ok(a.cmp(b)), - (Self::String(a), Self::Path { value: b, .. }) - | (Self::Path { value: b, .. }, Self::String(a)) => Ok(a.cmp(b)), + (Self::String(a), Self::Path { value: b, .. }) => Ok(a.cmp(b)), + (Self::Path { value: a, .. }, Self::String(b)) => Ok(a.cmp(b)), _ if self.is_list() && other.is_list() => { let (a_iter, b_iter) = match (self.list_iter(), other.list_iter()) { (Some(a), Some(b)) => (a, b), @@ -1594,12 +1594,13 @@ pub fn format_float(f: f64) -> String { } let abs = f.abs(); if !(1e-4..1e16).contains(&abs) { - format!("{:e}", f) - .replace("e-0", "e-") - .replace("e0", "e+0") - .replace("e", "e+") - .replace("e+-", "e-") - .replace("e++", "e+") + let rendered = format!("{f:e}"); + if let Some((mantissa, exponent)) = rendered.split_once('e') { + if let Ok(exponent) = exponent.parse::() { + return format!("{mantissa}e{exponent:+03}"); + } + } + rendered } else if f.fract() == 0.0 { format!("{}.0", f as i64) } else { diff --git a/crates/openjd-expr/tests/integration/test_arithmetic.rs b/crates/openjd-expr/tests/integration/test_arithmetic.rs index af7f6811..9eb4ee52 100644 --- a/crates/openjd-expr/tests/integration/test_arithmetic.rs +++ b/crates/openjd-expr/tests/integration/test_arithmetic.rs @@ -92,6 +92,14 @@ fn float_precision_display() { assert_eq!(eval("0.1 + 0.2").to_display_string(), "0.30000000000000004"); } +#[test] +fn computed_float_display_pads_negative_exponents() { + assert_eq!(eval("1e-7 + 0.0").to_display_string(), "1e-07"); + assert_eq!(eval("1e-8 + 0.0").to_display_string(), "1e-08"); + assert_eq!(eval("-1e-7 + 0.0").to_display_string(), "-1e-07"); + assert_eq!(eval("1e16 + 0.0").to_display_string(), "1e+16"); +} + #[test] fn float_passthrough_preserves_original() { let mut st = SymbolTable::new(); @@ -155,6 +163,22 @@ fn floordiv_float_truncates_negative() { assert_eq!(eval("-7.5 // 2.0").to_display_string(), "-4"); } +#[test] +fn floordiv_float_accounts_for_rounded_quotients() { + assert_eq!(eval("205.0 // 0.1").to_display_string(), "2049"); + assert_eq!(eval("-205.0 // 0.1").to_display_string(), "-2050"); + assert_eq!(eval("205.0 // (-0.1)").to_display_string(), "-2050"); + assert_eq!(eval("-205.0 // (-0.1)").to_display_string(), "2049"); + assert_eq!( + eval("4.485645604145502e-49 // -4.340298580774782e-64").to_display_string(), + "-1033487793677267" + ); + assert_eq!( + eval("-1.609523844009722e-76 // -5.270510475043238e-92").to_display_string(), + "3053829134067924" + ); +} + #[test] fn floordiv_by_zero_int() { assert_err( @@ -849,6 +873,26 @@ fn mod_float_small_negative() { assert_eq!(eval("-0.5 % 1.0").to_display_string(), "0.5"); } +#[test] +fn mod_float_large_quotient_preserves_remainder() { + assert_eq!( + eval("9.2e18 % 0.1").to_display_string(), + "0.09740867242801976" + ); + assert_eq!( + eval("-9.2e18 % 0.1").to_display_string(), + "0.0025913275719802453" + ); + assert_eq!( + eval("9.2e18 % (-0.1)").to_display_string(), + "-0.0025913275719802453" + ); + assert_eq!( + eval("-9.2e18 % (-0.1)").to_display_string(), + "-0.09740867242801976" + ); +} + // Floored division/modulo invariant: a == (a // b) * b + (a % b) #[test] fn floordiv_mod_invariant_negative() { diff --git a/crates/openjd-expr/tests/integration/test_comparison.rs b/crates/openjd-expr/tests/integration/test_comparison.rs index 57d8ef43..c19d7876 100644 --- a/crates/openjd-expr/tests/integration/test_comparison.rs +++ b/crates/openjd-expr/tests/integration/test_comparison.rs @@ -27,6 +27,15 @@ fn eval_posix(expr: &str, st: &SymbolTable) -> ExprValue { .unwrap() } +fn eval_windows(expr: &str, st: &SymbolTable) -> ExprValue { + let parsed = ParsedExpression::new(expr).unwrap(); + let symtabs = [st]; + parsed + .with_path_format(PathFormat::Windows) + .evaluate(&symtabs) + .unwrap() +} + #[allow(dead_code)] fn eval_err(expr: &str) -> String { ParsedExpression::new(expr) @@ -319,6 +328,60 @@ fn string_lt_path() { assert_eq!(eval_posix("s < p", &st).to_display_string(), "true"); } +#[test] +fn mixed_path_string_ordering_preserves_operand_order_posix() { + let st = SymbolTable::new(); + for (expr, expected) in [ + ("path('/a') < '/z'", "true"), + ("path('/a') <= '/z'", "true"), + ("path('/a') > '/z'", "false"), + ("path('/a') >= '/z'", "false"), + ("'/a' < path('/z')", "true"), + ("'/a' <= path('/z')", "true"), + ("'/a' > path('/z')", "false"), + ("'/a' >= path('/z')", "false"), + ("[path('/a')] < ['/z']", "true"), + ("['/a'] < [path('/z')]", "true"), + ] { + assert_eq!( + eval_posix(expr, &st).to_display_string(), + expected, + "{expr}" + ); + } +} + +#[test] +fn mixed_path_string_ordering_preserves_operand_order_windows() { + let st = SymbolTable::new(); + for (expr, expected) in [ + (r"path(r'C:\a') < r'C:\z'", "true"), + (r"path(r'C:\a') <= r'C:\z'", "true"), + (r"path(r'C:\a') > r'C:\z'", "false"), + (r"path(r'C:\a') >= r'C:\z'", "false"), + (r"r'C:\a' < path(r'C:\z')", "true"), + (r"r'C:\a' <= path(r'C:\z')", "true"), + (r"r'C:\a' > path(r'C:\z')", "false"), + (r"r'C:\a' >= path(r'C:\z')", "false"), + (r"[path(r'C:\a')] < [r'C:\z']", "true"), + (r"[r'C:\a'] < [path(r'C:\z')]", "true"), + (r"path(r'C:/dir\z') > r'C:\dir\a'", "true"), + (r"path(r'C:\dir/z') > r'C:\dir\a'", "true"), + (r"r'C:\dir\a' < path(r'C:/dir\z')", "true"), + (r"r'C:\dir\a' < path(r'C:\dir/z')", "true"), + (r"[path(r'C:/dir\z')] > [r'C:\dir\a']", "true"), + (r"[path(r'C:\dir/z')] > [r'C:\dir\a']", "true"), + (r"[r'C:\dir\a'] < [path(r'C:/dir\z')]", "true"), + (r"[r'C:\dir\a'] < [path(r'C:\dir/z')]", "true"), + ] { + assert_eq!( + eval_windows(expr, &st).to_display_string(), + expected, + "{expr}" + ); + } +} + // === TestCrossTypeOrderingErrors === #[test] fn string_lt_int_errors() { diff --git a/crates/openjd-expr/tests/integration/test_paths.rs b/crates/openjd-expr/tests/integration/test_paths.rs index aee328a5..66ad45de 100644 --- a/crates/openjd-expr/tests/integration/test_paths.rs +++ b/crates/openjd-expr/tests/integration/test_paths.rs @@ -1402,7 +1402,7 @@ fn relative_to_component_boundary_error() { // path::join — unit tests for the format-aware join function // ══════════════════════════════════════════════════════════════ -use openjd_expr::functions::path::join as path_join; +use openjd_expr::functions::path::{join as path_join, non_uri_join}; // --- POSIX format --- @@ -1534,6 +1534,54 @@ fn join_uri_left_normalizes_backslashes_posix_format() { ); } +#[test] +fn join_uri_left_absolute_right_replaces() { + assert_eq!( + path_join("s3://bucket/prefix", "/foo", PathFormat::Posix), + "/foo" + ); + assert_eq!( + path_join("s3://bucket/prefix", "/foo", PathFormat::Uri), + "/foo" + ); + assert_eq!( + eval_posix("P / '/foo'", &posix_st("P", "s3://bucket/prefix")).to_display_string(), + "/foo" + ); +} + +#[test] +fn join_uri_left_root_relative_windows_preserves_authority() { + assert_eq!( + path_join("s3://bucket/prefix", "/foo", PathFormat::Windows), + "s3://bucket/foo" + ); + assert_eq!( + path_join("s3://bucket/prefix", "\\foo", PathFormat::Windows), + "s3://bucket/foo" + ); + assert_eq!( + eval_windows(r"P / r'\foo'", &windows_st("P", "s3://bucket/prefix")).to_display_string(), + "s3://bucket/foo" + ); +} + +#[test] +fn join_uri_left_drive_relative_windows_replaces_uri() { + assert_eq!( + path_join("s3://bucket/prefix", "C:", PathFormat::Windows), + "C:" + ); + assert_eq!( + path_join("s3://bucket/prefix", "C:foo", PathFormat::Windows), + "C:foo" + ); + assert_eq!( + eval_windows("P / 'C:'", &windows_st("P", "s3://bucket/prefix")).to_display_string(), + "C:" + ); +} + // --- Absolute right (URI) --- #[test] @@ -1583,6 +1631,22 @@ fn join_windows_backslash_root_relative() { ); } +#[test] +fn join_windows_root_relative_replaces_drive_less_left() { + assert_eq!(path_join("a\\b", "\\foo", PathFormat::Windows), "\\foo"); + assert_eq!(path_join("a\\b", "/foo", PathFormat::Windows), "/foo"); + assert_eq!(non_uri_join("a\\b", "\\foo", PathFormat::Windows), "\\foo"); + assert_eq!(non_uri_join("a\\b", "/foo", PathFormat::Windows), "/foo"); + assert_eq!( + non_uri_join("C:\\base", "\\foo", PathFormat::Windows), + "C:\\foo" + ); + assert_eq!( + eval_windows(r"P / r'\x'", &windows_st("P", r"a\b")).to_display_string(), + r"\x" + ); +} + #[test] fn join_windows_root_relative_unc_backslash() { // UNC root \\server\share + root-relative /foo → keeps UNC root diff --git a/crates/openjd-expr/tests/integration/test_strings.rs b/crates/openjd-expr/tests/integration/test_strings.rs index 07f89ba7..00b4a2bb 100644 --- a/crates/openjd-expr/tests/integration/test_strings.rs +++ b/crates/openjd-expr/tests/integration/test_strings.rs @@ -2559,6 +2559,13 @@ fn repr_sh_list_with_null_byte_returns_error() { ); } +#[test] +fn center_odd_padding_matches_python_bias() { + assert_eq!(eval("center('ab', 5)").to_display_string(), " ab "); + assert_eq!(eval("center('abc', 6)").to_display_string(), " abc "); + assert_eq!(eval("center('a', 4)").to_display_string(), " a "); +} + #[test] fn padding_width_counts_characters_but_budgets_bytes() { assert_eq!(eval("center('界', 4)").to_display_string(), " 界 "); diff --git a/crates/openjd-expr/tests/integration/test_uri_paths.rs b/crates/openjd-expr/tests/integration/test_uri_paths.rs index 787d456e..c592094d 100644 --- a/crates/openjd-expr/tests/integration/test_uri_paths.rs +++ b/crates/openjd-expr/tests/integration/test_uri_paths.rs @@ -432,7 +432,6 @@ fn uri_from_parts_bare() { } #[test] fn uri_join_absolute_replaces() { - // Joining an absolute path replaces the left side assert_eq!( eval_with("P / '/new/path'", &uri_st("P", "s3://bucket/old")).to_display_string(), "/new/path" @@ -878,11 +877,13 @@ fn join_multi_four_segments() { ); } -// TestUriPathOperators: join with path() object (Python test_join_absolute_replaces) +// TestUriPathOperators: absolute path objects replace the URI. #[test] fn join_absolute_path_object_replaces() { - let r = eval("path('s3://bucket/dir') / path('/local/path')"); - assert!(r.to_display_string().ends_with("/local/path")); + assert_eq!( + eval("path('s3://bucket/dir') / path('/local/path')").to_display_string(), + "/local/path" + ); } // TestUriPathOperators: join_string with sub/file.obj (Python exact) diff --git a/specs/expr/function-library.md b/specs/expr/function-library.md index c442a483..a79d1303 100644 --- a/specs/expr/function-library.md +++ b/specs/expr/function-library.md @@ -117,7 +117,9 @@ The following function families use this pattern: counting characters, clamps negative widths to zero (matching Python), then charges generated padding bytes and checks the exact output byte count. `zfill` borrows string and preserved-float input text until this preflight - succeeds, and calls the same crate-visible helper from `misc.rs`. + succeeds, and calls the same crate-visible helper from `misc.rs`. For `center`, + odd padding places the extra space on the left only when the requested width is + also odd, matching Python; otherwise the extra space is on the right. - **Representation functions** (`repr_py`, `repr_json`, `repr_sh`, `repr_cmd`, `repr_pwsh`) use `preflight_repr`. It first charges the recursive list item count from `count_list_items`, then obtains a byte bound from @@ -455,6 +457,11 @@ path operations, etc.) are defined in the sections 2.1 (Operators) and 2.2 (Built-in Functions). Key implementation choices: - **Integer arithmetic** uses Python-style floored division and modulo (§2.1.1) +- **Float modulo** starts from the truncating floating-point remainder and corrects + its sign to match the divisor. It does not reconstruct the remainder from a rounded + quotient, which would lose precision for large quotients. +- **Float floor division** derives its quotient from that remainder before rounding + 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 diff --git a/specs/expr/path-mapping.md b/specs/expr/path-mapping.md index 572b2f0c..1cc02566 100644 --- a/specs/expr/path-mapping.md +++ b/specs/expr/path-mapping.md @@ -221,6 +221,18 @@ Path-related operations in the expression language: | `/` | `(path, path) -> path` | Join paths | | `+` | `(path, string) -> path` | Append to last component | +On Windows filesystem paths, a root-relative right operand (for example, +`\renders`) replaces the left path below its root. A drive or UNC root from the +left operand is retained; if the left operand has no drive or UNC root, the right +operand replaces it entirely. The same absolute-right rule applies to URI left +operands, so `/renders` replaces the URI entirely under POSIX and URI formats. +Under Windows, a single leading `/` or `\` is root-relative rather than absolute; +for a URI left operand it replaces only the path below the authority. For example, +`path("s3://bucket/prefix") / "/renders"` produces `s3://bucket/renders` under +Windows and `/renders` under POSIX. A Windows drive-relative right operand replaces +a URI left operand entirely, so `path("s3://bucket/prefix") / "C:render.exr"` +produces `C:render.exr`. + ### apply_path_mapping `apply_path_mapping(path_string)` is a host-context-only function that applies diff --git a/specs/expr/public-api.md b/specs/expr/public-api.md index 3b000590..27bdbd7d 100644 --- a/specs/expr/public-api.md +++ b/specs/expr/public-api.md @@ -784,8 +784,8 @@ Accessible as `openjd_expr::value::ListIter` (not re-exported at the crate root) ```rust /// Format an `f64` in the canonical spec form used by /// `Float64::to_display_string`. Exponent notation outside the range -/// `[1e-4, 1e16)`; "0.0" for zero; preserves exact decimal where -/// possible. +/// `[1e-4, 1e16)`, with an explicit sign and at least two exponent digits; +/// "0.0" for zero; preserves exact decimal where possible. pub fn value::format_float(f: f64) -> String; ``` diff --git a/specs/expr/values.md b/specs/expr/values.md index 8c4e66f7..4e9ccb0a 100644 --- a/specs/expr/values.md +++ b/specs/expr/values.md @@ -303,6 +303,10 @@ tie-break. This keeps `<`/`<=`/`>`/`>=` consistent with equality — `i64::MAX < float(2**63)` is true, where a widening comparison would call them equal while `==` says they differ. +String↔Path ordering compares the path's string value while preserving operand order. +For example, `path("/a") < "/z"` is the same comparison as `"/a" < "/z"`. +This rule also applies recursively during lexicographic list ordering. + ### Tag-Based Hashing Strategy The `Hash` implementation must satisfy the contract that `a == b` implies