postsharp / Metalama

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

Bug: Expression throwing exception when built using expression builder #201

Closed WhitWaldo closed 11 months ago

WhitWaldo commented 11 months ago

I'm trying to use the following to rewrite a LINQ expression within a template method:

                            var predBuilder = new ExpressionBuilder();
                            predBuilder.AppendVerbatim("var updatedPredicate = Expression.Lambda<Func<");
                            predBuilder.AppendTypeName(index.EntityPropertyType);
                            predBuilder.AppendVerbatim(", bool>> (");
                            predBuilder.AppendVerbatim("Expression.Equal(");
                            predBuilder.AppendVerbatim("Expression.Property(");
                            predBuilder.AppendVerbatim("Expression.Parameter(typeof(");
                            predBuilder.AppendTypeName(index.EntityPropertyType);
                            predBuilder.AppendVerbatim("), \"p\"),");
                            predBuilder.AppendVerbatim("visitor.PropertyName),");
                            predBuilder.AppendVerbatim("predicate.Parameters[0]),");
                            predBuilder.AppendVerbatim("predicate.Parameters)");

                            meta.InsertStatement(predBuilder.ToExpression());

This is yielding the following exception when I build a project this aspect is applied to with the LamaDebug configuration:

LAMA0041 'Exception of type 'Metalama.Framework.Engine.Diagnostics.DiagnosticException' thrown while executing the template method IndexedAttribute.QueryIndexAsync<TEntity, TEntityKey>(Dictionary<string, List>, IEntityRepository<TEntity, TEntityKey>, Expression<Func<TEntity, bool>>, CancellationToken): Code 'var updatedPredicate = Expression.Lambda<Func<global::System.Int32, bool>> (Expression.Equal(Expression.Property(Expression.Parameter(typeof(global::System.Int32), "p"),visitor.PropertyName),predicate.Parameters[0]),predicate.Parameters)' could not be parsed as an expression. ABC.Common.ServiceFabric.EntityManager.Objects I:\MyProject\CSC 1 Active

If I just grab the expression as provided in the error and drop it into the template method, VS doesn't report any issues with it (except for Metalama complaining about the mix of run-time and compile-time values, which is why I'm trying to use an expression builder in the first place).

                            var updatedPredicate = Expression.Lambda<Func<global::System.Int32, bool>>(
                                    Expression.Equal(
                                        Expression.Property(
                                            Expression.Parameter(typeof(global::System.Int32), "p"),
                                            visitor.PropertyName),
                                        predicate.Parameters[0]),
                                    predicate.Parameters);

Using 2023.3.1-preview

Any thoughts? Thanks!

PostSharpBot commented 11 months 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-33609.

WhitWaldo commented 11 months ago

Still throwing even if I try to move some of this out of AppendVertabim:

                            var funcType =
                                ((INamedType) TypeFactory.GetType(typeof(Func<,>))).WithTypeArguments(
                                    index.EntityPropertyType, typeof(bool));
                            var predBuilder = new ExpressionBuilder();
                            predBuilder.AppendVerbatim("var updatedPredicate = Expression.Lambda<");
                            predBuilder.AppendTypeName(funcType);
                            predBuilder.AppendVerbatim(">(");
                            predBuilder.AppendVerbatim("Expression.Equal(");
                            predBuilder.AppendVerbatim("Expression.Property(");
                            predBuilder.AppendVerbatim("Expression.Parameter(typeof(");
                            predBuilder.AppendTypeName(index.EntityPropertyType);
                            predBuilder.AppendVerbatim("), \"p\"),");
                            predBuilder.AppendExpression(visitor.PropertyName);
                            predBuilder.AppendVerbatim("),");
                            predBuilder.AppendExpression(predicate.Parameters[0]);
                            predBuilder.AppendVerbatim("),");
                            predBuilder.AppendExpression(predicate.Parameters);
                            predBuilder.AppendVerbatim(")");

                            meta.InsertStatement(predBuilder.ToExpression());

LAMA0041 'Exception of type 'Metalama.Framework.Engine.Diagnostics.DiagnosticException' thrown while executing the template method IndexedAttribute.QueryIndexAsync<TEntity, TEntityKey>(Dictionary<string, List>, IEntityRepository<TEntity, TEntityKey>, Expression<Func<TEntity, bool>>, CancellationToken): Code 'var updatedPredicate = Expression.Lambda<global::System.Func<global::System.Int32,global::System.Boolean>>(Expression.Equal(Expression.Property(Expression.Parameter(typeof(global::System.Int32), "p"),visitor.PropertyName),predicate.Parameters[0]),predicate.Parameters)' could not be parsed as an expression.

WhitWaldo commented 11 months ago

