Fix double evaluation when Operator Overloading is enabled (#945)

* Fix double evaluation when Operator Overloading is enabled.
* Small behaviour fixes to JintUnaryExpression
* Added test for the new feature. Fixed failing ones and reimplemented a bug that needs more work to actually fix correctly

Jint.Tests/Runtime/OperatorOverloadingTests.cs

@@ -315,5 +315,29 @@ namespace Jint.Tests.Runtime
             ");
         }
 
+        [Fact]
+        public void OperatorOverloading_ShouldEvaluateOnlyOnce()
+        {
+            RunTest(@"
+                var c;
+                var resolve = v => { c++; return v; };
+
+                c = 0;
+                var n1 = resolve(1) + 2;
+                equal(n1, 3);
+                equal(c, 1);
+
+                c = 0;
+                var n2 = resolve(2) + resolve(3);
+                equal(n2, 5);
+                equal(c, 2);
+
+                c = 0;
+                var n3 = -resolve(1);
+                equal(n3, -1);
+                equal(c, 1);
+            ");
+        }
+
     }
-}
+}

Jint/Runtime/Interpreter/Expressions/JintBinaryExpression.cs

@@ -27,9 +27,9 @@ namespace Jint.Runtime.Interpreter.Expressions
             _right = Build(_engine, expression.Right);
         }
 
-        protected bool TryOperatorOverloading(string clrName, out object result)
+        protected bool TryOperatorOverloading(JsValue left, JsValue right, string clrName, out object result)
         {
-            return TryOperatorOverloading(_engine, _left.GetValue(), _right.GetValue(), clrName, out result);
+            return TryOperatorOverloading(_engine, left, right, clrName, out result);
         }
 
         internal static bool TryOperatorOverloading(Engine engine, JsValue leftValue, JsValue rightValue, string clrName, out object result)
@@ -267,14 +267,14 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_LessThan", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_LessThan", out var opResult))
                 {
                     return opResult;
                 }
-
-                var left = _left.GetValue();
-                var right = _right.GetValue();
                 var value = Compare(left, right);
 
                 return value._type == InternalTypes.Undefined
@@ -291,14 +291,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_GreaterThan", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_GreaterThan", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
                 var value = Compare(right, left, false);
 
                 return value._type == InternalTypes.Undefined
@@ -315,15 +316,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_Addition", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_Addition", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 if (AreIntegerOperands(left, right))
                 {
                     return JsNumber.Create(left.AsInteger() + right.AsInteger());
@@ -344,15 +345,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_Subtraction", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_Subtraction", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 return AreIntegerOperands(left, right)
                     ? JsNumber.Create(left.AsInteger() - right.AsInteger())
                     : JsNumber.Create(TypeConverter.ToNumber(left) - TypeConverter.ToNumber(right));
@@ -367,15 +368,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_Multiply", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_Multiply", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 if (AreIntegerOperands(left, right))
                 {
                     return JsNumber.Create((long) left.AsInteger() * right.AsInteger());
@@ -398,15 +399,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_Division", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_Division", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 return Divide(left, right);
             }
         }
@@ -422,15 +423,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading(_invert ? "op_Inequality" : "op_Equality", out var opResult))
+                    && TryOperatorOverloading(left, right, _invert ? "op_Inequality" : "op_Equality", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 return Equal(left, right) == !_invert
                     ? JsBoolean.True
                     : JsBoolean.False;
@@ -448,15 +449,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var leftValue = _left.GetValue();
+                var rightValue = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading(_leftFirst ? "op_GreaterThanOrEqual" : "op_LessThanOrEqual", out var opResult))
+                    && TryOperatorOverloading(leftValue, rightValue, _leftFirst ? "op_GreaterThanOrEqual" : "op_LessThanOrEqual", out var opResult))
                 {
                     return opResult;
                 }
 
-                var leftValue = _left.GetValue();
-                var rightValue = _right.GetValue();
-
                 var left = _leftFirst ? leftValue : rightValue;
                 var right = _leftFirst ? rightValue : leftValue;
 
