Convert IMessagePackConverter to an abstract base class

src/Nerdbank.MessagePack.Analyzers/ConverterAnalyzers.cs

@@ -191,7 +191,7 @@ public override void Initialize(AnalysisContext context)
 							context.RegisterSymbolEndAction(context =>
 							{
 								INamedTypeSymbol symbol = (INamedTypeSymbol)context.Symbol;
-								if (!symbol.GetAllMembers().Any(m => m is IMethodSymbol { Name: "GetJsonSchema", OverriddenMethod: not null }))
+								if (!symbol.GetAllMembers().Any(m => m is IMethodSymbol { Name: "GetJsonSchema", OverriddenMethod: not null } && IsOverriddenInConcreteConverter(m)))
 								{
 									if (symbol.Locations.FirstOrDefault(l => l.IsInSource) is { } location)
 									{
@@ -202,13 +202,16 @@ public override void Initialize(AnalysisContext context)
 								if (isAsyncConverter)
 								{
 									// This converter specifically implements async functionality.
-									IPropertySymbol? prefersAsyncSerialization = symbol.GetAllMembers().OfType<IPropertySymbol>().FirstOrDefault(p => p is { Name: "PreferAsyncSerialization", OverriddenProperty: not null });
+									IPropertySymbol? prefersAsyncSerialization = symbol.GetAllMembers().OfType<IPropertySymbol>().FirstOrDefault(p => p is { Name: "PreferAsyncSerialization", OverriddenProperty: not null } && IsOverriddenInConcreteConverter(p));
 									if (prefersAsyncSerialization is null)
 									{
 										context.ReportDiagnostic(Diagnostic.Create(AsyncConverterShouldOverridePreferAsyncSerializationDescriptor, symbol.Locations.FirstOrDefault(l => l.IsInSource)));
 									}
 								}
 							});
+
+							bool IsOverriddenInConcreteConverter(ISymbol member)
+								=> !(member.ContainingType.IsGenericType && SymbolEqualityComparer.Default.Equals(member.ContainingType.ConstructUnboundGenericType(), referenceSymbols.MessagePackConverterUnbound));
 						}
 					}
 				},
@@ -242,11 +245,11 @@ string t when t.StartsWith("Write", StringComparison.Ordinal) => (1, true),
 							_ => (0, true),
 						};
 					}
-					else if (i.TargetMethod.ContainingSymbol is ITypeSymbol s && (s.IsOrDerivedFrom(referenceSymbols.IMessagePackConverter) || s.IsOrDerivedFrom(referenceSymbols.MessagePackConverterUnbound)))
+					else if (i.TargetMethod.ContainingSymbol is ITypeSymbol s && (s.IsOrDerivedFrom(referenceSymbols.MessagePackConverterNonGeneric) || s.IsOrDerivedFrom(referenceSymbols.MessagePackConverterUnbound)))
 					{
 						return i.TargetMethod.Name switch
 						{
-							"Write" or "WriteAsync" => (1, true),
+							"Write" or "WriteAsync" or "WriteObject" or "WriteObjectAsync" => (1, true),
 							_ => (0, true),
 						};
 					}
@@ -273,11 +276,11 @@ string t when t.StartsWith("Read", StringComparison.Ordinal) => (1, true),
 							_ => (0, true),
 						};
 					}
-					else if (i.TargetMethod.ContainingSymbol is ITypeSymbol s && (s.IsOrDerivedFrom(referenceSymbols.IMessagePackConverter) || s.IsOrDerivedFrom(referenceSymbols.MessagePackConverterUnbound)))
+					else if (i.TargetMethod.ContainingSymbol is ITypeSymbol s && (s.IsOrDerivedFrom(referenceSymbols.MessagePackConverterNonGeneric) || s.IsOrDerivedFrom(referenceSymbols.MessagePackConverterUnbound)))
 					{
 						return i.TargetMethod.Name switch
 						{
-							"Read" or "ReadAsync" => (1, true),
+							"Read" or "ReadAsync" or "ReadObject" or "ReadObjectAsync" => (1, true),
 							_ => (0, true),
 						};
 					}

src/Nerdbank.MessagePack.Analyzers/ReferenceSymbols.cs

@@ -8,7 +8,7 @@ namespace Nerdbank.MessagePack.Analyzers;
 public record ReferenceSymbols(
 	INamedTypeSymbol MessagePackSerializer,
 	INamedTypeSymbol MessagePackConverter,
-	INamedTypeSymbol IMessagePackConverter,
+	INamedTypeSymbol MessagePackConverterNonGeneric,
 	INamedTypeSymbol MessagePackConverterAttribute,
 	INamedTypeSymbol MessagePackReader,
 	INamedTypeSymbol MessagePackStreamingReader,
@@ -53,8 +53,8 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out Re
 			return false;
 		}
 
