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: Ability to intercept constructors #242

Closed jon-armen closed 7 months ago

jon-armen commented 10 months ago

We currently have a PostSharp aspect that intercepts a constructor by inheriting from OnMethodBoundaryAspect. We're in the process of transitioning to Metalama, and it would be great if we were able to change out our aspect. We use the aspect for logging purposes.

PostSharpBot commented 10 months ago

Hello @jon-armen, 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-34141.

gfraiteur commented 10 months ago

This feature is planned for v2024.1, which we will start working on from December 2023. Currently, you can add statements at the beginning of a constructor using the AddInitializer advice.

prochan2 commented 8 months ago

The first bits of this feature are released: https://github.com/orgs/postsharp/discussions/256

jon-armen commented 8 months ago

@prochan2 , thank you for the work on this. As a heads up, when I updated our metalama packages and implementing a constructor override, I did start getting crash reports on build. I don't believe this is related to introducing parameters or statements. MyConstructorOverrideAspect does work when applied directly to a constructor.

[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Constructor)]
public sealed class MyConstructorOverrideAspect : ConstructorAspect
{
    public override void BuildAspect(IAspectBuilder<IConstructor> builder)
    {
        builder.Advice.Override(builder.Target, "OverrideConstructor");
    }

    [Template]
    public void OverrideConstructor()
    {
        try
        {
            meta.Proceed();
        }
        catch (Exception ex)
        {
            throw;
        }
    }
}
public sealed class MyMulticastAspect : MulticastAspect, IAspect<IConstructor>
{
    public MyMulticastAspect()
        : base(MulticastTargets.InstanceConstructor | MulticastTargets.StaticConstructor)
    {
    }

