Merge pull request #1796 from MessagePack-CSharp/fix1739

Include all hand-written formatters in the source generated resolver

Directory.Packages.props

@@ -80,7 +80,6 @@
   </ItemGroup>
   <ItemGroup>
     <GlobalPackageReference Include="CSharpIsNullAnalyzer" Version="0.1.495" />
-    <GlobalPackageReference Include="DotNetAnalyzers.DocumentationAnalyzers" Version="1.0.0-beta.59" />
     <GlobalPackageReference Include="Nerdbank.GitVersioning" Version="3.6.133" />
     <GlobalPackageReference Include="Nullable" Version="1.3.1" />
     <GlobalPackageReference Include="StyleCop.Analyzers.Unstable" Version="1.2.0.556" />

MessagePack.sln

@@ -7,7 +7,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{86309CF6-005
 	ProjectSection(SolutionItems) = preProject
 		src\Directory.Build.props = src\Directory.Build.props
 		src\Directory.Build.targets = src\Directory.Build.targets
-		src\SourceGenerator.props = src\SourceGenerator.props
 	EndProjectSection
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MessagePack", "src\MessagePack\MessagePack.csproj", "{7ABB33EE-A2F1-492B-8DAF-5DF89F0F0B79}"

README.md

@@ -1625,7 +1625,7 @@ so you should only need to interact with this resolver directly for advanced sce
 
 Leveraging this resolver at runtime happens automatically by default,
 since the `StandardResolver` includes the `SourceGeneratedFormatterResolver`
-which discovers and your source generated resolver.
+which discovers and uses your source generated resolver.
 
 ### Customizations
 

sandbox/Sandbox/Program.cs

@@ -253,8 +253,8 @@ public CustomObject()
             this.internalId = Guid.NewGuid().ToString();
         }
 
