Merge pull request #44 from AArnott/fix38

Add `MessagePackConverterAttribute` to prescribe default custom converters for custom types

docfx/analyzers/NBMsgPack020.md

@@ -0,0 +1,37 @@
+# NBMsgPack020: `[MessagePackConverter]` type must be compatible converter
+
+@Nerdbank.MessagePack.MessagePackConverterAttribute should specify a type that derives from @Nerdbank.MessagePack.MessagePackConverter`1 where the type argument is the type the attribute is applied to.
+
+Learn more about [custom converters](../docs/custom-converters.md).
+
+## Example violations
+
+```cs
+[MessagePackConverter(typeof(MyTypeConverter))] // MyTypeConverter does not derive from the correct base type
+public class MyType
+{
+}
+
+public class MyTypeConverter
+{
+	public MyType Deserialize(ref MessagePackReader reader, SerializationContext context) => throw new System.NotImplementedException();
+	public void Serialize(ref MessagePackWriter writer, ref MyType value, SerializationContext context) => throw new System.NotImplementedException();
+}
+```
+
+## Resolution
+
+Fix the converter type to derive from @Nerdbank.MessagePack.MessagePackConverter`1.
+
+```cs
+[MessagePackConverter(typeof(MyTypeConverter)]
+class MyType
+{
+}
+
+public class MyTypeConverter : MessagePackConverter<MyType>
+{
+	public override MyType Deserialize(ref MessagePackReader reader, SerializationContext context) => throw new System.NotImplementedException();
+	public override void Serialize(ref MessagePackWriter writer, ref MyType value, SerializationContext context) => throw new System.NotImplementedException();
+}
+```

docfx/analyzers/NBMsgPack021.md

@@ -0,0 +1,40 @@
+# NBMsgPack021: `[MessagePackConverter]` type missing default constructor
+
+@Nerdbank.MessagePack.MessagePackConverterAttribute should specify a type that declares a public default constructor.
+
+Learn more about [custom converters](../docs/custom-converters.md).
+
+## Example violations
+
+In the below example, `MyTypeConverter` declares an explicit, *non-public* constructor.
+
+```cs
+[MessagePackConverter(typeof(MyTypeConverter))]
+public class MyType
+{
+}
+
+public class MyTypeConverter : MessagePackConverter<MyType>
+{
+	private MyTypeConverter() { }
+	public override MyType Deserialize(ref MessagePackReader reader, SerializationContext context) => throw new System.NotImplementedException();
+	public override void Serialize(ref MessagePackWriter writer, ref MyType value, SerializationContext context) => throw new System.NotImplementedException();
+}
+```
+
+## Resolution
+
+Fix the converter type to declare a public default constructor, or remove the explicit constructor so the C# language provides a default constructor.
+
+```cs
+[MessagePackConverter(typeof(MyTypeConverter))]
+public class MyType
+{
+}
+
+public class MyTypeConverter : MessagePackConverter<MyType>
+{
+	public override MyType Deserialize(ref MessagePackReader reader, SerializationContext context) => throw new System.NotImplementedException();
+	public override void Serialize(ref MessagePackWriter writer, ref MyType value, SerializationContext context) => throw new System.NotImplementedException();
+}
+```

docfx/analyzers/index.md

@@ -13,4 +13,6 @@ Rule ID | Category | Severity | Notes
 [NBMsgPack010](NBMsgPack010.md) | Usage | Error | `[KnownSubType]` should specify an assignable type
 [NBMsgPack011](NBMsgPack011.md) | Usage | Error | `[KnownSubType]` alias must be unique
 [NBMsgPack012](NBMsgPack012.md) | Usage | Error | `[KnownSubType]` type must be unique
-[NBMsgPack012](NBMsgPack013.md) | Usage | Error | `[KnownSubType]` type must not be an open generic
+[NBMsgPack013](NBMsgPack013.md) | Usage | Error | `[KnownSubType]` type must not be an open generic
+[NBMsgPack020](NBMsgPack020.md) | Usage | Error | `[MessagePackConverter]` type must be compatible converter
+[NBMsgPack021](NBMsgPack021.md) | Usage | Error | `[MessagePackConverter]` type missing default constructor

