feat: implement date/time formatting functions (issue #283)#659
Conversation
📝 WalkthroughWalkthroughAdds a new PRD and implementation for XPath‑style formatting: fn:format-integer, fn:format-dateTime, fn:format-date, fn:format-time; introduces DateTimeFormatUtil parser/formatter, new format exceptions, registers functions in DefaultFunctionLibrary, and adds extensive unit/integration tests covering parsing, formatting, modifiers, widths, locales, and error cases. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (metapath)
participant Lib as DefaultFunctionLibrary
participant Fn as FnFormat* (Integer / Date / Time / DateTime)
participant Util as DateTimeFormatUtil
participant Temporal as ITemporalItem
Caller->>Lib: resolve function
Lib->>Fn: invoke handler(args)
Fn->>Temporal: coerce/inspect value
Fn->>Util: formatDateTime(value, picture, language, calendar, place, allowedMarkers)
Util->>Util: parsePictureString(picture) / build components
Util->>Temporal: extract components (year, month, hour, tz, digits...)
Util->>Fn: return formatted string
Fn->>Caller: return IStringItem
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.21.0)core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. core/src/main/java/dev/metaschema/core/metapath/function/FormatDateTimeFunctionException.java[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. core/src/main/java/dev/metaschema/core/metapath/function/FormatFunctionException.java[ERROR] Cannot load ruleset pmd/category/java/custom.xml: Cannot resolve rule/ruleset reference 'pmd/category/java/custom.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@PRDs/20260207-format-datetime/implementation-plan.md`:
- Line 199: The example date/weekday pair in the spec for the format string
"[FNn], [D1o] [MNn] [Y]" is incorrect; change the example "Tuesday, 5th March
2026" to a correct pairing such as "Thursday, 5th March 2026" (or substitute a
matching date like "Wednesday, 4th March 2026" or "Monday, 2nd March 2026") so
the weekday and date are consistent with the format example.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@core/src/main/java/dev/metaschema/core/metapath/function/library/DateTimeFormatUtil.java`:
- Line 610: The current line in DateTimeFormatUtil uses int year =
Math.abs(value.getYear()), which can overflow for Integer.MIN_VALUE; change the
handling to compute an absolute year safely (e.g., use a long and take
Math.abs((long) value.getYear()) or use Math.absExact if on Java 15+) and update
any dependent variables/formatting logic that assume an int to use the safe long
value or handle the possible ArithmeticException from Math.absExact; ensure
references to year in methods inside DateTimeFormatUtil are adjusted to the new
type or guarded accordingly.
- Line 794: DateTimeFormatUtil uses LocalDate.of(...) which throws for year <=
0; update all places that create a LocalDate from value.getYear() (e.g., the
LocalDate.of(...) call used by formatDayOfYear, formatDayOfWeek,
formatWeekOfYear, formatWeekOfMonth) to use a proxy year >= 1 (e.g., int
proxyYear = Math.max(1, value.getYear())) when constructing LocalDate so
calculations succeed for year 0 and negative years while leaving formatYear
(which already handles negatives) unchanged.
In
`@core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatInteger.java`:
- Around line 676-704: formatLargeNumber currently only knows scales up to
"trillion" (scaleWords) so for groups with index i >= scaleWords.length it
silently omits the scale; update formatLargeNumber to handle larger magnitudes
by either (a) extending the scaleWords array with additional entries
("quadrillion", "quintillion", etc.) to cover expected ranges or (b) adding a
clear fallback when i >= scaleWords.length that appends a decimal/exponential
representation (or returns value.toString()) instead of omitting the scale;
locate the scaleWords array and the loop that appends scaleWords in
formatLargeNumber and implement one of these fixes (ensure
numberToWords(BigInteger.valueOf(group)) is still used for the group text and
the fallback triggers before finishing the result).
- Around line 41-88: The two-argument signature currently marks contextDependent
but executeTwoArg ignores the DynamicContext and passes null for language to
fnFormatInteger; modify executeTwoArg to call
dynamicContext.getStaticContext().getDefaultLanguage() and pass that value into
fnFormatInteger (same approach as FnDefaultLanguage), so the 2-arg form uses the
context's default language for language-dependent formatting.
🧹 Nitpick comments (4)
core/src/test/java/dev/metaschema/core/metapath/function/library/DateTimeFormatUtilTest.java (1)
919-931: Test assertion could be more specific.The
testWeekOfMonthtest uses a loose assertion (>= 1 && <= 6) rather than verifying the exact expected value. For2002-12-31, the week of month should be deterministic. Consider usingassertEqualswith the expected value for more precise validation.💡 Proposed fix for more specific assertion
`@Test` void testWeekOfMonth() { - // 2002-12-31 -> week of month varies; just check it's a valid number String result = format(TEST_DATE, "[w]", DATE_MARKERS); - assertTrue(Integer.parseInt(result) >= 1 && Integer.parseInt(result) <= 6); + // 2002-12-31 is in week 5 of December + assertEquals("5", result); }core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java (1)
36-43: Clarify TIME_MARKERS inclusion of C and E in comment.The Javadoc at lines 37-38 states the set "excludes date-only markers", but
'C'(calendar) and'E'(era) are included. While this may be correct per the XPath 3.1 spec (as these can relate to time zone context), the comment could be misleading. Consider updating the comment to clarify why C and E are included for time values.📝 Suggested documentation improvement
/** - * The set of component specifiers allowed for time values, which excludes - * date-only markers (Y, M, D, d, F, W, w). + * The set of component specifiers allowed for time values. Excludes + * date-only markers (Y, M, D, d, F, W, w) but includes C (calendar) and + * E (era) which can apply to time values in certain contexts. */core/src/main/java/dev/metaschema/core/metapath/function/library/DateTimeFormatUtil.java (2)
473-479: Unused parameterscalendarandplace.The
calendarandplaceparameters are declared but not used in the formatting logic. While this may be intentional for API compatibility with the XPath 5-argument signature (where behavior is implementation-defined), consider either:
- Adding a brief comment noting these are reserved for future implementation, or
- Logging a warning when non-null values are passed
916-921: UnusedsecondModparameter informatAmPm.The
secondModparameter is declared and documented but never used in the implementation. Either remove it from the signature or add handling for relevant modifiers.♻️ Remove unused parameter
`@NonNull` private static String formatAmPm( int hour, `@Nullable` String primaryMod, - `@Nullable` Character secondMod, `@Nullable` Integer minWidth, `@Nullable` Integer maxWidth) {Note: This would also require updating the call site at line 550.
core/src/main/java/dev/metaschema/core/metapath/function/library/DateTimeFormatUtil.java
Outdated
Show resolved
Hide resolved
core/src/main/java/dev/metaschema/core/metapath/function/library/DateTimeFormatUtil.java
Outdated
Show resolved
Hide resolved
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatInteger.java
Show resolved
Hide resolved
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatInteger.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@PRDs/20260207-format-datetime/implementation-plan.md`:
- Around line 417-465: Add an explicit acceptance checklist item under the
"Quality" (or "Acceptance Criteria") section to ensure website/README updates
are performed; specifically add a bullet like "Website/README updated to
document new fn:format-date/time/integer behavior and examples" so reviewers can
verify the documentation task is completed alongside code changes and tests.
In `@PRDs/20260207-format-datetime/PRD.md`:
- Around line 316-319: The phrase "single digit" in section 9.8.4.5 should be
hyphenated for grammar: change occurrences of "single digit format token" to
"single-digit format token" in the fractional seconds section (referencing
tokens [f1], [f001], [f01] and the section header 9.8.4.5) so the spec reads "A
single-digit format token (`[f1]`) retains all fractional digits" for clarity
and consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java`:
- Around line 36-43: TIME_MARKERS in FnFormatTime currently includes the era
marker 'E', which is date-related and must be excluded; update the static final
Set<Character> TIME_MARKERS in FnFormatTime.java to remove 'E' so it only
contains 'H', 'h', 'P', 'm', 's', 'f', 'Z', 'z', 'C', ensuring fn:format-time
will correctly treat era markers as invalid for time values and trigger FOFD1350
where appropriate.
🧹 Nitpick comments (1)
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatInteger.java (1)
656-663: Minor: Number-to-words format doesn't include "and" connector.The implementation produces "one hundred twenty-three" while the implementation plan (line 116) shows "one hundred and twenty-three". Both are acceptable English forms, but ensure tests align with the actual output format. The current implementation (no "and") is consistent with American English convention.
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java`:
- Around line 48-66: The implementations in FnFormatTime must return an empty
sequence (no item) when the input time argument is null/empty and must use the
dynamic context default language when the 5‑arg call receives an empty
$language; update executeTwoArg and executeFiveArg to check for a
missing/time-null value and return an empty sequence instead of a single
empty-string item, ensure executeFiveArg falls back to the dynamic context’s
default language when $language is empty, and update the function signatures
(e.g., SIGNATURE_TWO_ARG and the corresponding SIGNATURE_FIVE_ARG) to use the
correct return cardinality (zeroOrOne) matching the empty-sequence behavior.
In `@PRDs/20260207-format-datetime/PRD.md`:
- Around line 494-502: Update the Verification Checklist under the "### 5.4
Verification Checklist" section in PRD.md to include a new checkbox for the
website/README update required by the acceptance criteria; add a line like "- [
] Website and README updated to reflect changes" (or similar) so the checklist
explicitly tracks that task and ensure it appears alongside the other existing
checkboxes.
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java
Show resolved
Hide resolved
david-waltermire
force-pushed
the
feature/283-format-datetime
branch
from
e074689 to
16def85
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatDateTime.java`:
- Around line 63-178: The function signatures currently declare .returnOne() and
the handlers executeTwoArg and executeFiveArg always return a string item (using
formatDateTime even when value==null), but per spec a null/empty $value should
produce an empty sequence; change the signature builders that use executeTwoArg
and executeFiveArg to .returnZeroOrOne(), and update executeTwoArg and
executeFiveArg to return ISequence.empty() when the parsed IDateTimeItem value
is null (otherwise keep returning IStringItem.of(formatDateTime(...))). Ensure
formatDateTime remains unchanged.
In
`@core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java`:
- Around line 47-104: The function signatures SIGNATURE_TWO_ARG and
SIGNATURE_FIVE_ARG declare .returnOne() but must be .returnZeroOrOne() to match
xs:string? per XPath 3.1; update both builders to .returnZeroOrOne() and adjust
the handlers executeTwoArg and executeFiveArg to return ISequence.empty() when
the incoming "value" is null/empty instead of returning a single empty-string
item (do not wrap "" in a sequence). Ensure the change is applied to the builder
calls and the null/empty branch in both executeTwoArg and executeFiveArg so
empty sequence input yields an empty sequence.
🧹 Nitpick comments (1)
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java (1)
128-150: Consider falling back to default language in 5-arg form when$languageis empty.The 5-arg handler passes
nullfor language when the argument is absent (line 147). The 2-arg form falls back todynamicContext.getStaticContext().getDefaultLanguage()(line 122). For consistency with XPath 3.1 behavior where absent/empty language uses the default from context, consider applying the same fallback in the 5-arg form:- return ISequence.of(IStringItem.valueOf( - formatTime( - value, - picture.asString(), - language == null ? null : language.asString(), + String lang = language == null + ? dynamicContext.getStaticContext().getDefaultLanguage() + : language.asString(); + return ISequence.of(IStringItem.valueOf( + formatTime( + value, + picture.asString(), + lang,
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatDateTime.java
Show resolved
Hide resolved
core/src/main/java/dev/metaschema/core/metapath/function/library/FnFormatTime.java
Show resolved
Hide resolved
Implement the XPath 3.1 date/time and integer formatting functions per the W3C XPath Functions 3.1 specification (sections 4.6.1 and 9.8). Functions added: - fn:format-integer with 2-arg and 3-arg signatures supporting decimal digit patterns, Roman numerals (i/I), alphabetic (a/A), words (w/W/Ww), ordinal numbers, and grouping separators - fn:format-dateTime with 2-arg and 5-arg signatures - fn:format-date with 2-arg and 5-arg signatures - fn:format-time with 2-arg and 5-arg signatures Key capabilities: - Picture string parser supporting all 16 component specifiers (Y, M, D, d, F, W, w, H, h, P, m, s, f, Z, z, C, E) with presentation modifiers and width modifiers - Formatting engine with year modulo, fractional seconds, timezone (numeric, military, GMT prefix), named months/days, and ordinal support - Proper marker validation per value type (FOFD1350 for invalid markers) - Ordinal modifier falls back to cardinal for non-decimal format tokens - Safe Math.abs handling for Integer.MIN_VALUE year values - FormatFunctionException (FODF1310) and FormatDateTimeFunctionException (FOFD1340, FOFD1350) error classes Includes PRD documentation and 206 unit tests. Closes metaschema-framework#283
david-waltermire
force-pushed
the
feature/283-format-datetime
branch
from
16def85 to
6a2a840
Compare
david-waltermire
merged commit 671b7f4
into
metaschema-framework:develop
2 participants
Summary
fn:format-integer(2-arg and 3-arg) per XPath Functions 3.1 section 4.6.1, supporting decimal digit patterns, Roman numerals, alphabetic, words, ordinal numbers, and grouping separatorsfn:format-dateTime,fn:format-date,fn:format-time(2-arg and 5-arg each) per XPath Functions 3.1 sections 9.8.1-9.8.3, with a shared picture string parser and formatting engine supporting all 16 component specifiersFormatFunctionException(FODF1310) andFormatDateTimeFunctionException(FOFD1340, FOFD1350) error classesDefaultFunctionLibraryCloses #283
Test plan
mvn -pl core test -Dtest="FormatFunctionExceptionTest,FormatDateTimeFunctionExceptionTest,FnFormatIntegerTest,DateTimeFormatUtilTest,FnFormatDateTimeTest,FnFormatDateTest,FnFormatTimeTest"passes (204 tests)mvn clean install -PCI -Preleasepasses all modules with 0 PMD violations, 0 SpotBugs bugs, 0 Checkstyle violationsSummary by CodeRabbit
New Features
Documentation
Tests
Chores