From 6e56d014a277f5628dbbe0125f0d9761f1ac13f8 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 6 Oct 2025 00:12:58 +0100 Subject: [PATCH 1/9] Add template time helpers --- Cargo.lock | 1 + Cargo.toml | 1 + docs/netsuke-design.md | 16 ++ docs/roadmap.md | 2 +- src/stdlib/mod.rs | 2 + src/stdlib/time/mod.rs | 360 +++++++++++++++++++++++++++++ src/stdlib/time/tests.rs | 133 +++++++++++ tests/features/stdlib_time.feature | 38 +++ tests/steps/stdlib_steps.rs | 86 ++++++- 9 files changed, 635 insertions(+), 4 deletions(-) create mode 100644 src/stdlib/time/mod.rs create mode 100644 src/stdlib/time/tests.rs create mode 100644 tests/features/stdlib_time.feature diff --git a/Cargo.lock b/Cargo.lock index 3754aa2a..79135282 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -389,6 +389,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d630bccd429a5bb5a64b5e94f693bfc48c9f8566418fda4c494cc94f911f87cc" dependencies = [ "powerfmt", + "serde", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4de313ef..000326b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ tempfile = "3.8.0" ninja_env = { path = "ninja_env" } shell-quote = { version = "0.7.2", default-features = false, features = ["sh"] } shlex = "1.3.0" +time = { version = "0.3.42", features = ["formatting", "macros", "parsing", "serde"] } [build-dependencies] clap = { version = "4.5.0", features = ["derive"] } diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index f540fbdf..ffc2f861 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -932,6 +932,22 @@ be marked `pure` if safe for caching or `impure` otherwise. | `now()` | function | Current `datetime` (UTC by default) | | `timedelta(**kwargs)` | function | Convenience creator for `age` comparisons | +The `now()` helper produces an object that renders as an ISO 8601 +timestamp and exposes `iso8601`, `unix_timestamp`, and `offset` accessors so +templates can serialise or compare values without string parsing. It defaults +to UTC but accepts an `offset="+HH:MM"` keyword argument that re-bases the +captured time on another fixed offset. Time is captured lazily when the helper +executes so behaviour remains deterministic during a render. + +`timedelta(**kwargs)` constructs a duration object that renders using the +ISO 8601 duration grammar (for example, `P1DT2H30M5.75025S`). The helper +accepts integer keyword arguments `weeks`, `days`, `hours`, `minutes`, +`seconds`, `milliseconds`, `microseconds`, and `nanoseconds`, allowing callers +to describe durations at nanosecond precision. Arguments may be negative, but +overflow or non-integer inputs raise `InvalidOperation` errors so templates +cannot silently wrap. The resulting object exposes `.iso8601`, `.seconds`, and +`.nanoseconds` attributes for downstream predicates. + ##### Example usage ```jinja diff --git a/docs/roadmap.md b/docs/roadmap.md index 0a46a264..908201f6 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -168,7 +168,7 @@ library, and CLI ergonomics. - [ ] Implement the network and command functions/filters (fetch, shell, grep), ensuring shell marks templates as impure to disable caching. - - [ ] Implement the time helpers (`now`, `timedelta`). + - [x] Implement the time helpers (`now`, `timedelta`). - [ ] **CLI and Feature Completeness:** diff --git a/src/stdlib/mod.rs b/src/stdlib/mod.rs index caff31d9..9051c2f6 100644 --- a/src/stdlib/mod.rs +++ b/src/stdlib/mod.rs @@ -10,6 +10,7 @@ mod collections; mod path; +mod time; use camino::Utf8Path; use cap_std::fs; @@ -39,6 +40,7 @@ pub fn register(env: &mut Environment<'_>) { register_file_tests(env); path::register_filters(env); collections::register_filters(env); + time::register_functions(env); } fn register_file_tests(env: &mut Environment<'_>) { diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs new file mode 100644 index 00000000..01b5e481 --- /dev/null +++ b/src/stdlib/time/mod.rs @@ -0,0 +1,360 @@ +//! Time helpers for the `MiniJinja` standard library. +//! +//! The helpers expose UTC timestamps and duration arithmetic in a deterministic +//! manner so templates can reason about file ages without shelling out. Values +//! round-trip through `MiniJinja` as lightweight objects so other predicates can +//! downcast them later without reparsing strings. + +use std::{fmt, sync::Arc}; + +use minijinja::{ + Environment, Error, ErrorKind, + value::{Kwargs, Object, ObjectRepr, Value}, +}; +use time::{Duration, OffsetDateTime, UtcOffset, format_description::well_known::Iso8601}; + +const SECONDS_PER_MINUTE: i64 = 60; +const SECONDS_PER_HOUR: i64 = 60 * SECONDS_PER_MINUTE; +const SECONDS_PER_DAY: i64 = 24 * SECONDS_PER_HOUR; +const SECONDS_PER_WEEK: i64 = 7 * SECONDS_PER_DAY; +const NANOS_PER_MICROSECOND: i64 = 1_000; +const NANOS_PER_MILLISECOND: i64 = 1_000 * NANOS_PER_MICROSECOND; +const SECONDS_PER_MINUTE_I32: i32 = 60; +const SECONDS_PER_HOUR_I32: i32 = 3_600; + +/// Register time helpers with the environment. +pub(crate) fn register_functions(env: &mut Environment<'_>) { + env.add_function("now", |kwargs: Kwargs| now(&kwargs)); + env.add_function("timedelta", |kwargs: Kwargs| timedelta(&kwargs)); +} + +fn now(kwargs: &Kwargs) -> Result { + let offset_spec: Option = kwargs.get("offset")?; + kwargs.assert_all_used()?; + + let mut timestamp = OffsetDateTime::now_utc(); + if let Some(raw) = offset_spec { + let parsed = parse_offset(&raw)?; + timestamp = timestamp.to_offset(parsed); + } + + Ok(Value::from_object(TimestampValue::new(timestamp))) +} + +fn timedelta(kwargs: &Kwargs) -> Result { + let weeks: Option = kwargs.get("weeks")?; + let days: Option = kwargs.get("days")?; + let hours: Option = kwargs.get("hours")?; + let minutes: Option = kwargs.get("minutes")?; + let seconds: Option = kwargs.get("seconds")?; + let milliseconds: Option = kwargs.get("milliseconds")?; + let microseconds: Option = kwargs.get("microseconds")?; + let nanoseconds: Option = kwargs.get("nanoseconds")?; + kwargs.assert_all_used()?; + + let mut total = Duration::ZERO; + total = add_seconds_component(total, weeks, SECONDS_PER_WEEK, "weeks")?; + total = add_seconds_component(total, days, SECONDS_PER_DAY, "days")?; + total = add_seconds_component(total, hours, SECONDS_PER_HOUR, "hours")?; + total = add_seconds_component(total, minutes, SECONDS_PER_MINUTE, "minutes")?; + total = add_seconds_component(total, seconds, 1, "seconds")?; + total = add_nanoseconds_component(total, milliseconds, NANOS_PER_MILLISECOND, "milliseconds")?; + total = add_nanoseconds_component(total, microseconds, NANOS_PER_MICROSECOND, "microseconds")?; + total = add_nanoseconds_component(total, nanoseconds, 1, "nanoseconds")?; + + Ok(Value::from_object(TimeDeltaValue::new(total))) +} + +fn parse_offset(raw: &str) -> Result { + let trimmed = raw.trim(); + if trimmed.eq_ignore_ascii_case("z") { + return Ok(UtcOffset::UTC); + } + + let (sign, rest) = if let Some(remaining) = trimmed.strip_prefix('+') { + (1_i64, remaining) + } else if let Some(remaining) = trimmed.strip_prefix('-') { + (-1_i64, remaining) + } else { + return Err(invalid_offset(raw)); + }; + + let (hours_part, remaining) = rest.split_once(':').ok_or_else(|| invalid_offset(raw))?; + if hours_part.contains(':') { + return Err(invalid_offset(raw)); + } + + let (minutes_part, seconds_part) = match remaining.split_once(':') { + Some((mins, secs)) if !secs.contains(':') => (mins, Some(secs)), + Some(_) => return Err(invalid_offset(raw)), + None => (remaining, None), + }; + + if minutes_part.contains(':') { + return Err(invalid_offset(raw)); + } + + let hours = parse_component(hours_part, raw)?; + let minutes = parse_component(minutes_part, raw)?; + let seconds = seconds_part + .map(|value| parse_component(value, raw)) + .transpose()?; + + if !(0..=23).contains(&hours) || !(0..=59).contains(&minutes) { + return Err(invalid_offset(raw)); + } + + let seconds_value = seconds.unwrap_or_default(); + if !(0..=59).contains(&seconds_value) { + return Err(invalid_offset(raw)); + } + + let total_seconds = sign + * (i64::from(hours) * SECONDS_PER_HOUR + + i64::from(minutes) * SECONDS_PER_MINUTE + + i64::from(seconds_value)); + + let total_seconds = i32::try_from(total_seconds).map_err(|_| invalid_offset(raw))?; + + UtcOffset::from_whole_seconds(total_seconds).map_err(|err| { + Error::new( + ErrorKind::InvalidOperation, + format!("now offset '{raw}' is invalid: {err}"), + ) + }) +} + +fn parse_component(component: &str, original: &str) -> Result { + component + .trim() + .parse::() + .map_err(|_| invalid_offset(original)) +} + +fn invalid_offset(raw: &str) -> Error { + Error::new( + ErrorKind::InvalidOperation, + format!("now offset '{raw}' is invalid: expected '+HH:MM' or 'Z'"), + ) +} + +fn add_seconds_component( + mut total: Duration, + amount: Option, + multiplier: i64, + label: &str, +) -> Result { + if let Some(value) = amount { + let seconds = value + .checked_mul(multiplier) + .ok_or_else(|| overflow_error(label))?; + let component = Duration::seconds(seconds); + total = total + .checked_add(component) + .ok_or_else(|| overflow_error(label))?; + } + Ok(total) +} + +fn add_nanoseconds_component( + mut total: Duration, + amount: Option, + multiplier: i64, + label: &str, +) -> Result { + if let Some(value) = amount { + let nanos = value + .checked_mul(multiplier) + .ok_or_else(|| overflow_error(label))?; + let component = Duration::nanoseconds(nanos); + total = total + .checked_add(component) + .ok_or_else(|| overflow_error(label))?; + } + Ok(total) +} + +fn overflow_error(label: &str) -> Error { + Error::new( + ErrorKind::InvalidOperation, + format!("timedelta overflow when adding {label}"), + ) +} + +fn format_offset_datetime(datetime: OffsetDateTime) -> String { + datetime.format(&Iso8601::DEFAULT).map_or_else( + |_| datetime.to_string(), + |mut formatted| { + if let Some(pos) = formatted.find(".000000000") + && formatted + .get(pos + 10..) + .and_then(|rest| rest.chars().next()) + .is_some_and(|next| matches!(next, 'Z' | '+' | '-')) + { + formatted.replace_range(pos..pos + 10, ""); + } + formatted + }, + ) +} + +fn format_utc_offset(offset: UtcOffset) -> String { + let total_seconds = offset.whole_seconds(); + let sign = if total_seconds >= 0 { '+' } else { '-' }; + let abs_seconds = total_seconds.abs(); + let hours = abs_seconds.div_euclid(SECONDS_PER_HOUR_I32); + let remainder = abs_seconds.rem_euclid(SECONDS_PER_HOUR_I32); + let minutes = remainder.div_euclid(SECONDS_PER_MINUTE_I32); + let seconds = remainder.rem_euclid(SECONDS_PER_MINUTE_I32); + + if seconds == 0 { + format!("{sign}{hours:02}:{minutes:02}") + } else { + format!("{sign}{hours:02}:{minutes:02}:{seconds:02}") + } +} + +fn format_duration_iso8601(duration: Duration) -> String { + if duration.is_zero() { + return "PT0S".to_owned(); + } + + let mut buffer = String::new(); + if duration.is_negative() { + buffer.push('-'); + } + buffer.push('P'); + + let absolute = duration.abs(); + let days = absolute.whole_days(); + let mut remainder = absolute - Duration::days(days); + + if days != 0 { + buffer.push_str(&days.to_string()); + buffer.push('D'); + } + + let mut time_section = String::new(); + + let hours = remainder.whole_hours(); + if hours != 0 { + time_section.push_str(&hours.to_string()); + time_section.push('H'); + remainder -= Duration::hours(hours); + } + + let minutes = remainder.whole_minutes(); + if minutes != 0 { + time_section.push_str(&minutes.to_string()); + time_section.push('M'); + remainder -= Duration::minutes(minutes); + } + + let seconds = remainder.whole_seconds(); + let nanos = remainder.subsec_nanoseconds(); + if seconds != 0 || nanos != 0 { + time_section.push_str(&format_seconds_with_fraction(seconds, nanos)); + } + + if time_section.is_empty() { + if buffer.ends_with('P') { + buffer.push_str("T0S"); + } + } else { + buffer.push('T'); + buffer.push_str(&time_section); + } + + buffer +} + +fn format_seconds_with_fraction(seconds: i64, nanos: i32) -> String { + let seconds = u64::try_from(seconds).expect("seconds must be non-negative"); + if nanos == 0 { + return format!("{seconds}S"); + } + + let mut fraction = format!("{nanos:09}"); + while fraction.ends_with('0') { + fraction.pop(); + } + + format!("{seconds}.{fraction}S") +} + +#[derive(Clone, Copy)] +struct TimestampValue { + datetime: OffsetDateTime, +} + +impl TimestampValue { + fn new(datetime: OffsetDateTime) -> Self { + Self { datetime } + } + + fn iso8601(&self) -> String { + format_offset_datetime(self.datetime) + } +} + +impl fmt::Debug for TimestampValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.iso8601()) + } +} + +impl Object for TimestampValue { + fn repr(self: &Arc) -> ObjectRepr { + ObjectRepr::Plain + } + + fn get_value(self: &Arc, key: &Value) -> Option { + let attr = key.as_str()?; + match attr { + "iso8601" => Some(Value::from(self.iso8601())), + "unix_timestamp" => Some(Value::from(self.datetime.unix_timestamp())), + "offset" => Some(Value::from(format_utc_offset(self.datetime.offset()))), + _ => None, + } + } +} + +#[derive(Clone, Copy)] +struct TimeDeltaValue { + duration: Duration, +} + +impl TimeDeltaValue { + fn new(duration: Duration) -> Self { + Self { duration } + } + + fn iso8601(&self) -> String { + format_duration_iso8601(self.duration) + } +} + +impl fmt::Debug for TimeDeltaValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.iso8601()) + } +} + +impl Object for TimeDeltaValue { + fn repr(self: &Arc) -> ObjectRepr { + ObjectRepr::Plain + } + + fn get_value(self: &Arc, key: &Value) -> Option { + let attr = key.as_str()?; + match attr { + "iso8601" => Some(Value::from(self.iso8601())), + "seconds" => Some(Value::from(self.duration.whole_seconds())), + "nanoseconds" => Some(Value::from(i64::from(self.duration.subsec_nanoseconds()))), + _ => None, + } + } +} + +#[cfg(test)] +mod tests; diff --git a/src/stdlib/time/tests.rs b/src/stdlib/time/tests.rs new file mode 100644 index 00000000..4155a91d --- /dev/null +++ b/src/stdlib/time/tests.rs @@ -0,0 +1,133 @@ +use super::*; +use minijinja::{Environment, context, value::Value}; +use rstest::rstest; +use time::{Duration, OffsetDateTime, macros::datetime}; + +fn eval_expression(env: &Environment<'_>, expr: &str) -> Value { + env.compile_expression(expr) + .expect("compile expression") + .eval(context! {}) + .expect("evaluate expression") +} + +fn build_env() -> Environment<'static> { + let mut env = Environment::new(); + register_functions(&mut env); + env +} + +fn value_as_timestamp(value: &Value) -> OffsetDateTime { + value + .as_object() + .and_then(|obj| obj.downcast_ref::()) + .map(|stored| stored.datetime) + .expect("timestamp object") +} + +fn value_as_duration(value: &Value) -> Duration { + value + .as_object() + .and_then(|obj| obj.downcast_ref::()) + .map(|stored| stored.duration) + .expect("duration object") +} + +#[rstest] +fn now_defaults_to_utc() { + let env = build_env(); + let value = eval_expression(&env, "now()"); + let captured = value_as_timestamp(&value); + let now = OffsetDateTime::now_utc(); + let delta = (now - captured).abs(); + assert!(delta <= Duration::seconds(2)); + assert_eq!(captured.offset(), UtcOffset::UTC); +} + +#[rstest] +fn now_applies_custom_offset() { + let env = build_env(); + let value = eval_expression(&env, "now(offset='+02:30')"); + let captured = value_as_timestamp(&value); + let offset = UtcOffset::from_hms(2, 30, 0).expect("offset"); + assert_eq!(captured.offset(), offset); +} + +#[rstest] +fn now_rejects_invalid_offset() { + let env = build_env(); + let err = env + .compile_expression("now(offset='bogus')") + .expect("compile expression") + .eval(context! {}); + let err = err.expect_err("invalid offset should error"); + assert_eq!(err.kind(), ErrorKind::InvalidOperation); +} + +#[rstest] +fn timedelta_defaults_to_zero() { + let env = build_env(); + let value = eval_expression(&env, "timedelta()"); + let duration = value_as_duration(&value); + assert!(duration.is_zero()); +} + +#[rstest] +fn timedelta_accumulates_components() { + let env = build_env(); + let value = eval_expression( + &env, + "timedelta(days=1, hours=2, minutes=30, seconds=5, milliseconds=750, microseconds=250, nanoseconds=1)", + ); + let duration = value_as_duration(&value); + let expected = Duration::seconds(SECONDS_PER_DAY) + + Duration::seconds(SECONDS_PER_HOUR * 2) + + Duration::seconds(SECONDS_PER_MINUTE * 30) + + Duration::seconds(5) + + Duration::nanoseconds(750 * NANOS_PER_MILLISECOND) + + Duration::nanoseconds(250 * NANOS_PER_MICROSECOND) + + Duration::nanoseconds(1); + assert_eq!(duration, expected); +} + +#[rstest] +fn timedelta_supports_negative_values() { + let env = build_env(); + let value = eval_expression(&env, "timedelta(hours=-1, seconds=-30)"); + let duration = value_as_duration(&value); + assert_eq!(duration, Duration::seconds(-SECONDS_PER_HOUR - 30)); +} + +#[rstest] +fn timedelta_detects_overflow() { + let env = build_env(); + let err = env + .compile_expression("timedelta(days=9223372036854775807)") + .expect("compile expression") + .eval(context! {}); + let err = err.expect_err("overflow should error"); + assert_eq!(err.kind(), ErrorKind::InvalidOperation); +} + +#[rstest] +fn timestamp_iso8601_property() { + let reference = datetime!(2024-05-21 10:30:00 +00:00); + let value = Value::from_object(TimestampValue::new(reference)); + let object = value.as_object().expect("object"); + let iso = object + .get_value(&Value::from("iso8601")) + .expect("iso8601 attr") + .to_string(); + assert_eq!(iso, "2024-05-21T10:30:00Z"); +} + +#[rstest] +fn timedelta_iso8601_property() { + let duration = Duration::seconds(SECONDS_PER_DAY + 30) + Duration::nanoseconds(500_000_000); + let value = Value::from_object(TimeDeltaValue::new(duration)); + let object = value.as_object().expect("object"); + let iso = object + .get_value(&Value::from("iso8601")) + .expect("iso8601 attr") + .to_string(); + assert_eq!(iso, "P1DT30.5S"); +} diff --git a/tests/features/stdlib_time.feature b/tests/features/stdlib_time.feature new file mode 100644 index 00000000..a3420685 --- /dev/null +++ b/tests/features/stdlib_time.feature @@ -0,0 +1,38 @@ +Feature: Template time helpers + Netsuke exposes deterministic time primitives so manifests can reason about + file ages without shelling out to external tools. + + Scenario: Rendering now() yields a UTC timestamp + Given a stdlib workspace + When I render the stdlib template "{{ now() }}" + Then the stdlib output is an ISO8601 UTC timestamp + + Scenario: Rendering now() with an offset preserves the offset + Given a stdlib workspace + When I render the stdlib template "{{ now(offset='+02:00').iso8601 }}" + Then the stdlib output offset is "+02:00" + + Scenario: Timedelta composes multiple components + Given a stdlib workspace + When I render the stdlib template "{{ timedelta(days=1, hours=2, minutes=30, seconds=5, milliseconds=750, microseconds=250).iso8601 }}" + Then the stdlib output is "P1DT2H30M5.75025S" + + Scenario: Timedelta captures nanosecond precision + Given a stdlib workspace + When I render the stdlib template "{{ timedelta(nanoseconds=1).iso8601 }}" + Then the stdlib output is "PT0.000000001S" + + Scenario: Timedelta supports negative values + Given a stdlib workspace + When I render the stdlib template "{{ timedelta(hours=-1).iso8601 }}" + Then the stdlib output is "-PT1H" + + Scenario: Timedelta overflow surfaces an error + Given a stdlib workspace + When I render the stdlib template "{{ timedelta(days=9223372036854775807) }}" + Then the stdlib error contains "overflow" + + Scenario: now() rejects invalid offsets + Given a stdlib workspace + When I render the stdlib template "{{ now(offset='bogus') }}" + Then the stdlib error contains "invalid" diff --git a/tests/steps/stdlib_steps.rs b/tests/steps/stdlib_steps.rs index 73bd81ed..2ab196cf 100644 --- a/tests/steps/stdlib_steps.rs +++ b/tests/steps/stdlib_steps.rs @@ -6,10 +6,11 @@ use crate::CliWorld; use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ambient_authority, fs_utf8::Dir}; use cucumber::{given, then, when}; -use minijinja::{Environment, context}; +use minijinja::{Environment, context, value::Value}; use netsuke::stdlib; use std::ffi::OsStr; use test_support::env::set_var; +use time::{Duration, OffsetDateTime, UtcOffset, format_description::well_known::Iso8601}; const LINES_FIXTURE: &str = concat!( "one @@ -108,10 +109,10 @@ fn ensure_workspace(world: &mut CliWorld) -> Utf8PathBuf { root } -fn render_template(world: &mut CliWorld, template: &TemplateContent, path: &TemplatePath) { +fn render_template_with_context(world: &mut CliWorld, template: &TemplateContent, ctx: Value) { let mut env = Environment::new(); stdlib::register(&mut env); - let render = env.render_str(template.as_str(), context!(path => path.as_path().as_str())); + let render = env.render_str(template.as_str(), ctx); match render { Ok(output) => { world.stdlib_output = Some(output); @@ -124,6 +125,11 @@ fn render_template(world: &mut CliWorld, template: &TemplateContent, path: &Temp } } +fn render_template(world: &mut CliWorld, template: &TemplateContent, path: &TemplatePath) { + let ctx = context!(path => path.as_path().as_str()); + render_template_with_context(world, template, ctx); +} + #[given("a stdlib workspace")] fn stdlib_workspace(world: &mut CliWorld) { let root = ensure_workspace(world); @@ -188,6 +194,12 @@ fn render_stdlib_template(world: &mut CliWorld, template: String, path: String) render_template(world, &template_content, &target); } +#[when(regex = r#"^I render the stdlib template "(.+)"$"#)] +fn render_stdlib_template_without_path(world: &mut CliWorld, template: String) { + let template_content = TemplateContent::from(template); + render_template_with_context(world, &template_content, context! {}); +} + #[expect( clippy::needless_pass_by_value, reason = "Cucumber requires owned capture arguments" @@ -214,6 +226,49 @@ fn stdlib_root_and_output(world: &CliWorld) -> (&Utf8Path, &str) { (root, output) } +fn stdlib_output(world: &CliWorld) -> &str { + world + .stdlib_output + .as_deref() + .expect("expected stdlib output") +} + +fn parse_iso_timestamp(raw: &str) -> OffsetDateTime { + OffsetDateTime::parse(raw, &Iso8601::DEFAULT).expect("valid ISO8601 timestamp") +} + +fn parse_expected_offset(raw: &str) -> UtcOffset { + if raw.eq_ignore_ascii_case("z") { + return UtcOffset::UTC; + } + + let mut chars = raw.chars(); + let first = chars + .next() + .unwrap_or_else(|| panic!("unsupported offset format: {raw}")); + let rest = chars.as_str(); + let (sign, rest) = match first { + '+' => (1, rest), + '-' => (-1, rest), + _ => panic!("unsupported offset format: {raw}"), + }; + + let mut parts = rest.split(':'); + let hours: i8 = parts + .next() + .expect("hour component") + .parse() + .expect("valid hour"); + let minutes: i8 = parts + .next() + .map_or(0, |value| value.parse().expect("valid minute")); + let seconds: i8 = parts + .next() + .map_or(0, |value| value.parse().expect("valid second")); + + UtcOffset::from_hms(sign * hours, sign * minutes, sign * seconds).expect("offset within range") +} + #[expect( clippy::needless_pass_by_value, reason = "Cucumber requires owned capture arguments" @@ -240,3 +295,28 @@ fn assert_stdlib_output_is_workspace_path(world: &mut CliWorld, relative: String let expected = root.join(relative_path.to_path_buf()); assert_eq!(output, expected.as_str()); } + +#[then("the stdlib output is an ISO8601 UTC timestamp")] +fn assert_stdlib_output_is_utc_timestamp(world: &mut CliWorld) { + let output = stdlib_output(world); + let parsed = parse_iso_timestamp(output); + let now = OffsetDateTime::now_utc(); + let delta = (now - parsed).abs(); + assert!( + delta <= Duration::seconds(5), + "timestamp `{output}` should be within five seconds of now", + ); + assert_eq!(parsed.offset(), UtcOffset::UTC); +} + +#[expect( + clippy::needless_pass_by_value, + reason = "Cucumber requires owned capture arguments" +)] +#[then(regex = r#"^the stdlib output offset is "(.+)"$"#)] +fn assert_stdlib_output_offset(world: &mut CliWorld, expected: String) { + let output = stdlib_output(world); + let parsed = parse_iso_timestamp(output); + let expected_offset = parse_expected_offset(&expected); + assert_eq!(parsed.offset(), expected_offset); +} From 756cfd1649fc2fa32c2ca73d7304555aec690114 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 6 Oct 2025 07:45:16 +0100 Subject: [PATCH 2/9] Refactor ISO8601 duration formatting --- src/stdlib/time/mod.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index 01b5e481..1adeea13 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -227,13 +227,18 @@ fn format_duration_iso8601(duration: Duration) -> String { let absolute = duration.abs(); let days = absolute.whole_days(); - let mut remainder = absolute - Duration::days(days); + let remainder = absolute - Duration::days(days); if days != 0 { buffer.push_str(&days.to_string()); buffer.push('D'); } + let time_section = format_time_components(remainder); + finalize_duration_buffer(buffer, &time_section) +} + +fn format_time_components(mut remainder: Duration) -> String { let mut time_section = String::new(); let hours = remainder.whole_hours(); @@ -256,13 +261,17 @@ fn format_duration_iso8601(duration: Duration) -> String { time_section.push_str(&format_seconds_with_fraction(seconds, nanos)); } + time_section +} + +fn finalize_duration_buffer(mut buffer: String, time_section: &str) -> String { if time_section.is_empty() { if buffer.ends_with('P') { buffer.push_str("T0S"); } } else { buffer.push('T'); - buffer.push_str(&time_section); + buffer.push_str(time_section); } buffer From f7a91aa86ccab8be5b2afb9e4bfd0c05448a0bf5 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 6 Oct 2025 07:45:22 +0100 Subject: [PATCH 3/9] Refactor offset parsing helpers --- src/stdlib/time/mod.rs | 58 +++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index 1adeea13..18a2aff5 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -71,14 +71,33 @@ fn parse_offset(raw: &str) -> Result { return Ok(UtcOffset::UTC); } - let (sign, rest) = if let Some(remaining) = trimmed.strip_prefix('+') { - (1_i64, remaining) - } else if let Some(remaining) = trimmed.strip_prefix('-') { - (-1_i64, remaining) - } else { - return Err(invalid_offset(raw)); - }; + let (sign, rest) = extract_sign(trimmed, raw)?; + let (hours, minutes, seconds) = parse_offset_components(rest, raw)?; + validate_offset_ranges(hours, minutes, seconds, raw)?; + + let total_seconds = calculate_total_seconds(sign, hours, minutes, seconds); + let total_seconds = i32::try_from(total_seconds).map_err(|_| invalid_offset(raw))?; + + UtcOffset::from_whole_seconds(total_seconds).map_err(|err| { + Error::new( + ErrorKind::InvalidOperation, + format!("now offset '{raw}' is invalid: {err}"), + ) + }) +} + +fn extract_sign<'a>(trimmed: &'a str, raw: &str) -> Result<(i64, &'a str), Error> { + if let Some(remaining) = trimmed.strip_prefix('+') { + return Ok((1_i64, remaining)); + } + + trimmed + .strip_prefix('-') + .map(|remaining| (-1_i64, remaining)) + .ok_or_else(|| invalid_offset(raw)) +} +fn parse_offset_components(rest: &str, raw: &str) -> Result<(i32, i32, i32), Error> { let (hours_part, remaining) = rest.split_once(':').ok_or_else(|| invalid_offset(raw))?; if hours_part.contains(':') { return Err(invalid_offset(raw)); @@ -100,28 +119,25 @@ fn parse_offset(raw: &str) -> Result { .map(|value| parse_component(value, raw)) .transpose()?; + Ok((hours, minutes, seconds.unwrap_or_default())) +} + +fn validate_offset_ranges(hours: i32, minutes: i32, seconds: i32, raw: &str) -> Result<(), Error> { if !(0..=23).contains(&hours) || !(0..=59).contains(&minutes) { return Err(invalid_offset(raw)); } - let seconds_value = seconds.unwrap_or_default(); - if !(0..=59).contains(&seconds_value) { + if !(0..=59).contains(&seconds) { return Err(invalid_offset(raw)); } - let total_seconds = sign - * (i64::from(hours) * SECONDS_PER_HOUR - + i64::from(minutes) * SECONDS_PER_MINUTE - + i64::from(seconds_value)); - - let total_seconds = i32::try_from(total_seconds).map_err(|_| invalid_offset(raw))?; + Ok(()) +} - UtcOffset::from_whole_seconds(total_seconds).map_err(|err| { - Error::new( - ErrorKind::InvalidOperation, - format!("now offset '{raw}' is invalid: {err}"), - ) - }) +fn calculate_total_seconds(sign: i64, hours: i32, minutes: i32, seconds: i32) -> i64 { + sign * (i64::from(hours) * SECONDS_PER_HOUR + + i64::from(minutes) * SECONDS_PER_MINUTE + + i64::from(seconds)) } fn parse_component(component: &str, original: &str) -> Result { From 5f94b2d884f4a36057ed0a22d05582f162d32711 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 6 Oct 2025 12:49:24 +0100 Subject: [PATCH 4/9] Deduplicate ISO8601 property tests --- src/stdlib/time/tests.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/stdlib/time/tests.rs b/src/stdlib/time/tests.rs index 4155a91d..02a1ba50 100644 --- a/src/stdlib/time/tests.rs +++ b/src/stdlib/time/tests.rs @@ -32,6 +32,15 @@ fn value_as_duration(value: &Value) -> Duration { .expect("duration object") } +fn get_iso8601_property(value: &Value) -> String { + value + .as_object() + .expect("object") + .get_value(&Value::from("iso8601")) + .expect("iso8601 attr") + .to_string() +} + #[rstest] fn now_defaults_to_utc() { let env = build_env(); @@ -112,11 +121,7 @@ fn timedelta_detects_overflow() { fn timestamp_iso8601_property() { let reference = datetime!(2024-05-21 10:30:00 +00:00); let value = Value::from_object(TimestampValue::new(reference)); - let object = value.as_object().expect("object"); - let iso = object - .get_value(&Value::from("iso8601")) - .expect("iso8601 attr") - .to_string(); + let iso = get_iso8601_property(&value); assert_eq!(iso, "2024-05-21T10:30:00Z"); } @@ -124,10 +129,6 @@ fn timestamp_iso8601_property() { fn timedelta_iso8601_property() { let duration = Duration::seconds(SECONDS_PER_DAY + 30) + Duration::nanoseconds(500_000_000); let value = Value::from_object(TimeDeltaValue::new(duration)); - let object = value.as_object().expect("object"); - let iso = object - .get_value(&Value::from("iso8601")) - .expect("iso8601 attr") - .to_string(); + let iso = get_iso8601_property(&value); assert_eq!(iso, "P1DT30.5S"); } From a595a8a269a238518eb71eb645312bd2fa4a034b Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 6 Oct 2025 18:06:18 +0100 Subject: [PATCH 5/9] Streamline time offset parsing and tests - rely on time's format_description parser for offsets and share a generic component accumulator to remove timedelta duplication\n- broaden time helper coverage with invalid offset cases, negative components, and ISO-8601 assertions for offsets, fractions, and zero/negative durations\n- align documentation spelling with en-oxendic guidance --- docs/netsuke-design.md | 2 +- src/stdlib/time/mod.rs | 146 +++++++++++---------------------------- src/stdlib/time/tests.rs | 70 ++++++++++++++++--- 3 files changed, 101 insertions(+), 117 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index ffc2f861..090ac912 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -934,7 +934,7 @@ be marked `pure` if safe for caching or `impure` otherwise. The `now()` helper produces an object that renders as an ISO 8601 timestamp and exposes `iso8601`, `unix_timestamp`, and `offset` accessors so -templates can serialise or compare values without string parsing. It defaults +templates can serialize or compare values without string parsing. It defaults to UTC but accepts an `offset="+HH:MM"` keyword argument that re-bases the captured time on another fixed offset. Time is captured lazily when the helper executes so behaviour remains deterministic during a render. diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index 18a2aff5..13f823bd 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -11,7 +11,11 @@ use minijinja::{ Environment, Error, ErrorKind, value::{Kwargs, Object, ObjectRepr, Value}, }; -use time::{Duration, OffsetDateTime, UtcOffset, format_description::well_known::Iso8601}; +use time::{ + Duration, OffsetDateTime, UtcOffset, + format_description::{FormatItem, well_known::Iso8601}, + macros::format_description, +}; const SECONDS_PER_MINUTE: i64 = 60; const SECONDS_PER_HOUR: i64 = 60 * SECONDS_PER_MINUTE; @@ -22,6 +26,10 @@ const NANOS_PER_MILLISECOND: i64 = 1_000 * NANOS_PER_MICROSECOND; const SECONDS_PER_MINUTE_I32: i32 = 60; const SECONDS_PER_HOUR_I32: i32 = 3_600; +const OFFSET_FORMAT: &[FormatItem<'static>] = format_description!( + "[offset_hour sign:mandatory]:[offset_minute][optional [:[offset_second]]]" +); + /// Register time helpers with the environment. pub(crate) fn register_functions(env: &mut Environment<'_>) { env.add_function("now", |kwargs: Kwargs| now(&kwargs)); @@ -53,14 +61,32 @@ fn timedelta(kwargs: &Kwargs) -> Result { kwargs.assert_all_used()?; let mut total = Duration::ZERO; - total = add_seconds_component(total, weeks, SECONDS_PER_WEEK, "weeks")?; - total = add_seconds_component(total, days, SECONDS_PER_DAY, "days")?; - total = add_seconds_component(total, hours, SECONDS_PER_HOUR, "hours")?; - total = add_seconds_component(total, minutes, SECONDS_PER_MINUTE, "minutes")?; - total = add_seconds_component(total, seconds, 1, "seconds")?; - total = add_nanoseconds_component(total, milliseconds, NANOS_PER_MILLISECOND, "milliseconds")?; - total = add_nanoseconds_component(total, microseconds, NANOS_PER_MICROSECOND, "microseconds")?; - total = add_nanoseconds_component(total, nanoseconds, 1, "nanoseconds")?; + total = add_component(total, weeks, SECONDS_PER_WEEK, Duration::seconds, "weeks")?; + total = add_component(total, days, SECONDS_PER_DAY, Duration::seconds, "days")?; + total = add_component(total, hours, SECONDS_PER_HOUR, Duration::seconds, "hours")?; + total = add_component( + total, + minutes, + SECONDS_PER_MINUTE, + Duration::seconds, + "minutes", + )?; + total = add_component(total, seconds, 1, Duration::seconds, "seconds")?; + total = add_component( + total, + milliseconds, + NANOS_PER_MILLISECOND, + Duration::nanoseconds, + "milliseconds", + )?; + total = add_component( + total, + microseconds, + NANOS_PER_MICROSECOND, + Duration::nanoseconds, + "microseconds", + )?; + total = add_component(total, nanoseconds, 1, Duration::nanoseconds, "nanoseconds")?; Ok(Value::from_object(TimeDeltaValue::new(total))) } @@ -71,118 +97,28 @@ fn parse_offset(raw: &str) -> Result { return Ok(UtcOffset::UTC); } - let (sign, rest) = extract_sign(trimmed, raw)?; - let (hours, minutes, seconds) = parse_offset_components(rest, raw)?; - validate_offset_ranges(hours, minutes, seconds, raw)?; - - let total_seconds = calculate_total_seconds(sign, hours, minutes, seconds); - let total_seconds = i32::try_from(total_seconds).map_err(|_| invalid_offset(raw))?; - - UtcOffset::from_whole_seconds(total_seconds).map_err(|err| { - Error::new( - ErrorKind::InvalidOperation, - format!("now offset '{raw}' is invalid: {err}"), - ) - }) -} - -fn extract_sign<'a>(trimmed: &'a str, raw: &str) -> Result<(i64, &'a str), Error> { - if let Some(remaining) = trimmed.strip_prefix('+') { - return Ok((1_i64, remaining)); - } - - trimmed - .strip_prefix('-') - .map(|remaining| (-1_i64, remaining)) - .ok_or_else(|| invalid_offset(raw)) -} - -fn parse_offset_components(rest: &str, raw: &str) -> Result<(i32, i32, i32), Error> { - let (hours_part, remaining) = rest.split_once(':').ok_or_else(|| invalid_offset(raw))?; - if hours_part.contains(':') { - return Err(invalid_offset(raw)); - } - - let (minutes_part, seconds_part) = match remaining.split_once(':') { - Some((mins, secs)) if !secs.contains(':') => (mins, Some(secs)), - Some(_) => return Err(invalid_offset(raw)), - None => (remaining, None), - }; - - if minutes_part.contains(':') { - return Err(invalid_offset(raw)); - } - - let hours = parse_component(hours_part, raw)?; - let minutes = parse_component(minutes_part, raw)?; - let seconds = seconds_part - .map(|value| parse_component(value, raw)) - .transpose()?; - - Ok((hours, minutes, seconds.unwrap_or_default())) -} - -fn validate_offset_ranges(hours: i32, minutes: i32, seconds: i32, raw: &str) -> Result<(), Error> { - if !(0..=23).contains(&hours) || !(0..=59).contains(&minutes) { - return Err(invalid_offset(raw)); - } - - if !(0..=59).contains(&seconds) { - return Err(invalid_offset(raw)); - } - - Ok(()) -} - -fn calculate_total_seconds(sign: i64, hours: i32, minutes: i32, seconds: i32) -> i64 { - sign * (i64::from(hours) * SECONDS_PER_HOUR - + i64::from(minutes) * SECONDS_PER_MINUTE - + i64::from(seconds)) -} - -fn parse_component(component: &str, original: &str) -> Result { - component - .trim() - .parse::() - .map_err(|_| invalid_offset(original)) + UtcOffset::parse(trimmed, OFFSET_FORMAT).map_err(|_| invalid_offset(raw)) } fn invalid_offset(raw: &str) -> Error { Error::new( ErrorKind::InvalidOperation, - format!("now offset '{raw}' is invalid: expected '+HH:MM' or 'Z'"), + format!("now offset '{raw}' is invalid: expected '+HH:MM[:SS]' or 'Z'"), ) } -fn add_seconds_component( - mut total: Duration, - amount: Option, - multiplier: i64, - label: &str, -) -> Result { - if let Some(value) = amount { - let seconds = value - .checked_mul(multiplier) - .ok_or_else(|| overflow_error(label))?; - let component = Duration::seconds(seconds); - total = total - .checked_add(component) - .ok_or_else(|| overflow_error(label))?; - } - Ok(total) -} - -fn add_nanoseconds_component( +fn add_component( mut total: Duration, amount: Option, multiplier: i64, + constructor: fn(i64) -> Duration, label: &str, ) -> Result { if let Some(value) = amount { - let nanos = value + let scaled = value .checked_mul(multiplier) .ok_or_else(|| overflow_error(label))?; - let component = Duration::nanoseconds(nanos); + let component = constructor(scaled); total = total .checked_add(component) .ok_or_else(|| overflow_error(label))?; diff --git a/src/stdlib/time/tests.rs b/src/stdlib/time/tests.rs index 02a1ba50..57f1c583 100644 --- a/src/stdlib/time/tests.rs +++ b/src/stdlib/time/tests.rs @@ -62,10 +62,17 @@ fn now_applies_custom_offset() { } #[rstest] -fn now_rejects_invalid_offset() { +#[case::nonsense("bogus")] +#[case::missing_sign("01:00")] +#[case::hours_out_of_range("+25:00")] +#[case::minutes_out_of_range("+01:60")] +#[case::seconds_out_of_range("+01:01:61")] +#[case::empty("")] +fn now_rejects_invalid_offset(#[case] offset: &str) { let env = build_env(); + let expr = format!("now(offset='{offset}')"); let err = env - .compile_expression("now(offset='bogus')") + .compile_expression(&expr) .expect("compile expression") .eval(context! {}); let err = err.expect_err("invalid offset should error"); @@ -99,11 +106,25 @@ fn timedelta_accumulates_components() { } #[rstest] -fn timedelta_supports_negative_values() { +#[case("timedelta(weeks=-1)", Duration::seconds(-SECONDS_PER_WEEK))] +#[case("timedelta(days=-1)", Duration::seconds(-SECONDS_PER_DAY))] +#[case("timedelta(hours=-1)", Duration::seconds(-SECONDS_PER_HOUR))] +#[case("timedelta(minutes=-1)", Duration::seconds(-SECONDS_PER_MINUTE))] +#[case("timedelta(seconds=-1)", Duration::seconds(-1))] +#[case( + "timedelta(milliseconds=-1)", + Duration::nanoseconds(-NANOS_PER_MILLISECOND), +)] +#[case( + "timedelta(microseconds=-1)", + Duration::nanoseconds(-NANOS_PER_MICROSECOND), +)] +#[case("timedelta(nanoseconds=-1)", Duration::nanoseconds(-1))] +fn timedelta_supports_negative_values(#[case] expr: &str, #[case] expected: Duration) { let env = build_env(); - let value = eval_expression(&env, "timedelta(hours=-1, seconds=-30)"); + let value = eval_expression(&env, expr); let duration = value_as_duration(&value); - assert_eq!(duration, Duration::seconds(-SECONDS_PER_HOUR - 30)); + assert_eq!(duration, expected); } #[rstest] @@ -118,17 +139,44 @@ fn timedelta_detects_overflow() { } #[rstest] -fn timestamp_iso8601_property() { - let reference = datetime!(2024-05-21 10:30:00 +00:00); +#[case( + datetime!(2024-05-21 10:30:00 +00:00), + "2024-05-21T10:30:00Z", +)] +#[case( + datetime!(2024-05-21 10:30:00 +05:45), + "2024-05-21T10:30:00+05:45", +)] +#[case( + datetime!(2024-05-21 10:30:00.123456789 +00:00), + "2024-05-21T10:30:00.123456789Z", +)] +#[case( + datetime!(2024-05-21 10:30:00.5 -03:30), + "2024-05-21T10:30:00.500000000-03:30", +)] +fn timestamp_iso8601_property(#[case] reference: OffsetDateTime, #[case] expected: &str) { let value = Value::from_object(TimestampValue::new(reference)); let iso = get_iso8601_property(&value); - assert_eq!(iso, "2024-05-21T10:30:00Z"); + assert_eq!(iso, expected); } #[rstest] -fn timedelta_iso8601_property() { - let duration = Duration::seconds(SECONDS_PER_DAY + 30) + Duration::nanoseconds(500_000_000); +#[case( + Duration::seconds(SECONDS_PER_DAY + 30) + Duration::nanoseconds(500_000_000), + "P1DT30.5S", +)] +#[case(Duration::ZERO, "PT0S")] +#[case( + Duration::seconds(-SECONDS_PER_DAY - 90), + "-P1DT1M30S", +)] +#[case( + Duration::seconds(-30) + Duration::nanoseconds(-250_000_000), + "-PT30.25S", +)] +fn timedelta_iso8601_property(#[case] duration: Duration, #[case] expected: &str) { let value = Value::from_object(TimeDeltaValue::new(duration)); let iso = get_iso8601_property(&value); - assert_eq!(iso, "P1DT30.5S"); + assert_eq!(iso, expected); } From 87d1022c1adbe53b12f4f79e93246fd13ebf1fd0 Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 8 Oct 2025 17:32:27 +0100 Subject: [PATCH 6/9] Group timedelta component specs --- src/stdlib/time/mod.rs | 93 ++++++++++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 21 deletions(-) diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index 13f823bd..b4ecc702 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -61,32 +61,78 @@ fn timedelta(kwargs: &Kwargs) -> Result { kwargs.assert_all_used()?; let mut total = Duration::ZERO; - total = add_component(total, weeks, SECONDS_PER_WEEK, Duration::seconds, "weeks")?; - total = add_component(total, days, SECONDS_PER_DAY, Duration::seconds, "days")?; - total = add_component(total, hours, SECONDS_PER_HOUR, Duration::seconds, "hours")?; + total = add_component( + total, + weeks, + ComponentSpec { + multiplier: SECONDS_PER_WEEK, + constructor: Duration::seconds, + label: "weeks", + }, + )?; + total = add_component( + total, + days, + ComponentSpec { + multiplier: SECONDS_PER_DAY, + constructor: Duration::seconds, + label: "days", + }, + )?; + total = add_component( + total, + hours, + ComponentSpec { + multiplier: SECONDS_PER_HOUR, + constructor: Duration::seconds, + label: "hours", + }, + )?; total = add_component( total, minutes, - SECONDS_PER_MINUTE, - Duration::seconds, - "minutes", + ComponentSpec { + multiplier: SECONDS_PER_MINUTE, + constructor: Duration::seconds, + label: "minutes", + }, + )?; + total = add_component( + total, + seconds, + ComponentSpec { + multiplier: 1, + constructor: Duration::seconds, + label: "seconds", + }, )?; - total = add_component(total, seconds, 1, Duration::seconds, "seconds")?; total = add_component( total, milliseconds, - NANOS_PER_MILLISECOND, - Duration::nanoseconds, - "milliseconds", + ComponentSpec { + multiplier: NANOS_PER_MILLISECOND, + constructor: Duration::nanoseconds, + label: "milliseconds", + }, )?; total = add_component( total, microseconds, - NANOS_PER_MICROSECOND, - Duration::nanoseconds, - "microseconds", + ComponentSpec { + multiplier: NANOS_PER_MICROSECOND, + constructor: Duration::nanoseconds, + label: "microseconds", + }, + )?; + total = add_component( + total, + nanoseconds, + ComponentSpec { + multiplier: 1, + constructor: Duration::nanoseconds, + label: "nanoseconds", + }, )?; - total = add_component(total, nanoseconds, 1, Duration::nanoseconds, "nanoseconds")?; Ok(Value::from_object(TimeDeltaValue::new(total))) } @@ -107,21 +153,26 @@ fn invalid_offset(raw: &str) -> Error { ) } +#[derive(Clone, Copy)] +struct ComponentSpec { + multiplier: i64, + constructor: fn(i64) -> Duration, + label: &'static str, +} + fn add_component( mut total: Duration, amount: Option, - multiplier: i64, - constructor: fn(i64) -> Duration, - label: &str, + spec: ComponentSpec, ) -> Result { if let Some(value) = amount { let scaled = value - .checked_mul(multiplier) - .ok_or_else(|| overflow_error(label))?; - let component = constructor(scaled); + .checked_mul(spec.multiplier) + .ok_or_else(|| overflow_error(spec.label))?; + let component = (spec.constructor)(scaled); total = total .checked_add(component) - .ok_or_else(|| overflow_error(label))?; + .ok_or_else(|| overflow_error(spec.label))?; } Ok(total) } From 92d4bc3a6dfdab64680b22b41c772ba1c6b318df Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 8 Oct 2025 19:14:50 +0100 Subject: [PATCH 7/9] Refine time helper parsing --- Cargo.lock | 13 ++-- Cargo.toml | 4 +- src/stdlib/time/mod.rs | 138 ++++++++++++++++++++++------------------- 3 files changed, 83 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 79135282..1b39a7e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1847,11 +1847,12 @@ dependencies = [ [[package]] name = "time" -version = "0.3.42" +version = "0.3.44" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ca967379f9d8eb8058d86ed467d81d03e81acd45757e4ca341c24affbe8e8e3" +checksum = "91e7d9e3bb61134e77bde20dd4825b97c010155709965fedf0f49bb138e52a9d" dependencies = [ "deranged", + "itoa", "num-conv", "powerfmt", "serde", @@ -1861,15 +1862,15 @@ dependencies = [ [[package]] name = "time-core" -version = "0.1.5" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9108bb380861b07264b950ded55a44a14a4adc68b9f5efd85aafc3aa4d40a68" +checksum = "40868e7c1d2f0b8d73e4a8c7f0ff63af4f6d19be117e90bd73eb1d62cf831c6b" [[package]] name = "time-macros" -version = "0.2.23" +version = "0.2.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7182799245a7264ce590b349d90338f1c1affad93d2639aed5f8f69c090b334c" +checksum = "30cfb0125f12d9c277f35663a0a33f8c30190f4e4574868a330595412d34ebf3" dependencies = [ "num-conv", "time-core", diff --git a/Cargo.toml b/Cargo.toml index 000326b3..4d497fa0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,12 +43,12 @@ tempfile = "3.8.0" ninja_env = { path = "ninja_env" } shell-quote = { version = "0.7.2", default-features = false, features = ["sh"] } shlex = "1.3.0" -time = { version = "0.3.42", features = ["formatting", "macros", "parsing", "serde"] } +time = { version = "0.3.44", features = ["formatting", "macros", "parsing", "serde"] } [build-dependencies] clap = { version = "4.5.0", features = ["derive"] } clap_mangen = "0.2.29" -time = { version = "0.3.42", features = ["formatting"] } +time = { version = "0.3.44", features = ["formatting"] } [lints.clippy] pedantic = { level = "warn", priority = -1 } diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index b4ecc702..032af4bf 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -12,7 +12,7 @@ use minijinja::{ value::{Kwargs, Object, ObjectRepr, Value}, }; use time::{ - Duration, OffsetDateTime, UtcOffset, + Duration, OffsetDateTime, Time, UtcOffset, format_description::{FormatItem, well_known::Iso8601}, macros::format_description, }; @@ -27,7 +27,7 @@ const SECONDS_PER_MINUTE_I32: i32 = 60; const SECONDS_PER_HOUR_I32: i32 = 3_600; const OFFSET_FORMAT: &[FormatItem<'static>] = format_description!( - "[offset_hour sign:mandatory]:[offset_minute][optional [:[offset_second]]]" + "[hour padding:zero]:[minute padding:zero][optional [:[second padding:zero]]]" ); /// Register time helpers with the environment. @@ -49,109 +49,107 @@ fn now(kwargs: &Kwargs) -> Result { Ok(Value::from_object(TimestampValue::new(timestamp))) } -fn timedelta(kwargs: &Kwargs) -> Result { - let weeks: Option = kwargs.get("weeks")?; - let days: Option = kwargs.get("days")?; - let hours: Option = kwargs.get("hours")?; - let minutes: Option = kwargs.get("minutes")?; - let seconds: Option = kwargs.get("seconds")?; - let milliseconds: Option = kwargs.get("milliseconds")?; - let microseconds: Option = kwargs.get("microseconds")?; - let nanoseconds: Option = kwargs.get("nanoseconds")?; - kwargs.assert_all_used()?; +fn parse_offset(raw: &str) -> Result { + let trimmed = raw.trim(); + if trimmed.eq_ignore_ascii_case("z") { + return Ok(UtcOffset::UTC); + } - let mut total = Duration::ZERO; - total = add_component( - total, - weeks, + let mut chars = trimmed.chars(); + let sign_char = chars.next().ok_or_else(|| invalid_offset(raw))?; + let sign: i8 = match sign_char { + '+' => 1, + '-' => -1, + _ => return Err(invalid_offset(raw)), + }; + + let digits = chars.as_str(); + if digits.is_empty() { + return Err(invalid_offset(raw)); + } + + let parsed = Time::parse(digits, OFFSET_FORMAT).map_err(|_| invalid_offset(raw))?; + let hours = i8::try_from(parsed.hour()).map_err(|_| invalid_offset(raw))?; + let minutes = i8::try_from(parsed.minute()).map_err(|_| invalid_offset(raw))?; + let seconds = i8::try_from(parsed.second()).map_err(|_| invalid_offset(raw))?; + + UtcOffset::from_hms(hours * sign, minutes * sign, seconds * sign) + .map_err(|_| invalid_offset(raw)) +} + +fn invalid_offset(raw: &str) -> Error { + Error::new( + ErrorKind::InvalidOperation, + format!("now offset '{raw}' is invalid: expected '+HH:MM[:SS]' or 'Z'"), + ) +} + +const COMPONENT_SPECS: &[(&str, ComponentSpec)] = &[ + ( + "weeks", ComponentSpec { multiplier: SECONDS_PER_WEEK, constructor: Duration::seconds, label: "weeks", }, - )?; - total = add_component( - total, - days, + ), + ( + "days", ComponentSpec { multiplier: SECONDS_PER_DAY, constructor: Duration::seconds, label: "days", }, - )?; - total = add_component( - total, - hours, + ), + ( + "hours", ComponentSpec { multiplier: SECONDS_PER_HOUR, constructor: Duration::seconds, label: "hours", }, - )?; - total = add_component( - total, - minutes, + ), + ( + "minutes", ComponentSpec { multiplier: SECONDS_PER_MINUTE, constructor: Duration::seconds, label: "minutes", }, - )?; - total = add_component( - total, - seconds, + ), + ( + "seconds", ComponentSpec { multiplier: 1, constructor: Duration::seconds, label: "seconds", }, - )?; - total = add_component( - total, - milliseconds, + ), + ( + "milliseconds", ComponentSpec { multiplier: NANOS_PER_MILLISECOND, constructor: Duration::nanoseconds, label: "milliseconds", }, - )?; - total = add_component( - total, - microseconds, + ), + ( + "microseconds", ComponentSpec { multiplier: NANOS_PER_MICROSECOND, constructor: Duration::nanoseconds, label: "microseconds", }, - )?; - total = add_component( - total, - nanoseconds, + ), + ( + "nanoseconds", ComponentSpec { multiplier: 1, constructor: Duration::nanoseconds, label: "nanoseconds", }, - )?; - - Ok(Value::from_object(TimeDeltaValue::new(total))) -} - -fn parse_offset(raw: &str) -> Result { - let trimmed = raw.trim(); - if trimmed.eq_ignore_ascii_case("z") { - return Ok(UtcOffset::UTC); - } - - UtcOffset::parse(trimmed, OFFSET_FORMAT).map_err(|_| invalid_offset(raw)) -} - -fn invalid_offset(raw: &str) -> Error { - Error::new( - ErrorKind::InvalidOperation, - format!("now offset '{raw}' is invalid: expected '+HH:MM[:SS]' or 'Z'"), - ) -} + ), +]; #[derive(Clone, Copy)] struct ComponentSpec { @@ -184,6 +182,18 @@ fn overflow_error(label: &str) -> Error { ) } +fn timedelta(kwargs: &Kwargs) -> Result { + let mut total = Duration::ZERO; + + for (name, spec) in COMPONENT_SPECS { + let amount: Option = kwargs.get(name)?; + total = add_component(total, amount, *spec)?; + } + + kwargs.assert_all_used()?; + Ok(Value::from_object(TimeDeltaValue::new(total))) +} + fn format_offset_datetime(datetime: OffsetDateTime) -> String { datetime.format(&Iso8601::DEFAULT).map_or_else( |_| datetime.to_string(), From fc3f8b8a7f8837c9476c2d14d3cbcfc348082b7d Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 8 Oct 2025 21:27:28 +0100 Subject: [PATCH 8/9] Parse now offsets with UtcOffset::parse --- src/stdlib/time/mod.rs | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index 032af4bf..e891e701 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -12,7 +12,7 @@ use minijinja::{ value::{Kwargs, Object, ObjectRepr, Value}, }; use time::{ - Duration, OffsetDateTime, Time, UtcOffset, + Duration, OffsetDateTime, UtcOffset, format_description::{FormatItem, well_known::Iso8601}, macros::format_description, }; @@ -26,8 +26,8 @@ const NANOS_PER_MILLISECOND: i64 = 1_000 * NANOS_PER_MICROSECOND; const SECONDS_PER_MINUTE_I32: i32 = 60; const SECONDS_PER_HOUR_I32: i32 = 3_600; -const OFFSET_FORMAT: &[FormatItem<'static>] = format_description!( - "[hour padding:zero]:[minute padding:zero][optional [:[second padding:zero]]]" +const OFFSET_FMT: &[FormatItem<'static>] = format_description!( + "[offset_hour sign:mandatory]:[offset_minute][optional [:[offset_second]]]" ); /// Register time helpers with the environment. @@ -55,26 +55,7 @@ fn parse_offset(raw: &str) -> Result { return Ok(UtcOffset::UTC); } - let mut chars = trimmed.chars(); - let sign_char = chars.next().ok_or_else(|| invalid_offset(raw))?; - let sign: i8 = match sign_char { - '+' => 1, - '-' => -1, - _ => return Err(invalid_offset(raw)), - }; - - let digits = chars.as_str(); - if digits.is_empty() { - return Err(invalid_offset(raw)); - } - - let parsed = Time::parse(digits, OFFSET_FORMAT).map_err(|_| invalid_offset(raw))?; - let hours = i8::try_from(parsed.hour()).map_err(|_| invalid_offset(raw))?; - let minutes = i8::try_from(parsed.minute()).map_err(|_| invalid_offset(raw))?; - let seconds = i8::try_from(parsed.second()).map_err(|_| invalid_offset(raw))?; - - UtcOffset::from_hms(hours * sign, minutes * sign, seconds * sign) - .map_err(|_| invalid_offset(raw)) + UtcOffset::parse(trimmed, OFFSET_FMT).map_err(|_| invalid_offset(raw)) } fn invalid_offset(raw: &str) -> Error { From 9cb2d0ac7ea744a44f65e1b44b919c6c7b481d6d Mon Sep 17 00:00:00 2001 From: Leynos Date: Wed, 8 Oct 2025 21:52:25 +0100 Subject: [PATCH 9/9] Clarify ISO-8601 helper formatting Extract a predicate for detecting timezone suffixes when trimming fractional seconds, tighten the offset parser to require an explicit sign before delegating to the time crate, and expand the expect message for fractional-second formatting. --- src/stdlib/time/mod.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/stdlib/time/mod.rs b/src/stdlib/time/mod.rs index e891e701..39aa0135 100644 --- a/src/stdlib/time/mod.rs +++ b/src/stdlib/time/mod.rs @@ -26,9 +26,8 @@ const NANOS_PER_MILLISECOND: i64 = 1_000 * NANOS_PER_MICROSECOND; const SECONDS_PER_MINUTE_I32: i32 = 60; const SECONDS_PER_HOUR_I32: i32 = 3_600; -const OFFSET_FMT: &[FormatItem<'static>] = format_description!( - "[offset_hour sign:mandatory]:[offset_minute][optional [:[offset_second]]]" -); +const OFFSET_FMT: &[FormatItem<'static>] = + format_description!("[offset_hour]:[offset_minute][optional [:[offset_second]]]"); /// Register time helpers with the environment. pub(crate) fn register_functions(env: &mut Environment<'_>) { @@ -55,6 +54,10 @@ fn parse_offset(raw: &str) -> Result { return Ok(UtcOffset::UTC); } + if !trimmed.starts_with(['+', '-']) { + return Err(invalid_offset(raw)); + } + UtcOffset::parse(trimmed, OFFSET_FMT).map_err(|_| invalid_offset(raw)) } @@ -175,15 +178,19 @@ fn timedelta(kwargs: &Kwargs) -> Result { Ok(Value::from_object(TimeDeltaValue::new(total))) } +fn has_timezone_after(formatted: &str, pos: usize) -> bool { + formatted + .get(pos + 10..) + .and_then(|rest| rest.chars().next()) + .is_some_and(|next| matches!(next, 'Z' | '+' | '-')) +} + fn format_offset_datetime(datetime: OffsetDateTime) -> String { datetime.format(&Iso8601::DEFAULT).map_or_else( |_| datetime.to_string(), |mut formatted| { if let Some(pos) = formatted.find(".000000000") - && formatted - .get(pos + 10..) - .and_then(|rest| rest.chars().next()) - .is_some_and(|next| matches!(next, 'Z' | '+' | '-')) + && has_timezone_after(&formatted, pos) { formatted.replace_range(pos..pos + 10, ""); } @@ -272,7 +279,8 @@ fn finalize_duration_buffer(mut buffer: String, time_section: &str) -> String { } fn format_seconds_with_fraction(seconds: i64, nanos: i32) -> String { - let seconds = u64::try_from(seconds).expect("seconds must be non-negative"); + let seconds = u64::try_from(seconds) + .expect("seconds must be non-negative when formatting from absolute duration remainder"); if nanos == 0 { return format!("{seconds}S"); }