Skip to content

Commit

Permalink
[release/9.0-rc1] Fallback to treating as object if not collection (#…
Browse files Browse the repository at this point in the history 
…106517)

* Fallback to treating as object if not collection

The reflection binder will fallback if a type does not meet collection
heuristics, but the source generator did not.

* Fix UnsupportedTypes test

* Update collection to object fallback condition

This matches what the refelction binder does, and fixes the baseline
diffs (and diagnostics changes) we were seeing for unsupported
collection types.

* Refactor fallback from collection to object mode

---------

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
  • Loading branch information
@github-actions @ericstj
github-actions[bot] and ericstj authored Aug 16, 2024
1 parent e4335df commit 63c9565
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 18 deletions.
43 changes: 25 additions & 18 deletions ...ies/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ private TypeSpec CreateTypeSpec(TypeParseInfo typeParseInfo)
}
else if (IsCollection(type))
{
spec = CreateCollectionSpec(typeParseInfo);
spec = CreateCollectionSpec(typeParseInfo) ?? CreateObjectSpec(typeParseInfo);
}
else if (SymbolEqualityComparer.Default.Equals(type, _typeSymbols.IConfigurationSection))
{
Expand Down Expand Up @@ -360,34 +360,37 @@ private TypeSpec CreateArraySpec(TypeParseInfo typeParseInfo)
};
}

private TypeSpec CreateCollectionSpec(TypeParseInfo typeParseInfo)
private TypeSpec? CreateCollectionSpec(TypeParseInfo typeParseInfo)
{
INamedTypeSymbol type = (INamedTypeSymbol)typeParseInfo.TypeSymbol;

TypeSpec spec;
TypeSpec? spec;
if (IsCandidateDictionary(type, out ITypeSymbol? keyType, out ITypeSymbol? elementType))
{
spec = CreateDictionarySpec(typeParseInfo, keyType, elementType);
Debug.Assert(spec is DictionarySpec or UnsupportedTypeSpec);
Debug.Assert(spec is DictionarySpec or UnsupportedTypeSpec or null);
}
else
{
spec = CreateEnumerableSpec(typeParseInfo);
Debug.Assert(spec is EnumerableSpec or UnsupportedTypeSpec);
Debug.Assert(spec is EnumerableSpec or UnsupportedTypeSpec or null);
}

return spec;
}

private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol keyTypeSymbol, ITypeSymbol elementTypeSymbol)
private TypeSpec? CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol keyTypeSymbol, ITypeSymbol elementTypeSymbol)
{
INamedTypeSymbol type = (INamedTypeSymbol)typeParseInfo.TypeSymbol;

// treat as unsupported if it implements IDictionary<,>, otherwise we'll try to fallback and treat as an object
bool isDictionary = _typeSymbols.GenericICollection is not null && GetInterface(type, _typeSymbols.GenericIDictionary_Unbound) is not null;

if (IsUnsupportedType(keyTypeSymbol) || IsUnsupportedType(elementTypeSymbol))
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
return isDictionary ? CreateUnsupportedCollectionSpec(typeParseInfo) : null;
}

INamedTypeSymbol type = (INamedTypeSymbol)typeParseInfo.TypeSymbol;

CollectionInstantiationStrategy instantiationStrategy;
CollectionInstantiationConcreteType instantiationConcreteType;
CollectionPopulationCastType populationCastType;
Expand All @@ -402,14 +405,15 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
{
populationCastType = CollectionPopulationCastType.NotApplicable;
}
else if (_typeSymbols.GenericIDictionary is not null && GetInterface(type, _typeSymbols.GenericIDictionary_Unbound) is not null)
else if (isDictionary)
{
// implements IDictionary<,> -- cast to it.
populationCastType = CollectionPopulationCastType.IDictionary;
}
else
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
// not a dictionary
return null;
}
}
else if (_typeSymbols.Dictionary is not null &&
Expand All @@ -429,7 +433,7 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
}
else
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
return isDictionary ? CreateUnsupportedCollectionSpec(typeParseInfo) : null;
}

