postsharp / Metalama

Metalama is a Roslyn-based meta-programming framework. Use this repo to report bugs or ask questions.
176 stars 5 forks source link

Feature Request: Allow [Template] methods to be sourced from utility classes and not exclusively the aspect class #155

Closed WhitWaldo closed 1 year ago

WhitWaldo commented 1 year ago

Per the comment here, templates must be saved in the aspect class and not in a separate utility class.

I'd like to request that restriction be lifted. Especially for broad type-modifying aspects, I'd prefer to put different functionality in different static utility classes so I might better organize the aspect whenever I come back to it (as opposed to having to work out what I'm trying to do in one large aspect class).

Please loosen the restriction on methods marked with [Template] to allow them to reside in other utility classes so I can ensure that what's in the aspect class is strictly the workflow logic governing what it's supposed to be doing.

Thank you!

PostSharpBot commented 1 year ago

Hello @WhitWaldo, thank you for submitting this issue. We will try to get back to you as soon as possible. Note to the PostSharp team, this ticket is being tracked in our dashboard under ID TP-33330.

gfraiteur commented 1 year ago

Thank you. We call that "subtemplates". It's in our backlog.

svick commented 1 year ago

Actually, I think making this work does not require subtemplates, only a template provider, which is already implemented.

Using code from https://github.com/postsharp/Metalama/issues/153, it would look like this:

using Metalama.Framework.Advising;
using Metalama.Framework.Aspects;
using Metalama.Framework.Code;
using Metalama.Framework.Code.SyntaxBuilders;
using System;
using System.Linq;

[CompileTime]
internal class MethodBuilder : ITemplateProvider
{
    [CompileTime]
    internal static IIntroductionAdviceResult<IMethod> Build(IAspectBuilder<INamedType> builder,
        IFieldOrProperty lockObjField)
    {
        var method = builder.Advice.WithTemplateProvider(new MethodBuilder())
            .IntroduceMethod(builder.Target, nameof(DoSomething), IntroductionScope.Target, OverrideStrategy.Override,
                b =>
                {
                    b.Accessibility = Accessibility.Private;
                    b.IsStatic = false;
                }, args: new
                {
                    lockObjField = lockObjField
                });
        return method;
    }

    [Template]
    private static void DoSomething([CompileTime] IFieldOrProperty lockObjField)
    {
        var lockObj = (object)lockObjField.Value;

        lock (lockObj)
        {
        }
    }
}

class Aspect : TypeAspect
{
    public override void BuildAspect(IAspectBuilder<INamedType> builder)
    {
        base.BuildAspect(builder);

        MethodBuilder.Build(builder, GetLockObject(builder));
    }

    [CompileTime]
    internal static IFieldOrProperty GetLockObject(IAspectBuilder<INamedType> builder)
    {
        var initExpr = ExpressionFactory.Parse("new()");
        return GetObject(builder, typeof(object), "_lockObj", initExpr, Accessibility.PrivateProtected);
    }

    [CompileTime]
    private static IFieldOrProperty GetObject(IAspectBuilder<INamedType> builder, Type expectedType,
        string expectedName, IExpression initializerExpression, Accessibility accessibility = Accessibility.Private)
    {
        var obj = builder.Target.AllFieldsAndProperties.FirstOrDefault(a =>
            a.Type.Is(expectedType) && a.Name == expectedName);

        if (obj != default)
            return obj;

        //Else, introduce and override it
        var targetType = ((INamedType)TypeFactory.GetType(expectedType));
        var field = builder.Advice.IntroduceField(builder.Target, expectedName, targetType, IntroductionScope.Target,
            OverrideStrategy.Override,
            b =>
            {
                b.Accessibility = accessibility;
                b.InitializerExpression = initializerExpression;
            });

        return field.Declaration;
    }
}

[Aspect]
class Target
{
}

Notice how you need to call builder.Advice.WithTemplateProvider() and that you need an instance of the class containing the templates.

gfraiteur commented 1 year ago

Implemented in 2023.3.3-preview.