From b10261b44445c59b94a4cc08024414051608e93d Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 29 Jun 2022 09:34:29 +0200 Subject: [PATCH 1/8] Introduce own `Call` class for less tree calls --- library/Icinga/Less/Call.php | 70 ++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 library/Icinga/Less/Call.php diff --git a/library/Icinga/Less/Call.php b/library/Icinga/Less/Call.php new file mode 100644 index 000000000..bb5139d14 --- /dev/null +++ b/library/Icinga/Less/Call.php @@ -0,0 +1,70 @@ +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) { + $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 (array_reverse($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); + } +} From 7ac2dccbd13ac96b21978830913fcb4a2981a525 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 29 Jun 2022 09:35:22 +0200 Subject: [PATCH 2/8] Introduce `DeferredColorProp` class --- library/Icinga/Less/DeferredColorProp.php | 133 ++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 library/Icinga/Less/DeferredColorProp.php diff --git a/library/Icinga/Less/DeferredColorProp.php b/library/Icinga/Less/DeferredColorProp.php new file mode 100644 index 000000000..7c2468f0b --- /dev/null +++ b/library/Icinga/Less/DeferredColorProp.php @@ -0,0 +1,133 @@ +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) { + throw new Less_Exception_Compiler( + "Recursive variable definition for " . $this->name, + null, + $this->index, + $this->currentFileInfo + ); + } + + $this->evaluating = true; + + foreach (array_reverse($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); + } +} From a2932bd5ce66f558d98ae820fe8cb3f6a1705a27 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 29 Jun 2022 09:36:00 +0200 Subject: [PATCH 3/8] Visitor: Transform less tree calls & variable definitions into our own classes --- library/Icinga/Less/Visitor.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/library/Icinga/Less/Visitor.php b/library/Icinga/Less/Visitor.php index 87036bef5..030ea7bc9 100644 --- a/library/Icinga/Less/Visitor.php +++ b/library/Icinga/Less/Visitor.php @@ -4,7 +4,10 @@ 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; @@ -68,6 +71,10 @@ CSS; } $this->callingVar = spl_object_hash($c); + } else { + // We need to use our own tree call class , so that we can precompile the arguments before making + // the actual LESS function calls. Otherwise, it will produce lots of invalid argument exceptions! + $c = Call::fromCall($c); } return $c; @@ -136,6 +143,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; @@ -186,6 +202,15 @@ CSS; ->setVariable($v); } + public function visitColor($c) + { + if ($this->definingVariable !== false) { + $c->name = $this->variableOrigin->name; + } + + return $c; + } + public function run($node) { $this->lightMode = new LightMode(); From 5a04480245708e34a8824ffc528032502bfc1959 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 29 Jun 2022 09:42:53 +0200 Subject: [PATCH 4/8] Check for deferred color prop when defining variable variable & some fixlets for naming issue --- library/Icinga/Less/Call.php | 2 +- library/Icinga/Less/ColorProp.php | 10 +++++++++- library/Icinga/Less/ColorPropOrVariable.php | 9 +++++++-- library/Icinga/Less/DeferredColorProp.php | 5 +++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/library/Icinga/Less/Call.php b/library/Icinga/Less/Call.php index bb5139d14..0e6074dbe 100644 --- a/library/Icinga/Less/Call.php +++ b/library/Icinga/Less/Call.php @@ -34,7 +34,7 @@ class Call extends Less_Tree_Call } if ($name) { - foreach (array_reverse($env->frames) as $frame) { + 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; diff --git a/library/Icinga/Less/ColorProp.php b/library/Icinga/Less/ColorProp.php index f10115d75..b6fe6d2e2 100644 --- a/library/Icinga/Less/ColorProp.php +++ b/library/Icinga/Less/ColorProp.php @@ -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; diff --git a/library/Icinga/Less/ColorPropOrVariable.php b/library/Icinga/Less/ColorPropOrVariable.php index 371cca47f..8920999bb 100644 --- a/library/Icinga/Less/ColorPropOrVariable.php +++ b/library/Icinga/Less/ColorPropOrVariable.php @@ -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; diff --git a/library/Icinga/Less/DeferredColorProp.php b/library/Icinga/Less/DeferredColorProp.php index 7c2468f0b..becb04251 100644 --- a/library/Icinga/Less/DeferredColorProp.php +++ b/library/Icinga/Less/DeferredColorProp.php @@ -5,6 +5,7 @@ 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; @@ -89,7 +90,7 @@ class DeferredColorProp extends Less_Tree_Variable $this->evaluating = true; - foreach (array_reverse($env->frames) as $frame) { + foreach ($env->frames as $frame) { if (($v = $frame->variable($this->getRef()->name))) { $rv = $v->value; if ($rv instanceof Less_Tree_Value) { @@ -122,7 +123,7 @@ class DeferredColorProp extends Less_Tree_Variable $css = (new Less_Tree_Call( 'var', [ - new \Less_Tree_Keyword('--' . $this->getName()), + new Less_Tree_Keyword('--' . $this->getName()), $this->getRef() // Each of the references will be generated recursively ], $this->index From 1ec6913a0429304f3500b343ded31a45d610bb11 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 29 Jun 2022 09:45:51 +0200 Subject: [PATCH 5/8] Tests: Add less parser nested variables test cases --- .../library/Icinga/Util/LessParserTest.php | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/php/library/Icinga/Util/LessParserTest.php b/test/php/library/Icinga/Util/LessParserTest.php index e32e32056..fe09bb8d5 100644 --- a/test/php/library/Icinga/Util/LessParserTest.php +++ b/test/php/library/Icinga/Util/LessParserTest.php @@ -60,6 +60,59 @@ LESS ); } + public function testNestedVariables() + { + $this->assertEquals( + <<compileLess(<<assertEquals( + <<compileLess(<<assertEquals( From e45c53ac3c42d63dbeac3d993b4096d6ade43233 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 29 Jun 2022 11:39:39 +0200 Subject: [PATCH 6/8] Test nested LESS variables in minin calls --- .../library/Icinga/Util/LessParserTest.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/php/library/Icinga/Util/LessParserTest.php b/test/php/library/Icinga/Util/LessParserTest.php index fe09bb8d5..7f36c52cb 100644 --- a/test/php/library/Icinga/Util/LessParserTest.php +++ b/test/php/library/Icinga/Util/LessParserTest.php @@ -90,6 +90,40 @@ LESS ); } + public function testNestedVariablesInMixinCalls() + { + $this->assertEquals( + <<compileLess(<<assertEquals( From 9ac1a00e942f037bd26f7150c7649bd02f29254a Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 29 Jun 2022 15:51:46 +0200 Subject: [PATCH 7/8] Remove some unused codes & adjust the license headers --- library/Icinga/Less/Call.php | 3 +++ library/Icinga/Less/ColorProp.php | 2 +- library/Icinga/Less/ColorPropOrVariable.php | 2 +- library/Icinga/Less/DeferredColorProp.php | 4 +++- library/Icinga/Less/LightMode.php | 2 +- library/Icinga/Less/LightModeCall.php | 2 +- library/Icinga/Less/LightModeDefinition.php | 2 +- library/Icinga/Less/LightModeTrait.php | 2 +- library/Icinga/Less/LightModeVisitor.php | 2 +- library/Icinga/Less/Visitor.php | 20 ++++---------------- 10 files changed, 17 insertions(+), 24 deletions(-) diff --git a/library/Icinga/Less/Call.php b/library/Icinga/Less/Call.php index 0e6074dbe..449ede622 100644 --- a/library/Icinga/Less/Call.php +++ b/library/Icinga/Less/Call.php @@ -1,5 +1,7 @@ value[0] = $vr; + break; } } diff --git a/library/Icinga/Less/ColorProp.php b/library/Icinga/Less/ColorProp.php index b6fe6d2e2..3f83c5eee 100644 --- a/library/Icinga/Less/ColorProp.php +++ b/library/Icinga/Less/ColorProp.php @@ -1,5 +1,5 @@ evaluating) { + if ($this->evaluating) { // Just like the parent method throw new Less_Exception_Compiler( "Recursive variable definition for " . $this->name, null, diff --git a/library/Icinga/Less/LightMode.php b/library/Icinga/Less/LightMode.php index f12cc03bf..b4b72a074 100644 --- a/library/Icinga/Less/LightMode.php +++ b/library/Icinga/Less/LightMode.php @@ -1,5 +1,5 @@ name === 'var') { - if ($this->callingVar !== false) { - throw new LogicException('Already calling var'); - } - - $this->callingVar = spl_object_hash($c); - } else { + if ($c->name !== 'var') { // We need to use our own tree call class , so that we can precompile the arguments before making // the actual LESS function calls. Otherwise, it will produce lots of invalid argument exceptions! $c = Call::fromCall($c); @@ -80,13 +74,6 @@ CSS; 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) { @@ -194,7 +181,7 @@ CSS; public function visitVariable($v) { - if ($this->callingVar !== false || $this->definingVariable !== false) { + if ($this->definingVariable !== false) { return $v; } @@ -205,6 +192,7 @@ CSS; 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; } From c29ac9842d2d602184e266bb02e76050cf27894d Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 30 Jun 2022 12:03:48 +0200 Subject: [PATCH 8/8] Call: Don't precompile arg values when it's not an array The CSS/LESS callable argument values may not always be an array, but also an object or whateever, in this case we don't need to precompile the values as they could never be a variable. --- library/Icinga/Less/Call.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/Icinga/Less/Call.php b/library/Icinga/Less/Call.php index 449ede622..0a78cb541 100644 --- a/library/Icinga/Less/Call.php +++ b/library/Icinga/Less/Call.php @@ -24,6 +24,10 @@ class Call extends Less_Tree_Call } 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..