Make serializer work nicely with autoproperties
Barsonax opened this issue · 3 comments
Summary
Currently we try to avoid using auto properties because when serializing a instance the serializer will use the name of the backing fields which have a ugly name like <Foo>k__BackingField
.
Consider changing the serializers to understand the relationship between the backing fields and the properties to allow them to handle these in a nicer way
Analysis
- Allows the use of auto properties without downsides in both our code and end users code. Which can clean up some files alot.
- 3th party types that use autoproperties will end up with the ugly backingfield name in serialized form.
- In the future features like records might be added to C# which might also make use of properties?
- We have to find a reliable way to find the relationship between backingfields and properties.
Simple example of getting the backingfield or the autoproperty. No idea if this is stable enough, needs more research.
const string prefix = "<";
const string suffix = ">k__BackingField";
public static FieldInfo GetBackingField(PropertyInfo propertyInfo)
{
var backingFieldName = $"{prefix}{propertyInfo.Name}{suffix}";
return propertyInfo.DeclaringType.GetFields(BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static).FirstOrDefault(x => x.Name == backingFieldName && x.IsDefined(typeof(CompilerGeneratedAttribute)));
}
public static PropertyInfo GetAutoProperty(FieldInfo field)
{
if (field.Name.StartsWith(prefix) && field.Name.EndsWith(suffix) && field.IsDefined(typeof(CompilerGeneratedAttribute), true))
{
var propertyName = field.Name[prefix.Length..^suffix.Length];
if (propertyName != null)
{
return field.DeclaringType.GetProperties(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static)
.Where(x => x.SetMethod != null && x.SetMethod.IsDefined(typeof(CompilerGeneratedAttribute)))
.Where(x => x.GetMethod != null && x.GetMethod.IsDefined(typeof(CompilerGeneratedAttribute)))
.FirstOrDefault(x => x.Name == propertyName);
}
}
return null;
}
Some notes:
- Regardless of implementation, our serializers should still support reading and an ugly backing field name as-is, if it is present in the serialized data stream.
- We'd also need to expose the name-translation logic via public API, because if a field is renamed or its type changed, a
AssignFieldError
will be invoked and user-handled. Usually they'll just check for a hardcoded name, but there may be cases where reflection will be involved and name translation required. - There appears to be a
CompilerGenerated
attribute on both the properties and fields, which we could potentially use to filter out candidates for any matching. - With Roslyn around, we probably shouldn't rely too much on specific compiler behavior. Alternate compilers might handle stuff differently, and even mainstream C# compiler development is pretty fast these days. If there's a recent spec somewhere on exact behavior, we might be safe though.
- Maybe we could re-phrase the problem into "nice-ify" variable names for human-readable serialization formats? This would deal with auto-properties for now, but would conceptually not be limited to them and allow us to evolve the rules over time with slightly less of a headache to match exactly what a compiler is doing.
- It's an open question whether this is a general "serializer" feature in Duality, or just a bit of utility code that is used by the
XmlSerializer
and potentially others that might write a human-readable format in the future. I'm currently leaning towards the former though, since it reduces the mental load for implementers of specific serialization formats. - I'd probably go for adding a change here in
SerializeType
, replacingFieldInfo
with a struct that has a field info, but also a "serialized name". I'd not make this specific for auto-properties, or even mention them. - We could even go with a primary serialized name, and a list of fallback names in the future (separate issue) to enable easily usable
FormerlySerializedAs
user attributes without having to rely on implementing aSerializeErrorHandler
there.
There appears to be a CompilerGenerated attribute on both the properties and fields, which we could potentially use to filter out candidates for any matching.
Oh also on the property? Thats nice. Can make it quite robust then by just checking if theres a attribute on both sides.
EDIT: updated the example on this.
Not the property itself, but its internal getter and setter methods get one. Still, could be useful enough for some sort of verification.