Eligibility inquiry

[WhitWaldo]

I've declared the BuildEligibility on three aspects indicating the following:

//Type cannot be static
builder.MustSatisfy(t => !t.IsStatic, t => $"The type cannot be static");

//Type cannot be a record type
builder.MustSatisfy(t => t.TypeKind is not (TypeKind.RecordStruct or TypeKind.RecordClass),
    x => $"Cannot inject into a record type");

//Type must be explicitly declared
builder.MustBeExplicitlyDeclared();

I've then got a fabric that's attempting to broadly apply aspects to those types with more than one method that don't implement a base type called "Workflow", but to only add the aspect if eligible:

public sealed class Fabric : TransitiveProjectFabric
{
    public override void AmendProject(IProjectAmender amender)
    {
        amender.SelectMany(p => p.Types)
            .Where(t => t.Methods.Count > 0) //Should have at least one method to decorate or no point
            .Where(t => t.BaseType?.Name != "Workflow") //Should not log within Dapr Workflow instances
            .AddAspectIfEligible<InjectLoggerAttribute>();
    }
}

The aspect has an intended application order:

[assembly: AspectOrder(AspectOrderDirection.RunTime, typeof(LogAttribute), typeof(InjectLoggerFactoryAttribute), typeof(InjectLoggerAttribute))]

And the InjectLoggerAttribute applies the remaining two aspects as an outbound activity:

builder.Outbound.SelectMany(a => a.Constructors)
    .AddAspectIfEligible<InjectLoggerFactoryAttribute>();
builder.Outbound.SelectMany(a => a.Methods)
    .AddAspectIfEligible<LogAttribute>();

Nonetheless, I'm showing errors in VS that Metalama has paused analysis of the project because of errors in the compile-time code. When I dig into what these errors are, I'm seeing that it's because the fabric appears to be applying the aspect to types that match the fabric, but should be deemed ineligible per the eligibility methods on each aspect:

I would instead expect that the following would happen:

• Fabric identifies the static class as a viable target per the amender filter
• It validates the eligibility method on the attribute against the target
• Eligibility fails, so the attribute isn't applied. Because the attribute isn't applied, it also doesn't attempt to apply either the InjectLogerFactoryAttribute or the LogAttribute

Rather, it appears to be ignoring the eligibility check on the InjectLoggerAttribute and instead applying it per the fabric, then ignoring the eligibility check for the InjectLoggerFactoryAttribute and trying to apply it.. but not finding it for some reason. Perhaps because it's ineligible.

Am I doing something wrong here or should I re-file this as a bug report?

[WhitWaldo]

On the theory that perhaps I'm applying the eligibility incorrectly by specifying all the individual builder.MustSatisfy statements, I combined them and instead have:

        builder.MustSatisfyAll(
            //Must be explicitly declared
            _ => builder.MustBeExplicitlyDeclared(),
            //Must not be static
            _ => builder.MustSatisfy(t => !t.IsStatic, n => $"The method cannot be static"),
            //Must not be a record type
            _ => builder.DeclaringType().MustSatisfy(t => t.TypeKind is not (TypeKind.RecordStruct or TypeKind.RecordClass),
                x => $"The declaring type must not be a record"));

Nonetheless, I still see precisely the same thing happening in that it's attempting to apply the aspect to the static class and, of course, failing.

[gfraiteur]

Off-topic but your message for the reason must include the lambda parameter:

 => builder.MustSatisfy(t => !t.IsStatic, n => $"{n} cannot be static"),

[svick]

Can you provide the crash report mentioned in the error message? Thanks.

[WhitWaldo]