TypeRef keyTypeRef = EnqueueTransitiveType(typeParseInfo, keyTypeSymbol, DiagnosticDescriptors.DictionaryKeyNotSupported);
Expand All @@ -447,18 +451,20 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
};
}

private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
private TypeSpec? CreateEnumerableSpec(TypeParseInfo typeParseInfo)
{
INamedTypeSymbol type = (INamedTypeSymbol)typeParseInfo.TypeSymbol;

bool isCollection = _typeSymbols.GenericICollection is not null && GetInterface(type, _typeSymbols.GenericICollection_Unbound) is not null;

if (!TryGetElementType(type, out ITypeSymbol? elementType))
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
return isCollection ? CreateUnsupportedCollectionSpec(typeParseInfo) : null;
}

if (IsUnsupportedType(elementType))
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
return isCollection ? CreateUnsupportedCollectionSpec(typeParseInfo) : null;
}

CollectionInstantiationStrategy instantiationStrategy;
Expand All @@ -475,14 +481,15 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
{
populationCastType = CollectionPopulationCastType.NotApplicable;
}
else if (_typeSymbols.GenericICollection is not null && GetInterface(type, _typeSymbols.GenericICollection_Unbound) is not null)
else if (isCollection)
{
// implements ICollection<> -- cast to it
populationCastType = CollectionPopulationCastType.ICollection;
}
else
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
// not a collection
return null;
}
}
else if ((IsInterfaceMatch(type, _typeSymbols.GenericICollection_Unbound) || IsInterfaceMatch(type, _typeSymbols.GenericIList_Unbound)))
Expand Down Expand Up @@ -523,7 +530,7 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
}
else
{
return CreateUnsupportedCollectionSpec(typeParseInfo);
return isCollection ? CreateUnsupportedCollectionSpec(typeParseInfo) : null;
}

TypeRef elementTypeRef = EnqueueTransitiveType(typeParseInfo, elementType, DiagnosticDescriptors.ElementTypeNotSupported);
Expand Down
21 changes: 21 additions & 0 deletions ...soft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;
Expand Down Expand Up @@ -1067,5 +1068,25 @@ public class DerivedClassWithHiddenMembers : IntermediateDerivedClass
public override int X { set => base.X = value + 1; }
}

public class EnumerableNotCollection : IEnumerable<KeyValuePair<string, string>>
{
public string Names { get; set; }

public string[] Keywords { get; set; }

public bool Enabled { get; set; }

private IEnumerable<KeyValuePair<string, string>> enumerate()
{
yield return new KeyValuePair<string, string>(nameof(Names), Names);
yield return new KeyValuePair<string, string>(nameof(Keywords), string.Join(",", Keywords));
yield return new KeyValuePair<string, string>(nameof(Enabled), Enabled.ToString());
}

public IEnumerator<KeyValuePair<string, string>> GetEnumerator() => enumerate().GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => enumerate().GetEnumerator();
}

}
}
22 changes: 22 additions & 0 deletions ...raries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2602,5 +2602,27 @@ public void CanBindToClassWithNewProperties()
Assert.Equal(53, obj.X);
Assert.Equal(53, obj.XBase);
}

[Fact]
public void CanGetEnumerableNotCollection()
{
var builder = new ConfigurationBuilder();
builder.AddInMemoryCollection(new KeyValuePair<string, string?>[]
{
new("Names", "John,Jane,Stephen"),
new("Enabled", "true"),
new("Keywords:1", "new"),
new("Keywords:2", "class"),
new("Keywords:3", "rosebud")
});

var config = builder.Build();

var result = config.Get<EnumerableNotCollection>();

Assert.Equal("John,Jane,Stephen", result.Names);
Assert.True(result.Enabled);
Assert.Equal(new [] { "new", "class", "rosebud"}, result.Keywords);
}
}
}

0 comments on commit 63c9565

Please sign in to comment.