-        // serialize/deserialize internal field.
-        private class CustomObjectFormatter : IMessagePackFormatter<CustomObject>
+        // serialize/deserialize private field.
+        internal class CustomObjectFormatter : IMessagePackFormatter<CustomObject>
         {
             public void Serialize(ref MessagePackWriter writer, CustomObject value, MessagePackSerializerOptions options)
             {

src/MessagePack.SourceGenerator/AnalyzerReleases.Unshipped.md

@@ -12,4 +12,6 @@ MsgPack004 | Usage | Error | MsgPack00xMessagePackAnalyzer
 MsgPack005 | Usage | Error | MsgPack00xMessagePackAnalyzer
 MsgPack006 | Usage | Error | MsgPack00xMessagePackAnalyzer
 MsgPack007 | Usage | Error | MsgPack00xMessagePackAnalyzer
-MsgPack008 | Usage | Error | MsgPack00xMessagePackAnalyzer
+MsgPack008 | Usage | Error | MsgPack00xMessagePackAnalyzer
+MsgPack009 | Usage | Error | MsgPack00xMessagePackAnalyzer
+MsgPack010 | Usage | Warning | MsgPack00xMessagePackAnalyzer

src/MessagePack.SourceGenerator/Analyzers/MsgPack00xMessagePackAnalyzer.cs

@@ -17,6 +17,8 @@ public class MsgPack00xMessagePackAnalyzer : DiagnosticAnalyzer
     public const string MessagePackFormatterMustBeMessagePackFormatterId = "MsgPack006";
     public const string DeserializingConstructorId = "MsgPack007";
     public const string AOTLimitationsId = "MsgPack008";
+    public const string CollidingFormattersId = "MsgPack009";
+    public const string InaccessibleFormatterId = "MsgPack010";
 
     internal const string Category = "Usage";
 
@@ -185,11 +187,33 @@ public class MsgPack00xMessagePackAnalyzer : DiagnosticAnalyzer
         isEnabledByDefault: true,
         helpLinkUri: AnalyzerUtilities.GetHelpLink(AOTLimitationsId));
 
+    internal static readonly DiagnosticDescriptor CollidingFormatters = new(
+        id: CollidingFormattersId,
+        title: "Colliding formatters",
+        category: Category,
+        messageFormat: "Multiple formatters for type {0} found",
+        description: "Only one formatter per type is allowed.",
+        defaultSeverity: DiagnosticSeverity.Error,
+        isEnabledByDefault: true,
+        helpLinkUri: AnalyzerUtilities.GetHelpLink(CollidingFormattersId));
+
+    internal static readonly DiagnosticDescriptor InaccessibleFormatter = new(
+        id: InaccessibleFormatterId,
+        title: "Inaccessible formatter",
+        category: Category,
+        messageFormat: "Formatter should declare a default constructor with at least internal visibility",
+        description: "The auto-generated resolver cannot construct this formatter without a constructor.",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        helpLinkUri: AnalyzerUtilities.GetHelpLink(InaccessibleFormatterId));
+
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
         TypeMustBeMessagePackObject,
         PublicMemberNeedsKey,
         InvalidMessagePackObject,
-        MessageFormatterMustBeMessagePackFormatter);
+        MessageFormatterMustBeMessagePackFormatter,
+        CollidingFormatters,
+        InaccessibleFormatter);
 
     public override void Initialize(AnalysisContext context)
     {
@@ -199,7 +223,12 @@ public override void Initialize(AnalysisContext context)
         {
             if (ReferenceSymbols.TryCreate(context.Compilation, out ReferenceSymbols? typeReferences))
             {
-                AnalyzerOptions options = new AnalyzerOptions().WithAssemblyAttributes(context.Compilation.Assembly.GetAttributes(), context.CancellationToken);
+                // Search the compilation for implementations of IMessagePackFormatter<T>.
+                ImmutableHashSet<CustomFormatter> formatterTypes = this.SearchNamespaceForFormatters(context.Compilation.Assembly.GlobalNamespace).ToImmutableHashSet();
+
+                AnalyzerOptions options = new AnalyzerOptions()
+                    .WithFormatterTypes(ImmutableArray<FormattableType>.Empty, formatterTypes)
+                    .WithAssemblyAttributes(context.Compilation.Assembly.GetAttributes(), context.CancellationToken);
                 context.RegisterSymbolAction(context => this.AnalyzeSymbol(context, typeReferences, options), SymbolKind.NamedType);
             }
         });
@@ -208,6 +237,23 @@ public override void Initialize(AnalysisContext context)
     private void AnalyzeSymbol(SymbolAnalysisContext context, ReferenceSymbols typeReferences, AnalyzerOptions options)
     {
         INamedTypeSymbol declaredSymbol = (INamedTypeSymbol)context.Symbol;
+        QualifiedTypeName typeName = new(declaredSymbol);
+
+        // If this is a formatter, confirm that it meets requirements.
+        if (options.KnownFormattersByName.TryGetValue(typeName, out CustomFormatter? formatter))
+        {
+            // Look for colliding formatters (multiple formatters that want to format the same type).
+            foreach (FormattableType formattableType in options.GetCollidingFormatterDataTypes(typeName))
+            {
+                context.ReportDiagnostic(Diagnostic.Create(CollidingFormatters, declaredSymbol.Locations[0], formattableType.Name.GetQualifiedName(Qualifiers.Namespace)));
+            }
+
+            if (formatter.IsInaccessible)
+            {
+                context.ReportDiagnostic(Diagnostic.Create(InaccessibleFormatter, declaredSymbol.Locations[0]));
+            }
+        }
+
         switch (declaredSymbol.TypeKind)
         {
             case TypeKind.Interface when declaredSymbol.GetAttributes().Any(x2 => SymbolEqualityComparer.Default.Equals(x2.AttributeClass, typeReferences.UnionAttribute)):
@@ -216,4 +262,23 @@ private void AnalyzeSymbol(SymbolAnalysisContext context, ReferenceSymbols typeR
                 break;
         }
     }
+
+    private IEnumerable<CustomFormatter> SearchNamespaceForFormatters(INamespaceSymbol ns)
+    {
+        foreach (INamespaceSymbol childNamespace in ns.GetNamespaceMembers())
+        {
+            foreach (CustomFormatter x in this.SearchNamespaceForFormatters(childNamespace))
+            {
+                yield return x;
+            }
+        }
+
+        foreach (INamedTypeSymbol type in ns.GetTypeMembers())
+        {
+            if (CustomFormatter.TryCreate(type, out CustomFormatter? formatter))
+            {
+                yield return formatter;
+            }
+        }
+    }
 }