-		INamedTypeSymbol? imessagePackConverter = libraryAssembly.GetTypeByMetadataName("Nerdbank.MessagePack.IMessagePackConverter");
-		if (imessagePackConverter is null)
+		INamedTypeSymbol? messagePackConverterNonGeneric = libraryAssembly.GetTypeByMetadataName("Nerdbank.MessagePack.MessagePackConverter");
+		if (messagePackConverterNonGeneric is null)
 		{
 			referenceSymbols = null;
 			return false;
@@ -133,7 +133,7 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out Re
 		referenceSymbols = new ReferenceSymbols(
 			messagePackSerializer,
 			messagePackConverter,
-			imessagePackConverter,
+			messagePackConverterNonGeneric,
 			messagePackConverterAttribute,
 			messagePackReader,
 			messagePackStreamingReader,

src/Nerdbank.MessagePack/ConverterCache.cs

@@ -286,8 +286,8 @@ internal MessagePackConverter<T> GetOrAddConverter<T>(ITypeShape<T> shape)
 	/// </summary>
 	/// <param name="shape">The shape of the type to convert.</param>
 	/// <returns>A msgpack converter.</returns>
-	internal IMessagePackConverterInternal GetOrAddConverter(ITypeShape shape)
-		=> (IMessagePackConverterInternal)this.CachedConverters.GetOrAdd(shape)!;
+	internal MessagePackConverter GetOrAddConverter(ITypeShape shape)
+		=> (MessagePackConverter)this.CachedConverters.GetOrAdd(shape)!;
 
 	/// <summary>
 	/// Gets a converter for the given type shape.

src/Nerdbank.MessagePack/Converters/CommonRecords.cs

@@ -138,15 +138,15 @@ internal record SubTypes
 	/// <summary>
 	/// Gets the converters to use to deserialize a subtype, keyed by its integer alias.
 	/// </summary>
-	internal required FrozenDictionary<int, IMessagePackConverter> DeserializersByIntAlias { get; init; }
+	internal required FrozenDictionary<int, MessagePackConverter> DeserializersByIntAlias { get; init; }
 
 	/// <summary>
 	/// Gets the converter to use to deserialize a subtype, keyed by its UTF-8 encoded string alias.
 	/// </summary>
-	internal required SpanDictionary<byte, IMessagePackConverter> DeserializersByStringAlias { get; init; }
+	internal required SpanDictionary<byte, MessagePackConverter> DeserializersByStringAlias { get; init; }
 
 	/// <summary>
 	/// Gets the converter and alias to use for a subtype, keyed by their <see cref="Type"/>.
 	/// </summary>
-	internal required FrozenDictionary<Type, (SubTypeAlias Alias, IMessagePackConverter Converter, ITypeShape Shape)> Serializers { get; init; }
+	internal required FrozenDictionary<Type, (SubTypeAlias Alias, MessagePackConverter Converter, ITypeShape Shape)> Serializers { get; init; }
 }

src/Nerdbank.MessagePack/Converters/SubTypeUnionConverter`1.cs

@@ -51,7 +51,7 @@
 			return this.baseConverter.Read(ref reader, context);
 		}
 
-		IMessagePackConverter? converter;
+		MessagePackConverter? converter;
 		if (reader.NextMessagePackType == MessagePackType.Integer)
 		{
 			int alias = reader.ReadInt32();
@@ -69,10 +69,9 @@
 			}
 		}
 
-		return (TBase?)converter.Read(ref reader, context);
+		return (TBase?)converter.ReadObject(ref reader, context);
 	}
 
-#pragma warning disable NBMsgPack031 // Exactly one structure -- it can't see internal IMessagePackConverter.Write calls
 	/// <inheritdoc/>
 	public override void Write(ref MessagePackWriter writer, in TBase? value, SerializationContext context)
 	{
@@ -91,17 +90,16 @@
 			writer.WriteNil();
 			this.baseConverter.Write(ref writer, value, context);
 		}
-		else if (this.subTypes.Serializers.TryGetValue(valueType, out (SubTypeAlias Alias, IMessagePackConverter Converter, ITypeShape Shape) result))
+		else if (this.subTypes.Serializers.TryGetValue(valueType, out (SubTypeAlias Alias, MessagePackConverter Converter, ITypeShape Shape) result))
 		{
 			writer.WriteRaw(result.Alias.MsgPackAlias.Span);
-			result.Converter.Write(ref writer, value, context);
+			result.Converter.WriteObject(ref writer, value, context);
 		}
 		else
 		{
 			throw new MessagePackSerializationException($"value is of type {valueType.FullName} which is not one of those listed via {KnownSubTypeAttribute.TypeName} on the declared base type {typeof(TBase).FullName}.");
 		}
 	}
-#pragma warning restore NBMsgPack031
 
 	/// <inheritdoc/>
 	[Experimental("NBMsgPackAsync")]
@@ -151,7 +149,7 @@
 			streamingReader = new(await streamingReader.FetchMoreBytesAsync().ConfigureAwait(false));
 		}
 
-		IMessagePackConverter? converter;
+		MessagePackConverter? converter;
 		if (nextMessagePackType == MessagePackType.Integer)
 		{
 			int alias;
@@ -187,7 +185,7 @@
 		}
 
 		reader.ReturnReader(ref streamingReader);
-		return (TBase?)await converter.ReadAsync(reader, context).ConfigureAwait(false);
+		return (TBase?)await converter.ReadObjectAsync(reader, context).ConfigureAwait(false);
 	}
 
 	/// <inheritdoc/>
@@ -219,17 +217,17 @@
 				writer.ReturnWriter(ref syncWriter);
 			}
 		}
-		else if (this.subTypes.Serializers.TryGetValue(valueType, out (SubTypeAlias Alias, IMessagePackConverter Converter, ITypeShape Shape) result))
+		else if (this.subTypes.Serializers.TryGetValue(valueType, out (SubTypeAlias Alias, MessagePackConverter Converter, ITypeShape Shape) result))
 		{
 			syncWriter.WriteRaw(result.Alias.MsgPackAlias.Span);
 			if (result.Converter.PreferAsyncSerialization)
 			{
 				writer.ReturnWriter(ref syncWriter);
-				await result.Converter.WriteAsync(writer, value, context).ConfigureAwait(false);
+				await result.Converter.WriteObjectAsync(writer, value, context).ConfigureAwait(false);
 			}
 			else
 			{
-				result.Converter.Write(ref syncWriter, value, context);
+				result.Converter.WriteObject(ref syncWriter, value, context);
 				writer.ReturnWriter(ref syncWriter);
 			}
 		}

src/Nerdbank.MessagePack/IMessagePackConverterInternal.cs

@@ -6,17 +6,17 @@ namespace Nerdbank.MessagePack;
 /// <summary>
 /// Non-generic access to internal methods of <see cref="MessagePackConverter{T}"/>.
 /// </summary>
-internal interface IMessagePackConverterInternal : IMessagePackConverter
+internal interface IMessagePackConverterInternal
 {
 	/// <summary>
 	/// Wraps this converter with a reference preservation converter.
 	/// </summary>
 	/// <returns>A converter. Possibly <see langword="this"/> if this instance is already reference preserving.</returns>
-	IMessagePackConverterInternal WrapWithReferencePreservation();
+	MessagePackConverter WrapWithReferencePreservation();
 
 	/// <summary>
 	/// Removes the outer reference preserving converter, if present.
 	/// </summary>
 	/// <returns>The unwrapped converter.</returns>
-	IMessagePackConverterInternal UnwrapReferencePreservation();
+	MessagePackConverter UnwrapReferencePreservation();
 }

src/Nerdbank.MessagePack/JsonSchemaContext.cs

@@ -54,7 +54,7 @@ public JsonObject GetJsonSchema(ITypeShape typeShape)
 			return CreateReference(qualifiedReference);
 		}
 
-		IMessagePackConverter converter = this.cache.GetOrAddConverter(typeShape);
+		MessagePackConverter converter = this.cache.GetOrAddConverter(typeShape);
 		if (converter.GetJsonSchema(this, typeShape) is not JsonObject schema)
 		{
 			schema = MessagePackConverter<int>.CreateUndocumentedSchema(converter.GetType());

src/Nerdbank.MessagePack/IMessagePackConverter.cs → src/Nerdbank.MessagePack/MessagePackConverter.cs

@@ -7,47 +7,50 @@
 namespace Nerdbank.MessagePack;
 
 /// <summary>
-/// A non-generic, <see cref="object"/>-based interface for all message pack converters.
+/// A non-generic, <see cref="object"/>-based base class for all message pack converters.
 /// </summary>
-public interface IMessagePackConverter
+public abstract class MessagePackConverter
 {
 	/// <summary>
 	/// Gets a value indicating whether callers should prefer the async methods on this object.
 	/// </summary>
 	/// <value>Unless overridden in a derived converter, this value is always <see langword="false"/>.</value>
 	/// <remarks>
-	/// Derived types that override the <see cref="WriteAsync"/> and/or <see cref="ReadAsync"/> methods
+	/// Derived types that override the <see cref="WriteObjectAsync"/> and/or <see cref="ReadObjectAsync"/> methods
 	/// should also override this property and have it return <see langword="true" />.
 	/// </remarks>
-	bool PreferAsyncSerialization { get; }
+	public abstract bool PreferAsyncSerialization { get; }
 
 	/// <summary>
 	/// Serializes an instance of an object.
 	/// </summary>
 	/// <param name="writer">The writer to use.</param>
 	/// <param name="value">The value to serialize.</param>
 	/// <param name="context">Context for the serialization.</param>
-	void Write(ref MessagePackWriter writer, object? value, SerializationContext context);
+	/// <remarks>
+	/// Implementations of this method should not flush the writer.
+	/// </remarks>
+	public abstract void WriteObject(ref MessagePackWriter writer, object? value, SerializationContext context);
 
 	/// <summary>
 	/// Deserializes an instance of an object.
 	/// </summary>
 	/// <param name="reader">The reader to use.</param>
 	/// <param name="context">Context for the deserialization.</param>
 	/// <returns>The deserialized value.</returns>
-	object? Read(ref MessagePackReader reader, SerializationContext context);
+	public abstract object? ReadObject(ref MessagePackReader reader, SerializationContext context);
 
-	/// <inheritdoc cref="Write"/>
+	/// <inheritdoc cref="WriteObject"/>
 	/// <returns>A task that tracks the asynchronous operation.</returns>
 	[Experimental("NBMsgPackAsync")]
-	ValueTask WriteAsync(MessagePackAsyncWriter writer, object? value, SerializationContext context);
+	public abstract ValueTask WriteObjectAsync(MessagePackAsyncWriter writer, object? value, SerializationContext context);
 
-	/// <inheritdoc cref="Read"/>
+	/// <inheritdoc cref="ReadObject"/>
 	[Experimental("NBMsgPackAsync")]
-	ValueTask<object?> ReadAsync(MessagePackAsyncReader reader, SerializationContext context);
+	public abstract ValueTask<object?> ReadObjectAsync(MessagePackAsyncReader reader, SerializationContext context);
 
 	/// <inheritdoc cref="MessagePackConverter{T}.GetJsonSchema(JsonSchemaContext, ITypeShape)"/>
-	JsonObject? GetJsonSchema(JsonSchemaContext context, ITypeShape typeShape);
+	public abstract JsonObject? GetJsonSchema(JsonSchemaContext context, ITypeShape typeShape);
 
 	/// <summary>
 	/// Skips ahead in the msgpack data to the point where the value of the specified property can be read.
@@ -58,7 +61,7 @@ public interface IMessagePackConverter
 	/// <returns><see langword="true" /> if the specified property was found in the data and the value is ready to be read; <see langword="false" /> otherwise.</returns>
 	/// <remarks><inheritdoc cref="SkipToIndexValueAsync(MessagePackAsyncReader, object?, SerializationContext)" path="/remarks"/></remarks>
 	[Experimental("NBMsgPackAsync")]
-	ValueTask<bool> SkipToPropertyValueAsync(MessagePackAsyncReader reader, IPropertyShape propertyShape, SerializationContext context);
+	public abstract ValueTask<bool> SkipToPropertyValueAsync(MessagePackAsyncReader reader, IPropertyShape propertyShape, SerializationContext context);
 
 	/// <summary>
 	/// Skips ahead in the msgpack data to the point where the value at the specified index can be read.
@@ -72,5 +75,10 @@ public interface IMessagePackConverter
 	/// to skip to the starting position of a sequence that should be asynchronously enumerated.
 	/// </remarks>
 	[Experimental("NBMsgPackAsync")]
-	ValueTask<bool> SkipToIndexValueAsync(MessagePackAsyncReader reader, object? index, SerializationContext context);
+	public abstract ValueTask<bool> SkipToIndexValueAsync(MessagePackAsyncReader reader, object? index, SerializationContext context);
+
+	/// <summary>
+	/// Just insurance that no external assembly can derive a concrete type from this type, except through the generic <see cref="MessagePackConverter{T}"/>.
+	/// </summary>
+	internal abstract void DerivationGuard();
 }