Skip to content

Commit 27568b2

Browse files
captainsafiacaptainsafia
committed
Harden handling for generic type arguments in source generator
1 parent df0c459 commit 27568b2

File tree

4 files changed

+278
-67
lines changed

4 files changed

+278
-67
lines changed

‎src/OpenApi/gen/XmlComments/MemberKey.cs‎

Lines changed: 175 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.Linq;
7-
using System.Text;
88
using Microsoft.CodeAnalysis;
99

1010
namespace Microsoft.AspNetCore.OpenApi.SourceGenerators.Xml;
@@ -14,14 +14,19 @@ internal sealed record MemberKey(
1414
MemberType MemberKind,
1515
string? Name,
1616
string? ReturnType,
17-
string[]? Parameters) : IEquatable<MemberKey>
17+
List<string>? Parameters) : IEquatable<MemberKey>
1818
{
19-
private static readonly SymbolDisplayFormat _typeKeyFormat = new(
19+
private static readonly SymbolDisplayFormat _withTypeParametersFormat = new(
2020
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Included,
2121
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
2222
genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters);
2323

24-
public static MemberKey FromMethodSymbol(IMethodSymbol method, Compilation compilation)
24+
private static readonly SymbolDisplayFormat _sansTypeParametersFormat = new(
25+
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Included,
26+
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
27+
genericsOptions: SymbolDisplayGenericsOptions.None);
28+
29+
public static MemberKey? FromMethodSymbol(IMethodSymbol method, Compilation compilation)
2530
{
2631
string returnType;
2732
if (method.ReturnsVoid)
@@ -44,94 +49,200 @@ public static MemberKey FromMethodSymbol(IMethodSymbol method, Compilation compi
4449
}
4550
}
4651

47-
returnType = actualReturnType.TypeKind == TypeKind.TypeParameter
48-
? "typeof(object)"
49-
: $"typeof({ReplaceGenericArguments(actualReturnType.ToDisplayString(_typeKeyFormat))})";
52+
if (actualReturnType.TypeKind == TypeKind.TypeParameter)
53+
{
54+
returnType = "typeof(object)";
55+
}
56+
else if (TryGetFormattedTypeName(actualReturnType, out var formattedReturnType))
57+
{
58+
returnType = $"typeof({formattedReturnType})";
59+
}
60+
else
61+
{
62+
return null;
63+
}
5064
}
5165

5266
// Handle extension methods by skipping the 'this' parameter
53-
var parameters = method.Parameters
54-
.Where(p => !p.IsThis)
55-
.Select(p =>
67+
List<string> parameters = [];
68+
foreach (var parameter in method.Parameters)
69+
{
70+
if (parameter.IsThis)
71+
{
72+
continue;
73+
}
74+
75+
if (parameter.Type.TypeKind == TypeKind.TypeParameter)
76+
{
77+
parameters.Add("typeof(object)");
78+
}
79+
else if (parameter.IsParams && parameter.Type is IArrayTypeSymbol arrayType)
5680
{
57-
if (p.Type.TypeKind == TypeKind.TypeParameter)
81+
if (TryGetFormattedTypeName(arrayType.ElementType, out var formattedArrayType))
5882
{
59-
return "typeof(object)";
83+
parameters.Add($"typeof({formattedArrayType}[])");
6084
}
61-
62-
// For params arrays, use the array type
63-
if (p.IsParams && p.Type is IArrayTypeSymbol arrayType)
85+
else
6486
{
65-
return $"typeof({ReplaceGenericArguments(arrayType.ToDisplayString(_typeKeyFormat))})";
87+
return null;
6688
}
89+
}
90+
else if (TryGetFormattedTypeName(parameter.Type, out var formattedParameterType))
91+
{
92+
parameters.Add($"typeof({formattedParameterType})");
93+
}
94+
else
95+
{
96+
return null;
97+
}
98+
}
6799

68-
return $"typeof({ReplaceGenericArguments(p.Type.ToDisplayString(_typeKeyFormat))})";
69-
})
70-
.ToArray();
71-
72-
// For generic methods, use the containing type with generic parameters
73-
var declaringType = method.ContainingType;
74-
var typeDisplay = declaringType.ToDisplayString(_typeKeyFormat);
75-
76-
// If the method is in a generic type, we need to handle the type parameters
77-
if (declaringType.IsGenericType)
100+
if (TryGetFormattedTypeName(method.ContainingType, out var formattedDeclaringType))
78101
{
79-
typeDisplay = ReplaceGenericArguments(typeDisplay);
102+
return new MemberKey(
103+
$"typeof({formattedDeclaringType})",
104+
MemberType.Method,
105+
method.MetadataName, // Use MetadataName to match runtime MethodInfo.Name
106+
returnType,
107+
parameters);
80108
}
81-
82-
return new MemberKey(
83-
$"typeof({typeDisplay})",
84-
MemberType.Method,
85-
method.MetadataName, // Use MetadataName to match runtime MethodInfo.Name
86-
returnType,
87-
parameters);
109+
return null;
88110
}
89111

90-
public static MemberKey FromPropertySymbol(IPropertySymbol property)
112+
public static MemberKey? FromPropertySymbol(IPropertySymbol property)
91113
{
92-
return new MemberKey(
93-
$"typeof({ReplaceGenericArguments(property.ContainingType.ToDisplayString(_typeKeyFormat))})",
94-
MemberType.Property,
95-
property.Name,
96-
null,
97-
null);
114+
if (TryGetFormattedTypeName(property.ContainingType, out var typeName))
115+
{
116+
return new MemberKey(
117+
$"typeof({typeName})",
118+
MemberType.Property,
119+
property.Name,
120+
null,
121+
null);
122+
}
123+
return null;
98124
}
99125