src/MessagePack.SourceGenerator/CodeAnalysis/AnalyzerOptions.cs

@@ -4,6 +4,7 @@
 #pragma warning disable SA1402 // File may only contain a single type
 
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.CodeAnalysis;
 
 namespace MessagePack.SourceGenerator.CodeAnalysis;
@@ -16,16 +17,71 @@ namespace MessagePack.SourceGenerator.CodeAnalysis;
 /// </remarks>
 public record AnalyzerOptions
 {
+    private readonly ImmutableHashSet<CustomFormatter> knownFormatters = ImmutableHashSet<CustomFormatter>.Empty;
+
+    private readonly ImmutableDictionary<QualifiedTypeName, ImmutableArray<FormattableType>> collidingFormatters = ImmutableDictionary<QualifiedTypeName, ImmutableArray<FormattableType>>.Empty;
+
     /// <summary>
     /// Gets the set fully qualified names of types that are assumed to have custom formatters written that will be included by a resolver by the program.
     /// </summary>
-    public ImmutableHashSet<string> AssumedFormattableTypes { get; init; } = ImmutableHashSet<string>.Empty;
+    public ImmutableHashSet<FormattableType> AssumedFormattableTypes { get; init; } = ImmutableHashSet<FormattableType>.Empty;
 
     /// <summary>
-    /// Gets the set fully qualified names of custom formatters that should be considered by the analyzer and included in the generated resolver,
-    /// and the collection of types that they can format.
+    /// Gets the set of custom formatters that should be considered by the analyzer and included in the generated resolver.
     /// </summary>
-    public ImmutableDictionary<string, ImmutableHashSet<string>> KnownFormatters { get; init; } = ImmutableDictionary<string, ImmutableHashSet<string>>.Empty;
+    public ImmutableHashSet<CustomFormatter> KnownFormatters
+    {
+        get => this.knownFormatters;
+        init
+        {
+            this.knownFormatters = value;
+            this.KnownFormattersByName = value.ToImmutableDictionary(f => f.Name);
+
+            Dictionary<FormattableType, ImmutableArray<CustomFormatter>> formattableTypes = new();
+            bool collisionsEncountered = false;
+            foreach (CustomFormatter formatter in value)
+            {
+                foreach (FormattableType dataType in formatter.FormattableTypes)
+                {
+                    if (formattableTypes.ContainsKey(dataType))
+                    {
+                        formattableTypes[dataType] = formattableTypes[dataType].Add(formatter);
+                        collisionsEncountered = true;
+                    }
+                    else
+                    {
+                        formattableTypes.Add(dataType, ImmutableArray.Create(formatter));
+                    }
+                }
+            }
+
+            var collidingFormatters = ImmutableDictionary<QualifiedTypeName, ImmutableArray<FormattableType>>.Empty;
+            if (collisionsEncountered)
+            {
+                foreach (KeyValuePair<FormattableType, ImmutableArray<CustomFormatter>> kvp in formattableTypes)
+                {
+                    if (kvp.Value.Length > 1)
+                    {
+                        foreach (CustomFormatter collidingFormatter in kvp.Value)
+                        {
+                            if (collidingFormatters.TryGetValue(collidingFormatter.Name, out ImmutableArray<FormattableType> collidingTypes))
+                            {
+                                collidingFormatters = collidingFormatters.SetItem(collidingFormatter.Name, collidingTypes.Add(kvp.Key));
+                            }
+                            else
+                            {
+                                collidingFormatters = collidingFormatters.Add(collidingFormatter.Name, ImmutableArray.Create(kvp.Key));
+                            }
+                        }
+                    }
+                }
+            }
+
+            this.collidingFormatters = collidingFormatters;
+        }
+    }
+
+    public ImmutableDictionary<QualifiedTypeName, CustomFormatter> KnownFormattersByName { get; private init; } = ImmutableDictionary<QualifiedTypeName, CustomFormatter>.Empty;
 
     public GeneratorOptions Generator { get; init; } = new();
 
@@ -34,12 +90,12 @@ public record AnalyzerOptions
     /// </summary>
     public bool IsGeneratingSource { get; init; }
 
-    internal AnalyzerOptions WithFormatterTypes(ImmutableArray<string> formattableTypes, ImmutableDictionary<string, ImmutableHashSet<string>> formatterTypes)
+    internal AnalyzerOptions WithFormatterTypes(ImmutableArray<FormattableType> formattableTypes, ImmutableHashSet<CustomFormatter> customFormatters)
     {
         return this with
         {
-            AssumedFormattableTypes = ImmutableHashSet.CreateRange(formattableTypes).Union(formatterTypes.SelectMany(t => t.Value)),
-            KnownFormatters = formatterTypes,
+            AssumedFormattableTypes = ImmutableHashSet.CreateRange(formattableTypes).Union(customFormatters.SelectMany(t => t.FormattableTypes)),
+            KnownFormatters = customFormatters,
         };
     }
 
@@ -51,10 +107,12 @@ internal AnalyzerOptions WithFormatterTypes(ImmutableArray<string> formattableTy
     /// <returns>The modified set of options.</returns>
     internal AnalyzerOptions WithAssemblyAttributes(ImmutableArray<AttributeData> assemblyAttributes, CancellationToken cancellationToken)
     {
-        ImmutableDictionary<string, ImmutableHashSet<string>> customFormatters = AnalyzerUtilities.ParseKnownFormatterAttribute(assemblyAttributes, cancellationToken);
-        ImmutableArray<string> customFormattedTypes = AnalyzerUtilities.ParseAssumedFormattableAttribute(assemblyAttributes, cancellationToken);
+        ImmutableHashSet<CustomFormatter> customFormatters = AnalyzerUtilities.ParseKnownFormatterAttribute(assemblyAttributes, cancellationToken).Union(this.KnownFormatters);
+        ImmutableArray<FormattableType> customFormattedTypes = this.AssumedFormattableTypes.Union(AnalyzerUtilities.ParseAssumedFormattableAttribute(assemblyAttributes, cancellationToken)).ToImmutableArray();
         return this.WithFormatterTypes(customFormattedTypes, customFormatters);
     }
+
+    internal ImmutableArray<FormattableType> GetCollidingFormatterDataTypes(QualifiedTypeName formatter) => this.collidingFormatters.GetValueOrDefault(formatter, ImmutableArray<FormattableType>.Empty);
 }
 
 /// <summary>
@@ -99,3 +157,54 @@ public record GeneratorOptions
     /// </summary>
     public FormattersOptions Formatters { get; init; } = new();
 }
