Merge pull request #4835 from Icinga/less-wip

Fix light mode variable references resolution issue

library/Icinga/Less/Call.php

@@ -0,0 +1,77 @@
+<?php
+
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
+
+namespace Icinga\Less;
+
+use Less_Tree_Call;
+use Less_Tree_Color;
+use Less_Tree_Value;
+use Less_Tree_Variable;
+
+class Call extends Less_Tree_Call
+{
+    public static function fromCall(Less_Tree_Call $call)
+    {
+        return new static($call->name, $call->args, $call->index, $call->currentFileInfo);
+    }
+
+    public function compile($env = null)
+    {
+        if (! $env) {
+            // Not sure how to trigger this, but if there is no $env, there is nothing we can do
+            return parent::compile($env);
+        }
+
+        foreach ($this->args as $arg) {
+            if (! is_array($arg->value)) {
+                continue;
+            }
+
+            $name = null;
+            if ($arg->value[0] instanceof Less_Tree_Variable) {
+                // This is the case when defining a variable with a callable LESS rules such as fade, fadeout..
+                // Example: `@foo: #fff; @foo-bar: fade(@foo, 10);`
+                $name = $arg->value[0]->name;
+            } elseif ($arg->value[0] instanceof ColorPropOrVariable) {
+                // This is the case when defining a CSS rule using the LESS functions and passing
+                // a variable as an argument to them. Example: `... { color: fade(@foo, 10%); }`
+                $name = $arg->value[0]->getVariable()->name;
+            }
+
+            if ($name) {
+                foreach ($env->frames as $frame) {
+                    if (($v = $frame->variable($name))) {
+                        // Variables from the frame stack are always of type LESS Tree Rule
+                        $vr = $v->value;
+                        if ($vr instanceof Less_Tree_Value) {
+                            // Get the actual color prop, otherwise this may cause an invalid argument error
+                            $vr = $vr->compile($env);
+                        }
+
+                        if ($vr instanceof DeferredColorProp) {
+                            if (! $vr->hasReference()) {
+                                // Should never happen, though just for safety's sake
+                                $vr->compile($env);
+                            }
+
+                            // Get the uppermost variable of the variable references
+                            while (! $vr instanceof ColorProp) {
+                                $vr = $vr->getRef();
+                            }
+                        } elseif ($vr instanceof Less_Tree_Color) {
+                            $vr = ColorProp::fromColor($vr);
+                            $vr->setName($name);
+                        }
+
+                        $arg->value[0] = $vr;
+
+                        break;
+                    }
+                }
+            }
+        }
+
+        return parent::compile($env);
+    }
+}

library/Icinga/Less/ColorProp.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 
@@ -38,7 +38,11 @@ class ColorProp extends Less_Tree_Color
         $self->color = $color;
 
         foreach ($color as $k => $v) {
-            $self->$k = $v;
+            if ($k === 'name') {
+                $self->setName($v); // Removes the @ char from the name
+            } else {
+                $self->$k = $v;
+            }
         }
 
         return $self;
@@ -79,6 +83,10 @@ class ColorProp extends Less_Tree_Color
      */
     public function setName($name)
     {
+        if ($name[0] === '@') {
+            $name = substr($name, 1);
+        }
+
         $this->name = $name;
 
         return $this;

library/Icinga/Less/ColorPropOrVariable.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 
@@ -45,7 +45,12 @@ class ColorPropOrVariable extends Less_Tree
             // Evaluate variable variable as in Less_Tree_Variable:28.
             $vv = new Less_Tree_Variable(substr($v->name, 1), $v->index + 1, $v->currentFileInfo);
             // Overwrite the name so that the variable variable is not evaluated again.
-            $v->name = '@' . $vv->compile($env)->value;
+            $result = $vv->compile($env);
+            if ($result instanceof DeferredColorProp) {
+                $v->name = $result->name;
+            } else {
+                $v->name = '@' . $result->value;
+            }
         }
 
         $compiled = $v->compile($env);
@@ -58,7 +63,7 @@ class ColorPropOrVariable extends Less_Tree
         if ($compiled instanceof Less_Tree_Color) {
             return ColorProp::fromColor($compiled)
                 ->setIndex($v->index)
-                ->setName(substr($v->name, 1));
+                ->setName($v->name);
         }
 
         return $compiled;

