From 18007437bd2985e41a43c55580fe95ee1f37f12a Mon Sep 17 00:00:00 2001
From: Matthew Dean <matthew-dean@users.noreply.github.com>
Date: Mon, 6 Jul 2020 20:44:32 -0700
Subject: [PATCH] Fixes #3371 Allow conditional evaluation of function args

---
 packages/less/src/less/functions/boolean.js   | 11 +++++++---
 .../src/less/functions/function-caller.js     |  8 +++++++
 packages/less/src/less/tree/call.js           | 22 +++++++++++++------
 packages/test-data/css/_main/functions.css    |  1 +
 .../errors/eval/recursive-variable.txt        |  2 +-
 packages/test-data/less/_main/functions.less  |  5 +++++
 6 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/packages/less/src/less/functions/boolean.js b/packages/less/src/less/functions/boolean.js
index 97b514a93..981fce8c8 100644
--- a/packages/less/src/less/functions/boolean.js
+++ b/packages/less/src/less/functions/boolean.js
@@ -5,9 +5,14 @@ function boolean(condition) {
     return condition ? Keyword.True : Keyword.False;
 }
 
-function If(condition, trueValue, falseValue) {
-    return condition ? trueValue
-        : (falseValue || new Anonymous);
+/**
+ * Functions with evalArgs set to false are sent context
+ * as the first argument.
+ */
+function If(context, condition, trueValue, falseValue) {
+    return condition.eval(context) ? trueValue.eval(context)
+        : (falseValue ? falseValue.eval(context) : new Anonymous);
 }
+If.evalArgs = false
 
 export default { boolean, 'if': If };
diff --git a/packages/less/src/less/functions/function-caller.js b/packages/less/src/less/functions/function-caller.js
index 76b4c413c..346ab82e4 100644
--- a/packages/less/src/less/functions/function-caller.js
+++ b/packages/less/src/less/functions/function-caller.js
@@ -15,6 +15,10 @@ class functionCaller {
     }
 
     call(args) {
+        const evalArgs = this.func.evalArgs;
+        if (evalArgs !== false) {
+            args = args.map(a => a.eval(this.context));
+        }
         // This code is terrible and should be replaced as per this issue...
         // https://github.com/less/less.js/issues/2477
         if (Array.isArray(args)) {
@@ -42,6 +46,10 @@ class functionCaller {
                 });
         }
 
+        if (evalArgs === false) {
+            return this.func(this.context, ...args);
+        }
+
         return this.func(...args);
     }
 }
diff --git a/packages/less/src/less/tree/call.js b/packages/less/src/less/tree/call.js
index eb9b161d1..f94af57fa 100644
--- a/packages/less/src/less/tree/call.js
+++ b/packages/less/src/less/tree/call.js
@@ -42,19 +42,25 @@ class Call extends Node {
         if (this.calc || context.inCalc) {
             context.enterCalc();
         }
-        const args = this.args.map(a => a.eval(context));
-        if (this.calc || context.inCalc) {
-            context.exitCalc();
-        }
-        context.mathOn = currentMathContext;
+
+        const exitCalc = () => {
+            if (this.calc || context.inCalc) {
+                context.exitCalc();
+            }
+            context.mathOn = currentMathContext;
+        };
 
         let result;
         const funcCaller = new FunctionCaller(this.name, context, this.getIndex(), this.fileInfo());
 
         if (funcCaller.isValid()) {
             try {
-                result = funcCaller.call(args);
+                result = funcCaller.call(this.args);
+                exitCalc();
             } catch (e) {
+                if (e.hasOwnProperty('line') && e.hasOwnProperty('column')) {
+                    throw e
+                }
                 throw { 
                     type: e.type || 'Runtime',
                     message: `error evaluating function \`${this.name}\`${e.message ? `: ${e.message}` : ''}`,
@@ -81,9 +87,11 @@ class Call extends Node {
                 result._fileInfo = this._fileInfo;
                 return result;
             }
-
         }
 
+        const args = this.args.map(a => a.eval(context));
+        exitCalc();
+
         return new Call(this.name, args, this.getIndex(), this.fileInfo());
     }
 
diff --git a/packages/test-data/css/_main/functions.css b/packages/test-data/css/_main/functions.css
index c2fdb6162..99323620e 100644
--- a/packages/test-data/css/_main/functions.css
+++ b/packages/test-data/css/_main/functions.css
@@ -227,6 +227,7 @@ html {
   i: 6;
   j: 8;
   k: 1;
+  l: black;
   /* results in void */
   color: green;
   color: purple;
diff --git a/packages/test-data/errors/eval/recursive-variable.txt b/packages/test-data/errors/eval/recursive-variable.txt
index eb616e7d2..b8bd6fdd9 100644
--- a/packages/test-data/errors/eval/recursive-variable.txt
+++ b/packages/test-data/errors/eval/recursive-variable.txt
@@ -1,2 +1,2 @@
-NameError: Recursive variable definition for @bodyColor in {path}recursive-variable.less on line 1, column 20:
+NameError: error evaluating function `darken`: Recursive variable definition for @bodyColor in {path}recursive-variable.less on line 1, column 13:
 1 @bodyColor: darken(@bodyColor, 30%);
diff --git a/packages/test-data/less/_main/functions.less b/packages/test-data/less/_main/functions.less
index 8057a6649..f6db3b41c 100644
--- a/packages/test-data/less/_main/functions.less
+++ b/packages/test-data/less/_main/functions.less
@@ -263,6 +263,11 @@ html {
   j: if(not(true) and true, 6, 8);
   k: if(true or true, 1);
 
+  // see: https://github.com/less/less.js/issues/3371
+  @some: foo;
+  l: if((iscolor(@some)), darken(@some, 10%), black);
+  
+
   if((false), {g: 7}); /* results in void */
 
   @conditional: if((true), {