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

Provide better exception messages for ambiguous matches #299

Merged
merged 2 commits into from
Jan 30, 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
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,15 @@ public virtual BinaryDeserializerBuilderCaseResult BuildExpression(Type type, Sc

var underlying = Nullable.GetUnderlyingType(type) ?? type;

// enum fields will always be public static, so no need to expose binding flags:
var fields = underlying.GetFields(BindingFlags.Public | BindingFlags.Static);

var cases = underlying.IsEnum
? enumSchema.Symbols
.Select((symbol, index) =>
{
var match = fields.SingleOrDefault(field => IsMatch(symbol, field));
var match = GetMatch(symbol, underlying);

if (enumSchema.Default != null)
{
match ??= fields.SingleOrDefault(field => IsMatch(enumSchema.Default, field));
match ??= GetMatch(enumSchema.Default, underlying);
}

if (match == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ public virtual BinaryDeserializerBuilderCaseResult BuildExpression(Type type, Sc
}
else
{
var members = underlying.GetMembers(MemberVisibility);

// support dynamic deserialization:
var value = Expression.Parameter(
underlying.IsAssignableFrom(typeof(ExpandoObject))
Expand All @@ -126,7 +124,7 @@ public virtual BinaryDeserializerBuilderCaseResult BuildExpression(Type type, Sc
new[] { (Expression)Expression.Assign(value, Expression.New(value.Type)) }
.Concat(recordSchema.Fields.Select(field =>
{
var match = members.SingleOrDefault(member => IsMatch(field, member));
var match = GetMatch(field, underlying, MemberVisibility);

Expression expression;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,11 @@ public virtual BinarySerializerBuilderCaseResult BuildExpression(Expression valu
// then build/set the delegate if it hasn’t been built yet:
if (parameter == reference)
{
var members = type.GetMembers(MemberVisibility);

var argument = Expression.Variable(type);
var writes = recordSchema.Fields
.Select(field =>
{
var match = members.SingleOrDefault(member => IsMatch(field, member));
var match = GetMatch(field, type, MemberVisibility);

Expression inner;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,16 @@ public virtual JsonDeserializerBuilderCaseResult BuildExpression(Type type, Sche

var underlying = Nullable.GetUnderlyingType(type) ?? type;

// enum fields will always be public static, so no need to expose binding flags:
var fields = underlying.GetFields(BindingFlags.Public | BindingFlags.Static);

// find a match for each enum in the schema:
var cases = underlying.IsEnum
? enumSchema.Symbols
.Select(symbol =>
{
var match = fields.SingleOrDefault(field => IsMatch(symbol, field));
var match = GetMatch(symbol, underlying);

if (enumSchema.Default != null)
{
match ??= fields.SingleOrDefault(field => IsMatch(enumSchema.Default, field));
match ??= GetMatch(enumSchema.Default, underlying);
}

if (match == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,6 @@ public virtual JsonDeserializerBuilderCaseResult BuildExpression(Type type, Sche
}
else
{
var members = underlying.GetMembers(MemberVisibility);

// support dynamic deserialization:
var value = Expression.Parameter(
underlying.IsAssignableFrom(typeof(ExpandoObject))
Expand Down Expand Up @@ -200,7 +198,7 @@ public virtual JsonDeserializerBuilderCaseResult BuildExpression(Type type, Sche
recordSchema.Fields
.Select(field =>
{
var match = members.SingleOrDefault(member => IsMatch(field, member));
var match = GetMatch(field, underlying, MemberVisibility);

Expression expression;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ namespace Chr.Avro.Serialization
using System;
using System.Collections.Generic;
using System.Dynamic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Text.Json;
Expand Down Expand Up @@ -74,8 +73,6 @@ public virtual JsonSerializerBuilderCaseResult BuildExpression(Expression value,
// then build/set the delegate if it hasn’t been built yet:
if (parameter == reference)
{
var members = type.GetMembers(MemberVisibility);

var writeStartObject = typeof(Utf8JsonWriter)
.GetMethod(nameof(Utf8JsonWriter.WriteStartObject), Type.EmptyTypes);

Expand All @@ -93,7 +90,7 @@ public virtual JsonSerializerBuilderCaseResult BuildExpression(Expression value,

foreach (var field in recordSchema.Fields)
{
var match = members.SingleOrDefault(member => IsMatch(field, member));
var match = GetMatch(field, type, MemberVisibility);

Expression inner;

Expand Down
38 changes: 38 additions & 0 deletions src/Chr.Avro/Serialization/EnumDeserializerBuilderCase.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace Chr.Avro.Serialization
{
using System;
using System.Linq;
using System.Reflection;
using System.Runtime.Serialization;
using System.Text.RegularExpressions;
Expand All @@ -14,6 +15,38 @@ public abstract class EnumDeserializerBuilderCase : DeserializerBuilderCase
{
private static readonly Regex FuzzyCharacters = new(@"[^A-Za-z0-9]");

/// <summary>
/// Finds at most one type member that matches an enum symbol.
/// </summary>
/// <param name="symbol">
/// An enum symbol.
/// </param>
/// <param name="type">
/// The type to inspect.
/// </param>
/// <returns>
/// A match if <see cref="IsMatch(string, MemberInfo)" /> returns <c>true</c> for at
/// most one type member; <c>null</c> otherwise.
/// </returns>
/// <exception cref="UnsupportedTypeException">
/// Thrown when <paramref name="type" /> has multiple members that match
/// <paramref name="symbol" />.
/// </exception>
protected virtual MemberInfo? GetMatch(string symbol, Type type)
{
// enum fields will always be public static, so no need to expose binding flags:
var matches = type.GetFields(BindingFlags.Public | BindingFlags.Static)
.Where(member => IsMatch(symbol, member))
.ToList();

if (matches.Count > 1)
{
throw new UnsupportedTypeException(type, $"Multiple members ({string.Join(", ", matches.Select(m => m.Name))}) on {type} match the {symbol} symbol.");
}

return matches.FirstOrDefault();
}

/// <summary>
/// Determines whether an enum symbol is a match for a type member.
/// </summary>
Expand All @@ -28,6 +61,11 @@ public abstract class EnumDeserializerBuilderCase : DeserializerBuilderCase
/// </returns>
protected virtual bool IsMatch(string symbol, MemberInfo member)
{
if (member.MemberType != MemberTypes.Field)
{
return false;
}

if (member.DeclaringType.HasAttribute<DataContractAttribute>())
{
if (member.GetAttribute<EnumMemberAttribute>() is EnumMemberAttribute memberAttribute)
Expand Down
5 changes: 5 additions & 0 deletions src/Chr.Avro/Serialization/EnumSerializerBuilderCase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ protected override Expression BuildDynamicConversion(Expression value, Type targ
/// </returns>
protected virtual bool IsMatch(string symbol, MemberInfo member)
{
if (member.MemberType != MemberTypes.Field)
{
return false;
}

if (member.DeclaringType.HasAttribute<DataContractAttribute>())
{
if (member.GetAttribute<EnumMemberAttribute>() is EnumMemberAttribute memberAttribute)
Expand Down
39 changes: 39 additions & 0 deletions src/Chr.Avro/Serialization/RecordDeserializerBuilderCase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,40 @@ public abstract class RecordDeserializerBuilderCase : DeserializerBuilderCase
});
}

/// <summary>
/// Finds at most one type member that matches a <see cref="RecordField" />.
/// </summary>
/// <param name="field">
/// A record field.
/// </param>
/// <param name="type">
/// The type to inspect.
/// </param>
/// <param name="memberVisibility">
/// The binding flags used to select fields and properties.
/// </param>
/// <returns>
/// A match if <see cref="IsMatch(RecordField, MemberInfo)" /> returns <c>true</c> for at
/// most one type member; <c>null</c> otherwise.
/// </returns>
/// <exception cref="UnsupportedTypeException">
/// Thrown when <paramref name="type" /> has multiple members that match
/// <paramref name="field" />.
/// </exception>
protected virtual MemberInfo? GetMatch(RecordField field, Type type, BindingFlags memberVisibility)
{
var matches = type.GetMembers(memberVisibility)
.Where(member => IsMatch(field, member))
.ToList();

if (matches.Count > 1)
{
throw new UnsupportedTypeException(type, $"Multiple members ({string.Join(", ", matches.Select(m => m.Name))}) on {type} match the {field.Name} field.");
}

return matches.FirstOrDefault();
}

/// <summary>
/// Determines whether a <see cref="RecordField" /> is a match for a type member.
/// </summary>
Expand All @@ -72,6 +106,11 @@ public abstract class RecordDeserializerBuilderCase : DeserializerBuilderCase
/// </returns>
protected virtual bool IsMatch(RecordField field, MemberInfo member)
{
if (member.MemberType != MemberTypes.Field && member.MemberType != MemberTypes.Property)
{
return false;
}

if (member.DeclaringType.HasAttribute<DataContractAttribute>())
{
if (member.GetAttribute<DataMemberAttribute>() is DataMemberAttribute memberAttribute)
Expand Down
40 changes: 39 additions & 1 deletion src/Chr.Avro/Serialization/RecordSerializerBuilderCase.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
namespace Chr.Avro.Serialization
{
using System;
using System.Dynamic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
Expand Down Expand Up @@ -62,6 +61,40 @@ protected virtual Expression BuildDynamicGet(Expression @object, string name)
Expression.Call(null, getValue, Expression.ArrayAccess(members, Expression.Constant(0)), @object)));
}

/// <summary>
/// Finds at most one type member that matches a <see cref="RecordField" />.
/// </summary>
/// <param name="field">
/// A record field.
/// </param>
/// <param name="type">
/// The type to inspect.
/// </param>
/// <param name="memberVisibility">
/// The binding flags used to select fields and properties.
/// </param>
/// <returns>
/// A match if <see cref="IsMatch(RecordField, MemberInfo)" /> returns <c>true</c> for at
/// most one type member; <c>null</c> otherwise.
/// </returns>
/// <exception cref="UnsupportedTypeException">
/// Thrown when <paramref name="type" /> has multiple members that match
/// <paramref name="field" />.
/// </exception>
protected virtual MemberInfo? GetMatch(RecordField field, Type type, BindingFlags memberVisibility)
{
var matches = type.GetMembers(memberVisibility)
.Where(member => IsMatch(field, member))
.ToList();

if (matches.Count > 1)
{
throw new UnsupportedTypeException(type, $"Multiple members ({string.Join(", ", matches.Select(m => m.Name))}) on {type} match the {field.Name} field.");
}

return matches.FirstOrDefault();
}

/// <summary>
/// Determines whether a <see cref="RecordField" /> is a match for a type member.
/// </summary>
Expand All @@ -76,6 +109,11 @@ protected virtual Expression BuildDynamicGet(Expression @object, string name)
/// </returns>
protected virtual bool IsMatch(RecordField field, MemberInfo member)
{
if (member.MemberType != MemberTypes.Field && member.MemberType != MemberTypes.Property)
{
return false;
}

if (member.DeclaringType.HasAttribute<DataContractAttribute>())
{
if (member.GetAttribute<DataMemberAttribute>() is DataMemberAttribute memberAttribute)
Expand Down
Loading