Skip to content

Commit

Permalink
Add MessagePackConverterAttribute to prescribe default custom conve…
Browse files Browse the repository at this point in the history
…rters for custom types

This comes with two new analyzers.
  • Loading branch information
AArnott committed Nov 6, 2024
1 parent 72069c3 commit 18abf91
Show file tree
Hide file tree
Showing 14 changed files with 384 additions and 9 deletions.
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

0 comments on commit 18abf91

Please sign in to comment.