Address feedback, add MemberKey, harden tests

Author: captainsafia

src/OpenApi/build/Microsoft.AspNetCore.OpenApi.targets

@@ -2,8 +2,6 @@
 <Project>
   <PropertyGroup>
     <InterceptorsNamespaces>$(InterceptorsNamespaces);Microsoft.AspNetCore.OpenApi.Generated</InterceptorsNamespaces>
-    <InterceptorsPreviewNamespaces>
-      $(InterceptorsPreviewNamespaces);Microsoft.AspNetCore.OpenApi.Generated</InterceptorsPreviewNamespaces>
   </PropertyGroup>
   <Target Name="GenerateAdditionalXmlFilesForOpenApi"
     AfterTargets="ResolveReferences">

src/OpenApi/gen/XmlCommentGenerator.Emitter.cs

@@ -8,6 +8,7 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.AspNetCore.OpenApi.SourceGenerators.Xml;
 using System.Threading;
+using System.Linq;
 
 namespace Microsoft.AspNetCore.OpenApi.SourceGenerators;
 
@@ -74,28 +75,149 @@ file record XmlComment(
     {{GeneratedCodeAttribute}}
     file record XmlResponseComment(string Code, string? Description, string? Example);
 
+    {{GeneratedCodeAttribute}}
+    file sealed record MemberKey(
+        Type? DeclaringType,
+        MemberType MemberKind,
+        string? Name,
+        Type? ReturnType,
+        Type[]? Parameters) : IEquatable<MemberKey>
+    {
+        public bool Equals(MemberKey? other)
+        {
+            if (other is null) return false;
+
+            // Check member kind
+            if (MemberKind != other.MemberKind) return false;
+
+            // Check declaring type, handling generic types
+            if (!TypesEqual(DeclaringType, other.DeclaringType)) return false;
+
+            // Check name
+            if (Name != other.Name) return false;
+
+            // For methods, check return type and parameters
+            if (MemberKind == MemberType.Method)
+            {
+                if (!TypesEqual(ReturnType, other.ReturnType)) return false;
+                if (Parameters is null || other.Parameters is null) return false;
+                if (Parameters.Length != other.Parameters.Length) return false;
+
+                for (int i = 0; i < Parameters.Length; i++)
+                {
+                    if (!TypesEqual(Parameters[i], other.Parameters[i])) return false;
+                }
+            }
+
+            return true;
+        }
+
+        private static bool TypesEqual(Type? type1, Type? type2)
+        {
+            if (type1 == type2) return true;
+            if (type1 == null || type2 == null) return false;
+
+            if (type1.IsGenericType && type2.IsGenericType)
+            {
+                return type1.GetGenericTypeDefinition() == type2.GetGenericTypeDefinition();
+            }
+
+            return type1 == type2;
+        }
+
+        public override int GetHashCode()
+        {
+            var hash = new HashCode();
+            hash.Add(GetTypeHashCode(DeclaringType));
+            hash.Add(MemberKind);
+            hash.Add(Name);
+
+            if (MemberKind == MemberType.Method)
+            {
+                hash.Add(GetTypeHashCode(ReturnType));
+                if (Parameters is not null)
+                {
+                    foreach (var param in Parameters)
+                    {
+                        hash.Add(GetTypeHashCode(param));
+                    }
+                }
+            }
+
+            return hash.ToHashCode();
+        }
+
+        private static int GetTypeHashCode(Type? type)
+        {
+            if (type == null) return 0;
+            return type.IsGenericType ? type.GetGenericTypeDefinition().GetHashCode() : type.GetHashCode();
+        }
+
+        public static MemberKey FromMethodInfo(MethodInfo method)
+        {
+            return new MemberKey(
+                method.DeclaringType,
+                MemberType.Method,
+                method.Name,
+                method.ReturnType.IsGenericParameter ? typeof(object) : method.ReturnType,
+                method.GetParameters().Select(p => p.ParameterType.IsGenericParameter ? typeof(object) : p.ParameterType).ToArray());
+        }
+
+        public static MemberKey FromPropertyInfo(PropertyInfo property)
+        {
+            return new MemberKey(
+                property.DeclaringType,
+                MemberType.Property,
+                property.Name,
+                null,
+                null);
+        }
+
+        public static MemberKey FromTypeInfo(Type type)
+        {
+            return new MemberKey(
+                type,
+                MemberType.Type,
+                null,
+                null,
+                null);
+        }
+    }
+
+    file enum MemberType
+    {
+        Type,
+        Property,
+        Method
+    }
+
     {{GeneratedCodeAttribute}}
     file static class XmlCommentCache
     {
-        private static Dictionary<(Type?, string?), XmlComment>? _cache;
-        public static Dictionary<(Type?, string?), XmlComment> Cache => _cache ??= GenerateCacheEntries();
+        private static Dictionary<MemberKey, XmlComment>? _cache;
+        public static Dictionary<MemberKey, XmlComment> Cache => _cache ??= GenerateCacheEntries();
 
-        private static Dictionary<(Type?, string?), XmlComment> GenerateCacheEntries()
+        private static Dictionary<MemberKey, XmlComment> GenerateCacheEntries()
         {
-            var _cache = new Dictionary<(Type?, string?), XmlComment>();
+            var _cache = new Dictionary<MemberKey, XmlComment>();
 {{commentsFromXmlFile}}
 {{commentsFromCompilation}}
             return _cache;
         }
 
-        internal static bool TryGetXmlComment(Type? type, string? memberName, [NotNullWhen(true)] out XmlComment? xmlComment)
+        internal static bool TryGetXmlComment(Type? type, MethodInfo? methodInfo, [NotNullWhen(true)] out XmlComment? xmlComment)
         {
-            if (type is not null && type.IsGenericType)
+            if (methodInfo is null)
             {
-                type = type.GetGenericTypeDefinition();
+                return Cache.TryGetValue(new MemberKey(type, MemberType.Property, null, null, null), out xmlComment);
             }
 
-            return XmlCommentCache.Cache.TryGetValue((type, memberName), out xmlComment);
+            return Cache.TryGetValue(MemberKey.FromMethodInfo(methodInfo), out xmlComment);
+        }
+
+        internal static bool TryGetXmlComment(Type? type, string? memberName, [NotNullWhen(true)] out XmlComment? xmlComment)
+        {
+            return Cache.TryGetValue(new MemberKey(type, memberName is null ? MemberType.Type : MemberType.Property, memberName, null, null), out xmlComment);
         }
     }
 
@@ -112,7 +234,7 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
             {
                 return Task.CompletedTask;
             }
-            if (XmlCommentCache.TryGetXmlComment(methodInfo.DeclaringType, methodInfo.Name, out var methodComment))
+            if (XmlCommentCache.TryGetXmlComment(methodInfo.DeclaringType, methodInfo, out var methodComment))
             {
                 if (methodComment.Summary is { } summary)
                 {
@@ -167,7 +289,6 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform
                         {
                             response.Value.Description = responseComment.Description;
                         }
-
                     }
                 }
             }
