From 2dff7aec4fa7751a3dfd2aa2bc3c933e04b0aaa6 Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Thu, 28 Aug 2025 18:06:56 -0400 Subject: [PATCH 1/5] Narrow AdvancedCollectionView trimming warning so that it only triggers in unsafe usages Fixes #723 --- .../src/AdvancedCollectionView/AdvancedCollectionView.cs | 5 ++--- .../src/AdvancedCollectionView/SortDescription.cs | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs b/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs index 416d87f8..246f451a 100644 --- a/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs +++ b/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs @@ -13,9 +13,6 @@ namespace CommunityToolkit.WinUI.Collections; /// /// A collection view implementation that supports filtering, sorting and incremental loading /// -#if NET8_0_OR_GREATER -[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Item sorting uses reflection to get property types and may not be AOT compatible.")] -#endif public partial class AdvancedCollectionView : IAdvancedCollectionView, INotifyPropertyChanged, ISupportIncrementalLoading, IComparer { private readonly List _view; @@ -380,6 +377,8 @@ public Predicate Filter /// Object B /// Comparison value #pragma warning disable CA1033 // Interface methods should be callable by child types + [System.Diagnostics.CodeAnalysis.SuppressMessage("Trimming", "IL2065:The method has a DynamicallyAccessedMembersAttribute (which applies to the implicit 'this' parameter), but the value used for the 'this' parameter can not be statically analyzed.", Justification = "Trimmer warnings are surfaced to the user with SortDescription")] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Trimming", "IL2075:'this' argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.", Justification = "Trimmer warnings are surfaced to the user with SortDescription")] int IComparer.Compare(object x, object y) #pragma warning restore CA1033 // Interface methods should be callable by child types { diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription.cs b/components/Collections/src/AdvancedCollectionView/SortDescription.cs index abc6b83a..47bb734f 100644 --- a/components/Collections/src/AdvancedCollectionView/SortDescription.cs +++ b/components/Collections/src/AdvancedCollectionView/SortDescription.cs @@ -32,6 +32,7 @@ public class SortDescription /// /// Direction of sort /// Comparer to use. If null, will use default comparer + [System.Diagnostics.CodeAnalysis.SuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "Not using property name")] public SortDescription(SortDirection direction, IComparer? comparer = null) : this(null!, direction, comparer!) { @@ -43,6 +44,9 @@ public SortDescription(SortDirection direction, IComparer? comparer = null) /// Name of property to sort on /// Direction of sort /// Comparer to use. If null, will use default comparer +#if NET8_0_OR_GREATER + [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Item sorting with the property name uses reflection to get the property and is not trim-safe. Either use SortDescription to preserve the required metadata, or use the other constructor without a property name.")] +#endif public SortDescription(string propertyName, SortDirection direction, IComparer? comparer = null) { PropertyName = propertyName; From 68766f736cc168e381581efd8e9f5c09ec44b25f Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Thu, 28 Aug 2025 18:13:37 -0400 Subject: [PATCH 2/5] Add SortDescription --- .../SortDescription{T}.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs new file mode 100644 index 00000000..b1cf3192 --- /dev/null +++ b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs @@ -0,0 +1,26 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections; +using System.Diagnostics.CodeAnalysis; + +namespace CommunityToolkit.WinUI.Collections; + +/// +/// A generic version of which preserves the required metadata for reflection-based sorting. +/// +/// The type to sort +public class SortDescription<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> : SortDescription +{ + /// + /// Initializes a new instance of the class. + /// + /// Name of property to sort on + /// Direction of sort + /// Comparer to use. If null, will use default comparer + [SuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "This class preserves metadata")] + public SortDescription(string propertyName, SortDirection direction, IComparer? comparer = null) : base(propertyName, direction, comparer) + { + } +} From c19cac2af552048bafd081a4b5a0e5b31b88a77d Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Thu, 28 Aug 2025 19:58:27 -0400 Subject: [PATCH 3/5] Gate DynamicallyAccessedMembers to .NET 5 or greater, to allow .NET Native usage --- .../src/AdvancedCollectionView/SortDescription.cs | 2 +- .../src/AdvancedCollectionView/SortDescription{T}.cs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription.cs b/components/Collections/src/AdvancedCollectionView/SortDescription.cs index 47bb734f..e2bebd4d 100644 --- a/components/Collections/src/AdvancedCollectionView/SortDescription.cs +++ b/components/Collections/src/AdvancedCollectionView/SortDescription.cs @@ -44,7 +44,7 @@ public SortDescription(SortDirection direction, IComparer? comparer = null) /// Name of property to sort on /// Direction of sort /// Comparer to use. If null, will use default comparer -#if NET8_0_OR_GREATER +#if NET5_0_OR_GREATER [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Item sorting with the property name uses reflection to get the property and is not trim-safe. Either use SortDescription to preserve the required metadata, or use the other constructor without a property name.")] #endif public SortDescription(string propertyName, SortDirection direction, IComparer? comparer = null) diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs index b1cf3192..b97847ad 100644 --- a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs +++ b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs @@ -11,7 +11,11 @@ namespace CommunityToolkit.WinUI.Collections; /// A generic version of which preserves the required metadata for reflection-based sorting. /// /// The type to sort -public class SortDescription<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T> : SortDescription +public class SortDescription< +#if NET5_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] +#endif + T> : SortDescription { /// /// Initializes a new instance of the class. From ed2ef3b915035752a5c4c276d67aa1e94b3ba588 Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Fri, 24 Oct 2025 22:55:09 -0400 Subject: [PATCH 4/5] Reduce suppressions to only GetProperty method implementation --- .../samples/AdvancedCollectionViewSample.xaml.cs | 2 +- .../AdvancedCollectionView.cs | 10 ++++------ .../AdvancedCollectionView/SortDescription.cs | 16 ++++++++++++---- .../AdvancedCollectionView/SortDescription{T}.cs | 12 ++++++++++-- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/components/Collections/samples/AdvancedCollectionViewSample.xaml.cs b/components/Collections/samples/AdvancedCollectionViewSample.xaml.cs index 8dfb75e6..932ea5b7 100644 --- a/components/Collections/samples/AdvancedCollectionViewSample.xaml.cs +++ b/components/Collections/samples/AdvancedCollectionViewSample.xaml.cs @@ -48,7 +48,7 @@ private void Setup() // right list AdvancedCollectionView acv = new(EmployeeCollection); acv.Filter = x => !int.TryParse(((Employee)x).Name, out _); - acv.SortDescriptions.Add(new(nameof(Employee.Name), SortDirection.Ascending)); + acv.SortDescriptions.Add(new SortDescription(nameof(Employee.Name), SortDirection.Ascending)); CollectionView = acv; } diff --git a/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs b/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs index 246f451a..b889c35c 100644 --- a/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs +++ b/components/Collections/src/AdvancedCollectionView/AdvancedCollectionView.cs @@ -377,12 +377,10 @@ public Predicate Filter /// Object B /// Comparison value #pragma warning disable CA1033 // Interface methods should be callable by child types - [System.Diagnostics.CodeAnalysis.SuppressMessage("Trimming", "IL2065:The method has a DynamicallyAccessedMembersAttribute (which applies to the implicit 'this' parameter), but the value used for the 'this' parameter can not be statically analyzed.", Justification = "Trimmer warnings are surfaced to the user with SortDescription")] - [System.Diagnostics.CodeAnalysis.SuppressMessage("Trimming", "IL2075:'this' argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.", Justification = "Trimmer warnings are surfaced to the user with SortDescription")] int IComparer.Compare(object x, object y) #pragma warning restore CA1033 // Interface methods should be callable by child types { - if (!_sortProperties.Any()) + if (_sortProperties.Count == 0) { var listType = _source?.GetType(); Type type; @@ -400,7 +398,7 @@ int IComparer.Compare(object x, object y) { if (!string.IsNullOrEmpty(sd.PropertyName)) { - _sortProperties[sd.PropertyName] = type.GetProperty(sd.PropertyName); + _sortProperties[sd.PropertyName] = sd.GetProperty(type); } } } @@ -418,8 +416,8 @@ int IComparer.Compare(object x, object y) { var pi = _sortProperties[sd.PropertyName]; - cx = pi.GetValue(x!); - cy = pi.GetValue(y!); + cx = pi.GetValue(x); + cy = pi.GetValue(y); } var cmp = sd.Comparer.Compare(cx, cy); diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription.cs b/components/Collections/src/AdvancedCollectionView/SortDescription.cs index e2bebd4d..18e0bb46 100644 --- a/components/Collections/src/AdvancedCollectionView/SortDescription.cs +++ b/components/Collections/src/AdvancedCollectionView/SortDescription.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections; +using System.Diagnostics.CodeAnalysis; namespace CommunityToolkit.WinUI.Collections; @@ -14,7 +15,7 @@ public class SortDescription /// /// Gets the name of property to sort on /// - public string PropertyName { get; } + public virtual string? PropertyName { get; } /// /// Gets the direction of sort @@ -32,10 +33,11 @@ public class SortDescription /// /// Direction of sort /// Comparer to use. If null, will use default comparer - [System.Diagnostics.CodeAnalysis.SuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "Not using property name")] public SortDescription(SortDirection direction, IComparer? comparer = null) - : this(null!, direction, comparer!) { + PropertyName = null; + Direction = direction; + Comparer = comparer ?? ObjectComparer.Instance; } /// @@ -45,7 +47,7 @@ public SortDescription(SortDirection direction, IComparer? comparer = null) /// Direction of sort /// Comparer to use. If null, will use default comparer #if NET5_0_OR_GREATER - [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Item sorting with the property name uses reflection to get the property and is not trim-safe. Either use SortDescription to preserve the required metadata, or use the other constructor without a property name.")] + [RequiresUnreferencedCode("Item sorting with the property name uses reflection to get the property and is not trim-safe. Either use SortDescription to preserve the required metadata, or use the other constructor without a property name.")] #endif public SortDescription(string propertyName, SortDirection direction, IComparer? comparer = null) { @@ -54,6 +56,12 @@ public SortDescription(string propertyName, SortDirection direction, IComparer? Comparer = comparer ?? ObjectComparer.Instance; } + + [UnconditionalSuppressMessage("Trimming", "IL2070:'this' argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The parameter of method does not have matching annotations.", + Justification = "The path which does reflection is only triggered if the user uses the constructor with RequiresUnreferencedCode (which bubbles the warning to them)")] + internal virtual PropertyInfo? GetProperty(Type type) + => PropertyName != null ? type.GetProperty(PropertyName) : null; + private class ObjectComparer : IComparer { public static readonly IComparer Instance = new ObjectComparer(); diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs index b97847ad..acc5996a 100644 --- a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs +++ b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs @@ -17,14 +17,22 @@ public class SortDescription< #endif T> : SortDescription { + private readonly PropertyInfo _prop; + + /// + public override string PropertyName => _prop.Name; + /// /// Initializes a new instance of the class. /// /// Name of property to sort on /// Direction of sort /// Comparer to use. If null, will use default comparer - [SuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "This class preserves metadata")] - public SortDescription(string propertyName, SortDirection direction, IComparer? comparer = null) : base(propertyName, direction, comparer) + public SortDescription(string propertyName, SortDirection direction, IComparer? comparer = null) : base(direction, comparer) { + _prop = typeof(T).GetProperty(propertyName) ?? throw new ArgumentException("Type does not have the expected property"); } + + override internal PropertyInfo? GetProperty(Type type) => + type.IsAssignableTo(_prop.DeclaringType) ? _prop : throw new ArgumentException("This SortDescription is not compatible with this type"); } From fc613e91f5aa059278b4792b95f3596dd6be8b0a Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Fri, 24 Oct 2025 22:58:32 -0400 Subject: [PATCH 5/5] Style fix --- .../src/AdvancedCollectionView/SortDescription{T}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs index acc5996a..ea65f3d4 100644 --- a/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs +++ b/components/Collections/src/AdvancedCollectionView/SortDescription{T}.cs @@ -33,6 +33,6 @@ public SortDescription(string propertyName, SortDirection direction, IComparer? _prop = typeof(T).GetProperty(propertyName) ?? throw new ArgumentException("Type does not have the expected property"); } - override internal PropertyInfo? GetProperty(Type type) => + internal override PropertyInfo? GetProperty(Type type) => type.IsAssignableTo(_prop.DeclaringType) ? _prop : throw new ArgumentException("This SortDescription is not compatible with this type"); }