Unseal attributes to allow extension

[Thundarr1974]

One of the most annoying things in c# is the lack of a "fullnameof" operator, so we end up with code like this:

$"{nameof(Root)}.{nameof(Root.Parent)}.{nameof(Root.Parent.Child)}.{nameof(Root.Parent.Child.Id)}"

Thanks to the new [CallerArgumentExpression] attribute, there is a DRY workaround but it requires inheriting from an attribute to introduce a new constructor (or have the functionality exist in the base attribute already)

See: dotnet/csharplang#701 (comment)

[latonz]

Unsealing doesn't help much here. Since Mapperly is a source generator it reads the syntax / symbols of the code and extracts the configuration from it. This current implementation doesn't take inherited attributes into account.

The thing you are trying to achieve was discussed here: #317
I have mixed feelings about it, on one side it would really improve the DX of Mapperly, on the other side it changes the behaviour of the built-in nameof operator of c# which i strongly dislike.

[Thundarr1974]

Unsealing/Inheritable would allow us to derive our own MapperXYZ attributes with more flexible constructors, so instead of having to write this:

[MapProperty(new[] { nameof(Root), nameof(Root.SearchRequest), nameof(Root.SearchRequest.Context), nameof(Root.SearchRequest.Context.Id) }, new[] { nameof(Target), nameof(Target.SearchRequest), nameof(Target.SearchRequest.Comments) })]

We would be writing this

[MapPropertyEx(nameof(Root.SearchRequest.Context.Id), nameof(Target.SearchRequest.Comments))]

Otherwise my team won't consider switching to Mapperly :)

[latonz]

Even if the attributes are unsealed this won't work. This may work at runtime but does not easily work with a source generator as the source code works on the syntax / symbol level. Supporting inherited attributes would need a lot of effort in Mapperly.
Therefore Mapperly may support the fullname nameof variant but may never support inherited attributes.

[TimothyMakkison]

Even if the attributes are unsealed this won't work. This may work at runtime but does not easily work with a source generator as the source code works on the syntax / symbol level. Supporting inherited attributes would need a lot of effort in Mapperly.

I might be misremembering but I think it did work with source generators. Because we use Activator.CreateInstance(typeof(TData), newInstanceArguments); to get the attribute properties it might work. Switching to a system where we just use the constructor arguments and never build the attribute would break this (see StronglyTypedIds)

edit: I'm being silly and didn't read Latonz's comment, this wouldn't work because Mapperly only looks for its defined attributes and not any derivatives mb

[Thundarr1974]

Yeah, I saw in AttributeDataAccessor where it's instantiating the attribute so I did a quick POC to pickup my custom attribute name and it's all good.

[Thundarr1974]

Yeah, loading any Type from a project referenced assembly isn't a viable option, so we ended up adding a derived attribute to the Abstractions project for the POC. And since everything has to remain in .NET Standard the [CallerArgumentExpression] attribute isn't available either.

So our current workaround is a bit of a hack - we're using the @ symbol as a marker to indicate to the generator that the value is intended to be fully qualified - "nameof(@SomeModel.SomeParent.SomeChild.SomeValue)". Not ideal, but it saves us from having to do this:

[MapProperty(new[] { nameof(Domestic.SearchResult.MatchingNameAndAddress), nameof(Domestic.SearchResult.MatchingNameAndAddress.Address), nameof(Domestic.SearchResult.MatchingNameAndAddress.Address.ZipExtension) }, new[] { nameof(ProviderSearchCandidate.Branch), nameof(ProviderSearchCandidate.Branch.ZipCodeExt) })]

[TimothyMakkison]

Interesting, so you're creating an internal fork of mapperly with these changes?

since everything has to remain in .NET Standard the [CallerArgumentExpression] attribute isn't available either.

I got around this by using preprocessors for .NET 6+ and a dummy CallerArgumentExpression class, I can't remember why but you can use later .NET Core features with .NET standard as long as you are using at least that version.

I found the stash I originally worked on.this here, it has a working example of a nested MapPropperty.

[Thundarr1974]

Thanks for the tip @TimothyMakkison! The .NET6 directive made it much simpler to implement as I was able to just add the attributes to the existing MapPropertyAttribute constructor.

Here's a fork with the POC code - https://github.com/Thundarr1974/mapperly_nameof_ext

I fully acknowledge the @ prefix is a bit of a hack, however, with no viable workaround it seems the least of all evils...

[TimothyMakkison]

Thanks for sharing this! I'm a little confused about the @ symbol, I didn't need to use it. Is it so you can explicitly enable the 'fullnameof' trick and to let reviewers know that something abnornal is occurring?

[Thundarr1974]

You would need to use it in order to get [MapProperty] to use the fully-qualified value inside of nameof(...). Take these 2 examples:

[MapProperty(nameof(Domestic.SearchResult.MatchingNameAndAddress.ZipExtension), nameof(ProviderSearchCandidate.Branch.ZipCodeExt))]

[MapProperty(nameof(@Domestic.SearchResult.MatchingNameAndAddress.ZipExtension), nameof(@ProviderSearchCandidate.Branch.ZipCodeExt))]

In the first one, "ZipExtension"/"ZipCodeExt" is sent to the constructor. And since there is no "@" prefix, the new code just uses these values for the property names. Assuming the "Domestic" class doesn't have a "ZipExtension" property, Mapperly would complain.

In the second, "ZipExtension"/"ZipCodeExt" is still sent to the constructor, but since there is a "@" prefix the new code extracts "SearchResult.MatchingNameAndAddress.ZipExtension"/"Branch.ZipCodeExt" from the arg expression parameters to use for the property names.

(Credit to SeriLog for the @ prefix inspiration :) )

[TimothyMakkison]

Would using StringOf with nameof be an acceptable workaround? It doesn't feel good using an extra unneeded function, but you would still get all the benefits of a fully typed member withut using Roslyn magic. Take care to strip away the brackets and the first member.

[MapProperty(StringOf(nameof(Car.Make.Id)), StringOf(nameof(CarDto.MakeId"))]
// or rename StringOf to Mem, Fno, S etc
[MapProperty(Mem(nameof(Car.Make.Id)), Mem(nameof(CarDto.MakeId"))]

[latonz]

I don't think this is possible either, as attribute constructor arguments and initialisation values only allow constant expressions. A method call isn't a constant expression.

[latonz]

Closing this, experiment PR is open: #518. May need some time / ideas on how to fix several things but then may land on main.

[latonz]

This is now available in 3.1.0-next.1 as an experiment, see the docs.