library/Icinga/Less/DeferredColorProp.php

@@ -0,0 +1,136 @@
+<?php
+
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
+
+namespace Icinga\Less;
+
+use Less_Exception_Compiler;
+use Less_Tree_Call;
+use Less_Tree_Color;
+use Less_Tree_Keyword;
+use Less_Tree_Value;
+use Less_Tree_Variable;
+
+class DeferredColorProp extends Less_Tree_Variable
+{
+    /** @var DeferredColorProp|ColorProp */
+    protected $reference;
+
+    protected $resolved = false;
+
+    public function __construct($name, $variable, $index = null, $currentFileInfo = null)
+    {
+        parent::__construct($name, $index, $currentFileInfo);
+
+        if ($variable instanceof Less_Tree_Variable) {
+            $this->reference = self::fromVariable($variable);
+        }
+    }
+
+    public function isResolved()
+    {
+        return $this->resolved;
+    }
+
+    public function getName()
+    {
+        $name = $this->name;
+        if ($this->name[0] === '@') {
+            $name = substr($this->name, 1);
+        }
+
+        return $name;
+    }
+
+    public function hasReference()
+    {
+        return $this->reference !== null;
+    }
+
+    public function getRef()
+    {
+        return $this->reference;
+    }
+
+    public function setReference($ref)
+    {
+        $this->reference = $ref;
+
+        return $this;
+    }
+
+    public static function fromVariable(Less_Tree_Variable $variable)
+    {
+        $static = new static($variable->name, $variable->index, $variable->currentFileInfo);
+        $static->evaluating = $variable->evaluating;
+        $static->type = $variable->type;
+
+        return $static;
+    }
+
+    public function compile($env)
+    {
+        if (! $this->hasReference()) {
+            // This is never supposed to happen, however, we might have a deferred color prop
+            // without a reference. In this case we can simply use the parent method.
+            return parent::compile($env);
+        }
+
+        if ($this->isResolved()) {
+            // The dependencies are already resolved, no need to traverse the frame stack over again!
+            return $this;
+        }
+
+        if ($this->evaluating) { // Just like the parent method
+            throw new Less_Exception_Compiler(
+                "Recursive variable definition for " . $this->name,
+                null,
+                $this->index,
+                $this->currentFileInfo
+            );
+        }
+
+        $this->evaluating = true;
+
+        foreach ($env->frames as $frame) {
+            if (($v = $frame->variable($this->getRef()->name))) {
+                $rv = $v->value;
+                if ($rv instanceof Less_Tree_Value) {
+                    $rv = $rv->compile($env);
+                }
+
+                // As we are at it anyway, let's cast the tree color to our color prop as well!
+                if ($rv instanceof Less_Tree_Color) {
+                    $rv = ColorProp::fromColor($rv);
+                    $rv->setName($this->getRef()->getName());
+                }
+
+                $this->evaluating = false;
+                $this->resolved = true;
+                $this->setReference($rv);
+
+                break;
+            }
+        }
+
+        return $this;
+    }
+
+    public function genCSS($output)
+    {
+        if (! $this->hasReference()) {
+            return; // Nothing to generate
+        }
+
+        $css = (new Less_Tree_Call(
+            'var',
+            [
+                new Less_Tree_Keyword('--' . $this->getName()),
+                $this->getRef() // Each of the references will be generated recursively
+            ],
+            $this->index
+        ))->toCSS();
+
+        $output->add($css);
+    }
+}

library/Icinga/Less/LightMode.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 

library/Icinga/Less/LightModeCall.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 

library/Icinga/Less/LightModeDefinition.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 

library/Icinga/Less/LightModeTrait.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 

library/Icinga/Less/LightModeVisitor.php

@@ -1,5 +1,5 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 

library/Icinga/Less/Visitor.php

@@ -1,10 +1,13 @@
 <?php
-/* Icinga Web 2 | (c) 2022 Icinga Development Team | GPLv2+ */
+/* Icinga Web 2 | (c) 2022 Icinga GmbH | GPLv2+ */
 
 namespace Icinga\Less;
 
 use Less_Parser;
+use Less_Tree_Expression;
 use Less_Tree_Rule;
