Skip to content

Commit 4bd1492

Browse files
ericstjericstj
authored
Make Config generator test only for down-casts (#106145)
* Make Config generator test for downcasts We were hitting compiler errors when the generator emitted test casts for value types. Since those can never be true (value types cannot be derived from, and the compiler can see if the cast will succeed or not). Fix this by only doing a test cast when we are trying to do a runtime downcast. * Rename ShouldTestCast -> ShouldTryCast * Add tests for readonly collection structs These are unsupported and ignored by the generator and a diagnostic is emitted. Binding data to them does nothing.
1 parent 07f9843 commit 4bd1492

File tree

10 files changed

+355
-15
lines changed

10 files changed

+355
-15
lines changed

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
391391
CollectionInstantiationStrategy instantiationStrategy;
392392
CollectionInstantiationConcreteType instantiationConcreteType;
393393
CollectionPopulationCastType populationCastType;
394+
bool shouldTryCast = false;
394395

395396
if (HasPublicParameterLessCtor(type))
396397
{
@@ -403,6 +404,7 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
403404
}
404405
else if (_typeSymbols.GenericIDictionary is not null && GetInterface(type, _typeSymbols.GenericIDictionary_Unbound) is not null)
405406
{
407+
// implements IDictionary<,> -- cast to it.
406408
populationCastType = CollectionPopulationCastType.IDictionary;
407409
}
408410
else
@@ -421,7 +423,9 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
421423
{
422424
instantiationStrategy = CollectionInstantiationStrategy.LinqToDictionary;
423425
instantiationConcreteType = CollectionInstantiationConcreteType.Dictionary;
426+
// is IReadonlyDictionary<,> -- test cast to IDictionary<,>
424427
populationCastType = CollectionPopulationCastType.IDictionary;
428+
shouldTryCast = true;
425429
}
426430
else
427431
{
@@ -431,13 +435,15 @@ private TypeSpec CreateDictionarySpec(TypeParseInfo typeParseInfo, ITypeSymbol k
431435
TypeRef keyTypeRef = EnqueueTransitiveType(typeParseInfo, keyTypeSymbol, DiagnosticDescriptors.DictionaryKeyNotSupported);
432436
TypeRef elementTypeRef = EnqueueTransitiveType(typeParseInfo, elementTypeSymbol, DiagnosticDescriptors.ElementTypeNotSupported);
433437

438+
Debug.Assert(!shouldTryCast || !type.IsValueType, "Should not test cast for value types.");
434439
return new DictionarySpec(type)
435440
{
436441
KeyTypeRef = keyTypeRef,
437442
ElementTypeRef = elementTypeRef,
438443
InstantiationStrategy = instantiationStrategy,
439444
InstantiationConcreteType = instantiationConcreteType,
440445
PopulationCastType = populationCastType,
446+
ShouldTryCast = shouldTryCast
441447
};
442448
}
443449

@@ -458,6 +464,7 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
458464
CollectionInstantiationStrategy instantiationStrategy;
459465
CollectionInstantiationConcreteType instantiationConcreteType;
460466
CollectionPopulationCastType populationCastType;
467+
bool shouldTryCast = false;
461468

462469
if (HasPublicParameterLessCtor(type))
463470
{
@@ -470,6 +477,7 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
470477
}
471478
else if (_typeSymbols.GenericICollection is not null && GetInterface(type, _typeSymbols.GenericICollection_Unbound) is not null)
472479
{
480+
// implements ICollection<> -- cast to it
473481
populationCastType = CollectionPopulationCastType.ICollection;
474482
}
475483
else
@@ -487,7 +495,9 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
487495
{
488496
instantiationStrategy = CollectionInstantiationStrategy.CopyConstructor;
489497
instantiationConcreteType = CollectionInstantiationConcreteType.List;
498+
// is IEnumerable<> -- test cast to ICollection<>
490499
populationCastType = CollectionPopulationCastType.ICollection;
500+
shouldTryCast = true;
491501
}
492502
else if (IsInterfaceMatch(type, _typeSymbols.ISet_Unbound))
493503
{
@@ -499,13 +509,17 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
499509
{
500510
instantiationStrategy = CollectionInstantiationStrategy.CopyConstructor;
501511
instantiationConcreteType = CollectionInstantiationConcreteType.HashSet;
512+
// is IReadOnlySet<> -- test cast to ISet<>
502513
populationCastType = CollectionPopulationCastType.ISet;
514+
shouldTryCast = true;
503515
}
504516
else if (IsInterfaceMatch(type, _typeSymbols.IReadOnlyList_Unbound) || IsInterfaceMatch(type, _typeSymbols.IReadOnlyCollection_Unbound))
505517
{
506518
instantiationStrategy = CollectionInstantiationStrategy.CopyConstructor;
507519
instantiationConcreteType = CollectionInstantiationConcreteType.List;
520+
// is IReadOnlyList<> or IReadOnlyCollection<> -- test cast to ICollection<>
508521
populationCastType = CollectionPopulationCastType.ICollection;
522+
shouldTryCast = true;
509523
}
510524
else
511525
{
@@ -514,12 +528,14 @@ private TypeSpec CreateEnumerableSpec(TypeParseInfo typeParseInfo)
514528

515529
TypeRef elementTypeRef = EnqueueTransitiveType(typeParseInfo, elementType, DiagnosticDescriptors.ElementTypeNotSupported);
516530

531+
Debug.Assert(!shouldTryCast || !type.IsValueType, "Should not test cast for value types.");
517532
return new EnumerableSpec(type)
518533
{
519534
ElementTypeRef = elementTypeRef,
520535
InstantiationStrategy = instantiationStrategy,
521536
InstantiationConcreteType = instantiationConcreteType,
522537
PopulationCastType = populationCastType,
538+
ShouldTryCast = shouldTryCast
523539
};
524540
}
525541

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,15 +1230,25 @@ private void EmitCollectionCastIfRequired(CollectionWithCtorInitSpec type, out s
12301230
return;
12311231
}
12321232

1233-
string castTypeDisplayString = TypeIndex.GetPopulationCastTypeDisplayString(type);
12341233
instanceIdentifier = Identifier.temp;
1234+
string castExpression = $"{TypeIndex.GetPopulationCastTypeDisplayString(type)} {instanceIdentifier}";
1235+
1236+
if (type.ShouldTryCast)
1237+
{
1238+
_writer.WriteLine($$"""
1239+
if ({{Identifier.instance}} is not {{castExpression}})
1240+
{
1241+
return;
1242+
}
1243+
""");
1244+
}
1245+
else
1246+
{
1247+
_writer.WriteLine($$"""
1248+
{{castExpression}} = {{Identifier.instance}};
1249+
""");
1250+
}
12351251

1236-
_writer.WriteLine($$"""
1237-
if ({{Identifier.instance}} is not {{castTypeDisplayString}} {{instanceIdentifier}})
1238-
{
1239-
return;
1240-
}
1241-
""");
12421252
_writer.WriteLine();
12431253

12441254
}

‎src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Types/CollectionSpec.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ protected CollectionWithCtorInitSpec(ITypeSymbol type) : base(type) { }
2323
public required CollectionInstantiationConcreteType InstantiationConcreteType { get; init; }
2424

2525
public required CollectionPopulationCastType PopulationCastType { get; init; }
26+
27+
public required bool ShouldTryCast { get; init; }
2628
}
2729

2830
internal sealed record ArraySpec : CollectionSpec

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,83 @@ public void TestCollectionWithNullOrEmptyItems()
23682368
Assert.Equal("System.Boolean", result[0].Elements[1].Type);
23692369
}
23702370

