DbConnectionStringBuilder annotations force invalid suppressions on library authors #121718
Description
DbConnectionStringBuilder has DynamicallyAccessedMembers on the type:
which says that it's trim-compatible to reflect over the specified members of DbConnectionStringBuilder or derived types. But at the same item, several members are marked as RequiresUnreferencedCode:
The suppression on GetProperties is invalid because it allows:
new DbConnectionStringBuilder().GetType().GetMethods(...)which is reflecting over RUC methods but doesn't produce trim warnings. The annotations also lead to unactionable trim warnings:
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using System.Collections;
using System.ComponentModel.DataAnnotations;
using System.Data.Common;
using System.Diagnostics.CodeAnalysis;
var db = new MyStringBuilder();
new ServiceCollection()
.Configure<MySettings>(settings =>
{
settings.Option = true;
})
.AddSingleton<IValidateOptions<MySettings>, MyValidator>()
.AddSingleton<SettingsHelper>()
.BuildServiceProvider();
public class MySettings
{
[Required]
public bool Option { get; set; }
}
class SettingsHelper
{
private readonly IOptions<MySettings> _options;
public SettingsHelper(IOptions<MySettings> options)
{
_options = options;
}
}
[OptionsValidator]
public partial class MyValidator : IValidateOptions<MySettings>
{
}
class MyStringBuilder : DbConnectionStringBuilder
{
[RequiresUnreferencedCode("GetProperties")]
protected override void GetProperties(Hashtable propertyDescriptors)
{
base.GetProperties(propertyDescriptors);
}
}This produces a trim warning from MyStringBuilder.GetProperties, but only when both MyStringBuilder and the options validation logic are referenced from the app (because the options validation logic internally uses ICustomTypeDescriptor.GetProperties):
warning IL2112: 'DynamicallyAccessedMembersAttribute' on 'MyStringBuilder' or one of its base types references 'MyStringBuilder.GetProperties(Hashtable)' which requires unreferenced code. GetProperties.
I'm opening this issue because I have seen this cause confusion in various places, and I think it's worth writing down our recommendations. Given the way we have annotated DbConnectiontStringBuilder, I think the current recommendation is for all derived types to suppress the warnings from the derived members (so add UnconditionalSuppressMessage("IL2112", ...) to the GetProperties override in this example). This is not ideal because it forces the library author to add an invalid suppression.