    public void BuildAspect([NotNull] IAspectBuilder<IConstructor> builder)
    {
        builder.Outbound.AddAspectIfEligible<MyConstructorOverrideAspect>();
    }
}
Metalama Version: 2024.1.1-preview
Runtime: .NET Framework 4.8.9181.0
Processor Architecture: X64
OS Description: Microsoft Windows 10.0.22631 
OS Architecture: X64
Exception type: System.AggregateException
Exception message: One or more errors occurred.
===== Exception ===== 
System.AggregateException: One or more errors occurred. ---> System.ArgumentNullException: Value cannot be null.
Parameter name: key
   at System.Collections.Concurrent.ConcurrentDictionary`2.TryAdd(TKey key, TValue value)
   at Metalama.Framework.Engine.Linking.LinkerInjectionRegistry.<>c__DisplayClass10_0.<.ctor>g__ProcessOverride|4(KeyValuePair`2 value)
   at Metalama.Framework.Engine.Utilities.Threading.ConcurrentTaskRunner.<>c__DisplayClass1_0`1.<RunInParallelAsync>g__ProcessQueue|0()
   at System.Threading.Tasks.Task.Execute()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Utilities.Threading.ConcurrentTaskRunner.<RunInParallelAsync>d__1`1.MoveNext()
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at Metalama.Framework.Engine.Linking.LinkerInjectionRegistry..ctor(TransformationLinkerOrderComparer comparer, CompilationModel finalCompilationModel, PartialCompilation intermediateCompilation, IEnumerable`1 transformations, IReadOnlyCollection`1 injectedMembers, IDictionary`2 builderToTransformationMap, IConcurrentTaskRunner concurrentTaskRunner, CancellationToken cancellationToken)
   at Metalama.Framework.Engine.Linking.LinkerInjectionStep.<ExecuteAsync>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Linking.AspectLinker.<ExecuteAsync>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Pipeline.CompileTime.LinkerPipelineStage.<GetStageResultAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Pipeline.HighLevelPipelineStage.<ExecuteAsync>d__5.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Metalama.Framework.Engine.Pipeline.AspectPipeline.<ExecuteAsync>d__28.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Pipeline.CompileTime.CompileTimeAspectPipeline.<ExecuteCoreAsync>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Pipeline.CompileTime.CompileTimeAspectPipeline.<ExecuteAsync>d__2.MoveNext()
---> (Inner Exception #0) System.ArgumentNullException: Value cannot be null.
Parameter name: key
   at System.Collections.Concurrent.ConcurrentDictionary`2.TryAdd(TKey key, TValue value)
   at Metalama.Framework.Engine.Linking.LinkerInjectionRegistry.<>c__DisplayClass10_0.<.ctor>g__ProcessOverride|4(KeyValuePair`2 value)
   at Metalama.Framework.Engine.Utilities.Threading.ConcurrentTaskRunner.<>c__DisplayClass1_0`1.<RunInParallelAsync>g__ProcessQueue|0()
   at System.Threading.Tasks.Task.Execute()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Metalama.Framework.Engine.Utilities.Threading.ConcurrentTaskRunner.<RunInParallelAsync>d__1`1.MoveNext()<---
addabis commented 8 months ago

Thanks for letting us know @jon-armen. Indeed we have two outstanding bugs, one for compatibility with initializers and one for compatibility with parameter introduction. The exception is consistent with the latter. We will let you know when both are fixed.

gfraiteur commented 7 months ago

It has been fixed in 2024.1.3-preview.

jon-armen commented 7 months ago

@gfraiteur , great! I will try this out when I have the opportunity and let you know how it works for us.

jon-armen commented 7 months ago

@gfraiteur , @addabis , the normal application of the aspect working now. I have application of the aspect via multicasting that does not work. Here's a rough layout of the classes:

public sealed class TraceAttribute : MulticastAspect, IAspect<IConstructor>
{
    public TraceAttribute()
        : base(MulticastTargets.Property | MulticastTargets.Method | MulticastTargets.InstanceConstructor | MulticastTargets.StaticConstructor)
    {
    }

    /// <inheritdoc/>
    public void BuildAspect([NotNull] IAspectBuilder<IConstructor> builder)
    {
        builder.Outbound.AddAspectIfEligible<XRayTraceConstructorAttribute>();
    }
}
[AttributeUsage(AttributeTargets.Constructor)]
public sealed class TraceConstructorAttribute : ConstructorAspect
{
    /// <inheritdoc/>
    public override void BuildAspect(IAspectBuilder<IConstructor> builder)
    {
        builder.Advice.Override(builder.Target, "OverrideConstructor");
    }

    [Template]
    public void OverrideConstructor()
    {
        try
        {
            meta.Proceed();
        }
        catch (Exception)
        {
            throw;
        }
    }
}
   [Trace]
   public sealed class TestTarget
   {
       // Works when there is a constructor, does not work if constructor is missing.
       //public TestTarget()
       //{
       //    throw new NotImplementedException();
       //}
   }

Metalama Application: Metalama.DesignTime Metalama Version: 2024.1.3-preview Runtime: .NET 8.0.2 Processor Architecture: X64 OS Description: Microsoft Windows 10.0.22631 OS Architecture: X64 Exception type: System.AggregateException Exception message: One or more errors occurred. (Object reference not set to an instance of an object.) ===== Exception ===== System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.) ---> System.NullReferenceException: Object reference not set to an instance of an object. at Metalama.Framework.DesignTime.Pipeline.AspectPipelineResult.SplitResultsByTree(PartialCompilation compilation, DesignTimePipelineExecutionResult pipelineResults) at Metalama.Framework.DesignTime.Pipeline.AspectPipelineResult.Update(PartialCompilation compilation, DesignTimeProjectVersion projectVersion, DesignTimePipelineExecutionResult pipelineResults, AspectPipelineConfiguration configuration) at Metalama.Framework.DesignTime.Pipeline.DesignTimeAspectPipeline.PipelineState.ExecuteAsync(PipelineState state, PartialCompilation compilation, DesignTimeProjectVersion projectVersion, TestableCancellationToken cancellationToken) at Metalama.Framework.DesignTime.Pipeline.DesignTimeAspectPipeline.ExecutePartialAsync(PartialCompilation partialCompilation, DesignTimeProjectVersion projectVersion, AsyncExecutionContext executionContext, TestableCancellationToken cancellationToken) at Metalama.Framework.DesignTime.Pipeline.DesignTimeAspectPipeline.ExecuteAsync(Compilation compilation, Boolean autoResumePipeline, AsyncExecutionContext executionContext, TestableCancellationToken cancellationToken) at Metalama.Framework.DesignTime.Pipeline.DesignTimeAspectPipeline.ExecuteAsync(Compilation compilation, Boolean autoResumePipeline, AsyncExecutionContext executionContext, TestableCancellationToken cancellationToken) --- End of inner exception stack trace --- at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions) at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken) at Metalama.Framework.Engine.Utilities.Threading.TaskRunner.RunSynchronously[T](Func`1 func, CancellationToken cancellationToken) at Metalama.Framework.DesignTime.TheDiagnosticAnalyzer.AnalyzeSemanticModel(ISemanticModelAnalysisContext context)

addabis commented 7 months ago

We've released 2024.1.4-preview, which fixes several issues around overriding implicit constructors.

https://github.com/orgs/postsharp/discussions/265

jon-armen commented 7 months ago

Working great now! Thank you for all of the efforts.

addabis commented 7 months ago

No problem!

ManlyBoy commented 4 months ago

@gfraiteur Is there example code that implements a constructor aspect online yet. @jon-armen Any chance of posting your code that now works. I too just want to add logging to the constructors.

gfraiteur commented 4 months ago

@ManlyBoy Here is the documentation. There is a short example. https://doc.postsharp.net/metalama/conceptual/aspects/advising/overriding-constructors