100-
public static MemberKey FromTypeSymbol(INamedTypeSymbol type)
126+
public static MemberKey? FromTypeSymbol(INamedTypeSymbol type)
101127
{
102-
return new MemberKey(
103-
$"typeof({ReplaceGenericArguments(type.ToDisplayString(_typeKeyFormat))})",
104-
MemberType.Type,
105-
null,
106-
null,
107-
null);
128+
if (TryGetFormattedTypeName(type, out var typeName))
129+
{
130+
return new MemberKey(
131+
$"typeof({typeName})",
132+
MemberType.Type,
133+
null,
134+
null,
135+
null);
136+
}
137+
return null;
108138
}
109139

110140
/// Supports replacing generic type arguments to support use of open
111141
/// generics in `typeof` expressions for the declaring type.
112-
private static string ReplaceGenericArguments(string typeName)
142+
private static bool TryGetFormattedTypeName(ITypeSymbol typeSymbol, [NotNullWhen(true)] out string? typeName, bool isNestedCall = false)
113143
{
114-
var stack = new Stack<int>();
115-
var result = new StringBuilder(typeName);
116-
for (var i = 0; i < result.Length; i++)
144+
if (typeSymbol is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } nullableType)
117145
{
118-
if (result[i] == '<')
146+
// For nullable value types, use the underlying type's name + ?
147+
if (TryGetFormattedTypeName(nullableType.TypeArguments[0], out var underlyingTypeName, isNestedCall))
119148
{
120-
stack.Push(i);
149+
typeName = $"{underlyingTypeName}?";
150+
return true;
121151
}
122-
else if (result[i] == '>' && stack.Count > 0)
152+
153+
typeName = null;
154+
return false;
155+
}
156+
157+
// Handle tuples specially since they are represented as generic
158+
// ValueTuple types and trigger the logic for handling generics in
159+
// nested values.
160+
if (typeSymbol is INamedTypeSymbol { IsTupleType: true } namedType)
161+
{
162+
return TryHandleTupleType(namedType, out typeName);
163+
}
164+
165+
if (typeSymbol is INamedTypeSymbol { IsGenericType: true } genericType)
166+
{
167+
// If any of the type arguments are type parameters, then they have not
168+
// been substituted for a concrete type and we need to model them as open
169+
// generics if possible to avoid emitting a type with type parameters that
170+
// cannot be used in a typeof expression.
171+
var hasTypeParameters = genericType.TypeArguments.Any(t => t.TypeKind == TypeKind.TypeParameter);
172+
var baseTypeName = genericType.ToDisplayString(_sansTypeParametersFormat);
173+
174+
if (!hasTypeParameters)
123175
{
124-
var start = stack.Pop();
125-
// Replace everything between < and > with empty strings separated by commas
126-
var segment = result.ToString(start + 1, i - start - 1);
127-
var commaCount = segment.Count(c => c == ',');
128-
var replacement = new string(',', commaCount);
129-
result.Remove(start + 1, i - start - 1);
130-
result.Insert(start + 1, replacement);
131-
i = start + replacement.Length + 1;
176+
var typeArgStrings = new List<string>();
177+
var allArgumentsResolved = true;
178+
179+
// Loop through each type argument to handle nested generics.
180+
foreach (var typeArg in genericType.TypeArguments)
181+
{
182+
if (TryGetFormattedTypeName(typeArg, out var argTypeName, isNestedCall: true))
183+
{
184+
typeArgStrings.Add(argTypeName);
185+
}
186+
else
187+
{
188+
typeName = null;
189+
return false;
190+
}
191+
}
192+
193+
if (allArgumentsResolved)
194+
{
195+
typeName = $"{baseTypeName}<{string.Join(", ", typeArgStrings)}>";
196+
return true;
197+
}
198+
}
199+
else
200+
{
201+
if (isNestedCall)
202+
{
203+
// If this is a nested call, we can't use open generics so there's no way
204+
// for us to emit a member key. Return false and skip over this type in the code
205+
// generation.
206+
typeName = null;
207+
return false;
208+
}
209+
210+
// If we got here, we can successfully emit a member key for the open generic type.
211+
var genericArgumentsCount = genericType.TypeArguments.Length;
212+
var openGenericsPlaceholder = "<" + new string(',', genericArgumentsCount - 1) + ">";
213+
214+
typeName = baseTypeName + openGenericsPlaceholder;
215+
return true;
132216
}
133217
}
134-
return result.ToString();
218+
219+
typeName = typeSymbol.ToDisplayString(_withTypeParametersFormat);
220+
return true;
221+
}
222+
223+
private static bool TryHandleTupleType(INamedTypeSymbol tupleType, [NotNullWhen(true)] out string? typeName)
224+
{
225+
List<string> elementTypes = [];
226+
foreach (var element in tupleType.TupleElements)
227+
{
228+
if (element.Type.TypeKind == TypeKind.TypeParameter)
229+
{
230+
elementTypes.Add("object");
231+
}
232+
else
233+
{
234+
// Process each tuple element and handle nested generics
235+
if (!TryGetFormattedTypeName(element.Type, out var elementTypeName, isNestedCall: true))
236+
{
237+
typeName = null;
238+
return false;
239+
}
240+
elementTypes.Add(elementTypeName);
241+
}
242+
}
243+
244+
typeName = $"global::System.ValueTuple<{string.Join(", ", elementTypes)}>";
245+
return true;
135246
}
136247
}
137248

0 commit comments

Comments
 (0)