time: Add is_datetime() for RFC 3339 Internet Date/Time Format#2345
time: Add is_datetime() for RFC 3339 Internet Date/Time Format#2345vtushar06 wants to merge 1 commit intosourcemeta:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds sourcemeta::core::is_datetime(std::string_view) -> bool to validate RFC 3339 date-time strings (Section 5.6) using a pure string_view state machine, and introduces a dedicated unit test suite.
Changes:
- Add
is_datetime()public API tosourcemeta::core::time. - Implement RFC 3339 date-time validation (including leap-year rules, leap seconds, and required timezone).
- Add comprehensive unit tests and wire new sources into the build.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/core/time/include/sourcemeta/core/time.h |
Declares is_datetime() and documents RFC 3339 expectations/usage. |
src/core/time/datetime.cc |
Implements the RFC 3339 date-time validator as a zero-allocation parser. |
src/core/time/CMakeLists.txt |
Adds datetime.cc to the time library build. |
test/time/datetime_test.cc |
Adds 40 unit tests covering valid/invalid RFC 3339 date-time cases. |
test/time/CMakeLists.txt |
Adds datetime_test.cc to the time unit test target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/core/time/datetime.cc">
<violation number="1" location="src/core/time/datetime.cc:128">
P2: `is_datetime` accepts `:60` unconditionally, making RFC 3339 validation too permissive for leap-second semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
23466b0 to
cf89346
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implements sourcemeta::core::is_datetime() in src/core/time following the same pattern as is_ipv4() and is_ipv6() in src/core/ip. - Pure string_view state machine, no heap allocations - Validates full date-time production from RFC 3339 §5.6 - Leap year per RFC 3339 Appendix C formula - Accepts leap seconds (second=60) for any date per §5.7 - Case-insensitive T/Z separators per §5.6 NOTE - Rejects colonless offsets, space separators, missing timezone - 40 unit tests covering valid and invalid inputs Relates to format-assertion support for Draft 4 and Draft 6. Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
cf89346 to
00ba70a
Compare
|
hey @jviotti, PR is ready for review, let me know if any further tweaks are needed :) |
Body
Adds
sourcemeta::core::is_datetime()tosrc/core/timefollowing the same pattern asis_ipv4()andis_ipv6()insrc/core/ip.This is a building block for format-assertion support in Blaze for Draft 4 and Draft 6.
Function signature:
Pure
string_viewstate machine. No heap allocations. No external dependencies.RFC 3339 §5.6 compliance:
full-date "T" full-timeproduction rulesecond=60(leap seconds) per §5.7+0530)Tests: 40 cases (17 valid, 23 invalid)
Files changed:
src/core/time/include/sourcemeta/core/time.his_datetime()declarationsrc/core/time/datetime.ccsrc/core/time/CMakeLists.txtdatetime.ccto SOURCEStest/time/datetime_test.cctest/time/CMakeLists.txtdatetime_test.ccto SOURCESOut of scope for this PR:
Blaze integration -
is_datetime()is the building block. Hooking it intoformat-assertionin the Blaze compiler is a separate PR once this is reviewed.