Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions crates/openjd-expr/src/functions/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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 {
Expand Down
55 changes: 34 additions & 21 deletions crates/openjd-expr/src/functions/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:".
Expand All @@ -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();
Comment thread
mwiebe marked this conversation as resolved.
}
}
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}")
}

Expand All @@ -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'\\') {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/openjd-expr/src/functions/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 9 additions & 8 deletions crates/openjd-expr/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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::<i32>() {
return format!("{mantissa}e{exponent:+03}");
}
}
rendered
} else if f.fract() == 0.0 {
format!("{}.0", f as i64)
} else {
Expand Down
44 changes: 44 additions & 0 deletions crates/openjd-expr/tests/integration/test_arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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() {
Expand Down
63 changes: 63 additions & 0 deletions crates/openjd-expr/tests/integration/test_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down
Loading
Loading