2371+
[Fact]
2372+
public void TestStringValues()
2373+
{
2374+
// StringValues is a struct that implements IList<string> -- though it doesn't actually support Add
2375+
2376+
var dic = new Dictionary<string, string>
2377+
{
2378+
{"StringValues:0", "Yo1"},
2379+
{"StringValues:1", "Yo2"},
2380+
};
2381+
var configurationBuilder = new ConfigurationBuilder();
2382+
configurationBuilder.AddInMemoryCollection(dic);
2383+
2384+
var config = configurationBuilder.Build();
2385+
2386+
var options = new OptionsWithStructs();
2387+
2388+
#if BUILDING_SOURCE_GENERATOR_TESTS
2389+
Assert.Throws<NotSupportedException>(() => config.Bind(options));
2390+
#else
2391+
Assert.Throws<InvalidOperationException>(() => config.Bind(options, (bo) => bo.ErrorOnUnknownConfiguration = true));
2392+
#endif
2393+
}
2394+
2395+
[Fact]
2396+
public void TestOptionsWithStructs()
2397+
{
2398+
var dic = new Dictionary<string, string>
2399+
{
2400+
{"CollectionStructExplicit:0", "cs1"},
2401+
{"CollectionStructExplicit:1", "cs2"},
2402+
{"DictionaryStructExplicit:k0", "ds1"},
2403+
{"DictionaryStructExplicit:k1", "ds2"},
2404+
};
2405+
var configurationBuilder = new ConfigurationBuilder();
2406+
configurationBuilder.AddInMemoryCollection(dic);
2407+
2408+
var config = configurationBuilder.Build();
2409+
2410+
var options = new OptionsWithStructs();
2411+
config.Bind(options);
2412+
2413+
ICollection<string> collection = options.CollectionStructExplicit;
2414+
Assert.Equal(2, collection.Count);
2415+
Assert.Equal(collection, ["cs1", "cs2"]);
2416+
2417+
IDictionary<string, string> dictionary = options.DictionaryStructExplicit;
2418+
Assert.Equal(2, dictionary.Count);
2419+
Assert.Equal("ds1", dictionary["k0"]);
2420+
Assert.Equal("ds2", dictionary["k1"]);
2421+
}
2422+
2423+
[Fact]
2424+
public void TestOptionsWithUnsupportedStructs()
2425+
{
2426+
var dic = new Dictionary<string, string>
2427+
{
2428+
{"ReadOnlyCollectionStructExplicit:0", "cs1"},
2429+
{"ReadOnlyCollectionStructExplicit:1", "cs2"},
2430+
{"ReadOnlyDictionaryStructExplicit:k0", "ds1"},
2431+
{"ReadOnlyDictionaryStructExplicit:k1", "ds2"},
2432+
};
2433+
var configurationBuilder = new ConfigurationBuilder();
2434+
configurationBuilder.AddInMemoryCollection(dic);
2435+
2436+
var config = configurationBuilder.Build();
2437+
2438+
var options = new OptionsWithUnsupportedStructs();
2439+
config.Bind(options);
2440+
2441+
IReadOnlyCollection<string> collection = options.ReadOnlyCollectionStructExplicit;
2442+
Assert.Equal(0, collection.Count);
2443+
2444+
IReadOnlyDictionary<string, string> dictionary = options.ReadOnlyDictionaryStructExplicit;
2445+
Assert.Equal(0, dictionary.Count);
2446+
}
2447+
23712448
// Test behavior for root level arrays.
23722449

