From 3ed847191feb6148ace17725729c7e1e120c0199 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Wed, 26 Jul 2023 15:44:29 -0700 Subject: [PATCH] Flow binder gen diag locations correctly & pick up more bind input patterns --- .../ConfigurationBindingGenerator.Parser.cs | 121 +++++++++---- .../gen/Helpers/Parser/ConfigurationBinder.cs | 9 +- .../OptionsBuilderConfigurationExtensions.cs | 2 +- ...onfigurationServiceCollectionExtensions.cs | 2 +- .../ConfigurationBinderTests.Collections.cs | 2 + .../ConfigurationBinderTests.TestClasses.cs | 31 +++- .../tests/Common/ConfigurationBinderTests.cs | 60 ++++++- ...gurationBindingGeneratorTests.Baselines.cs | 87 +++++----- .../ConfigurationBindingGeneratorTests.cs | 159 ++++++++++++++++-- ...ation.Binder.SourceGeneration.Tests.csproj | 10 +- .../src/JsonConsoleFormatterOptions.cs | 3 + ...icrosoft.Extensions.Logging.Console.csproj | 2 - 12 files changed, 376 insertions(+), 112 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs index 9127046f22c5e..758311958c451 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs @@ -15,14 +15,18 @@ public sealed partial class ConfigurationBindingGenerator : IIncrementalGenerato { private sealed partial class Parser { + private record struct InvocationDiagnosticInfo(DiagnosticDescriptor Descriptor, object[]? MessageArgs); + private readonly SourceProductionContext _context; + private readonly SourceGenerationSpec _sourceGenSpec = new(); private readonly KnownTypeSymbols _typeSymbols; private readonly ImmutableArray _invocations; - private readonly HashSet _unsupportedTypes = new(SymbolEqualityComparer.Default); private readonly Dictionary _createdSpecs = new(SymbolEqualityComparer.Default); + private readonly HashSet _unsupportedTypes = new(SymbolEqualityComparer.Default); - private readonly SourceGenerationSpec _sourceGenSpec = new(); + private readonly List _invocationTargetTypeDiags = new(); + private readonly Dictionary> _typeDiagnostics = new(SymbolEqualityComparer.Default); public Parser(SourceProductionContext context, KnownTypeSymbols typeSymbols, ImmutableArray invocations) { @@ -33,7 +37,7 @@ public Parser(SourceProductionContext context, KnownTypeSymbols typeSymbols, Imm public SourceGenerationSpec? GetSourceGenerationSpec() { - if (_typeSymbols.IConfiguration is null) + if (_typeSymbols is not { IConfiguration: { }, ConfigurationBinder: { } }) { return null; } @@ -78,21 +82,39 @@ type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error || return true; } - private TypeSpec? GetBindingConfigType(ITypeSymbol? type, Location? location) + private TypeSpec? GetTargetTypeForRootInvocation(ITypeSymbol? type, Location? invocationLocation) { if (!IsValidRootConfigType(type)) { - _context.ReportDiagnostic(Diagnostic.Create(Diagnostics.CouldNotDetermineTypeInfo, location)); + _context.ReportDiagnostic(Diagnostic.Create(Diagnostics.CouldNotDetermineTypeInfo, invocationLocation)); return null; } - return GetOrCreateTypeSpec(type, location); + return GetTargetTypeForRootInvocationCore(type, invocationLocation); + } + + public TypeSpec? GetTargetTypeForRootInvocationCore(ITypeSymbol type, Location? invocationLocation) + { + TypeSpec? spec = GetOrCreateTypeSpec(type); + + foreach (InvocationDiagnosticInfo diag in _invocationTargetTypeDiags) + { + _context.ReportDiagnostic(Diagnostic.Create(diag.Descriptor, invocationLocation, diag.MessageArgs)); + } + + _invocationTargetTypeDiags.Clear(); + return spec; } - private TypeSpec? GetOrCreateTypeSpec(ITypeSymbol type, Location? location = null) + private TypeSpec? GetOrCreateTypeSpec(ITypeSymbol type) { if (_createdSpecs.TryGetValue(type, out TypeSpec? spec)) { + if (_typeDiagnostics.TryGetValue(type, out HashSet? typeDiags)) + { + _invocationTargetTypeDiags.AddRange(typeDiags); + } + return spec; } @@ -113,13 +135,13 @@ type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error || spec = stringParsableSpec; } - else if (IsSupportedArrayType(type, location)) + else if (IsSupportedArrayType(type)) { spec = CreateArraySpec((type as IArrayTypeSymbol)); } else if (IsCollection(type)) { - spec = CreateCollectionSpec((INamedTypeSymbol)type, location); + spec = CreateCollectionSpec((INamedTypeSymbol)type); } else if (SymbolEqualityComparer.Default.Equals(type, _typeSymbols.IConfigurationSection)) { @@ -131,12 +153,20 @@ type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error || // an error for config properties that don't map to object properties. _sourceGenSpec.TypeNamespaces.Add("System.Collections.Generic"); - spec = CreateObjectSpec(namedType, location); + spec = CreateObjectSpec(namedType); + } + else + { + RegisterUnsupportedType(type, Diagnostics.TypeNotSupported); + } + + foreach (InvocationDiagnosticInfo diag in _invocationTargetTypeDiags) + { + RegisterTypeDiagnostic(type, diag); } if (spec is null) { - ReportUnsupportedType(type, Diagnostics.TypeNotSupported, location); return null; } @@ -293,7 +323,7 @@ private bool TryGetTypeSpec(ITypeSymbol type, DiagnosticDescriptor descriptor, o if (spec is null) { - ReportUnsupportedType(type, descriptor); + RegisterUnsupportedType(type, descriptor); return false; } @@ -326,7 +356,7 @@ private bool TryGetTypeSpec(ITypeSymbol type, DiagnosticDescriptor descriptor, o return spec; } - private bool IsSupportedArrayType(ITypeSymbol type, Location? location) + private bool IsSupportedArrayType(ITypeSymbol type) { if (type is not IArrayTypeSymbol arrayType) { @@ -335,24 +365,24 @@ private bool IsSupportedArrayType(ITypeSymbol type, Location? location) if (arrayType.Rank > 1) { - ReportUnsupportedType(arrayType, Diagnostics.MultiDimArraysNotSupported, location); + RegisterUnsupportedType(arrayType, Diagnostics.MultiDimArraysNotSupported); return false; } return true; } - private CollectionSpec? CreateCollectionSpec(INamedTypeSymbol type, Location? location) + private CollectionSpec? CreateCollectionSpec(INamedTypeSymbol type) { CollectionSpec? spec; if (IsCandidateDictionary(type, out ITypeSymbol keyType, out ITypeSymbol elementType)) { - spec = CreateDictionarySpec(type, location, keyType, elementType); + spec = CreateDictionarySpec(type, keyType, elementType); Debug.Assert(spec is null or DictionarySpec { KeyType: null or ParsableFromStringSpec }); } else { - spec = CreateEnumerableSpec(type, location); + spec = CreateEnumerableSpec(type); } if (spec is not null) @@ -364,7 +394,7 @@ private bool IsSupportedArrayType(ITypeSymbol type, Location? location) return spec; } - private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? location, ITypeSymbol keyType, ITypeSymbol elementType) + private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, ITypeSymbol keyType, ITypeSymbol elementType) { if (!TryGetTypeSpec(keyType, Diagnostics.DictionaryKeyNotSupported, out TypeSpec keySpec) || !TryGetTypeSpec(elementType, Diagnostics.ElementTypeNotSupported, out TypeSpec elementSpec)) @@ -374,7 +404,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc if (keySpec.SpecKind != TypeSpecKind.ParsableFromString) { - ReportUnsupportedType(type, Diagnostics.DictionaryKeyNotSupported, location); + RegisterUnsupportedType(type, Diagnostics.DictionaryKeyNotSupported); return null; } @@ -399,7 +429,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc } else { - ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location); + RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported); return null; } } @@ -420,7 +450,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc } else { - ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location); + RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported); return null; } @@ -440,7 +470,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc return spec; } - private EnumerableSpec? CreateEnumerableSpec(INamedTypeSymbol type, Location? location) + private EnumerableSpec? CreateEnumerableSpec(INamedTypeSymbol type) { if (!TryGetElementType(type, out ITypeSymbol? elementType) || !TryGetTypeSpec(elementType, Diagnostics.ElementTypeNotSupported, out TypeSpec elementSpec)) @@ -468,7 +498,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc } else { - ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location); + RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported); return null; } } @@ -508,7 +538,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc } else { - ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location); + RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported); return null; } @@ -529,7 +559,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc return spec; } - private ObjectSpec? CreateObjectSpec(INamedTypeSymbol type, Location? location) + private ObjectSpec? CreateObjectSpec(INamedTypeSymbol type) { // Add spec to cache before traversing properties to avoid stack overflow. ObjectSpec objectSpec = new(type); @@ -590,7 +620,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc if (diagnosticDescriptor is not null) { Debug.Assert(objectSpec.InitExceptionMessage is not null); - ReportUnsupportedType(type, diagnosticDescriptor); + RegisterUnsupportedType(type, diagnosticDescriptor); return objectSpec; } @@ -602,17 +632,21 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc { if (member is IPropertySymbol { IsIndexer: false, IsImplicitlyDeclared: false } property) { - AttributeData? attributeData = property.GetAttributes().FirstOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, _typeSymbols.ConfigurationKeyNameAttribute)); string propertyName = property.Name; - string configKeyName = attributeData?.ConstructorArguments.FirstOrDefault().Value as string ?? propertyName; - TypeSpec? propertyTypeSpec = GetOrCreateTypeSpec(property.Type); - if (propertyTypeSpec is null) + + if (propertyTypeSpec?.CanInitialize is not true) { - _context.ReportDiagnostic(Diagnostic.Create(Diagnostics.PropertyNotSupported, location, new string[] { propertyName, type.ToDisplayString() })); + InvocationDiagnosticInfo propertyDiagnostic = new InvocationDiagnosticInfo(Diagnostics.PropertyNotSupported, new string[] { propertyName, type.ToDisplayString() }); + RegisterTypeDiagnostic(causingType: type, propertyDiagnostic); + _invocationTargetTypeDiags.Add(propertyDiagnostic); } - else + + if (propertyTypeSpec is not null) { + AttributeData? attributeData = property.GetAttributes().FirstOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, _typeSymbols.ConfigurationKeyNameAttribute)); + string configKeyName = attributeData?.ConstructorArguments.FirstOrDefault().Value as string ?? propertyName; + PropertySpec spec = new(property) { Type = propertyTypeSpec, ConfigurationKeyName = configKeyName }; objectSpec.Properties[propertyName] = spec; Register_AsConfigWithChildren_HelperForGen_IfRequired(propertyTypeSpec); @@ -831,17 +865,32 @@ private static bool HasAddMethod(INamedTypeSymbol type, ITypeSymbol key, ITypeSy { Debug.Assert(type.IsGenericType); INamedTypeSymbol constructedType = type.Construct(parameters); - return CreateCollectionSpec(constructedType, location: null); + return CreateCollectionSpec(constructedType); } - private void ReportUnsupportedType(ITypeSymbol type, DiagnosticDescriptor descriptor, Location? location = null) + private void RegisterUnsupportedType(ITypeSymbol type, DiagnosticDescriptor descriptor = null) { + InvocationDiagnosticInfo diagInfo = new(descriptor, new string[] { type.ToDisplayString() }); + if (!_unsupportedTypes.Contains(type)) { - _context.ReportDiagnostic( - Diagnostic.Create(descriptor, location, new string[] { type.ToDisplayString() })); + RegisterTypeDiagnostic(type, diagInfo); _unsupportedTypes.Add(type); } + + _invocationTargetTypeDiags.Add(diagInfo); + } + + private void RegisterTypeDiagnostic(ITypeSymbol causingType, InvocationDiagnosticInfo info) + { + bool typeHadDiags = _typeDiagnostics.TryGetValue(causingType, out HashSet? typeDiags); + typeDiags ??= new HashSet(); + typeDiags.Add(info); + + if (!typeHadDiags) + { + _typeDiagnostics[causingType] = typeDiags; + } } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/ConfigurationBinder.cs index 26287798463ea..a663c441c55ce 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/ConfigurationBinder.cs @@ -100,7 +100,7 @@ private void RegisterBindInvocation(BinderInvocation invocation) return; } - if (GetOrCreateTypeSpec(type, invocation.Location) is TypeSpec typeSpec) + if (GetTargetTypeForRootInvocationCore(type, invocation.Location) is TypeSpec typeSpec) { Dictionary> types = _sourceGenSpec.TypesForGen_ConfigurationBinder_BindMethods; @@ -120,10 +120,12 @@ private void RegisterBindInvocation(BinderInvocation invocation) IInstanceReferenceOperation i => i.Type, ILocalReferenceOperation l => l.Local.Type, IFieldReferenceOperation f => f.Field.Type, + IPropertyReferenceOperation o => o.Type, IMethodReferenceOperation m when m.Method.MethodKind == MethodKind.Constructor => m.Method.ContainingType, IMethodReferenceOperation m => m.Method.ReturnType, IAnonymousFunctionOperation f => f.Symbol.ReturnType, IParameterReferenceOperation p => p.Parameter.Type, + IObjectCreationOperation o => o.Type, _ => null }; } @@ -180,11 +182,12 @@ private void RegisterGetInvocation(BinderInvocation invocation) } } - if (GetBindingConfigType(type, invocation.Location) is TypeSpec typeSpec) + if (GetTargetTypeForRootInvocation(type, invocation.Location) is TypeSpec typeSpec) { _sourceGenSpec.MethodsToGen_ConfigurationBinder |= overload; RegisterTypeForMethodGen(MethodsToGen_CoreBindingHelper.GetCore, typeSpec); } + } private void RegisterGetValueInvocation(BinderInvocation invocation) @@ -248,7 +251,7 @@ private void RegisterGetValueInvocation(BinderInvocation invocation) } if (IsParsableFromString(effectiveType, out _) && - GetOrCreateTypeSpec(type, invocation.Location) is TypeSpec typeSpec) + GetTargetTypeForRootInvocationCore(type, invocation.Location) is TypeSpec typeSpec) { _sourceGenSpec.MethodsToGen_ConfigurationBinder |= overload; RegisterTypeForMethodGen(MethodsToGen_CoreBindingHelper.GetValueCore, typeSpec); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsBuilderConfigurationExtensions.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsBuilderConfigurationExtensions.cs index 016d87aa37368..d01e80d708ca1 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsBuilderConfigurationExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsBuilderConfigurationExtensions.cs @@ -25,7 +25,7 @@ private void RegisterMethodInvocation_OptionsBuilderExt(BinderInvocation invocat return; } - TypeSpec typeSpec = GetBindingConfigType( + TypeSpec typeSpec = GetTargetTypeForRootInvocation( type: targetMethod.TypeArguments[0].WithNullableAnnotation(NullableAnnotation.None), invocation.Location); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsConfigurationServiceCollectionExtensions.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsConfigurationServiceCollectionExtensions.cs index 93ef2d584cfe3..02c75b4ab653b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsConfigurationServiceCollectionExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Parser/OptionsConfigurationServiceCollectionExtensions.cs @@ -69,7 +69,7 @@ @params[1].Type.SpecialType is SpecialType.System_String && return; } - TypeSpec typeSpec = GetBindingConfigType( + TypeSpec typeSpec = GetTargetTypeForRootInvocation( type: targetMethod.TypeArguments[0].WithNullableAnnotation(NullableAnnotation.None), invocation.Location); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs index fb51320941bef..a04f8e60f343b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs @@ -286,7 +286,9 @@ private void GetIntDictionaryT(T k1, T k2, T k3) var config = configurationBuilder.Build(); var options = new Dictionary(); +#pragma warning disable SYSLIB1104 config.GetSection("IntegerKeyDictionary").Bind(options); +#pragma warning restore SYSLIB1104 Assert.Equal(3, options.Count); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 50d1a48a3eccf..495e6246803fe 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -4,11 +4,12 @@ using System; using System.Collections.Generic; using System.ComponentModel; -using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Text.Json; using System.Linq; using Microsoft.Extensions.Configuration; using Xunit; +using System.Collections.Immutable; namespace Microsoft.Extensions #if BUILDING_SOURCE_GENERATOR_TESTS @@ -714,5 +715,33 @@ public class GeolocationWrapper { public Geolocation Location { get; set; } } + + public class GraphWithUnsupportedMember + { + public JsonWriterOptions WriterOptions { get; set; } + } + + public record RemoteAuthenticationOptions where TRemoteAuthenticationProviderOptions : new() + { + public TRemoteAuthenticationProviderOptions GenericProp { get; } = new(); + public OidcProviderOptions NonGenericProp { get; } = new(); + + public TRemoteAuthenticationProviderOptions _genericField { get; } = new(); + public OidcProviderOptions _nonGenericField { get; } = new(); + + public static TRemoteAuthenticationProviderOptions StaticGenericProp { get; } = new(); + public static OidcProviderOptions StaticNonGenericProp { get; } = new(); + + public static TRemoteAuthenticationProviderOptions s_GenericField = new(); + public static OidcProviderOptions s_NonGenericField = new(); + + public TRemoteAuthenticationProviderOptions? NullGenericProp { get; } + public static OidcProviderOptions? s_NullNonGenericField; + } + + public record OidcProviderOptions + { + public string? Authority { get; set; } + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index e4bc2205b517f..e5a48f2c5714f 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -8,6 +8,7 @@ using System.Globalization; using System.Linq; using System.Reflection; +using System.Text; #if BUILDING_SOURCE_GENERATOR_TESTS using Microsoft.Extensions.Configuration; #endif @@ -541,10 +542,12 @@ public void CanReadAllSupportedTypes(string value, Type type) var expectedValue = TypeDescriptor.GetConverter(type).ConvertFromInvariantString(value); // act +#pragma warning disable SYSLIB1104 config.Bind(options); var optionsValue = options.GetType().GetProperty("Value").GetValue(options); var getValueValue = config.GetValue(type, "Value"); var getValue = config.GetSection("Value").Get(type); +#pragma warning restore SYSLIB1104 // assert Assert.Equal(expectedValue, optionsValue); @@ -588,6 +591,7 @@ public void ConsistentExceptionOnFailedBinding(Type type) var options = Activator.CreateInstance(optionsType); // act +#pragma warning disable SYSLIB1104 var exception = Assert.Throws( () => config.Bind(options)); @@ -596,6 +600,7 @@ public void ConsistentExceptionOnFailedBinding(Type type) var getException = Assert.Throws( () => config.GetSection("Value").Get(type)); +#pragma warning restore SYSLIB1104 // assert Assert.NotNull(exception.InnerException); @@ -953,8 +958,7 @@ public void ExceptionWhenTryingToBindToInterface() configurationBuilder.AddInMemoryCollection(input); var config = configurationBuilder.Build(); - var exception = Assert.Throws( - () => config.Bind(new TestOptions())); + var exception = Assert.Throws(() => config.Bind(new TestOptions())); Assert.Equal( SR.Format(SR.Error_CannotActivateAbstractOrInterface, typeof(ISomeInterface)), exception.Message); @@ -1938,7 +1942,9 @@ public void BindRootStructIsNoOp() """); StructWithNestedStructs.DeeplyNested obj = new(); +#pragma warning disable SYSLIB1103 configuration.Bind(obj); +#pragma warning restore SYSLIB1103 Assert.Equal(0, obj.Int32); Assert.False(obj.Boolean); } @@ -2056,5 +2062,55 @@ private void ValidateGeolocation(IGeolocation location) Assert.Equal(3, location.Latitude); Assert.Equal(4, location.Longitude); } + + [Fact] + public void TestGraphWithUnsupportedMember() + { + var configuration = TestHelpers.GetConfigurationFromJsonString("""{ "WriterOptions": { "Indented": "true" } }"""); + var obj = new GraphWithUnsupportedMember(); + configuration.Bind(obj); + Assert.True(obj.WriterOptions.Indented); + + // Encoder prop not supported; throw if there's config data. + configuration = TestHelpers.GetConfigurationFromJsonString("""{ "WriterOptions": { "Indented": "true", "Encoder": { "Random": "" } } }"""); + Assert.Throws(() => configuration.Bind(obj)); + } + + [Fact] + public void CanBindToObjectMembers() + { + var config = TestHelpers.GetConfigurationFromJsonString("""{ "Local": { "Authority": "Auth1" } }"""); + + // Regression tests for https://github.com/dotnet/runtime/issues/89273 and https://github.com/dotnet/runtime/issues/89732. + TestBind(options => config.Bind("Local", options.GenericProp), obj => obj.GenericProp); + TestBind(options => config.GetSection("Local").Bind(options.NonGenericProp), obj => obj.NonGenericProp); + TestBind(options => config.GetSection("Local").Bind(options._genericField, _ => { }), obj => obj._genericField); + TestBind(options => config.Bind("Local", options._nonGenericField), obj => obj._nonGenericField); + + // Check statics. + TestBind(options => config.GetSection("Local").Bind(RemoteAuthenticationOptions.StaticGenericProp), obj => RemoteAuthenticationOptions.StaticGenericProp); + TestBind(options => config.GetSection("Local").Bind(RemoteAuthenticationOptions.StaticNonGenericProp, _ => { }), obj => RemoteAuthenticationOptions.StaticNonGenericProp); + TestBind(options => config.Bind("Local", RemoteAuthenticationOptions.s_GenericField), obj => RemoteAuthenticationOptions.s_GenericField); + TestBind(options => config.GetSection("Local").Bind(RemoteAuthenticationOptions.s_NonGenericField), obj => RemoteAuthenticationOptions.s_NonGenericField); + + // No null refs. + Assert.Throws(() => config.GetSection("Local").Bind(new RemoteAuthenticationOptions().NullGenericProp)); + Assert.Throws(() => config.GetSection("Local").Bind(RemoteAuthenticationOptions.s_NullNonGenericField)); + + static void TestBind(Action> configure, Func, OidcProviderOptions> getBindedProp) + { + var obj = new RemoteAuthenticationOptions(); + configure(obj); + Assert.Equal("Auth1", getBindedProp(obj).Authority); + } + } + + [Fact] + public void BinderSupportsObjCreationInput() + { + var configuration = new ConfigurationBuilder().Build(); + // No diagnostic warning SYSLIB1104. + configuration.Bind(new GraphWithUnsupportedMember()); + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs index bf75dd35adbf1..aba2a9f6184f2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Threading.Tasks; @@ -607,64 +606,54 @@ public class MyClass public async Task Collections() { string source = """ - using System.Collections.Generic; - using Microsoft.Extensions.Configuration; - - public class Program - { - public static void Main() - { - ConfigurationBuilder configurationBuilder = new(); - IConfiguration config = configurationBuilder.Build(); - IConfigurationSection section = config.GetSection(""MySection""); - - section.Get(); - } + using System.Collections.Generic; + using Microsoft.Extensions.Configuration; - public class MyClassWithCustomCollections - { - public CustomDictionary CustomDictionary { get; set; } - public CustomList CustomList { get; set; } - public ICustomDictionary ICustomDictionary { get; set; } - public ICustomSet ICustomCollection { get; set; } - public IReadOnlyList IReadOnlyList { get; set; } - public IReadOnlyDictionary UnsupportedIReadOnlyDictionaryUnsupported { get; set; } - public IReadOnlyDictionary IReadOnlyDictionary { get; set; } - } + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfiguration config = configurationBuilder.Build(); + IConfigurationSection section = config.GetSection(""MySection""); - public class CustomDictionary : Dictionary - { - } + section.Get(); + } - public class CustomList : List - { - } + public class MyClassWithCustomCollections + { + public CustomDictionary CustomDictionary { get; set; } + public CustomList CustomList { get; set; } + public ICustomDictionary ICustomDictionary { get; set; } + public ICustomSet ICustomCollection { get; set; } + public IReadOnlyList IReadOnlyList { get; set; } + public IReadOnlyDictionary UnsupportedIReadOnlyDictionaryUnsupported { get; set; } + public IReadOnlyDictionary IReadOnlyDictionary { get; set; } + } - public interface ICustomDictionary : IDictionary - { - } + public class CustomDictionary : Dictionary + { + } - public interface ICustomSet : ISet - { - } - } - """; + public class CustomList : List + { + } - await VerifyAgainstBaselineUsingFile("Collections.generated.txt", source, assessDiagnostics: (d) => - { - Assert.Equal(6, d.Length); - Test(d.Where(diagnostic => diagnostic.Id is "SYSLIB1100"), "Did not generate binding logic for a type"); - Test(d.Where(diagnostic => diagnostic.Id is "SYSLIB1101"), "Did not generate binding logic for a property on a type"); + public interface ICustomDictionary : IDictionary + { + } - static void Test(IEnumerable d, string expectedTitle) - { - Assert.Equal(3, d.Count()); - foreach (Diagnostic diagnostic in d) + public interface ICustomSet : ISet { - Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); - Assert.Contains(expectedTitle, diagnostic.Descriptor.Title.ToString(CultureInfo.InvariantCulture)); } } + """; + + await VerifyAgainstBaselineUsingFile("Collections.generated.txt", source, assessDiagnostics: (d) => + { + Console.WriteLine((d.Where(diag => diag.Id == Diagnostics.TypeNotSupported.Id).Count() , d.Where(diag => diag.Id == Diagnostics.PropertyNotSupported.Id).Count())); + Assert.Equal(3, d.Where(diag => diag.Id == Diagnostics.TypeNotSupported.Id).Count()); + Assert.Equal(6, d.Where(diag => diag.Id == Diagnostics.PropertyNotSupported.Id).Count()); }); } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs index 4a8296294d4db..5bc5145739daa 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs @@ -3,10 +3,14 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Collections.Immutable; using System.Globalization; using System.IO; using System.Linq; +using System.Reflection; +using System.Text; +using System.Text.Json; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -29,6 +33,17 @@ private static class Diagnostics public static (string Id, string Title) CouldNotDetermineTypeInfo = ("SYSLIB1104", "The target type for a binder call could not be determined"); } + private static readonly Assembly[] s_compilationAssemblyRefs = new[] { + typeof(ConfigurationBinder).Assembly, + typeof(CultureInfo).Assembly, + typeof(IConfiguration).Assembly, + typeof(IServiceCollection).Assembly, + typeof(IDictionary).Assembly, + typeof(OptionsBuilder<>).Assembly, + typeof(OptionsConfigurationServiceCollectionExtensions).Assembly, + typeof(Uri).Assembly, + }; + private enum ExtensionClassType { None, @@ -242,6 +257,118 @@ public class MyClass2 { } Assert.Empty(d); } + [Fact] + public async Task SucceedForMinimalInput() + { + string source = """ + using System; + using Microsoft.Extensions.Configuration; + + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfiguration config = configurationBuilder.Build(); + config.Bind(new MyClass0()); + } + + public class MyClass0 { } + } + """; + + HashSet exclusions = new() + { + typeof(CultureInfo), + typeof(IServiceCollection), + typeof(IDictionary), + typeof(ServiceCollection), + typeof(OptionsBuilder<>), + typeof(OptionsConfigurationServiceCollectionExtensions), + typeof(Uri) + }; + + await Test(expectOutput: true); + + exclusions.Add(typeof(ConfigurationBinder)); + await Test(expectOutput: false); + + exclusions.Remove(typeof(ConfigurationBinder)); + exclusions.Add(typeof(IConfiguration)); + await Test(expectOutput: false); + + async Task Test(bool expectOutput) + { + var (d, r) = await RunGenerator(source, references: GetFilteredAssemblyRefs(exclusions)); + + Assert.Empty(d); + + if (expectOutput) + { + Assert.Single(r); + } + else + { + Assert.Empty(r); + } + } + } + + [Fact] + public async Task IssueDiagnosticsForAllOffendingCallsites() + { + string source = """ + using System.Collections.Immutable; + using System.Text; + using System.Text.Json; + using Microsoft.AspNetCore.Builder; + using Microsoft.Extensions.Configuration; + using Microsoft.Extensions.DependencyInjection; + + public class Program + { + public static void Main() + { + ConfigurationBuilder configurationBuilder = new(); + IConfiguration configuration = configurationBuilder.Build(); + + var obj = new TypeGraphWithUnsupportedMember(); + configuration.Bind(obj); + + var obj2 = new AnotherGraphWithUnsupportedMembers(); + var obj4 = Encoding.UTF8; + + // Must require separate suppression. + configuration.Bind(obj2); + configuration.Bind(obj2, _ => { }); + configuration.Bind("", obj2); + configuration.Get(); + configuration.Get(_ => { }); + configuration.Get(typeof(TypeGraphWithUnsupportedMember)); + configuration.Get(typeof(AnotherGraphWithUnsupportedMembers), _ => { }); + configuration.Bind(obj4); + configuration.Get(); + } + + public class TypeGraphWithUnsupportedMember + { + public JsonWriterOptions WriterOptions { get; set; } + } + + public class AnotherGraphWithUnsupportedMembers + { + public JsonWriterOptions WriterOptions { get; set; } + public ImmutableArray UnsupportedArray { get; set; } + } + } + """; + + var (d, r) = await RunGenerator(source, references: GetAssemblyRefsWithAdditional(typeof(ImmutableArray<>), typeof(Encoding), typeof(JsonSerializer))); + Assert.Single(r); + Assert.Equal(12, d.Where(diag => diag.Id == Diagnostics.TypeNotSupported.Id).Count()); + Assert.Equal(10, d.Where(diag => diag.Id == Diagnostics.PropertyNotSupported.Id).Count()); + } + private static async Task VerifyAgainstBaselineUsingFile( string filename, string testSourceCode, @@ -268,21 +395,29 @@ private static async Task VerifyAgainstBaselineUsingFile( private static async Task<(ImmutableArray, ImmutableArray)> RunGenerator( string testSourceCode, - LanguageVersion langVersion = LanguageVersion.CSharp11) => + LanguageVersion langVersion = LanguageVersion.CSharp11, + IEnumerable? references = null) => await RoslynTestUtils.RunGenerator( new ConfigurationBindingGenerator(), - new[] { - typeof(ConfigurationBinder).Assembly, - typeof(CultureInfo).Assembly, - typeof(IConfiguration).Assembly, - typeof(IServiceCollection).Assembly, - typeof(IDictionary).Assembly, - typeof(ServiceCollection).Assembly, - typeof(OptionsBuilder<>).Assembly, - typeof(OptionsConfigurationServiceCollectionExtensions).Assembly, - typeof(Uri).Assembly, - }, + references ?? s_compilationAssemblyRefs, new[] { testSourceCode }, langVersion: langVersion).ConfigureAwait(false); + + public static List GetAssemblyRefsWithAdditional(params Type[] additional) + { + List assemblies = new(s_compilationAssemblyRefs); + assemblies.AddRange(additional.Select(t => t.Assembly)); + return assemblies; + } + + public static HashSet GetFilteredAssemblyRefs(IEnumerable exclusions) + { + HashSet assemblies = new(s_compilationAssemblyRefs); + foreach (Type exclusion in exclusions) + { + assemblies.Remove(exclusion.Assembly); + } + return assemblies; + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj index b67b5ba6a753b..2108bc2574ed2 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj @@ -2,8 +2,8 @@ $(NetCoreAppCurrent);$(NetFrameworkMinimum) true - - SYSLIB1100,SYSLIB1101,SYSLIB1103,SYSLIB1104 + + SYSLIB1100,SYSLIB1101 true @@ -44,9 +44,9 @@ + Baselines\ConfigurationBinder\*.generated.txt; + Baselines\OptionsBuilder\*.generated.txt; + Baselines\ServiceCollection\*.generated.txt"> PreserveNewest diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterOptions.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterOptions.cs index 2a9d4e5901a49..00f0f3773dadb 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterOptions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterOptions.cs @@ -21,6 +21,9 @@ public JsonConsoleFormatterOptions() { } /// public JsonWriterOptions JsonWriterOptions { get; set; } +#pragma warning disable SYSLIB1100 +#pragma warning disable SYSLIB1101 internal override void Configure(IConfiguration configuration) => configuration.Bind(this); +#pragma warning restore } } diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj b/src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj index 8cec9d1b8ca78..abc5c9d9792ee 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj +++ b/src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj @@ -8,8 +8,6 @@ true true true - - $(NoWarn);SYSLIB1100 Console logger provider implementation for Microsoft.Extensions.Logging.