I'm starting to think Metalama is misidentifying this as compile-time code when it should be outputting as run-time code. I say this because when I just pasted in the expression as produced in the error (and cleaned up a little):

                            var updatedPredicate =
                                Expression.Lambda<Func<int, bool>>(
                                    Expression.Equal(
                                        Expression.Property(Expression.Parameter(typeof(int), "p"),
                                            visitor.PropertyName), predicate.Parameters[0]), predicate.Parameters);

This now instead calls out Expression.Parameter(typeof(int), "p") explicitly with an error:

LAMA0200 A compile-time value of type 'ParameterExpression' was used in a context where a run-time value was expected.

I'm intending to have Metalama produce this expression based on a compile-time type which I'll use downstream via updatedPredicate.Compile() in indexField.TryGetValues(updatedPredicate.Compile(), out HashSet<TEntityKey> recordKeys); meaning that Metalama is incorrectly identifying the rendered expression as compile-time code that should instead be considered run-time code.

My hope is that fixing that misidentification would also then fix the exception thrown indicating this expression isn't valid.

svick commented 11 months ago

The error is saying that the built string could not be parsed as an expression and that's accurate: var x = expression is not a valid C# expression. Since you're trying to build a statement, not an expression, you need to use StatementBuilder instead of ExpressionBuilder. I think switching to that (and adding a ; at the end) will fix the error.

svick commented 11 months ago

The error for Expression.Parameter looks like a bug to me and I will look into it. If you did want to write that code directly in a template, you can use meta.RunTime as a workaround:

Expression.Property(meta.RunTime(Expression.Parameter(typeof(int), "p")), "propertyName")

But this problem shouldn't have any effect on your original code; switching to StatementBuild should still work.

WhitWaldo commented 11 months ago

You're right - I completely forgot about the StatementBuilder. However, I just tried using that, making no changes to the latest iteration and I'm still seeing an exception (though it doesn't output the attempted statement this time in the text, so hard to see if it's right or not):

'Exception of type 'Metalama.Framework.Engine.Diagnostics.DiagnosticException' thrown while executing the template method IndexedAttribute.QueryIndexAsync<TEntity, TEntityKey>(Dictionary<string, List>, IEntityRepository<TEntity, TEntityKey>, Expression<Func<TEntity, bool>>, CancellationToken): Code could not be parsed as a statement.

                            var funcType =
                                ((INamedType)TypeFactory.GetType(typeof(Func<,>))).WithTypeArguments(
                                    index.EntityPropertyType, typeof(bool));
                            var predBuilder = new StatementBuilder();
                            predBuilder.AppendVerbatim("var updatedPredicate = Expression.Lambda<");
                            predBuilder.AppendTypeName(funcType);
                            predBuilder.AppendVerbatim(">(");
                            predBuilder.AppendVerbatim("Expression.Equal(");
                            predBuilder.AppendVerbatim("Expression.Property(");
                            predBuilder.AppendVerbatim("Expression.Parameter(typeof(");
                            predBuilder.AppendTypeName(index.EntityPropertyType);
                            predBuilder.AppendVerbatim("), \"p\"),");
                            predBuilder.AppendExpression(visitor.PropertyName);
                            predBuilder.AppendVerbatim("),");
                            predBuilder.AppendExpression(predicate.Parameters[0]);
                            predBuilder.AppendVerbatim("),");
                            predBuilder.AppendExpression(predicate.Parameters);
                            predBuilder.AppendVerbatim(")");

                            meta.InsertStatement(predBuilder.ToStatement());
svick commented 11 months ago

The exception does not make it easy to diagnose this, but it looks like you are missing a semicolon at the end of the statement.

WhitWaldo commented 11 months ago

Yeah, it'd be helpful is the exception included the produced statement that failed like the expression builder does. I added the semicolon, but I'm still getting the exception that this code could not be parsed as a statement.

Oddly, that hasn't stopped it from producing... something that looks close to what I'm aiming for. I'll tear this apart and see if I can't tease out a further question here.

WhitWaldo commented 11 months ago

Yes, so looking at the output, this appears to be producing a valid statement:

var updatedPredicate0 = Expression.Lambda<global::System.Func<global::System.Int32, global::System.Boolean>>(Expression.Equal(Expression.Property(Expression.Parameter(typeof(global::System.Int32), "p"), visitor.PropertyName), predicate.Parameters[0]), predicate.Parameters);

So I'm getting two exceptions when I attempt to build this (though it does oddly allow the downstream project to rebuild properly):

LAMA0041 'Exception of type 'Metalama.Framework.Engine.Diagnostics.DiagnosticException' thrown while executing the template method IndexedAttribute.QueryIndexAsync<TEntity, TEntityKey>(Dictionary<string, List>, IEntityRepository<TEntity, TEntityKey>, Expression<Func<TEntity, bool>>, CancellationToken): Code could not be parsed as a statement. ... I:...\CSC 1 Active