23732450
// Tests for TypeConverter usage.

‎src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections.Generic;
77
using System.Collections.ObjectModel;
88
using System.Linq;
9+
using Microsoft.Extensions.Primitives;
910

1011
namespace Microsoft.Extensions
1112
#if BUILDING_SOURCE_GENERATOR_TESTS
@@ -388,5 +389,123 @@ public class Element
388389
{
389390
public string Type { get; set; }
390391
}
392+
393+
public class OptionsWithStructs
394+
{
395+
public StringValues StringValues { get; set; }
396+
397+
public CollectionStructExplicit CollectionStructExplicit { get; set; } = new();
398+
399+
public DictionaryStructExplicit DictionaryStructExplicit { get; set; } = new();
400+
}
401+
402+
public struct CollectionStructExplicit : ICollection<string>
403+
{
404+
public CollectionStructExplicit() {}
405+
406+
ICollection<string> _collection = new List<string>();
407+
408+
int ICollection<string>.Count => _collection.Count;
409+
410+
bool ICollection<string>.IsReadOnly => _collection.IsReadOnly;
411+
412+
void ICollection<string>.Add(string item) => _collection.Add(item);
413+
414+
void ICollection<string>.Clear() => _collection.Clear();
415+
416+
bool ICollection<string>.Contains(string item) => _collection.Contains(item);
417+
418+
void ICollection<string>.CopyTo(string[] array, int arrayIndex) => _collection.CopyTo(array, arrayIndex);
419+
420+
IEnumerator<string> IEnumerable<string>.GetEnumerator() => _collection.GetEnumerator();
421+
422+
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_collection).GetEnumerator();
423+
424+
bool ICollection<string>.Remove(string item) => _collection.Remove(item);
425+
}
426+
427+
public struct DictionaryStructExplicit : IDictionary<string, string>
428+
{
429+
public DictionaryStructExplicit() {}
430+
431+
IDictionary<string, string> _dictionary = new Dictionary<string, string>();
432+
433+
string IDictionary<string, string>.this[string key] { get => _dictionary[key]; set => _dictionary[key] = value; }
434+
435+
ICollection<string> IDictionary<string, string>.Keys => _dictionary.Keys;
436+
437+
ICollection<string> IDictionary<string, string>.Values => _dictionary.Values;
438+
439+
int ICollection<KeyValuePair<string, string>>.Count => _dictionary.Count;
440+
441+
bool ICollection<KeyValuePair<string, string>>.IsReadOnly => _dictionary.IsReadOnly;
442+
443+
void IDictionary<string, string>.Add(string key, string value) => _dictionary.Add(key, value);
444+
445+
void ICollection<KeyValuePair<string, string>>.Add(KeyValuePair<string, string> item) => _dictionary.Add(item);
446+
447+
void ICollection<KeyValuePair<string, string>>.Clear() => _dictionary.Clear();
448+
449+
bool ICollection<KeyValuePair<string, string>>.Contains(KeyValuePair<string, string> item) => _dictionary.Contains(item);
450+
451+
bool IDictionary<string, string>.ContainsKey(string key) => _dictionary.ContainsKey(key);
452+
453+
void ICollection<KeyValuePair<string, string>>.CopyTo(KeyValuePair<string, string>[] array, int arrayIndex) => _dictionary.CopyTo(array, arrayIndex);
454+
455+
IEnumerator<KeyValuePair<string, string>> IEnumerable<KeyValuePair<string, string>>.GetEnumerator() => _dictionary.GetEnumerator();
456+
457+
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_dictionary).GetEnumerator();
458+
459+
bool IDictionary<string, string>.Remove(string key) => _dictionary.Remove(key);
460+
461+
bool ICollection<KeyValuePair<string, string>>.Remove(KeyValuePair<string, string> item) => _dictionary.Remove(item);
462+
463+
bool IDictionary<string, string>.TryGetValue(string key, out string value) => _dictionary.TryGetValue(key, out value);
464+
}
465+
466+
public class OptionsWithUnsupportedStructs
467+
{
468+
public ReadOnlyCollectionStructExplicit ReadOnlyCollectionStructExplicit { get; set; } = new();
469+
470+
public ReadOnlyDictionaryStructExplicit ReadOnlyDictionaryStructExplicit { get; set; } = new();
471+
}
472+
473+
public struct ReadOnlyCollectionStructExplicit : IReadOnlyCollection<string>
474+
{
475+
public ReadOnlyCollectionStructExplicit()
476+
{
477+
_collection = new List<string>();
478+
}
479+
480+
private readonly IReadOnlyCollection<string> _collection;
481+
int IReadOnlyCollection<string>.Count => _collection.Count;
482+
IEnumerator<string> IEnumerable<string>.GetEnumerator() => _collection.GetEnumerator();
483+
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_collection).GetEnumerator();
484+
}
485+
486+
public struct ReadOnlyDictionaryStructExplicit : IReadOnlyDictionary<string, string>
487+
{
488+
public ReadOnlyDictionaryStructExplicit()
489+
{
490+
_dictionary = new Dictionary<string, string>();
491+
}
492+
493+
private readonly IReadOnlyDictionary<string, string> _dictionary;
494+
string IReadOnlyDictionary<string, string>.this[string key] => _dictionary[key];
495+
496+
IEnumerable<string> IReadOnlyDictionary<string, string>.Keys => _dictionary.Keys;
497+
498+
IEnumerable<string> IReadOnlyDictionary<string, string>.Values => _dictionary.Values;
499+
500+
int IReadOnlyCollection<KeyValuePair<string, string>>.Count => _dictionary.Count;
501+
502+
bool IReadOnlyDictionary<string, string>.ContainsKey(string key) => _dictionary.ContainsKey(key);
503+
504+
IEnumerator<KeyValuePair<string, string>> IEnumerable<KeyValuePair<string, string>>.GetEnumerator() => _dictionary.GetEnumerator();
505+
506+
IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_dictionary).GetEnumerator();
507+
508+
bool IReadOnlyDictionary<string, string>.TryGetValue(string key, out string value) => _dictionary.TryGetValue(key, out value);
509+
}
391510
}
392511
}

0 commit comments

Comments
 (0)