postsharp / Metalama

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

Suggestion: Automatically remove semicolon from expression passed to ExpressionFactory.Parse #112

Closed WhitWaldo closed 1 year ago

WhitWaldo commented 1 year ago

Given the class I'm looking to apply the aspect to:

[MyAspect]
public class Repository<T>
{
}

And my aspect:

public class MyAspect : TypeAspect
{
        var listType = ((INamedType) TypeFactory.GetType(typeof(List<>))).WithTypeArguments(typeof(string));
        builder.Advice.IntroduceField(builder.Target, "_entities", listType,
            IntroductionScope.Instance, OverrideStrategy.Ignore,
            fieldBuilder =>
            {
                //fieldBuilder.InitializerExpression = ExpressionFactory.Parse("new List<string>();"); // Exception 1
                //fieldBuilder.InitializerExpression = ExpressionFactory.Parse("new();"); // Exception 2
                fieldBuilder.Accessibility = Accessibility.Private;
            });
}

Applying the aspect as-is works exactly as expected. But applying either of the commented out lines throws an exception simply because there's a semicolon on the end. The error sheds no real light on that being the issue (simply that it cannot be parsed as an expression):

Metalama.Framework.Engine.Diagnostics.DiagnosticException: Code 'new List();' could not be parsed as an expression. (1,1): error CS1073: Unexpected token ';' at Metalama.Framework.Engine.Templating.SyntaxFactoryEx.ParseExpressionSafe(String text) at Metalama.Framework.Engine.Templating.Expressions.SyntaxBuilderImpl.ParseExpression(String code) at Metalama.Framework.Code.SyntaxBuilders.ExpressionFactory.Parse(String code) at Metalama.Bits.Repository.FieldInjectAspect.<>c.b00(IFieldBuilder fieldBuilder) in C:\Users\whit\AppData\Local\Temp\Metalama\CompileTime\Metalama.Bits.netcoreapp7.0\11663c6ec12c7c16\2023.0.113-rc\FieldInject_a9099473.cs:line 23 at Metalama.Framework.Engine.Advising.AdviceFactory.<>c__DisplayClass31_0.b0(IFieldBuilder builder) at Metalama.Framework.Engine.Advising.IntroduceMemberAdvice2.Initialize(ProjectServiceProvider serviceProvider, IDiagnosticAdder diagnosticAdder) at Metalama.Framework.Engine.Advising.AdviceFactory.ExecuteAdvice[T](Advice advice) at Metalama.Framework.Engine.Advising.AdviceFactory.IntroduceField(INamedType targetType, String fieldName, IType fieldType, IntroductionScope scope, OverrideStrategy whenExists, Action1 buildField, Object tags) at Metalama.Bits.Repository.FieldInjectAspect.BuildAspect(IAspectBuilder1 builder) in C:\Users\whit_\AppData\Local\Temp\Metalama\CompileTime\Metalama.Bits\.netcoreapp7.0\11663c6ec12c7c16\2023.0.113-rc\FieldInject_a9099473.cs:line 19 at Metalama.Framework.Engine.Aspects.AspectDriver.<>c__DisplayClass6_11.b3() at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.<>cDisplayClass5_0.b__0() at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[TResult,TPayload](UserCodeFunc2 func, TPayload& payload, UserCodeExecutionContext context) at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[T](Func1 func, UserCodeExecutionContext context) at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.TryInvoke[T](Func`1 func, UserCodeExecutionContext context, T& result)

I would suggest either improving the error to suggest that the semicolon should be left off the expression or automatically accommodate that yourself (and just drop the semicolon from the end of the input text).

Thanks for the consideration!

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-33071.

gfraiteur commented 1 year ago

I will decline this one because we don't want to go that path of fixing user's input. I don't see how we could improve the error message in any significant way. The text CS1073: Unexpected token ';' is what Roslyn gives us and we're just passing that along to you.