Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MessagePackConverterAttribute to prescribe default custom converters for custom types #44

Merged
merged 1 commit into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions docfx/analyzers/NBMsgPack020.md
Original file line number Diff line number Diff line change
@@ -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();
}
```
40 changes: 40 additions & 0 deletions docfx/analyzers/NBMsgPack021.md
Original file line number Diff line number Diff line change
@@ -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();
}
```
4 changes: 3 additions & 1 deletion docfx/analyzers/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions docfx/analyzers/toc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@
href: NBMsgPack012.md
- name: NBMsgPack013
href: NBMsgPack013.md
- name: NBMsgPack020
href: NBMsgPack020.md
- name: NBMsgPack021
href: NBMsgPack021.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 12 additions & 0 deletions src/Nerdbank.MessagePack.Analyzers/AnalyzerUtilities.cs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
}
}
9 changes: 2 additions & 7 deletions src/Nerdbank.MessagePack.Analyzers/KnownSubTypeAnalyzers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
}
}
10 changes: 9 additions & 1 deletion src/Nerdbank.MessagePack.Analyzers/ReferenceSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace Nerdbank.MessagePack.Analyzers;

internal record ReferenceSymbols(
INamedTypeSymbol MessagePackConverter,
INamedTypeSymbol MessagePackConverterAttribute,
INamedTypeSymbol KeyAttribute,
INamedTypeSymbol KnownSubTypeAttribute,
INamedTypeSymbol PropertyShapeAttribute)
Expand All @@ -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)
{
Expand All @@ -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;
}
}
12 changes: 12 additions & 0 deletions src/Nerdbank.MessagePack.Analyzers/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
32 changes: 32 additions & 0 deletions src/Nerdbank.MessagePack/MessagePackConverterAttribute.cs
Original file line number Diff line number Diff line change
@@ -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; }
}
21 changes: 21 additions & 0 deletions src/Nerdbank.MessagePack/StandardVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#pragma warning disable NBMsgPackAsync

using System.Collections.Frozen;
using System.Reflection;
using System.Text;
using Microsoft;

Expand All @@ -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();
Expand Down Expand Up @@ -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?>());
}
}
Loading