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"
riganti · Mar 11, 2022 · a19a16b · a19a16b
1 parent 62ad3de
commit a19a16b
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -407,6 +407,16 @@ public void StaticCommandCompilation_LinqTranslations()
             AreEqual(expectedResult, result);
         }
 
+        [TestMethod]
+        public void StaticCommandCompilation_FailReasonablyOnInvalidMethod()
+        {
+            TestMarkupControl.CreateInitialized();
+
+            var result = Assert.ThrowsException<BindingPropertyException>(() => 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));