docfx/analyzers/toc.yml

@@ -14,3 +14,7 @@
   href: NBMsgPack012.md
 - name: NBMsgPack013
   href: NBMsgPack013.md
+- name: NBMsgPack020
+  href: NBMsgPack020.md
+- name: NBMsgPack021
+  href: NBMsgPack021.md

src/Nerdbank.MessagePack.Analyzers/AnalyzerReleases.Unshipped.md

@@ -12,3 +12,5 @@ NBMsgPack010 | Usage | Error | `[KnownSubType]` should specify an assignable typ
 NBMsgPack011 | Usage | Error | `[KnownSubType]` alias must be unique
 NBMsgPack012 | Usage | Error | `[KnownSubType]` type must be unique
 NBMsgPack013 | Usage | Error | `[KnownSubType]` type must not be an open generic
+NBMsgPack020 | Usage | Error | `[MessagePackConverter]` type must be compatible converter
+NBMsgPack021 | Usage | Error | `[MessagePackConverter]` type missing default constructor

src/Nerdbank.MessagePack.Analyzers/AnalyzerUtilities.cs

@@ -1,6 +1,8 @@
 // Copyright (c) Andrew Arnott. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
 namespace Nerdbank.MessagePack.Analyzers;
 
 internal static class AnalyzerUtilities
@@ -91,4 +93,14 @@ internal static bool IsInNamespace(ISymbol? symbol, ReadOnlySpan<string> namespa
 
 		return targetSymbol.ContainingNamespace.IsGlobalNamespace;
 	}
+
+	internal static Location? GetArgumentLocation(AttributeData att, int argumentIndex, CancellationToken cancellationToken)
+	{
+		if (att.ApplicationSyntaxReference?.GetSyntax(cancellationToken) is AttributeSyntax a && a.ArgumentList?.Arguments.Count >= argumentIndex)
+		{
+			return a.ArgumentList.Arguments[argumentIndex].GetLocation();
+		}
+
+		return null;
+	}
 }

src/Nerdbank.MessagePack.Analyzers/KnownSubTypeAnalyzers.cs

@@ -138,13 +138,8 @@ public override void Initialize(AnalysisContext context)
 
 						Location? GetArgumentLocation(int argumentIndex)
 						{
-							Location? location = appliedSymbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken).GetLocation();
-							if (att.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken) is AttributeSyntax a && a.ArgumentList?.Arguments.Count >= argumentIndex)
-							{
-								location = a.ArgumentList.Arguments[argumentIndex].GetLocation();
-							}
-
-							return location;
+							return AnalyzerUtilities.GetArgumentLocation(att, argumentIndex, context.CancellationToken)
+								?? appliedSymbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken).GetLocation();
 						}
 					}
 				},

src/Nerdbank.MessagePack.Analyzers/MessagePackConverterAttributeAnalyzer.cs

