From a19a16b4919570b9ad05b132331ac239dd6cfbd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Fri, 11 Mar 2022 10:59:49 +0000 Subject: [PATCH] Fix error message when a method in binding does not exist It used to produce something like "must be reducible node", which is really unhelpful. Now it will be something like "Method X was not found" --- .../Binding/MethodGroupExpression.cs | 40 ++++++++++++++++++- .../Binding/StaticClassIdentifier.cs | 5 ++- .../UnknownStaticClassIdentifierExpression.cs | 2 +- .../Binding/StaticCommandCompilationTests.cs | 10 +++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/Framework/Framework/Compilation/Binding/MethodGroupExpression.cs b/src/Framework/Framework/Compilation/Binding/MethodGroupExpression.cs index 4ba90ec768..eb53b1380e 100644 --- a/src/Framework/Framework/Compilation/Binding/MethodGroupExpression.cs +++ b/src/Framework/Framework/Compilation/Binding/MethodGroupExpression.cs @@ -4,11 +4,12 @@ using System.Linq.Expressions; using System.Reflection; using DotVVM.Framework.Utils; +using FastExpressionCompiler; namespace DotVVM.Framework.Compilation.Binding { - public class MethodGroupExpression : Expression + public sealed class MethodGroupExpression : Expression { public override ExpressionType NodeType => ExpressionType.Extension; public override Type Type => typeof(Delegate); @@ -52,9 +53,32 @@ public MethodGroupExpression(Expression target, string methodName, Type[]? typeA return Expression.Lambda(delegateType, call, args); } - protected MethodInfo? GetMethod() + private MethodInfo? GetMethod() => Target.Type.GetMethod(MethodName, BindingFlags.Public | (IsStatic ? BindingFlags.Static : BindingFlags.Instance)); + private Exception Error() + { + if (Target.Type == typeof(UnknownTypeSentinel)) + return new Exception($"Type of '{Target}' could not be resolved."); + + var candidateMethods = + Target.Type + .GetAllMethods(BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance | BindingFlags.NonPublic) + .Where(m => m.Name == MethodName) + .ToArray(); + + if (!candidateMethods.Any()) + return new Exception($"Method '{Target.Type.ToCode(stripNamespace: true)}.{MethodName}' not found."); + if (!candidateMethods.Any(m => m.IsStatic == this.IsStatic)) + return new Exception($"{(this.IsStatic ? "Static" : "Instance")} method '{Target.Type.ToCode(stripNamespace: true)}.{MethodName}' not found, but {(this.IsStatic ? "an instance" : "a static")} method exists."); + var matchingMethods = candidateMethods.Where(m => m.IsStatic == this.IsStatic).ToArray(); + if (!matchingMethods.Any()) + return new Exception($"Method '{Target.Type.ToCode(stripNamespace: true)}.{MethodName}' not found, but a private method exists."); + if (matchingMethods.Length > 1) + return new Exception($"Multiple matching overloads of method '{Target.Type.ToCode(stripNamespace: true)}.{MethodName}' exist."); + throw new Exception("Internal error"); + } + public Expression CreateDelegateExpression() { var methodInfo = GetMethod(); @@ -82,6 +106,18 @@ public override Expression Reduce() { return CreateDelegateExpression(); } + protected override Expression VisitChildren(ExpressionVisitor visitor) + { + if (GetMethod() is null) throw Error(); + + return base.VisitChildren(visitor); + } + protected override Expression Accept(ExpressionVisitor visitor) + { + if (GetMethod() is null) throw Error(); + + return base.Accept(visitor); + } public override string ToString() { diff --git a/src/Framework/Framework/Compilation/Binding/StaticClassIdentifier.cs b/src/Framework/Framework/Compilation/Binding/StaticClassIdentifier.cs index 3c79c074c7..7539b1042c 100644 --- a/src/Framework/Framework/Compilation/Binding/StaticClassIdentifier.cs +++ b/src/Framework/Framework/Compilation/Binding/StaticClassIdentifier.cs @@ -7,7 +7,10 @@ public sealed class StaticClassIdentifierExpression: Expression { public override Type Type { get; } public override ExpressionType NodeType => ExpressionType.Extension; - public override Expression Reduce() => throw new Exception($"Cannot use type name {this.Type.FullName} as an expression"); + public Exception Error() => new Exception($"Cannot use type name {this.Type.FullName} as an expression"); + public override Expression Reduce() => throw Error(); + protected override Expression VisitChildren(ExpressionVisitor visitor) => throw Error(); + protected override Expression Accept(ExpressionVisitor visitor) => throw Error(); public StaticClassIdentifierExpression(Type type) :base() diff --git a/src/Framework/Framework/Compilation/Binding/UnknownStaticClassIdentifierExpression.cs b/src/Framework/Framework/Compilation/Binding/UnknownStaticClassIdentifierExpression.cs index 421894cea6..6a7c1c1bd6 100644 --- a/src/Framework/Framework/Compilation/Binding/UnknownStaticClassIdentifierExpression.cs +++ b/src/Framework/Framework/Compilation/Binding/UnknownStaticClassIdentifierExpression.cs @@ -3,7 +3,7 @@ namespace DotVVM.Framework.Compilation.Binding { - public class UnknownStaticClassIdentifierExpression: Expression + public sealed class UnknownStaticClassIdentifierExpression: Expression { public UnknownStaticClassIdentifierExpression(string name) { diff --git a/src/Tests/Binding/StaticCommandCompilationTests.cs b/src/Tests/Binding/StaticCommandCompilationTests.cs index d547ce9881..c299cd2a1d 100644 --- a/src/Tests/Binding/StaticCommandCompilationTests.cs +++ b/src/Tests/Binding/StaticCommandCompilationTests.cs @@ -407,6 +407,16 @@ public void StaticCommandCompilation_LinqTranslations() AreEqual(expectedResult, result); } + [TestMethod] + public void StaticCommandCompilation_FailReasonablyOnInvalidMethod() + { + TestMarkupControl.CreateInitialized(); + + var result = Assert.ThrowsException(() => CompileBinding("TestViewModel.GetCharCode", false, typeof(TestViewModel))); + + Assert.AreEqual("Static method 'TestViewModel.GetCharCode' not found, but an instance method exists.", result.GetBaseException().Message); + } + public void AreEqual(string expected, string actual) => Assert.AreEqual(RemoveWhitespaces(expected), RemoveWhitespaces(actual));