Skip to content

Deprecate lenient RFC 3339 date time separators - #191

Open
youdie006 wants to merge 1 commit into
ruby:masterfrom
youdie006:rfc3339-separator-deprecation
Open

Deprecate lenient RFC 3339 date time separators#191
youdie006 wants to merge 1 commit into
ruby:masterfrom
youdie006:rfc3339-separator-deprecation

Conversation

@youdie006

@youdie006 youdie006 commented Sep 11, 2026

Copy link
Copy Markdown

Follow-up to ruby/time#85, where you specified the approach:

To detect the invalid character, in the rfc3339 static function, add a capture group for the separator ("(?:t|\\s)" -> "(t|\\s)"), and in the rfc3339_cb static function, issue the warning if the capture group character doesn't equal t, T, or .

Date.rfc3339, DateTime.rfc3339 and Date._rfc3339 accept seven separators: T, a space, and \t\n\v\f\r. This warns on the last five under Warning[:deprecated], changing nothing else, so they can become Date::Error later.

Warning[:deprecated] = true
Date._rfc3339("2001-02-03\t04:05:06Z")
# warning: Invalid RFC3339 date time separator provided, this will raise an error in a future version.

Adding the capture group shifts the remaining groups, so SNUM goes 8 to 9 and the hour/min/sec/sec_fraction/zone indices move up by one.

One deviation from your suggestion

rb_warning_category_enabled_p is not usable from an extension: it is in no public header, and on 3.4 it is not exported from libruby at all (nm -D libruby.so | grep -c rb_warning_category_enabled_p is 0). Building with it fails on ruby 2.7 ('RB_WARN_CATEGORY_DEPRECATED' undeclared) and on ruby 3.4 (implicit declaration of function).

So the warning goes through rb_category_warn, which is public and which ext/date/extconf.rb:10 already probes, with the #ifndef HAVE_RB_CATEGORY_WARN fallback date_core.c:4690-4692 already uses so the gemspec's 2.6 floor keeps building. That also makes the outer guard redundant: rb_category_warn reports only when $VERBOSE is non-nil and the category is enabled. Happy to change it if you had something else in mind.

Verification

Date._rfc3339 parses identically for all seven separators in every row; only the warning is new.

row result
new test, fix applied 2 tests, 19 assertions, 0 failures
new test, date_parse.c reverted 1 failure
warn unconditionally (drop the t/T/space check) 1 failure, the T/t/space rows
keep the capture group but leave hour reading s[4] 2 failures
revert (t|\s) to (?:t|\s), keep the renumbering 2 errors
suite result
full test/date, ruby 2.7.8 148 tests, 162607 assertions, 0 failures
full test/date, ruby 3.2.11 148 tests, 162619 assertions, 0 failures
full test/date, ruby 3.4.10 148 tests, 162619 assertions, 0 failures
test_date_parse.rb, truffleruby 3.3.7 28 tests, 928 assertions, 0 failures (1 failure before the fix)

With deprecation warnings off, which is the default, nothing is emitted.

I kept this on its own branch so it stays independent of #190.


Disclosure: I used Claude (an AI assistant) while preparing this change. Every result above I ran and verified myself.

RFC 3339 allows only "T", and by convention a space, between the date
and the time, but Date._rfc3339 also accepts \t, \n, \v, \f and \r.
Warn on those under Warning[:deprecated] before they become an error.

The separator is now captured so rfc3339_cb can inspect it, which
shifts the remaining group indices.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant