-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix binder gen compile issues due to inaccessible members and identifier name clashes #91657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
082b7f1
737cd33
86b1afe
f3942d6
5cb4d0e
832c9bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| // 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) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| List<ITypeSymbol>? args = null; | ||
| TraverseContainingTypes(type); | ||
| return args; | ||
|
|
||
| void TraverseContainingTypes(INamedTypeSymbol current) | ||
| { | ||
| if (current.ContainingType is INamedTypeSymbol parent) | ||
| { | ||
| TraverseContainingTypes(parent); | ||
| } | ||
|
|
||
| if (!current.TypeArguments.IsEmpty) | ||
| { | ||
| (args ??= new()).AddRange(current.TypeArguments); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| 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); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| using System.Linq; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Operations; | ||
| using SourceGenerators; | ||
|
|
||
| namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration | ||
| { | ||
|
|
@@ -70,6 +71,7 @@ private static bool IsValidRootConfigType(ITypeSymbol? type) | |
| { | ||
| if (type is null || | ||
| type.SpecialType is SpecialType.System_Object or SpecialType.System_Void || | ||
| !IsAccessibleFromGenBinders(type) || | ||
layomia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error || | ||
| type.IsRefLikeType || | ||
| ContainsGenericParameters(type)) | ||
|
|
@@ -78,6 +80,27 @@ type.TypeKind is TypeKind.TypeParameter or TypeKind.Pointer or TypeKind.Error || | |
| } | ||
|
|
||
| return true; | ||
|
|
||
| static bool IsAccessibleFromGenBinders(ITypeSymbol type) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't you use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a "within" symbol. The symbol would be the generated binder extension class which doesn't exist yet.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't you use another type symbol in the same assembly as a proxy for the one that will be generated?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be possible to use the current IAssemblySymbol as the 'within' parameter although I haven't tested that myself.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I considered the proxy approach. It didn't feel deterministic on first thought, but yes it would be a correct/better check. I'll also try using the assembly symbol.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a proxy would work in the common case, but we'd need to account for the assembly possibly having just one (e.g. a singular, simple target-config POCO with primitive fields). Any handwritten fallback would have a vastly different impl. Comparing with assembly doesn't work - nested privates aren't visible.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Are any of the types being generated nested within user-defined types? If not, I would expect its visibility to be equivalent to that of the assembly?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I tried it again; the assembly check works. Must have not used the |
||
| { | ||
| 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; | ||
layomia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| private TypeSpec? GetTargetTypeForRootInvocation(ITypeSymbol? type, Location? invocationLocation) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.