@@ -0,0 +1,79 @@
+// Copyright (c) Andrew Arnott. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Nerdbank.MessagePack.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public class MessagePackConverterAttributeAnalyzer : DiagnosticAnalyzer
+{
+	public const string InvalidConverterTypeDiagnosticId = "NBMsgPack020";
+	public const string ConverterMissingDefaultCtorDiagnosticId = "NBMsgPack021";
+
+	public static readonly DiagnosticDescriptor InvalidConverterTypeDescriptor = new(
+		id: InvalidConverterTypeDiagnosticId,
+		title: Strings.NBMsgPack020_Title,
+		messageFormat: Strings.NBMsgPack020_MessageFormat,
+		category: "Usage",
+		defaultSeverity: DiagnosticSeverity.Error,
+		isEnabledByDefault: true,
+		helpLinkUri: AnalyzerUtilities.GetHelpLink(InvalidConverterTypeDiagnosticId));
+
+	public static readonly DiagnosticDescriptor ConverterMissingDefaultCtorDescriptor = new(
+		id: ConverterMissingDefaultCtorDiagnosticId,
+		title: Strings.NBMsgPack021_Title,
+		messageFormat: Strings.NBMsgPack021_MessageFormat,
+		category: "Usage",
+		defaultSeverity: DiagnosticSeverity.Error,
+		isEnabledByDefault: true,
+		helpLinkUri: AnalyzerUtilities.GetHelpLink(ConverterMissingDefaultCtorDiagnosticId));
+
+	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [
+		InvalidConverterTypeDescriptor,
+		ConverterMissingDefaultCtorDescriptor,
+	];
+
+	public override void Initialize(AnalysisContext context)
+	{
+		context.EnableConcurrentExecution();
+		context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
+
+		context.RegisterCompilationStartAction(context =>
+		{
+			if (!ReferenceSymbols.TryCreate(context.Compilation, out ReferenceSymbols? referenceSymbols))
+			{
+				return;
+			}
+
+			context.RegisterSymbolAction(
+				context =>
+				{
+					var appliedType = (INamedTypeSymbol)context.Symbol;
+					if (appliedType.FindAttributes(referenceSymbols.MessagePackConverterAttribute).FirstOrDefault() is not { } att)
+					{
+						return;
+					}
+
+					if (att.ConstructorArguments is not [{ Value: INamedTypeSymbol converterType }])
+					{
+						return;
+					}
+
+					if (!converterType.IsDerivedFrom(referenceSymbols.MessagePackConverter.Construct(appliedType)))
+					{
+						context.ReportDiagnostic(Diagnostic.Create(InvalidConverterTypeDescriptor, GetArgumentLocation(0), appliedType.Name));
+					}
+					else if (!converterType.InstanceConstructors.Any(c => c.Parameters.IsEmpty && c.DeclaredAccessibility == Accessibility.Public))
+					{
+						context.ReportDiagnostic(Diagnostic.Create(ConverterMissingDefaultCtorDescriptor, GetArgumentLocation(0), appliedType.Name));
+					}
+
+					Location? GetArgumentLocation(int argumentIndex)
+					{
+						return AnalyzerUtilities.GetArgumentLocation(att, argumentIndex, context.CancellationToken)
+							?? appliedType.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken).GetLocation();
+					}
+				},
+				SymbolKind.NamedType);
+		});
+	}
+}

src/Nerdbank.MessagePack.Analyzers/ReferenceSymbols.cs

@@ -7,6 +7,7 @@ namespace Nerdbank.MessagePack.Analyzers;
 
 internal record ReferenceSymbols(
 	INamedTypeSymbol MessagePackConverter,
+	INamedTypeSymbol MessagePackConverterAttribute,
 	INamedTypeSymbol KeyAttribute,
 	INamedTypeSymbol KnownSubTypeAttribute,
 	INamedTypeSymbol PropertyShapeAttribute)
@@ -26,6 +27,13 @@ internal static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out
 			return false;
 		}
 
+		INamedTypeSymbol? messagePackConverterAttribute = compilation.GetTypeByMetadataName("Nerdbank.MessagePack.MessagePackConverterAttribute");
+		if (messagePackConverterAttribute is null)
+		{
+			referenceSymbols = null;
+			return false;
+		}
+
 		INamedTypeSymbol? keyAttribute = compilation.GetTypeByMetadataName("Nerdbank.MessagePack.KeyAttribute");
 		if (keyAttribute is null)
 		{
@@ -47,7 +55,7 @@ internal static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out
 			return false;
 		}
 
-		referenceSymbols = new ReferenceSymbols(messagePackConverter, keyAttribute, knownSubTypeAttribute, propertyShapeAttribute);
+		referenceSymbols = new ReferenceSymbols(messagePackConverter, messagePackConverterAttribute, keyAttribute, knownSubTypeAttribute, propertyShapeAttribute);
 		return true;
 	}
 }

src/Nerdbank.MessagePack.Analyzers/Strings.resx

