Add implicit conversion of DateTime/DateOnly to CalDateTime

[NRG-Drink]

• Add implicit conversion from DateTime to CalDateTime
• Add implicit conversion from DateOnly to CalDateTime

closes #821

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

@@         Coverage Diff         @@
##           main   #822   +/-   ##
===================================
  Coverage    68%    68%           
===================================
  Files       106    106           
  Lines      4209   4211    +2     
  Branches    945    945           
===================================
+ Hits       2842   2844    +2     
  Misses     1039   1039           
  Partials    328    328           

Files with missing lines	Coverage Δ
Ical.Net/DataTypes/CalDateTime.cs	88% <100%> (+<1%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[minichma]

@NRG-Drink The implicit conversion operators have intentionally been removed. The reason is that there is no obvious conversion between the two. E.g. one has a TZ the other hasn't. And then there's the problem with DateTimeKind, which is a complete story of its own. By offering an implicit conversion, it will be much easier for users to incorrectly confuse DateTime and CalDateTime. It might also be worth comparing NodaTime's appraoch. There, no implicit conversion exists between Instant, ZonedDateTime, LocalDateTime, etc. and System.DateTime either.

So I suggest to not introduce an implicit conversion. I'd also discourage an explicit conversion. To simplify things, I could imagine publishing DateUtil.AsCalDateTime(). This would require moving the method to a different, public type, probably introducing a new one like DateTimeExtensions. This would also allow using it with null-coalescing (dt?.AsCalDateTime()). (Maybe even consider renaming the method from AsCalDateTime to ToCalDateTime).

[axunonb]

Referring to the #822 (comment):
There are use cases where implicit operators are great. For CalDateTime vs. DateTime they can be quite confusing.

So what @minichma proposes could be viable option to simplify conversion.
Currently we use the method from DateUtil

public static CalDateTime AsCalDateTime(this DateTime t)
    => new CalDateTime(t);

in our unit tests, and yes, we could add it to the public API e.g. in a static class DateTimeExtensions with

public static CalDateTime ToCalDateTime(this DateTime t)
    => new CalDateTime(t);
public static CalDateTime ToCalDateTime(this DateOnly t)
    => new CalDateTime(t);

and add the corresponding xmldoc form the CalDateTime CTORs. As opposed to implicit operator we can explain how the conversion works.

Is this case it would be good to migrate DateUtil.AsCalDateTime to the new extension method(s).

What do you think?

[NRG-Drink]

Thanks for the comments @minichma @axunonb .
Personally, I'm not a big fan of static util classes, specially when we have a constructor that does the same thing. So it would be a discussion about how to construct an object and there the construction should win imo.

new CalDateTime(dt);
dt.AsCalDateTime();
DateUtil.AsCalDateTime(dt);

A more straight forward way is to use the implicit operator but I can understand the point that it can confuse some users with the time-zone and the exact behavior. It has to be practical for the user, not me 😅

So I suggest to close this PR (#822) and the corresponding issue (#821).
Is that okay or would you like to implement the extension-property way?