System.Collections.Generic.KeyNotFoundException: The given key 'xxx.Common.Aspects.DI.LoggerFactory.InjectLoggerFactoryAttribute' was not present in the dictionary.
   at System.Collections.Immutable.ImmutableDictionary`2.get_Item(TKey key)
   at Metalama.Framework.Engine.Fabrics.AspectReceiver`2.GetAspectClass(Type aspectType)
   at Metalama.Framework.Engine.Fabrics.AspectReceiver`2.AddAspectIfEligible[TAspect](EligibleScenarios eligibility)
   at Empirica.Common.Aspects.DI.LoggerFactory.InjectLoggerAttribute.BuildAspect(IAspectBuilder`1 builder) in C:\Users\whit_\AppData\Local\Temp\Metalama\CompileTime\xxx.Common.Aspects.DI.Logge\.netcoreapp8.0\230693273045dd61\2024.2.16-rc\InjectLogge_1747e784.cs:line 68
   at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.<>c__DisplayClass6_0.<TryInvoke>b__0()
   at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[TResult,TPayload](UserCodeFunc`2 func, TPayload& payload, UserCodeExecutionContext context, Boolean wrapException)
   at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.TryInvoke[T](Func`1 func, UserCodeExecutionContext context, T& result)

[svick]

Though I don't see why the eligibility check for InjectLoggerAttribute would fail here, since the type in question is not static.

So, as far as I can tell, what should and does happen here is:

• Fabric filters pass for MyNotStaticThing.
• Eligibility for InjectLoggerAttribute on MyNotStaticThing passes, so InjectLoggerAttribute is applied and executed.
• Eligibility for LogAttribute on MyNotStaticThing.TheStaticMethodHiddenBehindTooltip fails, so that aspect is not applied.

The exception indicates there is still some problem here (possibly a bug in Metalama), but I don't think it's related to eligibility.

[WhitWaldo]

I can put this aspect up on GitHub in a few minutes and link it so you can fiddle with it yourself - going to apply some QoL changes to it that @gfraiteur drew my attention to on the other thread.

[WhitWaldo]

@svick Created the repo here - if you look at the attempted aspect tests in Innovian.Aspects.Logging.Testing, you'll see what I'm having an issue with (I renamed each type to be a little clearer):

• MultipleConstructors.cs - just see the red squiggly
• TestWorkflow.cs - throws when trying to view the transformed code
• StaticMethod.cs - throws when trying to view the transformed code
• StaticClass.cs - works fine here
• StandardExample.cs - throws when trying to view the transformed code
• RecordType.cs - works fine
• DoubleInjection.cs - see red squiggly on class name + throws when trying to view transformed code

[svick]

When I built your solution, I got the following error:

InjectLoggerFactoryAttribute.cs(20,9,20,75): error LAMA0041: 'Exception of type 'System.InvalidOperationException' thrown while executing BuildEligibility for aspect 'InjectLoggerFactory': TypeFactory is not available in this context. In BuildEligibility, TypeFactory can only be used inside eligibility delegates. ...

Once I fixed that, by changing the method to the following, things started working.

public override void BuildEligibility(IEligibilityBuilder<IConstructor> builder)
{
    builder.MustSatisfyAll(
        //Must be explicitly declared
        _ => builder.MustBeExplicitlyDeclared(),
        //Must not be static
        _ => builder.MustSatisfy(t => !t.IsStatic, n => $"The method cannot be static"),
        //Must not be a record type
        _ => builder.DeclaringType().MustSatisfy(t => t.TypeKind is not (TypeKind.RecordStruct or TypeKind.RecordClass),
            x => $"The declaring type must not be a record"),
        _ => builder.DeclaringType()
            .MustSatisfy(t => t.AllFieldsAndProperties.All(fp => fp.Name != "_logger" && fp.Type != (INamedType)TypeFactory.GetType(typeof(ILogger))),
                _ => $"The field _logger of type ILogger cannot already exist in the type"));
}

So, the bug in Metalama is twofold:

1. It doesn't display the relevant error at design-time (i.e. if you don't explicitly build).
2. It does display bogus errors.

I will file this and try to fix it during our current design-time bug-fixing push. But since this shouldn't be blocking you, it might take some time before I get to it.

[gfraiteur]

An alternative would be to write fp.Type is not INamedType { Name: nameof(ILogger) } or more simply !f.Type.Is(typeof(ILogger)),

[WhitWaldo]

Thank you for the help! It's working fine on my end now.

[gfraiteur]

The bug is fixed in 2024.2.25.