Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable NRT for classes in namespace Ical.Net.Utility #706

Merged
merged 2 commits into from
Jan 26, 2025
Merged

Enable NRT for classes in namespace Ical.Net.Utility #706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f91d5c
Enable NRT for Ical.Net.Utility classes
axunonb Jan 25, 2025
eb7f917
Merge branch 'main' into pr/nrt-for-ical-net-utility
axunonb Jan 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Ical.Net/DataTypes/CalDateTime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,12 +434,12 @@ public CalDateTime ToTimeZone(string? otherTzId)
ZonedDateTime converted;
if (IsFloating)
{
// Make sure, we properly fix the time if it dosen't exist in the target tz.
// Make sure, we properly fix the time if it doesn't exist in the target tz.
converted = DateUtil.ToZonedDateTimeLeniently(Value, otherTzId);
}
else
{
var zonedOriginal = DateUtil.ToZonedDateTimeLeniently(Value, TzId);
var zonedOriginal = DateUtil.ToZonedDateTimeLeniently(Value, TzId!);
converted = zonedOriginal.WithZone(DateUtil.GetZone(otherTzId));
}

Expand Down Expand Up @@ -634,7 +634,7 @@ public int CompareTo(IDateTime? dt)
public string ToString(string? format, IFormatProvider? formatProvider)
{
formatProvider ??= CultureInfo.InvariantCulture;
var dateTimeOffset = DateUtil.ToZonedDateTimeLeniently(Value, _tzId).ToDateTimeOffset();
var dateTimeOffset = DateUtil.ToZonedDateTimeLeniently(Value, _tzId ?? string.Empty).ToDateTimeOffset();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we handle floating separately? Not sure how the method behaves if we pass an empty string.


// Use the .NET format options to format the DateTimeOffset
var tzIdString = _tzId is not null ? $" {_tzId}" : string.Empty;
Expand Down
4 changes: 2 additions & 2 deletions Ical.Net/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
<PackageIcon>assets/icon.png</PackageIcon>
<PackageReadmeFile>assets/readme.md</PackageReadmeFile>
</PropertyGroup>
<PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<!-- netstandard2.0 does not support NRT -->
<PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0' Or '$(TargetFramework)' == 'netstandard2.1'">
<!-- netstandard2.x does not or not fully support NRT -->
<NoWarn>$(NoWarn);CS8600;CS8601;CS8602;CS8603;CS8604;CS8618;CS8620;CS8714</NoWarn>
</PropertyGroup>
<ItemGroup>
Expand Down
27 changes: 13 additions & 14 deletions Ical.Net/Utility/CollectionHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT license.
//

#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -12,7 +13,7 @@
internal static class CollectionHelpers
{
/// <summary> Commutative, stable, order-independent hashing for collections of collections. </summary>
public static int GetHashCode<T>(IEnumerable<T> collection)
public static int GetHashCode<T>(IEnumerable<T>? collection)
{
unchecked
{
Expand All @@ -23,12 +24,12 @@

return collection
.Where(e => e != null)
.Aggregate(0, (current, element) => current + element.GetHashCode());
.Aggregate(0, (current, element) => current + element?.GetHashCode() ?? 0);
}
}

/// <summary> Commutative, stable, order-independent hashing for collections of collections. </summary>
public static int GetHashCodeForNestedCollection<T>(IEnumerable<IEnumerable<T>> nestedCollection)
public static int GetHashCodeForNestedCollection<T>(IEnumerable<IEnumerable<T>>? nestedCollection)
{
unchecked
{
Expand All @@ -40,23 +41,18 @@
return nestedCollection
.SelectMany(c => c)
.Where(e => e != null)
.Aggregate(0, (current, element) => current + element.GetHashCode());
.Aggregate(0, (current, element) => current + element?.GetHashCode() ?? 0);
}
}

public static bool Equals<T>(IEnumerable<T> left, IEnumerable<T> right, bool orderSignificant = false)
public static bool Equals<T>(IEnumerable<T>? left, IEnumerable<T>? right, bool orderSignificant = false)
{
if (ReferenceEquals(left, right))
{
return true;
}

if (left == null && right != null)
{
return false;
}

if (left != null && right == null)
if (left == null || right == null)
{
return false;
}
Expand All @@ -66,18 +62,21 @@
return left.SequenceEqual(right);
}

// No multiple enumerations
var rightArray = right as T[] ?? right.ToArray();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eager iteration would only help if right/left would be generators, which doesn't seem to be the case in any of today's uses, otherwise it would rather add overhead.

var leftArray = left as T[] ?? left.ToArray();
try
{
//Many things have natural IComparers defined, but some don't, because no natural comparer exists
return left.OrderBy(l => l).SequenceEqual(right.OrderBy(r => r));
return leftArray.OrderBy(l => l).SequenceEqual(rightArray.OrderBy(r => r));
}
catch (Exception)
{
//It's not possible to sort some collections of things (like Calendars) in any meaningful way. Properties can be null, and there's no natural
//ordering for the contents therein. In cases like that, the best we can do is treat them like sets, and compare them. We don't maintain
//fidelity with respect to duplicates, but it seems better than doing nothing
var leftSet = new HashSet<T>(left);
var rightSet = new HashSet<T>(right);
var leftSet = new HashSet<T>(leftArray);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this whole way of comparing isn't too helpful. We discussed this in another ticket, that Equals might not be too helpful for many of the types, e.g. Calendar. But that's a topic for a different PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I wanted to stick to NRT

var rightSet = new HashSet<T>(rightArray);
return leftSet.SetEquals(rightSet);
}
}
Expand Down Expand Up @@ -262,7 +261,7 @@

foreach (var item in items)
{
if (first || !comparer.Equals(prev, item))

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.

Check warning on line 264 in Ical.Net/Utility/CollectionHelpers.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference argument for parameter 'x' in 'bool IEqualityComparer<T>.Equals(T x, T y)'.
yield return item;

prev = item;
Expand Down
16 changes: 9 additions & 7 deletions Ical.Net/Utility/DateUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT license.
//

#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -23,8 +24,6 @@ public static CalDateTime AsCalDateTime(this DateTime t)

public static DateTime AddWeeks(DateTime dt, int interval, DayOfWeek firstDayOfWeek)
{
// NOTE: fixes WeeklyUntilWkst2() eval.
// NOTE: simplified the execution of this - fixes bug #3119920 - missing weekly occurences also
dt = dt.AddDays(interval * 7);
while (dt.DayOfWeek != firstDayOfWeek)
{
Expand Down Expand Up @@ -63,8 +62,11 @@ private static Dictionary<string, string> InitializeWindowsMappings()
/// <param name="tzId">A BCL, IANA, or serialization time zone identifier</param>
/// <param name="useLocalIfNotFound">If true, this method will return the system local time zone if tzId doesn't match a known time zone identifier.
/// Otherwise, it will throw an exception.</param>
/// <exception cref="ArgumentException">Unrecognized time zone id</exception>
public static DateTimeZone GetZone(string tzId, bool useLocalIfNotFound = true)
{
var exMsg = $"Unrecognized time zone id {tzId}";

if (string.IsNullOrWhiteSpace(tzId))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed the implicit usage of the local time zone in other parts, didn't we? Hope, this is the only one left.

{
return LocalDateTimeZone;
Expand All @@ -83,7 +85,7 @@ public static DateTimeZone GetZone(string tzId, bool useLocalIfNotFound = true)

if (_windowsMapping.Value.TryGetValue(tzId, out var ianaZone))
{
return DateTimeZoneProviders.Tzdb.GetZoneOrNull(ianaZone);
return DateTimeZoneProviders.Tzdb.GetZoneOrNull(ianaZone) ?? throw new ArgumentException(exMsg);
}

zone = NodaTime.Xml.XmlSerializationSettings.DateTimeZoneProvider.GetZoneOrNull(tzId);
Expand All @@ -103,29 +105,29 @@ public static DateTimeZone GetZone(string tzId, bool useLocalIfNotFound = true)
var providerId = DateTimeZoneProviders.Tzdb.Ids.FirstOrDefault(tzId.Contains);
if (providerId != null)
{
return DateTimeZoneProviders.Tzdb.GetZoneOrNull(providerId);
return DateTimeZoneProviders.Tzdb.GetZoneOrNull(providerId) ?? throw new ArgumentException(exMsg);
}

if (_windowsMapping.Value.Keys
.Where(tzId.Contains)
.Any(pId => _windowsMapping.Value.TryGetValue(pId, out ianaZone))
)
{
return DateTimeZoneProviders.Tzdb.GetZoneOrNull(ianaZone);
return DateTimeZoneProviders.Tzdb.GetZoneOrNull(ianaZone!) ?? throw new ArgumentException(exMsg);
}

providerId = NodaTime.Xml.XmlSerializationSettings.DateTimeZoneProvider.Ids.FirstOrDefault(tzId.Contains);
if (providerId != null)
{
return NodaTime.Xml.XmlSerializationSettings.DateTimeZoneProvider.GetZoneOrNull(providerId);
return NodaTime.Xml.XmlSerializationSettings.DateTimeZoneProvider.GetZoneOrNull(providerId) ?? throw new ArgumentException(exMsg);
}

if (useLocalIfNotFound)
{
return LocalDateTimeZone;
}

throw new ArgumentException($"Unrecognized time zone id {tzId}");
throw new ArgumentException(exMsg);
}

public static ZonedDateTime AddYears(ZonedDateTime zonedDateTime, int years)
Expand Down
1 change: 1 addition & 0 deletions Ical.Net/Utility/TextUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT license.
//

#nullable enable
using System;
using System.Collections.Generic;
using System.IO;
Expand Down
Loading