Metalama Community•2y ago

AddAspect Refactoring

Consider the following scenario. There are two aspects that can potentially be added to a method (in this case it's logging but I think that it's actually immaterial). [LogMethod] logs a method. [TimedLogMethod] does exactly the same only adds timing as well. Now consider the following method.

c#
private double Add(double a, double b) => a + b;

c#
private double Add(double a, double b) => a + b;

The LogMethodAttribute has the following BuildAspect and BuildEligibility options.

c#
public override void BuildAspect(IAspectBuilder<IMethod> builder)
        {
            if(!(builder.Target.Attributes.OfAttributeType(typeof(NoLogAttribute)).Any() ||
                builder.Target.Attributes.OfAttributeType(typeof(TimedLogMethodAttribute)).Any() ||
                builder.Target.DeclaringType.Attributes.OfAttributeType(typeof(NoLogAttribute)).Any()))
            {
                builder.Advice.Override(builder.Target, nameof(this.OverrideMethod));
            }
        }


public override void BuildEligibility(IEligibilityBuilder<IMethod> builder)
        {
            base.BuildEligibility(builder);
            builder.MustNotBeStatic();
            builder.MustNotHaveAspectOfType(typeof(NoLogAttribute));
            builder.MustNotHaveAspectOfType(typeof(TimedLogMethodAttribute));
        }

c#
public override void BuildAspect(IAspectBuilder<IMethod> builder)
        {
            if(!(builder.Target.Attributes.OfAttributeType(typeof(NoLogAttribute)).Any() ||
                builder.Target.Attributes.OfAttributeType(typeof(TimedLogMethodAttribute)).Any() ||
                builder.Target.DeclaringType.Attributes.OfAttributeType(typeof(NoLogAttribute)).Any()))
            {
                builder.Advice.Override(builder.Target, nameof(this.OverrideMethod));
            }
        }


public override void BuildEligibility(IEligibilityBuilder<IMethod> builder)
        {
            base.BuildEligibility(builder);
            builder.MustNotBeStatic();
            builder.MustNotHaveAspectOfType(typeof(NoLogAttribute));
            builder.MustNotHaveAspectOfType(typeof(TimedLogMethodAttribute));
        }

First issue If I select the add method and call up the refactor AddAspect I would expect to be offered both, I'm not though just the TimedLogMethod is offered. If I comment out the builder.MustNotHaveAspectOfType(typeof(TimedLogMethodAttribute)); line then I get both offered. If I do this in the Editor (without commenting out the linne in the buildEligibility method)

c#

[TimedLogMethod]
        [LogMethod]
        private double Add(double a, double b) => a + b;

c#

[TimedLogMethod]
        [LogMethod]
        private double Add(double a, double b) => a + b;

I would expect, or thought I would get a red squiggle under [LogMethod] but I don't. I'm guessing my BuildEligibility Method is wrong but I can't work out why.

9 Replies

Gael Fraiteur•2y ago

What happens when you build?

domsinclairOP•2y ago

That's interesting I get a Lama0042 error InvalidOperationException System.InvalidOperationException: Operation is not valid due to the current state of the object. at Metalama.Framework.Engine.Utilities.UserCode.UserCodeExecutionContext.get_CompilationContext() at Metalama.Framework.Engine.Utilities.UserCode.UserCodeExecutionContext.ResolveCompileTimeTypeOf(String id, IReadOnlyDictionary

2 substitutions)
   at VtlSoftware.Logging.LogMethodAttribute.BuildEligibility(IEligibilityBuilder

1 builder) in C:\Users\dom\AppData\Local\Temp\Metalama\CompileTime\VtlSoftware.Logging.netcoreapp6.0\024aaf5bfa13ba0f\2023.1.5-preview\LogMethodAt_f67819f8.cs:line 51 at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.<>cDisplayClass5_0.<TryInvoke>b0() at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[TResult,TPayload](UserCodeFunc

2 func, TPayload& payload, UserCodeExecutionContext context)
   at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[T](Func

1 func, UserCodeExecutionContext context) at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.TryInvoke[T](Func`1 func, UserCodeExecutionContext context, T& result)

Gael Fraiteur•2y ago

So that's a bug. Could you report this call stack to GitHub pls?

domsinclairOP•2y ago

It's not a bug, it's my own stupid fault. The error was one I've made before. The cause was [NoLogAttribute] which is not technically a metalama aspect. I'm tempted to say that MustNotHaveAspectOfType and its partner MustHaveAspectOfType should both be renamed to: MustNotHaveMetalamaAspectOfType and MustHaveMetalamaAspectOfType That or make sure it's very clearly defined in the code comment so that intellisense picks it up.

Gael Fraiteur•2y ago

The call stack seems a bug.

domsinclairOP•2y ago

The project is on github if you want to have a look yourself. Certainly if you add the line:

c#
builder.MustNotHaveAspectOfType(typeof(NoLogAttribute));

c#
builder.MustNotHaveAspectOfType(typeof(NoLogAttribute));

to the BuildEligibility method of either the LogMethodAttribute or the TimedLogMethodAttribute then it will fail with that LAMA error, but as you pointed out to me last week in this context AspectOfType is specifically referring to metalama aspects. Conceivably It shouldn't fall over because of that which in turn makes it a bug, but equally I should have known better than to type what I did. Catch 22? https://github.com/domsinclair/VtlSoftware.LoggingAspects

GitHub

GitHub - domsinclair/VtlSoftware.LoggingAspects

Contribute to domsinclair/VtlSoftware.LoggingAspects development by creating an account on GitHub.

Gael Fraiteur•2y ago

@addabis Could you look at this?

addabis•2y ago

I have filed a bug.

Gael Fraiteur•2y ago

Thanks

Gaming

Programming

AddAspect Refactoring