Allow the usage of external methods with `MapProperty.Use`, `MapPropertyFromSource.Use` and `MapValue.Use`

[Kibonik]

Describe the bug

UseStaticMapper doesnt work with MapPropertyFromSource

Code

	public class SomeTest
	{
		public int Id { get; set; }
		public string? Number { get; set; }
		public string? Prefix { get; set; }
	}
	public class SomeTestDto
	{
		public int Id { get; set; }
		public string? NumberFull { get; set; }


	}
	[Mapper]
	[UseStaticMapper(typeof(TestMapper))]
	public static partial class SomeMapper
	{
		[MapPropertyFromSource(nameof(SomeTestDto.NumberFull), Use = nameof(MapProducer))] //or Use = nameof(TestMapper.MapProducer)
		public static partial SomeTestDto MapMe(SomeTest s);
	}

	public static class TestMapper
	{
		public static string? MapProducer(SomeTest s)
				   => s.Prefix + s.Number;
	}

Generated code

            public static partial global::TimberErp.Shared.Models.LoadMovesDtoMapper.SomeTestDto MapMe(global::TimberErp.Shared.Models.LoadMovesDtoMapper.SomeTest s)
            {
                var target = new global::TimberErp.Shared.Models.LoadMovesDtoMapper.SomeTestDto();
                target.Id = s.Id;
                target.NumberFull = s.ToString();
                return target;
            }

Error

Severity	Code	Description	Project	File	Line	Suppression State
Error	CS0103	The name 'MapProducer' does not exist in the current context	
Error	RMG061	The referenced mapping named MapProducer was not found	

[latonz]

This is not supported and isn't expected to work. Right now Use does only allow methods of the same class and its type hierarchy. This needs some concept on how to address these external methods (probably use a "full-name" concept similar to the one we use for properties (see docs)?)...

[IanKemp]

100% support the @nameof / "fullnameof" special-casing as a way to achieve this feature, especially since the C# language implementers can't be bothered to implement it. I would consider this a "small feature" as outlined in your contribution docs, @latonz would you agree and is this something you would be happy to entertain a PR to implement?

[latonz]

@IanKemp Go for it! 😊 I'm looking forward to review your PR. Note: the implementation should work for all possible variations (e.g. a static method on a static type, an instance method on a field of the mapper class, ...).

[IanKemp]

@latonz Would appreciate a pointer on the architectural approach before I go ahead and jump into implementation.

From my initial investigation it seems that the simplest way to implement this would be to change MemberMappingConfiguration.Use from being typed as string? to StringMemberPath? and then special-case handling of the latter type in AttributeDataAccessor.BuildArgumentValue (which currently is only handling the non-nullable version). I'd probably also have to pass the targetType to CreateMemberPath such that the latter can differentiate between "short full nameof" (current behaviour, used for MemberMappingConfiguration.Source and Target) and "actual full nameof" (new behaviour for Use). This approach is not great because if there should ever be a desire to change the nullability of any of the three aforementioned properties, then their behaviour will also implicitly change.

Then I looked at making StringMemberPath an abstract record class as opposed to the record struct it currently is; having two derived types, ShortFullNameOfStringMemberPath for Source and Target and FullNameOfStringMemberPath for Use; and then special-casing BuildArgumentValue and CreateMemberPath on these discrete derived types. The problem there is that changing struct to class has semantics around allocation and therefore performance, and of course source generators (and indeed anything Roslyn-related) is performance-sensitive, so this seems like a bit of an invasive change for the desired feature.

Thoughts?

[latonz]

@IanKemp instead of adding nullable value type handling to all of the arms in AttributeDataAccessor.BuildArgumentValue, we could simply use Nullable.GetUnderlyingType and overwrite the targetType if the underlying type is non-null.

The second problem with the .Skip(1): i really dislike this Skip(1) and I planned already to rework this to get the fullnameof feature to work also with namespaced/nested type references. I think with the help of SemanticModel.GetOperation we could directly get the full path to the target member. I think if we could use such an approach, there would be no need for a string representation of the member path, instead a symbol member path could be used. If that doesn't work out, I'd probably simply create a completely separate FullStringMemberPath for now (and document the differences in a xml comment).

[latonz]

@IanKemp I pushed my experiments here https://github.com/riok/mapperly/pull/1518, see especially Riok.Mapperly.Configuration.SymbolMemberPath which could be what you need.

[faddiv]

I think this is fixed by this PR: https://github.com/riok/mapperly/pull/1890

[aktasahmet]

I think this is fixed by this PR: #1890

I have tried it for MapProperty.Use with the version 4.3.0-next.5 but it still gives the same error. Am I missing something?

[latonz]

After upgrading Mapperly, have you restarted your IDE? Does it work if you rebuild on the console?