+
+/// <summary>
+/// Describes a custom formatter.
+/// </summary>
+/// <param name="Name">The name (without namespace) of the type that implements at least one <c>IMessagePackFormatter</c> interface. If the formatter is a generic type, this should <em>not</em> include any generic type parameters.</param>
+/// <param name="FormattableTypes">The type arguments that appear in each implemented <c>IMessagePackFormatter</c> interface. When generic, these should be the full name of their type definitions.</param>
+public record CustomFormatter(QualifiedTypeName Name, ImmutableHashSet<FormattableType> FormattableTypes)
+{
+    public static bool TryCreate(INamedTypeSymbol type, [NotNullWhen(true)] out CustomFormatter? formatter)
+    {
+        var formattedTypes =
+            AnalyzerUtilities.SearchTypeForFormatterImplementations(type)
+            .Select(i => new FormattableType(i))
+            .ToImmutableHashSet();
+        if (formattedTypes.IsEmpty)
+        {
+            formatter = null;
+            return false;
+        }
+
+        formatter = new CustomFormatter(new QualifiedTypeName(type), formattedTypes)
+        {
+            IsInaccessible = !type.InstanceConstructors.Any(ctor => ctor.Parameters.Length == 0 && ctor.DeclaredAccessibility >= Accessibility.Internal),
+        };
+
+        return true;
+    }
+
+    public bool IsInaccessible { get; init; }
+
+    public virtual bool Equals(CustomFormatter other)
+    {
+        return this.Name.Equals(other.Name)
+            && this.FormattableTypes.SetEquals(other.FormattableTypes)
+            && this.IsInaccessible == other.IsInaccessible;
+    }
+
+    public override int GetHashCode() => this.Name.GetHashCode();
+}
+
+/// <summary>
+/// Describes a formattable type.
+/// </summary>
+/// <param name="Name">The name of the formattable type.</param>
+public record FormattableType(QualifiedTypeName Name)
+{
+    public FormattableType(ITypeSymbol type)
+        : this(new QualifiedTypeName(type))
+    {
+    }
+}

