Microsoft.Extensions.Http.Resilience is raising trim/AOT warnings in published app

[eerhardt]

When When analyzing the Microsoft.Extensions.Http.Resilience library using the AOT IlcCompiler, we get the following warnings:

    ILC : Trim analysis error IL2026: Microsoft.Extensions.Http.Resilience.WeightedGroupsRoutingOptions.Groups: Using member 'System.ComponentModel.DataAnnotations.LengthAttribute.LengthAttribute(Int32,Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved. [D:\git\extensions\test\Libraries\Microsoft.Extensions.AotCompatibility.TestApp\Microsoft.Extensions.AotCompatibility.TestApp.csproj]
    ILC : Trim analysis error IL2026: Microsoft.Extensions.Http.Resilience.WeightedGroupsRoutingOptions.Groups: Using member 'System.ComponentModel.DataAnnotations.LengthAttribute.LengthAttribute(Int32,Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved. [D:\git\extensions\test\Libraries\Microsoft.Extensions.AotCompatibility.TestApp\Microsoft.Extensions.AotCompatibility.TestApp.csproj]
    ILC : Trim analysis error IL2026: Microsoft.Extensions.Http.Resilience.UriEndpointGroup.Endpoints: Using member 'System.ComponentModel.DataAnnotations.LengthAttribute.LengthAttribute(Int32,Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved. [D:\git\extensions\test\Libraries\Microsoft.Extensions.AotCompatibility.TestApp\Microsoft.Extensions.AotCompatibility.TestApp.csproj]
    ILC : Trim analysis error IL2026: Microsoft.Extensions.Http.Resilience.UriEndpointGroup.Endpoints: Using member 'System.ComponentModel.DataAnnotations.LengthAttribute.LengthAttribute(Int32,Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved. [D:\git\extensions\test\Libraries\Microsoft.Extensions.AotCompatibility.TestApp\Microsoft.Extensions.AotCompatibility.TestApp.csproj]
    ILC : Trim analysis error IL2026: Microsoft.Extensions.Http.Resilience.OrderedGroupsRoutingOptions.Groups: Using member 'System.ComponentModel.DataAnnotations.LengthAttribute.LengthAttribute(Int32,Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved. [D:\git\extensions\test\Libraries\Microsoft.Extensions.AotCompatibility.TestApp\Microsoft.Extensions.AotCompatibility.TestApp.csproj]
    ILC : Trim analysis error IL2026: Microsoft.Extensions.Http.Resilience.OrderedGroupsRoutingOptions.Groups: Using member 'System.ComponentModel.DataAnnotations.LengthAttribute.LengthAttribute(Int32,Int32)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Uses reflection to get the 'Count' property on types that don't implement ICollection. This 'Count' property may be trimmed. Ensure it is preserved. [D:\git\extensions\test\Libraries\Microsoft.Extensions.AotCompatibility.TestApp\Microsoft.Extensions.AotCompatibility.TestApp.csproj]

These warnings are currently being suppressed from the Roslyn analyzers using #pragma:

extensions/src/Libraries/Microsoft.Extensions.Http.Resilience/Routing/OrderedGroupsRoutingOptions.cs

Lines 22 to 31 in 3f1f59c

	#pragma warning disable IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code
	[Required]
	#if NET8_0_OR_GREATER
	[System.ComponentModel.DataAnnotations.Length(1, int.MaxValue)]
	#else
	[Microsoft.Shared.Data.Validation.Length(1)]
	#endif
	[ValidateEnumeratedItems]
	public IList<UriEndpointGroup> Groups { get; set; } = new List<UriEndpointGroup>();
	#pragma warning restore IL2026 // Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code

However, suppressioning like this doesn't suppress the warning when used in an actual AOT application.

I believe the suppressions are valid, in these cases, because the property being validated is an ICollection, so reflection won't be used when validating the length.

I think we should reopen #5305 or make some sort of similar change.

cc @martincostello @MichalStrehovsky

[MichalStrehovsky]

I see, so if the type of the property is ICollection, it's contractual that the attribute will not do reflection. Makes sense. #5305 had the justification for the suppression filled out as "TODO" and typically when someone suppresses a warning without knowing why, the suppression is simply illegal. I've seen so many of those that I stopped checking if it could be legal.

[martincostello]

I just put TODO at the time as I didn't know the specifics of why they'd been suppressed with pragmas in the first place (Git history didn't give any clues), so figured someone who reviewed it would know and then I'd fill it in.