Skip to content

Commit

Permalink
Minimize net diff of binder-gen visibility/identifier-clash fix (#91974)
Browse files Browse the repository at this point in the history 
* Minimize net diff of binder-gen visibility/identifier-clash fix

* Improve identifier formatting

* Use built-in API for accessiblity check
  • Loading branch information
@layomia
layomia authored Sep 14, 2023
1 parent 9fdabb6 commit adb0250
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 138 deletions.
84 changes: 0 additions & 84 deletions src/libraries/Common/src/SourceGenerators/TypeModelHelper.cs
View file
Original file line number Diff line number Diff line change
@@ -1,78 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text;
using Microsoft.CodeAnalysis;
using System.Collections.Generic;
using System.Diagnostics;

namespace SourceGenerators
{
internal static class TypeModelHelper
{
private static readonly SymbolDisplayFormat s_minimalDisplayFormat = new SymbolDisplayFormat(
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted,
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypes,
genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes);

public static string ToIdentifierCompatibleSubstring(this ITypeSymbol type, bool useUniqueName)
{
if (type is IArrayTypeSymbol arrayType)
{
int rank = arrayType.Rank;
string suffix = rank == 1 ? "Array" : $"Array{rank}D"; // Array, Array2D, Array3D, ...
return ToIdentifierCompatibleSubstring(arrayType.ElementType, useUniqueName) + suffix;
}

StringBuilder? sb = null;
string? symbolName = null;

if (useUniqueName)
{
string uniqueDisplayString = type.ToMinimalDisplayString();
PopulateIdentifierCompatibleSubstring(sb = new(), uniqueDisplayString);
}
else
{
symbolName = type.Name;
}

#if DEBUG
bool usingUniqueName = (symbolName is null || sb is not null) && useUniqueName;
bool usingSymbolName = (symbolName is not null && sb is null) && !useUniqueName;
bool configuredNameCorrectly = (usingUniqueName && !usingSymbolName) || (!usingUniqueName && usingSymbolName);
Debug.Assert(configuredNameCorrectly);
#endif

if (type is not INamedTypeSymbol namedType || !namedType.IsGenericType)
{
return symbolName ?? sb!.ToString();
}

if (sb is null)
{
(sb = new()).Append(symbolName!);
}

Debug.Assert(sb.Length > 0);

if (namedType.GetAllTypeArgumentsInScope() is List<ITypeSymbol> typeArgsInScope)
{
foreach (ITypeSymbol genericArg in typeArgsInScope)
{
sb.Append(ToIdentifierCompatibleSubstring(genericArg, useUniqueName));
}
}

return sb.ToString();
}

/// <summary>
/// Type name, prefixed with containing type names if it is nested (e.g. ContainingType.NestedType).
/// </summary>
public static string ToMinimalDisplayString(this ITypeSymbol type) => type.ToDisplayString(s_minimalDisplayFormat);

public static List<ITypeSymbol>? GetAllTypeArgumentsInScope(this INamedTypeSymbol type)
{
if (!type.IsGenericType)
Expand All @@ -97,24 +32,5 @@ void TraverseContainingTypes(INamedTypeSymbol current)
}
}
}

private static void PopulateIdentifierCompatibleSubstring(StringBuilder sb, string input)
{
foreach (char c in input)
{
if (c is '[')
{
sb.Append("Array");
}
else if (c is ',')
{
sb.Append("Comma");
}
else if (char.IsLetterOrDigit(c))
{
sb.Append(c);
}
}
}
}
}
46 changes: 2 additions & 44 deletions ...ies/Microsoft.Extensions.Configuration.Binder/gen/ConfigurationBindingGenerator.Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ public Parser(SourceProductionContext context, KnownTypeSymbols typeSymbols, Imm
return _sourceGenSpec;
}

private static bool IsValidRootConfigType(ITypeSymbol? type)
private bool IsValidRootConfigType(ITypeSymbol? type)
{
if (type is null ||
type.SpecialType is SpecialType.System_Object or SpecialType.System_Void ||
!IsAccessibleFromGenBinders(type) ||
!_typeSymbols.Compilation.IsSymbolAccessibleWithin(type, _typeSymbols.Compilation.Assembly) ||
type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error ||
type.IsRefLikeType ||
ContainsGenericParameters(type))
Expand All @@ -80,27 +80,6 @@ type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error ||
}

return true;

static bool IsAccessibleFromGenBinders(ITypeSymbol type)
{
if (type is IArrayTypeSymbol array)
{
return IsAccessibleFromGenBinders(array.ElementType);
}

if (type is INamedTypeSymbol namedType && namedType.GetAllTypeArgumentsInScope() is List<ITypeSymbol> typeArgsInScope)
{
foreach (ITypeSymbol genericArg in typeArgsInScope)
{
if (!IsAccessibleFromGenBinders(genericArg))
{
return false;
}
}
}

return type.DeclaredAccessibility is Accessibility.Public or Accessibility.Internal or Accessibility.Friend;
}
}

private TypeSpec? GetTargetTypeForRootInvocation(ITypeSymbol? type, Location? invocationLocation)
Expand Down Expand Up @@ -930,25 +909,4 @@ private void RegisterTypeDiagnostic(ITypeSymbol causingType, InvocationDiagnosti
}
}
}

internal static class ParserExtensions
{
public static void RegisterCacheEntry<TKey, TValue, TEntry>(this Dictionary<TKey, TValue> cache, TKey key, TEntry entry)
where TKey : notnull
where TValue : ICollection<TEntry>, new()
{
if (!cache.TryGetValue(key, out TValue? entryCollection))
{
cache[key] = entryCollection = new TValue();
}

entryCollection.Add(entry);
}

public static void Deconstruct(this KeyValuePair<TypeSpec, List<InterceptorLocationInfo>> source, out ComplexTypeSpec Key, out List<InterceptorLocationInfo> Value)
{
Key = (ComplexTypeSpec)source.Key;
Value = source.Value;
}
}
}
1 change: 1 addition & 0 deletions ...onfiguration.Binder/gen/Microsoft.Extensions.Configuration.Binder.SourceGeneration.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
<Compile Include="Parser\BinderInvocation.cs" />
<Compile Include="Parser\ConfigurationBinder.cs" />
<Compile Include="Parser\Diagnostics.cs" />
<Compile Include="Parser\Extensions.cs" />
<Compile Include="Parser\OptionsBuilderConfigurationExtensions.cs" />
<Compile Include="Parser\OptionsConfigurationServiceCollectionExtensions.cs" />
<Compile Include="Specs\InterceptorLocationInfo.cs" />
Expand Down
68 changes: 68 additions & 0 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Parser/Extensions.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Text;
using Microsoft.CodeAnalysis;
using SourceGenerators;

namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
internal static class ParserExtensions
{
private static readonly SymbolDisplayFormat s_identifierCompatibleFormat = new SymbolDisplayFormat(
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted,
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypes,
genericsOptions: SymbolDisplayGenericsOptions.None,
miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes);

public static void RegisterCacheEntry<TKey, TValue, TEntry>(this Dictionary<TKey, TValue> cache, TKey key, TEntry entry)
where TKey : notnull
where TValue : ICollection<TEntry>, new()
{
if (!cache.TryGetValue(key, out TValue? entryCollection))
{
cache[key] = entryCollection = new TValue();
}

entryCollection.Add(entry);
}

public static void Deconstruct(this KeyValuePair<TypeSpec, List<InterceptorLocationInfo>> source, out ComplexTypeSpec Key, out List<InterceptorLocationInfo> Value)
{
Key = (ComplexTypeSpec)source.Key;
Value = source.Value;
}

public static string ToIdentifierCompatibleSubstring(this ITypeSymbol type)
{
if (type is IArrayTypeSymbol arrayType)
{
int rank = arrayType.Rank;
string suffix = rank == 1 ? "Array" : $"Array{rank}D"; // Array, Array2D, Array3D, ...
return ToIdentifierCompatibleSubstring(arrayType.ElementType) + suffix;
}

string displayString = type.ContainingType is null
? type.Name
: type.ToDisplayString(s_identifierCompatibleFormat).Replace(".", string.Empty);

if (type is not INamedTypeSymbol { IsGenericType: true } namedType)
{
return displayString;
}

StringBuilder sb = new(displayString);

if (namedType.GetAllTypeArgumentsInScope() is List<ITypeSymbol> typeArgsInScope)
{
foreach (ITypeSymbol genericArg in typeArgsInScope)
{
sb.Append(ToIdentifierCompatibleSubstring(genericArg));
}
}

return sb.ToString();
}
}
}
4 changes: 4 additions & 0 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/KnownTypeSymbols.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
internal sealed record KnownTypeSymbols
{
public CSharpCompilation Compilation { get; }

public INamedTypeSymbol String { get; }
public INamedTypeSymbol? CultureInfo { get; }
public INamedTypeSymbol? DateOnly { get; }
Expand Down Expand Up @@ -57,6 +59,8 @@ internal sealed record KnownTypeSymbols

public KnownTypeSymbols(CSharpCompilation compilation)
{
Compilation = compilation;

// Primitives (needed because they are Microsoft.CodeAnalysis.SpecialType.None)
CultureInfo = compilation.GetBestTypeByMetadataName(typeof(CultureInfo));
DateOnly = compilation.GetBestTypeByMetadataName("System.DateOnly");
Expand Down
11 changes: 8 additions & 3 deletions src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Specs/Types/TypeSpec.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,24 @@

using System.Diagnostics;
using Microsoft.CodeAnalysis;
using SourceGenerators;

namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
[DebuggerDisplay("Name={DisplayString}, Kind={SpecKind}")]
internal abstract record TypeSpec
{
private static readonly SymbolDisplayFormat s_minimalDisplayFormat = new SymbolDisplayFormat(
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted,
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypes,
genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes);

public TypeSpec(ITypeSymbol type)
{
Namespace = type.ContainingNamespace?.ToDisplayString();
DisplayString = type.ToMinimalDisplayString();
DisplayString = type.ToDisplayString(s_minimalDisplayFormat);
Name = (Namespace is null ? string.Empty : Namespace + ".") + DisplayString.Replace(".", "+");
IdentifierCompatibleSubstring = type.ToIdentifierCompatibleSubstring(useUniqueName: true);
IdentifierCompatibleSubstring = type.ToIdentifierCompatibleSubstring();
IsValueType = type.IsValueType;
}

Expand Down
6 changes: 3 additions & 3 deletions ....Configuration.Binder/tests/SourceGenerationTests/Baselines/EmptyConfigType.generated.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration

/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
[InterceptsLocation(@"src-0.cs", 15, 23)]
public static void Bind_TypeWithNoMembersWrapper(this IConfiguration configuration, object? instance)
public static void Bind_TypeWithNoMembers_Wrapper(this IConfiguration configuration, object? instance)
{
if (configuration is null)
{
Expand All @@ -56,11 +56,11 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
#endregion IConfiguration extensions.

#region Core binding extensions.
private readonly static Lazy<HashSet<string>> s_configKeys_TypeWithNoMembersWrapper = new(() => new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "Member" });
private readonly static Lazy<HashSet<string>> s_configKeys_TypeWithNoMembers_Wrapper = new(() => new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "Member" });

public static void BindCore(IConfiguration configuration, ref TypeWithNoMembers_Wrapper instance, BinderOptions? binderOptions)
{
ValidateConfigurationKeys(typeof(TypeWithNoMembers_Wrapper), s_configKeys_TypeWithNoMembersWrapper, configuration, binderOptions);
ValidateConfigurationKeys(typeof(TypeWithNoMembers_Wrapper), s_configKeys_TypeWithNoMembers_Wrapper, configuration, binderOptions);

if (AsConfigWithChildren(configuration.GetSection("Member")) is IConfigurationSection section0)
{
Expand Down
37 changes: 33 additions & 4 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,19 +590,17 @@ private TypeGenerationSpec ParseTypeGenerationSpec(in TypeToGenerate typeToGener
}

var typeRef = new TypeRef(type);
string typeInfoPropertyName = typeToGenerate.TypeInfoPropertyName ??
type.ToIdentifierCompatibleSubstring(useUniqueName: false /* We leave identifier name conflicts to users, as codified below. */);
string typeInfoPropertyName = typeToGenerate.TypeInfoPropertyName ?? GetTypeInfoPropertyName(type);

if (classType is ClassType.TypeUnsupportedBySourceGen)
{
ReportDiagnostic(DiagnosticDescriptors.TypeNotSupported, typeToGenerate.AttributeLocation ?? typeToGenerate.Location, type.ToDisplayString());
}

// Workaround for https://github.com/dotnet/roslyn/issues/54185 by keeping track of the file names we've used.
if (!_generatedContextAndTypeNames.Add((contextType.Name, typeInfoPropertyName)))
{
// The context name/property name combination will result in a conflict in generated types.
// This is by design; we leave conflict resolution to the user.
// Workaround for https://github.com/dotnet/roslyn/issues/54185 by keeping track of the file names we've used.
ReportDiagnostic(DiagnosticDescriptors.DuplicateTypeName, typeToGenerate.AttributeLocation ?? _contextClassLocation, typeInfoPropertyName);
classType = ClassType.TypeUnsupportedBySourceGen;
}
Expand Down Expand Up @@ -1592,6 +1590,37 @@ private static string DeterminePropertyNameFieldName(string effectiveJsonPropert
return null;
}

private static string GetTypeInfoPropertyName(ITypeSymbol type)
{
if (type is IArrayTypeSymbol arrayType)
{
int rank = arrayType.Rank;
string suffix = rank == 1 ? "Array" : $"Array{rank}D"; // Array, Array2D, Array3D, ...
return GetTypeInfoPropertyName(arrayType.ElementType) + suffix;
}

if (type is not INamedTypeSymbol namedType || !namedType.IsGenericType)
{
return type.Name;
}

StringBuilder sb = new();

string name = namedType.Name;

sb.Append(name);

if (namedType.GetAllTypeArgumentsInScope() is List<ITypeSymbol> typeArgsInScope)
{
foreach (ITypeSymbol genericArg in typeArgsInScope)
{
sb.Append(GetTypeInfoPropertyName(genericArg));
}
}

return sb.ToString();
}

private bool TryGetDeserializationConstructor(
ITypeSymbol type,
bool useDefaultCtorInAnnotatedStructs,
Expand Down

0 comments on commit adb0250

Please sign in to comment.