src/MessagePack.SourceGenerator/CodeAnalysis/CodeAnalysisUtilities.cs

@@ -1,6 +1,9 @@
 // Copyright (c) All contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+
 namespace MessagePack.SourceGenerator.CodeAnalysis;
 
 public static class CodeAnalysisUtilities
@@ -34,4 +37,31 @@ public static string GetSanitizedFileName(string fileName)
 
         return fileName;
     }
+
+    internal static int GetArity(ITypeSymbol dataType)
+        => dataType switch
+        {
+            INamedTypeSymbol namedType => namedType.Arity,
+            IArrayTypeSymbol arrayType => GetArity(arrayType.ElementType),
+            ITypeParameterSymbol => 0,
+            _ => throw new NotSupportedException(),
+        };
+
+    internal static ImmutableArray<string> GetTypeParameters(ITypeSymbol dataType)
+        => dataType switch
+        {
+            INamedTypeSymbol namedType => namedType.TypeParameters.Select(t => t.Name).ToImmutableArray(),
+            IArrayTypeSymbol arrayType => GetTypeParameters(arrayType.ElementType),
+            ITypeParameterSymbol => ImmutableArray<string>.Empty,
+            _ => throw new NotSupportedException(),
+        };
+
+    internal static ImmutableArray<string> GetTypeArguments(ITypeSymbol dataType)
+        => dataType switch
+        {
+            INamedTypeSymbol namedType => namedType.TypeArguments.Select(t => t.GetCanonicalTypeFullName()).ToImmutableArray(),
+            IArrayTypeSymbol arrayType => GetTypeArguments(arrayType.ElementType),
+            ITypeParameterSymbol => ImmutableArray<string>.Empty,
+            _ => throw new NotSupportedException(),
+        };
 }

src/MessagePack.SourceGenerator/CodeAnalysis/CustomFormatterRegisterInfo.cs

@@ -0,0 +1,9 @@
+// Copyright (c) All contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace MessagePack.SourceGenerator.CodeAnalysis;
+
+public record CustomFormatterRegisterInfo : ResolverRegisterInfo
+{
+    public override string GetFormatterNameForResolver(GenericParameterStyle style) => this.Formatter.GetQualifiedName(Qualifiers.GlobalNamespace, style);
+}