CS1002 ; expected. Exception details are in 'C:\Users...\AppData\Local\Temp\Metalama\CrashReports\2023.3.1-preview\exception-2c943536-a3a9-4dc4-827b-fab6c68d81c7.txt'. To attach a debugger to the compiler, use the '-p:MetalamaDebugCompiler=True' command-line option. ... I:...\CSC 1 Active

Here's the body of the exception itself:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> Metalama.Framework.Engine.Diagnostics.DiagnosticException: Code could not be parsed as a statement. (1,269): error CS1002: ; expected at Metalama.Framework.Engine.Templating.SyntaxFactoryEx.ParseStatementSafe(String text) at Metalama.Framework.Engine.Templating.Expressions.SyntaxBuilderImpl.ParseStatement(String code) at Metalama.Framework.Code.SyntaxBuilders.StatementBuilder.ToStatement() at ....Aspects.IndexedAttribute.<>cDisplayClass16_1.<__QueryIndexAsync_cd5bf237e59cd8ad>b1() in C:\Users...\AppData\Local\Temp\Metalama\CompileTime....netcoreapp7.0\e5747381169872eb\2023.3.1-preview\IndexedAttr_05669800.cs:line 694 at ....Aspects.IndexedAttribute.<>cDisplayClass16_1.<__QueryIndexAsync_cd5bf237e59cd8ad>b0() in C:\Users...\AppData\Local\Temp\Metalama\CompileTime....netcoreapp7.0\e5747381169872eb\2023.3.1-preview\IndexedAttr_05669800.cs:line 595 at ....Aspects.IndexedAttribute.QueryIndexAsync_cd5bf237e59cd8ad(ITemplateSyntaxFactory templateSyntaxFactory, Dictionary`2 indexMetadata, ExpressionSyntax entityStateStore, ExpressionSyntax predicate, ExpressionSyntax cancellationToken, TemplateTypeArgument TEntity, TemplateTypeArgument TEntityKey) in C:\Users...\AppData\Local\Temp\Metalama\CompileTime....netcoreapp7.0\e5747381169872eb\2023.3.1-preview\IndexedAttr_05669800.cs:line 339 --- End of inner exception stack trace --- at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor) at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments) at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) at Metalama.Framework.Engine.Templating.TemplateDriver.<>cDisplayClass3_0.b__0() at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[TResult,TPayload](UserCodeFunc2 func, TPayload& payload, UserCodeExecutionContext context, Boolean wrapException) at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.Invoke[T](Func1 func, UserCodeExecutionContext context, Boolean wrapExceptions) at Metalama.Framework.Engine.Utilities.UserCode.UserCodeInvoker.TryInvoke[T](Func`1 func, UserCodeExecutionContext context, T& result)

It appears to reference line 269 from my source. That aligns with:

predBuilder.AppendExpression(predicate.Parameters[0]);

This doesn't make any sense as that's not the end of the statement I'm adding. Rather, the whole thing is:

                                var funcType =
                                    ((INamedType) TypeFactory.GetType(typeof(Func<,>))).WithTypeArguments(
                                        index.EntityPropertyType, typeof(bool));
                                var predBuilder = new StatementBuilder();
                                predBuilder.AppendVerbatim($"var updatedPredicate{ii} = Expression.Lambda<");
                                predBuilder.AppendTypeName(funcType);
                                predBuilder.AppendVerbatim(">(");
                                predBuilder.AppendVerbatim("Expression.Equal(");
                                predBuilder.AppendVerbatim("Expression.Property(");
                                predBuilder.AppendVerbatim("Expression.Parameter(typeof(");
                                predBuilder.AppendTypeName(index.EntityPropertyType);
                                predBuilder.AppendVerbatim("), \"p\"),");
                                predBuilder.AppendExpression(visitor.PropertyName);
                                predBuilder.AppendVerbatim("),");
                                predBuilder.AppendExpression(predicate.Parameters[0]);
                                predBuilder.AppendVerbatim("),");
                                predBuilder.AppendExpression(predicate.Parameters);
                                predBuilder.AppendVerbatim(");");

                                meta.InsertStatement(predBuilder.ToStatement());

Thank you!

svick commented 11 months ago

I don't see how you could be both getting an exception and also an output. Do you have another StatementBuilder somewhere else? (Possibly another configuration or target framework of the same project.) Or trying to build from the command line while having unsaved changes?

It appears to reference line 269 from my source.

No, it references line 1, column 269 of the code that's inside the StatementBuilder.

If you can't figure out what's going on in another way, consider debugging into the template code.

WhitWaldo commented 11 months ago

I think Visual Studio must have cached something badly. A reboot and relaunching of Visual Studio and I'm unable to reproduce the exception any longer. Thanks for the assist!

gfraiteur commented 10 months ago

Solved in 2023.3.3-preview.