Bug 24704 – The error message for DateTime.fromISOExtString says that valid ISO extended strings that it does not support are invalid ISO extended strings

## Summary When parsing ISO8601 date-times using DateTime.fromISOExtString, the fraction component of the last time unit causes an exception to be thrown. ## Context The method https://dlang.org/phobos/std_datetime_date.html#.DateTime.fromISOExtString is built to support the ISO8601 standard: https://en.wikipedia.org/wiki/ISO_8601#Times According to this standard: > A decimal fraction may be added to the lowest order time element present in any of these representations. A decimal mark, either a comma or a dot on the baseline, is used as a separator between the time element and its fraction. (Following ISO 80000-1 according to ISO 8601:1-2019,[27] it does not stipulate a preference except within International Standards, but with a preference for a comma according to ISO 8601:2004.[28]) For example, to denote "14 hours, 30 and one half minutes", do not include a seconds figure; represent it as "14:30,5", "T1430,5", "14:30.5", or "T1430.5". > > There is no limit on the number of decimal places for the decimal fraction. However, the number of decimal places needs to be agreed to by the communicating parties. For example, in Microsoft SQL Server, the precision of a decimal fraction is 3 for a DATETIME, i.e., "yyyy-mm-ddThh:mm:ss[.mmm]".[29] ## Steps To Reproduce the Problem Create the following program: ``` void main() { import std.datetime; auto b = DateTime.fromISOExtString("2024-08-15T08:13:23.000"); } ``` ## Expected Results The date-time should be parsed, with the fraction being interpreted as a fraction of the seconds unit. ## Actual Results An exception is thrown of the form: ``` core.time.TimeException@/dlang/dmd/linux/bin64/../../src/phobos/std/datetime/date.d(9334): Invalid ISO Extended String: 08:13:23.000 ---------------- /dlang/dmd/linux/bin64/../../src/phobos/std/datetime/date.d:9334 pure @safe std.datetime.date.TimeOfDay std.datetime.date.TimeOfDay.fromISOExtString!(immutable(char)[]).fromISOExtString(scope const(immutable(char)[])) [0x5626aa88fea9] /dlang/dmd/linux/bin64/../../src/phobos/std/datetime/date.d:3271 pure @safe std.datetime.date.DateTime std.datetime.date.DateTime.fromISOExtString!(immutable(char)[]).fromISOExtString(scope const(immutable(char)[])) [0x5626aa87f85e] ./onlineapp.d:7 _Dmain [0x5626aa87ebc7] ```

I remember that I stumbeled about that issue, but I had no time to care about it or report it as bug. I just examined the code - it converts the string to a TimeOfDay structure, which has noch fractions of a second. So, in a logic consequence, no support for fractions of a second. So we can either ignore the fractions (no good), or implement the missing bits.

Added a draft PR for this issue: https://github.com/dlang/phobos/pull/9046 At the moment only the ISOExtString beviour is modified. We might need some reasoning, if similiar changes schould be applied for the other from/to functions. Most important, some implementation hacks must cleaned up and replaced by some 'better' solution.

DateTime is not supposed to support fractional seconds. It's specifically for dealing with calendar operations. SysTime is the time intended for the system time. The documentation is clear on this. As such, if you have an ISO or ISO extended string which may contain fractional seconds, you should use SysTime's fromISO(Ext)String, not DateTime's.