@@ -192,8 +313,7 @@ public Task TransformAsync(OpenApiSchema schema, OpenApiSchemaTransformerContext
                     }
                 }
             }
-            System.Diagnostics.Debugger.Break();
-            if (XmlCommentCache.TryGetXmlComment(context.JsonTypeInfo.Type, null, out var typeComment))
+            if (XmlCommentCache.TryGetXmlComment(context.JsonTypeInfo.Type, (string?)null, out var typeComment))
             {
                 schema.Description = typeComment.Summary;
                 if (typeComment.Examples?.FirstOrDefault() is { } jsonString)
@@ -268,9 +388,9 @@ public static IServiceCollection AddOpenApi(this IServiceCollection services, Ac
                 {
                     return services.AddOpenApi("v1", options =>
                     {
-                        configureOptions(options);
                         options.AddSchemaTransformer(new XmlCommentSchemaTransformer());
                         options.AddOperationTransformer(new XmlCommentOperationTransformer());
+                        configureOptions(options);
                     });
                 }
         """,
@@ -280,9 +400,9 @@ public static IServiceCollection AddOpenApi(this IServiceCollection services, st
                     // This overload is not intercepted.
                     return OpenApiServiceCollectionExtensions.AddOpenApi(services, documentName, options =>
                     {
-                        configureOptions(options);
                         options.AddSchemaTransformer(new XmlCommentSchemaTransformer());
                         options.AddOperationTransformer(new XmlCommentOperationTransformer());
+                        configureOptions(options);
                     });
                 }
         """,
@@ -307,20 +427,29 @@ internal static string GenerateAddOpenApiInterceptions(ImmutableArray<(AddOpenAp
         return writer.ToString();
     }
 
-    internal static string EmitCommentsCache(IEnumerable<(string, string?, XmlComment?)> comments, CancellationToken cancellationToken)
+    internal static string EmitCommentsCache(IEnumerable<(MemberKey MemberKey, XmlComment? Comment)> comments, CancellationToken cancellationToken)
     {
         var writer = new StringWriter();
         var codeWriter = new CodeWriter(writer, baseIndent: 3);
-        foreach (var (type, member, comment) in comments)
+        foreach (var (memberKey, comment) in comments)
         {
             if (comment is not null)
             {
-                var typeKey = $"(typeof({type})";
-                var memberKey = member is not null ? $"{SymbolDisplay.FormatLiteral(member, true)}" : "null";
-                codeWriter.WriteLine($"_cache.Add({typeKey}, {memberKey}), {EmitSourceGeneratedXmlComment(comment)});");
+                codeWriter.WriteLine($"_cache.Add(new MemberKey(" +
+                    $"{FormatLiteralOrNull(memberKey.DeclaringType)}, " +
+                    $"MemberType.{memberKey.MemberKind}, " +
+                    $"{FormatLiteralOrNull(memberKey.Name, true)}, " +
+                    $"{FormatLiteralOrNull(memberKey.ReturnType)}, " +
+                    $"[{(memberKey.Parameters != null ? string.Join(", ", memberKey.Parameters.Select(p => SymbolDisplay.FormatLiteral(p, false))) : "")}]), " +
+                    $"{EmitSourceGeneratedXmlComment(comment)});");
             }
         }
         return writer.ToString();
+
+        static string FormatLiteralOrNull(string? input, bool quote = false)
+        {
+            return input == null ? "null" : SymbolDisplay.FormatLiteral(input, quote);
+        }
     }
 
     private static string FormatStringForCode(string? input)

src/OpenApi/gen/XmlCommentGenerator.Parser.cs

@@ -3,37 +3,38 @@
 
 using System.Collections.Generic;
 using System.Globalization;
-using System.Linq;
 using System.Threading;
 using System.Xml.Linq;
 using Microsoft.AspNetCore.OpenApi.SourceGenerators.Xml;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
-using System.Text;
 
 namespace Microsoft.AspNetCore.OpenApi.SourceGenerators;
 
 public sealed partial class XmlCommentGenerator
 {
-    private static readonly SymbolDisplayFormat _typeKeyFormat = new(
-        globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Included,
-        typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
-        genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters);
-
     internal static List<(string, string)> ParseXmlFile(AdditionalText additionalText, CancellationToken cancellationToken)
     {
         var text = additionalText.GetText(cancellationToken);
         if (text is null)
         {
             return [];
         }
-        var xml = XDocument.Parse(text.ToString());
+        XElement xml;
+        try
+        {
+            xml = XElement.Parse(text.ToString());
+        }
+        catch
+        {
+            return [];
+        }
         var members = xml.Descendants("member");
         var comments = new List<(string, string)>();
         foreach (var member in members)
         {
-            var name = member.Attribute("name")?.Value;
+            var name = member.Attribute(DocumentationCommentXmlNames.NameAttributeName)?.Value;
             if (name is not null)
             {
                 comments.Add((name, member.ToString()));
@@ -82,55 +83,30 @@ public sealed partial class XmlCommentGenerator
         return comments;
     }
 
-    // Type names are used in a `typeof()` expression, so we need to replace generic arguments
-    // with empty strings to avoid compiler errors.
-    private static string ReplaceGenericArguments(string typeName)
-    {
-        var stack = new Stack<int>();
-        var result = new StringBuilder(typeName);
-
-        for (var i = 0; i < result.Length; i++)
-        {
-            if (result[i] == '<')
-            {
-                stack.Push(i);
-            }
-            else if (result[i] == '>' && stack.Count > 0)
-            {
-                var start = stack.Pop();
-                // Replace everything between < and > with empty strings separated by commas
-                var segment = result.ToString(start + 1, i - start - 1);
-                var commaCount = segment.Count(c => c == ',');
-                var replacement = new string(',', commaCount);
-                result.Remove(start + 1, i - start - 1);
-                result.Insert(start + 1, replacement);
-                i = start + replacement.Length + 1;
-            }
-        }
-
-        return result.ToString();
-    }
-
-    internal static IEnumerable<(string, string?, XmlComment?)> ParseComments(
+    internal static IEnumerable<(MemberKey, XmlComment?)> ParseComments(
         (List<(string, string)> RawComments, Compilation Compilation) input,
         CancellationToken cancellationToken)
     {
         var compilation = input.Compilation;
-        var comments = new List<(string, string?, XmlComment?)>();
+        var comments = new List<(MemberKey, XmlComment?)>();
         foreach (var (name, value) in input.RawComments)
         {
             if (DocumentationCommentId.GetFirstSymbolForDeclarationId(name, compilation) is ISymbol symbol)
             {
                 var parsedComment = XmlComment.Parse(symbol, compilation, value, cancellationToken);
                 if (parsedComment is not null)
                 {
-                    var typeInfo = symbol is IPropertySymbol or IMethodSymbol
-                    ? ReplaceGenericArguments(symbol.ContainingType.OriginalDefinition.ToDisplayString(_typeKeyFormat))
-                    : ReplaceGenericArguments(symbol.OriginalDefinition.ToDisplayString(_typeKeyFormat));
-                    var propertyInfo = symbol is IPropertySymbol or IMethodSymbol
-                        ? symbol.Name
-                        : null;
-                    comments.Add((typeInfo, propertyInfo, parsedComment));
+                    var memberKey = symbol switch
+                    {
+                        IMethodSymbol methodSymbol => MemberKey.FromMethodSymbol(methodSymbol),
+                        IPropertySymbol propertySymbol => MemberKey.FromPropertySymbol(propertySymbol),
+                        INamedTypeSymbol typeSymbol => MemberKey.FromTypeSymbol(typeSymbol),
+                        _ => null
+                    };
+                    if (memberKey is not null)
+                    {
+                        comments.Add((memberKey, parsedComment));
+                    }
                 }
             }
         }
@@ -174,10 +150,14 @@ internal static AddOpenApiInvocation GetAddOpenApiOverloadVariant(GeneratorSynta
             {
                 return new(AddOpenApiOverloadVariant.AddOpenApiDocumentName, invocationExpression, interceptableLocation);
             }
-            else
+            else if (argument.Expression is LambdaExpressionSyntax)
             {
                 return new(AddOpenApiOverloadVariant.AddOpenApiConfigureOptions, invocationExpression, interceptableLocation);
             }
+            else
+            {
+                return new(AddOpenApiOverloadVariant.Unknown, invocationExpression, null);
+            }
         }
     }
 }