@@ -165,4 +165,16 @@
   <data name="NBMsgPack014_Title" xml:space="preserve">
     <value>[KnownSubType] type must have a shape</value>
   </data>
+  <data name="NBMsgPack020_MessageFormat" xml:space="preserve">
+    <value>[MessagePackConverter] type must derive from MessagePackConverter&lt;T&gt; where T is the type the attribute is applied to</value>
+  </data>
+  <data name="NBMsgPack020_Title" xml:space="preserve">
+    <value>[MessagePackConverter] type must be compatible converter</value>
+  </data>
+  <data name="NBMsgPack021_MessageFormat" xml:space="preserve">
+    <value>[MessagePackConverter] type must declare a public constructor with no parameters</value>
+  </data>
+  <data name="NBMsgPack021_Title" xml:space="preserve">
+    <value>[MessagePackConverter] type missing default constructor</value>
+  </data>
 </root>

src/Nerdbank.MessagePack/MessagePackConverterAttribute.cs

@@ -0,0 +1,32 @@
+// Copyright (c) Andrew Arnott. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Diagnostics.CodeAnalysis;
+
+namespace Nerdbank.MessagePack;
+
+/// <summary>
+/// A class applied to a custom data type to prescribe a custom <see cref="MessagePackConverter{T}"/>
+/// implementation to use for serialization.
+/// </summary>
+[AttributeUsage(AttributeTargets.Interface | AttributeTargets.Class | AttributeTargets.Struct, AllowMultiple = false, Inherited = false)]
+public class MessagePackConverterAttribute : Attribute
+{
+	/// <summary>
+	/// Initializes a new instance of the <see cref="MessagePackConverterAttribute"/> class.
+	/// </summary>
+	/// <param name="converterType">
+	/// A type that implements <see cref="MessagePackConverter{T}"/>
+	/// where <c>T</c> is a type argument matching the type to which this attribute is applied.
+	/// </param>
+	public MessagePackConverterAttribute(Type converterType)
+	{
+		this.ConverterType = converterType;
+	}
+
+	/// <summary>
+	/// Gets the type that implements <see cref="MessagePackConverter{T}"/>.
+	/// </summary>
+	[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
+	public Type ConverterType { get; }
+}

src/Nerdbank.MessagePack/StandardVisitor.cs

@@ -4,6 +4,7 @@
 #pragma warning disable NBMsgPackAsync
 
 using System.Collections.Frozen;
+using System.Reflection;
 using System.Text;
 using Microsoft;
 
@@ -28,6 +29,11 @@ internal class StandardVisitor(MessagePackSerializer owner) : TypeShapeVisitor,
 	/// <inheritdoc/>
 	public override object? VisitObject<T>(IObjectTypeShape<T> objectShape, object? state = null)
 	{
+		if (this.GetCustomConverter(objectShape) is MessagePackConverter<T> customConverter)
+		{
+			return customConverter;
+		}
+
 		SubTypes? unionTypes = this.DiscoverUnionTypes(objectShape);
 
 		IConstructorShape? ctorShape = objectShape.GetConstructor();
@@ -380,4 +386,19 @@ protected IMessagePackConverter GetConverter(ITypeShape shape, object? state = n
 			Serializers = serializerData.ToFrozenDictionary(),
 		};
 	}
+
+	private MessagePackConverter<T>? GetCustomConverter<T>(ITypeShape<T> typeShape)
+	{
+		if (typeShape.AttributeProvider?.GetCustomAttributes(typeof(MessagePackConverterAttribute), false).FirstOrDefault() is not MessagePackConverterAttribute customConverterAttribute)
+		{
+			return null;
+		}
+
+		if (customConverterAttribute.ConverterType.GetConstructor(Type.EmptyTypes) is not ConstructorInfo ctor)
+		{
+			throw new MessagePackSerializationException($"{typeof(T).FullName} has {typeof(MessagePackConverterAttribute)} that refers to {customConverterAttribute.ConverterType.FullName} but that converter has no default constructor.");
+		}
+
+		return (MessagePackConverter<T>)ctor.Invoke(Array.Empty<object?>());
+	}
 }