(In reply to Jonathan M Davis from comment #3) > As such, if you have an ISO or ISO extended string which may contain > fractional seconds, you should use SysTime's fromISO(Ext)String, not > DateTime's. And if for some reason, you really do need a DateTime rather than a SysTime, you can cast the resulting SysTime to a DateTime.

Good point. So I move my changes to SysTime, if this is the better place for it.

(In reply to Carsten Schlote from comment #5) > Good point. So I move my changes to SysTime, if this is the better place for > it. SysTime already supports fractional seconds. This already compiles and runs just fine: --- void main() { import std.datetime; auto dt = cast(DateTime)SysTime.fromISOExtString("2024-08-15T08:13:23.000"); assert(dt.year == 2024); assert(dt.month == 8); assert(dt.day == 15); assert(dt.hour == 8); assert(dt.minute == 13); assert(dt.second == 23); } ---

Problem can be easily fixed like this: ``` import std.datetime; auto b = SysTime.fromISOExtString("2024-08-15T08:13:23.000"); assert("2024-Aug-15 08:13:23" == b.toString); auto c = SysTime.fromISOExtString("2024-08-15T08:13:23.25"); assert("2024-Aug-15 08:13:23.25" == c.toString); auto d = SysTime(DateTime(2024, 8, 15, 13, 23, 25)).toISOExtString(); assert("2024-08-15T13:23:25" == d); auto e = cast(DateTime)c; assert("2024-Aug-15 08:13:23" == e.toString); ```

It wasn't actually a bug - just a misunderstanding on how to use the library - so marking it as fixed isn't appropriate (that would result in an entry in the changelog), so I'm changing it to wontfix.

I think the error message should be altered to prevent confusion in the future. Currently it states: > Invalid ISO Extended String: 08:13:23.000 This message gives the impression that DateTime is supporting the full ISO Extended String standard. One can also reproduce the error by having a fraction on the minute unit: > Invalid ISO Extended String: 08:13.000 Regardless of whether DateTime is meant to use second or millisecond resolution, `fromISOExtString` claims to support ISO Ext Strings. The ISO standard doesn't change simply because of an implementation detail of DateTime. Because the ISO standard does not control the number of decimal digits that the fraction can have, in fact, every implementation will not be able to fully represent a valid ISO string, but they continue to accept the ISO standard, extracting as much information as they can. For example, a utility that only supports millisecond resolution would still be able to parse an ISO string of "2024-08-15T08:13:25.123456789", only it will only support saving a value equivalent to "2024-08-15T08:13:25.123". In my opinion, there are still two issues remaining: 1. Valid ISO Strings within the capabilities of `DateTime` are being rejected, e.g. "2024-08-15T08:13.5". 2. Valid ISO Strings beyond the resolution of `DateTime`, like "2024-08-15T08:13:25.123", should either be parsed to include as much information as they can, or the error message should be clarified rather than claiming that the string is not ISO.

As a point of reference, the approach taken by Java's LocalTime (which has nanosecond precision) when given a string that has more data than it can handle, is to throw a parse exception. E.g. the following action produces the following error: ``` // 10 fraction digits, 1 more than is supported LocalTime myTime = LocalTime.parse("08:15:23.12345678910") ``` ``` Exception in thread "main" java.time.format.DateTimeParseException: Text '08:15:23.12345678910' could not be parsed, unparsed text found at index 18 ```

I can see good reasons to add a 'fraction of a second' to TimeOfDay. In this case it would be really easy to add the missing functionality to to/fromISOExtString. And it also fits reality a little bit better, because fractions of a seconds do matter in real life. However, there are also good reasons to avoid changes in the core libs without very good reasons. At least it will increase memory footprint because you must store the fraction of a second somewhere. So any change must be very intensively and well tested. Otherwise, if we add sucha fraction, in return we would get: - No/less info ist lost, when cast from SysTime to DateTime. And vice versa. - No more confusion with different variant of from ISOExtString.

The documentation on fromISOExtString on each of the types says what that function accepts on that type. In all cases, it is an ISO extended string which corresponds to the portions of the time that type supports. The standard says what ISO extended strings look like, but it does not require that all fields be present in an ISO extended string, and there is no requirement that code parsing an ISO extended string support every variant of ISO extended string in existence, and none of the functions in std.datetime are intended to support every variant of ISO extended string. Not even SysTime.fromISOExtString does, since it doesn't support dates or times on their own (which would be valid ISO extended strings so long as they're formatted correctly for those portions of the date or time). As such, I see no problem with what these functions parse. They do exactly what they say they'll do. That being said, it is true that it's possible to pass an ISO extended string which either has unsupported fields or which is missing required fields to fromISOExtString. So, perhaps the exception's message should say something like "String not in the required format:" instead of "Invalid ISO Extended String:". So, I'll create a PR adjusting the error messages.

@jmdavis updated dlang/phobos pull request #9047 "Fix issue 24704: Improve error messages for from*String in std.datetime." fixing this issue: - Fix Bugzilla issue 24704: Improve error messages for from*String in std.datetime. The documentation for the from*String functions specifies what they accept, and the fromISO*String functions accept ISO or ISO extended strings, but it is technically the case that there are ISO and ISO extended strings which they do not accept (since the standard specifies what the fields should look like for each string but doesn't specify that all fields must be present or that any code parsing them must accept all variants). So, technically, the error messages which have said that the given strings are not valid ISO or ISO extended strings are not necessarily correct. So, this changes the error messages to remove that ambiguity. https://github.com/dlang/phobos/pull/9047

dlang/phobos pull request #9047 "Fix issue 24704: Improve error messages for from*String in std.datetime." was merged into master: - 2a3374fb3bbb4c246204139c1393b843aafa2d4f by Jonathan M Davis: Fix Bugzilla issue 24704: Improve error messages for from*String in std.datetime. The documentation for the from*String functions specifies what they accept, and the fromISO*String functions accept ISO or ISO extended strings, but it is technically the case that there are ISO and ISO extended strings which they do not accept (since the standard specifies what the fields should look like for each string but doesn't specify that all fields must be present or that any code parsing them must accept all variants). So, technically, the error messages which have said that the given strings are not valid ISO or ISO extended strings are not necessarily correct. So, this changes the error messages to remove that ambiguity. https://github.com/dlang/phobos/pull/9047