Code quality and documentation

library/Icinga/Less/ColorProp.php

@@ -6,16 +6,21 @@ use Less_Tree_Call;
 use Less_Tree_Color;
 use Less_Tree_Keyword;
 
+/**
+ * ColorProp renders Less colors as CSS var() function calls
+ *
+ * It extends {@link Less_Tree_Color} so that Less functions that take a Less_Tree_Color as an argument do not fail.
+ */
 class ColorProp extends Less_Tree_Color
 {
-    /** @var Less_Tree_Color */
+    /** @var Less_Tree_Color Color with which we created the ColorProp */
     protected $color;
 
     /** @var int */
     protected $index;
 
-    /** @var string */
+    /** @var string Color variable name */
-    protected $origin;
+    protected $name;
 
     public function __construct()
     {
@@ -24,11 +29,11 @@ class ColorProp extends Less_Tree_Color
     /**
      * @param Less_Tree_Color $color
      *
-     * @return self
+     * @return static
      */
     public static function fromColor(Less_Tree_Color $color)
     {
-        $self = new self();
+        $self = new static();
         $self->color = $color;
 
         foreach ($color as $k => $v) {
@@ -61,34 +66,30 @@ class ColorProp extends Less_Tree_Color
     /**
      * @return string
      */
-    public function getOrigin()
+    public function getName()
     {
-        return $this->origin;
+        return $this->name;
     }
 
     /**
-     * @param string $origin
+     * @param string $name
      *
      * @return $this
      */
-    public function setOrigin($origin)
+    public function setName($name)
     {
-        $this->origin = $origin;
+        $this->name = $name;
 
         return $this;
     }
 
-    public function compile()
-    {
-        return $this;
-    }
-
     public function genCSS($output)
     {
         $css = (new Less_Tree_Call(
             'var',
             [
-                new Less_Tree_Keyword('--' . $this->getOrigin()),
+                new Less_Tree_Keyword('--' . $this->getName()),
+                // Use the Less_Tree_Color with which we created the ColorProp so that we don't get into genCSS() loops.
                 $this->color
             ],
             $this->getIndex()

library/Icinga/Less/ColorPropOrVariable.php

@@ -6,6 +6,9 @@ use Less_Tree;
 use Less_Tree_Color;
 use Less_Tree_Variable;
 
+/**
+ * Compile a Less variable to {@link ColorProp} if it is a color
+ */
 class ColorPropOrVariable extends Less_Tree
 {
     public $type = 'Variable';
@@ -38,9 +41,9 @@ class ColorPropOrVariable extends Less_Tree
         $v = $this->getVariable();
 
         if ($v->name[1] === '@') {
-            // Evaluate variable variable as in Less_Tree_Variable:28
+            // 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
+            // Overwrite the name so that the variable variable is not evaluated again.
             $v->name = '@' . $vv->compile($env)->value;
         }
 
@@ -54,7 +57,7 @@ class ColorPropOrVariable extends Less_Tree
         if ($compiled instanceof Less_Tree_Color) {
             return ColorProp::fromColor($compiled)
                 ->setIndex($v->index)
-                ->setOrigin(substr($v->name, 1));
+                ->setName(substr($v->name, 1));
         }
 
         return $compiled;

library/Icinga/Less/Visitor.php

@@ -7,6 +7,9 @@ use LogicException;
 
 /**
  * Replace compiled Less colors with CSS var() function calls
+ *
+ * This basically works by replacing every visited Less variable with {@link ColorPropOrVariable},
+ * which is later compiled to {@link ColorProp} if it is a color.
  */
 class Visitor extends Less_VisitorReplacing
 {
@@ -21,16 +24,6 @@ class Visitor extends Less_VisitorReplacing
      */
     protected $callingVar = false;
 
-    /**
-     * Whether defining a mixin
-     *
-     * If that's the case, don't try to replace compiled Less colors with CSS var() function calls.
-     * This is handled explicitly.
-     *
-     * @var bool|string
-     */
-    protected $definingMixin = false;
-
     /**
      * Whether defining a variable
      *
@@ -71,6 +64,7 @@ class Visitor extends Less_VisitorReplacing
 
     public function visitMixinCall($c)
     {
+        // Less_Tree_Mixin_Call::accept() does not visit arguments, but we have to replace them if necessary.
         foreach ($c->arguments as $a) {
             $a['value'] = $this->visitObj($a['value']);
         }
@@ -80,10 +74,7 @@ class Visitor extends Less_VisitorReplacing
 
     public function visitMixinDefinition($m)
     {
-        if ($this->definingMixin !== false) {
+        // Less_Tree_Mixin_Definition::accept() does not visit params, but we have to replace them if necessary.
-            throw new LogicException('Already defining mixin');
-        }
-
         foreach ($m->params as $p) {
             if (! isset($p['value'])) {
                 continue;
@@ -92,20 +83,9 @@ class Visitor extends Less_VisitorReplacing
             $p['value'] = $this->visitObj($p['value']);
         }
 
-        $m->rules = $this->visitArray($m->rules);
-
-        $this->definingMixin = spl_object_hash($m);
-
         return $m;
     }
 
-    public function visitMixinDefinitionOut($m)
-    {
-        if ($this->definingMixin !== false && $this->definingMixin === spl_object_hash($m)) {
-            $this->definingMixin = false;
-        }
-    }
-
     public function visitRule($r)
     {
         if ($r->name[0] === '@' && $r->variable) {
@@ -128,7 +108,7 @@ class Visitor extends Less_VisitorReplacing
 
     public function visitVariable($v)
     {
-        if ($this->callingVar !== false || $this->definingMixin !== false || $this->definingVar !== false) {
+        if ($this->callingVar !== false || $this->definingVar !== false) {
             return $v;
         }