@@ -525,15 +526,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading("op_Modulus", out var opResult))
+                    && TryOperatorOverloading(left, right, "op_Modulus", out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 if (AreIntegerOperands(left, right))
                 {
                     var leftInteger = left.AsInteger();
@@ -588,15 +589,15 @@ namespace Jint.Runtime.Interpreter.Expressions
 
             protected override object EvaluateInternal()
             {
+                var left = _left.GetValue();
+                var right = _right.GetValue();
+
                 if (_engine.Options._IsOperatorOverloadingAllowed
-                    && TryOperatorOverloading(OperatorClrName, out var opResult))
+                    && TryOperatorOverloading(left, right, OperatorClrName, out var opResult))
                 {
                     return opResult;
                 }
 
-                var left = _left.GetValue();
-                var right = _right.GetValue();
-
                 if (AreIntegerOperands(left, right))
                 {
                     int leftValue = left.AsInteger();

Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs

@@ -53,50 +53,54 @@ namespace Jint.Runtime.Interpreter.Expressions
 
         protected override object EvaluateInternal()
         {
-            if (_engine.Options._IsOperatorOverloadingAllowed)
-            {
-                string operatorClrName = null;
-                switch (_operator)
-                {
-                    case UnaryOperator.Plus:
-                        operatorClrName = "op_UnaryPlus";
-                        break;
-                    case UnaryOperator.Minus:
-                        operatorClrName = "op_UnaryNegation";
-                        break;
-                    case UnaryOperator.BitwiseNot:
-                        operatorClrName = "op_OnesComplement";
-                        break;
-                    case UnaryOperator.LogicalNot:
-                        operatorClrName = "op_LogicalNot";
-                        break;
-                    default:
-                        break;
-                }
-
-                if (operatorClrName != null &&
-                    TryOperatorOverloading(_engine, _argument.GetValue(), operatorClrName, out var result))
-                {
-                    return result;
-                }
-            }
-
             switch (_operator)
             {
                 case UnaryOperator.Plus:
-                    var plusValue = _argument.GetValue();
-                    return plusValue.IsInteger() && plusValue.AsInteger() != 0
-                        ? plusValue
-                        : JsNumber.Create(TypeConverter.ToNumber(plusValue));
+                {
+                    var v = _argument.GetValue();
+                    if (_engine.Options._IsOperatorOverloadingAllowed &&
+                        TryOperatorOverloading(_engine, v, "op_UnaryPlus", out var result))
+                    {
+                        return result;
+                    }
 
+                    return v.IsInteger() && v.AsInteger() != 0
+                        ? v
+                        : JsNumber.Create(TypeConverter.ToNumber(v));
+                }
                 case UnaryOperator.Minus:
-                    return EvaluateMinus(_argument.GetValue());
+                {
+                    var v = _argument.GetValue();
+                    if (_engine.Options._IsOperatorOverloadingAllowed &&
+                        TryOperatorOverloading(_engine, v, "op_UnaryNegation", out var result))
+                    {
+                        return result;
+                    }
 
+                    return EvaluateMinus(v);
+                }
                 case UnaryOperator.BitwiseNot:
-                    return JsNumber.Create(~TypeConverter.ToInt32(_argument.GetValue()));
+                {
+                    var v = _argument.GetValue();
+                    if (_engine.Options._IsOperatorOverloadingAllowed &&
+                        TryOperatorOverloading(_engine, v, "op_OnesComplement", out var result))
+                    {
+                        return result;
+                    }
 
+                    return JsNumber.Create(~TypeConverter.ToInt32(v));
+                }
                 case UnaryOperator.LogicalNot:
-                    return !TypeConverter.ToBoolean(_argument.GetValue()) ? JsBoolean.True : JsBoolean.False;
+                {
+                    var v = _argument.GetValue();
+                    if (_engine.Options._IsOperatorOverloadingAllowed &&
+                        TryOperatorOverloading(_engine, v, "op_LogicalNot", out var result))
+                    {
+                        return result;
+                    }
+
+                    return !TypeConverter.ToBoolean(v) ? JsBoolean.True : JsBoolean.False;
+                }
 
                 case UnaryOperator.Delete:
                     var r = _argument.Evaluate() as Reference;
@@ -150,6 +154,7 @@ namespace Jint.Runtime.Interpreter.Expressions
                     return Undefined.Instance;
 
                 case UnaryOperator.TypeOf:
+                {
                     var value = _argument.Evaluate();
                     r = value as Reference;
                     if (r != null)
@@ -161,8 +166,8 @@ namespace Jint.Runtime.Interpreter.Expressions
                         }
                     }
 
+                    // TODO: double evaluation problem
                     var v = _argument.GetValue();
-
                     if (v.IsUndefined())
                     {
                         return JsString.UndefinedString;
@@ -187,7 +192,7 @@ namespace Jint.Runtime.Interpreter.Expressions
                     }
 
                     return JsString.ObjectString;
-
+                }
                 default:
                     ExceptionHelper.ThrowArgumentException();
                     return null;