+use Less_Tree_Value;
+use Less_Tree_Variable;
 use Less_VisitorReplacing;
 use LogicException;
 use ReflectionProperty;
@@ -62,24 +65,15 @@ CSS;
 
     public function visitCall($c)
     {
-        if ($c->name === 'var') {
+        if ($c->name !== 'var') {
-            if ($this->callingVar !== false) {
+            // We need to use our own tree call class , so that we can precompile the arguments before making
-                throw new LogicException('Already calling var');
+            // the actual LESS function calls. Otherwise, it will produce lots of invalid argument exceptions!
-            }
+            $c = Call::fromCall($c);
-
-            $this->callingVar = spl_object_hash($c);
         }
 
         return $c;
     }
 
-    public function visitCallOut($c)
-    {
-        if ($this->callingVar !== false && $this->callingVar === spl_object_hash($c)) {
-            $this->callingVar = false;
-        }
-    }
-
     public function visitDetachedRuleset($drs)
     {
         if ($this->variableOrigin->name === '@' . static::LIGHT_MODE_NAME) {
@@ -136,6 +130,15 @@ CSS;
 
             $this->definingVariable = spl_object_hash($r);
             $this->variableOrigin = $r;
+
+            if ($r->value instanceof Less_Tree_Value) {
+                if ($r->value->value[0] instanceof Less_Tree_Expression) {
+                    if ($r->value->value[0]->value[0] instanceof Less_Tree_Variable) {
+                        // Transform the variable definition rule into our own class
+                        $r->value->value[0]->value[0] = new DeferredColorProp($r->name, $r->value->value[0]->value[0]);
+                    }
+                }
+            }
         }
 
         return $r;
@@ -178,7 +181,7 @@ CSS;
 
     public function visitVariable($v)
     {
-        if ($this->callingVar !== false || $this->definingVariable !== false) {
+        if ($this->definingVariable !== false) {
             return $v;
         }
 
@@ -186,6 +189,16 @@ CSS;
             ->setVariable($v);
     }
 
+    public function visitColor($c)
+    {
+        if ($this->definingVariable !== false) {
+            // Make sure that all less tree colors do have a proper name
+            $c->name = $this->variableOrigin->name;
+        }
+
+        return $c;
+    }
+
     public function run($node)
     {
         $this->lightMode = new LightMode();

test/php/library/Icinga/Util/LessParserTest.php

@@ -60,6 +60,93 @@ LESS
         );
     }
 
+    public function testNestedVariables()
+    {
+        $this->assertEquals(
+            <<<CSS
+.black {
+  color: var(--my-color, var(--black, #000000));
+}
+.notBlack {
+  color: var(--my-black-color, var(--my-color, var(--black, #000000)));
+}
+
+CSS
+            ,
+            $this->compileLess(<<<LESS
+@black: black;
+@my-color: @black;
+@my-black-color: @my-color;
+
+.black {
+  color: @my-color;
+}
+
+.notBlack {
+  color: @my-black-color;
+}
+LESS
+            )
+        );
+    }
+
+    public function testNestedVariablesInMixinCalls()
+    {
+        $this->assertEquals(
+            <<<CSS
+.button1 {
+  background-color: var(--my-color, var(--black, #000000));
+}
+.button2 {
+  background-color: var(--my-black-color, var(--my-color, var(--black, #000000)));
+}
+
+CSS
+            ,
+            $this->compileLess(<<<LESS
+@black: black;
+@my-color: @black;
+@my-black-color: @my-color;
+
+.button(@bg-color: @my-color) {
+  background-color: @bg-color;
+}
+
+.button1 {
+  .button();
+}
+
+.button2 {
+  .button(@my-black-color)
+}
+LESS
+            )
+        );
+    }
+
+    public function testDefiningVariablesWithLessCallables()
+    {
+        $this->assertEquals(
+            <<<CSS
+.my-rule {
+  color: var(--fade-color, rgba(221, 221, 221, 0.5));
+}
+
+CSS
+            ,
+            $this->compileLess(<<<LESS
+@color: #ddd;
+@my-color: @color;
+@fade-color: fade(@my-color, 50%);
+
+.my-rule {
+    color: @fade-color;
+}
+LESS
+            )
+        );
+    }
+
     public function testVariablesUsedInFunctions()
     {
         $this->assertEquals(