Skip to content

Commit

Permalink
Flow binder gen diag locations correctly & pick up more bind input pa…
Browse files Browse the repository at this point in the history
…tterns (#89537)

* Flow binder gen diag locations correctly & pick up more bind input patterns

* Fix CI issue

* Temporarily remove LOC failing on Build browser-wasm linux Release LibraryTests_EAT to unblock PR.

* Disable failing test for browser + reflection impl
  • Loading branch information
layomia committed Aug 1, 2023
1 parent ac464d2 commit b1f9634
Show file tree
Hide file tree
Showing 12 changed files with 383 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<BinderInvocation> _invocations;

private readonly HashSet<ITypeSymbol> _unsupportedTypes = new(SymbolEqualityComparer.Default);
private readonly Dictionary<ITypeSymbol, TypeSpec?> _createdSpecs = new(SymbolEqualityComparer.Default);
private readonly HashSet<ITypeSymbol> _unsupportedTypes = new(SymbolEqualityComparer.Default);

private readonly SourceGenerationSpec _sourceGenSpec = new();
private readonly List<InvocationDiagnosticInfo> _invocationTargetTypeDiags = new();
private readonly Dictionary<ITypeSymbol, HashSet<InvocationDiagnosticInfo>> _typeDiagnostics = new(SymbolEqualityComparer.Default);

public Parser(SourceProductionContext context, KnownTypeSymbols typeSymbols, ImmutableArray<BinderInvocation> invocations)
{
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<InvocationDiagnosticInfo>? typeDiags))
{
_invocationTargetTypeDiags.AddRange(typeDiags);
}

return spec;
}

Expand All @@ -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))
{
Expand All @@ -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;
}

Expand Down Expand Up @@ -293,7 +323,7 @@ private bool TryGetTypeSpec(ITypeSymbol type, DiagnosticDescriptor descriptor, o

if (spec is null)
{
ReportUnsupportedType(type, descriptor);
RegisterUnsupportedType(type, descriptor);
return false;
}

Expand Down Expand Up @@ -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)
{
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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;
}

Expand All @@ -399,7 +429,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc
}
else
{
ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location);
RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported);
return null;
}
}
Expand All @@ -420,7 +450,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc
}
else
{
ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location);
RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported);
return null;
}

Expand All @@ -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))
Expand Down Expand Up @@ -468,7 +498,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc
}
else
{
ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location);
RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported);
return null;
}
}
Expand Down Expand Up @@ -508,7 +538,7 @@ private DictionarySpec CreateDictionarySpec(INamedTypeSymbol type, Location? loc
}
else
{
ReportUnsupportedType(type, Diagnostics.CollectionNotSupported, location);
RegisterUnsupportedType(type, Diagnostics.CollectionNotSupported);
return null;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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<InvocationDiagnosticInfo>? typeDiags);
typeDiags ??= new HashSet<InvocationDiagnosticInfo>();
typeDiags.Add(info);

if (!typeHadDiags)
{
_typeDiagnostics[causingType] = typeDiags;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<MethodsToGen_ConfigurationBinder, HashSet<TypeSpec>> types = _sourceGenSpec.TypesForGen_ConfigurationBinder_BindMethods;

Expand All @@ -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
};
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,9 @@ private void GetIntDictionaryT<T>(T k1, T k2, T k3)
var config = configurationBuilder.Build();

var options = new Dictionary<T, string>();
#pragma warning disable SYSLIB1104
config.GetSection("IntegerKeyDictionary").Bind(options);
#pragma warning restore SYSLIB1104

Assert.Equal(3, options.Count);

Expand Down
Loading

0 comments on commit b1f9634

Please sign in to comment.