diff --git a/.editorconfig b/.editorconfig index 1ee1b1ee..e72ea925 100644 --- a/.editorconfig +++ b/.editorconfig @@ -84,7 +84,7 @@ csharp_prefer_braces = false:suggestion #Style - expression bodied member options # Expression-bodied members -csharp_style_expression_bodied_methods = false:suggestion +csharp_style_expression_bodied_methods = when_on_single_line:suggestion csharp_style_expression_bodied_properties = true csharp_style_expression_bodied_properties = when_on_single_line:suggestion csharp_style_expression_bodied_indexers = false:warning diff --git a/Ical.Net.Tests/CalDateTimeTests.cs b/Ical.Net.Tests/CalDateTimeTests.cs index 5a35ebb4..2fef5491 100644 --- a/Ical.Net.Tests/CalDateTimeTests.cs +++ b/Ical.Net.Tests/CalDateTimeTests.cs @@ -234,20 +234,20 @@ public void EqualityShouldBeTransitive() } - [Test, TestCaseSource(nameof(DateOnlyArithmeticTestCases))] - public (DateTime Value, bool HasTime) DateOnlyArithmeticTests(Func operation) + [Test, TestCaseSource(nameof(DateOnlyValidArithmeticTestCases))] + public (DateTime Value, bool HasTime) DateOnlyValidArithmeticTests(Func operation) { var result = operation(new CalDateTime(2025, 1, 15)); return (result.Value, result.HasTime); } - public static IEnumerable DateOnlyArithmeticTestCases() + public static IEnumerable DateOnlyValidArithmeticTestCases() { var dateTime = new DateTime(2025, 1, 15); yield return new TestCaseData(new Func(dt => dt.Subtract(TimeSpan.FromDays(1)))) .Returns((dateTime.AddDays(-1), false)) - .SetName($"{nameof(IDateTime.Subtract)} 1 day TimeSpan HasTime=false"); + .SetName($"{nameof(IDateTime.Subtract)} 1 day TimeSpan"); yield return new TestCaseData(new Func(dt => dt.AddYears(1))) .Returns((dateTime.AddYears(1), false)) @@ -263,31 +263,37 @@ public static IEnumerable DateOnlyArithmeticTestCases() yield return new TestCaseData(new Func(dt => dt.AddHours(24))) .Returns((dateTime.AddHours(24), false)) - .SetName($"{nameof(IDateTime.AddHours)} 24 hours HasTime=false"); - - yield return new TestCaseData(new Func(dt => dt.AddHours(1))) - .Returns((dateTime.AddHours(1), true)) - .SetName($"{nameof(IDateTime.AddHours)} 1 hour HasTime=true"); + .SetName($"{nameof(IDateTime.AddHours)} 24 hours"); yield return new TestCaseData(new Func(dt => dt.AddMinutes(24 * 60))) .Returns((dateTime.AddMinutes(24 * 60), false)) - .SetName($"{nameof(IDateTime.AddMinutes)} 1 day in minutes HasTime=false"); - - yield return new TestCaseData(new Func(dt => dt.AddMinutes(23 * 60))) - .Returns((dateTime.AddMinutes(23 * 60), true)) - .SetName($"{nameof(IDateTime.AddMinutes)} 23 hours in minutes HasTime=true"); + .SetName($"{nameof(IDateTime.AddMinutes)} 1 day in minutes"); yield return new TestCaseData(new Func(dt => dt.AddSeconds(TimeSpan.FromDays(1).Seconds))) .Returns((dateTime.AddSeconds(TimeSpan.FromDays(1).Seconds), false)) - .SetName($"{nameof(IDateTime.AddSeconds)} 1 day in seconds HasTime=false"); + .SetName($"{nameof(IDateTime.AddSeconds)} 1 day in seconds"); yield return new TestCaseData(new Func(dt => dt.Add(TimeSpan.FromDays(1)))) .Returns((dateTime.Add(TimeSpan.FromDays(1)), false)) - .SetName($"{nameof(IDateTime.Add)} 1 day TimeSpan HasTime=false"); + .SetName($"{nameof(IDateTime.Add)} 1 day TimeSpan"); - yield return new TestCaseData(new Func(dt => dt.Add(TimeSpan.FromSeconds(30)))) - .Returns((dateTime.Add(TimeSpan.FromSeconds(30)), true)) - .SetName($"{nameof(IDateTime.Add)} 30 seconds TimeSpan HasTime=true"); + yield return new TestCaseData(new Func(dt => dt.Add(TimeSpan.Zero))) + .Returns((dateTime.Add(TimeSpan.Zero), false)) + .SetName($"{nameof(IDateTime.Add)} TimeSpan.Zero"); + } + + [Test] + public void DateOnlyInvalidArithmeticTests() + { + var dt = new CalDateTime(2025, 1, 15); + + Assert.Multiple(() => + { + Assert.That(() => dt.Add(TimeSpan.FromHours(1)), Throws.TypeOf()); + Assert.That(() => dt.AddHours(2), Throws.TypeOf()); + Assert.That(() => dt.AddMinutes(3), Throws.TypeOf()); + Assert.That(() => dt.AddSeconds(4), Throws.TypeOf()); + }); } [Test] @@ -316,4 +322,26 @@ public void Simple_PropertyAndMethod_HasTime_Tests() Assert.That(new CalDateTime(new CalDateTime(2025, 1, 1)), Is.EqualTo(new CalDateTime(2025, 1, 1))); }); } + + public static IEnumerable AddAndSubtractTestCases() + { + yield return new TestCaseData(new CalDateTime(2024, 10, 27, 0, 0, 0, tzId: null), TimeSpan.FromHours(4)) + .SetName("Floating"); + + yield return new TestCaseData(new CalDateTime(2024, 10, 27, 0, 0, 0, tzId: CalDateTime.UtcTzId), TimeSpan.FromHours(4)) + .SetName("UTC"); + + yield return new TestCaseData(new CalDateTime(2024, 10, 27, 0, 0, 0, tzId: "Europe/Paris"), TimeSpan.FromHours(4)) + .SetName("Zoned Date/Time with DST change"); + } + + [Test, TestCaseSource(nameof(AddAndSubtractTestCases))] + public void AddAndSubtract_ShouldBeReversible(CalDateTime t, TimeSpan d) + { + Assert.Multiple(() => + { + Assert.That(t.Add(d).Subtract(d), Is.EqualTo(t)); + Assert.That(t.Add(d).Subtract(t), Is.EqualTo(d)); + }); + } } diff --git a/Ical.Net/DataTypes/CalDateTime.cs b/Ical.Net/DataTypes/CalDateTime.cs index 6a23c15d..771d0033 100644 --- a/Ical.Net/DataTypes/CalDateTime.cs +++ b/Ical.Net/DataTypes/CalDateTime.cs @@ -222,30 +222,24 @@ public override ICalendarObject? AssociatedObject /// public override void CopyFrom(ICopyable obj) { - base.CopyFrom(obj); - - if (obj is not IDateTime dt) - { + if (obj is not CalDateTime calDt) return; - } - if (dt is CalDateTime calDt) - { - // Maintain the private date/time backing fields - _dateOnly = calDt._dateOnly; - _timeOnly = TruncateTimeToSeconds(calDt._timeOnly); - _tzId = calDt._tzId; - } + base.CopyFrom(obj); - AssociateWith(dt); + // Maintain the private date/time backing fields + _dateOnly = calDt._dateOnly; + _timeOnly = TruncateTimeToSeconds(calDt._timeOnly); + _tzId = calDt._tzId; + + AssociateWith(calDt); } - public bool Equals(CalDateTime other) - => this == other; + public bool Equals(CalDateTime other) => this == other; /// public override bool Equals(object? obj) - => obj is IDateTime && (CalDateTime)obj == this; + => obj is CalDateTime other && this == other; /// public override int GetHashCode() @@ -261,25 +255,33 @@ public override int GetHashCode() } public static bool operator <(CalDateTime? left, IDateTime? right) - => left != null - && right != null - && (left.IsFloating || right.IsFloating ? left.Value < right.Value : left.AsUtc < right.AsUtc); + { + return left != null + && right != null + && ((left.IsFloating || right.IsFloating) ? left.Value < right.Value : left.AsUtc < right.AsUtc); + } public static bool operator >(CalDateTime? left, IDateTime? right) - => left != null - && right != null - && (left.IsFloating || right.IsFloating ? left.Value > right.Value : left.AsUtc > right.AsUtc); - + { + return left != null + && right != null + && ((left.IsFloating || right.IsFloating) ? left.Value > right.Value : left.AsUtc > right.AsUtc); + } + public static bool operator <=(CalDateTime? left, IDateTime? right) - => left != null - && right != null - && (left.IsFloating || right.IsFloating ? left.Value <= right.Value : left.AsUtc <= right.AsUtc); + { + return left != null + && right != null + && ((left.IsFloating || right.IsFloating) ? left.Value <= right.Value : left.AsUtc <= right.AsUtc); + } public static bool operator >=(CalDateTime? left, IDateTime? right) - => left != null - && right != null - && (left.IsFloating || right.IsFloating ? left.Value >= right.Value : left.AsUtc >= right.AsUtc); - + { + return left != null + && right != null + && ((left.IsFloating || right.IsFloating) ? left.Value >= right.Value : left.AsUtc >= right.AsUtc); + } + public static bool operator ==(CalDateTime? left, IDateTime? right) { if (ReferenceEquals(left, right)) @@ -303,60 +305,19 @@ public override int GetHashCode() } public static bool operator !=(CalDateTime? left, IDateTime? right) - => !(left == right); - - /// - /// Subtracts a from the . - /// - /// - /// This will also add a part that did not exist before the operation, - /// if the is not a multiple of 24 hours. - /// - public static IDateTime operator -(CalDateTime left, TimeSpan right) { - var copy = left.Copy(); - var newValue = copy.Value - right; - if (!copy.HasTime && (right.Ticks % TimeSpan.TicksPerDay) == 0) - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - } - else - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - copy._timeOnly = TruncateTimeToSeconds(newValue); - } - return copy; + return !(left == right); } /// - /// Adds a to the . + /// Creates a new instance of with for /// - /// - /// This will also add a part that did not exist before the operation, - /// if the is not a multiple of 24 hours. - /// - public static IDateTime operator +(CalDateTime left, TimeSpan right) + public static implicit operator CalDateTime(DateTime left) { - var copy = left.Copy(); - var newValue = copy.Value + right; - if (!copy.HasTime && (right.Ticks % TimeSpan.TicksPerDay) == 0) - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - } - else - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - copy._timeOnly = TruncateTimeToSeconds(newValue); - } - return copy; + return new CalDateTime(left); } - /// - /// Creates a new instance of with for - /// - public static implicit operator CalDateTime(DateTime left) => new CalDateTime(left); - -/// + /// public DateTime AsUtc => DateTime.SpecifyKind(ToTimeZone(UtcTzId).Value, DateTimeKind.Utc); /// @@ -364,7 +325,6 @@ public DateTime Value { get { - // HasDate and HasTime both have setters, so they can be changed. if (_timeOnly.HasValue) { return new DateTime(_dateOnly.Year, _dateOnly.Month, @@ -393,9 +353,9 @@ public DateTime Value private string? _tzId; /// - /// Gets the IANA timezone ID of this instance. + /// Gets the timezone ID of this instance. /// It can be for Coordinated Universal Time, - /// or for a floating date/time, or value for a specific timezone. + /// or for a floating date/time, or a value for a specific timezone. /// public string? TzId => _tzId; @@ -434,7 +394,7 @@ public DateTime Value public TimeOnly? Time => _timeOnly; /// - /// Any values are always rounded to the nearest second. + /// Any values are truncated to seconds, because /// RFC 5545, Section 3.3.5 does not allow for fractional seconds. /// private static TimeOnly? TruncateTimeToSeconds(TimeOnly? time) @@ -448,13 +408,10 @@ public DateTime Value } /// - /// Any values are always rounded to the nearest second. + /// Any values are truncated to seconds, because /// RFC 5545, Section 3.3.5 does not allow for fractional seconds. /// - private static TimeOnly? TruncateTimeToSeconds(DateTime dateTime) - { - return new TimeOnly(dateTime.Hour, dateTime.Minute, dateTime.Second); - } + private static TimeOnly? TruncateTimeToSeconds(DateTime dateTime) => new TimeOnly(dateTime.Hour, dateTime.Minute, dateTime.Second); /// public IDateTime ToTimeZone(string otherTzId) @@ -470,20 +427,40 @@ public IDateTime ToTimeZone(string otherTzId) } /// - /// - /// This will also add a part that did not exist before the operation, - /// if the hours are not a multiple of 24. - /// - public IDateTime Add(TimeSpan ts) => this + ts; + /// + /// Thrown when attempting to add a time span to a date-only instance, + /// and the time span is not a multiple of full days. + /// + public IDateTime Add(TimeSpan ts) + { + if (!HasTime && (ts.Ticks % TimeSpan.TicksPerDay) != 0) + { + throw new InvalidOperationException("This instance represents a 'date-only' value. Only multiples of full days can be added to a 'date-only' instance."); + } + + // Ensure to handle DST transitions correctly when timezones are involved. + var newDateTime = TzId is null ? Value.Add(ts) : AsUtc.Add(ts); + + var copy = Copy(); + copy._dateOnly = DateOnly.FromDateTime(newDateTime); + copy._timeOnly = HasTime ? TruncateTimeToSeconds(newDateTime) : null; + copy._tzId = TzId is null ? null : UtcTzId; + + return TzId is null ? copy : copy.ToTimeZone(TzId); + } /// Returns a new from subtracting the specified from to the value of this instance. /// - public TimeSpan Subtract(IDateTime dt) => (AsUtc - dt.AsUtc)!; + public TimeSpan Subtract(IDateTime dt) => AsUtc - dt.AsUtc; /// Returns a new by subtracting the specified from the value of this instance. /// An interval. /// An object whose value is the difference of the date and time represented by this instance and the time interval represented by . - public IDateTime Subtract(TimeSpan ts) => this - ts; + /// + /// Thrown when attempting to subtracting a time span from a date-only instance, + /// and the time span is not a multiple of full days. + /// + public IDateTime Subtract(TimeSpan ts) => Add(-ts); /// public IDateTime AddYears(int years) @@ -510,67 +487,25 @@ public IDateTime AddDays(int days) } /// - /// - /// This will also add a part that did not exist before the operation, - /// if the hours are not a multiple of 24. - /// - public IDateTime AddHours(int hours) - { - var copy = Copy(); - var newValue = copy.Value.AddHours(hours); - if (!copy.HasTime && (hours % 24 == 0)) - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - } - else - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - copy._timeOnly = TruncateTimeToSeconds(newValue); - } - return copy; - } + /// + /// Thrown when attempting to add a time span to a date-only instance, + /// and the time span is not a multiple of full days. + /// + public IDateTime AddHours(int hours) => Add(TimeSpan.FromHours(hours)); /// - /// - /// This will also add a part that did not exist before the operation, - /// if the minutes are not a multiple of 1440. - /// - public IDateTime AddMinutes(int minutes) - { - var copy = Copy(); - var newValue = copy.Value.AddMinutes(minutes); - if (!copy.HasTime && (minutes % 1440 == 0)) - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - } - else - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - copy._timeOnly = TruncateTimeToSeconds(newValue); - } - return copy; - } + /// + /// Thrown when attempting to add a time span to a date-only instance, + /// and the time span is not a multiple of full days. + /// + public IDateTime AddMinutes(int minutes) => Add(TimeSpan.FromMinutes(minutes)); /// - /// - /// This will also add a part that did not exist before the operation, - /// if the seconds are not a multiple of 86400. - /// - public IDateTime AddSeconds(int seconds) - { - var copy = Copy(); - var newValue = copy.Value.AddSeconds(seconds); - if (!copy.HasTime && (seconds % 86400 == 0)) - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - } - else - { - copy._dateOnly = DateOnly.FromDateTime(newValue); - copy._timeOnly = TruncateTimeToSeconds(newValue); - } - return copy; - } + /// + /// Thrown when attempting to add a time span to a date-only instance, + /// and the time span is not a multiple of full days. + /// + public IDateTime AddSeconds(int seconds) => Add(TimeSpan.FromSeconds(seconds)); /// /// Returns if the current instance is less than .